{"thread":{"id":"53005","subject":"[PATCH 0/3] I keep typing \"git log --no-mailmap\" X-<","startedAt":"2020-03-16T21:29:06Z","lastAt":"2020-03-16T22:38:41Z","messageCount":6,"participants":["Junio C Hamano","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"393330","messageId":"20200316212857.259093-1-gitster@pobox.com","threadId":"53005","inReplyTo":null,"subject":"[PATCH 0/3] I keep typing \"git log --no-mailmap\" X-<","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-03-16T21:28:54Z","receivedAt":"2020-03-16T21:29:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The ultimate objective of this miniseries is to allow me to say\n\"git log --no-mailmap\" by adding an alias \"--mailmap\" to the \nexisting \"--use-mailmap\" option.\n\nBut I had to do a little cleaning up of the OPT_ALIAS() interface,\nwhich is the topic of the first two patches. Without them, an\noption that is an alias to another option is shown in \"git cmd -h\"\noutput like this illustration:\n\n $ git clone -h\n usage: git clone [] [--] []\n\t...\n\t--recursive[=]\n\t\t\t initialize submodules in the clone\n\t--recurse-submodules[=]\n\t\t\t initialize submodules in the clone\n\t-j, --jobs number of submodules cloned in parallel\n\t...\n\nThe \"recursive\" option is defined as an alias to \"recurse-submodules\",\nbut with exactly the same short text, the user may realize that they\nare identical, or they may suspect they are largely similar with\nsubtle differences that the short help text cannot adequately convey.\n\nWith the first two patches, the output becomes like this:\n\n $ git clone -h\n usage: git clone [] [--] []\n\t...\n\t--recurse-submodules[=]\n\t\t\t initialize submodules in the clone\n\t--recursive[=]\n\t\t\t alias of --recurse-submodules\n\t-j, --jobs number of submodules cloned in parallel\n\t...\n\nwhich would make it clear that the latter is a way to spell the same\nthing as the other.\n\nJunio C Hamano (3):\n parse-options: teach \"git cmd -h\" to show alias as alias\n clone: reorder --recursive/--recurse-submodules\n log: give --[no-]use-mailmap a more sensible synonym --[no-]mailmap\n\n Documentation/git-log.txt | 1 +\n builtin/clone.c | 2 +-\n builtin/log.c | 1 +\n parse-options.c | 9 +++------\n t/t0040-parse-options.sh | 2 +-\n 5 files changed, 7 insertions(+), 8 deletions(-)\n\n-- \n2.26.0-rc1-11-g30e9940356\n\n\n"},{"id":"393331","messageId":"20200316212857.259093-3-gitster@pobox.com","threadId":"53005","inReplyTo":"20200316212857.259093-1-gitster@pobox.com","subject":"[PATCH 2/3] clone: reorder --recursive/--recurse-submodules","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-03-16T21:28:56Z","receivedAt":"2020-03-16T21:29:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The previous step made an option that is an alias to another option\nidentify itself as an alias to the latter. Because it is easier to\nscan the list when a pointer goes backward to what a reader already\nhas seen, mention \"recurse-submodules\" first with its true short\nhelp string, and then \"recurse\" with the statement that it is a\nsynonym to \"recurse-submodules\".\n\nSigned-off-by: Junio C Hamano \n---\n\n The history of --recursive/--recurse-submodules options dates back\n to ccdd3da6 (clone: add the --recurse-submodules option as alias\n for --recursive, 2010-11-04), where we decided to encourage use of\n more descriptive \"--recurse-submodules\" over \"--recursive\" which\n does not say to what extent the process is allowed to recurse.\n Documentation/git-clone.txt mentions only \"--recurse-submodules\"\n for this exact reason, and it may not be a bad idea to start\n planning to deprecate the alias \"--recursive\". \n\n But that obviously is a separate topic.\n\n builtin/clone.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 1ad26f4d8c..54b0acbe73 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -102,10 +102,10 @@ static struct option builtin_clone_options[] = {\n \t\t N_(\"don't use local hardlinks, always copy\")),\n \tOPT_BOOL('s', \"shared\", &option_shared,\n \t\t N_(\"setup as shared repository\")),\n-\tOPT_ALIAS(0, \"recursive\", \"recurse-submodules\"),\n \t{ OPTION_CALLBACK, 0, \"recurse-submodules\", &option_recurse_submodules,\n \t N_(\"pathspec\"), N_(\"initialize submodules in the clone\"),\n \t PARSE_OPT_OPTARG, recurse_submodules_cb, (intptr_t)\".\" },\n+\tOPT_ALIAS(0, \"recursive\", \"recurse-submodules\"),\n \tOPT_INTEGER('j', \"jobs\", &max_jobs,\n \t\t N_(\"number of submodules cloned in parallel\")),\n \tOPT_STRING(0, \"template\", &option_template, N_(\"template-directory\"),\n-- \n2.26.0-rc1-11-g30e9940356\n\n"},{"id":"393332","messageId":"20200316212857.259093-2-gitster@pobox.com","threadId":"53005","inReplyTo":"20200316212857.259093-1-gitster@pobox.com","subject":"[PATCH 1/3] parse-options: teach \"git cmd -h\" to show alias as alias","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-03-16T21:28:55Z","receivedAt":"2020-03-16T21:29:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"There is a long-standing NEEDSWORK comment that complains about\ninconsistency between how an aliased option (\"git clone --recurse\"\nwhich is the only one that currently exists) gives a help text in\na usage-error message vs \"git cmd -h\"). Get rid of it and then\nmake sure we say an option is an alias for another, instead of\nrepeating the same short help text for both, which leads to \"they\nseem to do the same---is there any subtle difference?\" puzzlement\nto end-users.\n\nSigned-off-by: Junio C Hamano \n---\n\n * I couldn't easily see how to trigger usage_with_options() with\n the original options[], which is the potential cause of the\n inconsistency the NEEDSWORK comment mentions. But if there were,\n this should take care of it. Our test suite did not have an\n example, though.\n\n parse-options.c | 9 +++------\n t/t0040-parse-options.sh | 2 +-\n 2 files changed, 4 insertions(+), 7 deletions(-)\n\ndiff --git a/parse-options.c b/parse-options.c\nindex 63d6bab60c..c57618d537 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -648,6 +648,7 @@ static struct option *preprocess_options(struct parse_opt_ctx_t *ctx,\n \t\tint short_name;\n \t\tconst char *long_name;\n \t\tconst char *source;\n+\t\tstruct strbuf help = STRBUF_INIT;\n \t\tint j;\n \n \t\tif (newopt[i].type != OPTION_ALIAS)\n@@ -659,6 +660,7 @@ static struct option *preprocess_options(struct parse_opt_ctx_t *ctx,\n \n \t\tif (!long_name)\n \t\t\tBUG(\"An alias must have long option name\");\n+\t\tstrbuf_addf(&help, _(\"alias of --%s\"), source);\n \n \t\tfor (j = 0; j < nr; j++) {\n \t\t\tconst char *name = options[j].long_name;\n@@ -669,15 +671,10 @@ static struct option *preprocess_options(struct parse_opt_ctx_t *ctx,\n \t\t\tif (options[j].type == OPTION_ALIAS)\n \t\t\t\tBUG(\"No please. Nested aliases are not supported.\");\n \n-\t\t\t/*\n-\t\t\t * NEEDSWORK: this is a bit inconsistent because\n-\t\t\t * usage_with_options() on the original options[] will print\n-\t\t\t * help string as \"alias of %s\" but \"git cmd -h\" will\n-\t\t\t * print the original help string.\n-\t\t\t */\n \t\t\tmemcpy(newopt + i, options + j, sizeof(*newopt));\n \t\t\tnewopt[i].short_name = short_name;\n \t\t\tnewopt[i].long_name = long_name;\n+\t\t\tnewopt[i].help = strbuf_detach(&help, NULL);\n \t\t\tbreak;\n \t\t}\n \ndiff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh\nindex 3483b72db4..f8178ee4e3 100755\n--- a/t/t0040-parse-options.sh\n+++ b/t/t0040-parse-options.sh\n@@ -54,7 +54,7 @@ Alias\n -A, --alias-source \n get a string\n -Z, --alias-target \n- get a string\n+ alias of --alias-source\n \n EOF\n \n-- \n2.26.0-rc1-11-g30e9940356\n\n"},{"id":"393333","messageId":"20200316212857.259093-4-gitster@pobox.com","threadId":"53005","inReplyTo":"20200316212857.259093-1-gitster@pobox.com","subject":"[PATCH 3/3] log: give --[no-]use-mailmap a more sensible synonym --[no-]mailmap","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-03-16T21:28:57Z","receivedAt":"2020-03-16T21:29:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The option name \"--use-mailmap\" looks OK, but it becomes awkward\nwhen you have to negate it, i.e. \"--no-use-mailmap\". I, perhaps\nwith many other users, always try \"--no-mailmap\" and become unhappy\nto see it fail.\n\nAdd an alias \"--[no-]mailmap\" to remedy this.\n\nSigned-off-by: Junio C Hamano \n---\n Documentation/git-log.txt | 1 +\n builtin/log.c | 1 +\n 2 files changed, 2 insertions(+)\n\ndiff --git a/Documentation/git-log.txt b/Documentation/git-log.txt\nindex bed09bb09e..619577f23b 100644\n--- a/Documentation/git-log.txt\n+++ b/Documentation/git-log.txt\n@@ -49,6 +49,7 @@ OPTIONS\n \tPrint out the ref name given on the command line by which each\n \tcommit was reached.\n \n+--[no-]mailmap::\n --[no-]use-mailmap::\n \tUse mailmap file to map author and committer names and email\n \taddresses to canonical real names and email addresses. See\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 83a4a6188e..ca1e789ba0 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -173,6 +173,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n \t\tOPT__QUIET(&quiet, N_(\"suppress diff output\")),\n \t\tOPT_BOOL(0, \"source\", &source, N_(\"show source\")),\n \t\tOPT_BOOL(0, \"use-mailmap\", &mailmap, N_(\"Use mail map file\")),\n+\t\tOPT_ALIAS(0, \"mailmap\", \"use-mailmap\"),\n \t\tOPT_STRING_LIST(0, \"decorate-refs\", &decorate_refs_include,\n \t\t\t\tN_(\"pattern\"), N_(\"only decorate refs that match \")),\n \t\tOPT_STRING_LIST(0, \"decorate-refs-exclude\", &decorate_refs_exclude,\n-- \n2.26.0-rc1-11-g30e9940356\n\n"},{"id":"393334","messageId":"CAPig+cRj5c-AEOySWkfM=PX-CUFO-R-cN8pgyTm0kD32xyhihA@mail.gmail.com","threadId":"53005","inReplyTo":"20200316212857.259093-4-gitster@pobox.com","subject":"Re: [PATCH 3/3] log: give --[no-]use-mailmap a more sensible synonym --[no-]mailmap","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-03-16T21:39:50Z","receivedAt":"2020-03-16T21:40:05Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Mar 16, 2020 at 5:29 PM Junio C Hamano wrote:\n> The option name \"--use-mailmap\" looks OK, but it becomes awkward\n> when you have to negate it, i.e. \"--no-use-mailmap\". I, perhaps\n> with many other users, always try \"--no-mailmap\" and become unhappy\n> to see it fail.\n>\n> Add an alias \"--[no-]mailmap\" to remedy this.\n>\n> Signed-off-by: Junio C Hamano \n> ---\n> diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt\n> @@ -49,6 +49,7 @@ OPTIONS\n> +--[no-]mailmap::\n> --[no-]use-mailmap::\n> Use mailmap file to map author and committer names and email\n> addresses to canonical real names and email addresses. See\n\nHere, the documentation seems to promote --mailmap over --use-mailmap.\n\n> diff --git a/builtin/log.c b/builtin/log.c\n> @@ -173,6 +173,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n> OPT_BOOL(0, \"use-mailmap\", &mailmap, N_(\"Use mail map file\")),\n> + OPT_ALIAS(0, \"mailmap\", \"use-mailmap\"),\n\nSo, along the lines of patch 2/3, I wonder if this should instead make\n--mailmap the \"real\" option and --use-mailmap the alias; namely, use\nOPT_ALIAS for --use-mailmap and place it after --mailmap. (Genuine but\nvery minor question; should not hold up acceptance of patch.)\n"},{"id":"393335","messageId":"xmqqk13khqmw.fsf@gitster.c.googlers.com","threadId":"53005","inReplyTo":"CAPig+cRj5c-AEOySWkfM=PX-CUFO-R-cN8pgyTm0kD32xyhihA@mail.gmail.com","subject":"Re: [PATCH 3/3] log: give --[no-]use-mailmap a more sensible synonym --[no-]mailmap","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-03-16T22:38:31Z","receivedAt":"2020-03-16T22:38:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> On Mon, Mar 16, 2020 at 5:29 PM Junio C Hamano wrote:\n>> The option name \"--use-mailmap\" looks OK, but it becomes awkward\n>> when you have to negate it, i.e. \"--no-use-mailmap\". I, perhaps\n>> with many other users, always try \"--no-mailmap\" and become unhappy\n>> to see it fail.\n>>\n>> Add an alias \"--[no-]mailmap\" to remedy this.\n>>\n>> Signed-off-by: Junio C Hamano \n>> ---\n>> diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt\n>> @@ -49,6 +49,7 @@ OPTIONS\n>> +--[no-]mailmap::\n>> --[no-]use-mailmap::\n>> Use mailmap file to map author and committer names and email\n>> addresses to canonical real names and email addresses. See\n>\n> Here, the documentation seems to promote --mailmap over --use-mailmap.\n>\n>> diff --git a/builtin/log.c b/builtin/log.c\n>> @@ -173,6 +173,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n>> OPT_BOOL(0, \"use-mailmap\", &mailmap, N_(\"Use mail map file\")),\n>> + OPT_ALIAS(0, \"mailmap\", \"use-mailmap\"),\n>\n> So, along the lines of patch 2/3, I wonder if this should instead make\n> --mailmap the \"real\" option and --use-mailmap the alias; namely, use\n> OPT_ALIAS for --use-mailmap and place it after --mailmap. (Genuine but\n> very minor question; should not hold up acceptance of patch.)\n\nActually, I do not think \"--use-mailmap\" is too bad. It is just\n\"--no-use-mailmap\" felt horrible. If the enable-disable interface\nfor this feature were \"--(no|use)-mailmap\", I would not have written\nthis series.\n\nThere are two subcommands other than \"log\" that has \"use-something\"\nthat needs \"--no-use-something\" to countermand:\n\n \"git fast-export\" has \"--use-done-feature\"\n \"git pack-objects\" has \"--use-bitmap-index\"\n\nSo an alternative approach which might be better is to teach\nparse-options interface to handle \"--no-something\" by doing the\nfollowing:\n\n - first find \"--something\" in the option[] array, and if there is,\n use it just like we do today;\n\n - if there is no option \"--something\" in the option[] array,\n instead of erroring out, see if \"--use-something\" is there, and\n pretend as if the user said \"--no-use-something\".\n\nI guess that is very similar to the way we avoid \"--no-no-frotz\" for\noption[] entries whose long name already begins with \"--no-\".\n\n\n[Footnote]\n\nOptionally, when the command line option says \"--something\", and\nthere is no such entry in the option[] array, we could check it with\nthe \"use-\" prefix, and pretend that \"--use-something\" was what the\nuser asked if there is such an entry. But if we were to go that\nroute, it is not all that different to have an alias, as there are\nonly three subcommands (counting \"log\") that has the \"use-something\"\n(and one of them, i.e. \"pack-objects\", is not even an end-user\nfacing command).\n"}]}