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

[PATCH v3 6/6] parse-options: properly align continued usage output

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Sep 11, 2021, 19:09 UTC
Message-ID
<patch-v3-6.6-0df2840ce4e-20210911T190239Z-avarab@gmail.com>
In-Reply-To
<cover-v3-0.6-00000000000-20210911T190239Z-avarab@gmail.com>

Some commands such as "git stash" emit continued options output with e.g. "git stash -h", because usage_with_options_internal() prefixes with its own whitespace the resulting output wasn't properly aligned. Let's account for the added whitespace, which properly aligns the output.

The "git stash" command has usage output with a N_() translation that legitimately stretches across multiple lines;

	N_("git stash [push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n"
	   "          [-u|--include-untracked] [-a|--all] [-m|--message <message>]\n"
           [...]

We'd like to have that output aligned with the length of the initial "git stash " output, but since usage_with_options_internal() adds its own whitespace prefixing we fell short, before this change we'd emit:

    $ git stash -h
    usage: git stash list [<options>]
       or: git stash show [<options>] [<stash>]
       [...]
       or: git stash [push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
              [-u|--include-untracked] [-a|--all] [-m|--message <message>]
              [...]

Now we'll properly emit aligned output. I.e. the last four lines above will instead be (a whitespace-only change to the above):

       [...]
       or: git stash [push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
                     [-u|--include-untracked] [-a|--all] [-m|--message <message>]
                     [...]

We could also go for an approach where we have the caller support no padding of their own, i.e. (same as the first example, except for the padding on the second line):

	N_("git stash [push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n"
	   "[-u|--include-untracked] [-a|--all] [-m|--message <message>]\n"
           [...]

But to do that we'll need to find the length of "git stash". We can discover that from the "cmd" in the "struct cmd_struct", but there might cases with sub-commands or "git" itself taking arguments that would make that non-trivial.

Even if it was I still think this approach is better, because this way we'll get the same legible alignment in the C code. The fact that usage_with_options_internal() is adding its own prefix padding is an implementation detail that callers shouldn't need to worry about.

Implementation notes:

We could skip the string_list_split() with a strchr(str, '\n') check, but we'd then need to duplicate our state machine for strings that do and don't contain a "\n". It's simpler to just always split into a "struct string_list", even though the common case is that that "struct string_list" will contain only one element. This is not performance-sensitive code.

This change is relatively more complex since I've accounted for making it future-proof for RTL translation support. Later in usage_with_options_internal() we have some existing padding code dating back to d7a38c54a6c (parse-options: be able to generate usages automatically, 2007-10-15) which isn't RTL-safe, but that code would be easy to fix. Let's not introduce new RTL translation problems here.

I'm also adding a check to catch the mistake of needlessly adding a trailing "\n", such as:

	N_("git stash save [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n"
	   "          [-u|--include-untracked] [-a|--all] [<message>]\n"),

Or even a mistake like adding just one "\n" in a string with no other newlines:

	N_("git stash list [<options>]\n"),

This catches the cases already tested for in cmd_parseopt(), but this covers the purely C API. As noted a preceding commit that added the die() to cmd_parseopt() I'm fairly confident that this will be triggered by no in-tree user I've missed.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 parse-options.c | 65 ++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 59 insertions(+), 6 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index 950a8279beb..ff28869d2c9 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -917,19 +917,72 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx,
 	FILE *outfile = err ? stderr : stdout;
 	int need_newline;
 
+	const char *usage_prefix = _("usage: %s");
+	/*
+	 * The translation could be anything, but we can count on
+	 * msgfmt(1)'s --check option to have asserted that "%s" is in
+	 * the translation. So compute the length of the "usage: "
+	 * part. We are assuming that the translator wasn't overly
+	 * clever and used e.g. "%1$s" instead of "%s", there's only
+	 * one "%s" in "usage_prefix" above, so there's no reason to
+	 * do so even with a RTL language.
+	 */
+	size_t usage_len = strlen(usage_prefix) - strlen("%s");
+	/*
+	 * TRANSLATORS: the colon here should align with the
+	 * one in "usage: %s" translation.
+	 */
+	const char *or_prefix = _("   or: %s");
+
+	/*
+	 * TRANSLATORS: You should only need to translate this format
+	 * string if your language is a RTL language (e.g. Arabic,
+	 * Hebrew etc.), not if it's a LTR language (e.g. German,
+	 * Russian, Chinese etc.).
+	 *
+	 * When a translated usage string has an embedded "\n" it's
+	 * because options have wrapped to the next line. The line
+	 * after the "\n" will then be padded to align with the
+	 * command name, such as N_("git cmd [opt]\n<8
+	 * spaces>[opt2]"), where the 8 spaces are the same length as
+	 * "git cmd ".
+	 *
+	 * This format string prints out that already-translated
+	 * line. The "%*s" is whitespace padding to account for the
+	 * padding at the start of the line that we add in this
+	 * function. The "%s" is a line in the (hopefully already
+	 * translated) N_() usage string, which contained embedded
+	 * newlines before we split it up.
+	 */
+	const char *usage_continued = _("%*s%s");
+	const char *prefix = usage_prefix;
+
 	if (!usagestr)
 		return PARSE_OPT_HELP;
 
 	if (!err && ctx && ctx->flags & PARSE_OPT_SHELL_EVAL)
 		fprintf(outfile, "cat <<\\EOF\n");
 
-	fprintf_ln(outfile, _("usage: %s"), _(*usagestr++));
 	while (*usagestr) {
-		/*
-		 * TRANSLATORS: the colon here should align with the
-		 * one in "usage: %s" translation.
-		 */
-		fprintf_ln(outfile, _("   or: %s"), _(*usagestr++));
+		struct string_list list = STRING_LIST_INIT_DUP;
+		unsigned int j;
+
+		string_list_split(&list, _(*usagestr++), '\n', -1);
+		for (j = 0; j < list.nr; j++) {
+			const char *line = list.items[j].string;
+
+			if (!*line)
+				BUG("empty or trailing line in usage string");
+
+			if (!j)
+				fprintf_ln(outfile, prefix, line);
+			else
+				fprintf_ln(outfile, usage_continued,
+					   (int)usage_len, "", line);
+		}
+		string_list_clear(&list, 0);
+
+		prefix = or_prefix;
 	}
 
 	need_newline = 1;
-- 
2.33.0.995.ga5ea46173a2
Previous: Ævar Arnfjörð BjarmasonNext: Ævar Arnfjörð Bjarmason
Message 34 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.