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

From: Masahiro Ikeda <ikedamsh(at)oss(dot)nttdata(dot)com>
To: torikoshia <torikoshia(at)oss(dot)nttdata(dot)com>
Cc: Pgsql Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Make --help output fit within 80 columns per line
Date: 2023-08-21 04:08:58
Message-ID: 3965013067e7b6195132da6c0b80a609@oss.nttdata.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Masahiro Ikeda 2023-08-21 04:32:24 Re: Fix pg_stat_reset_single_table_counters function
Previous Message Richard Guo 2023-08-21 03:48:07 Support run-time partition pruning for hash join