On Tue, Sep 19, 2023 at 3:23 AM Greg Sabino Mullane <htamf...@gmail.com>
wrote:
Thanks for your investigation!
On Fri, Sep 15, 2023 at 11:11 AM torikoshia
<torikos...@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
From b0ae0826374ce86c95ee36637913b65f865d6e0b Mon Sep 17 00:00:00 2001
From: Atsushi Torikoshi <torikos...@oss.nttdata.com>
Date: Mon, 25 Sep 2023 10:40:36 +0900
Subject: [PATCH v1 1/2] Added a test for checking the line length of --help
output.
---
src/test/perl/PostgreSQL/Test/Utils.pm | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/src/test/perl/PostgreSQL/Test/Utils.pm b/src/test/perl/PostgreSQL/Test/Utils.pm
index d66fe1cf45..291b5dcbbb 100644
--- a/src/test/perl/PostgreSQL/Test/Utils.pm
+++ b/src/test/perl/PostgreSQL/Test/Utils.pm
@@ -884,6 +884,14 @@ 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");
+
+ # We set the hard limit on the length of line to 120
+ subtest "$cmd --help outputs fit within 120 columns per line" => sub {
+ foreach my $line (split /\n/, $stdout)
+ {
+ ok(length($line) <= 120, "$line");
+ }
+};
return;
}
--
2.39.2
From 207c4059b42e975bc788452838099da825972d15 Mon Sep 17 00:00:00 2001
From: Atsushi Torikoshi <torikos...@oss.nttdata.com>
Date: Mon, 25 Sep 2023 10:52:08 +0900
Subject: [PATCH v1 2/2] Removed environment-variable-based defaults in psql --help
---
src/bin/psql/help.c | 33 ++++-----------------------------
1 file changed, 4 insertions(+), 29 deletions(-)
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index 12280c0e54..3b2d59e2ee 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -50,22 +50,10 @@
void
usage(unsigned short int pager)
{
- const char *env;
- const char *user;
- char *errstr;
PQExpBufferData buf;
int nlcount;
FILE *output;
- /* Find default user, in case we need it. */
- user = getenv("PGUSER");
- if (!user)
- {
- user = get_user_name(&errstr);
- if (!user)
- pg_fatal("%s", errstr);
- }
-
/*
* To avoid counting the output lines manually, build the output in "buf"
* and then count them.
@@ -77,13 +65,8 @@ usage(unsigned short int pager)
HELP0(" psql [OPTION]... [DBNAME [USERNAME]]\n\n");
HELP0("General options:\n");
- /* Display default database */
- env = getenv("PGDATABASE");
- if (!env)
- env = user;
HELP0(" -c, --command=COMMAND run only single command (SQL or internal) and exit\n");
- HELPN(" -d, --dbname=DBNAME database name to connect to (default: \"%s\")\n",
- env);
+ HELP0(" -d, --dbname=DBNAME database name to connect to\n");
HELP0(" -f, --file=FILENAME execute commands from file, then exit\n");
HELP0(" -l, --list list available databases, then exit\n");
HELP0(" -v, --set=, --variable=NAME=VALUE\n"
@@ -128,17 +111,9 @@ usage(unsigned short int pager)
" set record separator for unaligned output to zero byte\n");
HELP0("\nConnection options:\n");
- /* Display default host */
- env = getenv("PGHOST");
- HELPN(" -h, --host=HOSTNAME database server host or socket directory (default: \"%s\")\n",
- env ? env : _("local socket"));
- /* Display default port */
- env = getenv("PGPORT");
- HELPN(" -p, --port=PORT database server port (default: \"%s\")\n",
- env ? env : DEF_PGPORT_STR);
- /* Display default user */
- HELPN(" -U, --username=USERNAME database user name (default: \"%s\")\n",
- user);
+ HELP0(" -h, --host=HOSTNAME database server host or socket directory\n");
+ HELP0(" -p, --port=PORT database server port\n");
+ HELP0(" -U, --username=USERNAME database user name\n");
HELP0(" -w, --no-password never prompt for password\n");
HELP0(" -W, --password force password prompt (should happen automatically)\n");
--
2.39.2
