Re: Proposal: adding a better description in psql command about large objects

2022-06-07 Thread Thibaud W.

On 6/3/22 19:29, Nathan Bossart wrote:

On Fri, Jun 03, 2022 at 12:56:20PM -0400, Tom Lane wrote:

Nathan Bossart  writes:

Another option could be to move it after the "Input/Output" section so that
it's closer to some other commands that involve files.  I can't say I have
a strong opinion about whether/where to move it, though.

Yeah, I thought of that choice too, but it ends up placing the
Large Objects section higher up the list than seems warranted on
frequency-of-use grounds.

Fair point.


After looking at the output I concluded that we'd be better off to
stick with the normal indentation amount, and break the lo_import
entry into two lines to make that work.  One reason for this is
that some translators might've already settled on a different
indentation amount in order to cope with translated parameter names,
and deviating from the normal here will just complicate their lives.
So that leaves me proposing v5.

I see.  As you noted earlier, moving the entries higher makes the
inconsistent indentation less appealing, too.  So this LGTM.


(I also fixed the out-of-date line count in helpVariables.)

Yeah, it looks like 7844c99 missed this.

Thanks, output is more readable this way.

Best regards.
--
Thibaud W.




Re: Proposal: adding a better description in psql command about large objects

2022-06-05 Thread Guillaume Lelarge
Le ven. 3 juin 2022 à 19:29, Nathan Bossart  a
écrit :

> On Fri, Jun 03, 2022 at 12:56:20PM -0400, Tom Lane wrote:
> > Nathan Bossart  writes:
> >> Another option could be to move it after the "Input/Output" section so
> that
> >> it's closer to some other commands that involve files.  I can't say I
> have
> >> a strong opinion about whether/where to move it, though.
> >
> > Yeah, I thought of that choice too, but it ends up placing the
> > Large Objects section higher up the list than seems warranted on
> > frequency-of-use grounds.
>
> Fair point.
>
> > After looking at the output I concluded that we'd be better off to
> > stick with the normal indentation amount, and break the lo_import
> > entry into two lines to make that work.  One reason for this is
> > that some translators might've already settled on a different
> > indentation amount in order to cope with translated parameter names,
> > and deviating from the normal here will just complicate their lives.
> > So that leaves me proposing v5.
>
> I see.  As you noted earlier, moving the entries higher makes the
> inconsistent indentation less appealing, too.  So this LGTM.
>
>
Sounds good to me too.

Thanks.


-- 
Guillaume.


Re: Proposal: adding a better description in psql command about large objects

2022-06-03 Thread Nathan Bossart
On Fri, Jun 03, 2022 at 12:56:20PM -0400, Tom Lane wrote:
> Nathan Bossart  writes:
>> Another option could be to move it after the "Input/Output" section so that
>> it's closer to some other commands that involve files.  I can't say I have
>> a strong opinion about whether/where to move it, though.
> 
> Yeah, I thought of that choice too, but it ends up placing the
> Large Objects section higher up the list than seems warranted on
> frequency-of-use grounds.

Fair point.

> After looking at the output I concluded that we'd be better off to
> stick with the normal indentation amount, and break the lo_import
> entry into two lines to make that work.  One reason for this is
> that some translators might've already settled on a different
> indentation amount in order to cope with translated parameter names,
> and deviating from the normal here will just complicate their lives.
> So that leaves me proposing v5.

I see.  As you noted earlier, moving the entries higher makes the
inconsistent indentation less appealing, too.  So this LGTM.

> (I also fixed the out-of-date line count in helpVariables.)

Yeah, it looks like 7844c99 missed this.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com




Re: Proposal: adding a better description in psql command about large objects

2022-06-03 Thread Tom Lane
Nathan Bossart  writes:
> On Fri, Jun 03, 2022 at 11:12:11AM -0400, Tom Lane wrote:
>> * While we're here, it seems like this whole group was placed at the
>> end because of add-it-to-the-end-itis, not because that was the
>> most logical place for it.  The other commands that interact with
>> the server are mostly further up.  My first thought is to move it
>> to just after the "Informational" group, but I'm not especially
>> set on that.  Making it not-last might make it harder to get away
>> with the inconsistent indentation, though.

> Another option could be to move it after the "Input/Output" section so that
> it's closer to some other commands that involve files.  I can't say I have
> a strong opinion about whether/where to move it, though.

Yeah, I thought of that choice too, but it ends up placing the
Large Objects section higher up the list than seems warranted on
frequency-of-use grounds.

After looking at the output I concluded that we'd be better off to
stick with the normal indentation amount, and break the lo_import
entry into two lines to make that work.  One reason for this is
that some translators might've already settled on a different
indentation amount in order to cope with translated parameter names,
and deviating from the normal here will just complicate their lives.
So that leaves me proposing v5.

(I also fixed the out-of-date line count in helpVariables.)

regards, tom lane

diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index 49eb116f33..81a4739cda 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -163,7 +163,7 @@ slashUsage(unsigned short int pager)
 	 * Use "psql --help=commands | wc" to count correctly.  It's okay to count
 	 * the USE_READLINE line even in builds without that.
 	 */
-	output = PageOutput(138, pager ? &(pset.popt.topt) : NULL);
+	output = PageOutput(139, pager ? &(pset.popt.topt) : NULL);
 
 	fprintf(output, _("General\n"));
 	fprintf(output, _("  \\copyright show PostgreSQL usage and distribution terms\n"));
@@ -272,6 +272,14 @@ slashUsage(unsigned short int pager)
 	fprintf(output, _("  \\z  [PATTERN]  same as \\dp\n"));
 	fprintf(output, "\n");
 
+	fprintf(output, _("Large Objects\n"));
+	fprintf(output, _("  \\lo_export LOBOID FILE write large object to file\n"));
+	fprintf(output, _("  \\lo_import FILE [COMMENT]\n"
+	  " read large object from file\n"));
+	fprintf(output, _("  \\lo_list[+]list large objects\n"));
+	fprintf(output, _("  \\lo_unlink LOBOID  delete a large object\n"));
+	fprintf(output, "\n");
+
 	fprintf(output, _("Formatting\n"));
 	fprintf(output, _("  \\a toggle between unaligned and aligned output mode\n"));
 	fprintf(output, _("  \\C [STRING]set table title, or unset if none\n"));
@@ -318,13 +326,6 @@ slashUsage(unsigned short int pager)
 	fprintf(output, _("  \\prompt [TEXT] NAMEprompt user to set internal variable\n"));
 	fprintf(output, _("  \\set [NAME [VALUE]]set internal variable, or list all if no parameters\n"));
 	fprintf(output, _("  \\unset NAMEunset (delete) internal variable\n"));
-	fprintf(output, "\n");
-
-	fprintf(output, _("Large Objects\n"));
-	fprintf(output, _("  \\lo_export LOBOID FILE\n"
-	  "  \\lo_import FILE [COMMENT]\n"
-	  "  \\lo_list[+]\n"
-	  "  \\lo_unlink LOBOID  large object operations\n"));
 
 	ClosePager(output);
 }
@@ -346,7 +347,7 @@ helpVariables(unsigned short int pager)
 	 * Windows builds currently print one fewer line than non-Windows builds.
 	 * Using the larger number is fine.
 	 */
-	output = PageOutput(161, pager ? &(pset.popt.topt) : NULL);
+	output = PageOutput(163, pager ? &(pset.popt.topt) : NULL);
 
 	fprintf(output, _("List of specially treated variables\n\n"));
 


Re: Proposal: adding a better description in psql command about large objects

2022-06-03 Thread Nathan Bossart
On Fri, Jun 03, 2022 at 11:12:11AM -0400, Tom Lane wrote:
> * How about "write large object to file" and "read large object from
> file"?  As it stands, if you are not totally sure which direction is
> export and which is import, this description teaches you little.

+1

> * While we're here, it seems like this whole group was placed at the
> end because of add-it-to-the-end-itis, not because that was the
> most logical place for it.  The other commands that interact with
> the server are mostly further up.  My first thought is to move it
> to just after the "Informational" group, but I'm not especially
> set on that.  Making it not-last might make it harder to get away
> with the inconsistent indentation, though.

Another option could be to move it after the "Input/Output" section so that
it's closer to some other commands that involve files.  I can't say I have
a strong opinion about whether/where to move it, though.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com




Re: Proposal: adding a better description in psql command about large objects

2022-06-03 Thread Tom Lane
Nathan Bossart  writes:
> Yes, it looks like the precedent is to have an fprintf() per command.  I
> still think the indentation needs some adjustment for readability.  In the
> attached, I've lined up all the large object commands.  This is offset from
> most other commands, but IMO this is far easier to read, and something
> similar was done for the operator class/family commands.  Thoughts?

Generally +1 here.  The other style that is used in some places is to
put the description on a separate line, but given that we're setting
the indent for a whole command group I think this looks better.

A couple of other random thoughts:

* How about "write large object to file" and "read large object from
file"?  As it stands, if you are not totally sure which direction is
export and which is import, this description teaches you little.

* While we're here, it seems like this whole group was placed at the
end because of add-it-to-the-end-itis, not because that was the
most logical place for it.  The other commands that interact with
the server are mostly further up.  My first thought is to move it
to just after the "Informational" group, but I'm not especially
set on that.  Making it not-last might make it harder to get away
with the inconsistent indentation, though.

regards, tom lane




Re: Proposal: adding a better description in psql command about large objects

2022-06-03 Thread Nathan Bossart
On Fri, Jun 03, 2022 at 10:12:30AM +0200, Thibaud W. wrote:
> In fact the original tabs were missing in the first file.
> In version v2, it seems interesting to keep calls to the fprintf function
> for translation. I attached a new file.

Yes, it looks like the precedent is to have an fprintf() per command.  I
still think the indentation needs some adjustment for readability.  In the
attached, I've lined up all the large object commands.  This is offset from
most other commands, but IMO this is far easier to read, and something
similar was done for the operator class/family commands.  Thoughts?

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
>From 046e450e6d578fe365ef6c561a477d70d09f3076 Mon Sep 17 00:00:00 2001
From: Nathan Bossart 
Date: Thu, 2 Jun 2022 14:35:31 -0700
Subject: [PATCH v4 1/1] Add descriptions for psql's large object backslash
 commands.

These should be mostly self-explanatory, but they are the only
backslash commands lacking individual short descriptions.

Author: Thibaud W.
Reviewed by: Nathan Bossart
Description: https://postgr.es/m/43f0439c-df3e-a045-ac99-af33523cc2d4%40dalibo.com
---
 src/bin/psql/help.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index 49eb116f33..33902d496a 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -321,10 +321,10 @@ slashUsage(unsigned short int pager)
 	fprintf(output, "\n");
 
 	fprintf(output, _("Large Objects\n"));
-	fprintf(output, _("  \\lo_export LOBOID FILE\n"
-	  "  \\lo_import FILE [COMMENT]\n"
-	  "  \\lo_list[+]\n"
-	  "  \\lo_unlink LOBOID  large object operations\n"));
+	fprintf(output, _("  \\lo_export LOBOID FILE export large object to file\n"));
+	fprintf(output, _("  \\lo_import FILE [COMMENT]  import large object from file\n"));
+	fprintf(output, _("  \\lo_list[+]list large objects\n"));
+	fprintf(output, _("  \\lo_unlink LOBOID  delete a large object\n"));
 
 	ClosePager(output);
 }
-- 
2.25.1



Re: Proposal: adding a better description in psql command about large objects

2022-06-03 Thread Thibaud W.

On 6/2/22 23:46, Nathan Bossart wrote:

On Thu, Jun 02, 2022 at 11:12:46AM +0200, Thibaud W. wrote:

Attached is a small patch to add a description to the meta commands
regarding
large objects.

This seems reasonable to me.  Your patch wasn't applying for some reason,
so I created a new one with a commit message and some small adjustments.
What do you think?

Thanks for reading and fixing.

In fact the original tabs were missing in the first file.
In version v2, it seems interesting to keep calls to the fprintf 
function for translation. I attached a new file.


Thanks.
Regards.
--
Thibaud W.diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index 49eb116f33..b7dbd9f7f2 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -321,10 +321,10 @@ slashUsage(unsigned short int pager)
 	fprintf(output, "\n");
 
 	fprintf(output, _("Large Objects\n"));
-	fprintf(output, _("  \\lo_export LOBOID FILE\n"
-	  "  \\lo_import FILE [COMMENT]\n"
-	  "  \\lo_list[+]\n"
-	  "  \\lo_unlink LOBOID  large object operations\n"));
+	fprintf(output, _("  \\lo_export LOBOID FILE export large object to a file\n"));
+	fprintf(output, _("  \\lo_import FILE [COMMENT]import large object from a file\n"));
+	fprintf(output, _("  \\lo_list[+]list large objects\n"));
+	fprintf(output, _("  \\lo_unlink LOBOID  delete a large object\n"));
 
 	ClosePager(output);
 }


Re: Proposal: adding a better description in psql command about large objects

2022-06-02 Thread Nathan Bossart
On Thu, Jun 02, 2022 at 11:12:46AM +0200, Thibaud W. wrote:
> Attached is a small patch to add a description to the meta commands
> regarding
> large objects.

This seems reasonable to me.  Your patch wasn't applying for some reason,
so I created a new one with a commit message and some small adjustments.
What do you think?

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
>From ca23b5c3b1ad3af4c47bb6d438c58a6e3ba5a1b6 Mon Sep 17 00:00:00 2001
From: Nathan Bossart 
Date: Thu, 2 Jun 2022 14:35:31 -0700
Subject: [PATCH v2 1/1] Add descriptions for psql's large object backslash
 commands.

These should be mostly self-explanatory, but they are the only
backslash commands lacking individual short descriptions.

Author: Thibaud W.
Reviewed by: Nathan Bossart
Description: https://postgr.es/m/43f0439c-df3e-a045-ac99-af33523cc2d4%40dalibo.com
---
 src/bin/psql/help.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index 49eb116f33..54580ac928 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -321,10 +321,10 @@ slashUsage(unsigned short int pager)
 	fprintf(output, "\n");
 
 	fprintf(output, _("Large Objects\n"));
-	fprintf(output, _("  \\lo_export LOBOID FILE\n"
-	  "  \\lo_import FILE [COMMENT]\n"
-	  "  \\lo_list[+]\n"
-	  "  \\lo_unlink LOBOID  large object operations\n"));
+	fprintf(output, _("  \\lo_export LOBOID FILE export large object to file\n"
+	  "  \\lo_import FILE [COMMENT]  import large object from file\n"
+	  "  \\lo_list[+]list large objects\n"
+	  "  \\lo_unlink LOBOID  delete a large object\n"));
 
 	ClosePager(output);
 }
-- 
2.25.1