{"thread":{"id":"49002","subject":"[PATCH] push: comment on a funny unbalanced option help","startedAt":"2018-08-01T18:43:20Z","lastAt":"2018-08-03T15:30:16Z","messageCount":38,"participants":["Junio C Hamano","Ævar Arnfjörð Bjarmason","Jonathan Nieder","René Scharfe","Jeff King","Ramsay Jones","Andrei Rybak","Kerry, Richard"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"354177","messageId":"xmqqh8ke6i7w.fsf@gitster-ct.c.googlers.com","threadId":"49002","inReplyTo":null,"subject":"[PATCH] push: comment on a funny unbalanced option help","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-01T18:43:15Z","receivedAt":"2018-08-01T18:43:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The option help text for the force-with-lease option to \"git push\"\nreads like this:\n\n $ git push -h 2>&1 | grep -e force-with-lease\n --force-with-lease[=:]\n\nwhich come from this\n\n \t\t 0, CAS_OPT_NAME, &cas, N_(\"refname>:\" at both\nends.\n\nIt turns out that parse-options machinery takes the whole string and\nencloses it inside a pair of \"<>\", expecting that it is a single\nplaceholder. The help string was written in a funnily unbalanced\nway knowing that the end result would balance out.\n\nAdd a comment to save future readers from wasting time just like I\ndid ;-)\n\nSigned-off-by: Junio C Hamano \n---\n builtin/push.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 9cd8e8cd56..9608b0cc4f 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -558,6 +558,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n \t\tOPT_BIT( 0, \"porcelain\", &flags, N_(\"machine-readable output\"), TRANSPORT_PUSH_PORCELAIN),\n \t\tOPT_BIT('f', \"force\", &flags, N_(\"force updates\"), TRANSPORT_PUSH_FORCE),\n \t\t{ OPTION_CALLBACK,\n+\t\t /* N_() will get \"<>\" around, resulting in \":\" */\n \t\t 0, CAS_OPT_NAME, &cas, N_(\"refname>: The option help text for the force-with-lease option to \"git push\"\n> reads like this:\n>\n> $ git push -h 2>&1 | grep -e force-with-lease\n> --force-with-lease[=:]\n>\n> which come from this\n>\n> \t\t 0, CAS_OPT_NAME, &cas, N_(\"refname>:\n> in the source code, with an aparent lack of \"<\" and \">\" at both\n> ends.\n>\n> It turns out that parse-options machinery takes the whole string and\n> encloses it inside a pair of \"<>\", expecting that it is a single\n> placeholder. The help string was written in a funnily unbalanced\n> way knowing that the end result would balance out.\n>\n> Add a comment to save future readers from wasting time just like I\n> did ;-)\n\nThere's something worth fixing here for sure...\n\n> +\t\t /* N_() will get \"<>\" around, resulting in \":\" */\n\n...but this comment isn't accurate at all, N_() doesn't wrap the string\nwith <>'s, as can be seen by applying this patch:\n\n - 0, CAS_OPT_NAME, &cas, N_(\"refname>::&1 | grep -e force-with-lease\n --force-with-lease[=:]\n\nRather, it's the usage_argh() function in parse-options.c that's doing\nthis. Looks like the logic was added in 29f25d493c (\"parse-options: add\nPARSE_OPT_LITERAL_ARGHELP for complicated argh's\", 2009-05-21).\n\nAlso because of this I looked at:\n\n $ git grep -P 'N_\\(\"<'\n\nWhich shows e.g.:\n\n builtin/difftool.c:706: OPT_STRING('t', \"tool\", &difftool_cmd, N_(\"\"),\n builtin/difftool.c:714: OPT_STRING('x', \"extcmd\", &extcmd, N_(\"\"),\n\nProducing this bug:\n\n $ git difftool -h 2>&1|grep '<<'\n -t, --tool <> use the specified diff tool\n -x, --extcmd <>\n\nBut these all do the right thing for some reason, just looked briefly\nand didn't see how they're different & manage to avoid this:\n\n builtin/read-tree.c:134: { OPTION_STRING, 0, \"prefix\", &opts.prefix, N_(\"/\"),\n builtin/show-branch.c:673: { OPTION_CALLBACK, 'g', \"reflog\", &reflog_base, N_(\"[,]\"),\n builtin/update-index.c:969: N_(\",,\"),\n builtin/write-tree.c:27: { OPTION_STRING, 0, \"prefix\", &prefix, N_(\"/\"),\n"},{"id":"354198","messageId":"xmqqwot969mp.fsf@gitster-ct.c.googlers.com","threadId":"49002","inReplyTo":"87k1p9u860.fsf@evledraar.gmail.com","subject":"Re: [PATCH] push: comment on a funny unbalanced option help","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-01T21:48:46Z","receivedAt":"2018-08-01T21:48:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n>> +\t\t /* N_() will get \"<>\" around, resulting in \":\" */\n>\n> ...but this comment isn't accurate at all, N_() doesn't wrap the string\n> with <>'s, as can be seen by applying this patch:\n\nI know. It is a short-hand for \"what's inside N_() we see here\".\nTry to come up with an equivalent that fits on a 80-char line.\n"},{"id":"354203","messageId":"87h8kdu3ay.fsf@evledraar.gmail.com","threadId":"49002","inReplyTo":"xmqqwot969mp.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] push: comment on a funny unbalanced option help","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-08-01T22:31:33Z","receivedAt":"2018-08-01T22:31:38Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Aug 01 2018, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>>> +\t\t /* N_() will get \"<>\" around, resulting in \":\" */\n>>\n>> ...but this comment isn't accurate at all, N_() doesn't wrap the string\n>> with <>'s, as can be seen by applying this patch:\n>\n> I know. It is a short-hand for \"what's inside N_() we see here\".\n> Try to come up with an equivalent that fits on a 80-char line.\n\nI was going to say:\n\n/* parse_options() will munge this to \":\" */\n\nor:\n\n/* note: parse_options() will add surrounding <>'s for us */\n\nor:\n\n/* missing <>'s are not a bug, parse_options() adds them */\n\nBut looking at this again it looks like this whole thing should just be\nreplaced by:\n\n diff --git a/builtin/push.c b/builtin/push.c\n index 9cd8e8cd56..b8fa15c101 100644\n --- a/builtin/push.c\n +++ b/builtin/push.c\n @@ -558,9 +558,10 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n OPT_BIT( 0, \"porcelain\", &flags, N_(\"machine-readable output\"), TRANSPORT_PUSH_PORCELAIN),\n OPT_BIT('f', \"force\", &flags, N_(\"force updates\"), TRANSPORT_PUSH_FORCE),\n { OPTION_CALLBACK,\n - 0, CAS_OPT_NAME, &cas, N_(\"refname>::\"),\n N_(\"require old value of ref to be at this value\"),\n - PARSE_OPT_OPTARG, parseopt_push_cas_option },\n + PARSE_OPT_OPTARG | PARSE_OPT_LITERAL_ARGHELP,\n + parseopt_push_cas_option },\n { OPTION_CALLBACK, 0, \"recurse-submodules\", &recurse_submodules, \"check|on-demand|no\",\n N_(\"control recursive pushing of submodules\"),\n PARSE_OPT_OPTARG, option_parse_recurse_submodules },\n\nI.e. the reason this is confusing is because the code originally added\nin 28f5d17611 (\"remote.c: add command line option parser for\n\"--force-with-lease\"\", 2013-07-08) didn't use PARSE_OPT_LITERAL_ARGHELP,\nwhich I also see is what read-tree etc. use already to not end up with\nthese double <>'s, see also 29f25d493c (\"parse-options: add\nPARSE_OPT_LITERAL_ARGHELP for complicated argh's\", 2009-05-21).\n"},{"id":"354206","messageId":"20180801225713.GB189024@aiede.svl.corp.google.com","threadId":"49002","inReplyTo":"xmqqh8ke6i7w.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] push: comment on a funny unbalanced option help","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-08-01T22:57:14Z","receivedAt":"2018-08-01T22:57:18Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nJunio C Hamano wrote:\n\n> Add a comment to save future readers from wasting time just like I\n> did ;-)\n\nThanks. I think we should go further, and start the comment with\nTRANSLATORS so it shows up for the audience most affected by this as\nwell. See the note on TRANSLATORS in po/README for details.\n\nSincerely,\nJonathan\n"},{"id":"354240","messageId":"30a6105c-4cb7-b52f-0b0a-c4504b90a5b1@web.de","threadId":"49002","inReplyTo":"87h8kdu3ay.fsf@evledraar.gmail.com","subject":"Re: [PATCH] push: comment on a funny unbalanced option help","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2018-08-02T12:16:17Z","receivedAt":"2018-08-02T12:16:31Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 02.08.2018 um 00:31 schrieb Ævar Arnfjörð Bjarmason:\n> But looking at this again it looks like this whole thing should just be\n> replaced by:\n> \n> diff --git a/builtin/push.c b/builtin/push.c\n> index 9cd8e8cd56..b8fa15c101 100644\n> --- a/builtin/push.c\n> +++ b/builtin/push.c\n> @@ -558,9 +558,10 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n> OPT_BIT( 0, \"porcelain\", &flags, N_(\"machine-readable output\"), TRANSPORT_PUSH_PORCELAIN),\n> OPT_BIT('f', \"force\", &flags, N_(\"force updates\"), TRANSPORT_PUSH_FORCE),\n> { OPTION_CALLBACK,\n> - 0, CAS_OPT_NAME, &cas, N_(\"refname>: + 0, CAS_OPT_NAME, &cas, N_(\":\"),\n> N_(\"require old value of ref to be at this value\"),\n> - PARSE_OPT_OPTARG, parseopt_push_cas_option },\n> + PARSE_OPT_OPTARG | PARSE_OPT_LITERAL_ARGHELP,\n> + parseopt_push_cas_option },\n> { OPTION_CALLBACK, 0, \"recurse-submodules\", &recurse_submodules, \"check|on-demand|no\",\n> N_(\"control recursive pushing of submodules\"),\n> PARSE_OPT_OPTARG, option_parse_recurse_submodules },\n> \n> I.e. the reason this is confusing is because the code originally added\n> in 28f5d17611 (\"remote.c: add command line option parser for\n> \"--force-with-lease\"\", 2013-07-08) didn't use PARSE_OPT_LITERAL_ARGHELP,\n> which I also see is what read-tree etc. use already to not end up with\n> these double <>'s, see also 29f25d493c (\"parse-options: add\n> PARSE_OPT_LITERAL_ARGHELP for complicated argh's\", 2009-05-21).\n\nWe could check if argh comes with its own angle brackets already and\nnot add a second pair in that case, making PARSE_OPT_LITERAL_ARGHELP\nredundant in most cases, including the one above. Any downsides?\nToo magical?\n\n-- >8 --\nSubject: [PATCH] parse-options: automatically infer PARSE_OPT_LITERAL_ARGHELP\n\nAvoid adding an extra pair of angular brackets if the argh string\nalready contains one. Remove the flag PARSE_OPT_LITERAL_ARGHELP in the\ncases where the new automatic handling suffices. This shortens and\nsimplifies option definitions with special argument help strings a bit.\n\nSigned-off-by: Rene Scharfe \n---\n builtin/read-tree.c | 2 +-\n builtin/show-branch.c | 2 +-\n builtin/update-index.c | 2 +-\n builtin/write-tree.c | 5 ++---\n parse-options.c | 3 ++-\n 5 files changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/read-tree.c b/builtin/read-tree.c\nindex ebc43eb805..fbbc98e516 100644\n--- a/builtin/read-tree.c\n+++ b/builtin/read-tree.c\n@@ -133,7 +133,7 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)\n \t\t\t N_(\"same as -m, but discard unmerged entries\")),\n \t\t{ OPTION_STRING, 0, \"prefix\", &opts.prefix, N_(\"/\"),\n \t\t N_(\"read the tree into the index under /\"),\n-\t\t PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP },\n+\t\t PARSE_OPT_NONEG },\n \t\tOPT_BOOL('u', NULL, &opts.update,\n \t\t\t N_(\"update working tree with merge result\")),\n \t\t{ OPTION_CALLBACK, 0, \"exclude-per-directory\", &opts,\ndiff --git a/builtin/show-branch.c b/builtin/show-branch.c\nindex f2e985c00a..9106da1985 100644\n--- a/builtin/show-branch.c\n+++ b/builtin/show-branch.c\n@@ -673,7 +673,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \t\t{ OPTION_CALLBACK, 'g', \"reflog\", &reflog_base, N_(\"[,]\"),\n \t\t\t N_(\"show most recent ref-log entries starting at \"\n \t\t\t \"base\"),\n-\t\t\t PARSE_OPT_OPTARG | PARSE_OPT_LITERAL_ARGHELP,\n+\t\t\t PARSE_OPT_OPTARG,\n \t\t\t parse_reflog_param },\n \t\tOPT_END()\n \t};\ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex a8709a26ec..22749fc2c7 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -969,7 +969,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n \t\t\tN_(\",,\"),\n \t\t\tN_(\"add the specified entry to the index\"),\n \t\t\tPARSE_OPT_NOARG | /* disallow --cacheinfo= form */\n-\t\t\tPARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,\n+\t\t\tPARSE_OPT_NONEG,\n \t\t\t(parse_opt_cb *) cacheinfo_callback},\n \t\t{OPTION_CALLBACK, 0, \"chmod\", &set_executable_bit, N_(\"(+/-)x\"),\n \t\t\tN_(\"override the executable bit of the listed files\"),\ndiff --git a/builtin/write-tree.c b/builtin/write-tree.c\nindex c9d3c544e7..cdcbf8264e 100644\n--- a/builtin/write-tree.c\n+++ b/builtin/write-tree.c\n@@ -24,9 +24,8 @@ int cmd_write_tree(int argc, const char **argv, const char *unused_prefix)\n \tstruct option write_tree_options[] = {\n \t\tOPT_BIT(0, \"missing-ok\", &flags, N_(\"allow missing objects\"),\n \t\t\tWRITE_TREE_MISSING_OK),\n-\t\t{ OPTION_STRING, 0, \"prefix\", &prefix, N_(\"/\"),\n-\t\t N_(\"write tree object for a subdirectory \") ,\n-\t\t PARSE_OPT_LITERAL_ARGHELP },\n+\t\tOPT_STRING(0, \"prefix\", &prefix, N_(\"/\"),\n+\t\t\t N_(\"write tree object for a subdirectory \")),\n \t\t{ OPTION_BIT, 0, \"ignore-cache-tree\", &flags, NULL,\n \t\t N_(\"only useful for debugging\"),\n \t\t PARSE_OPT_HIDDEN | PARSE_OPT_NOARG, NULL,\ndiff --git a/parse-options.c b/parse-options.c\nindex 7db84227ab..fadfc6a833 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -660,7 +660,8 @@ int parse_options(int argc, const char **argv, const char *prefix,\n static int usage_argh(const struct option *opts, FILE *outfile)\n {\n \tconst char *s;\n-\tint literal = (opts->flags & PARSE_OPT_LITERAL_ARGHELP) || !opts->argh;\n+\tint literal = (opts->flags & PARSE_OPT_LITERAL_ARGHELP) || !opts->argh ||\n+\t\t(opts->argh[0] == '<' && strchr(opts->argh, '>'));\n \tif (opts->flags & PARSE_OPT_OPTARG)\n \t\tif (opts->long_name)\n \t\t\ts = literal ? \"[=%s]\" : \"[=<%s>]\";\n-- \n2.18.0\n"},{"id":"354247","messageId":"87ftzwu9wg.fsf@evledraar.gmail.com","threadId":"49002","inReplyTo":"30a6105c-4cb7-b52f-0b0a-c4504b90a5b1@web.de","subject":"Re: [PATCH] push: comment on a funny unbalanced option help","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-08-02T14:21:19Z","receivedAt":"2018-08-02T14:21:29Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Aug 02 2018, René Scharfe wrote:\n\n> Am 02.08.2018 um 00:31 schrieb Ævar Arnfjörð Bjarmason:\n>> But looking at this again it looks like this whole thing should just be\n>> replaced by:\n>>\n>> diff --git a/builtin/push.c b/builtin/push.c\n>> index 9cd8e8cd56..b8fa15c101 100644\n>> --- a/builtin/push.c\n>> +++ b/builtin/push.c\n>> @@ -558,9 +558,10 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n>> OPT_BIT( 0, \"porcelain\", &flags, N_(\"machine-readable output\"), TRANSPORT_PUSH_PORCELAIN),\n>> OPT_BIT('f', \"force\", &flags, N_(\"force updates\"), TRANSPORT_PUSH_FORCE),\n>> { OPTION_CALLBACK,\n>> - 0, CAS_OPT_NAME, &cas, N_(\"refname>:> + 0, CAS_OPT_NAME, &cas, N_(\":\"),\n>> N_(\"require old value of ref to be at this value\"),\n>> - PARSE_OPT_OPTARG, parseopt_push_cas_option },\n>> + PARSE_OPT_OPTARG | PARSE_OPT_LITERAL_ARGHELP,\n>> + parseopt_push_cas_option },\n>> { OPTION_CALLBACK, 0, \"recurse-submodules\", &recurse_submodules, \"check|on-demand|no\",\n>> N_(\"control recursive pushing of submodules\"),\n>> PARSE_OPT_OPTARG, option_parse_recurse_submodules },\n>>\n>> I.e. the reason this is confusing is because the code originally added\n>> in 28f5d17611 (\"remote.c: add command line option parser for\n>> \"--force-with-lease\"\", 2013-07-08) didn't use PARSE_OPT_LITERAL_ARGHELP,\n>> which I also see is what read-tree etc. use already to not end up with\n>> these double <>'s, see also 29f25d493c (\"parse-options: add\n>> PARSE_OPT_LITERAL_ARGHELP for complicated argh's\", 2009-05-21).\n>\n> We could check if argh comes with its own angle brackets already and\n> not add a second pair in that case, making PARSE_OPT_LITERAL_ARGHELP\n> redundant in most cases, including the one above. Any downsides?\n> Too magical?\n\nI'm more inclined to say that we should stop using\nPARSE_OPT_LITERAL_ARGHELP in some of these cases, and change\n\"refname>::\" in push.c, so that the help\nwe emit is --force-with-lease[=<:>].\n\nAs noted in 29f25d493c this facility wasn't added with the intent\nturning --refspec=<> into --refspec=, but to do stuff\nlike --option=[,] for options that take comma-delimited\noptions.\n\nIf we're magically removing <>'s we have no consistent convention to\ntell apart --opt= meaning \"one of a, b or c\", --refspec=\nmeaning \"the literal string 'refspec'\" and --refspec=<> meaning\nadd a string, i.e. fill in your refspec here.\n"},{"id":"354262","messageId":"e7e5f54d-04b6-4d0f-0bcb-692801f0719c@web.de","threadId":"49002","inReplyTo":"87ftzwu9wg.fsf@evledraar.gmail.com","subject":"Re: [PATCH] push: comment on a funny unbalanced option help","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2018-08-02T15:06:05Z","receivedAt":"2018-08-02T15:06:13Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 02.08.2018 um 16:21 schrieb Ævar Arnfjörð Bjarmason:\n> \n> On Thu, Aug 02 2018, René Scharfe wrote:\n> \n>> Am 02.08.2018 um 00:31 schrieb Ævar Arnfjörð Bjarmason:\n>>> But looking at this again it looks like this whole thing should just be\n>>> replaced by:\n>>>\n>>> diff --git a/builtin/push.c b/builtin/push.c\n>>> index 9cd8e8cd56..b8fa15c101 100644\n>>> --- a/builtin/push.c\n>>> +++ b/builtin/push.c\n>>> @@ -558,9 +558,10 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n>>> OPT_BIT( 0, \"porcelain\", &flags, N_(\"machine-readable output\"), TRANSPORT_PUSH_PORCELAIN),\n>>> OPT_BIT('f', \"force\", &flags, N_(\"force updates\"), TRANSPORT_PUSH_FORCE),\n>>> { OPTION_CALLBACK,\n>>> - 0, CAS_OPT_NAME, &cas, N_(\"refname>:>> + 0, CAS_OPT_NAME, &cas, N_(\":\"),\n>>> N_(\"require old value of ref to be at this value\"),\n>>> - PARSE_OPT_OPTARG, parseopt_push_cas_option },\n>>> + PARSE_OPT_OPTARG | PARSE_OPT_LITERAL_ARGHELP,\n>>> + parseopt_push_cas_option },\n>>> { OPTION_CALLBACK, 0, \"recurse-submodules\", &recurse_submodules, \"check|on-demand|no\",\n>>> N_(\"control recursive pushing of submodules\"),\n>>> PARSE_OPT_OPTARG, option_parse_recurse_submodules },\n>>>\n>>> I.e. the reason this is confusing is because the code originally added\n>>> in 28f5d17611 (\"remote.c: add command line option parser for\n>>> \"--force-with-lease\"\", 2013-07-08) didn't use PARSE_OPT_LITERAL_ARGHELP,\n>>> which I also see is what read-tree etc. use already to not end up with\n>>> these double <>'s, see also 29f25d493c (\"parse-options: add\n>>> PARSE_OPT_LITERAL_ARGHELP for complicated argh's\", 2009-05-21).\n>>\n>> We could check if argh comes with its own angle brackets already and\n>> not add a second pair in that case, making PARSE_OPT_LITERAL_ARGHELP\n>> redundant in most cases, including the one above. Any downsides?\n>> Too magical?\n> \n> I'm more inclined to say that we should stop using\n> PARSE_OPT_LITERAL_ARGHELP in some of these cases, and change\n> \"refname>::\" in push.c, so that the help\n> we emit is --force-with-lease[=<:>].\n> \n> As noted in 29f25d493c this facility wasn't added with the intent\n> turning --refspec=<> into --refspec=, but to do stuff\n> like --option=[,] for options that take comma-delimited\n> options.\n> \n> If we're magically removing <>'s we have no consistent convention to\n> tell apart --opt= meaning \"one of a, b or c\", --refspec=\n> meaning \"the literal string 'refspec'\" and --refspec=<> meaning\n> add a string, i.e. fill in your refspec here.\n\nThe notation for requiring a literal string is to use no special markers:\n\n\t--option=literal_string\n\nAlternatives can be grouped with parentheses:\n\n\t--option=(either|or)\n\nIn both cases you'd need PARSE_OPT_LITERAL_ARGHELP.\n\nI haven't seen double angle brackets before in command line help strings.\nThe commit message of 29f25d493c doesn't mention them either. A single\npair is used to indicate that users need to fill in a value of a certain\ntype:\n\n\t--refspec=\n\nMulti-part options aren't special in this syntax:\n\n\t--force-with-lease=:\n\nNB: The \"--refspec=\" in the example before that is a literal string, so\nthis is also already a multi-part option if you will.\n\nAccording to its manpage the option should rather be shown like this:\n\n\t--force-with-lease[=[:]]\n\n... to indicate that all three forms are valid:\n\n\t--force-with-lease\n\t--force-with-lease=some_ref\n\t--force-with-lease=some_ref:but_not_this\n\nThe current code doesn't allow that to be expressed, while it's possible\nwith my patch. And nothing is removed -- you can specify as many angle\nbrackets as you like, if that turns out to be useful; parseopt just won't\nadd any more on top automatically anymore if you do that.\n\nSide note: The remaining user of PARSE_OPT_LITERAL_ARGHELP in\nbuiltin/update-index.c uses a slash for alternatives; we should probably\nuse pipe instead:\n\n\t{OPTION_CALLBACK, 0, \"chmod\", &set_executable_bit, N_(\"(+/-)x\"),\n\t\tN_(\"override the executable bit of the listed files\"),\n\t\tPARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,\n\t\tchmod_callback},\n\nRené\n"},{"id":"354263","messageId":"80e861f0-717e-6678-ff1d-0c1198cb9c6f@web.de","threadId":"49002","inReplyTo":"e7e5f54d-04b6-4d0f-0bcb-692801f0719c@web.de","subject":"Re: [PATCH] push: comment on a funny unbalanced option help","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2018-08-02T15:16:09Z","receivedAt":"2018-08-02T15:16:16Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 02.08.2018 um 17:06 schrieb René Scharfe:\n> According to its manpage the option should rather be shown like this:\n> \n> \t--force-with-lease[=[:]]\n> \n> ... to indicate that all three forms are valid:\n> \n> \t--force-with-lease\n> \t--force-with-lease=some_ref\n> \t--force-with-lease=some_ref:but_not_this\n> \n> The current code doesn't allow that to be expressed, while it's possible\n> with my patch.\nErr, anything is possible with PARSE_OPT_LITERAL_ARGHELP, of course, but\nwith my patch that flag is automatically inferred if you use the argh\nstring \"[:]\".\n\nRené\n"},{"id":"354265","messageId":"xmqqk1p84wrd.fsf@gitster-ct.c.googlers.com","threadId":"49002","inReplyTo":"87ftzwu9wg.fsf@evledraar.gmail.com","subject":"Re: [PATCH] push: comment on a funny unbalanced option help","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-02T15:24:22Z","receivedAt":"2018-08-02T15:24:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> On Thu, Aug 02 2018, René Scharfe wrote:\n>\n>> Am 02.08.2018 um 00:31 schrieb Ævar Arnfjörð Bjarmason:\n>>> But looking at this again it looks like this whole thing should just be\n>>> replaced by:\n>>>\n>>> diff --git a/builtin/push.c b/builtin/push.c\n>>> index 9cd8e8cd56..b8fa15c101 100644\n>>> --- a/builtin/push.c\n>>> +++ b/builtin/push.c\n>>> @@ -558,9 +558,10 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n>>> OPT_BIT( 0, \"porcelain\", &flags, N_(\"machine-readable output\"), TRANSPORT_PUSH_PORCELAIN),\n>>> OPT_BIT('f', \"force\", &flags, N_(\"force updates\"), TRANSPORT_PUSH_FORCE),\n>>> { OPTION_CALLBACK,\n>>> - 0, CAS_OPT_NAME, &cas, N_(\"refname>:>> + 0, CAS_OPT_NAME, &cas, N_(\":\"),\n>>> N_(\"require old value of ref to be at this value\"),\n>>> - PARSE_OPT_OPTARG, parseopt_push_cas_option },\n>>> + PARSE_OPT_OPTARG | PARSE_OPT_LITERAL_ARGHELP,\n>>> + parseopt_push_cas_option },\n>>> { OPTION_CALLBACK, 0, \"recurse-submodules\", &recurse_submodules, \"check|on-demand|no\",\n>>> N_(\"control recursive pushing of submodules\"),\n>>> PARSE_OPT_OPTARG, option_parse_recurse_submodules },\n>>>\n>>> I.e. the reason this is confusing is because the code originally added\n>>> in 28f5d17611 (\"remote.c: add command line option parser for\n>>> \"--force-with-lease\"\", 2013-07-08) didn't use PARSE_OPT_LITERAL_ARGHELP,\n>>> which I also see is what read-tree etc. use already to not end up with\n>>> these double <>'s, see also 29f25d493c (\"parse-options: add\n>>> PARSE_OPT_LITERAL_ARGHELP for complicated argh's\", 2009-05-21).\n\nYup. It shows that I did not know (or remember) about LIT-ARGH when\nI wrote it (the line stayed in the same shape since its introduction\nto the codebase), and I did not know (or remember) when I sent this\npatch. The above is the best solution to my puzzlement within the\nframework of the current codebase.\n\n>> We could check if argh comes with its own angle brackets already and\n>> not add a second pair in that case, making PARSE_OPT_LITERAL_ARGHELP\n>> redundant in most cases, including the one above. Any downsides?\n>> Too magical?\n>\n> I'm more inclined to say that we should stop using\n> PARSE_OPT_LITERAL_ARGHELP in some of these cases, and change\n> \"refname>::\" in push.c, so that the help\n> we emit is --force-with-lease[=<:>].\n\nI fail to see why the outermost <> pair could be a good idea.\nWithout them, i.e. in what the current output shows, I can see\n and are something that I should supply real\nvalues (i.e. placeholders) and I should have a colon (literal) in\nbetween them. It is an established convention that a token enclosed\nin a <> pair is a placeholder.\n\nBut I am not sure what you mean by <:>.\n\n> As noted in 29f25d493c this facility wasn't added with the intent\n> turning --refspec=<> into --refspec=, but to do stuff\n> like --option=[,] for options that take comma-delimited\n> options.\n\nThere is no --refspec=<> to begin with.\n\nA single placeholder can be written in the source as \"refspec\" and\nshown as \"--refspec=\" because you get the surrounding <>\npair for free by default. Nobody would want to write \"\" in\nthe arg help, as most of the option arguments are a single value\nplaceholder. But if you want \"[,]\" in the final output,\nyou do *not* want surrounding <> pair, so you use the option and\nwrite everythnig manually in the source without magic.\n\nOr are you saying that we should consistently write surrounding \"<>\"\nto all placeholders, and stop special casing single-token ones?\nIOW, get rid of literal-arghelp and instead make that the default?\n\n> If we're magically removing <>'s we have no consistent convention to\n> tell apart --opt= meaning \"one of a, b or c\", --refspec=\n> meaning \"the literal string 'refspec'\" and --refspec=<> meaning\n> add a string, i.e. fill in your refspec here.\n\nAh, this is where you are off-by-one. \"--refspec=\" means\n\"give your refspec as its value\".\n"},{"id":"354266","messageId":"xmqqftzw4weq.fsf@gitster-ct.c.googlers.com","threadId":"49002","inReplyTo":"30a6105c-4cb7-b52f-0b0a-c4504b90a5b1@web.de","subject":"Re: [PATCH] push: comment on a funny unbalanced option help","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-02T15:31:57Z","receivedAt":"2018-08-02T15:32:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe writes:\n\n> We could check if argh comes with its own angle brackets already and\n> not add a second pair in that case, making PARSE_OPT_LITERAL_ARGHELP\n> redundant in most cases, including the one above. Any downsides?\n> Too magical?\n\nHmph.\n\n> -- >8 --\n> Subject: [PATCH] parse-options: automatically infer PARSE_OPT_LITERAL_ARGHELP\n>\n> Avoid adding an extra pair of angular brackets if the argh string\n> already contains one. Remove the flag PARSE_OPT_LITERAL_ARGHELP in the\n> cases where the new automatic handling suffices. This shortens and\n> simplifies option definitions with special argument help strings a bit.\n>\n> Signed-off-by: Rene Scharfe \n> ---\n\n> \t\t{ OPTION_STRING, 0, \"prefix\", &opts.prefix, N_(\"/\"),\n> \t\t{ OPTION_CALLBACK, 'g', \"reflog\", &reflog_base, N_(\"[,]\"),\n> \t\t\tN_(\",,\"),\n> +\t\tOPT_STRING(0, \"prefix\", &prefix, N_(\"/\"),\n\nHmph.\n\n> diff --git a/parse-options.c b/parse-options.c\n> index 7db84227ab..fadfc6a833 100644\n> --- a/parse-options.c\n> +++ b/parse-options.c\n> @@ -660,7 +660,8 @@ int parse_options(int argc, const char **argv, const char *prefix,\n> static int usage_argh(const struct option *opts, FILE *outfile)\n> {\n> \tconst char *s;\n> -\tint literal = (opts->flags & PARSE_OPT_LITERAL_ARGHELP) || !opts->argh;\n> +\tint literal = (opts->flags & PARSE_OPT_LITERAL_ARGHELP) || !opts->argh ||\n> +\t\t(opts->argh[0] == '<' && strchr(opts->argh, '>'));\n\nYou are greedier than what I thought ;-) I would have limited this\nmagic to the case where argh is surrounded by \"<>\", but you allow\none closing \">\" anywhere, which allows us to grab more complex cases.\n\nThe lack of escape hatch to decline this auto-literal magic makes me\nsomewhat nervous, but I do not offhand think of a reason why I do\nwant an arg-help string that _begins_ with '<' to be enclosed by\nanother set of \"<>\", so perhaps this is a good kind of magic.\n"},{"id":"354267","messageId":"xmqq36vw4vu5.fsf_-_@gitster-ct.c.googlers.com","threadId":"49002","inReplyTo":"xmqqwot969mp.fsf@gitster-ct.c.googlers.com","subject":"Re* [PATCH] push: comment on a funny unbalanced option help","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-02T15:44:18Z","receivedAt":"2018-08-02T15:44:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>>> +\t\t /* N_() will get \"<>\" around, resulting in \":\" */\n>>\n>> ...but this comment isn't accurate at all, N_() doesn't wrap the string\n>> with <>'s, as can be seen by applying this patch:\n>\n> I know. It is a short-hand for \"what's inside N_() we see here\".\n> Try to come up with an equivalent that fits on a 80-char line.\n\nOK, let's scrap the comment patch but do this instead.\n\nIf we decide to take René's \"auto-literal\" change, the update to the\narg help string in this patch will become a necessary preparatory\nstep, even though \"auto-literal\" will make the addition of the\nPARSE_OPT_LITERAL_ARGHELP bit redundant.\n\n-- >8 --\nSubject: [PATCH] push: use PARSE_OPT_LITERAL_ARGHELP instead of unbalanced brackets\nFrom: Ævar Arnfjörð Bjarmason \nDate: Thu, 02 Aug 2018 00:31:33 +0200\n\nThe option help text for the force-with-lease option to \"git push\"\nreads like this:\n\n $ git push -h 2>&1 | grep -e force-with-lease\n --force-with-lease[=:]\n\nwhich comes from having N_(\"refname>:\" at both\nends.\n\nIt turns out that parse-options machinery takes the whole string and\nencloses it inside a pair of \"<>\", to make it easier for majority\ncases that uses a single token placeholder. \n\nThe help string was written in a funnily unbalanced way knowing that\nthe end result would balance out, by somebody who forgot the\npresence of PARSE_OPT_LITERAL_ARGHELP, which is the escape hatch\nmechanism designed to help such a case. We just should use the\nofficial escape hatch instead.\n\nHelped-by: René Scharfe \nSigned-off-by: Junio C Hamano \n---\n builtin/push.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 1c28427d82..ef4032a9ef 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -542,9 +542,9 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n \t\tOPT_BIT( 0, \"porcelain\", &flags, N_(\"machine-readable output\"), TRANSPORT_PUSH_PORCELAIN),\n \t\tOPT_BIT('f', \"force\", &flags, N_(\"force updates\"), TRANSPORT_PUSH_FORCE),\n \t\t{ OPTION_CALLBACK,\n-\t\t 0, CAS_OPT_NAME, &cas, N_(\"refname>::\"),\n \t\t N_(\"require old value of ref to be at this value\"),\n-\t\t PARSE_OPT_OPTARG, parseopt_push_cas_option },\n+\t\t PARSE_OPT_OPTARG | PARSE_OPT_LITERAL_ARGHELP, parseopt_push_cas_option },\n \t\t{ OPTION_CALLBACK, 0, \"recurse-submodules\", &recurse_submodules, \"check|on-demand|no\",\n \t\t\tN_(\"control recursive pushing of submodules\"),\n \t\t\tPARSE_OPT_OPTARG, option_parse_recurse_submodules },\n-- \n2.18.0-321-gffc6fa0e39\n\n\n\n"},{"id":"354269","messageId":"b72e5848-a51d-a62d-2a3c-36ea6b8582d0@web.de","threadId":"49002","inReplyTo":"xmqq36vw4vu5.fsf_-_@gitster-ct.c.googlers.com","subject":"Re: Re* [PATCH] push: comment on a funny unbalanced option help","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2018-08-02T15:59:18Z","receivedAt":"2018-08-02T15:59:26Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 02.08.2018 um 17:44 schrieb Junio C Hamano:\n> Subject: [PATCH] push: use PARSE_OPT_LITERAL_ARGHELP instead of unbalanced brackets\n> From: Ævar Arnfjörð Bjarmason \n> Date: Thu, 02 Aug 2018 00:31:33 +0200\n> \n> The option help text for the force-with-lease option to \"git push\"\n> reads like this:\n> \n> $ git push -h 2>&1 | grep -e force-with-lease\n> --force-with-lease[=:]\n> \n> which comes from having N_(\"refname>: text in the source code, with an aparent lack of \"<\" and \">\" at both\n> ends.\n> \n> It turns out that parse-options machinery takes the whole string and\n> encloses it inside a pair of \"<>\", to make it easier for majority\n> cases that uses a single token placeholder.\n> \n> The help string was written in a funnily unbalanced way knowing that\n> the end result would balance out, by somebody who forgot the\n> presence of PARSE_OPT_LITERAL_ARGHELP, which is the escape hatch\n> mechanism designed to help such a case. We just should use the\n> official escape hatch instead.\n> \n> Helped-by: René Scharfe \n\nI didn't do anything for this particular patch so far? But...\n\n> Signed-off-by: Junio C Hamano \n> ---\n> builtin/push.c | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/builtin/push.c b/builtin/push.c\n> index 1c28427d82..ef4032a9ef 100644\n> --- a/builtin/push.c\n> +++ b/builtin/push.c\n> @@ -542,9 +542,9 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n> \t\tOPT_BIT( 0, \"porcelain\", &flags, N_(\"machine-readable output\"), TRANSPORT_PUSH_PORCELAIN),\n> \t\tOPT_BIT('f', \"force\", &flags, N_(\"force updates\"), TRANSPORT_PUSH_FORCE),\n> \t\t{ OPTION_CALLBACK,\n> -\t\t 0, CAS_OPT_NAME, &cas, N_(\"refname>: +\t\t 0, CAS_OPT_NAME, &cas, N_(\":\"),\n\n... shouldn't we use this opportunity to document that \"expect\" is\noptional?\n\n+\t\t 0, CAS_OPT_NAME, &cas, N_(\"[:]\"),\n\n> \t\t N_(\"require old value of ref to be at this value\"),\n> -\t\t PARSE_OPT_OPTARG, parseopt_push_cas_option },\n> +\t\t PARSE_OPT_OPTARG | PARSE_OPT_LITERAL_ARGHELP, parseopt_push_cas_option },\n> \t\t{ OPTION_CALLBACK, 0, \"recurse-submodules\", &recurse_submodules, \"check|on-demand|no\",\n> \t\t\tN_(\"control recursive pushing of submodules\"),\n> \t\t\tPARSE_OPT_OPTARG, option_parse_recurse_submodules },\n> \n"},{"id":"354270","messageId":"xmqqtvoc3fgl.fsf@gitster-ct.c.googlers.com","threadId":"49002","inReplyTo":"b72e5848-a51d-a62d-2a3c-36ea6b8582d0@web.de","subject":"Re: Re* [PATCH] push: comment on a funny unbalanced option help","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-02T16:23:22Z","receivedAt":"2018-08-02T16:23:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe writes:\n\n> Am 02.08.2018 um 17:44 schrieb Junio C Hamano:\n>> Subject: [PATCH] push: use PARSE_OPT_LITERAL_ARGHELP instead of unbalanced brackets\n>> From: Ævar Arnfjörð Bjarmason \n>> Date: Thu, 02 Aug 2018 00:31:33 +0200\n>> ...\n>> official escape hatch instead.\n>> \n>> Helped-by: René Scharfe \n>\n> I didn't do anything for this particular patch so far? But...\n>\n>> Signed-off-by: Junio C Hamano \n\nYeah, I realized it after I sent it out. The line has been replaced\nwith a forged sign-off from Ævar.\n\n>> ---\n>> builtin/push.c | 4 ++--\n>> 1 file changed, 2 insertions(+), 2 deletions(-)\n>> \n>> diff --git a/builtin/push.c b/builtin/push.c\n>> index 1c28427d82..ef4032a9ef 100644\n>> --- a/builtin/push.c\n>> +++ b/builtin/push.c\n>> @@ -542,9 +542,9 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n>> \t\tOPT_BIT( 0, \"porcelain\", &flags, N_(\"machine-readable output\"), TRANSPORT_PUSH_PORCELAIN),\n>> \t\tOPT_BIT('f', \"force\", &flags, N_(\"force updates\"), TRANSPORT_PUSH_FORCE),\n>> \t\t{ OPTION_CALLBACK,\n>> -\t\t 0, CAS_OPT_NAME, &cas, N_(\"refname>:> +\t\t 0, CAS_OPT_NAME, &cas, N_(\":\"),\n>\n> ... shouldn't we use this opportunity to document that \"expect\" is\n> optional?\n\nI consider that it is a separate topic.\n\nI thought that we achieved a consensus that making the code guess\nmissing \":\" is a misfeature that should be deprecated (in\nwhich case we would not want to \"s|:|[&]|\"), but I may be\nmisremembering it.\n\n"},{"id":"354275","messageId":"e18e9ebb-a78c-150c-9e63-e9249eb2e96d@web.de","threadId":"49002","inReplyTo":"xmqqftzw4weq.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] push: comment on a funny unbalanced option help","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2018-08-02T16:50:58Z","receivedAt":"2018-08-02T16:51:07Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 02.08.2018 um 17:31 schrieb Junio C Hamano:\n> René Scharfe writes:\n>> diff --git a/parse-options.c b/parse-options.c\n>> index 7db84227ab..fadfc6a833 100644\n>> --- a/parse-options.c\n>> +++ b/parse-options.c\n>> @@ -660,7 +660,8 @@ int parse_options(int argc, const char **argv, const char *prefix,\n>> static int usage_argh(const struct option *opts, FILE *outfile)\n>> {\n>> \tconst char *s;\n>> -\tint literal = (opts->flags & PARSE_OPT_LITERAL_ARGHELP) || !opts->argh;\n>> +\tint literal = (opts->flags & PARSE_OPT_LITERAL_ARGHELP) || !opts->argh ||\n>> +\t\t(opts->argh[0] == '<' && strchr(opts->argh, '>'));\n> \n> You are greedier than what I thought ;-) I would have limited this\n> magic to the case where argh is surrounded by \"<>\", but you allow\n> one closing \">\" anywhere, which allows us to grab more complex cases.\n\nThat's what I had initially, but only one of the current cases would\nhave benefited from that strict behavior, so it's not useful enough.\n\n> The lack of escape hatch to decline this auto-literal magic makes me\n> somewhat nervous, but I do not offhand think of a reason why I do\n> want an arg-help string that _begins_ with '<' to be enclosed by\n> another set of \"<>\", so perhaps this is a good kind of magic.\n\nThe escape hatch is to add the extra pair manually to the argh string.\n\nRené\n"},{"id":"354276","messageId":"20180802165457.GC15984@sigill.intra.peff.net","threadId":"49002","inReplyTo":"xmqqftzw4weq.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] push: comment on a funny unbalanced option help","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-08-02T16:54:58Z","receivedAt":"2018-08-02T16:55:01Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Aug 02, 2018 at 08:31:57AM -0700, Junio C Hamano wrote:\n\n> > diff --git a/parse-options.c b/parse-options.c\n> > index 7db84227ab..fadfc6a833 100644\n> > --- a/parse-options.c\n> > +++ b/parse-options.c\n> > @@ -660,7 +660,8 @@ int parse_options(int argc, const char **argv, const char *prefix,\n> > static int usage_argh(const struct option *opts, FILE *outfile)\n> > {\n> > \tconst char *s;\n> > -\tint literal = (opts->flags & PARSE_OPT_LITERAL_ARGHELP) || !opts->argh;\n> > +\tint literal = (opts->flags & PARSE_OPT_LITERAL_ARGHELP) || !opts->argh ||\n> > +\t\t(opts->argh[0] == '<' && strchr(opts->argh, '>'));\n> \n> You are greedier than what I thought ;-) I would have limited this\n> magic to the case where argh is surrounded by \"<>\", but you allow\n> one closing \">\" anywhere, which allows us to grab more complex cases.\n> \n> The lack of escape hatch to decline this auto-literal magic makes me\n> somewhat nervous, but I do not offhand think of a reason why I do\n> want an arg-help string that _begins_ with '<' to be enclosed by\n> another set of \"<>\", so perhaps this is a good kind of magic.\n\nI think that case is actually easy; once the caller provides a \"<>\",\nthey are in \"literal\" mode, so they are free to add extra brackets if\nyou want. I.e., any \"bar\" that you do want enclosed could be\nspelled \"<bar>\". It's the opposite that becomes impossible: you do\nnot have an opening bracket and nor do you want one. But as long as we\nretain LITERAL_ARGHELP for that case, we have an escape hatch.\n\nI was going to suggest that this conversion has the downside of being\nsomewhat one-way. That is, it is easy for us to find affected sites now\nsince they contain the string LITERAL_ARGHELP. Whereas if we wanted to\nback it out in the future, it is hard to know which sites are depending\non this new behavior.\n\nBut I am having trouble thinking of a reason we would want to back it\nout. This makes most cases easier, and has a good escape hatch.\n\n-Peff\n\nPS I actually would have made the rule simply \"does it begin with a\n '<'\", which seems simpler still. If people accidentally write \" PS I actually would have made the rule simply \"does it begin with a\n> '<'\", which seems simpler still. If people accidentally write \" forgetting to close their brackets, that is a bug under both the\n> old and new behavior (just with slightly different outcomes).\n\nGood point. We could also extend it further and check if it contains\nany special character, which would allow us to convert the remaining\nuser of the flag as well:\n\n\t{OPTION_CALLBACK, 0, \"chmod\", &set_executable_bit, N_(\"(+/-)x\"),\n\t\tN_(\"override the executable bit of the listed files\"),\n\t\tPARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,\n\t\tchmod_callback},\n\nSpecial characters are (, ), <, >, [, ], and |.\n\nThe idea is that we shouldn't automatically treat a string as a\nsimple replacement specifier if it looks like it has some structure\nto it.\n\nSide note: \"(+/-)x\" is marked for translation above. Any translation\nthat is not identical would be wrong, though, because the command only\naccepts a literal \"+x\" or \"-x\" in any locale. So the N_ wrapper is\nbogus, right?\n\nChecked the output with that extended check by generating all help\npages with:\n\n\tfor cmd in $(git --list-cmds=parseopt)\n\tdo git-$cmd -h\n\tdone\n\n... and found a few differences:\n\ngit add:\n- --chmod <(+/-)x> override the executable bit of the listed files\n+ --chmod (+/-)x override the executable bit of the listed files\n\nGood change. We also should change the slash to a pipe.\n\ngit checkout-index:\n- --stage <1-3|all> copy out the files from named stage\n+ --stage 1-3|all copy out the files from named stage\n\nGood change, but perhaps mention number two explicitly?\n\ngit difftool:\n- -t, --tool <> use the specified diff tool\n+ -t, --tool use the specified diff tool\n- -x, --extcmd <>\n+ -x, --extcmd \n\nAha, double angle brackets in the wild! Good change. We could\nalso remove the explicit pairs from the option definitions.\n\ngit pack-objects:\n- --index-version \n+ --index-version version[,offset]\n\nNot good before, worse after. Should be to \"[,]\".\n\ngit pull:\n- -r, --rebase[=]\n+ -r, --rebase[=false|true|merges|preserve|interactive]\n\nGood change, but wouldn't we want to add a pair of parentheses around\nthe list of alternatives?\n\ngit push:\n- --force-with-lease[=:]\n+ --force-with-lease[=refname>:]\n+ --recurse-submodules[=check|on-demand|no]\n- --signed[=]\n+ --signed[=yes|no|if-asked]\n\ngit send-pack:\n- --signed[=]\n+ --signed[=yes|no|if-asked]\n\nGood changes all three, but need parentheses..\n\n- --force-with-lease[=:]\n+ --force-with-lease[=refname>:] Linewrap output\n+ -w[w[,i1[,i2]]] Linewrap output\n\nNot good before, worse after. Should be \"[[,[,]]]\".\n\ngit update-index:\n- --cacheinfo ,,\n- add the specified entry to the index\n+ --cacheinfo add the specified entry to the index\n\nEh, what? Ah, that option is defined with PARSE_OPT_NOARG, and we only\nshow argument help because PARSE_OPT_LITERAL_ARGHELP is also given, so\nwe need to keep that flag for this special option.\n\nRené\n"},{"id":"354302","messageId":"2bc31a96-1d1a-3b71-59cc-47a3a2e29e16@web.de","threadId":"49002","inReplyTo":"ad2d8f99-07a3-0191-88a2-c43081657988@web.de","subject":"[PATCH 1/6] add, update-index: fix --chmod argument help","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2018-08-02T19:17:35Z","receivedAt":"2018-08-02T19:17:50Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Don't translate the argument specification for --chmod; \"+x\" and \"-x\"\nare the literal strings that the commands accept.\n\nSeparate alternatives using a pipe character instead of a slash, for\nconsistency.\n\nUse the flag PARSE_OPT_LITERAL_ARGHELP to prevent parseopt from adding a\npair of angular brackets around the argument help string, as that would\nwrongly indicate that users need to replace the literal strings with\nsome kind of value.\n\nSigned-off-by: Rene Scharfe \n---\n builtin/add.c | 4 +++-\n builtin/update-index.c | 2 +-\n 2 files changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 8a155dd41e..84bfec9b73 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -304,7 +304,9 @@ static struct option builtin_add_options[] = {\n \tOPT_BOOL( 0 , \"refresh\", &refresh_only, N_(\"don't add, only refresh the index\")),\n \tOPT_BOOL( 0 , \"ignore-errors\", &ignore_add_errors, N_(\"just skip files which cannot be added because of errors\")),\n \tOPT_BOOL( 0 , \"ignore-missing\", &ignore_missing, N_(\"check if - even missing - files are ignored in dry run\")),\n-\tOPT_STRING( 0 , \"chmod\", &chmod_arg, N_(\"(+/-)x\"), N_(\"override the executable bit of the listed files\")),\n+\t{ OPTION_STRING, 0, \"chmod\", &chmod_arg, \"(+|-)x\",\n+\t N_(\"override the executable bit of the listed files\"),\n+\t PARSE_OPT_LITERAL_ARGHELP },\n \tOPT_HIDDEN_BOOL(0, \"warn-embedded-repo\", &warn_on_embedded_repo,\n \t\t\tN_(\"warn when adding an embedded repository\")),\n \tOPT_END(),\ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex a8709a26ec..7feda6e271 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -971,7 +971,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n \t\t\tPARSE_OPT_NOARG | /* disallow --cacheinfo= form */\n \t\t\tPARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,\n \t\t\t(parse_opt_cb *) cacheinfo_callback},\n-\t\t{OPTION_CALLBACK, 0, \"chmod\", &set_executable_bit, N_(\"(+/-)x\"),\n+\t\t{OPTION_CALLBACK, 0, \"chmod\", &set_executable_bit, \"(+|-)x\",\n \t\t\tN_(\"override the executable bit of the listed files\"),\n \t\t\tPARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,\n \t\t\tchmod_callback},\n-- \n2.18.0\n"},{"id":"354303","messageId":"b6993beb-f9cc-225f-41e8-5a13b9e7ea58@web.de","threadId":"49002","inReplyTo":"ad2d8f99-07a3-0191-88a2-c43081657988@web.de","subject":"[PATCH 2/6] difftool: remove angular brackets from argument help","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2018-08-02T19:17:43Z","receivedAt":"2018-08-02T19:17:53Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Parseopt wraps arguments in a pair of angular brackets by default,\nsignifying that the user needs to replace it with a value of the\ndocumented type. Remove the pairs from the option definitions to\nduplication and confusion.\n\nSigned-off-by: Rene Scharfe \n---\n builtin/difftool.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/difftool.c b/builtin/difftool.c\nindex 51f6c9cdb4..172af0448b 100644\n--- a/builtin/difftool.c\n+++ b/builtin/difftool.c\n@@ -703,7 +703,7 @@ int cmd_difftool(int argc, const char **argv, const char *prefix)\n \t\t\t1, PARSE_OPT_NONEG | PARSE_OPT_HIDDEN),\n \t\tOPT_BOOL(0, \"symlinks\", &symlinks,\n \t\t\t N_(\"use symlinks in dir-diff mode\")),\n-\t\tOPT_STRING('t', \"tool\", &difftool_cmd, N_(\"\"),\n+\t\tOPT_STRING('t', \"tool\", &difftool_cmd, N_(\"tool\"),\n \t\t\t N_(\"use the specified diff tool\")),\n \t\tOPT_BOOL(0, \"tool-help\", &tool_help,\n \t\t\t N_(\"print a list of diff tools that may be used with \"\n@@ -711,7 +711,7 @@ int cmd_difftool(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL(0, \"trust-exit-code\", &trust_exit_code,\n \t\t\t N_(\"make 'git-difftool' exit when an invoked diff \"\n \t\t\t \"tool returns a non - zero exit code\")),\n-\t\tOPT_STRING('x', \"extcmd\", &extcmd, N_(\"\"),\n+\t\tOPT_STRING('x', \"extcmd\", &extcmd, N_(\"command\"),\n \t\t\t N_(\"specify a custom command for viewing diffs\")),\n \t\tOPT_END()\n \t};\n-- \n2.18.0\n"},{"id":"354304","messageId":"d400a141-ba11-605b-94ec-cb4c859d7a1f@web.de","threadId":"49002","inReplyTo":"ad2d8f99-07a3-0191-88a2-c43081657988@web.de","subject":"[PATCH 3/6] pack-objects: specify --index-version argument help explicitly","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2018-08-02T19:17:50Z","receivedAt":"2018-08-02T19:17:56Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Wrap both placeholders in the argument help string in angular brackets\nto signal that users needs replace them with some actual value. Use the\nflag PARSE_OPT_LITERAL_ARGHELP to prevent parseopt from adding another\npair.\n\nSigned-off-by: Rene Scharfe \n---\n builtin/pack-objects.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex ebc8cefb53..3a5d1fa317 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -3110,9 +3110,9 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL(0, \"all-progress-implied\",\n \t\t\t &all_progress_implied,\n \t\t\t N_(\"similar to --all-progress when progress meter is shown\")),\n-\t\t{ OPTION_CALLBACK, 0, \"index-version\", NULL, N_(\"version[,offset]\"),\n+\t\t{ OPTION_CALLBACK, 0, \"index-version\", NULL, N_(\"[,]\"),\n \t\t N_(\"write the pack index file in the specified idx format version\"),\n-\t\t 0, option_parse_index_version },\n+\t\t PARSE_OPT_LITERAL_ARGHELP, option_parse_index_version },\n \t\tOPT_MAGNITUDE(0, \"max-pack-size\", &pack_size_limit,\n \t\t\t N_(\"maximum size of each output pack file\")),\n \t\tOPT_BOOL(0, \"local\", &local,\n-- \n2.18.0\n"},{"id":"354305","messageId":"d4233136-45de-51ec-e8dd-b4879d42358e@web.de","threadId":"49002","inReplyTo":"ad2d8f99-07a3-0191-88a2-c43081657988@web.de","subject":"[PATCH 4/6] send-pack: specify --force-with-lease argument help explicitly","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2018-08-02T19:17:58Z","receivedAt":"2018-08-02T19:18:04Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Wrap each part of the argument help string in angular brackets to show\nthat users need to replace them with actual values. Do that explicitly\nto balance the pairs nicely in the code and avoid confusing casual\nreaders. Add the flag PARSE_OPT_LITERAL_ARGHELP to keep parseopt from\nadding another pair.\n\nSigned-off-by: Rene Scharfe \n---\n builtin/send-pack.c | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/send-pack.c b/builtin/send-pack.c\nindex 42fd8d1a35..0b517c9368 100644\n--- a/builtin/send-pack.c\n+++ b/builtin/send-pack.c\n@@ -178,9 +178,10 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL(0, \"stdin\", &from_stdin, N_(\"read refs from stdin\")),\n \t\tOPT_BOOL(0, \"helper-status\", &helper_status, N_(\"print status from remote helper\")),\n \t\t{ OPTION_CALLBACK,\n-\t\t 0, CAS_OPT_NAME, &cas, N_(\"refname>::\"),\n \t\t N_(\"require old value of ref to be at this value\"),\n-\t\t PARSE_OPT_OPTARG, parseopt_push_cas_option },\n+\t\t PARSE_OPT_OPTARG | PARSE_OPT_LITERAL_ARGHELP,\n+\t\t parseopt_push_cas_option },\n \t\tOPT_END()\n \t};\n \n-- \n2.18.0\n"},{"id":"354306","messageId":"4e96d2a2-dfdc-3f6f-e2c7-2eb954ed633c@web.de","threadId":"49002","inReplyTo":"ad2d8f99-07a3-0191-88a2-c43081657988@web.de","subject":"[PATCH 5/6] shortlog: correct option help for -w","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2018-08-02T19:18:06Z","receivedAt":"2018-08-02T19:18:13Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Wrap the placeholders in the option help string for -w in pairs of\nangular brackets to document that users need to replace them with actual\nnumbers. Use the flag PARSE_OPT_LITERAL_ARGHELP to prevent parseopt\nfrom adding another pair.\n\nSigned-off-by: Rene Scharfe \n---\n builtin/shortlog.c | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex 608d6ba77b..f555cf9e4f 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -268,8 +268,10 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"Suppress commit descriptions, only provides commit count\")),\n \t\tOPT_BOOL('e', \"email\", &log.email,\n \t\t\t N_(\"Show the email address of each author\")),\n-\t\t{ OPTION_CALLBACK, 'w', NULL, &log, N_(\"w[,i1[,i2]]\"),\n-\t\t\tN_(\"Linewrap output\"), PARSE_OPT_OPTARG, &parse_wrap_args },\n+\t\t{ OPTION_CALLBACK, 'w', NULL, &log, N_(\"[,[,]]\"),\n+\t\t\tN_(\"Linewrap output\"),\n+\t\t\tPARSE_OPT_OPTARG | PARSE_OPT_LITERAL_ARGHELP,\n+\t\t\t&parse_wrap_args },\n \t\tOPT_END(),\n \t};\n \n-- \n2.18.0\n"},{"id":"354307","messageId":"757ca573-3954-9c31-8a7b-56a2de8e3793@web.de","threadId":"49002","inReplyTo":"ad2d8f99-07a3-0191-88a2-c43081657988@web.de","subject":"[PATCH 6/6] parse-options: automatically infer PARSE_OPT_LITERAL_ARGHELP","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2018-08-02T19:18:14Z","receivedAt":"2018-08-02T19:18:25Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Parseopt wraps argument help strings in a pair of angular brackets by\ndefault, to tell users that they need to replace it with an actual\nvalue. This is useful in most cases, because most option arguments\nare indeed single values of a certain type. The option\nPARSE_OPT_LITERAL_ARGHELP needs to be used in option definitions with\narguments that have multiple parts or are literal strings.\n\nStop adding these angular brackets if special characters are present,\nas they indicate that we don't deal with a simple placeholder. This\nsimplifies the code a bit and makes defining special options slightly\neasier.\n\nRemove the flag PARSE_OPT_LITERAL_ARGHELP in the cases where the new\nand more cautious handling suffices.\n\nSigned-off-by: Rene Scharfe \n---\nThe patch to add PARSE_OPT_LITERAL_ARGHELP for push --force-with-lease\nshould be applied before this one.\n\n builtin/add.c | 5 ++---\n builtin/pack-objects.c | 2 +-\n builtin/read-tree.c | 2 +-\n builtin/send-pack.c | 3 +--\n builtin/shortlog.c | 3 +--\n builtin/show-branch.c | 2 +-\n builtin/update-index.c | 2 +-\n builtin/write-tree.c | 5 ++---\n parse-options.c | 3 ++-\n 9 files changed, 12 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 84bfec9b73..ba1ff5689d 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -304,9 +304,8 @@ static struct option builtin_add_options[] = {\n \tOPT_BOOL( 0 , \"refresh\", &refresh_only, N_(\"don't add, only refresh the index\")),\n \tOPT_BOOL( 0 , \"ignore-errors\", &ignore_add_errors, N_(\"just skip files which cannot be added because of errors\")),\n \tOPT_BOOL( 0 , \"ignore-missing\", &ignore_missing, N_(\"check if - even missing - files are ignored in dry run\")),\n-\t{ OPTION_STRING, 0, \"chmod\", &chmod_arg, \"(+|-)x\",\n-\t N_(\"override the executable bit of the listed files\"),\n-\t PARSE_OPT_LITERAL_ARGHELP },\n+\tOPT_STRING(0, \"chmod\", &chmod_arg, \"(+|-)x\",\n+\t\t N_(\"override the executable bit of the listed files\")),\n \tOPT_HIDDEN_BOOL(0, \"warn-embedded-repo\", &warn_on_embedded_repo,\n \t\t\tN_(\"warn when adding an embedded repository\")),\n \tOPT_END(),\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 3a5d1fa317..b2323613bc 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -3112,7 +3112,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"similar to --all-progress when progress meter is shown\")),\n \t\t{ OPTION_CALLBACK, 0, \"index-version\", NULL, N_(\"[,]\"),\n \t\t N_(\"write the pack index file in the specified idx format version\"),\n-\t\t PARSE_OPT_LITERAL_ARGHELP, option_parse_index_version },\n+\t\t 0, option_parse_index_version },\n \t\tOPT_MAGNITUDE(0, \"max-pack-size\", &pack_size_limit,\n \t\t\t N_(\"maximum size of each output pack file\")),\n \t\tOPT_BOOL(0, \"local\", &local,\ndiff --git a/builtin/read-tree.c b/builtin/read-tree.c\nindex ebc43eb805..fbbc98e516 100644\n--- a/builtin/read-tree.c\n+++ b/builtin/read-tree.c\n@@ -133,7 +133,7 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)\n \t\t\t N_(\"same as -m, but discard unmerged entries\")),\n \t\t{ OPTION_STRING, 0, \"prefix\", &opts.prefix, N_(\"/\"),\n \t\t N_(\"read the tree into the index under /\"),\n-\t\t PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP },\n+\t\t PARSE_OPT_NONEG },\n \t\tOPT_BOOL('u', NULL, &opts.update,\n \t\t\t N_(\"update working tree with merge result\")),\n \t\t{ OPTION_CALLBACK, 0, \"exclude-per-directory\", &opts,\ndiff --git a/builtin/send-pack.c b/builtin/send-pack.c\nindex 0b517c9368..724b484850 100644\n--- a/builtin/send-pack.c\n+++ b/builtin/send-pack.c\n@@ -180,8 +180,7 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)\n \t\t{ OPTION_CALLBACK,\n \t\t 0, CAS_OPT_NAME, &cas, N_(\":\"),\n \t\t N_(\"require old value of ref to be at this value\"),\n-\t\t PARSE_OPT_OPTARG | PARSE_OPT_LITERAL_ARGHELP,\n-\t\t parseopt_push_cas_option },\n+\t\t PARSE_OPT_OPTARG, parseopt_push_cas_option },\n \t\tOPT_END()\n \t};\n \ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex f555cf9e4f..3898a2c9c4 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -269,8 +269,7 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL('e', \"email\", &log.email,\n \t\t\t N_(\"Show the email address of each author\")),\n \t\t{ OPTION_CALLBACK, 'w', NULL, &log, N_(\"[,[,]]\"),\n-\t\t\tN_(\"Linewrap output\"),\n-\t\t\tPARSE_OPT_OPTARG | PARSE_OPT_LITERAL_ARGHELP,\n+\t\t\tN_(\"Linewrap output\"), PARSE_OPT_OPTARG,\n \t\t\t&parse_wrap_args },\n \t\tOPT_END(),\n \t};\ndiff --git a/builtin/show-branch.c b/builtin/show-branch.c\nindex f2e985c00a..9106da1985 100644\n--- a/builtin/show-branch.c\n+++ b/builtin/show-branch.c\n@@ -673,7 +673,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \t\t{ OPTION_CALLBACK, 'g', \"reflog\", &reflog_base, N_(\"[,]\"),\n \t\t\t N_(\"show most recent ref-log entries starting at \"\n \t\t\t \"base\"),\n-\t\t\t PARSE_OPT_OPTARG | PARSE_OPT_LITERAL_ARGHELP,\n+\t\t\t PARSE_OPT_OPTARG,\n \t\t\t parse_reflog_param },\n \t\tOPT_END()\n \t};\ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex 7feda6e271..293f59247b 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -973,7 +973,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n \t\t\t(parse_opt_cb *) cacheinfo_callback},\n \t\t{OPTION_CALLBACK, 0, \"chmod\", &set_executable_bit, \"(+|-)x\",\n \t\t\tN_(\"override the executable bit of the listed files\"),\n-\t\t\tPARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,\n+\t\t\tPARSE_OPT_NONEG,\n \t\t\tchmod_callback},\n \t\t{OPTION_SET_INT, 0, \"assume-unchanged\", &mark_valid_only, NULL,\n \t\t\tN_(\"mark files as \\\"not changing\\\"\"),\ndiff --git a/builtin/write-tree.c b/builtin/write-tree.c\nindex c9d3c544e7..cdcbf8264e 100644\n--- a/builtin/write-tree.c\n+++ b/builtin/write-tree.c\n@@ -24,9 +24,8 @@ int cmd_write_tree(int argc, const char **argv, const char *unused_prefix)\n \tstruct option write_tree_options[] = {\n \t\tOPT_BIT(0, \"missing-ok\", &flags, N_(\"allow missing objects\"),\n \t\t\tWRITE_TREE_MISSING_OK),\n-\t\t{ OPTION_STRING, 0, \"prefix\", &prefix, N_(\"/\"),\n-\t\t N_(\"write tree object for a subdirectory \") ,\n-\t\t PARSE_OPT_LITERAL_ARGHELP },\n+\t\tOPT_STRING(0, \"prefix\", &prefix, N_(\"/\"),\n+\t\t\t N_(\"write tree object for a subdirectory \")),\n \t\t{ OPTION_BIT, 0, \"ignore-cache-tree\", &flags, NULL,\n \t\t N_(\"only useful for debugging\"),\n \t\t PARSE_OPT_HIDDEN | PARSE_OPT_NOARG, NULL,\ndiff --git a/parse-options.c b/parse-options.c\nindex 7db84227ab..3b874a83a0 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -660,7 +660,8 @@ int parse_options(int argc, const char **argv, const char *prefix,\n static int usage_argh(const struct option *opts, FILE *outfile)\n {\n \tconst char *s;\n-\tint literal = (opts->flags & PARSE_OPT_LITERAL_ARGHELP) || !opts->argh;\n+\tint literal = (opts->flags & PARSE_OPT_LITERAL_ARGHELP) ||\n+\t\t!opts->argh || !!strpbrk(opts->argh, \"()<>[]|\");\n \tif (opts->flags & PARSE_OPT_OPTARG)\n \t\tif (opts->long_name)\n \t\t\ts = literal ? \"[=%s]\" : \"[=<%s>]\";\n-- \n2.18.0\n"},{"id":"354315","messageId":"xmqqzhy4zgfv.fsf@gitster-ct.c.googlers.com","threadId":"49002","inReplyTo":"ad2d8f99-07a3-0191-88a2-c43081657988@web.de","subject":"Re: [PATCH] push: comment on a funny unbalanced option help","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-02T20:01:08Z","receivedAt":"2018-08-02T20:01:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe writes:\n\n> Am 02.08.2018 um 18:54 schrieb Jeff King:\n>> PS I actually would have made the rule simply \"does it begin with a\n>> '<'\", which seems simpler still. If people accidentally write \"> forgetting to close their brackets, that is a bug under both the\n>> old and new behavior (just with slightly different outcomes).\n>\n> Good point.\n\nStraying sideways into a tangent, but do we know if any locale wants\nto use something other than \"<>\" as an enclosing braket around a\nplaceholder? This arg-help text, for example,\n\n\tN_(\"refspec\")\t\t\twithout LIT-ARG-HELP\n\nwould be irritating for such a locale's translator, who cannot\ndefeat the \"<>\" that is hardcoded and not inside _()\n\n\ts = literal ? \"%s\" : \"<%s>\";\n \nthat appear in parse-options.c::usage_argh().\n\nPerhaps we should do _(\"<%s>\") here? That way, the result would\nhopefully be made consistent with\n\n\tN_(\"[:]\")\twith LIT-ARG-HELP\n\nwhich allows translator to use the bracket of the locale's choice.\n"},{"id":"354316","messageId":"xmqqva8szg7u.fsf@gitster-ct.c.googlers.com","threadId":"49002","inReplyTo":"757ca573-3954-9c31-8a7b-56a2de8e3793@web.de","subject":"Re: [PATCH 6/6] parse-options: automatically infer PARSE_OPT_LITERAL_ARGHELP","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-02T20:05:57Z","receivedAt":"2018-08-02T20:06:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe writes:\n\n> diff --git a/parse-options.c b/parse-options.c\n> index 7db84227ab..3b874a83a0 100644\n> --- a/parse-options.c\n> +++ b/parse-options.c\n> @@ -660,7 +660,8 @@ int parse_options(int argc, const char **argv, const char *prefix,\n> static int usage_argh(const struct option *opts, FILE *outfile)\n> {\n> \tconst char *s;\n> -\tint literal = (opts->flags & PARSE_OPT_LITERAL_ARGHELP) || !opts->argh;\n> +\tint literal = (opts->flags & PARSE_OPT_LITERAL_ARGHELP) ||\n> +\t\t!opts->argh || !!strpbrk(opts->argh, \"()<>[]|\");\n\nGood that you did not include '-' in there, as that would have\nbroken a multi-word-placeholder.\n\nAll other changes in this patch looked sensible, too.\n"},{"id":"354318","messageId":"87effgtsnm.fsf@evledraar.gmail.com","threadId":"49002","inReplyTo":"xmqqtvoc3fgl.fsf@gitster-ct.c.googlers.com","subject":"Re: Re* [PATCH] push: comment on a funny unbalanced option help","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-08-02T20:33:49Z","receivedAt":"2018-08-02T20:33:54Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Aug 02 2018, Junio C Hamano wrote:\n\n> René Scharfe writes:\n>\n>> Am 02.08.2018 um 17:44 schrieb Junio C Hamano:\n>>> Subject: [PATCH] push: use PARSE_OPT_LITERAL_ARGHELP instead of unbalanced brackets\n>>> From: Ævar Arnfjörð Bjarmason \n>>> Date: Thu, 02 Aug 2018 00:31:33 +0200\n>>> ...\n>>> official escape hatch instead.\n>>>\n>>> Helped-by: René Scharfe \n>>\n>> I didn't do anything for this particular patch so far? But...\n>>\n>>> Signed-off-by: Junio C Hamano \n>\n> Yeah, I realized it after I sent it out. The line has been replaced\n> with a forged sign-off from Ævar.\n\nThanks, FWIW that's fine by me, and also if you want to drop this \"fake\"\npatch of mine and replace it with something René came up with (I have\nnot yet read his 1-6 patches submitted on this topic, so maybe they're\nnot mutually exclusive).\n\n>>> ---\n>>> builtin/push.c | 4 ++--\n>>> 1 file changed, 2 insertions(+), 2 deletions(-)\n>>>\n>>> diff --git a/builtin/push.c b/builtin/push.c\n>>> index 1c28427d82..ef4032a9ef 100644\n>>> --- a/builtin/push.c\n>>> +++ b/builtin/push.c\n>>> @@ -542,9 +542,9 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n>>> \t\tOPT_BIT( 0, \"porcelain\", &flags, N_(\"machine-readable output\"), TRANSPORT_PUSH_PORCELAIN),\n>>> \t\tOPT_BIT('f', \"force\", &flags, N_(\"force updates\"), TRANSPORT_PUSH_FORCE),\n>>> \t\t{ OPTION_CALLBACK,\n>>> -\t\t 0, CAS_OPT_NAME, &cas, N_(\"refname>:>> +\t\t 0, CAS_OPT_NAME, &cas, N_(\":\"),\n>>\n>> ... shouldn't we use this opportunity to document that \"expect\" is\n>> optional?\n>\n> I consider that it is a separate topic.\n>\n> I thought that we achieved a consensus that making the code guess\n> missing \":\" is a misfeature that should be deprecated (in\n> which case we would not want to \"s|:|[&]|\"), but I may be\n> misremembering it.\n"},{"id":"354319","messageId":"xmqqr2jgzess.fsf@gitster-ct.c.googlers.com","threadId":"49002","inReplyTo":"87effgtsnm.fsf@evledraar.gmail.com","subject":"Re: Re* [PATCH] push: comment on a funny unbalanced option help","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-02T20:36:35Z","receivedAt":"2018-08-02T20:36:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Thanks, FWIW that's fine by me, and also if you want to drop this \"fake\"\n> patch of mine and replace it with something René came up with (I have\n> not yet read his 1-6 patches submitted on this topic, so maybe they're\n> not mutually exclusive).\n\nI think the 6-patch series supersedes this one. Thanks anyway.\n"},{"id":"354320","messageId":"87d0v0tsb4.fsf@evledraar.gmail.com","threadId":"49002","inReplyTo":"2bc31a96-1d1a-3b71-59cc-47a3a2e29e16@web.de","subject":"Re: [PATCH 1/6] add, update-index: fix --chmod argument help","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-08-02T20:41:19Z","receivedAt":"2018-08-02T20:41:26Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Aug 02 2018, René Scharfe wrote:\n\n> Don't translate the argument specification for --chmod; \"+x\" and \"-x\"\n> are the literal strings that the commands accept.\n>\n> Separate alternatives using a pipe character instead of a slash, for\n> consistency.\n>\n> Use the flag PARSE_OPT_LITERAL_ARGHELP to prevent parseopt from adding a\n> pair of angular brackets around the argument help string, as that would\n> wrongly indicate that users need to replace the literal strings with\n> some kind of value.\n\nGood change.\n\nLet's mention in the commit message thath we ended up with this because\nof 4a4838b46a (\"i18n: update-index: mark parseopt strings for\ntranslation\", 2012-08-20) and 4e55ed32db (\"add: add --chmod=+x /\n--chmod=-x options\", 2016-05-31) , both of which obviously didn't intend\nfor this to happen, but were either mass-replacing \"...\" with N_(\"...\"),\nor blindly copy/pasting an existing option whose argument should have\nbeen translated.\n\n> Signed-off-by: Rene Scharfe \n> ---\n> builtin/add.c | 4 +++-\n> builtin/update-index.c | 2 +-\n> 2 files changed, 4 insertions(+), 2 deletions(-)\n>\n> diff --git a/builtin/add.c b/builtin/add.c\n> index 8a155dd41e..84bfec9b73 100644\n> --- a/builtin/add.c\n> +++ b/builtin/add.c\n> @@ -304,7 +304,9 @@ static struct option builtin_add_options[] = {\n> \tOPT_BOOL( 0 , \"refresh\", &refresh_only, N_(\"don't add, only refresh the index\")),\n> \tOPT_BOOL( 0 , \"ignore-errors\", &ignore_add_errors, N_(\"just skip files which cannot be added because of errors\")),\n> \tOPT_BOOL( 0 , \"ignore-missing\", &ignore_missing, N_(\"check if - even missing - files are ignored in dry run\")),\n> -\tOPT_STRING( 0 , \"chmod\", &chmod_arg, N_(\"(+/-)x\"), N_(\"override the executable bit of the listed files\")),\n> +\t{ OPTION_STRING, 0, \"chmod\", &chmod_arg, \"(+|-)x\",\n> +\t N_(\"override the executable bit of the listed files\"),\n> +\t PARSE_OPT_LITERAL_ARGHELP },\n> \tOPT_HIDDEN_BOOL(0, \"warn-embedded-repo\", &warn_on_embedded_repo,\n> \t\t\tN_(\"warn when adding an embedded repository\")),\n> \tOPT_END(),\n> diff --git a/builtin/update-index.c b/builtin/update-index.c\n> index a8709a26ec..7feda6e271 100644\n> --- a/builtin/update-index.c\n> +++ b/builtin/update-index.c\n> @@ -971,7 +971,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n> \t\t\tPARSE_OPT_NOARG | /* disallow --cacheinfo= form */\n> \t\t\tPARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,\n> \t\t\t(parse_opt_cb *) cacheinfo_callback},\n> -\t\t{OPTION_CALLBACK, 0, \"chmod\", &set_executable_bit, N_(\"(+/-)x\"),\n> +\t\t{OPTION_CALLBACK, 0, \"chmod\", &set_executable_bit, \"(+|-)x\",\n> \t\t\tN_(\"override the executable bit of the listed files\"),\n> \t\t\tPARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,\n> \t\t\tchmod_callback},\n"},{"id":"354324","messageId":"4b6fa9ca-97ad-74e8-f252-cc62470f5946@ramsayjones.plus.com","threadId":"49002","inReplyTo":"2bc31a96-1d1a-3b71-59cc-47a3a2e29e16@web.de","subject":"Re: [PATCH 1/6] add, update-index: fix --chmod argument help","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2018-08-02T20:59:44Z","receivedAt":"2018-08-02T20:59:51Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 02/08/18 20:17, René Scharfe wrote:\n> Don't translate the argument specification for --chmod; \"+x\" and \"-x\"\n> are the literal strings that the commands accept.\n> \n> Separate alternatives using a pipe character instead of a slash, for\n> consistency.\n> \n> Use the flag PARSE_OPT_LITERAL_ARGHELP to prevent parseopt from adding a\n> pair of angular brackets around the argument help string, as that would\n> wrongly indicate that users need to replace the literal strings with\n> some kind of value.\n> \n> Signed-off-by: Rene Scharfe \n> ---\n> builtin/add.c | 4 +++-\n> builtin/update-index.c | 2 +-\n> 2 files changed, 4 insertions(+), 2 deletions(-)\n> \n> diff --git a/builtin/add.c b/builtin/add.c\n> index 8a155dd41e..84bfec9b73 100644\n> --- a/builtin/add.c\n> +++ b/builtin/add.c\n> @@ -304,7 +304,9 @@ static struct option builtin_add_options[] = {\n> \tOPT_BOOL( 0 , \"refresh\", &refresh_only, N_(\"don't add, only refresh the index\")),\n> \tOPT_BOOL( 0 , \"ignore-errors\", &ignore_add_errors, N_(\"just skip files which cannot be added because of errors\")),\n> \tOPT_BOOL( 0 , \"ignore-missing\", &ignore_missing, N_(\"check if - even missing - files are ignored in dry run\")),\n> -\tOPT_STRING( 0 , \"chmod\", &chmod_arg, N_(\"(+/-)x\"), N_(\"override the executable bit of the listed files\")),\n> +\t{ OPTION_STRING, 0, \"chmod\", &chmod_arg, \"(+|-)x\",\n\nAm I alone in thinking that \"(+x|-x)\" is more readable?\n\nATB,\nRamsay Jones\n\n> +\t N_(\"override the executable bit of the listed files\"),\n> +\t PARSE_OPT_LITERAL_ARGHELP },\n> \tOPT_HIDDEN_BOOL(0, \"warn-embedded-repo\", &warn_on_embedded_repo,\n> \t\t\tN_(\"warn when adding an embedded repository\")),\n> \tOPT_END(),\n> diff --git a/builtin/update-index.c b/builtin/update-index.c\n> index a8709a26ec..7feda6e271 100644\n> --- a/builtin/update-index.c\n> +++ b/builtin/update-index.c\n> @@ -971,7 +971,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n> \t\t\tPARSE_OPT_NOARG | /* disallow --cacheinfo= form */\n> \t\t\tPARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,\n> \t\t\t(parse_opt_cb *) cacheinfo_callback},\n> -\t\t{OPTION_CALLBACK, 0, \"chmod\", &set_executable_bit, N_(\"(+/-)x\"),\n> +\t\t{OPTION_CALLBACK, 0, \"chmod\", &set_executable_bit, \"(+|-)x\",\n> \t\t\tN_(\"override the executable bit of the listed files\"),\n> \t\t\tPARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,\n> \t\t\tchmod_callback},\n> \n"},{"id":"354325","messageId":"4a570679-3024-1541-95da-0fa8baef75ba@gmail.com","threadId":"49002","inReplyTo":"2bc31a96-1d1a-3b71-59cc-47a3a2e29e16@web.de","subject":"Re: [PATCH 1/6] add, update-index: fix --chmod argument help","fromName":"Andrei Rybak","fromEmail":"rybak.a.v@gmail.com","sentAt":"2018-08-02T21:04:48Z","receivedAt":"2018-08-02T21:04:54Z","isPatch":true,"sender":{"key":"rybak.a.v@gmail.com","avatar":"https://avatars.githubusercontent.com/u/624072?v=4"},"body":"On 2018-08-02 21:17, René Scharfe wrote:\n> Don't translate the argument specification for --chmod; \"+x\" and \"-x\"\n> are the literal strings that the commands accept.\n> > [...]\n> \n> -\tOPT_STRING( 0 , \"chmod\", &chmod_arg, N_(\"(+/-)x\"), N_(\"override the executable bit of the listed files\")),\n> +\t{ OPTION_STRING, 0, \"chmod\", &chmod_arg, \"(+|-)x\",\n> +\t N_(\"override the executable bit of the listed files\"),\n> +\t PARSE_OPT_LITERAL_ARGHELP },\n\nWould it make sense to drop the localizations in po/* as well?\nOr should such things be handled with l10n rounds?\n\nCan be found using:\n\n grep '(+/-)x' po/*\n"},{"id":"354328","messageId":"xmqqin4szcxk.fsf@gitster-ct.c.googlers.com","threadId":"49002","inReplyTo":"4a570679-3024-1541-95da-0fa8baef75ba@gmail.com","subject":"Re: [PATCH 1/6] add, update-index: fix --chmod argument help","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-02T21:16:55Z","receivedAt":"2018-08-02T21:17:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andrei Rybak writes:\n\n> On 2018-08-02 21:17, René Scharfe wrote:\n>> Don't translate the argument specification for --chmod; \"+x\" and \"-x\"\n>> are the literal strings that the commands accept.\n>> > [...]\n>> \n>> -\tOPT_STRING( 0 , \"chmod\", &chmod_arg, N_(\"(+/-)x\"), N_(\"override the executable bit of the listed files\")),\n>> +\t{ OPTION_STRING, 0, \"chmod\", &chmod_arg, \"(+|-)x\",\n>> +\t N_(\"override the executable bit of the listed files\"),\n>> +\t PARSE_OPT_LITERAL_ARGHELP },\n>\n> Would it make sense to drop the localizations in po/* as well?\n> Or should such things be handled with l10n rounds?\n\nIt should happen \"automatically\" (eh, rather, thanks to hard work by\nJiang); for the rest of us, when the l10n coordinator updates the\nmaster .*pot file next time.\n"},{"id":"354329","messageId":"xmqqeffgzcw5.fsf@gitster-ct.c.googlers.com","threadId":"49002","inReplyTo":"4b6fa9ca-97ad-74e8-f252-cc62470f5946@ramsayjones.plus.com","subject":"Re: [PATCH 1/6] add, update-index: fix --chmod argument help","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-02T21:17:46Z","receivedAt":"2018-08-02T21:17:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramsay Jones writes:\n\n>> \tOPT_BOOL( 0 , \"ignore-missing\", &ignore_missing, N_(\"check if - even missing - files are ignored in dry run\")),\n>> -\tOPT_STRING( 0 , \"chmod\", &chmod_arg, N_(\"(+/-)x\"), N_(\"override the executable bit of the listed files\")),\n>> +\t{ OPTION_STRING, 0, \"chmod\", &chmod_arg, \"(+|-)x\",\n>\n> Am I alone in thinking that \"(+x|-x)\" is more readable?\n\nI think I am guilty of that, and I agree yours is much easier to\nread.\n\nIt can of course come on top of the series.\n"},{"id":"354336","messageId":"ea253620-ed28-54a4-0e52-16cbf4ec8451@web.de","threadId":"49002","inReplyTo":"xmqqzhy4zgfv.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] push: comment on a funny unbalanced option help","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2018-08-02T22:38:51Z","receivedAt":"2018-08-02T22:39:10Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 02.08.2018 um 22:01 schrieb Junio C Hamano:\n> René Scharfe writes:\n> \n>> Am 02.08.2018 um 18:54 schrieb Jeff King:\n>>> PS I actually would have made the rule simply \"does it begin with a\n>>> '<'\", which seems simpler still. If people accidentally write \">> forgetting to close their brackets, that is a bug under both the\n>>> old and new behavior (just with slightly different outcomes).\n>>\n>> Good point.\n> \n> Straying sideways into a tangent, but do we know if any locale wants\n> to use something other than \"<>\" as an enclosing braket around a\n> placeholder?\n\nBulgarian seems to use capitals instead; here are some examples found\nwith git grep -A1 'msgid \"<' po/:\n\npo/bg.po:msgid \"\"\npo/bg.po-msgstr \"ОТДАЛЕЧЕНО_ХРАНИЛИЩЕ\"\n--\npo/bg.po:msgid \"\"\npo/bg.po-msgstr \"КЛОН\"\n--\npo/bg.po:msgid \"/\"\npo/bg.po-msgstr \"ПОДДИРЕКТОРИЯ/\"\n--\npo/bg.po:msgid \"[,]\"\npo/bg.po-msgstr \"БРОЙ[,БАЗА]\"\n\n> This arg-help text, for example,\n> \n> \tN_(\"refspec\")\t\t\twithout LIT-ARG-HELP\n> \n> would be irritating for such a locale's translator, who cannot\n> defeat the \"<>\" that is hardcoded and not inside _()\n> \n> \ts = literal ? \"%s\" : \"<%s>\";\n> \n> that appear in parse-options.c::usage_argh().\n> \n> Perhaps we should do _(\"<%s>\") here? That way, the result would\n> hopefully be made consistent with\n> \n> \tN_(\"[:]\")\twith LIT-ARG-HELP\n> \n> which allows translator to use the bracket of the locale's choice.\n\n@Alexander: Would that help you?\n\nRené\n"},{"id":"354351","messageId":"3120e9b3-f3e7-465c-63c3-084d3f010e1e@web.de","threadId":"49002","inReplyTo":"xmqqr2jgzess.fsf@gitster-ct.c.googlers.com","subject":"Re: Re* [PATCH] push: comment on a funny unbalanced option help","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2018-08-03T04:42:30Z","receivedAt":"2018-08-03T04:42:40Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 02.08.2018 um 22:36 schrieb Junio C Hamano:\n> Ævar Arnfjörð Bjarmason writes:\n> \n>> Thanks, FWIW that's fine by me, and also if you want to drop this \"fake\"\n>> patch of mine and replace it with something René came up with (I have\n>> not yet read his 1-6 patches submitted on this topic, so maybe they're\n>> not mutually exclusive).\n> \n> I think the 6-patch series supersedes this one. Thanks anyway.\n\nActually no, I specifically avoided touching builtin/push.c because this\npatch already handled it; it should still be applied before patch 6 from\nmy series.\n\nRené\n"},{"id":"354363","messageId":"AM0PR02MB371539182CB9D1FD201D1CAF9C230@AM0PR02MB3715.eurprd02.prod.outlook.com","threadId":"49002","inReplyTo":"757ca573-3954-9c31-8a7b-56a2de8e3793@web.de","subject":"Re: [PATCH 6/6] parse-options: automatically infer PARSE_OPT_LITERAL_ARGHELP","fromName":"Kerry, Richard","fromEmail":"richard.kerry@atos.net","sentAt":"2018-08-03T08:13:15Z","receivedAt":"2018-08-03T08:20:44Z","isPatch":true,"sender":{"key":"richard.kerry@atos.net","avatar":null},"body":"\ns/angular brackets/angle brackets/\n\n\n\nI've never seen these called \"angular brackets\".\n\n\nRichard Kerry\nBNCS Engineer, SI SOL Telco & Media Vertical Practice\nM: +44 (0)7812 325518\n2nd Floor, MidCity Place, 71 High Holborn, London, WC1V 6EA\nrichard.kerry@atos.net\n\nThis e-mail and the documents attached are confidential and intended solely for the addressee; it may also be privileged. If you receive this e-mail in error, please notify the sender immediately and destroy it. As its integrity cannot be secured on the Internet, the Atos group liability cannot be triggered for the message content. Although the sender endeavours to maintain a computer virus-free network, the sender does not warrant that this transmission is virus-free and will not be liable for any damages resulting from any virus transmitted.\n\n\n________________________________\nFrom: git-owner@vger.kernel.org on behalf of René Scharfe \nSent: 02 August 2018 20:18\nTo: Jeff King; Junio C Hamano\nCc: Ævar Arnfjörð Bjarmason; git@vger.kernel.org\nSubject: [PATCH 6/6] parse-options: automatically infer PARSE_OPT_LITERAL_ARGHELP\n\nParseopt wraps argument help strings in a pair of angular brackets by\ndefault, to tell users that they need to replace it with an actual\nvalue. This is useful in most cases, because most option arguments\nare indeed single values of a certain type. The option\nPARSE_OPT_LITERAL_ARGHELP needs to be used in option definitions with\narguments that have multiple parts or are literal strings.\n\nStop adding these angular brackets if special characters are present,\nas they indicate that we don't deal with a simple placeholder. This\nsimplifies the code a bit and makes defining special options slightly\neasier.\n\nRemove the flag PARSE_OPT_LITERAL_ARGHELP in the cases where the new\nand more cautious handling suffices.\n\nSigned-off-by: Rene Scharfe \n---\nThe patch to add PARSE_OPT_LITERAL_ARGHELP for push --force-with-lease\nshould be applied before this one.\n\n builtin/add.c | 5 ++---\n builtin/pack-objects.c | 2 +-\n builtin/read-tree.c | 2 +-\n builtin/send-pack.c | 3 +--\n builtin/shortlog.c | 3 +--\n builtin/show-branch.c | 2 +-\n builtin/update-index.c | 2 +-\n builtin/write-tree.c | 5 ++---\n parse-options.c | 3 ++-\n 9 files changed, 12 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 84bfec9b73..ba1ff5689d 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -304,9 +304,8 @@ static struct option builtin_add_options[] = {\n OPT_BOOL( 0 , \"refresh\", &refresh_only, N_(\"don't add, only refresh the index\")),\n OPT_BOOL( 0 , \"ignore-errors\", &ignore_add_errors, N_(\"just skip files which cannot be added because of errors\")),\n OPT_BOOL( 0 , \"ignore-missing\", &ignore_missing, N_(\"check if - even missing - files are ignored in dry run\")),\n- { OPTION_STRING, 0, \"chmod\", &chmod_arg, \"(+|-)x\",\n- N_(\"override the executable bit of the listed files\"),\n- PARSE_OPT_LITERAL_ARGHELP },\n+ OPT_STRING(0, \"chmod\", &chmod_arg, \"(+|-)x\",\n+ N_(\"override the executable bit of the listed files\")),\n OPT_HIDDEN_BOOL(0, \"warn-embedded-repo\", &warn_on_embedded_repo,\n N_(\"warn when adding an embedded repository\")),\n OPT_END(),\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 3a5d1fa317..b2323613bc 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -3112,7 +3112,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n N_(\"similar to --all-progress when progress meter is shown\")),\n { OPTION_CALLBACK, 0, \"index-version\", NULL, N_(\"[,]\"),\n N_(\"write the pack index file in the specified idx format version\"),\n- PARSE_OPT_LITERAL_ARGHELP, option_parse_index_version },\n+ 0, option_parse_index_version },\n OPT_MAGNITUDE(0, \"max-pack-size\", &pack_size_limit,\n N_(\"maximum size of each output pack file\")),\n OPT_BOOL(0, \"local\", &local,\ndiff --git a/builtin/read-tree.c b/builtin/read-tree.c\nindex ebc43eb805..fbbc98e516 100644\n--- a/builtin/read-tree.c\n+++ b/builtin/read-tree.c\n@@ -133,7 +133,7 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)\n N_(\"same as -m, but discard unmerged entries\")),\n { OPTION_STRING, 0, \"prefix\", &opts.prefix, N_(\"/\"),\n N_(\"read the tree into the index under /\"),\n- PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP },\n+ PARSE_OPT_NONEG },\n OPT_BOOL('u', NULL, &opts.update,\n N_(\"update working tree with merge result\")),\n { OPTION_CALLBACK, 0, \"exclude-per-directory\", &opts,\ndiff --git a/builtin/send-pack.c b/builtin/send-pack.c\nindex 0b517c9368..724b484850 100644\n--- a/builtin/send-pack.c\n+++ b/builtin/send-pack.c\n@@ -180,8 +180,7 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)\n { OPTION_CALLBACK,\n 0, CAS_OPT_NAME, &cas, N_(\":\"),\n N_(\"require old value of ref to be at this value\"),\n- PARSE_OPT_OPTARG | PARSE_OPT_LITERAL_ARGHELP,\n- parseopt_push_cas_option },\n+ PARSE_OPT_OPTARG, parseopt_push_cas_option },\n OPT_END()\n };\n\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex f555cf9e4f..3898a2c9c4 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -269,8 +269,7 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)\n OPT_BOOL('e', \"email\", &log.email,\n N_(\"Show the email address of each author\")),\n { OPTION_CALLBACK, 'w', NULL, &log, N_(\"[,[,]]\"),\n- N_(\"Linewrap output\"),\n- PARSE_OPT_OPTARG | PARSE_OPT_LITERAL_ARGHELP,\n+ N_(\"Linewrap output\"), PARSE_OPT_OPTARG,\n &parse_wrap_args },\n OPT_END(),\n };\ndiff --git a/builtin/show-branch.c b/builtin/show-branch.c\nindex f2e985c00a..9106da1985 100644\n--- a/builtin/show-branch.c\n+++ b/builtin/show-branch.c\n@@ -673,7 +673,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n { OPTION_CALLBACK, 'g', \"reflog\", &reflog_base, N_(\"[,]\"),\n N_(\"show most recent ref-log entries starting at \"\n \"base\"),\n- PARSE_OPT_OPTARG | PARSE_OPT_LITERAL_ARGHELP,\n+ PARSE_OPT_OPTARG,\n parse_reflog_param },\n OPT_END()\n };\ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex 7feda6e271..293f59247b 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -973,7 +973,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n (parse_opt_cb *) cacheinfo_callback},\n {OPTION_CALLBACK, 0, \"chmod\", &set_executable_bit, \"(+|-)x\",\n N_(\"override the executable bit of the listed files\"),\n- PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,\n+ PARSE_OPT_NONEG,\n chmod_callback},\n {OPTION_SET_INT, 0, \"assume-unchanged\", &mark_valid_only, NULL,\n N_(\"mark files as \\\"not changing\\\"\"),\ndiff --git a/builtin/write-tree.c b/builtin/write-tree.c\nindex c9d3c544e7..cdcbf8264e 100644\n--- a/builtin/write-tree.c\n+++ b/builtin/write-tree.c\n@@ -24,9 +24,8 @@ int cmd_write_tree(int argc, const char **argv, const char *unused_prefix)\n struct option write_tree_options[] = {\n OPT_BIT(0, \"missing-ok\", &flags, N_(\"allow missing objects\"),\n WRITE_TREE_MISSING_OK),\n- { OPTION_STRING, 0, \"prefix\", &prefix, N_(\"/\"),\n- N_(\"write tree object for a subdirectory \") ,\n- PARSE_OPT_LITERAL_ARGHELP },\n+ OPT_STRING(0, \"prefix\", &prefix, N_(\"/\"),\n+ N_(\"write tree object for a subdirectory \")),\n { OPTION_BIT, 0, \"ignore-cache-tree\", &flags, NULL,\n N_(\"only useful for debugging\"),\n PARSE_OPT_HIDDEN | PARSE_OPT_NOARG, NULL,\ndiff --git a/parse-options.c b/parse-options.c\nindex 7db84227ab..3b874a83a0 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -660,7 +660,8 @@ int parse_options(int argc, const char **argv, const char *prefix,\n static int usage_argh(const struct option *opts, FILE *outfile)\n {\n const char *s;\n- int literal = (opts->flags & PARSE_OPT_LITERAL_ARGHELP) || !opts->argh;\n+ int literal = (opts->flags & PARSE_OPT_LITERAL_ARGHELP) ||\n+ !opts->argh || !!strpbrk(opts->argh, \"()<>[]|\");\n if (opts->flags & PARSE_OPT_OPTARG)\n if (opts->long_name)\n s = literal ? \"[=%s]\" : \"[=<%s>]\";\n--\n2.18.0\nAtos, Atos Consulting, Worldline and Canopy The Open Cloud Company are trading names used by the Atos group. The following trading entities are registered in England and Wales: Atos IT Services UK Limited (registered number 01245534), Atos Consulting Limited (registered number 04312380), Atos Worldline UK Limited (registered number 08514184) and Canopy The Open Cloud Company Limited (registration number 08011902). The registered office for each is at Second Floor, Mid City Place, 71 High Holborn, London, WC1V 6EA. The VAT No. for each is: GB232327983.\n\nThis e-mail and the documents attached are confidential and intended solely for the addressee, and may contain confidential or privileged information. If you receive this e-mail in error, you are not authorised to copy, disclose, use or retain it. Please notify the sender immediately and delete this email from your systems. As emails may be intercepted, amended or lost, they are not secure. Atos therefore can accept no liability for any errors or their content. Although Atos endeavours to maintain a virus-free network, we do not warrant that this transmission is virus-free and can accept no liability for any damages resulting from any virus transmitted. The risks are deemed to be accepted by everyone who communicates with Atos by email.\n"},{"id":"354371","messageId":"AM0PR02MB37155300E283E58E3614E1899C230@AM0PR02MB3715.eurprd02.prod.outlook.com","threadId":"49002","inReplyTo":"757ca573-3954-9c31-8a7b-56a2de8e3793@web.de","subject":"Re: [PATCH 6/6] parse-options: automatically infer PARSE_OPT_LITERAL_ARGHELP","fromName":"Kerry, Richard","fromEmail":"richard.kerry@atos.net","sentAt":"2018-08-03T10:39:10Z","receivedAt":"2018-08-03T10:39:18Z","isPatch":true,"sender":{"key":"richard.kerry@atos.net","avatar":null},"body":"\ns/angular brackets/angle brackets/\n\nI've never seen these called \"angular brackets\". Maybe a non-native English speaker issue.\n\n\n\nRegards,\nRichard.\n\nPS. Please excuse my sending this twice, I don't seem to have default settings that are compatible with the list.\n\n\n\n\nRichard Kerry\nBNCS Engineer, SI SOL Telco & Media Vertical Practice\nM: +44 (0)7812 325518\n2nd Floor, MidCity Place, 71 High Holborn, London, WC1V 6EA\nrichard.kerry@atos.net\n\n\n\n This e-mail and the documents attached are confidential and intended solely for the addressee; it may also be privileged. If you receive this e-mail in error, please notify the sender immediately and destroy it. As its integrity cannot be secured on the Internet, the Atos group liability cannot be triggered for the message content. Although the sender endeavours to maintain a computer virus-free network, the sender does not warrant that this transmission is virus-free and will not be liable for any damages resulting from any virus transmitted.\n\n\n\n\nFrom: git-owner@vger.kernel.org on behalf of René Scharfe \nSent: 02 August 2018 20:18\nTo: Jeff King; Junio C Hamano\nCc: Ævar Arnfjörð Bjarmason; git@vger.kernel.org\nSubject: [PATCH 6/6] parse-options: automatically infer PARSE_OPT_LITERAL_ARGHELP\n\n\nParseopt wraps argument help strings in a pair of angular brackets by\ndefault, to tell users that they need to replace it with an actual\nvalue. This is useful in most cases, because most option arguments\nare indeed single values of a certain type. The option\nPARSE_OPT_LITERAL_ARGHELP needs to be used in option definitions with\narguments that have multiple parts or are literal strings.\n\nStop adding these angular brackets if special characters are present,\nas they indicate that we don't deal with a simple placeholder. This\nsimplifies the code a bit and makes defining special options slightly\neasier.\n\nRemove the flag PARSE_OPT_LITERAL_ARGHELP in the cases where the new\nand more cautious handling suffices.\n\nSigned-off-by: Rene Scharfe \n---\nThe patch to add PARSE_OPT_LITERAL_ARGHELP for push --force-with-lease\nshould be applied before this one.\n\n builtin/add.c | 5 ++---\n builtin/pack-objects.c | 2 +-\n builtin/read-tree.c | 2 +-\n builtin/send-pack.c | 3 +--\n builtin/shortlog.c | 3 +--\n builtin/show-branch.c | 2 +-\n builtin/update-index.c | 2 +-\n builtin/write-tree.c | 5 ++---\n parse-options.c | 3 ++-\n 9 files changed, 12 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 84bfec9b73..ba1ff5689d 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -304,9 +304,8 @@ static struct option builtin_add_options[] = {\n OPT_BOOL( 0 , \"refresh\", &refresh_only, N_(\"don't add, only refresh the index\")),\n OPT_BOOL( 0 , \"ignore-errors\", &ignore_add_errors, N_(\"just skip files which cannot be added because of errors\")),\n OPT_BOOL( 0 , \"ignore-missing\", &ignore_missing, N_(\"check if - even missing - files are ignored in dry run\")),\n- { OPTION_STRING, 0, \"chmod\", &chmod_arg, \"(+|-)x\",\n- N_(\"override the executable bit of the listed files\"),\n- PARSE_OPT_LITERAL_ARGHELP },\n+ OPT_STRING(0, \"chmod\", &chmod_arg, \"(+|-)x\",\n+ N_(\"override the executable bit of the listed files\")),\n OPT_HIDDEN_BOOL(0, \"warn-embedded-repo\", &warn_on_embedded_repo,\n N_(\"warn when adding an embedded repository\")),\n OPT_END(),\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 3a5d1fa317..b2323613bc 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -3112,7 +3112,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n N_(\"similar to --all-progress when progress meter is shown\")),\n { OPTION_CALLBACK, 0, \"index-version\", NULL, N_(\"[,]\"),\n N_(\"write the pack index file in the specified idx format version\"),\n- PARSE_OPT_LITERAL_ARGHELP, option_parse_index_version },\n+ 0, option_parse_index_version },\n OPT_MAGNITUDE(0, \"max-pack-size\", &pack_size_limit,\n N_(\"maximum size of each output pack file\")),\n OPT_BOOL(0, \"local\", &local,\ndiff --git a/builtin/read-tree.c b/builtin/read-tree.c\nindex ebc43eb805..fbbc98e516 100644\n--- a/builtin/read-tree.c\n+++ b/builtin/read-tree.c\n@@ -133,7 +133,7 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)\n N_(\"same as -m, but discard unmerged entries\")),\n { OPTION_STRING, 0, \"prefix\", &opts.prefix, N_(\"/\"),\n N_(\"read the tree into the index under /\"),\n- PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP },\n+ PARSE_OPT_NONEG },\n OPT_BOOL('u', NULL, &opts.update,\n N_(\"update working tree with merge result\")),\n { OPTION_CALLBACK, 0, \"exclude-per-directory\", &opts,\ndiff --git a/builtin/send-pack.c b/builtin/send-pack.c\nindex 0b517c9368..724b484850 100644\n--- a/builtin/send-pack.c\n+++ b/builtin/send-pack.c\n@@ -180,8 +180,7 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)\n { OPTION_CALLBACK,\n 0, CAS_OPT_NAME, &cas, N_(\":\"),\n N_(\"require old value of ref to be at this value\"),\n- PARSE_OPT_OPTARG | PARSE_OPT_LITERAL_ARGHELP,\n- parseopt_push_cas_option },\n+ PARSE_OPT_OPTARG, parseopt_push_cas_option },\n OPT_END()\n };\n\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex f555cf9e4f..3898a2c9c4 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -269,8 +269,7 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)\n OPT_BOOL('e', \"email\", &log.email,\n N_(\"Show the email address of each author\")),\n { OPTION_CALLBACK, 'w', NULL, &log, N_(\"[,[,]]\"),\n- N_(\"Linewrap output\"),\n- PARSE_OPT_OPTARG | PARSE_OPT_LITERAL_ARGHELP,\n+ N_(\"Linewrap output\"), PARSE_OPT_OPTARG,\n &parse_wrap_args },\n OPT_END(),\n };\ndiff --git a/builtin/show-branch.c b/builtin/show-branch.c\nindex f2e985c00a..9106da1985 100644\n--- a/builtin/show-branch.c\n+++ b/builtin/show-branch.c\n@@ -673,7 +673,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n { OPTION_CALLBACK, 'g', \"reflog\", &reflog_base, N_(\"[,]\"),\n N_(\"show most recent ref-log entries starting at \"\n \"base\"),\n- PARSE_OPT_OPTARG | PARSE_OPT_LITERAL_ARGHELP,\n+ PARSE_OPT_OPTARG,\n parse_reflog_param },\n OPT_END()\n };\ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex 7feda6e271..293f59247b 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -973,7 +973,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n (parse_opt_cb *) cacheinfo_callback},\n {OPTION_CALLBACK, 0, \"chmod\", &set_executable_bit, \"(+|-)x\",\n N_(\"override the executable bit of the listed files\"),\n- PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,\n+ PARSE_OPT_NONEG,\n chmod_callback},\n {OPTION_SET_INT, 0, \"assume-unchanged\", &mark_valid_only, NULL,\n N_(\"mark files as \\\"not changing\\\"\"),\ndiff --git a/builtin/write-tree.c b/builtin/write-tree.c\nindex c9d3c544e7..cdcbf8264e 100644\n--- a/builtin/write-tree.c\n+++ b/builtin/write-tree.c\n@@ -24,9 +24,8 @@ int cmd_write_tree(int argc, const char **argv, const char *unused_prefix)\n struct option write_tree_options[] = {\n OPT_BIT(0, \"missing-ok\", &flags, N_(\"allow missing objects\"),\n WRITE_TREE_MISSING_OK),\n- { OPTION_STRING, 0, \"prefix\", &prefix, N_(\"/\"),\n- N_(\"write tree object for a subdirectory \") ,\n- PARSE_OPT_LITERAL_ARGHELP },\n+ OPT_STRING(0, \"prefix\", &prefix, N_(\"/\"),\n+ N_(\"write tree object for a subdirectory \")),\n { OPTION_BIT, 0, \"ignore-cache-tree\", &flags, NULL,\n N_(\"only useful for debugging\"),\n PARSE_OPT_HIDDEN | PARSE_OPT_NOARG, NULL,\ndiff --git a/parse-options.c b/parse-options.c\nindex 7db84227ab..3b874a83a0 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -660,7 +660,8 @@ int parse_options(int argc, const char **argv, const char *prefix,\n static int usage_argh(const struct option *opts, FILE *outfile)\n {\n const char *s;\n- int literal = (opts->flags & PARSE_OPT_LITERAL_ARGHELP) || !opts->argh;\n+ int literal = (opts->flags & PARSE_OPT_LITERAL_ARGHELP) ||\n+ !opts->argh || !!strpbrk(opts->argh, \"()<>[]|\");\n if (opts->flags & PARSE_OPT_OPTARG)\n if (opts->long_name)\n s = literal ? \"[=%s]\" : \"[=<%s>]\";\n--\n2.18.0\n\nAtos, Atos Consulting, Worldline and Canopy The Open Cloud Company are trading names used by the Atos group. The following trading entities are registered in England and Wales: Atos IT Services UK Limited (registered number 01245534), Atos Consulting Limited (registered number 04312380), Atos Worldline UK Limited (registered number 08514184) and Canopy The Open Cloud Company Limited (registration number 08011902). The registered office for each is at Second Floor, Mid City Place, 71 High Holborn, London, WC1V 6EA. The VAT No. for each is: GB232327983.\n\nThis e-mail and the documents attached are confidential and intended solely for the addressee, and may contain confidential or privileged information. If you receive this e-mail in error, you are not authorised to copy, disclose, use or retain it. Please notify the sender immediately and delete this email from your systems. As emails may be intercepted, amended or lost, they are not secure. Atos therefore can accept no liability for any errors or their content. Although Atos endeavours to maintain a virus-free network, we do not warrant that this transmission is virus-free and can accept no liability for any damages resulting from any virus transmitted. The risks are deemed to be accepted by everyone who communicates with Atos by email.\n"},{"id":"354393","messageId":"xmqqa7q31naq.fsf@gitster-ct.c.googlers.com","threadId":"49002","inReplyTo":"ea253620-ed28-54a4-0e52-16cbf4ec8451@web.de","subject":"Re: [PATCH] push: comment on a funny unbalanced option help","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-03T15:29:17Z","receivedAt":"2018-08-03T15:29:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe writes:\n\n> Am 02.08.2018 um 22:01 schrieb Junio C Hamano:\n>> \n>> Straying sideways into a tangent, but do we know if any locale wants\n>> to use something other than \"<>\" as an enclosing braket around a\n>> placeholder?\n>\n> Bulgarian seems to use capitals instead; here are some examples found\n> with git grep -A1 'msgid \"<' po/:\n>\n> po/bg.po:msgid \"\"\n> po/bg.po-msgstr \"ОТДАЛЕЧЕНО_ХРАНИЛИЩЕ\"\n> ...\n>> \n>> Perhaps we should do _(\"<%s>\") here? That way, the result would\n>> hopefully be made consistent with\n>> \n>> \tN_(\"[:]\")\twith LIT-ARG-HELP\n>> \n>> which allows translator to use the bracket of the locale's choice.\n\nI did not consider locales that do not use any kind of bracket, but\nI guess _(\"<%s>\") would work for them, too, in this context. When\nthe locale's convention is to upcase, for example, the arg-help text\nthat is not marked with PARSE_OPT_LITERAL_ARGHELP would be upcased,\nand _(\"<%s>\") in the usage_argh() can be translated to \"%s\", which\npasses the translated (i.e. upcased) arg-help text through.\n"},{"id":"354394","messageId":"xmqq600r1n98.fsf@gitster-ct.c.googlers.com","threadId":"49002","inReplyTo":"3120e9b3-f3e7-465c-63c3-084d3f010e1e@web.de","subject":"Re: Re* [PATCH] push: comment on a funny unbalanced option help","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-03T15:30:11Z","receivedAt":"2018-08-03T15:30:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe writes:\n\n> Am 02.08.2018 um 22:36 schrieb Junio C Hamano:\n>> Ævar Arnfjörð Bjarmason writes:\n>> \n>>> Thanks, FWIW that's fine by me, and also if you want to drop this \"fake\"\n>>> patch of mine and replace it with something René came up with (I have\n>>> not yet read his 1-6 patches submitted on this topic, so maybe they're\n>>> not mutually exclusive).\n>> \n>> I think the 6-patch series supersedes this one. Thanks anyway.\n>\n> Actually no, I specifically avoided touching builtin/push.c because this\n> patch already handled it; it should still be applied before patch 6 from\n> my series.\n\nYou're of course correct. Thanks.\n"}]}