{"thread":{"id":"60006","subject":"[PATCH] short help: allow multi-line opthelp","startedAt":"2023-07-18T22:29:41Z","lastAt":"2023-07-19T23:27:06Z","messageCount":6,"participants":["Junio C Hamano","Beat Bolli"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"479615","messageId":"xmqq5y6gg8fn.fsf@gitster.g","threadId":"60006","inReplyTo":null,"subject":"[PATCH] short help: allow multi-line opthelp","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-07-18T22:29:32Z","receivedAt":"2023-07-18T22:29:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When \"-h\" triggers the short-help in a command that implements its\noption parsing using the parse-options API, the option help text is\nshown with a single fprintf() as a long line. When the text is\nmulti-line, the second and subsequent lines are not left padded,\nthat breaks the alignment across options.\n\nBorrowing the idea from the advice API where its hint strings are\nshown with (localized) \"hint:\" prefix, let's internally split the\n(localized) help text into lines, and showing the first line, pad\nthe remaining lines to align.\n\nSigned-off-by: Junio C Hamano \n---\n parse-options.c | 12 +++++++++++-\n 1 file changed, 11 insertions(+), 1 deletion(-)\n\ndiff --git a/parse-options.c b/parse-options.c\nindex f8a155ee13..817416db99 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -1109,6 +1109,7 @@ static enum parse_opt_result usage_with_options_internal(struct parse_opt_ctx_t\n \tfor (; opts->type != OPTION_END; opts++) {\n \t\tsize_t pos;\n \t\tint pad;\n+\t\tconst char *cp, *np;\n \n \t\tif (opts->type == OPTION_SUBCOMMAND)\n \t\t\tcontinue;\n@@ -1157,7 +1158,16 @@ static enum parse_opt_result usage_with_options_internal(struct parse_opt_ctx_t\n \t\t\t\t (const char *)opts->value);\n \t\t\tcontinue;\n \t\t}\n-\t\tfprintf(outfile, \"%*s%s\\n\", pad + USAGE_GAP, \"\", _(opts->help));\n+\n+\t\tfor (cp = _(opts->help); *cp; cp = np) {\n+\t\t\tnp = strchrnul(cp, '\\n');\n+\t\t\tfprintf(outfile,\n+\t\t\t\t\"%*s%.*s\\n\", pad + USAGE_GAP, \"\",\n+\t\t\t\t(int)(np - cp), cp);\n+\t\t\tif (*np)\n+\t\t\t\tnp++;\n+\t\t\tpad = USAGE_OPTS_WIDTH;\n+\t\t}\n \t}\n \tfputc('\\n', outfile);\n \n-- \n2.41.0-376-gcba07a324d\n\n"},{"id":"479616","messageId":"xmqqzg3setso.fsf@gitster.g","threadId":"60006","inReplyTo":"xmqq5y6gg8fn.fsf@gitster.g","subject":"[PATCH] remote: simplify \"remote add --tags\" help text","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-07-18T22:31:03Z","receivedAt":"2023-07-18T22:31:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The help text for the --tags option was split into two option[]\nentries, which was a hacky way to give two lines of help text (the\nsecond entry did not have either short or long help, and there was\nno way to invoke its entry---it was there only for the help text).\n\nAs we now support multi-line text in the option help, let's make\nthe second line of the help a proper second line and remove the\nhacky second entry.\n\nSigned-off-by: Junio C Hamano \n---\n builtin/remote.c | 5 ++---\n 1 file changed, 2 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex 1e0b137d97..09fb9ae30d 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -167,10 +167,9 @@ static int add(int argc, const char **argv, const char *prefix)\n \tstruct option options[] = {\n \t\tOPT_BOOL('f', \"fetch\", &fetch, N_(\"fetch the remote branches\")),\n \t\tOPT_SET_INT(0, \"tags\", &fetch_tags,\n-\t\t\t N_(\"import all tags and associated objects when fetching\"),\n+\t\t\t N_(\"import all tags and associated objects when fetching\\n\"\n+\t\t\t \"or do not fetch any tag at all (--no-tags)\"),\n \t\t\t TAGS_SET),\n-\t\tOPT_SET_INT(0, NULL, &fetch_tags,\n-\t\t\t N_(\"or do not fetch any tag at all (--no-tags)\"), TAGS_UNSET),\n \t\tOPT_STRING_LIST('t', \"track\", &track, N_(\"branch\"),\n \t\t\t\tN_(\"branch(es) to track\")),\n \t\tOPT_STRING('m', \"master\", &master, N_(\"branch\"), N_(\"master branch\")),\n-- \n2.41.0-376-gcba07a324d\n\n"},{"id":"479618","messageId":"xmqqttu0esqb.fsf@gitster.g","threadId":"60006","inReplyTo":"xmqq5y6gg8fn.fsf@gitster.g","subject":"[PATCH v2] short help: allow multi-line opthelp","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-07-18T22:54:04Z","receivedAt":"2023-07-18T22:54:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When \"-h\" triggers the short-help in a command that implements its\noption parsing using the parse-options API, the option help text is\nshown with a single fprintf() as a long line. When the text is\nmulti-line, the second and subsequent lines are not left padded,\nthat breaks the alignment across options.\n\nBorrowing the idea from the advice API where its hint strings are\nshown with (localized) \"hint:\" prefix, let's internally split the\n(localized) help text into lines, and showing the first line, pad\nthe remaining lines to align.\n\nSigned-off-by: Junio C Hamano \n---\n * With a new test.\n\n parse-options.c | 12 +++++++++++-\n t/t0040-parse-options.sh | 2 ++\n 2 files changed, 13 insertions(+), 1 deletion(-)\n\ndiff --git a/parse-options.c b/parse-options.c\nindex f8a155ee13..817416db99 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -1109,6 +1109,7 @@ static enum parse_opt_result usage_with_options_internal(struct parse_opt_ctx_t\n \tfor (; opts->type != OPTION_END; opts++) {\n \t\tsize_t pos;\n \t\tint pad;\n+\t\tconst char *cp, *np;\n \n \t\tif (opts->type == OPTION_SUBCOMMAND)\n \t\t\tcontinue;\n@@ -1157,7 +1158,16 @@ static enum parse_opt_result usage_with_options_internal(struct parse_opt_ctx_t\n \t\t\t\t (const char *)opts->value);\n \t\t\tcontinue;\n \t\t}\n-\t\tfprintf(outfile, \"%*s%s\\n\", pad + USAGE_GAP, \"\", _(opts->help));\n+\n+\t\tfor (cp = _(opts->help); *cp; cp = np) {\n+\t\t\tnp = strchrnul(cp, '\\n');\n+\t\t\tfprintf(outfile,\n+\t\t\t\t\"%*s%.*s\\n\", pad + USAGE_GAP, \"\",\n+\t\t\t\t(int)(np - cp), cp);\n+\t\t\tif (*np)\n+\t\t\t\tnp++;\n+\t\t\tpad = USAGE_OPTS_WIDTH;\n+\t\t}\n \t}\n \tfputc('\\n', outfile);\n \ndiff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh\nindex 7d7ecfd571..83e5d4eeb6 100755\n--- a/t/t0040-parse-options.sh\n+++ b/t/t0040-parse-options.sh\n@@ -35,6 +35,8 @@ String options\n --string2 get another string\n --st get another string (pervert ordering)\n -o get another string\n+ --longhelp help text of this entry\n+ spans multiple lines\n --list add str to list\n \n Magic arguments\n-- \n2.41.0-376-gcba07a324d\n\n"},{"id":"479619","messageId":"xmqqlefcesjr.fsf_-_@gitster.g","threadId":"60006","inReplyTo":"xmqqttu0esqb.fsf@gitster.g","subject":"[RFC] short help: allow a gap smaller than USAGE_GAP","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-07-18T22:58:00Z","receivedAt":"2023-07-18T22:58:12Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The parse-options API responds to \"git cmd -h\" by listing the option\nflag (padded to the USAGE_OPTS_WIDTH column), followed by USAGE_GAP\n(set to 2) whitespaces, followed by the help text. If the flags\npart does not fit within the USAGE_OPTS_WIDTH, the help text is given\non its own line. Imagine that \"@\" below depicts the USAGE_OPTS_WIDTH'th\ncolumn, and \"#\" are for the usage help text, the output may look\nlike this:\n\n @@@@@@@@@@@@@ ########################################\n -f\t\t description of the flag '-f' comes here\n --short= description of the flag '--short'\n --very-long-option=\n description of the flag '--very-long-option'\n\nThis is all good and nice in principle, but it becomes awkward when\nthe flags part is just one column over the limit and forces a line\nbreak. See the description of the \"--almost\" option below:\n\n @@@@@@@@@@@@@ ########################################\n -f\t\t description of the flag '-f' comes here\n --short= description of the flag '--short'\n --almost=\n description of the flag '--almost'\n --very-long-option=\n description of the flag '--very-long-option'\n\nIf we allow shrinking the gap to a single whitespace only in such a\ncase, we would instead get:\n\n @@@@@@@@@@@@@ ########################################\n -f\t\t description of the flag '-f' comes here\n --short= description of the flag '--short'\n --almost= description of the flag '--almost'\n --very-long-option=\n description of the flag '--very-long-option'\n\nand the boundary between the flags and their descriptions does not\nbecome any harder to see, while saving precious vertical screen real\nestate.\n\nSigned-off-by: Junio C Hamano \n---\n * This makes \"git remote add -h\" a bit easier to read, because\n there is one such option whose flag part is exactly 24-columns\n long. Also the changes to t0040 illustrates the effect.\n\n parse-options.c | 4 +++-\n t/helper/test-parse-options.c | 2 ++\n t/t0040-parse-options.sh | 3 +--\n 3 files changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/parse-options.c b/parse-options.c\nindex 817416db99..87c9fae634 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -1146,7 +1146,9 @@ static enum parse_opt_result usage_with_options_internal(struct parse_opt_ctx_t\n \t\t !(opts->flags & PARSE_OPT_NOARG))\n \t\t\tpos += usage_argh(opts, outfile);\n \n-\t\tif (pos <= USAGE_OPTS_WIDTH)\n+\t\tif (pos == USAGE_OPTS_WIDTH + 1)\n+\t\t\tpad = -1;\n+\t\telse if (pos <= USAGE_OPTS_WIDTH)\n \t\t\tpad = USAGE_OPTS_WIDTH - pos;\n \t\telse {\n \t\t\tfputc('\\n', outfile);\ndiff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c\nindex 00fa281a9c..a4f6e24b0c 100644\n--- a/t/helper/test-parse-options.c\n+++ b/t/helper/test-parse-options.c\n@@ -133,6 +133,8 @@ int cmd__parse_options(int argc, const char **argv)\n \t\tOPT_STRING(0, \"st\", &string, \"st\", \"get another string (pervert ordering)\"),\n \t\tOPT_STRING('o', NULL, &string, \"str\", \"get another string\"),\n \t\tOPT_NOOP_NOARG(0, \"obsolete\"),\n+\t\tOPT_SET_INT_F(0, \"longhelp\", &integer, \"help text of this entry\\n\"\n+\t\t\t \"spans multiple lines\", 0, PARSE_OPT_NONEG),\n \t\tOPT_STRING_LIST(0, \"list\", &list, \"str\", \"add str to list\"),\n \t\tOPT_GROUP(\"Magic arguments\"),\n \t\tOPT_NUMBER_CALLBACK(&integer, \"set integer to NUM\",\ndiff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh\nindex 83e5d4eeb6..e19a199636 100755\n--- a/t/t0040-parse-options.sh\n+++ b/t/t0040-parse-options.sh\n@@ -30,8 +30,7 @@ usage: test-tool parse-options \n -F, --file set file to \n \n String options\n- -s, --string \n- get a string\n+ -s, --string get a string\n --string2 get another string\n --st get another string (pervert ordering)\n -o get another string\n-- \n2.41.0-376-gcba07a324d\n\n"},{"id":"479661","messageId":"98b86433-053b-c44e-05a9-b2e3afd4b4f0@drbeat.li","threadId":"60006","inReplyTo":"xmqqlefcesjr.fsf_-_@gitster.g","subject":"Re: [RFC] short help: allow a gap smaller than USAGE_GAP","fromName":"Beat Bolli","fromEmail":"dev+git@drbeat.li","sentAt":"2023-07-19T22:04:45Z","receivedAt":"2023-07-19T22:16:31Z","isPatch":false,"sender":{"key":"dev+git@drbeat.li","avatar":"https://avatars.githubusercontent.com/u/21444?v=4"},"body":"Hi\n\nOn 19.07.23 00:58, Junio C Hamano wrote:\n\n[...]\n> diff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c\n> index 00fa281a9c..a4f6e24b0c 100644\n> --- a/t/helper/test-parse-options.c\n> +++ b/t/helper/test-parse-options.c\n> @@ -133,6 +133,8 @@ int cmd__parse_options(int argc, const char **argv)\n> \t\tOPT_STRING(0, \"st\", &string, \"st\", \"get another string (pervert ordering)\"),\n> \t\tOPT_STRING('o', NULL, &string, \"str\", \"get another string\"),\n> \t\tOPT_NOOP_NOARG(0, \"obsolete\"),\n> +\t\tOPT_SET_INT_F(0, \"longhelp\", &integer, \"help text of this entry\\n\"\n> +\t\t\t \"spans multiple lines\", 0, PARSE_OPT_NONEG),\n> \t\tOPT_STRING_LIST(0, \"list\", &list, \"str\", \"add str to list\"),\n> \t\tOPT_GROUP(\"Magic arguments\"),\n> \t\tOPT_NUMBER_CALLBACK(&integer, \"set integer to NUM\",\n\nI think this chunk should be part of the previous commit.\n\nBeat\n\n"},{"id":"479664","messageId":"xmqqfs5j5vp9.fsf@gitster.g","threadId":"60006","inReplyTo":"98b86433-053b-c44e-05a9-b2e3afd4b4f0@drbeat.li","subject":"Re: [RFC] short help: allow a gap smaller than USAGE_GAP","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-07-19T23:26:58Z","receivedAt":"2023-07-19T23:27:06Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Beat Bolli writes:\n\n>> +\t\tOPT_SET_INT_F(0, \"longhelp\", &integer, \"help text of this entry\\n\"\n>> +\t\t\t \"spans multiple lines\", 0, PARSE_OPT_NONEG),\n>> \t\tOPT_STRING_LIST(0, \"list\", &list, \"str\", \"add str to list\"),\n>> \t\tOPT_GROUP(\"Magic arguments\"),\n>> \t\tOPT_NUMBER_CALLBACK(&integer, \"set integer to NUM\",\n>\n> I think this chunk should be part of the previous commit.\n\nGood eyes. Thanks.\n"}]}