Thread: Proposal: adding a better description in psql command about large objects
Hello, Attached is a small patch to add a description to the meta commands regarding large objects. the actual description when using psql --help=commands is : Large Objects \lo_export LOBOID FILE \lo_import FILE [COMMENT] \lo_list \lo_unlink LOBOID large object operations the proposed description is : Large Objects \lo_export LOBOID FILE export large object to a file \lo_import FILE [COMMENT] import large object from a file \lo_list list large objects \lo_unlink LOBOID delete a large object I tried to make an alignment on the description of other meta-commands. Thanks. Regards. -- Thibaud W.
Attachment
Re: Proposal: adding a better description in psql command about large objects
From
Nathan Bossart
Date:
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
Attachment
Re: Proposal: adding a better description in psql command about large objects
From
"Thibaud W."
Date:
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.
Attachment
Re: Proposal: adding a better description in psql command about large objects
From
Nathan Bossart
Date:
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
Attachment
Nathan Bossart <nathandbossart@gmail.com> 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
From
Nathan Bossart
Date:
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
Nathan Bossart <nathandbossart@gmail.com> 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] NAME prompt user to set internal variable\n")); fprintf(output, _(" \\set [NAME [VALUE]] set internal variable, or list all if no parameters\n")); fprintf(output, _(" \\unset NAME unset (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
From
Nathan Bossart
Date:
On Fri, Jun 03, 2022 at 12:56:20PM -0400, Tom Lane wrote: > Nathan Bossart <nathandbossart@gmail.com> 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
From
Guillaume Lelarge
Date:
Le ven. 3 juin 2022 à 19:29, Nathan Bossart <nathandbossart@gmail.com> a écrit :
On Fri, Jun 03, 2022 at 12:56:20PM -0400, Tom Lane wrote:
> Nathan Bossart <nathandbossart@gmail.com> 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
From
"Thibaud W."
Date:
On 6/3/22 19:29, Nathan Bossart wrote: > On Fri, Jun 03, 2022 at 12:56:20PM -0400, Tom Lane wrote: >> Nathan Bossart <nathandbossart@gmail.com> 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.