Re: [Fwd: Re: proposal: new long psql parameter --on-error-stop]

On Mon, Jun 23, 2014 at 5:10 PM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
> Hello
>
>
> 2014-06-23 10:02 GMT+02:00 Fujii Masao <masao(dot)fujii(at)gmail(dot)com>:
>
>> On Mon, Jun 23, 2014 at 12:04 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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