{"thread":{"id":"48019","subject":"[PATCH 0/3] shortlog: do not accept revisions when run outside repo","startedAt":"2018-03-10T11:52:28Z","lastAt":"2018-05-10T07:11:08Z","messageCount":24,"participants":["Martin Ågren","Junio C Hamano","Jonathan Nieder","Jeff King","brian m. carlson"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"341387","messageId":"cover.1520680894.git.martin.agren@gmail.com","threadId":"48019","inReplyTo":null,"subject":"[PATCH 0/3] shortlog: do not accept revisions when run outside repo","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2018-03-10T11:52:09Z","receivedAt":"2018-03-10T11:52:28Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"Patch 3 stops git shortlog from BUG-ing when it's being used slightly\nwrong. Patches 1 and 2 are recursive preparation. Based on maint.\n\nSomeone trying this out might notice that `man git-shortlog` renders\n\"\\--\" as \"\\--\", which is not wanted. (Also visible on git-scm.com...)\nThere is quite some history around such double-slashes and compatibility\nwith AsciiDoc-versions, so I'd rather not do a \"while at it\" there.\nRegardless of the destiny of patch 1/3, I will follow up later to\naddress various forms of \"\\--\" throughout the tree.\n\nMartin Ågren (3):\n git-shortlog.txt: reorder usages\n shortlog: add usage-string for stdin-reading\n shortlog: do not accept revisions when run outside repo\n\n Documentation/git-shortlog.txt | 2 +-\n t/t4201-shortlog.sh | 5 +++++\n builtin/shortlog.c | 9 ++++++++-\n 3 files changed, 14 insertions(+), 2 deletions(-)\n\n-- \n2.16.2.246.ga4ee44448f\n\n"},{"id":"341388","messageId":"3d4119e89c8d7c1936c29cc68d7c3dcfed8a96f9.1520680894.git.martin.agren@gmail.com","threadId":"48019","inReplyTo":"cover.1520680894.git.martin.agren@gmail.com","subject":"[PATCH 1/3] git-shortlog.txt: reorder usages","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2018-03-10T11:52:10Z","receivedAt":"2018-03-10T11:52:32Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"The first usage we give is the original one where, e.g., `git log` is\npiped through `git shortlog`. The description that follows reads the\nother way round, by first focusing on the general behavior, then ending\nwith the behavior when reading from stdin.\n\nIt is also a tiny bit odd that what is probably the most common usage\nand the one a reader is probably looking for is not at the top of the\nlist. Of course, it is only a two-item list, so it is not _that_ hard to\nfind... The next commit will add the original usage to the usage string\nin builtin/shortlog.c, and it feels more natural to do so below the\nmost common usage. To avoid being inconsistent, reorder these two\nusages here first.\n\nSigned-off-by: Martin Ågren \n---\n Documentation/git-shortlog.txt | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-shortlog.txt b/Documentation/git-shortlog.txt\nindex ee6c5476c..5e35ea18a 100644\n--- a/Documentation/git-shortlog.txt\n+++ b/Documentation/git-shortlog.txt\n@@ -8,8 +8,8 @@ git-shortlog - Summarize 'git log' output\n SYNOPSIS\n --------\n [verse]\n-git log --pretty=short | 'git shortlog' []\n 'git shortlog' [] [] [[\\--] ...]\n+git log --pretty=short | 'git shortlog' []\n \n DESCRIPTION\n -----------\n-- \n2.16.2.246.ga4ee44448f\n\n"},{"id":"341389","messageId":"e5f4720c077ee613518ee91f814dab82265ea981.1520680894.git.martin.agren@gmail.com","threadId":"48019","inReplyTo":"cover.1520680894.git.martin.agren@gmail.com","subject":"[PATCH 2/3] shortlog: add usage-string for stdin-reading","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2018-03-10T11:52:11Z","receivedAt":"2018-03-10T11:52:36Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"This has been missing since we learned to print usage, way back in\n4e27fb06f (add commit count options to git-shortlog, 2006-10-06).\n\nWhile at it, drop the [] around \"...\". This matches `git log -h`\nand Documentation/git-{short}log.txt. It formally makes it look like we\ndo not allow `git shortlog --`, but we gain readability and consistency.\n\nSigned-off-by: Martin Ågren \n---\n builtin/shortlog.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex e29875b84..dc4af03fc 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -11,7 +11,8 @@\n #include \"parse-options.h\"\n \n static char const * const shortlog_usage[] = {\n-\tN_(\"git shortlog [] [] [[--] [...]]\"),\n+\tN_(\"git shortlog [] [] [[--] ...]\"),\n+\tN_(\"git log --pretty=short | git shortlog []\"),\n \tNULL\n };\n \n-- \n2.16.2.246.ga4ee44448f\n\n"},{"id":"341390","messageId":"78669e644b64fc10c34adb59717d2039f81cb092.1520680894.git.martin.agren@gmail.com","threadId":"48019","inReplyTo":"cover.1520680894.git.martin.agren@gmail.com","subject":"[PATCH 3/3] shortlog: do not accept revisions when run outside repo","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2018-03-10T11:52:12Z","receivedAt":"2018-03-10T11:52:38Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"If we are outside a repo and have any arguments left after\noption-parsing, `setup_revisions()` will try to do its job and\nsomething like this will happen:\n\n$ git shortlog v2.16.0..\nBUG: environment.c:183: git environment hasn't been setup\nAborted (core dumped)\n\nThe usage is wrong, but we could obviously handle this better. Note that\ncommit abe549e179 (shortlog: do not require to run from inside a git\nrepository, 2008-03-14) explicitly enabled `git shortlog` to run from\noutside a repo, since we do not need a repo for parsing data from stdin.\n\nDisallow left-over arguments when run from outside a repo. Another\napproach would be to disallow them when reading from stdin. However, our\nlogic is currently the other way round: we check the number of revisions\nin order to decide whether we should read from stdin. (So yes, after\nthis patch, we will still silently ignore stdin for confused usage such\nas `git log v2.15.0.. | git shortlog v2.16.0..`. But at least that does\nnot crash.)\n\nSigned-off-by: Martin Ågren \n---\n t/t4201-shortlog.sh | 5 +++++\n builtin/shortlog.c | 6 ++++++\n 2 files changed, 11 insertions(+)\n\ndiff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh\nindex da10478f5..78c5645a9 100755\n--- a/t/t4201-shortlog.sh\n+++ b/t/t4201-shortlog.sh\n@@ -127,6 +127,11 @@ test_expect_success !MINGW 'shortlog can read --format=raw output' '\n \ttest_cmp expect out\n '\n \n+test_expect_success 'shortlog from non-git directory refuses revisions' '\n+\ttest_must_fail env GIT_DIR=non-existing git shortlog HEAD 2>out &&\n+\ttest_i18ngrep \"no revisions can be given\" out\n+'\n+\n test_expect_success 'shortlog should add newline when input line matches wraplen' '\n \tcat >expect <<\\EOF &&\n A U Thor (2):\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex dc4af03fc..35e8c1ead 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -293,6 +293,12 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)\n parse_done:\n \targc = parse_options_end(&ctx);\n \n+\tif (nongit && argc != 1) {\n+\t\terror(_(\"no revisions can be given when running \"\n+\t\t\t\"from outside a repository\"));\n+\t\tusage_with_options(shortlog_usage, options);\n+\t}\n+\n \tif (setup_revisions(argc, argv, &rev, NULL) != 1) {\n \t\terror(_(\"unrecognized argument: %s\"), argv[1]);\n \t\tusage_with_options(shortlog_usage, options);\n-- \n2.16.2.246.ga4ee44448f\n\n"},{"id":"341572","messageId":"xmqq37133hs2.fsf@gitster-ct.c.googlers.com","threadId":"48019","inReplyTo":"3d4119e89c8d7c1936c29cc68d7c3dcfed8a96f9.1520680894.git.martin.agren@gmail.com","subject":"Re: [PATCH 1/3] git-shortlog.txt: reorder usages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-13T19:19:25Z","receivedAt":"2018-03-13T19:19:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin Ågren writes:\n\n> The first usage we give is the original one where, e.g., `git log` is\n> piped through `git shortlog`. The description that follows reads the\n> other way round, by first focusing on the general behavior, then ending\n> with the behavior when reading from stdin.\n> ...\n\nThe result looks a lot more useful than without the patch. I think\nthe current ordering is merely a historical accident made in 2006.\n\nWill queue; thanks.\n\n> diff --git a/Documentation/git-shortlog.txt b/Documentation/git-shortlog.txt\n> index ee6c5476c..5e35ea18a 100644\n> --- a/Documentation/git-shortlog.txt\n> +++ b/Documentation/git-shortlog.txt\n> @@ -8,8 +8,8 @@ git-shortlog - Summarize 'git log' output\n> SYNOPSIS\n> --------\n> [verse]\n> -git log --pretty=short | 'git shortlog' []\n> 'git shortlog' [] [] [[\\--] ...]\n> +git log --pretty=short | 'git shortlog' []\n> \n> DESCRIPTION\n> -----------\n"},{"id":"341578","messageId":"20180313195640.GA147135@aiede.svl.corp.google.com","threadId":"48019","inReplyTo":"78669e644b64fc10c34adb59717d2039f81cb092.1520680894.git.martin.agren@gmail.com","subject":"Re: [PATCH 3/3] shortlog: do not accept revisions when run outside repo","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-03-13T19:56:40Z","receivedAt":"2018-03-13T19:57:14Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nMartin Ågren wrote:\n\n> If we are outside a repo and have any arguments left after\n> option-parsing, `setup_revisions()` will try to do its job and\n> something like this will happen:\n>\n> $ git shortlog v2.16.0..\n> BUG: environment.c:183: git environment hasn't been setup\n> Aborted (core dumped)\n\nYikes. Thanks for fixing it.\n\n[...]\n> (So yes, after\n> this patch, we will still silently ignore stdin for confused usage such\n> as `git log v2.15.0.. | git shortlog v2.16.0..`. But at least that does\n> not crash.)\n\nI don't follow here. Are you saying this command should notice that\nthere is input in stdin? How would it notice?\n\n[...]\n> --- a/t/t4201-shortlog.sh\n> +++ b/t/t4201-shortlog.sh\n> @@ -127,6 +127,11 @@ test_expect_success !MINGW 'shortlog can read --format=raw output' '\n> \ttest_cmp expect out\n> '\n> \n> +test_expect_success 'shortlog from non-git directory refuses revisions' '\n> +\ttest_must_fail env GIT_DIR=non-existing git shortlog HEAD 2>out &&\n> +\ttest_i18ngrep \"no revisions can be given\" out\n> +'\n\n\\o/\n\n[...]\n> --- a/builtin/shortlog.c\n> +++ b/builtin/shortlog.c\n> @@ -293,6 +293,12 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)\n> parse_done:\n> \targc = parse_options_end(&ctx);\n> \n> +\tif (nongit && argc != 1) {\n\nJust curious: would argc ever be 0 here? 'argc <= 1' might be clearer.\n\n> +\t\terror(_(\"no revisions can be given when running \"\n> +\t\t\t\"from outside a repository\"));\n> +\t\tusage_with_options(shortlog_usage, options);\n> +\t}\n> +\n\nThe error message is\n\n\terror: no revisions can be given when running from outside a repository\n\tusage: ...\n\nDo we need to dump usage here? I wonder if a simple die() call would\nbe easier for the user to act on.\n\nNot about this patch: I was a little surprised to see 'error:' instead\nof 'usage:' or 'fatal:'. It turns out git is pretty inconsistent\nabout that: e.g. there is\n\n\terror(_(\"no remote specified\"));\n\tusage_with_options(builtin_remote_setbranches_usage, options);\n\nSome other callers just use usage_with_options without describing the\nerror. check-attr has a private error_with_usage() helper to implement\nthe error() followed by usage_with_options() idiom. Most callers just\nuse die(), like\n\n\tdie(_(\"'%s' cannot be used with %s\"), \"--merge\", \"--patch\");\n\nDocumentation/technical/api-error-handling.txt says\n\n - `usage` is for errors in command line usage. After printing its\n message, it exits with status 129. (See also `usage_with_options`\n in the link:api-parse-options.html[parse-options API].)\n\nwhich is not prescriptive enough to help.\n\nSeparate from that, I wonder if the error message can be made a little\nshorter and clearer. E.g.\n\n\tfatal: shortlog can only be used inside a git repository\n\nThanks and hope that helps,\nJonathan\n"},{"id":"341586","messageId":"CAN0heSoP1oVWH0YAFNcL5LG_K7TsmKMAHUA_TiDacVdPtWjTZw@mail.gmail.com","threadId":"48019","inReplyTo":"20180313195640.GA147135@aiede.svl.corp.google.com","subject":"Re: [PATCH 3/3] shortlog: do not accept revisions when run outside repo","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2018-03-13T20:47:28Z","receivedAt":"2018-03-13T20:47:33Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On 13 March 2018 at 20:56, Jonathan Nieder wrote:\n> Hi,\n>\n> Martin Ågren wrote:\n>\n>> (So yes, after\n>> this patch, we will still silently ignore stdin for confused usage such\n>> as `git log v2.15.0.. | git shortlog v2.16.0..`. But at least that does\n>> not crash.)\n>\n> I don't follow here. Are you saying this command should notice that\n> there is input in stdin? How would it notice?\n\nI have no idea how it would notice (portably!) and the gain seems\nminimal. I added this to keep the reader from wondering \"but wait a\nminute, doesn't that mean that we fail to catch this bad usage if we're\nin a repo?\". So my answer would be \"yep, but it's not a huge problem\".\nOf course, my attempt to pre-emptively answer a question only provoked\nanother one. :-) I could phrase this better.\n\n>> --- a/builtin/shortlog.c\n>> +++ b/builtin/shortlog.c\n>> @@ -293,6 +293,12 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)\n>> parse_done:\n>> argc = parse_options_end(&ctx);\n>>\n>> + if (nongit && argc != 1) {\n>\n> Just curious: would argc ever be 0 here? 'argc <= 1' might be clearer.\n\nHmm, good point. It \"shouldn't\" be 0, but I guess it's better to be safe\nthan sorry. (We seem to have both constructs, in various places.)\n\n>> + error(_(\"no revisions can be given when running \"\n>> + \"from outside a repository\"));\n>> + usage_with_options(shortlog_usage, options);\n>> + }\n>> +\n>\n> The error message is\n>\n> error: no revisions can be given when running from outside a repository\n> usage: ...\n>\n> Do we need to dump usage here? I wonder if a simple die() call would\n> be easier for the user to act on.\n\nI can see an argument for \"dumping the usage makes the error harder than\nnecessary to find\". I mainly went for consistency. This ties into your\nother observations below: what little consistency do we have and in\nwhich direction do we want to push it...\n\n> Not about this patch: I was a little surprised to see 'error:' instead\n> of 'usage:' or 'fatal:'. It turns out git is pretty inconsistent\n> about that: e.g. there is\n>\n> error(_(\"no remote specified\"));\n> usage_with_options(builtin_remote_setbranches_usage, options);\n>\n> Some other callers just use usage_with_options without describing the\n> error.\n\nThe other two approaches (\"die\" and \"error and usage\") can be argued\nfor, but this one (\"give usage\") just seems wrong to me. I haven't\nlooked for such a place in the code, and maybe they're \"obvious\", but it\nseems odd to just give the usage without any sort of hint about what was\nwrong.\n\n> check-attr has a private error_with_usage() helper to implement\n> the error() followed by usage_with_options() idiom. Most callers just\n> use die(), like\n>\n> die(_(\"'%s' cannot be used with %s\"), \"--merge\", \"--patch\");\n>\n> Documentation/technical/api-error-handling.txt says\n>\n> - `usage` is for errors in command line usage. After printing its\n> message, it exits with status 129. (See also `usage_with_options`\n> in the link:api-parse-options.html[parse-options API].)\n>\n> which is not prescriptive enough to help.\n\nI think it would be a larger project to make these consistent. The one\nI'm adding here is at least consistent with the other one in this file.\n\n> Separate from that, I wonder if the error message can be made a little\n> shorter and clearer. E.g.\n>\n> fatal: shortlog can only be used inside a git repository\n\nSome grepping suggests we do not usually name the command (\"shortlog\n...\"), perhaps to play well with aliasing, nor do we use \"such \"\nvery often, but it does happen. Quoting and allowing for options might\nmake this more correct, but perhaps less readable: \"'shortlog [...]\n' can only ...\". Slightly better than what I had, \"revisions can\nonly be given inside a git repository\" would avoid some negating.\n\n> Thanks and hope that helps,\n\nIt does indeed. I'll give this another 24h and see what I come up with.\nI believe it will end up in a change to \"<= 1\", an improved error\nmessage and a clearer last few words in the commit message.\n\nMartin\n"},{"id":"341590","messageId":"20180313213628.GB147135@aiede.svl.corp.google.com","threadId":"48019","inReplyTo":"CAN0heSoP1oVWH0YAFNcL5LG_K7TsmKMAHUA_TiDacVdPtWjTZw@mail.gmail.com","subject":"Re: [PATCH 3/3] shortlog: do not accept revisions when run outside repo","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-03-13T21:36:28Z","receivedAt":"2018-03-13T21:36:35Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Martin Ågren wrote:\n> On 13 March 2018 at 20:56, Jonathan Nieder wrote:\n>> Martin Ågren wrote:\n\n>>> (So yes, after\n>>> this patch, we will still silently ignore stdin for confused usage such\n>>> as `git log v2.15.0.. | git shortlog v2.16.0..`. But at least that does\n>>> not crash.)\n>>\n>> I don't follow here. Are you saying this command should notice that\n>> there is input in stdin? How would it notice?\n>\n> I have no idea how it would notice (portably!) and the gain seems\n> minimal. I added this to keep the reader from wondering \"but wait a\n> minute, doesn't that mean that we fail to catch this bad usage if we're\n> in a repo?\". So my answer would be \"yep, but it's not a huge problem\".\n> Of course, my attempt to pre-emptively answer a question only provoked\n> another one. :-) I could phrase this better.\n\nAh, I think I see what I was missing. Let me look again at the whole\nparagraph in context:\n\n[...]\n>>> Disallow left-over arguments when run from outside a repo. Another\n>>> approach would be to disallow them when reading from stdin. However, our\n>>> logic is currently the other way round: we check the number of revisions\n>>> in order to decide whether we should read from stdin. (So yes, after\n>>> this patch, we will still silently ignore stdin for confused usage such\n>>> as `git log v2.15.0.. | git shortlog v2.16.0..`. But at least that does\n>>> not crash.)\n\nHow about something like this?\n\n\tDisallow left-over arguments when run from outside a repo. This\n\tway, such invalid usage is diagnosed correctly:\n\n\t\t$ git shortlog v2.16.0..\n\t\terror: [...]\n\t\t[...]\n\n\twhile still permitting the piped form:\n\n\t\t$ git -C ~/src/git log --pretty=short | git shortlog\n\t\tA Large Angry SCM (15):\n\t\t[...]\n\n\tThis cannot catch an incorrect usage that combines the piped and\n\t forms:\n\n\t\t$ git log --pretty=short | git shortlog v2.16.0..\n\t\tAlban Gruin (1)\n\t\t[...]\n\n\tbut (1) the operating system does not provide a straightforward\n\tway to detect that and (2) at least it doesn't crash.\n\nThat detail is mostly irrelevant to what the patch does, though. I\nwouldn't expect git to be able to diagnose that third variant anyway.\nIf we want to make the command less error-prone, I think a good path\nforward would be to introduce an explicit --stdin option. So I'd be\ntempted to go with the short and sweet:\n\n\tDisallow left-over arguments when run from outside a repo.\n\n[...]\n>>> + error(_(\"no revisions can be given when running \"\n>>> + \"from outside a repository\"));\n>>> + usage_with_options(shortlog_usage, options);\n>>> + }\n>>> +\n>>\n>> The error message is\n>>\n>> error: no revisions can be given when running from outside a repository\n>> usage: ...\n>>\n>> Do we need to dump usage here? I wonder if a simple die() call would\n>> be easier for the user to act on.\n>\n> I can see an argument for \"dumping the usage makes the error harder than\n> necessary to find\". I mainly went for consistency. This ties into your\n> other observations below: what little consistency do we have and in\n> which direction do we want to push it...\n[...]\n> I think it would be a larger project to make these consistent. The one\n> I'm adding here is at least consistent with the other one in this file.\n\nAh, thanks. That makes sense.\n\n>> Separate from that, I wonder if the error message can be made a little\n>> shorter and clearer. E.g.\n>>\n>> fatal: shortlog can only be used inside a git repository\n>\n> Some grepping suggests we do not usually name the command (\"shortlog\n> ...\"), perhaps to play well with aliasing, nor do we use \"such \"\n> very often, but it does happen. Quoting and allowing for options might\n> make this more correct, but perhaps less readable: \"'shortlog [...]\n> ' can only ...\". Slightly better than what I had, \"revisions can\n> only be given inside a git repository\" would avoid some negating.\n\nSounds good to me.\n\nThanks,\nJonathan\n"},{"id":"341594","messageId":"xmqqo9jr1we3.fsf@gitster-ct.c.googlers.com","threadId":"48019","inReplyTo":"78669e644b64fc10c34adb59717d2039f81cb092.1520680894.git.martin.agren@gmail.com","subject":"Re: [PATCH 3/3] shortlog: do not accept revisions when run outside repo","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-13T21:46:44Z","receivedAt":"2018-03-13T21:46:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin Ågren writes:\n\n> in order to decide whether we should read from stdin. (So yes, after\n> this patch, we will still silently ignore stdin for confused usage such\n> as `git log v2.15.0.. | git shortlog v2.16.0..`. But at least that does\n> not crash.)\n\n $ git log -p | git shortlog Documentation/\n\nis also a nonsense request. When outside a repository, i.e.\n\n $ git -C $path_to_repo | git shortlog Documentation/\n\nis not giving any revisions, so the error message should not say \"no\nrevisions can be given\"---a nitpicky bug reporter would say \"I gave\nno revisions, why are you complaining to me?\"\n\n"},{"id":"341614","messageId":"CAN0heSoDSf+3VmRowHmiTPDDzfdvUjQi+tjpPF6RW172ZE5BWw@mail.gmail.com","threadId":"48019","inReplyTo":"xmqqo9jr1we3.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 3/3] shortlog: do not accept revisions when run outside repo","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2018-03-14T05:06:08Z","receivedAt":"2018-03-14T05:06:14Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On 13 March 2018 at 22:46, Junio C Hamano wrote:\n> Martin Ågren writes:\n>\n>> in order to decide whether we should read from stdin. (So yes, after\n>> this patch, we will still silently ignore stdin for confused usage such\n>> as `git log v2.15.0.. | git shortlog v2.16.0..`. But at least that does\n>> not crash.)\n>\n> $ git log -p | git shortlog Documentation/\n>\n> is also a nonsense request. When outside a repository, i.e.\n>\n> $ git -C $path_to_repo | git shortlog Documentation/\n>\n> is not giving any revisions, so the error message should not say \"no\n> revisions can be given\"---a nitpicky bug reporter would say \"I gave\n> no revisions, why are you complaining to me?\"\n\nGood point. I will see if I can make this similar to other places. Maybe\nsomething like \"too many arguments given outside repository\". Thanks.\n\nMartin\n"},{"id":"341712","messageId":"cover.1521062188.git.martin.agren@gmail.com","threadId":"48019","inReplyTo":"cover.1520680894.git.martin.agren@gmail.com","subject":"[PATCH v2 0/3] shortlog: disallow left-over arguments outside repo","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2018-03-14T21:34:16Z","receivedAt":"2018-03-14T21:34:45Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"This is v2 of my attempt at stopping shortlog from BUG-ing when it is\nused incorrectly outside a repo. Thanks Jonathan and Junio for helpful\ncomments.\n\nPatches 1 and 2 are identical to pu. The error message in patch 3 is now\nmore general. The error condition on the other hand is a bit more\nspecific, \"argc > 1\", to better match the intention and commit message.\n\nMartin\n\nMartin Ågren (3):\n git-shortlog.txt: reorder usages\n shortlog: add usage-string for stdin-reading\n shortlog: disallow left-over arguments when run outside repo\n\n Documentation/git-shortlog.txt | 2 +-\n t/t4201-shortlog.sh | 5 +++++\n builtin/shortlog.c | 8 +++++++-\n 3 files changed, 13 insertions(+), 2 deletions(-)\n\n-- \n2.16.2.246.ga4ee44448f\n\n"},{"id":"341713","messageId":"a8210328f4db27cc119178b33e9512f8fcbba35c.1521062188.git.martin.agren@gmail.com","threadId":"48019","inReplyTo":"cover.1521062188.git.martin.agren@gmail.com","subject":"[PATCH v2 1/3] git-shortlog.txt: reorder usages","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2018-03-14T21:34:17Z","receivedAt":"2018-03-14T21:34:52Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"The first usage we give is the original one where, e.g., `git log` is\npiped through `git shortlog`. The description that follows reads the\nother way round, by first focusing on the general behavior, then ending\nwith the behavior when reading from stdin.\n\nIt is also a tiny bit odd that what is probably the most common usage\nand the one a reader is probably looking for is not at the top of the\nlist. Of course, it is only a two-item list, so it is not _that_ hard to\nfind... The next commit will add the original usage to the usage string\nin builtin/shortlog.c, and it feels more natural to do so below the\nmost common usage. To avoid being inconsistent, reorder these two\nusages here first.\n\nSigned-off-by: Martin Ågren \nSigned-off-by: Junio C Hamano \n---\n Documentation/git-shortlog.txt | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-shortlog.txt b/Documentation/git-shortlog.txt\nindex ee6c5476c1..5e35ea18ac 100644\n--- a/Documentation/git-shortlog.txt\n+++ b/Documentation/git-shortlog.txt\n@@ -8,8 +8,8 @@ git-shortlog - Summarize 'git log' output\n SYNOPSIS\n --------\n [verse]\n-git log --pretty=short | 'git shortlog' []\n 'git shortlog' [] [] [[\\--] ...]\n+git log --pretty=short | 'git shortlog' []\n \n DESCRIPTION\n -----------\n-- \n2.16.2.246.ga4ee44448f\n\n"},{"id":"341714","messageId":"cd56d4e5b2ca315d51699984456e50e4c6715f49.1521062188.git.martin.agren@gmail.com","threadId":"48019","inReplyTo":"cover.1521062188.git.martin.agren@gmail.com","subject":"[PATCH v2 2/3] shortlog: add usage-string for stdin-reading","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2018-03-14T21:34:18Z","receivedAt":"2018-03-14T21:34:58Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"This has been missing since we learned to print usage, way back in\n4e27fb06f (add commit count options to git-shortlog, 2006-10-06).\n\nWhile at it, drop the [] around \"...\". This matches `git log -h`\nand Documentation/git-{short}log.txt. It formally makes it look like we\ndo not allow `git shortlog --`, but we gain readability and consistency.\n\nSigned-off-by: Martin Ågren \nSigned-off-by: Junio C Hamano \n---\n builtin/shortlog.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex e29875b843..dc4af03fca 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -11,7 +11,8 @@\n #include \"parse-options.h\"\n \n static char const * const shortlog_usage[] = {\n-\tN_(\"git shortlog [] [] [[--] [...]]\"),\n+\tN_(\"git shortlog [] [] [[--] ...]\"),\n+\tN_(\"git log --pretty=short | git shortlog []\"),\n \tNULL\n };\n \n-- \n2.16.2.246.ga4ee44448f\n\n"},{"id":"341715","messageId":"fa32e16aa01a46718c2fba665e8ab2c0c13bbcdf.1521062188.git.martin.agren@gmail.com","threadId":"48019","inReplyTo":"cover.1521062188.git.martin.agren@gmail.com","subject":"[PATCH v2 3/3] shortlog: disallow left-over arguments outside repo","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2018-03-14T21:34:19Z","receivedAt":"2018-03-14T21:35:03Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"If we are outside a repo and have any arguments left after\noption-parsing, `setup_revisions()` will try to do its job and\nsomething like this will happen:\n\n$ git shortlog v2.16.0..\nBUG: environment.c:183: git environment hasn't been setup\nAborted (core dumped)\n\nThe usage is wrong, but we could obviously handle this better. Note that\ncommit abe549e179 (shortlog: do not require to run from inside a git\nrepository, 2008-03-14) explicitly enabled `git shortlog` to run from\noutside a repo, since we do not need a repo for parsing data from stdin.\n\nDisallow left-over arguments when run from outside a repo.\n\nSigned-off-by: Martin Ågren \n---\n t/t4201-shortlog.sh | 5 +++++\n builtin/shortlog.c | 5 +++++\n 2 files changed, 10 insertions(+)\n\ndiff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh\nindex da10478f59..ff6649ed9a 100755\n--- a/t/t4201-shortlog.sh\n+++ b/t/t4201-shortlog.sh\n@@ -127,6 +127,11 @@ test_expect_success !MINGW 'shortlog can read --format=raw output' '\n \ttest_cmp expect out\n '\n \n+test_expect_success 'shortlog from non-git directory refuses extra arguments' '\n+\ttest_must_fail env GIT_DIR=non-existing git shortlog foo 2>out &&\n+\ttest_i18ngrep \"too many arguments\" out\n+'\n+\n test_expect_success 'shortlog should add newline when input line matches wraplen' '\n \tcat >expect <<\\EOF &&\n A U Thor (2):\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex dc4af03fca..3a823b3128 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -293,6 +293,11 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)\n parse_done:\n \targc = parse_options_end(&ctx);\n \n+\tif (nongit && argc > 1) {\n+\t\terror(_(\"too many arguments given outside repository\"));\n+\t\tusage_with_options(shortlog_usage, options);\n+\t}\n+\n \tif (setup_revisions(argc, argv, &rev, NULL) != 1) {\n \t\terror(_(\"unrecognized argument: %s\"), argv[1]);\n \t\tusage_with_options(shortlog_usage, options);\n-- \n2.16.2.246.ga4ee44448f\n\n"},{"id":"343215","messageId":"20180328084839.GA31002@sigill.intra.peff.net","threadId":"48019","inReplyTo":"cover.1520680894.git.martin.agren@gmail.com","subject":"Re: [PATCH 0/3] shortlog: do not accept revisions when run outside repo","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-03-28T08:48:39Z","receivedAt":"2018-03-28T08:48:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Mar 10, 2018 at 12:52:09PM +0100, Martin Ågren wrote:\n\n> Someone trying this out might notice that `man git-shortlog` renders\n> \"\\--\" as \"\\--\", which is not wanted. (Also visible on git-scm.com...)\n> There is quite some history around such double-slashes and compatibility\n> with AsciiDoc-versions, so I'd rather not do a \"while at it\" there.\n> Regardless of the destiny of patch 1/3, I will follow up later to\n> address various forms of \"\\--\" throughout the tree.\n\nI didn't see any follow-up here, but in case you were delaying because\nthe history search seemed boring: dropping the backslash is the right\nthing to do. See the discussion in 1c262bb7b2 (doc: convert \\--option\nto --option, 2015-05-13).\n\n-Peff\n"},{"id":"343221","messageId":"CAN0heSru4-VoCTxZ6OHU_jdTs0__xvRmd45Dd0H3L8apMKkW6w@mail.gmail.com","threadId":"48019","inReplyTo":"20180328084839.GA31002@sigill.intra.peff.net","subject":"Re: [PATCH 0/3] shortlog: do not accept revisions when run outside repo","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2018-03-28T12:24:26Z","receivedAt":"2018-03-28T12:24:34Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On 28 March 2018 at 10:48, Jeff King wrote:\n> On Sat, Mar 10, 2018 at 12:52:09PM +0100, Martin Ågren wrote:\n>\n>> Someone trying this out might notice that `man git-shortlog` renders\n>> \"\\--\" as \"\\--\", which is not wanted. (Also visible on git-scm.com...)\n>> There is quite some history around such double-slashes and compatibility\n>> with AsciiDoc-versions, so I'd rather not do a \"while at it\" there.\n>> Regardless of the destiny of patch 1/3, I will follow up later to\n>> address various forms of \"\\--\" throughout the tree.\n>\n> I didn't see any follow-up here, but in case you were delaying because\n> the history search seemed boring: dropping the backslash is the right\n> thing to do. See the discussion in 1c262bb7b2 (doc: convert \\--option\n> to --option, 2015-05-13).\n\nThanks for pinging and thanks for the pointer. That commit is indeed\nhelpful and I am referencing it in a local topic, which I will submit\nonce ma/shortlog-revparse hits master.\n\nMartin\n"},{"id":"344927","messageId":"cover.1523991648.git.martin.agren@gmail.com","threadId":"48019","inReplyTo":"CAN0heSru4-VoCTxZ6OHU_jdTs0__xvRmd45Dd0H3L8apMKkW6w@mail.gmail.com","subject":"[PATCH 0/4] doc: cleaning up instances of \\--","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2018-04-17T19:15:25Z","receivedAt":"2018-04-17T19:15:50Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"This is a patch series to convert \\-- to -- in our documentation. The\nfirst patch is a reiteration of 1c262bb7b2 (doc: convert \\--option to\n--option, 2015-05-13) to fix some instances that have appeared since.\nThe other three patches deal with standalone \"\\--\" which we can't\nalways turn into \"--\" since it can be rendered as an em dash.\n\nMartin\n\nMartin Ågren (4):\n doc: convert \\--option to --option\n doc: convert [\\--] to [--]\n git-[short]log.txt: unify quoted standalone --\n git-submodule.txt: quote usage in monospace, drop backslash\n\n Documentation/git-format-patch.txt | 2 +-\n Documentation/git-log.txt | 8 ++++----\n Documentation/git-push.txt | 2 +-\n Documentation/git-shortlog.txt | 6 +++---\n Documentation/git-submodule.txt | 4 ++--\n Documentation/gitk.txt | 2 +-\n 6 files changed, 12 insertions(+), 12 deletions(-)\n\n-- \n2.17.0.252.gfe0a9eaf31\n\n"},{"id":"344928","messageId":"e17f2ec4e78266a789c9c48c6afefdf53e15b7d5.1523991648.git.martin.agren@gmail.com","threadId":"48019","inReplyTo":"cover.1523991648.git.martin.agren@gmail.com","subject":"[PATCH 1/4] doc: convert \\--option to --option","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2018-04-17T19:15:26Z","receivedAt":"2018-04-17T19:15:58Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"Rather than using a backslash in \\--foo, with or without ''-quoting,\nwrite `--foo` for better rendering. As explained in commit 1c262bb7b\n(doc: convert \\--option to --option, 2015-05-13), the backslash is not\nneeded for the versions of AsciiDoc that we support, but is rendered\nliterally by Asciidoctor.\n\nSigned-off-by: Martin Ågren \n---\n Documentation/git-format-patch.txt | 2 +-\n Documentation/git-push.txt | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex 6cbe462a77..b41e1329a7 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -47,7 +47,7 @@ There are two ways to specify which commits to operate on.\n \n The first rule takes precedence in the case of a single . To\n apply the second rule, i.e., format everything since the beginning of\n-history up until , use the '\\--root' option: `git format-patch\n+history up until , use the `--root` option: `git format-patch\n --root `. If you want to format only itself, you\n can do this with `git format-patch -1 `.\n \ndiff --git a/Documentation/git-push.txt b/Documentation/git-push.txt\nindex 5b08302fc2..34410f9fca 100644\n--- a/Documentation/git-push.txt\n+++ b/Documentation/git-push.txt\n@@ -300,7 +300,7 @@ origin +master` to force a push to the `master` branch). See the\n \tThese options are passed to linkgit:git-send-pack[1]. A thin transfer\n \tsignificantly reduces the amount of sent data when the sender and\n \treceiver share many of the same objects in common. The default is\n-\t\\--thin.\n+\t`--thin`.\n \n -q::\n --quiet::\n-- \n2.17.0.252.gfe0a9eaf31\n\n"},{"id":"344929","messageId":"7a9c99617b0cc03897bf1f02a21a14ee6509cf81.1523991648.git.martin.agren@gmail.com","threadId":"48019","inReplyTo":"cover.1523991648.git.martin.agren@gmail.com","subject":"[PATCH 2/4] doc: convert [\\--] to [--]","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2018-04-17T19:15:27Z","receivedAt":"2018-04-17T19:16:06Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"Commit 1c262bb7b (doc: convert \\--option to --option, 2015-05-13)\nexplains that we used to need to write \\--option to play well with older\nversions of AsciiDoc, but that we do not support such versions anymore\nanyway, and that Asciidoctor literally renders \\--.\n\nWith [\\--], which is used to denote the optional separator between\nrevisions and paths, Asciidoctor renders the backslash literally.\nChange all [\\--] to [--]. This changes nothing for AsciiDoc version\n8.6.9, but is an improvement for Asciidoctor version 1.5.4.\n\nWe use double-dashes in several list entries (\\--::). In my testing, it\nappears that we do need to use the backslash there, so leave those.\n\nSigned-off-by: Martin Ågren \n---\n Documentation/git-log.txt | 4 ++--\n Documentation/git-shortlog.txt | 4 ++--\n Documentation/gitk.txt | 2 +-\n 3 files changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-log.txt b/Documentation/git-log.txt\nindex 5437f8b0f0..be2f10b70b 100644\n--- a/Documentation/git-log.txt\n+++ b/Documentation/git-log.txt\n@@ -9,7 +9,7 @@ git-log - Show commit logs\n SYNOPSIS\n --------\n [verse]\n-'git log' [] [] [[\\--] ...]\n+'git log' [] [] [[--] ...]\n \n DESCRIPTION\n -----------\n@@ -90,7 +90,7 @@ include::line-range-format.txt[]\n \tways to spell , see the 'Specifying Ranges'\n \tsection of linkgit:gitrevisions[7].\n \n-[\\--] ...::\n+[--] ...::\n \tShow only commits that are enough to explain how the files\n \tthat match the specified paths came to be. See 'History\n \tSimplification' below for details and other simplification\ndiff --git a/Documentation/git-shortlog.txt b/Documentation/git-shortlog.txt\nindex 5e35ea18ac..00a22152a2 100644\n--- a/Documentation/git-shortlog.txt\n+++ b/Documentation/git-shortlog.txt\n@@ -8,7 +8,7 @@ git-shortlog - Summarize 'git log' output\n SYNOPSIS\n --------\n [verse]\n-'git shortlog' [] [] [[\\--] ...]\n+'git shortlog' [] [] [[--] ...]\n git log --pretty=short | 'git shortlog' []\n \n DESCRIPTION\n@@ -69,7 +69,7 @@ them.\n \tways to spell , see the \"Specifying Ranges\"\n \tsection of linkgit:gitrevisions[7].\n \n-[\\--] ...::\n+[--] ...::\n \tConsider only commits that are enough to explain how the files\n \tthat match the specified paths came to be.\n +\ndiff --git a/Documentation/gitk.txt b/Documentation/gitk.txt\nindex ca96c281d1..244cd01493 100644\n--- a/Documentation/gitk.txt\n+++ b/Documentation/gitk.txt\n@@ -8,7 +8,7 @@ gitk - The Git repository browser\n SYNOPSIS\n --------\n [verse]\n-'gitk' [] [] [\\--] [...]\n+'gitk' [] [] [--] [...]\n \n DESCRIPTION\n -----------\n-- \n2.17.0.252.gfe0a9eaf31\n\n"},{"id":"344930","messageId":"4b11e73a82e65d19054bcef66c0c470822b96067.1523991648.git.martin.agren@gmail.com","threadId":"48019","inReplyTo":"cover.1523991648.git.martin.agren@gmail.com","subject":"[PATCH 3/4] git-[short]log.txt: unify quoted standalone --","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2018-04-17T19:15:28Z","receivedAt":"2018-04-17T19:16:12Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"In git-log.txt, we have an instance of \\--, which is known to sometimes\nrender badly. This one is even worse than normal though, since ``\\-- ''\n(with or without that trailing space) appears to be entirely broken,\nboth in HTML and manpages, both with AsciiDoc (version 8.6.9) and\nAsciidoctor (version 1.5.4).\n\nFurther down in git-log.txt we have a ``--'', which renders good. In\ngit-shortlog.txt, we use \"\\-- \" (including the quotes and the space),\nwhich happens to look fairly good. I failed to find any other similar\ninstances. So all in all, we quote a double-dash in three different\nplaces and do it differently each time, with various degrees of success.\n\nSwitch all of these to `--`. This sets the double-dash in monospace and\nmatches what we usually do with example command line usages and options.\nNote that we drop the trailing space as well, since `-- ` does not\nrender well. These should still be clear enough since just a few lines\nabove each instance, the space is clearly visible in a longer context.\n\nSigned-off-by: Martin Ågren \n---\n Documentation/git-log.txt | 4 ++--\n Documentation/git-shortlog.txt | 2 +-\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-log.txt b/Documentation/git-log.txt\nindex be2f10b70b..90761f1694 100644\n--- a/Documentation/git-log.txt\n+++ b/Documentation/git-log.txt\n@@ -96,7 +96,7 @@ include::line-range-format.txt[]\n \tSimplification' below for details and other simplification\n \tmodes.\n +\n-Paths may need to be prefixed with ``\\-- '' to separate them from\n+Paths may need to be prefixed with `--` to separate them from\n options or the revision range, when confusion arises.\n \n include::rev-list-options.txt[]\n@@ -125,7 +125,7 @@ EXAMPLES\n `git log --since=\"2 weeks ago\" -- gitk`::\n \n \tShow the changes during the last two weeks to the file 'gitk'.\n-\tThe ``--'' is necessary to avoid confusion with the *branch* named\n+\tThe `--` is necessary to avoid confusion with the *branch* named\n \t'gitk'\n \n `git log --name-status release..test`::\ndiff --git a/Documentation/git-shortlog.txt b/Documentation/git-shortlog.txt\nindex 00a22152a2..bc80905a8a 100644\n--- a/Documentation/git-shortlog.txt\n+++ b/Documentation/git-shortlog.txt\n@@ -73,7 +73,7 @@ them.\n \tConsider only commits that are enough to explain how the files\n \tthat match the specified paths came to be.\n +\n-Paths may need to be prefixed with \"\\-- \" to separate them from\n+Paths may need to be prefixed with `--` to separate them from\n options or the revision range, when confusion arises.\n \n MAPPING AUTHORS\n-- \n2.17.0.252.gfe0a9eaf31\n\n"},{"id":"344931","messageId":"c22515c89a336040f5af81bf4114e437c0eca1e7.1523991648.git.martin.agren@gmail.com","threadId":"48019","inReplyTo":"cover.1523991648.git.martin.agren@gmail.com","subject":"[PATCH 4/4] git-submodule.txt: quote usage in monospace, drop backslash","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2018-04-17T19:15:29Z","receivedAt":"2018-04-17T19:16:16Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"We tend to quote command line examples using `` to set them in a\nmonospace font. The immediate motivation for this patch is to get rid of\nanother instance of \\--. As noted in the previous commits, \\-- has a\ntendency of rendering badly. Here, it renders ok (at least with\nAsciiDoc 8.6.9 and Asciidoctor 1.5.4), but by getting rid of this\ninstance, we reduce the chances of \\-- cropping up in places where it\nmatters more.\n\nSigned-off-by: Martin Ågren \n---\n Documentation/git-submodule.txt | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt\nindex 71c5618e82..630999f41a 100644\n--- a/Documentation/git-submodule.txt\n+++ b/Documentation/git-submodule.txt\n@@ -213,8 +213,8 @@ sync [--recursive] [--] [...]::\n \tsubmodule URLs change upstream and you need to update your local\n \trepositories accordingly.\n +\n-\"git submodule sync\" synchronizes all submodules while\n-\"git submodule sync \\-- A\" synchronizes submodule \"A\" only.\n+`git submodule sync` synchronizes all submodules while\n+`git submodule sync -- A` synchronizes submodule \"A\" only.\n +\n If `--recursive` is specified, this command will recurse into the\n registered submodules, and sync any nested submodules within.\n-- \n2.17.0.252.gfe0a9eaf31\n\n"},{"id":"344967","messageId":"xmqq1sfdjg4o.fsf@gitster-ct.c.googlers.com","threadId":"48019","inReplyTo":"cover.1523991648.git.martin.agren@gmail.com","subject":"Re: [PATCH 0/4] doc: cleaning up instances of \\--","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-04-18T04:24:55Z","receivedAt":"2018-04-18T04:25:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin Ågren writes:\n\n> This is a patch series to convert \\-- to -- in our documentation. The\n> first patch is a reiteration of 1c262bb7b2 (doc: convert \\--option to\n> --option, 2015-05-13) to fix some instances that have appeared since.\n> The other three patches deal with standalone \"\\--\" which we can't\n> always turn into \"--\" since it can be rendered as an em dash.\n\nAll looked sensible. As you mentioned in [2/4], \"\\--::\" that is\npart of an enumulation appear in documentation for about a dozen\ncommands after the series, but I do not think we can avoid it.\n\nOne thing that makes me wonder related to these patches is if a\nnewer AsciiDoc we assume lets us do without {litdd} macro. This\nseries and our earlier effort like 1c262bb7 (\"doc: convert \\--option\nto --option\", 2015-05-13) mentions that \"\\--word\" is less pleasant\non the eyes than \"--word\", but the ugliness \"two{litdd}words\" has\nover \"two--words\" is far worse than that, so...\n\nThanks, will queue.\n\n\n"},{"id":"345046","messageId":"20180419012517.GE14631@genre.crustytoothpaste.net","threadId":"48019","inReplyTo":"cover.1523991648.git.martin.agren@gmail.com","subject":"Re: [PATCH 0/4] doc: cleaning up instances of \\--","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-04-19T01:25:17Z","receivedAt":"2018-04-19T01:25:29Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Tue, Apr 17, 2018 at 09:15:25PM +0200, Martin Ågren wrote:\n> This is a patch series to convert \\-- to -- in our documentation. The\n> first patch is a reiteration of 1c262bb7b2 (doc: convert \\--option to\n> --option, 2015-05-13) to fix some instances that have appeared since.\n> The other three patches deal with standalone \"\\--\" which we can't\n> always turn into \"--\" since it can be rendered as an em dash.\n\nThis series looked sane to me as well. I appreciate the work to keep\nour documentation working in both AsciiDoc and Asciidoctor.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"347185","messageId":"20180510071103.GC31779@sigill.intra.peff.net","threadId":"48019","inReplyTo":"xmqq1sfdjg4o.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 0/4] doc: cleaning up instances of \\--","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-05-10T07:11:03Z","receivedAt":"2018-05-10T07:11:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Apr 18, 2018 at 01:24:55PM +0900, Junio C Hamano wrote:\n\n> Martin Ågren writes:\n> \n> > This is a patch series to convert \\-- to -- in our documentation. The\n> > first patch is a reiteration of 1c262bb7b2 (doc: convert \\--option to\n> > --option, 2015-05-13) to fix some instances that have appeared since.\n> > The other three patches deal with standalone \"\\--\" which we can't\n> > always turn into \"--\" since it can be rendered as an em dash.\n> \n> All looked sensible. As you mentioned in [2/4], \"\\--::\" that is\n> part of an enumulation appear in documentation for about a dozen\n> commands after the series, but I do not think we can avoid it.\n> \n> One thing that makes me wonder related to these patches is if a\n> newer AsciiDoc we assume lets us do without {litdd} macro. This\n> series and our earlier effort like 1c262bb7 (\"doc: convert \\--option\n> to --option\", 2015-05-13) mentions that \"\\--word\" is less pleasant\n> on the eyes than \"--word\", but the ugliness \"two{litdd}words\" has\n> over \"two--words\" is far worse than that, so...\n\nI think many cases that use {litdd} would be better off using literal\nbackticks anyway (e.g., git-add.txt mentions the filename\n`git-add--interactive.perl`).\n\nThere are certainly a few that can't, though (e.g., config.txt uses\nlinkgit:git-web{litdd}browse[1]). I agree that \"\\--\" is less ugly there\n(and seems to work on my modern asciidoc). There's some history on the\nlitdd versus \"\\--\" choice in 565e135a1e (Documentation: quote\ndouble-dash for AsciiDoc, 2011-06-29). That in turn references the\n2839478774 (Work around em-dash handling in newer AsciiDoc, 2010-08-23),\nbut I wouldn't be surprised if all of that is now obsolete with our\nAsciiDoc 8+ requirement.\n\n-Peff\n\nPS Late review, I know, but the patches look good to me. :)\n"}]}