From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>, "Jeff King" <peff@peff.net>,
"Carlo Arenas" <carenas@gmail.com>,
"Eric Sunshine" <sunshine@sunshineco.com>,
"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH v3 3/6] parse-options: stop supporting "" in the usagestr array
Date: Sat, 11 Sep 2021 21:09:02 +0200 [thread overview]
Message-ID: <patch-v3-3.6-e23a8231f38-20210911T190239Z-avarab@gmail.com> (raw)
In-Reply-To: <cover-v3-0.6-00000000000-20210911T190239Z-avarab@gmail.com>
The strings in the the "usagestr" array have special handling for the
empty string dating back to f389c808b67 (Rework make_usage to print
the usage message immediately, 2007-10-14).
We'll prefix all strings after the first one with "or: ". Then if we
encountered a "" we'll emit all strings after that point verbatim
without any "or: " prefixing.
In the preceding commits we got rid of the two users of this
undocumented part of the API. Let's remove it in preparation for
improving the output emitted by usage_with_options_internal().
I think we might want to use this in the future, but in that case
we'll be much better off with an API that emulates the
non-parse_options() way that git.c does this.
That git.c code uses a separate "git_usage_string" and
"git_more_info_string". See b7d9681974e (Print info about "git help
COMMAND" on git's main usage pages, 2008-06-06). By splitting the two
up we can emit something in the middle, as indeed git.c does.
I'd like our "git <cmd> -h" info to be more helpful, and I'd also like
parse_options() to handle the "git" command itself, because of the
limitations of how this was done in usage_with_options_internal() we
couldn't migrate a caller like git.c to parse_options().
So let's just remove this for now, it has no users, and once we want
to do this again we can simply add another argument to the relevant
functions, or otherwise hook into things so that we can print
something at the end and/or middle.
It's possible that this change introduce breakage somewhere. We'd only
catch these cases at runtime, and the "git rev-parse --parseopt"
command is used by shellscripts, see bac199b7b17 (Update
git-sh-setup(1) to allow transparent use of git-rev-parse --parseopt,
2007-11-04). I've grepped the codebase for "OPTIONS_SPEC",
"char.*\*.*usage\[\]" etc. I'm fairly sure there no outstanding users
of this functionality.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
builtin/rev-parse.c | 3 +++
parse-options.c | 8 +-------
t/helper/test-parse-options.c | 2 --
t/t0040-parse-options.sh | 2 --
t/t1502-rev-parse-parseopt.sh | 34 ++++++++++++++++++----------------
5 files changed, 22 insertions(+), 27 deletions(-)
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 22c4e1a4ff0..aeebfd52805 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -436,7 +436,10 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)
for (;;) {
if (strbuf_getline(&sb, stdin) == EOF)
die(_("premature end of input"));
+ if (!sb.len)
+ die(_("empty lines are not permitted before the `--' separator"));
ALLOC_GROW(usage, unb + 1, usz);
+
if (!strcmp("--", sb.buf)) {
if (unb < 1)
die(_("no usage string given before the `--' separator"));
diff --git a/parse-options.c b/parse-options.c
index 2abff136a17..950a8279beb 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -924,18 +924,12 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx,
fprintf(outfile, "cat <<\\EOF\n");
fprintf_ln(outfile, _("usage: %s"), _(*usagestr++));
- while (*usagestr && **usagestr)
+ while (*usagestr) {
/*
* TRANSLATORS: the colon here should align with the
* one in "usage: %s" translation.
*/
fprintf_ln(outfile, _(" or: %s"), _(*usagestr++));
- while (*usagestr) {
- if (**usagestr)
- fprintf_ln(outfile, _(" %s"), _(*usagestr));
- else
- fputc('\n', outfile);
- usagestr++;
}
need_newline = 1;
diff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c
index 2051ce57db7..e00aef073b0 100644
--- a/t/helper/test-parse-options.c
+++ b/t/helper/test-parse-options.c
@@ -102,8 +102,6 @@ int cmd__parse_options(int argc, const char **argv)
const char *prefix = "prefix/";
const char *usage[] = {
"test-tool parse-options <options>",
- "",
- "A helper function for the parse-options API.",
NULL
};
struct string_list expect = STRING_LIST_INIT_NODUP;
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index ad4746d899a..2910874ece5 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -10,8 +10,6 @@ test_description='our own option parser'
cat >expect <<\EOF
usage: test-tool parse-options <options>
- A helper function for the parse-options API.
-
--yes get a boolean
-D, --no-doubt begins with 'no-'
-B, --no-fear be brave
diff --git a/t/t1502-rev-parse-parseopt.sh b/t/t1502-rev-parse-parseopt.sh
index b29563fc997..6badc650d64 100755
--- a/t/t1502-rev-parse-parseopt.sh
+++ b/t/t1502-rev-parse-parseopt.sh
@@ -6,8 +6,6 @@ test_description='test git rev-parse --parseopt'
test_expect_success 'setup optionspec' '
sed -e "s/^|//" >optionspec <<\EOF
|some-command [options] <args>...
-|
-|some-command does foo and bar!
|--
|h,help show the help
|
@@ -41,8 +39,6 @@ EOF
test_expect_success 'setup optionspec-no-switches' '
sed -e "s/^|//" >optionspec_no_switches <<\EOF
|some-command [options] <args>...
-|
-|some-command does foo and bar!
|--
EOF
'
@@ -50,8 +46,6 @@ EOF
test_expect_success 'setup optionspec-only-hidden-switches' '
sed -e "s/^|//" >optionspec_only_hidden_switches <<\EOF
|some-command [options] <args>...
-|
-|some-command does foo and bar!
|--
|hidden1* A hidden switch
EOF
@@ -62,8 +56,6 @@ test_expect_success 'test --parseopt help output' '
|cat <<\EOF
|usage: some-command [options] <args>...
|
-| some-command does foo and bar!
-|
| -h, --help show the help
| --foo some nifty option --foo
| --bar ... some cool option --bar with an argument
@@ -103,8 +95,6 @@ test_expect_success 'test --parseopt help output no switches' '
|cat <<\EOF
|usage: some-command [options] <args>...
|
-| some-command does foo and bar!
-|
|EOF
END_EXPECT
test_expect_code 129 git rev-parse --parseopt -- -h > output < optionspec_no_switches &&
@@ -116,8 +106,6 @@ test_expect_success 'test --parseopt help output hidden switches' '
|cat <<\EOF
|usage: some-command [options] <args>...
|
-| some-command does foo and bar!
-|
|EOF
END_EXPECT
test_expect_code 129 git rev-parse --parseopt -- -h > output < optionspec_only_hidden_switches &&
@@ -129,8 +117,6 @@ test_expect_success 'test --parseopt help-all output hidden switches' '
|cat <<\EOF
|usage: some-command [options] <args>...
|
-| some-command does foo and bar!
-|
| --hidden1 A hidden switch
|
|EOF
@@ -144,8 +130,6 @@ test_expect_success 'test --parseopt invalid switch help output' '
|error: unknown option `does-not-exist'\''
|usage: some-command [options] <args>...
|
-| some-command does foo and bar!
-|
| -h, --help show the help
| --foo some nifty option --foo
| --bar ... some cool option --bar with an argument
@@ -282,4 +266,22 @@ test_expect_success 'test --parseopt --stuck-long and short option with unset op
test_cmp expect output
'
+test_expect_success 'test --parseopt help output hidden switches' '
+ sed -e "s/^|//" >optionspec-trailing-line <<-\EOF &&
+ |some-command [options] <args>...
+ |
+ |
+ |--
+ |h,help show the help
+ EOF
+
+ cat >expect <<-\EOF &&
+ fatal: empty lines are not permitted before the `--'"'"' separator
+ EOF
+
+ test_must_fail git rev-parse --parseopt -- -h >out < optionspec-trailing-line 2>actual &&
+ test_must_be_empty out &&
+ test_cmp expect actual
+'
+
test_done
--
2.33.0.995.ga5ea46173a2
next prev parent reply other threads:[~2021-09-11 19:10 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-01 11:12 [PATCH 0/2] parse-options: properly align continued usage output Ævar Arnfjörð Bjarmason
2021-09-01 11:12 ` [PATCH 1/2] built-ins: "properly" " Ævar Arnfjörð Bjarmason
2021-09-01 11:12 ` [PATCH 2/2] parse-options: properly " Ævar Arnfjörð Bjarmason
2021-09-10 7:51 ` Eric Sunshine
2021-09-10 15:38 ` [PATCH v2 0/6] parse-options: properly align continued usage output & related Ævar Arnfjörð Bjarmason
2021-09-10 15:38 ` [PATCH v2 1/6] test-lib.sh: add a UNIX_SOCKETS prerequisite Ævar Arnfjörð Bjarmason
2021-09-10 15:38 ` [PATCH v2 2/6] git.c: add a NEED_UNIX_SOCKETS option for built-ins Ævar Arnfjörð Bjarmason
2021-09-11 0:14 ` Junio C Hamano
2021-09-11 2:50 ` Ævar Arnfjörð Bjarmason
2021-09-11 3:47 ` Carlo Marcelo Arenas Belón
2021-09-11 6:12 ` Junio C Hamano
2021-09-11 7:16 ` Ævar Arnfjörð Bjarmason
2021-09-10 15:38 ` [PATCH v2 3/6] parse-options: stop supporting "" in the usagestr array Ævar Arnfjörð Bjarmason
2021-09-11 0:21 ` Junio C Hamano
2021-09-11 2:46 ` Ævar Arnfjörð Bjarmason
2021-09-11 6:52 ` Junio C Hamano
2021-09-10 15:38 ` [PATCH v2 4/6] built-ins: "properly" align continued usage output Ævar Arnfjörð Bjarmason
2021-09-11 0:25 ` Junio C Hamano
2021-09-11 2:40 ` Ævar Arnfjörð Bjarmason
2021-09-10 15:38 ` [PATCH v2 5/6] send-pack: properly use parse_options() API for usage string Ævar Arnfjörð Bjarmason
2021-09-10 15:38 ` [PATCH v2 6/6] parse-options: properly align continued usage output Ævar Arnfjörð Bjarmason
2021-09-11 7:41 ` [PATCH v2 0/6] parse-options: properly align continued usage output & related Eric Sunshine
2021-09-11 19:08 ` [PATCH v3 " Ævar Arnfjörð Bjarmason
2021-09-11 19:09 ` [PATCH v3 1/6] credential-cache{,--daemon}: don't build under NO_UNIX_SOCKETS Ævar Arnfjörð Bjarmason
2021-09-12 21:48 ` Junio C Hamano
2021-09-11 19:09 ` [PATCH v3 2/6] blame: replace usage end blurb with better option spec Ævar Arnfjörð Bjarmason
2021-09-12 4:45 ` Eric Sunshine
2021-09-12 22:22 ` Junio C Hamano
2021-09-11 19:09 ` Ævar Arnfjörð Bjarmason [this message]
2021-09-12 22:26 ` [PATCH v3 3/6] parse-options: stop supporting "" in the usagestr array Junio C Hamano
2021-09-13 0:21 ` Ævar Arnfjörð Bjarmason
2021-09-11 19:09 ` [PATCH v3 4/6] built-ins: "properly" align continued usage output Ævar Arnfjörð Bjarmason
2021-09-11 19:09 ` [PATCH v3 5/6] send-pack: properly use parse_options() API for usage string Ævar Arnfjörð Bjarmason
2021-09-11 19:09 ` [PATCH v3 6/6] parse-options: properly align continued usage output Ævar Arnfjörð Bjarmason
2021-09-13 0:13 ` [PATCH v4 0/4] " Ævar Arnfjörð Bjarmason
2021-09-13 0:13 ` [PATCH v4 1/4] parse-options API users: align usage output in C-strings Ævar Arnfjörð Bjarmason
2021-09-13 0:13 ` [PATCH v4 2/4] send-pack: properly use parse_options() API for usage string Ævar Arnfjörð Bjarmason
2021-09-13 0:13 ` [PATCH v4 3/4] git rev-parse --parseopt tests: add more usagestr tests Ævar Arnfjörð Bjarmason
2021-09-13 0:13 ` [PATCH v4 4/4] parse-options: properly align continued usage output Ævar Arnfjörð Bjarmason
2021-09-13 6:41 ` Junio C Hamano
2021-09-21 13:30 ` [PATCH v5 0/4] " Ævar Arnfjörð Bjarmason
2021-09-21 13:30 ` [PATCH v5 1/4] parse-options API users: align usage output in C-strings Ævar Arnfjörð Bjarmason
2021-09-21 13:30 ` [PATCH v5 2/4] send-pack: properly use parse_options() API for usage string Ævar Arnfjörð Bjarmason
2021-09-21 13:30 ` [PATCH v5 3/4] git rev-parse --parseopt tests: add more usagestr tests Ævar Arnfjörð Bjarmason
2021-09-21 13:30 ` [PATCH v5 4/4] parse-options: properly align continued usage output Ævar Arnfjörð Bjarmason
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=patch-v3-3.6-e23a8231f38-20210911T190239Z-avarab@gmail.com \
--to=avarab@gmail.com \
--cc=carenas@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=peff@peff.net \
--cc=sunshine@sunshineco.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).