Re: [Fwd: Re: proposal: new long psql parameter --on-error-stop] - Mailing list pgsql-hackers

On Mon, Jun 23, 2014 at 5:10 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> Hello
>
>
> 2014-06-23 10:02 GMT+02:00 Fujii Masao <masao.fujii@gmail.com>:
>
>> On Mon, Jun 23, 2014 at 12:04 AM, Pavel Stehule <pavel.stehule@gmail.com>
>> wrote:
>> > Hello
>> >
>> > third version with Erik's update
>>
>> Here are some my comments:
>>
>> The document of psql needs to be updated. At least the description of new
>> option
>> this patch adds needs to be added into the document.
>>
>> +    printf(_("  --help-variables         list of available
>> configuration variables (options), then exit\n"));
>>
>> We should get rid of "of" from the message, or add "show" in front of
>> "list of"?
>>
>> +    printf(_("  ECHO               write all input lines to standard
>> output\n"));
>>
>> This message seems not to be correct when ECHO=queries is set.
>>
>> +    printf(_("  COMP_KEYWORD_CASE  determines which letter case to
>> use when completing an SQL key word\n"));
>> +    printf(_("  DBNAME             name of currently connected
>> database\n"));
>> +    printf(_("  ECHO               write all input lines to standard
>> output\n"));
>>
>> I found that some help message line uses a normal form of a verb, but
>> other does not.
>> We should standardize them?
>>
>> +    printf(_("  PROMPT1, PROMPT2, PROMPT3  specify the psql prompt\n"));
>>
>> When the option name field is long, we should add a new line just
>> after the name field
>> and align the starting position of the option explanation field. That is,
>> for example, the above should be
>>
>> printf(_("  PROMPT1, PROMPT2, PROMPT3\n"
>>              "                     specify the psql prompt\n"));
>>
>> +    printf(_("  ON_ERROR_ROLLBACK  when on, ROLLBACK on error\n"));
>>
>> This message seems incorrect to me. When this option is on and an error
>> occurs
>> in transaction, transaction continues rather than ROLLBACK occurs, IIUC.
>> I did not check whole help messages yet, but ISTM some messages are not
>> correct.
>> It's better to check them again.
>>
>> +    printf(_("  PSQL_RC            alternative location of the user's
>> .psqlrc file\n"));
>>
>> Typo: PSQL_RC should be PSQLRC
>>
>> +    printf(_("  PGDATABASE         same as the dbname connection
>> parameter\n"));
>> +    printf(_("  PGHOST             same as the host connection
>> parameter\n"));
>> +        printf(_("  PGPORT             same as the port connection
>> parameter\n"));
>> +    printf(_("  PGUSER             same as the user connection
>> parameter\n"));
>> +    printf(_("  PGPASSWORD         possibility to set password (not
>> recommended)\n"));
>> +    printf(_("  PGPASSFILE         password file (default
>> ~/.pgpass)\n"));
>>
>> I don't think that psql needs to display the help messages of even
>> environment
>> variables supported by libpq.
>>
>
> Main reason is a PGPASSWORD -- it is probably most used env variable with
> psql
>
> PGPASSWORD=****** psql is very often used pattern

But it's not recommended as the help message which the patch added says ;)

Regards,

-- 
Fujii Masao