Horton

Thread

Commits

  1. Add test for checking the line length of --help output

  2. Remove environment-variable-based defaults in psql --help

  1. Make --help output fit within 80 columns per line

    torikoshia <torikoshia@oss.nttdata.com> — 2023-07-05T01:47:19Z

    Hi,

As discussed in [1], outputs of --help for some commands fits into 80 
columns
per line, while others do not.

Since it seems preferable to have consistent line break policy and some 
people
use 80-column terminal, wouldn't it be better to make all commands in 80
columns per line?

Attached patch which does this for src/bin commands.

If this is the way to go, I'll do same things for contrib commands.

[1] 
https://www.postgresql.org/message-id/3fe4af5a0a81fc6a2ec01cb484c0a487%40oss.nttdata.com


-- 
Regards,

--
Atsushi Torikoshi
NTT DATA CORPORATION

  2. Re: Make --help output fit within 80 columns per line

    Masahiro Ikeda <ikedamsh@oss.nttdata.com> — 2023-08-21T04:08:58Z

    On 2023-07-05 10:47, torikoshia wrote:
> Hi,
> 
> As discussed in [1], outputs of --help for some commands fits into 80 
> columns
> per line, while others do not.
> 
> Since it seems preferable to have consistent line break policy and some 
> people
> use 80-column terminal, wouldn't it be better to make all commands in 
> 80
> columns per line?
> 
> Attached patch which does this for src/bin commands.
> 
> If this is the way to go, I'll do same things for contrib commands.
> 
> [1] 
> https://www.postgresql.org/message-id/3fe4af5a0a81fc6a2ec01cb484c0a487%40oss.nttdata.com

Thanks for making the patches! I have some comments to v1 patch.

(1)

Why don't you add test for the purpose? It could be overkill...
I though the following function is the best place.

diff --git a/src/test/perl/PostgreSQL/Test/Utils.pm 
b/src/test/perl/PostgreSQL/Test/Utils.pm
index 617caa022f..1bdb81ac56 100644
--- a/src/test/perl/PostgreSQL/Test/Utils.pm
+++ b/src/test/perl/PostgreSQL/Test/Utils.pm
@@ -843,6 +843,10 @@ sub program_help_ok
         ok($result, "$cmd --help exit code 0");
         isnt($stdout, '', "$cmd --help goes to stdout");
         is($stderr, '', "$cmd --help nothing to stderr");
+       foreach my $line (split /\n/, $stdout)
+       {
+               ok(length($line) <= 80, "$cmd --help output fit within 
80 columns per line");
+       }
         return;
  }

(2)

Is there any reason that only src/bin commands are targeted? I found 
that
we also need to fix vacuumlo with the above test. I think it's better to
fix it because it's a contrib module.

$ vacuumlo --help | while IFS='' read line; do echo $((`echo $line | wc 
-m` - 1)) $line; done | sort -n -r  | head -n 2
84   -n, --dry-run             don't remove large objects, just show 
what would be done
74   -l, --limit=LIMIT         commit after removing each LIMIT large 
objects

(3)

Is to delete '/mnt/server' intended?  I though it better to leave it as
is since archive_cleanup_command example uses the absolute path.

-			 "  pg_archivecleanup /mnt/server/archiverdir 
000000010000000000000010.00000020.backup\n"));
+			 "  pg_archivecleanup archiverdir 
000000010000000000000010.00000020.backup\n"));

I will confirmed that the --help text are not changed and only
the line breaks are changed.  But, currently the above change
break it.

(4)

I found that some binaries, for example ecpg, are not tested with
program_help_ok(). Is it better to add tests in the patch?

BTW, I check the difference with the following commands
# files include "--help"
$ find -name "*.c" | xargs -I {} sh -c 'if [ `grep -e --help {} | wc -l` 
-gt 0 ]; then echo {}; fi'

# programs which is tested with program_help_ok
$ find -name "*.pl" | xargs -I {} sh -c 'grep -e program_help_ok {}'

Regards,
-- 
Masahiro Ikeda
NTT DATA CORPORATION

  3. Re: Make --help output fit within 80 columns per line

    torikoshia <torikoshia@oss.nttdata.com> — 2023-08-22T13:57:07Z

    On 2023-08-21 13:08, Masahiro Ikeda wrote:
Thanks for your review!

> (1)
> 
> Why don't you add test for the purpose? It could be overkill...
> I though the following function is the best place.
> 
> diff --git a/src/test/perl/PostgreSQL/Test/Utils.pm
> b/src/test/perl/PostgreSQL/Test/Utils.pm
> index 617caa022f..1bdb81ac56 100644
> --- a/src/test/perl/PostgreSQL/Test/Utils.pm
> +++ b/src/test/perl/PostgreSQL/Test/Utils.pm
> @@ -843,6 +843,10 @@ sub program_help_ok
>         ok($result, "$cmd --help exit code 0");
>         isnt($stdout, '', "$cmd --help goes to stdout");
>         is($stderr, '', "$cmd --help nothing to stderr");
> +       foreach my $line (split /\n/, $stdout)
> +       {
> +               ok(length($line) <= 80, "$cmd --help output fit within
> 80 columns per line");
> +       }
>         return;
>  }

Agreed.

> (2)
> 
> Is there any reason that only src/bin commands are targeted? I found 
> that
> we also need to fix vacuumlo with the above test. I think it's better 
> to
> fix it because it's a contrib module.
> 
> $ vacuumlo --help | while IFS='' read line; do echo $((`echo $line |
> wc -m` - 1)) $line; done | sort -n -r  | head -n 2
> 84   -n, --dry-run             don't remove large objects, just show
> what would be done
> 74   -l, --limit=LIMIT         commit after removing each LIMIT large 
> objects

This is because I wasn't sure making all --help outputs fit into 80 
columns per line is right thing to do as described below:

| If this is the way to go, I'll do same things for contrib commands.

If there are no objection, I'm going to make other commands fit within 
80 columns per line including (4).

> (3)
> 
> Is to delete '/mnt/server' intended?  I though it better to leave it as
> is since archive_cleanup_command example uses the absolute path.
> 
> -			 "  pg_archivecleanup /mnt/server/archiverdir
> 000000010000000000000010.00000020.backup\n"));
> +			 "  pg_archivecleanup archiverdir
> 000000010000000000000010.00000020.backup\n"));
> 
> I will confirmed that the --help text are not changed and only
> the line breaks are changed.  But, currently the above change
> break it.

Yes, it is intended as described in the thread.

https://www.postgresql.org/message-id/20230615.152036.1556630042388070221.horikyota.ntt%40gmail.com

| We could shorten it by removing the "/mnt/server" portion, but
I'm not sure if it's worth doing.

However, I feel it is acceptable to make an exception and exceed 80 
characters for this line.

> (4)
> 
> I found that some binaries, for example ecpg, are not tested with
> program_help_ok(). Is it better to add tests in the patch?
> 
Agreed.

> BTW, I check the difference with the following commands
> # files include "--help"
> $ find -name "*.c" | xargs -I {} sh -c 'if [ `grep -e --help {} | wc
> -l` -gt 0 ]; then echo {}; fi'
> 
> # programs which is tested with program_help_ok
> $ find -name "*.pl" | xargs -I {} sh -c 'grep -e program_help_ok {}'

Thanks for sharing your procedure!

-- 
Regards,

--
Atsushi Torikoshi
NTT DATA Group Corporation

  4. Re: Make --help output fit within 80 columns per line

    Masahiro Ikeda <ikedamsh@oss.nttdata.com> — 2023-08-23T00:45:50Z

    Hi,

On 2023-08-22 22:57, torikoshia wrote:
> On 2023-08-21 13:08, Masahiro Ikeda wrote:
>> (2)
>> 
>> Is there any reason that only src/bin commands are targeted? I found 
>> that
>> we also need to fix vacuumlo with the above test. I think it's better 
>> to
>> fix it because it's a contrib module.
>> 
>> $ vacuumlo --help | while IFS='' read line; do echo $((`echo $line |
>> wc -m` - 1)) $line; done | sort -n -r  | head -n 2
>> 84   -n, --dry-run             don't remove large objects, just show
>> what would be done
>> 74   -l, --limit=LIMIT         commit after removing each LIMIT large 
>> objects
> 
> This is because I wasn't sure making all --help outputs fit into 80
> columns per line is right thing to do as described below:
> 
> | If this is the way to go, I'll do same things for contrib commands.
> 
> If there are no objection, I'm going to make other commands fit within
> 80 columns per line including (4).

OK. Sorry, I missed the sentence above.
I'd like to hear what others comment too.

>> (3)
>> 
>> Is to delete '/mnt/server' intended?  I though it better to leave it 
>> as
>> is since archive_cleanup_command example uses the absolute path.
>> 
>> -			 "  pg_archivecleanup /mnt/server/archiverdir
>> 000000010000000000000010.00000020.backup\n"));
>> +			 "  pg_archivecleanup archiverdir
>> 000000010000000000000010.00000020.backup\n"));
>> 
>> I will confirmed that the --help text are not changed and only
>> the line breaks are changed.  But, currently the above change
>> break it.
> 
> Yes, it is intended as described in the thread.
> 
> https://www.postgresql.org/message-id/20230615.152036.1556630042388070221.horikyota.ntt%40gmail.com
> 
> | We could shorten it by removing the "/mnt/server" portion, but
> I'm not sure if it's worth doing.
> 
> However, I feel it is acceptable to make an exception and exceed 80
> characters for this line.

Thanks for sharing the thread. I understood.

It seems that the directory name should be consistent with the example
of archive_cleanup_command. However, it does not seem appropriate to
make archive_cleanup_command to use a relative path.

```
> pg_archivecleanup --help
(snip)
e.g.
   archive_cleanup_command = 'pg_archivecleanup /mnt/server/archiverdir 
%r'

Or for use as a standalone archive cleaner:
e.g.
   pg_archivecleanup /mnt/server/archiverdir 
000000010000000000000010.00000020.backup
```

IMHO, is simply breaking the line acceptable?

```
Or for use as a standalone archive cleaner:
e.g.
   pg_archivecleanup /mnt/server/archiverdir
       000000010000000000000010.00000020.backup
```

Regards,
-- 
Masahiro Ikeda
NTT DATA CORPORATION

  5. Re: Make --help output fit within 80 columns per line

    torikoshia <torikoshia@oss.nttdata.com> — 2023-08-31T07:47:21Z

    On Mon, Aug 21, 2023 at 1:09 PM Masahiro Ikeda 
<ikedamsh@oss.nttdata.com> wrote:
> (1)
> Why don't you add test for the purpose? It could be overkill...
> I though the following function is the best place.

Added the test.

BTW, psql --help outputs the content of PGHOST, which caused a failure 
in the test:

```
-h, --host=HOSTNAME      database server host or socket directory
                          (default: 
"/var/folders/m7/9snkd5b54cx_b4lxkl9ljlcc0000gn/T/LobrmSUf7t")
```

It may be overkill, added a logic for removing the content of PGHOST.


On 2023-08-23 09:45, Masahiro Ikeda wrote:
> Hi,
> 
> On 2023-08-22 22:57, torikoshia wrote:
>> On 2023-08-21 13:08, Masahiro Ikeda wrote:
>>> (2)
>>> 
>>> Is there any reason that only src/bin commands are targeted? I found 
>>> that
>>> we also need to fix vacuumlo with the above test. I think it's better 
>>> to
>>> fix it because it's a contrib module.
>>> 
>>> $ vacuumlo --help | while IFS='' read line; do echo $((`echo $line |
>>> wc -m` - 1)) $line; done | sort -n -r  | head -n 2
>>> 84   -n, --dry-run             don't remove large objects, just show
>>> what would be done
>>> 74   -l, --limit=LIMIT         commit after removing each LIMIT large 
>>> objects
>> 
>> This is because I wasn't sure making all --help outputs fit into 80
>> columns per line is right thing to do as described below:
>> 
>> | If this is the way to go, I'll do same things for contrib commands.
>> 
>> If there are no objection, I'm going to make other commands fit within
>> 80 columns per line including (4).
> 
> OK. Sorry, I missed the sentence above.
> I'd like to hear what others comment too.

Although there are no comments, attached patch modifies vaccumlo.

>>> (3)
>>> 
>>> Is to delete '/mnt/server' intended?  I though it better to leave it 
>>> as
>>> is since archive_cleanup_command example uses the absolute path.
>>> 
>>> -			 "  pg_archivecleanup /mnt/server/archiverdir
>>> 000000010000000000000010.00000020.backup\n"));
>>> +			 "  pg_archivecleanup archiverdir
>>> 000000010000000000000010.00000020.backup\n"));
>>> 
>>> I will confirmed that the --help text are not changed and only
>>> the line breaks are changed.  But, currently the above change
>>> break it.
>> 
>> Yes, it is intended as described in the thread.
>> 
>> https://www.postgresql.org/message-id/20230615.152036.1556630042388070221.horikyota.ntt%40gmail.com
>> 
>> | We could shorten it by removing the "/mnt/server" portion, but
>> I'm not sure if it's worth doing.
>> 
>> However, I feel it is acceptable to make an exception and exceed 80
>> characters for this line.
> 
> Thanks for sharing the thread. I understood.
> 
> It seems that the directory name should be consistent with the example
> of archive_cleanup_command. However, it does not seem appropriate to
> make archive_cleanup_command to use a relative path.
> 
> ```
>> pg_archivecleanup --help
> (snip)
> e.g.
>   archive_cleanup_command = 'pg_archivecleanup /mnt/server/archiverdir 
> %r'
> 
> Or for use as a standalone archive cleaner:
> e.g.
>   pg_archivecleanup /mnt/server/archiverdir
> 000000010000000000000010.00000020.backup
> ```
> 
> IMHO, is simply breaking the line acceptable?

Agreed.


> (4)

> I found that some binaries, for example ecpg, are not tested with
> program_help_ok(). Is it better to add tests in the patch?

Added program_help_ok() to ecpg and pgbench.
Although pg_regress and zic are not tested using program_help_ok, but 
left as they are because they are not commands that users execute 
directly.

-- 
Regards,

--
Atsushi Torikoshi
NTT DATA Group Corporation

  6. Re: Make --help output fit within 80 columns per line

    Peter Eisentraut <peter@eisentraut.org> — 2023-09-12T06:27:35Z

    I like this work a lot.  It's good to give developers easy to verify 
guidance about formatting the --help messages.

However, I think instead of just adding a bunch of line breaks to 
satisfy the test, we should really try to make the lines shorter by 
rewording.  Otherwise, chances are we are making the usability worse for 
many users, because it's more likely that part of the help will scroll 
off the screen.  For example, in many cases, we could replace "do not" 
by "don't", or we could decrease the indentation of the second column by 
a few spaces, or just reword altogether.

Also, it would be very useful if the TAP test function could print out 
the violating lines if a test fails.  (Similar to how is() and like() 
print the failing values.)  Maybe start with that, and then it's easier 
to play with different wording variants to try to make it fit better.

  7. Re: Make --help output fit within 80 columns per line

    torikoshia <torikoshia@oss.nttdata.com> — 2023-09-13T12:40:46Z

    On 2023-09-12 15:27, Peter Eisentraut wrote:
> I like this work a lot.  It's good to give developers easy to verify
> guidance about formatting the --help messages.
> 
> However, I think instead of just adding a bunch of line breaks to
> satisfy the test, we should really try to make the lines shorter by
> rewording.  Otherwise, chances are we are making the usability worse
> for many users, because it's more likely that part of the help will
> scroll off the screen.  For example, in many cases, we could replace
> "do not" by "don't", or we could decrease the indentation of the
> second column by a few spaces, or just reword altogether.
> 
> Also, it would be very useful if the TAP test function could print out
> the violating lines if a test fails.  (Similar to how is() and like()
> print the failing values.)  Maybe start with that, and then it's
> easier to play with different wording variants to try to make it fit
> better.

Thanks for the review!
I'll try to fix the patch according to your comments.

-- 
Regards,

--
Atsushi Torikoshi
NTT DATA Group Corporation

  8. Re: Make --help output fit within 80 columns per line

    Greg Sabino Mullane <htamfids@gmail.com> — 2023-09-13T17:46:23Z

    On Tue, Jul 4, 2023 at 9:47 PM torikoshia <torikoshia@oss.nttdata.com>
wrote:

> Since it seems preferable to have consistent line break policy and some
> people use 80-column terminal, wouldn't it be better to make all commands
> in 80
> columns per line?
>

All this seems an awful lot of work to support this mythical 80-column
terminal user. It's 2023, perhaps it's time to widen the default assumption
past 80 characters?

Cheers,
Greg

  9. Re: Make --help output fit within 80 columns per line

    torikoshia <torikoshia@oss.nttdata.com> — 2023-09-15T15:10:59Z

    On 2023-09-12 15:27, Peter Eisentraut wrote:
> Also, it would be very useful if the TAP test function could print out
> the violating lines if a test fails.  (Similar to how is() and like()
> print the failing values.)

Attached patch for this.
Below are the the outputs when test failed:

```
$ cd contrib/vacuumlo
$ make check
...(snip)...
t/001_basic.pl .. 1/?
     #   Failed test '  -n, --dry-run             don't remove large 
objects, just show what would be done'
     #   at 
/home/atorik/postgres/contrib/vacuumlo/../../src/test/perl/PostgreSQL/Test/Utils.pm 
line 850.
     # Looks like you failed 1 test of 21.

#   Failed test 'vacuumlo --help outputs fit within 80 columns per line'
#   at t/001_basic.pl line 10.
# Looks like you failed 1 test of 9.
t/001_basic.pl .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/9 subtests

Test Summary Report
-------------------
t/001_basic.pl (Wstat: 256 (exited 1) Tests: 9 Failed: 1)
   Failed test:  4
   Non-zero exit status: 1
Files=1, Tests=9,  0 wallclock secs ( 0.01 usr  0.01 sys +  0.04 cusr  
0.01 csys =  0.07 CPU)
Result: FAIL
```

```
$ cat tmp_check/log/regress_log_001_basic
# Running: vacuumlo --help
[23:11:10.378](0.230s) ok 1 - vacuumlo --help exit code 0
[23:11:10.379](0.001s) ok 2 - vacuumlo --help goes to stdout
[23:11:10.379](0.000s) ok 3 - vacuumlo --help nothing to stderr
[23:11:10.380](0.000s) # Subtest: vacuumlo --help outputs fit within 80 
columns per line
[23:11:10.380](0.001s)     ok 1 - vacuumlo removes unreferenced large 
objects from databases.
[23:11:10.380](0.000s)     ok 2 -
[23:11:10.381](0.000s)     ok 3 - Usage:
[23:11:10.381](0.000s)     ok 4 -   vacuumlo [OPTION]... DBNAME...
[23:11:10.381](0.000s)     ok 5 -
[23:11:10.381](0.000s)     ok 6 - Options:
[23:11:10.381](0.000s)     ok 7 -   -l, --limit=LIMIT         commit 
after removing each LIMIT large objects
[23:11:10.382](0.000s)     ok 20 - Report bugs to 
<pgsql-bugs@lists.postgresql.org>.
[23:11:10.382](0.000s)     ok 21 - PostgreSQL home page: 
<https://www.postgresql.org/>
[23:11:10.382](0.000s)     1..21
[23:11:10.382](0.000s)     # Looks like you failed 1 test of 21.
[23:11:10.382](0.000s) not ok 4 - vacuumlo --help outputs fit within 80 
columns per line
[23:11:10.382](0.000s)
[23:11:10.382](0.000s) #   Failed test 'vacuumlo --help outputs fit 
within 80 columns per line'
#   at t/001_basic.pl line 10.
# Running: vacuumlo --version
[23:11:10.388](0.005s) ok 5 - vacuumlo --version exit code 0
[23:11:10.388](0.000s) ok 6 - vacuumlo --version goes to stdout
[23:11:10.388](0.000s) ok 7 - vacuumlo --version nothing to stderr
# Running: vacuumlo --not-a-valid-option
[23:11:10.391](0.003s) ok 8 - vacuumlo with invalid option nonzero exit 
code
[23:11:10.391](0.000s) ok 9 - vacuumlo with invalid option prints error 
message
[23:11:10.391](0.000s) 1..9
[23:11:10.391](0.000s) # Looks like you failed 1 test of 9.
```

I feel using subtest in Test::More improves readability.


On 2023-09-14 02:46, Greg Sabino Mullane wrote:
> All this seems an awful lot of work to support this mythical 80-column 
> terminal user.
> It's 2023, perhaps it's time to widen the default assumption past 80 
> characters?

That may be a good idea.

However, from what I have seen some basic commands like `ls` in my Linux 
environments, the man command has over 100 characters per line, while 
the output of the --help option seems to be within 80 characters per 
line.
Also, the current PostgreSQL commands follow the "no more than 80 
characters per line".

I do not intend to adhere to this rule(my terminals are usually bigger 
than 80 chars per line), but wouldn't it be a not bad direction to use 
80 characters for all commands?

Thoughts?

-- 
Regards,

--
Atsushi Torikoshi
NTT DATA Group Corporation

  10. Re: Make --help output fit within 80 columns per line

    Greg Sabino Mullane <htamfids@gmail.com> — 2023-09-18T15:45:01Z

    On Fri, Sep 15, 2023 at 11:11 AM torikoshia <torikoshia@oss.nttdata.com>
wrote:

> I do not intend to adhere to this rule(my terminals are usually bigger
> than 80 chars per line), but wouldn't it be a not bad direction to use
> 80 characters for all commands?
>

Well, that's the question du jour, isn't it? The 80 character limit is
based on punch cards, and really has no place in modern systems. While gnu
systems are stuck in the past, many other ones have moved on to more
sensible defaults:

$ wget --help | wc -L
110

$ gcloud --help | wc -L
122

$ yum --help | wc -L
122

git is an interesting one, as they force things through a pager for their
help, but if you look at their raw help text files, they have plenty of
times they go past 80 when needed:

$ wc -L git/Documentation/git-*.txt | sort -g | tail -20
    109 git-filter-branch.txt
    109 git-rebase.txt
    116 git-diff-index.txt
    116 git-http-fetch.txt
    117 git-restore.txt
    122 git-checkout.txt
    122 git-ls-tree.txt
    129 git-init-db.txt
    131 git-push.txt
    132 git-update-ref.txt
    142 git-maintenance.txt
    144 git-interpret-trailers.txt
    146 git-cat-file.txt
    148 git-repack.txt
    161 git-config.txt
    162 git-notes.txt
    205 git-stash.txt
    251 git-submodule.txt

So in summary, I think 80 is a decent soft limit, but let's not stress out
about some lines going over that, and make a hard limit of perhaps 120.

See also: https://hilton.org.uk/blog/source-code-line-length

Cheers,
Greg

P.S. I know this won't change anything right away, but it will get the
conversation started, so we can escape the inertia of punch cards / VT100
terminals someday. :)

  11. Re: Make --help output fit within 80 columns per line

    Peter Eisentraut <peter@eisentraut.org> — 2023-09-21T07:45:22Z

    On 31.08.23 09:47, torikoshia wrote:
> BTW, psql --help outputs the content of PGHOST, which caused a failure 
> in the test:
> 
> ```
> -h, --host=HOSTNAME      database server host or socket directory
>                           (default: 
> "/var/folders/m7/9snkd5b54cx_b4lxkl9ljlcc0000gn/T/LobrmSUf7t")
> ```
> 
> It may be overkill, added a logic for removing the content of PGHOST.

I wonder, should we remove this?  We display the 
environment-variable-based defaults in psql --help, but not for any 
other programs.  This is potentially misleading.

  12. Re: Make --help output fit within 80 columns per line

    torikoshia <torikoshia@oss.nttdata.com> — 2023-09-25T06:27:12Z

    On Tue, Sep 19, 2023 at 3:23 AM Greg Sabino Mullane <htamfids@gmail.com> 
wrote:
Thanks for your investigation!

> On Fri, Sep 15, 2023 at 11:11 AM torikoshia 
> <torikoshia@oss.nttdata.com> wrote:
>> I do not intend to adhere to this rule(my terminals are usually bigger
>> than 80 chars per line), but wouldn't it be a not bad direction to use
>> 80 characters for all commands?
> 
> Well, that's the question du jour, isn't it? The 80 character limit is 
> based on punch cards, and really has no place in modern systems. While 
> gnu systems are stuck in the past, many other ones have moved on to 
> more sensible defaults:
> 
> $ wget --help | wc -L
> 110
> 
> $ gcloud --help | wc -L
> 122
> 
> $ yum --help | wc -L
> 122
> 
> git is an interesting one, as they force things through a pager for 
> their help, but if you look at their raw help text files, they have 
> plenty of times they go past 80 when needed:
> 
> $ wc -L git/Documentation/git-*.txt | sort -g | tail -20
>     109 git-filter-branch.txt
>     109 git-rebase.txt
>     116 git-diff-index.txt
>     116 git-http-fetch.txt
>     117 git-restore.txt
>     122 git-checkout.txt
>     122 git-ls-tree.txt
>     129 git-init-db.txt
>     131 git-push.txt
>     132 git-update-ref.txt
>     142 git-maintenance.txt
>     144 git-interpret-trailers.txt
>     146 git-cat-file.txt
>     148 git-repack.txt
>     161 git-config.txt
>     162 git-notes.txt
>     205 git-stash.txt
>     251 git-submodule.txt
> 
> So in summary, I think 80 is a decent soft limit, but let's not stress 
> out about some lines going over that, and make a hard limit of perhaps 
> 120.

+1. It may be a good compromise.
For enforcing the hard limit, is it better to add a regression test like 
patch 0001?


On 2023-09-21 16:45, Peter Eisentraut wrote:
> On 31.08.23 09:47, torikoshia wrote:
>> BTW, psql --help outputs the content of PGHOST, which caused a failure 
>> in the test:
>> 
>> ```
>> -h, --host=HOSTNAME      database server host or socket directory
>>                           (default: 
>> "/var/folders/m7/9snkd5b54cx_b4lxkl9ljlcc0000gn/T/LobrmSUf7t")
>> ```
>> 
>> It may be overkill, added a logic for removing the content of PGHOST.
> 
> I wonder, should we remove this?  We display the
> environment-variable-based defaults in psql --help, but not for any
> other programs.  This is potentially misleading.

Agreed. It seems inconsistent with other commands.
Patch 0002 removed environment-variable-based defaults in psql --help.

-- 
Regards,

--
Atsushi Torikoshi
NTT DATA Group Corporation

  13. Re: Make --help output fit within 80 columns per line

    torikoshia <torikoshia@oss.nttdata.com> — 2023-09-25T10:13:35Z

    On 2023-09-25 15:27, torikoshia wrote:
> On Tue, Sep 19, 2023 at 3:23 AM Greg Sabino Mullane 
> <htamfids@gmail.com> wrote:
> Thanks for your investigation!
> 
>> On Fri, Sep 15, 2023 at 11:11 AM torikoshia 
>> <torikoshia@oss.nttdata.com> wrote:
>>> I do not intend to adhere to this rule(my terminals are usually 
>>> bigger
>>> than 80 chars per line), but wouldn't it be a not bad direction to 
>>> use
>>> 80 characters for all commands?
>> 
>> Well, that's the question du jour, isn't it? The 80 character limit is 
>> based on punch cards, and really has no place in modern systems. While 
>> gnu systems are stuck in the past, many other ones have moved on to 
>> more sensible defaults:
>> 
>> $ wget --help | wc -L
>> 110
>> 
>> $ gcloud --help | wc -L
>> 122
>> 
>> $ yum --help | wc -L
>> 122
>> 
>> git is an interesting one, as they force things through a pager for 
>> their help, but if you look at their raw help text files, they have 
>> plenty of times they go past 80 when needed:
>> 
>> $ wc -L git/Documentation/git-*.txt | sort -g | tail -20
>>     109 git-filter-branch.txt
>>     109 git-rebase.txt
>>     116 git-diff-index.txt
>>     116 git-http-fetch.txt
>>     117 git-restore.txt
>>     122 git-checkout.txt
>>     122 git-ls-tree.txt
>>     129 git-init-db.txt
>>     131 git-push.txt
>>     132 git-update-ref.txt
>>     142 git-maintenance.txt
>>     144 git-interpret-trailers.txt
>>     146 git-cat-file.txt
>>     148 git-repack.txt
>>     161 git-config.txt
>>     162 git-notes.txt
>>     205 git-stash.txt
>>     251 git-submodule.txt
>> 
>> So in summary, I think 80 is a decent soft limit, but let's not stress 
>> out about some lines going over that, and make a hard limit of perhaps 
>> 120.
> 
> +1. It may be a good compromise.
> For enforcing the hard limit, is it better to add a regression test
> like patch 0001?

Ugh, regression tests failed and it appears to be due to reasons related 
to meson.
I'm going to investigate it.

-- 
Regards,

--
Atsushi Torikoshi
NTT DATA Group Corporation

  14. Re: Make --help output fit within 80 columns per line

    Peter Eisentraut <peter@eisentraut.org> — 2023-10-06T10:49:32Z

    On 25.09.23 08:27, torikoshia wrote:
>> So in summary, I think 80 is a decent soft limit, but let's not stress 
>> out about some lines going over that, and make a hard limit of perhaps 
>> 120.
> 
> +1. It may be a good compromise.
> For enforcing the hard limit, is it better to add a regression test like 
> patch 0001?

> Agreed. It seems inconsistent with other commands.
> Patch 0002 removed environment-variable-based defaults in psql --help.

I have committed 0002 and a different implementation of 0001.  I set the 
maximum line length to 95, which is the current maximum in use.

I'm open to discussing other line lengths, but

1) If we make it longer, then we should also adjust the existing 
wrapping so that we don't have a mix of lines wrapped at 80 and some 
significantly longer lines.

2) There are some general readability guidelines that suggest like 66 or 
72 characters per line.  If you take that and add the option name itself 
and some indentation, then around 90 does seem like a sensible limit.

3) The examples from other tools posted earlier don't convince me.  Some 
of their --help outputs look like junk and poorly taken care of.

So I think nudging people to aim for 80..95 seems like a good target 
right now.  But I'm not against adjustments.

  15. Re: Make --help output fit within 80 columns per line

    torikoshia <torikoshia@oss.nttdata.com> — 2023-10-06T12:44:48Z

    On 2023-09-25 15:27, torikoshia wrote:
> Ugh, regression tests failed and it appears to be due to reasons 
> related to meson.
> I'm going to investigate it.

ISTM

On 2023-10-06 19:49, Peter Eisentraut wrote:
> On 25.09.23 08:27, torikoshia wrote:
>>> So in summary, I think 80 is a decent soft limit, but let's not 
>>> stress out about some lines going over that, and make a hard limit of 
>>> perhaps 120.
>> 
>> +1. It may be a good compromise.
>> For enforcing the hard limit, is it better to add a regression test 
>> like patch 0001?
> 
>> Agreed. It seems inconsistent with other commands.
>> Patch 0002 removed environment-variable-based defaults in psql --help.
> 
> I have committed 0002 and a different implementation of 0001.  I set
> the maximum line length to 95, which is the current maximum in use.

Thanks!

BTW as far as I investigated, the original 0002 patch failed because
current meson doesn't accept subtest outputs.

As I commented below thread a few days ago, they once modified to
accept subtest outputs, but not anymore.
https://github.com/mesonbuild/meson/issues/10032

> I'm open to discussing other line lengths, but
> 
> 1) If we make it longer, then we should also adjust the existing
> wrapping so that we don't have a mix of lines wrapped at 80 and some
> significantly longer lines.
> 
> 2) There are some general readability guidelines that suggest like 66
> or 72 characters per line.  If you take that and add the option name
> itself and some indentation, then around 90 does seem like a sensible
> limit.
> 
> 3) The examples from other tools posted earlier don't convince me.
> Some of their --help outputs look like junk and poorly taken care of.
> 
> So I think nudging people to aim for 80..95 seems like a good target
> right now.  But I'm not against adjustments.

-- 
Regards,

--
Atsushi Torikoshi
NTT DATA Group Corporation