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
