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

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.

Regards,

--
Fujii Masao

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

Regards

Pavel Stehule

Show quoted text

Regards,

--
Fujii Masao

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

2014-06-23 10:57 GMT+02:00 Fujii Masao <masao.fujii@gmail.com>:

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 ;)

why?

who can see this value?

regards

Pavel

Show quoted text

Regards,

--
Fujii Masao

On Mon, Jun 23, 2014 at 6:06 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:

2014-06-23 10:57 GMT+02:00 Fujii Masao <masao.fujii@gmail.com>:

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 ;)

why?

who can see this value?

I'm sure that the document explains this.

http://www.postgresql.org/docs/devel/static/libpq-envars.html
---------------------------------------
PGPASSWORD behaves the same as the password connection parameter.
Use of this environment variable is not recommended for security reasons,
as some operating systems allow non-root users to see process environment
variables via ps; instead consider using the ~/.pgpass file
---------------------------------------

Regards,

--
Fujii Masao

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

2014-06-23 11:53 GMT+02:00 Fujii Masao <masao.fujii@gmail.com>:

On Mon, Jun 23, 2014 at 6:06 PM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

2014-06-23 10:57 GMT+02:00 Fujii Masao <masao.fujii@gmail.com>:

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

;)

why?

who can see this value?

I'm sure that the document explains this.

ok

I am too Linux centrist :( it is safe there and I don't see a others

Thank you for info

Regards

Pavel

Show quoted text

http://www.postgresql.org/docs/devel/static/libpq-envars.html
---------------------------------------
PGPASSWORD behaves the same as the password connection parameter.
Use of this environment variable is not recommended for security reasons,
as some operating systems allow non-root users to see process environment
variables via ps; instead consider using the ~/.pgpass file
---------------------------------------

Regards,

--
Fujii Masao

Hello

I am sending little bit modified patch by Fujii' comments - but I am not
able to fix it more - it is task for someone with better English skill then
I have

Regards

Pavel

2014-06-23 10:02 GMT+02:00 Fujii Masao <masao.fujii@gmail.com>:

Show quoted text

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.

Regards,

--
Fujii Masao

OK, let me help you, though I'm only a Japanese who is never confident in my
English.

(1)
As Fujii-san pointed out, could you add explanation for --help-variables in
doc/src/sgml/ref/psqlref.sgml?

(2)
+ printf(_(" --help-variables list of available configuration
variables (options), then exit\n"));

should better be:

+ printf(_(" --help-variables show A list of all specially treated
variables, then exit\n"));

This follows the psql manual page. Similarly,

+ printf(_("List of variables (options) for use from command line.\n"));

should be:

+ printf(_("List of specially treated variables.\n"));

(3)
+ printf(_(" ECHO control what input can be writtent to
standard output [all, queries]\n"));

"writtent" should be "written". "controls" should be "control" like other
options.

(4)
+ printf(_(" ECHO_HIDDEN display internal queries (same as -E
option)\n"));

should better be:

+ printf(_(" ECHO_HIDDEN display internal queries executed by
backslash commands\n"));

I think "(same as ...)" can be omitted to keep the description short. If
you want to retain it, other variables should also accompany similar
description, such as -a for ECHO.

(5)
+ printf(_(" FETCH_COUNT fetch many rows at a time (use less memory)
(default 0 unlimited)\n"));

should better be:

+ printf(_(" FETCH_COUNT the number of result rows to fetch and
display at a time (default: 0=unlimited)\n"));

(6)
+ printf(_(" HISTCONTROL when set, controls history list
[ignorespace, ignoredups, ignoreboth]\n"));

should better be:

+ printf(_(" HISTCONTROL control history list [ignorespace,
ignoredups, ignoreboth]\n"));

(7)
+ printf(_(" USER the database user currently connected\n"));

should add "as" at the end:

+ printf(_(" USER the database user currently connected
as\n"));

(8)
"Printing options" section lack the following ones described in psql manual:

columns
expanded (or x)
footer
numericlocale
tableattr (or T)

(9)
+ printf(_("\nEnvironment options:\n"));

should be ""Environment variables". And this section lacks description for
Windows, such as:

+ printf(_("  NAME=VALUE [NAME=VALUE] psql ...\n  or \\setenv NAME [VALUE] 
in interactive mode\n\n"));
+ printf(_("  PGPASSFILE         password file (default ~/.pgpass)\n"));

Regards
MauMau

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hello

Here is next update

2014-06-25 17:17 GMT+02:00 MauMau <maumau307@gmail.com>:

OK, let me help you, though I'm only a Japanese who is never confident in
my English.

(1)
As Fujii-san pointed out, could you add explanation for --help-variables
in doc/src/sgml/ref/psqlref.sgml?

(2)

+ printf(_(" --help-variables list of available configuration
variables (options), then exit\n"));

should better be:

+ printf(_(" --help-variables show A list of all specially
treated variables, then exit\n"));

This follows the psql manual page. Similarly,

+ printf(_("List of variables (options) for use from command line.\n"));

should be:

+ printf(_("List of specially treated variables.\n"));

(3)
+ printf(_(" ECHO control what input can be writtent to
standard output [all, queries]\n"));

"writtent" should be "written". "controls" should be "control" like other
options.

(4)
+ printf(_(" ECHO_HIDDEN display internal queries (same as -E
option)\n"));

should better be:

+ printf(_(" ECHO_HIDDEN display internal queries executed by
backslash commands\n"));

I think "(same as ...)" can be omitted to keep the description short. If
you want to retain it, other variables should also accompany similar
description, such as -a for ECHO.

(5)
+ printf(_(" FETCH_COUNT fetch many rows at a time (use less
memory) (default 0 unlimited)\n"));

should better be:

+ printf(_(" FETCH_COUNT the number of result rows to fetch and
display at a time (default: 0=unlimited)\n"));

(6)
+ printf(_(" HISTCONTROL when set, controls history list
[ignorespace, ignoredups, ignoreboth]\n"));

should better be:

+ printf(_(" HISTCONTROL control history list [ignorespace,
ignoredups, ignoreboth]\n"));

(7)
+ printf(_(" USER the database user currently
connected\n"));

should add "as" at the end:

+ printf(_(" USER the database user currently connected
as\n"));

(8)
"Printing options" section lack the following ones described in psql
manual:

columns
expanded (or x)
footer
numericlocale
tableattr (or T)

fixed

(9)
+ printf(_("\nEnvironment options:\n"));

should be ""Environment variables". And this section lacks description
for Windows, such as:

+ printf(_(" NAME=VALUE [NAME=VALUE] psql ...\n or \\setenv NAME [VALUE]
in interactive mode\n\n"));

+ printf(_(" PGPASSFILE password file (default ~/.pgpass)\n"));

??? -

Regards

Pavel

Show quoted text

Regards
MauMau

Hello,

From: "Pavel Stehule" <pavel.stehule@gmail.com>

fixed

Thank you. All is fine.

should be ""Environment variables". And this section lacks description
for Windows, such as:

+ printf(_(" NAME=VALUE [NAME=VALUE] psql ...\n or \\setenv NAME
[VALUE]
in interactive mode\n\n"));

+ printf(_(" PGPASSFILE password file (default ~/.pgpass)\n"));

??? -

I meant that how to set environment variables on Windows command prompt is
different from on UNIX/Linux, and the default password file path is also
different on Windows. Do we describe them in this help?

Lastly, to follow most of your descriptions, "s" at the end of the first
verb in these messages should be removed. For example, use "set" instead of
"sets".

+ printf(_("  COMP_KEYWORD_CASE  determines which letter case to use when 
completing an SQL key word\n"));
+ printf(_("  columns            sets the target width for the wrapped 
format\n"));
+ printf(_("  linestyle          sets the border line drawing style [ascii, 
old-ascii, unicode]\n"));
+ printf(_("  recordsep          specifies the record (line) separator to 
use in unaligned output format\n"));
+ printf(_("  title              sets the table title for any subsequently 
printed tables\n"));

This is all I noticed in the review.

Regards
MauMau

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

2014-06-26 15:26 GMT+02:00 MauMau <maumau307@gmail.com>:

Hello,

From: "Pavel Stehule" <pavel.stehule@gmail.com>

fixed

Thank you. All is fine.

should be ""Environment variables". And this section lacks description

for Windows, such as:

+ printf(_(" NAME=VALUE [NAME=VALUE] psql ...\n or \\setenv NAME
[VALUE]
in interactive mode\n\n"));

+ printf(_(" PGPASSFILE password file (default ~/.pgpass)\n"));

??? -

I meant that how to set environment variables on Windows command prompt is
different from on UNIX/Linux, and the default password file path is also
different on Windows. Do we describe them in this help?

hmm, I'll check it

Pavel

Show quoted text

Lastly, to follow most of your descriptions, "s" at the end of the first
verb in these messages should be removed. For example, use "set" instead
of "sets".

+ printf(_("  COMP_KEYWORD_CASE  determines which letter case to use when
completing an SQL key word\n"));
+ printf(_("  columns            sets the target width for the wrapped
format\n"));
+ printf(_("  linestyle          sets the border line drawing style
[ascii, old-ascii, unicode]\n"));
+ printf(_("  recordsep          specifies the record (line) separator to
use in unaligned output format\n"));
+ printf(_("  title              sets the table title for any subsequently
printed tables\n"));

This is all I noticed in the review.

Regards
MauMau

Hello,

I went through the patch, seems mostly fine, I adjusted some wording,
removed the default .pgpass file info since it's not accurate, and
replaced couple of phrases with (hopefully) more informative ones.

--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Hello

thank you Peter, so now only setting for MS Windows is missing?

Regards

Pavel

2014-06-26 21:57 GMT+02:00 Petr Jelinek <petr@2ndquadrant.com>:

Show quoted text

Hello,

I went through the patch, seems mostly fine, I adjusted some wording,
removed the default .pgpass file info since it's not accurate, and replaced
couple of phrases with (hopefully) more informative ones.

--
Petr Jelinek http://www.2ndQuadrant.com/

PostgreSQL Development, 24x7 Support, Training & Services

Hello

I modified description of setting system variables in dependency on O.S.

Regards

Pavel

2014-06-27 8:54 GMT+02:00 Pavel Stehule <pavel.stehule@gmail.com>:

Show quoted text

Hello

thank you Peter, so now only setting for MS Windows is missing?

Regards

Pavel

2014-06-26 21:57 GMT+02:00 Petr Jelinek <petr@2ndquadrant.com>:

Hello,

I went through the patch, seems mostly fine, I adjusted some wording,
removed the default .pgpass file info since it's not accurate, and replaced
couple of phrases with (hopefully) more informative ones.

--
Petr Jelinek http://www.2ndQuadrant.com/

PostgreSQL Development, 24x7 Support, Training & Services

From: "Pavel Stehule" <pavel.stehule@gmail.com>

I modified description of setting system variables in dependency on O.S.

Thank you, it's almost OK. As mentioned in my previous mail, I think
"determines" should be "determine" to follow other messages. I'll mark this
patch as ready for committer when this is fixed.

+ printf(_(" COMP_KEYWORD_CASE determines which letter case to use when
completing an SQL key word\n"));

Personally, I don't think we have to describe how to set environment
variables, because it's preliminary knowledge and not specific to
PostgreSQL. However, I don't mind if you retain or remove the description.

Regards
MauMau

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hi

2014-06-29 0:48 GMT+02:00 MauMau <maumau307@gmail.com>:

From: "Pavel Stehule" <pavel.stehule@gmail.com>

I modified description of setting system variables in dependency on O.S.

Thank you, it's almost OK. As mentioned in my previous mail, I think
"determines" should be "determine" to follow other messages. I'll mark
this patch as ready for committer when this is fixed.

fixes

+ printf(_(" COMP_KEYWORD_CASE determines which letter case to use when
completing an SQL key word\n"));

Personally, I don't think we have to describe how to set environment
variables, because it's preliminary knowledge and not specific to
PostgreSQL. However, I don't mind if you retain or remove the description.

ok

Regards

Pavel

Show quoted text

Regards
MauMau

Thanks, I marked it as ready for committer. I hope Fujii san or another
committer will commit this, refining English expression if necessary.

Regards
MauMau

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

2014-06-29 13:35 GMT+02:00 MauMau <maumau307@gmail.com>:

Thanks, I marked it as ready for committer. I hope Fujii san or another
committer will commit this, refining English expression if necessary.

sure

Thank you very much

Regards

Pavel

Show quoted text

Regards
MauMau

At 2014-06-29 20:35:04 +0900, maumau307@gmail.com wrote:

Thanks, I marked it as ready for committer. I hope Fujii san or
another committer will commit this, refining English expression if
necessary.

Since it was just a matter of editing, I went through the patch and
corrected various minor errors (typos, awkwardness, etc.). I agree
that this is now ready for committer.

I've attached the updated diff.

-- Abhijit