git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 3/6] parse-options: stop supporting "" in the usagestr array

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Sep 10, 2021, 15:38 UTC
Message-ID
<patch-v2-3.6-11f4a119563-20210910T153147Z-avarab@gmail.com>
In-Reply-To
<cover-v2-0.6-00000000000-20210910T153146Z-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.

This gets rid of that special case, which was added in f389c808b67 (Rework make_usage to print the usage message immediately, 2007-10-14). It was only used "blame" (the "credential-cache*" use of it was removed in the preceding commit). Before this change we'd emit:

    $ git blame -h
    usage: git blame [<options>] [<rev-opts>] [<rev>] [--] <file>
        <rev-opts> are documented in git-rev-list(1)

This changes that output to simply use "[<git-rev-list args>]" instead of "[<rev-opts>]". This accomplishes the same, is more consistent as "git bundle" and "git blame" use the same way of referring to these options now.

The use of this in "blame" dated back to 5817da01434 (git-blame: migrate to incremental parse-option [1/2], 2008-07-08), and the use in "bundle" to 2e0afafebd8 (Add git-bundle: move objects and references by archive, 2007-02-22).

Once we get rid of this special case we can also use usage_msg_opt() to emit the error message we'd get on an invalid "-L <range>" argument, which means we can get rid of the old-style "blame_usage" variable.

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/blame.c               |  9 +++------
 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 ++++++++++++++++++----------------
 6 files changed, 25 insertions(+), 33 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index 641523ff9af..45d9873a999 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -29,12 +29,8 @@
 #include "refs.h"
 #include "tag.h"
 
-static char blame_usage[] = N_("git blame [<options>] [<rev-opts>] [<rev>] [--] <file>");
-
 static const char *blame_opt_usage[] = {
-	blame_usage,
-	"",
-	N_("<rev-opts> are documented in git-rev-list(1)"),
+	N_("git blame [<options>] [<git rev-list args>] [<rev>] [--] <file>"),
 	NULL
 };
 
@@ -1107,7 +1103,8 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 				    nth_line_cb, &sb, lno, anchor,
 				    &bottom, &top, sb.path,
 				    the_repository->index))
-			usage(blame_usage);
+			usage_msg_opt(_("Invalid -L <range> parameters"),
+				      blame_opt_usage, options);
 		if ((!lno && (top || bottom)) || lno < bottom)
 			die(Q_("file %s has only %lu line",
 			       "file %s has only %lu lines",
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.876.g423ac861752
Previous: Ævar Arnfjörð BjarmasonNext: Junio C Hamano
Message 13 of 45 in “parse-options: properly align continued usage output”
  1. 0/2 parse-options: properly align continued usage outputÆvar Arnfjörð Bjarmason, Sep 1, 2021
  2. 1/2 built-ins: "properly" align continued usage outputÆvar Arnfjörð Bjarmason, Sep 1, 2021
  3. 2/2 parse-options: properly align continued usage outputÆvar Arnfjörð Bjarmason, Sep 1, 2021
  4. Eric SunshineSep 10, 2021
  5. 0/6 parse-options: properly align continued usage output & relatedÆvar Arnfjörð Bjarmason, Sep 10, 2021
  6. 1/6 test-lib.sh: add a UNIX_SOCKETS prerequisiteÆvar Arnfjörð Bjarmason, Sep 10, 2021
  7. 2/6 git.c: add a NEED_UNIX_SOCKETS option for built-insÆvar Arnfjörð Bjarmason, Sep 10, 2021
  8. Junio C HamanoSep 11, 2021
  9. Ævar Arnfjörð BjarmasonSep 11, 2021
  10. Carlo Marcelo Arenas BelónSep 11, 2021
  11. Junio C HamanoSep 11, 2021
  12. Ævar Arnfjörð BjarmasonSep 11, 2021
  13. 3/6 parse-options: stop supporting "" in the usagestr arrayÆvar Arnfjörð Bjarmason, Sep 10, 2021
  14. Junio C HamanoSep 11, 2021
  15. Ævar Arnfjörð BjarmasonSep 11, 2021
  16. Junio C HamanoSep 11, 2021
  17. 4/6 built-ins: "properly" align continued usage outputÆvar Arnfjörð Bjarmason, Sep 10, 2021
  18. Junio C HamanoSep 11, 2021
  19. Ævar Arnfjörð BjarmasonSep 11, 2021
  20. 5/6 send-pack: properly use parse_options() API for usage stringÆvar Arnfjörð Bjarmason, Sep 10, 2021
  21. 6/6 parse-options: properly align continued usage outputÆvar Arnfjörð Bjarmason, Sep 10, 2021
  22. Eric SunshineSep 11, 2021
  23. 0/6 parse-options: properly align continued usage output & relatedÆvar Arnfjörð Bjarmason, Sep 11, 2021
  24. 1/6 credential-cache{,--daemon}: don't build under NO_UNIX_SOCKETSÆvar Arnfjörð Bjarmason, Sep 11, 2021
  25. Junio C HamanoSep 12, 2021
  26. 2/6 blame: replace usage end blurb with better option specÆvar Arnfjörð Bjarmason, Sep 11, 2021
  27. Eric SunshineSep 12, 2021
  28. Junio C HamanoSep 12, 2021
  29. 3/6 parse-options: stop supporting "" in the usagestr arrayÆvar Arnfjörð Bjarmason, Sep 11, 2021
  30. Junio C HamanoSep 12, 2021
  31. Ævar Arnfjörð BjarmasonSep 13, 2021
  32. 4/6 built-ins: "properly" align continued usage outputÆvar Arnfjörð Bjarmason, Sep 11, 2021
  33. 5/6 send-pack: properly use parse_options() API for usage stringÆvar Arnfjörð Bjarmason, Sep 11, 2021
  34. 6/6 parse-options: properly align continued usage outputÆvar Arnfjörð Bjarmason, Sep 11, 2021
  35. 0/4 parse-options: properly align continued usage outputÆvar Arnfjörð Bjarmason, Sep 13, 2021
  36. 1/4 parse-options API users: align usage output in C-stringsÆvar Arnfjörð Bjarmason, Sep 13, 2021
  37. 2/4 send-pack: properly use parse_options() API for usage stringÆvar Arnfjörð Bjarmason, Sep 13, 2021
  38. 3/4 git rev-parse --parseopt tests: add more usagestr testsÆvar Arnfjörð Bjarmason, Sep 13, 2021
  39. 4/4 parse-options: properly align continued usage outputÆvar Arnfjörð Bjarmason, Sep 13, 2021
  40. Junio C HamanoSep 13, 2021
  41. 0/4 parse-options: properly align continued usage outputÆvar Arnfjörð Bjarmason, Sep 21, 2021
  42. 2/4 send-pack: properly use parse_options() API for usage stringÆvar Arnfjörð Bjarmason, Sep 21, 2021
  43. 3/4 git rev-parse --parseopt tests: add more usagestr testsÆvar Arnfjörð Bjarmason, Sep 21, 2021
  44. 4/4 parse-options: properly align continued usage outputÆvar Arnfjörð Bjarmason, Sep 21, 2021
  45. 1/4 parse-options API users: align usage output in C-stringsÆvar Arnfjörð Bjarmason, Sep 21, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.