* [PATCH 0/3] shortlog: do not accept revisions when run outside repo @ 2018-03-10 11:52 Martin Ågren 2018-03-10 11:52 ` [PATCH 1/3] git-shortlog.txt: reorder usages Martin Ågren ` (4 more replies) 0 siblings, 5 replies; 24+ messages in thread From: Martin Ågren @ 2018-03-10 11:52 UTC (permalink / raw) To: git Patch 3 stops git shortlog from BUG-ing when it's being used slightly wrong. Patches 1 and 2 are recursive preparation. Based on maint. Someone trying this out might notice that `man git-shortlog` renders "\--" as "\--", which is not wanted. (Also visible on git-scm.com...) There is quite some history around such double-slashes and compatibility with AsciiDoc-versions, so I'd rather not do a "while at it" there. Regardless of the destiny of patch 1/3, I will follow up later to address various forms of "\--" throughout the tree. Martin Ågren (3): git-shortlog.txt: reorder usages shortlog: add usage-string for stdin-reading shortlog: do not accept revisions when run outside repo Documentation/git-shortlog.txt | 2 +- t/t4201-shortlog.sh | 5 +++++ builtin/shortlog.c | 9 ++++++++- 3 files changed, 14 insertions(+), 2 deletions(-) -- 2.16.2.246.ga4ee44448f ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/3] git-shortlog.txt: reorder usages 2018-03-10 11:52 [PATCH 0/3] shortlog: do not accept revisions when run outside repo Martin Ågren @ 2018-03-10 11:52 ` Martin Ågren 2018-03-13 19:19 ` Junio C Hamano 2018-03-10 11:52 ` [PATCH 2/3] shortlog: add usage-string for stdin-reading Martin Ågren ` (3 subsequent siblings) 4 siblings, 1 reply; 24+ messages in thread From: Martin Ågren @ 2018-03-10 11:52 UTC (permalink / raw) To: git The first usage we give is the original one where, e.g., `git log` is piped through `git shortlog`. The description that follows reads the other way round, by first focusing on the general behavior, then ending with the behavior when reading from stdin. It is also a tiny bit odd that what is probably the most common usage and the one a reader is probably looking for is not at the top of the list. Of course, it is only a two-item list, so it is not _that_ hard to find... The next commit will add the original usage to the usage string in builtin/shortlog.c, and it feels more natural to do so below the most common usage. To avoid being inconsistent, reorder these two usages here first. Signed-off-by: Martin Ågren <martin.agren@gmail.com> --- Documentation/git-shortlog.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/git-shortlog.txt b/Documentation/git-shortlog.txt index ee6c5476c..5e35ea18a 100644 --- a/Documentation/git-shortlog.txt +++ b/Documentation/git-shortlog.txt @@ -8,8 +8,8 @@ git-shortlog - Summarize 'git log' output SYNOPSIS -------- [verse] -git log --pretty=short | 'git shortlog' [<options>] 'git shortlog' [<options>] [<revision range>] [[\--] <path>...] +git log --pretty=short | 'git shortlog' [<options>] DESCRIPTION ----------- -- 2.16.2.246.ga4ee44448f ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 1/3] git-shortlog.txt: reorder usages 2018-03-10 11:52 ` [PATCH 1/3] git-shortlog.txt: reorder usages Martin Ågren @ 2018-03-13 19:19 ` Junio C Hamano 0 siblings, 0 replies; 24+ messages in thread From: Junio C Hamano @ 2018-03-13 19:19 UTC (permalink / raw) To: Martin Ågren; +Cc: git Martin Ågren <martin.agren@gmail.com> writes: > The first usage we give is the original one where, e.g., `git log` is > piped through `git shortlog`. The description that follows reads the > other way round, by first focusing on the general behavior, then ending > with the behavior when reading from stdin. > ... The result looks a lot more useful than without the patch. I think the current ordering is merely a historical accident made in 2006. Will queue; thanks. > diff --git a/Documentation/git-shortlog.txt b/Documentation/git-shortlog.txt > index ee6c5476c..5e35ea18a 100644 > --- a/Documentation/git-shortlog.txt > +++ b/Documentation/git-shortlog.txt > @@ -8,8 +8,8 @@ git-shortlog - Summarize 'git log' output > SYNOPSIS > -------- > [verse] > -git log --pretty=short | 'git shortlog' [<options>] > 'git shortlog' [<options>] [<revision range>] [[\--] <path>...] > +git log --pretty=short | 'git shortlog' [<options>] > > DESCRIPTION > ----------- ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 2/3] shortlog: add usage-string for stdin-reading 2018-03-10 11:52 [PATCH 0/3] shortlog: do not accept revisions when run outside repo Martin Ågren 2018-03-10 11:52 ` [PATCH 1/3] git-shortlog.txt: reorder usages Martin Ågren @ 2018-03-10 11:52 ` Martin Ågren 2018-03-10 11:52 ` [PATCH 3/3] shortlog: do not accept revisions when run outside repo Martin Ågren ` (2 subsequent siblings) 4 siblings, 0 replies; 24+ messages in thread From: Martin Ågren @ 2018-03-10 11:52 UTC (permalink / raw) To: git This has been missing since we learned to print usage, way back in 4e27fb06f (add commit count options to git-shortlog, 2006-10-06). While at it, drop the [] around "<path>...". This matches `git log -h` and Documentation/git-{short}log.txt. It formally makes it look like we do not allow `git shortlog --`, but we gain readability and consistency. Signed-off-by: Martin Ågren <martin.agren@gmail.com> --- builtin/shortlog.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/builtin/shortlog.c b/builtin/shortlog.c index e29875b84..dc4af03fc 100644 --- a/builtin/shortlog.c +++ b/builtin/shortlog.c @@ -11,7 +11,8 @@ #include "parse-options.h" static char const * const shortlog_usage[] = { - N_("git shortlog [<options>] [<revision-range>] [[--] [<path>...]]"), + N_("git shortlog [<options>] [<revision-range>] [[--] <path>...]"), + N_("git log --pretty=short | git shortlog [<options>]"), NULL }; -- 2.16.2.246.ga4ee44448f ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 3/3] shortlog: do not accept revisions when run outside repo 2018-03-10 11:52 [PATCH 0/3] shortlog: do not accept revisions when run outside repo Martin Ågren 2018-03-10 11:52 ` [PATCH 1/3] git-shortlog.txt: reorder usages Martin Ågren 2018-03-10 11:52 ` [PATCH 2/3] shortlog: add usage-string for stdin-reading Martin Ågren @ 2018-03-10 11:52 ` Martin Ågren 2018-03-13 19:56 ` Jonathan Nieder 2018-03-13 21:46 ` Junio C Hamano 2018-03-14 21:34 ` [PATCH v2 0/3] shortlog: disallow left-over arguments " Martin Ågren 2018-03-28 8:48 ` [PATCH 0/3] shortlog: do not accept revisions when run " Jeff King 4 siblings, 2 replies; 24+ messages in thread From: Martin Ågren @ 2018-03-10 11:52 UTC (permalink / raw) To: git If we are outside a repo and have any arguments left after option-parsing, `setup_revisions()` will try to do its job and something like this will happen: $ git shortlog v2.16.0.. BUG: environment.c:183: git environment hasn't been setup Aborted (core dumped) The usage is wrong, but we could obviously handle this better. Note that commit abe549e179 (shortlog: do not require to run from inside a git repository, 2008-03-14) explicitly enabled `git shortlog` to run from outside a repo, since we do not need a repo for parsing data from stdin. Disallow left-over arguments when run from outside a repo. Another approach would be to disallow them when reading from stdin. However, our logic is currently the other way round: we check the number of revisions in order to decide whether we should read from stdin. (So yes, after this patch, we will still silently ignore stdin for confused usage such as `git log v2.15.0.. | git shortlog v2.16.0..`. But at least that does not crash.) Signed-off-by: Martin Ågren <martin.agren@gmail.com> --- t/t4201-shortlog.sh | 5 +++++ builtin/shortlog.c | 6 ++++++ 2 files changed, 11 insertions(+) diff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh index da10478f5..78c5645a9 100755 --- a/t/t4201-shortlog.sh +++ b/t/t4201-shortlog.sh @@ -127,6 +127,11 @@ test_expect_success !MINGW 'shortlog can read --format=raw output' ' test_cmp expect out ' +test_expect_success 'shortlog from non-git directory refuses revisions' ' + test_must_fail env GIT_DIR=non-existing git shortlog HEAD 2>out && + test_i18ngrep "no revisions can be given" out +' + test_expect_success 'shortlog should add newline when input line matches wraplen' ' cat >expect <<\EOF && A U Thor (2): diff --git a/builtin/shortlog.c b/builtin/shortlog.c index dc4af03fc..35e8c1ead 100644 --- a/builtin/shortlog.c +++ b/builtin/shortlog.c @@ -293,6 +293,12 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix) parse_done: argc = parse_options_end(&ctx); + if (nongit && argc != 1) { + error(_("no revisions can be given when running " + "from outside a repository")); + usage_with_options(shortlog_usage, options); + } + if (setup_revisions(argc, argv, &rev, NULL) != 1) { error(_("unrecognized argument: %s"), argv[1]); usage_with_options(shortlog_usage, options); -- 2.16.2.246.ga4ee44448f ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 3/3] shortlog: do not accept revisions when run outside repo 2018-03-10 11:52 ` [PATCH 3/3] shortlog: do not accept revisions when run outside repo Martin Ågren @ 2018-03-13 19:56 ` Jonathan Nieder 2018-03-13 20:47 ` Martin Ågren 2018-03-13 21:46 ` Junio C Hamano 1 sibling, 1 reply; 24+ messages in thread From: Jonathan Nieder @ 2018-03-13 19:56 UTC (permalink / raw) To: Martin Ågren; +Cc: git Hi, Martin Ågren wrote: > If we are outside a repo and have any arguments left after > option-parsing, `setup_revisions()` will try to do its job and > something like this will happen: > > $ git shortlog v2.16.0.. > BUG: environment.c:183: git environment hasn't been setup > Aborted (core dumped) Yikes. Thanks for fixing it. [...] > (So yes, after > this patch, we will still silently ignore stdin for confused usage such > as `git log v2.15.0.. | git shortlog v2.16.0..`. But at least that does > not crash.) I don't follow here. Are you saying this command should notice that there is input in stdin? How would it notice? [...] > --- a/t/t4201-shortlog.sh > +++ b/t/t4201-shortlog.sh > @@ -127,6 +127,11 @@ test_expect_success !MINGW 'shortlog can read --format=raw output' ' > test_cmp expect out > ' > > +test_expect_success 'shortlog from non-git directory refuses revisions' ' > + test_must_fail env GIT_DIR=non-existing git shortlog HEAD 2>out && > + test_i18ngrep "no revisions can be given" out > +' \o/ [...] > --- a/builtin/shortlog.c > +++ b/builtin/shortlog.c > @@ -293,6 +293,12 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix) > parse_done: > argc = parse_options_end(&ctx); > > + if (nongit && argc != 1) { Just curious: would argc ever be 0 here? 'argc <= 1' might be clearer. > + error(_("no revisions can be given when running " > + "from outside a repository")); > + usage_with_options(shortlog_usage, options); > + } > + The error message is error: no revisions can be given when running from outside a repository usage: ... Do we need to dump usage here? I wonder if a simple die() call would be easier for the user to act on. Not about this patch: I was a little surprised to see 'error:' instead of 'usage:' or 'fatal:'. It turns out git is pretty inconsistent about that: e.g. there is error(_("no remote specified")); usage_with_options(builtin_remote_setbranches_usage, options); Some other callers just use usage_with_options without describing the error. check-attr has a private error_with_usage() helper to implement the error() followed by usage_with_options() idiom. Most callers just use die(), like die(_("'%s' cannot be used with %s"), "--merge", "--patch"); Documentation/technical/api-error-handling.txt says - `usage` is for errors in command line usage. After printing its message, it exits with status 129. (See also `usage_with_options` in the link:api-parse-options.html[parse-options API].) which is not prescriptive enough to help. Separate from that, I wonder if the error message can be made a little shorter and clearer. E.g. fatal: shortlog <revs> can only be used inside a git repository Thanks and hope that helps, Jonathan ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/3] shortlog: do not accept revisions when run outside repo 2018-03-13 19:56 ` Jonathan Nieder @ 2018-03-13 20:47 ` Martin Ågren 2018-03-13 21:36 ` Jonathan Nieder 0 siblings, 1 reply; 24+ messages in thread From: Martin Ågren @ 2018-03-13 20:47 UTC (permalink / raw) To: Jonathan Nieder; +Cc: Git Mailing List On 13 March 2018 at 20:56, Jonathan Nieder <jrnieder@gmail.com> wrote: > Hi, > > Martin Ågren wrote: > >> (So yes, after >> this patch, we will still silently ignore stdin for confused usage such >> as `git log v2.15.0.. | git shortlog v2.16.0..`. But at least that does >> not crash.) > > I don't follow here. Are you saying this command should notice that > there is input in stdin? How would it notice? I have no idea how it would notice (portably!) and the gain seems minimal. I added this to keep the reader from wondering "but wait a minute, doesn't that mean that we fail to catch this bad usage if we're in a repo?". So my answer would be "yep, but it's not a huge problem". Of course, my attempt to pre-emptively answer a question only provoked another one. :-) I could phrase this better. >> --- a/builtin/shortlog.c >> +++ b/builtin/shortlog.c >> @@ -293,6 +293,12 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix) >> parse_done: >> argc = parse_options_end(&ctx); >> >> + if (nongit && argc != 1) { > > Just curious: would argc ever be 0 here? 'argc <= 1' might be clearer. Hmm, good point. It "shouldn't" be 0, but I guess it's better to be safe than sorry. (We seem to have both constructs, in various places.) >> + error(_("no revisions can be given when running " >> + "from outside a repository")); >> + usage_with_options(shortlog_usage, options); >> + } >> + > > The error message is > > error: no revisions can be given when running from outside a repository > usage: ... > > Do we need to dump usage here? I wonder if a simple die() call would > be easier for the user to act on. I can see an argument for "dumping the usage makes the error harder than necessary to find". I mainly went for consistency. This ties into your other observations below: what little consistency do we have and in which direction do we want to push it... > Not about this patch: I was a little surprised to see 'error:' instead > of 'usage:' or 'fatal:'. It turns out git is pretty inconsistent > about that: e.g. there is > > error(_("no remote specified")); > usage_with_options(builtin_remote_setbranches_usage, options); > > Some other callers just use usage_with_options without describing the > error. The other two approaches ("die" and "error and usage") can be argued for, but this one ("give usage") just seems wrong to me. I haven't looked for such a place in the code, and maybe they're "obvious", but it seems odd to just give the usage without any sort of hint about what was wrong. > check-attr has a private error_with_usage() helper to implement > the error() followed by usage_with_options() idiom. Most callers just > use die(), like > > die(_("'%s' cannot be used with %s"), "--merge", "--patch"); > > Documentation/technical/api-error-handling.txt says > > - `usage` is for errors in command line usage. After printing its > message, it exits with status 129. (See also `usage_with_options` > in the link:api-parse-options.html[parse-options API].) > > which is not prescriptive enough to help. I think it would be a larger project to make these consistent. The one I'm adding here is at least consistent with the other one in this file. > Separate from that, I wonder if the error message can be made a little > shorter and clearer. E.g. > > fatal: shortlog <revs> can only be used inside a git repository Some grepping suggests we do not usually name the command ("shortlog ..."), perhaps to play well with aliasing, nor do we use "such <syntax>" very often, but it does happen. Quoting and allowing for options might make this more correct, but perhaps less readable: "'shortlog [...] <revs>' can only ...". Slightly better than what I had, "revisions can only be given inside a git repository" would avoid some negating. > Thanks and hope that helps, It does indeed. I'll give this another 24h and see what I come up with. I believe it will end up in a change to "<= 1", an improved error message and a clearer last few words in the commit message. Martin ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/3] shortlog: do not accept revisions when run outside repo 2018-03-13 20:47 ` Martin Ågren @ 2018-03-13 21:36 ` Jonathan Nieder 0 siblings, 0 replies; 24+ messages in thread From: Jonathan Nieder @ 2018-03-13 21:36 UTC (permalink / raw) To: Martin Ågren; +Cc: Git Mailing List Martin Ågren wrote: > On 13 March 2018 at 20:56, Jonathan Nieder <jrnieder@gmail.com> wrote: >> Martin Ågren wrote: >>> (So yes, after >>> this patch, we will still silently ignore stdin for confused usage such >>> as `git log v2.15.0.. | git shortlog v2.16.0..`. But at least that does >>> not crash.) >> >> I don't follow here. Are you saying this command should notice that >> there is input in stdin? How would it notice? > > I have no idea how it would notice (portably!) and the gain seems > minimal. I added this to keep the reader from wondering "but wait a > minute, doesn't that mean that we fail to catch this bad usage if we're > in a repo?". So my answer would be "yep, but it's not a huge problem". > Of course, my attempt to pre-emptively answer a question only provoked > another one. :-) I could phrase this better. Ah, I think I see what I was missing. Let me look again at the whole paragraph in context: [...] >>> Disallow left-over arguments when run from outside a repo. Another >>> approach would be to disallow them when reading from stdin. However, our >>> logic is currently the other way round: we check the number of revisions >>> in order to decide whether we should read from stdin. (So yes, after >>> this patch, we will still silently ignore stdin for confused usage such >>> as `git log v2.15.0.. | git shortlog v2.16.0..`. But at least that does >>> not crash.) How about something like this? Disallow left-over arguments when run from outside a repo. This way, such invalid usage is diagnosed correctly: $ git shortlog v2.16.0.. error: [...] [...] while still permitting the piped form: $ git -C ~/src/git log --pretty=short | git shortlog A Large Angry SCM (15): [...] This cannot catch an incorrect usage that combines the piped and <revs> forms: $ git log --pretty=short | git shortlog v2.16.0.. Alban Gruin (1) [...] but (1) the operating system does not provide a straightforward way to detect that and (2) at least it doesn't crash. That detail is mostly irrelevant to what the patch does, though. I wouldn't expect git to be able to diagnose that third variant anyway. If we want to make the command less error-prone, I think a good path forward would be to introduce an explicit --stdin option. So I'd be tempted to go with the short and sweet: Disallow left-over arguments when run from outside a repo. [...] >>> + error(_("no revisions can be given when running " >>> + "from outside a repository")); >>> + usage_with_options(shortlog_usage, options); >>> + } >>> + >> >> The error message is >> >> error: no revisions can be given when running from outside a repository >> usage: ... >> >> Do we need to dump usage here? I wonder if a simple die() call would >> be easier for the user to act on. > > I can see an argument for "dumping the usage makes the error harder than > necessary to find". I mainly went for consistency. This ties into your > other observations below: what little consistency do we have and in > which direction do we want to push it... [...] > I think it would be a larger project to make these consistent. The one > I'm adding here is at least consistent with the other one in this file. Ah, thanks. That makes sense. >> Separate from that, I wonder if the error message can be made a little >> shorter and clearer. E.g. >> >> fatal: shortlog <revs> can only be used inside a git repository > > Some grepping suggests we do not usually name the command ("shortlog > ..."), perhaps to play well with aliasing, nor do we use "such <syntax>" > very often, but it does happen. Quoting and allowing for options might > make this more correct, but perhaps less readable: "'shortlog [...] > <revs>' can only ...". Slightly better than what I had, "revisions can > only be given inside a git repository" would avoid some negating. Sounds good to me. Thanks, Jonathan ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/3] shortlog: do not accept revisions when run outside repo 2018-03-10 11:52 ` [PATCH 3/3] shortlog: do not accept revisions when run outside repo Martin Ågren 2018-03-13 19:56 ` Jonathan Nieder @ 2018-03-13 21:46 ` Junio C Hamano 2018-03-14 5:06 ` Martin Ågren 1 sibling, 1 reply; 24+ messages in thread From: Junio C Hamano @ 2018-03-13 21:46 UTC (permalink / raw) To: Martin Ågren; +Cc: git Martin Ågren <martin.agren@gmail.com> writes: > in order to decide whether we should read from stdin. (So yes, after > this patch, we will still silently ignore stdin for confused usage such > as `git log v2.15.0.. | git shortlog v2.16.0..`. But at least that does > not crash.) $ git log -p | git shortlog Documentation/ is also a nonsense request. When outside a repository, i.e. $ git -C $path_to_repo | git shortlog Documentation/ is not giving any revisions, so the error message should not say "no revisions can be given"---a nitpicky bug reporter would say "I gave no revisions, why are you complaining to me?" ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/3] shortlog: do not accept revisions when run outside repo 2018-03-13 21:46 ` Junio C Hamano @ 2018-03-14 5:06 ` Martin Ågren 0 siblings, 0 replies; 24+ messages in thread From: Martin Ågren @ 2018-03-14 5:06 UTC (permalink / raw) To: Junio C Hamano; +Cc: Git Mailing List, Jonathan Nieder On 13 March 2018 at 22:46, Junio C Hamano <gitster@pobox.com> wrote: > Martin Ågren <martin.agren@gmail.com> writes: > >> in order to decide whether we should read from stdin. (So yes, after >> this patch, we will still silently ignore stdin for confused usage such >> as `git log v2.15.0.. | git shortlog v2.16.0..`. But at least that does >> not crash.) > > $ git log -p | git shortlog Documentation/ > > is also a nonsense request. When outside a repository, i.e. > > $ git -C $path_to_repo | git shortlog Documentation/ > > is not giving any revisions, so the error message should not say "no > revisions can be given"---a nitpicky bug reporter would say "I gave > no revisions, why are you complaining to me?" Good point. I will see if I can make this similar to other places. Maybe something like "too many arguments given outside repository". Thanks. Martin ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 0/3] shortlog: disallow left-over arguments outside repo 2018-03-10 11:52 [PATCH 0/3] shortlog: do not accept revisions when run outside repo Martin Ågren ` (2 preceding siblings ...) 2018-03-10 11:52 ` [PATCH 3/3] shortlog: do not accept revisions when run outside repo Martin Ågren @ 2018-03-14 21:34 ` Martin Ågren 2018-03-14 21:34 ` [PATCH v2 1/3] git-shortlog.txt: reorder usages Martin Ågren ` (2 more replies) 2018-03-28 8:48 ` [PATCH 0/3] shortlog: do not accept revisions when run " Jeff King 4 siblings, 3 replies; 24+ messages in thread From: Martin Ågren @ 2018-03-14 21:34 UTC (permalink / raw) To: git; +Cc: Jonathan Nieder, Junio C Hamano This is v2 of my attempt at stopping shortlog from BUG-ing when it is used incorrectly outside a repo. Thanks Jonathan and Junio for helpful comments. Patches 1 and 2 are identical to pu. The error message in patch 3 is now more general. The error condition on the other hand is a bit more specific, "argc > 1", to better match the intention and commit message. Martin Martin Ågren (3): git-shortlog.txt: reorder usages shortlog: add usage-string for stdin-reading shortlog: disallow left-over arguments when run outside repo Documentation/git-shortlog.txt | 2 +- t/t4201-shortlog.sh | 5 +++++ builtin/shortlog.c | 8 +++++++- 3 files changed, 13 insertions(+), 2 deletions(-) -- 2.16.2.246.ga4ee44448f ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 1/3] git-shortlog.txt: reorder usages 2018-03-14 21:34 ` [PATCH v2 0/3] shortlog: disallow left-over arguments " Martin Ågren @ 2018-03-14 21:34 ` Martin Ågren 2018-03-14 21:34 ` [PATCH v2 2/3] shortlog: add usage-string for stdin-reading Martin Ågren 2018-03-14 21:34 ` [PATCH v2 3/3] shortlog: disallow left-over arguments outside repo Martin Ågren 2 siblings, 0 replies; 24+ messages in thread From: Martin Ågren @ 2018-03-14 21:34 UTC (permalink / raw) To: git; +Cc: Jonathan Nieder, Junio C Hamano The first usage we give is the original one where, e.g., `git log` is piped through `git shortlog`. The description that follows reads the other way round, by first focusing on the general behavior, then ending with the behavior when reading from stdin. It is also a tiny bit odd that what is probably the most common usage and the one a reader is probably looking for is not at the top of the list. Of course, it is only a two-item list, so it is not _that_ hard to find... The next commit will add the original usage to the usage string in builtin/shortlog.c, and it feels more natural to do so below the most common usage. To avoid being inconsistent, reorder these two usages here first. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- Documentation/git-shortlog.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/git-shortlog.txt b/Documentation/git-shortlog.txt index ee6c5476c1..5e35ea18ac 100644 --- a/Documentation/git-shortlog.txt +++ b/Documentation/git-shortlog.txt @@ -8,8 +8,8 @@ git-shortlog - Summarize 'git log' output SYNOPSIS -------- [verse] -git log --pretty=short | 'git shortlog' [<options>] 'git shortlog' [<options>] [<revision range>] [[\--] <path>...] +git log --pretty=short | 'git shortlog' [<options>] DESCRIPTION ----------- -- 2.16.2.246.ga4ee44448f ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 2/3] shortlog: add usage-string for stdin-reading 2018-03-14 21:34 ` [PATCH v2 0/3] shortlog: disallow left-over arguments " Martin Ågren 2018-03-14 21:34 ` [PATCH v2 1/3] git-shortlog.txt: reorder usages Martin Ågren @ 2018-03-14 21:34 ` Martin Ågren 2018-03-14 21:34 ` [PATCH v2 3/3] shortlog: disallow left-over arguments outside repo Martin Ågren 2 siblings, 0 replies; 24+ messages in thread From: Martin Ågren @ 2018-03-14 21:34 UTC (permalink / raw) To: git; +Cc: Jonathan Nieder, Junio C Hamano This has been missing since we learned to print usage, way back in 4e27fb06f (add commit count options to git-shortlog, 2006-10-06). While at it, drop the [] around "<path>...". This matches `git log -h` and Documentation/git-{short}log.txt. It formally makes it look like we do not allow `git shortlog --`, but we gain readability and consistency. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- builtin/shortlog.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/builtin/shortlog.c b/builtin/shortlog.c index e29875b843..dc4af03fca 100644 --- a/builtin/shortlog.c +++ b/builtin/shortlog.c @@ -11,7 +11,8 @@ #include "parse-options.h" static char const * const shortlog_usage[] = { - N_("git shortlog [<options>] [<revision-range>] [[--] [<path>...]]"), + N_("git shortlog [<options>] [<revision-range>] [[--] <path>...]"), + N_("git log --pretty=short | git shortlog [<options>]"), NULL }; -- 2.16.2.246.ga4ee44448f ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 3/3] shortlog: disallow left-over arguments outside repo 2018-03-14 21:34 ` [PATCH v2 0/3] shortlog: disallow left-over arguments " Martin Ågren 2018-03-14 21:34 ` [PATCH v2 1/3] git-shortlog.txt: reorder usages Martin Ågren 2018-03-14 21:34 ` [PATCH v2 2/3] shortlog: add usage-string for stdin-reading Martin Ågren @ 2018-03-14 21:34 ` Martin Ågren 2 siblings, 0 replies; 24+ messages in thread From: Martin Ågren @ 2018-03-14 21:34 UTC (permalink / raw) To: git; +Cc: Jonathan Nieder, Junio C Hamano If we are outside a repo and have any arguments left after option-parsing, `setup_revisions()` will try to do its job and something like this will happen: $ git shortlog v2.16.0.. BUG: environment.c:183: git environment hasn't been setup Aborted (core dumped) The usage is wrong, but we could obviously handle this better. Note that commit abe549e179 (shortlog: do not require to run from inside a git repository, 2008-03-14) explicitly enabled `git shortlog` to run from outside a repo, since we do not need a repo for parsing data from stdin. Disallow left-over arguments when run from outside a repo. Signed-off-by: Martin Ågren <martin.agren@gmail.com> --- t/t4201-shortlog.sh | 5 +++++ builtin/shortlog.c | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh index da10478f59..ff6649ed9a 100755 --- a/t/t4201-shortlog.sh +++ b/t/t4201-shortlog.sh @@ -127,6 +127,11 @@ test_expect_success !MINGW 'shortlog can read --format=raw output' ' test_cmp expect out ' +test_expect_success 'shortlog from non-git directory refuses extra arguments' ' + test_must_fail env GIT_DIR=non-existing git shortlog foo 2>out && + test_i18ngrep "too many arguments" out +' + test_expect_success 'shortlog should add newline when input line matches wraplen' ' cat >expect <<\EOF && A U Thor (2): diff --git a/builtin/shortlog.c b/builtin/shortlog.c index dc4af03fca..3a823b3128 100644 --- a/builtin/shortlog.c +++ b/builtin/shortlog.c @@ -293,6 +293,11 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix) parse_done: argc = parse_options_end(&ctx); + if (nongit && argc > 1) { + error(_("too many arguments given outside repository")); + usage_with_options(shortlog_usage, options); + } + if (setup_revisions(argc, argv, &rev, NULL) != 1) { error(_("unrecognized argument: %s"), argv[1]); usage_with_options(shortlog_usage, options); -- 2.16.2.246.ga4ee44448f ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 0/3] shortlog: do not accept revisions when run outside repo 2018-03-10 11:52 [PATCH 0/3] shortlog: do not accept revisions when run outside repo Martin Ågren ` (3 preceding siblings ...) 2018-03-14 21:34 ` [PATCH v2 0/3] shortlog: disallow left-over arguments " Martin Ågren @ 2018-03-28 8:48 ` Jeff King 2018-03-28 12:24 ` Martin Ågren 4 siblings, 1 reply; 24+ messages in thread From: Jeff King @ 2018-03-28 8:48 UTC (permalink / raw) To: Martin Ågren; +Cc: git On Sat, Mar 10, 2018 at 12:52:09PM +0100, Martin Ågren wrote: > Someone trying this out might notice that `man git-shortlog` renders > "\--" as "\--", which is not wanted. (Also visible on git-scm.com...) > There is quite some history around such double-slashes and compatibility > with AsciiDoc-versions, so I'd rather not do a "while at it" there. > Regardless of the destiny of patch 1/3, I will follow up later to > address various forms of "\--" throughout the tree. I didn't see any follow-up here, but in case you were delaying because the history search seemed boring: dropping the backslash is the right thing to do. See the discussion in 1c262bb7b2 (doc: convert \--option to --option, 2015-05-13). -Peff ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 0/3] shortlog: do not accept revisions when run outside repo 2018-03-28 8:48 ` [PATCH 0/3] shortlog: do not accept revisions when run " Jeff King @ 2018-03-28 12:24 ` Martin Ågren 2018-04-17 19:15 ` [PATCH 0/4] doc: cleaning up instances of \-- Martin Ågren 0 siblings, 1 reply; 24+ messages in thread From: Martin Ågren @ 2018-03-28 12:24 UTC (permalink / raw) To: Jeff King; +Cc: Git Mailing List On 28 March 2018 at 10:48, Jeff King <peff@peff.net> wrote: > On Sat, Mar 10, 2018 at 12:52:09PM +0100, Martin Ågren wrote: > >> Someone trying this out might notice that `man git-shortlog` renders >> "\--" as "\--", which is not wanted. (Also visible on git-scm.com...) >> There is quite some history around such double-slashes and compatibility >> with AsciiDoc-versions, so I'd rather not do a "while at it" there. >> Regardless of the destiny of patch 1/3, I will follow up later to >> address various forms of "\--" throughout the tree. > > I didn't see any follow-up here, but in case you were delaying because > the history search seemed boring: dropping the backslash is the right > thing to do. See the discussion in 1c262bb7b2 (doc: convert \--option > to --option, 2015-05-13). Thanks for pinging and thanks for the pointer. That commit is indeed helpful and I am referencing it in a local topic, which I will submit once ma/shortlog-revparse hits master. Martin ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 0/4] doc: cleaning up instances of \-- 2018-03-28 12:24 ` Martin Ågren @ 2018-04-17 19:15 ` Martin Ågren 2018-04-17 19:15 ` [PATCH 1/4] doc: convert \--option to --option Martin Ågren ` (5 more replies) 0 siblings, 6 replies; 24+ messages in thread From: Martin Ågren @ 2018-04-17 19:15 UTC (permalink / raw) To: git; +Cc: Jeff King This is a patch series to convert \-- to -- in our documentation. The first patch is a reiteration of 1c262bb7b2 (doc: convert \--option to --option, 2015-05-13) to fix some instances that have appeared since. The other three patches deal with standalone "\--" which we can't always turn into "--" since it can be rendered as an em dash. Martin Martin Ågren (4): doc: convert \--option to --option doc: convert [\--] to [--] git-[short]log.txt: unify quoted standalone -- git-submodule.txt: quote usage in monospace, drop backslash Documentation/git-format-patch.txt | 2 +- Documentation/git-log.txt | 8 ++++---- Documentation/git-push.txt | 2 +- Documentation/git-shortlog.txt | 6 +++--- Documentation/git-submodule.txt | 4 ++-- Documentation/gitk.txt | 2 +- 6 files changed, 12 insertions(+), 12 deletions(-) -- 2.17.0.252.gfe0a9eaf31 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/4] doc: convert \--option to --option 2018-04-17 19:15 ` [PATCH 0/4] doc: cleaning up instances of \-- Martin Ågren @ 2018-04-17 19:15 ` Martin Ågren 2018-04-17 19:15 ` [PATCH 2/4] doc: convert [\--] to [--] Martin Ågren ` (4 subsequent siblings) 5 siblings, 0 replies; 24+ messages in thread From: Martin Ågren @ 2018-04-17 19:15 UTC (permalink / raw) To: git; +Cc: Jeff King Rather than using a backslash in \--foo, with or without ''-quoting, write `--foo` for better rendering. As explained in commit 1c262bb7b (doc: convert \--option to --option, 2015-05-13), the backslash is not needed for the versions of AsciiDoc that we support, but is rendered literally by Asciidoctor. Signed-off-by: Martin Ågren <martin.agren@gmail.com> --- Documentation/git-format-patch.txt | 2 +- Documentation/git-push.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt index 6cbe462a77..b41e1329a7 100644 --- a/Documentation/git-format-patch.txt +++ b/Documentation/git-format-patch.txt @@ -47,7 +47,7 @@ There are two ways to specify which commits to operate on. The first rule takes precedence in the case of a single <commit>. To apply the second rule, i.e., format everything since the beginning of -history up until <commit>, use the '\--root' option: `git format-patch +history up until <commit>, use the `--root` option: `git format-patch --root <commit>`. If you want to format only <commit> itself, you can do this with `git format-patch -1 <commit>`. diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt index 5b08302fc2..34410f9fca 100644 --- a/Documentation/git-push.txt +++ b/Documentation/git-push.txt @@ -300,7 +300,7 @@ origin +master` to force a push to the `master` branch). See the These options are passed to linkgit:git-send-pack[1]. A thin transfer significantly reduces the amount of sent data when the sender and receiver share many of the same objects in common. The default is - \--thin. + `--thin`. -q:: --quiet:: -- 2.17.0.252.gfe0a9eaf31 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 2/4] doc: convert [\--] to [--] 2018-04-17 19:15 ` [PATCH 0/4] doc: cleaning up instances of \-- Martin Ågren 2018-04-17 19:15 ` [PATCH 1/4] doc: convert \--option to --option Martin Ågren @ 2018-04-17 19:15 ` Martin Ågren 2018-04-17 19:15 ` [PATCH 3/4] git-[short]log.txt: unify quoted standalone -- Martin Ågren ` (3 subsequent siblings) 5 siblings, 0 replies; 24+ messages in thread From: Martin Ågren @ 2018-04-17 19:15 UTC (permalink / raw) To: git; +Cc: Jeff King Commit 1c262bb7b (doc: convert \--option to --option, 2015-05-13) explains that we used to need to write \--option to play well with older versions of AsciiDoc, but that we do not support such versions anymore anyway, and that Asciidoctor literally renders \--. With [\--], which is used to denote the optional separator between revisions and paths, Asciidoctor renders the backslash literally. Change all [\--] to [--]. This changes nothing for AsciiDoc version 8.6.9, but is an improvement for Asciidoctor version 1.5.4. We use double-dashes in several list entries (\--::). In my testing, it appears that we do need to use the backslash there, so leave those. Signed-off-by: Martin Ågren <martin.agren@gmail.com> --- Documentation/git-log.txt | 4 ++-- Documentation/git-shortlog.txt | 4 ++-- Documentation/gitk.txt | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt index 5437f8b0f0..be2f10b70b 100644 --- a/Documentation/git-log.txt +++ b/Documentation/git-log.txt @@ -9,7 +9,7 @@ git-log - Show commit logs SYNOPSIS -------- [verse] -'git log' [<options>] [<revision range>] [[\--] <path>...] +'git log' [<options>] [<revision range>] [[--] <path>...] DESCRIPTION ----------- @@ -90,7 +90,7 @@ include::line-range-format.txt[] ways to spell <revision range>, see the 'Specifying Ranges' section of linkgit:gitrevisions[7]. -[\--] <path>...:: +[--] <path>...:: Show only commits that are enough to explain how the files that match the specified paths came to be. See 'History Simplification' below for details and other simplification diff --git a/Documentation/git-shortlog.txt b/Documentation/git-shortlog.txt index 5e35ea18ac..00a22152a2 100644 --- a/Documentation/git-shortlog.txt +++ b/Documentation/git-shortlog.txt @@ -8,7 +8,7 @@ git-shortlog - Summarize 'git log' output SYNOPSIS -------- [verse] -'git shortlog' [<options>] [<revision range>] [[\--] <path>...] +'git shortlog' [<options>] [<revision range>] [[--] <path>...] git log --pretty=short | 'git shortlog' [<options>] DESCRIPTION @@ -69,7 +69,7 @@ them. ways to spell <revision range>, see the "Specifying Ranges" section of linkgit:gitrevisions[7]. -[\--] <path>...:: +[--] <path>...:: Consider only commits that are enough to explain how the files that match the specified paths came to be. + diff --git a/Documentation/gitk.txt b/Documentation/gitk.txt index ca96c281d1..244cd01493 100644 --- a/Documentation/gitk.txt +++ b/Documentation/gitk.txt @@ -8,7 +8,7 @@ gitk - The Git repository browser SYNOPSIS -------- [verse] -'gitk' [<options>] [<revision range>] [\--] [<path>...] +'gitk' [<options>] [<revision range>] [--] [<path>...] DESCRIPTION ----------- -- 2.17.0.252.gfe0a9eaf31 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 3/4] git-[short]log.txt: unify quoted standalone -- 2018-04-17 19:15 ` [PATCH 0/4] doc: cleaning up instances of \-- Martin Ågren 2018-04-17 19:15 ` [PATCH 1/4] doc: convert \--option to --option Martin Ågren 2018-04-17 19:15 ` [PATCH 2/4] doc: convert [\--] to [--] Martin Ågren @ 2018-04-17 19:15 ` Martin Ågren 2018-04-17 19:15 ` [PATCH 4/4] git-submodule.txt: quote usage in monospace, drop backslash Martin Ågren ` (2 subsequent siblings) 5 siblings, 0 replies; 24+ messages in thread From: Martin Ågren @ 2018-04-17 19:15 UTC (permalink / raw) To: git; +Cc: Jeff King In git-log.txt, we have an instance of \--, which is known to sometimes render badly. This one is even worse than normal though, since ``\-- '' (with or without that trailing space) appears to be entirely broken, both in HTML and manpages, both with AsciiDoc (version 8.6.9) and Asciidoctor (version 1.5.4). Further down in git-log.txt we have a ``--'', which renders good. In git-shortlog.txt, we use "\-- " (including the quotes and the space), which happens to look fairly good. I failed to find any other similar instances. So all in all, we quote a double-dash in three different places and do it differently each time, with various degrees of success. Switch all of these to `--`. This sets the double-dash in monospace and matches what we usually do with example command line usages and options. Note that we drop the trailing space as well, since `-- ` does not render well. These should still be clear enough since just a few lines above each instance, the space is clearly visible in a longer context. Signed-off-by: Martin Ågren <martin.agren@gmail.com> --- Documentation/git-log.txt | 4 ++-- Documentation/git-shortlog.txt | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt index be2f10b70b..90761f1694 100644 --- a/Documentation/git-log.txt +++ b/Documentation/git-log.txt @@ -96,7 +96,7 @@ include::line-range-format.txt[] Simplification' below for details and other simplification modes. + -Paths may need to be prefixed with ``\-- '' to separate them from +Paths may need to be prefixed with `--` to separate them from options or the revision range, when confusion arises. include::rev-list-options.txt[] @@ -125,7 +125,7 @@ EXAMPLES `git log --since="2 weeks ago" -- gitk`:: Show the changes during the last two weeks to the file 'gitk'. - The ``--'' is necessary to avoid confusion with the *branch* named + The `--` is necessary to avoid confusion with the *branch* named 'gitk' `git log --name-status release..test`:: diff --git a/Documentation/git-shortlog.txt b/Documentation/git-shortlog.txt index 00a22152a2..bc80905a8a 100644 --- a/Documentation/git-shortlog.txt +++ b/Documentation/git-shortlog.txt @@ -73,7 +73,7 @@ them. Consider only commits that are enough to explain how the files that match the specified paths came to be. + -Paths may need to be prefixed with "\-- " to separate them from +Paths may need to be prefixed with `--` to separate them from options or the revision range, when confusion arises. MAPPING AUTHORS -- 2.17.0.252.gfe0a9eaf31 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 4/4] git-submodule.txt: quote usage in monospace, drop backslash 2018-04-17 19:15 ` [PATCH 0/4] doc: cleaning up instances of \-- Martin Ågren ` (2 preceding siblings ...) 2018-04-17 19:15 ` [PATCH 3/4] git-[short]log.txt: unify quoted standalone -- Martin Ågren @ 2018-04-17 19:15 ` Martin Ågren 2018-04-18 4:24 ` [PATCH 0/4] doc: cleaning up instances of \-- Junio C Hamano 2018-04-19 1:25 ` brian m. carlson 5 siblings, 0 replies; 24+ messages in thread From: Martin Ågren @ 2018-04-17 19:15 UTC (permalink / raw) To: git; +Cc: Jeff King We tend to quote command line examples using `` to set them in a monospace font. The immediate motivation for this patch is to get rid of another instance of \--. As noted in the previous commits, \-- has a tendency of rendering badly. Here, it renders ok (at least with AsciiDoc 8.6.9 and Asciidoctor 1.5.4), but by getting rid of this instance, we reduce the chances of \-- cropping up in places where it matters more. Signed-off-by: Martin Ågren <martin.agren@gmail.com> --- Documentation/git-submodule.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt index 71c5618e82..630999f41a 100644 --- a/Documentation/git-submodule.txt +++ b/Documentation/git-submodule.txt @@ -213,8 +213,8 @@ sync [--recursive] [--] [<path>...]:: submodule URLs change upstream and you need to update your local repositories accordingly. + -"git submodule sync" synchronizes all submodules while -"git submodule sync \-- A" synchronizes submodule "A" only. +`git submodule sync` synchronizes all submodules while +`git submodule sync -- A` synchronizes submodule "A" only. + If `--recursive` is specified, this command will recurse into the registered submodules, and sync any nested submodules within. -- 2.17.0.252.gfe0a9eaf31 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 0/4] doc: cleaning up instances of \-- 2018-04-17 19:15 ` [PATCH 0/4] doc: cleaning up instances of \-- Martin Ågren ` (3 preceding siblings ...) 2018-04-17 19:15 ` [PATCH 4/4] git-submodule.txt: quote usage in monospace, drop backslash Martin Ågren @ 2018-04-18 4:24 ` Junio C Hamano 2018-05-10 7:11 ` Jeff King 2018-04-19 1:25 ` brian m. carlson 5 siblings, 1 reply; 24+ messages in thread From: Junio C Hamano @ 2018-04-18 4:24 UTC (permalink / raw) To: Martin Ågren; +Cc: git, Jeff King Martin Ågren <martin.agren@gmail.com> writes: > This is a patch series to convert \-- to -- in our documentation. The > first patch is a reiteration of 1c262bb7b2 (doc: convert \--option to > --option, 2015-05-13) to fix some instances that have appeared since. > The other three patches deal with standalone "\--" which we can't > always turn into "--" since it can be rendered as an em dash. All looked sensible. As you mentioned in [2/4], "\--::" that is part of an enumulation appear in documentation for about a dozen commands after the series, but I do not think we can avoid it. One thing that makes me wonder related to these patches is if a newer AsciiDoc we assume lets us do without {litdd} macro. This series and our earlier effort like 1c262bb7 ("doc: convert \--option to --option", 2015-05-13) mentions that "\--word" is less pleasant on the eyes than "--word", but the ugliness "two{litdd}words" has over "two--words" is far worse than that, so... Thanks, will queue. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 0/4] doc: cleaning up instances of \-- 2018-04-18 4:24 ` [PATCH 0/4] doc: cleaning up instances of \-- Junio C Hamano @ 2018-05-10 7:11 ` Jeff King 0 siblings, 0 replies; 24+ messages in thread From: Jeff King @ 2018-05-10 7:11 UTC (permalink / raw) To: Junio C Hamano; +Cc: Martin Ågren, git On Wed, Apr 18, 2018 at 01:24:55PM +0900, Junio C Hamano wrote: > Martin Ågren <martin.agren@gmail.com> writes: > > > This is a patch series to convert \-- to -- in our documentation. The > > first patch is a reiteration of 1c262bb7b2 (doc: convert \--option to > > --option, 2015-05-13) to fix some instances that have appeared since. > > The other three patches deal with standalone "\--" which we can't > > always turn into "--" since it can be rendered as an em dash. > > All looked sensible. As you mentioned in [2/4], "\--::" that is > part of an enumulation appear in documentation for about a dozen > commands after the series, but I do not think we can avoid it. > > One thing that makes me wonder related to these patches is if a > newer AsciiDoc we assume lets us do without {litdd} macro. This > series and our earlier effort like 1c262bb7 ("doc: convert \--option > to --option", 2015-05-13) mentions that "\--word" is less pleasant > on the eyes than "--word", but the ugliness "two{litdd}words" has > over "two--words" is far worse than that, so... I think many cases that use {litdd} would be better off using literal backticks anyway (e.g., git-add.txt mentions the filename `git-add--interactive.perl`). There are certainly a few that can't, though (e.g., config.txt uses linkgit:git-web{litdd}browse[1]). I agree that "\--" is less ugly there (and seems to work on my modern asciidoc). There's some history on the litdd versus "\--" choice in 565e135a1e (Documentation: quote double-dash for AsciiDoc, 2011-06-29). That in turn references the 2839478774 (Work around em-dash handling in newer AsciiDoc, 2010-08-23), but I wouldn't be surprised if all of that is now obsolete with our AsciiDoc 8+ requirement. -Peff PS Late review, I know, but the patches look good to me. :) ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 0/4] doc: cleaning up instances of \-- 2018-04-17 19:15 ` [PATCH 0/4] doc: cleaning up instances of \-- Martin Ågren ` (4 preceding siblings ...) 2018-04-18 4:24 ` [PATCH 0/4] doc: cleaning up instances of \-- Junio C Hamano @ 2018-04-19 1:25 ` brian m. carlson 5 siblings, 0 replies; 24+ messages in thread From: brian m. carlson @ 2018-04-19 1:25 UTC (permalink / raw) To: Martin Ågren; +Cc: git, Jeff King [-- Attachment #1: Type: text/plain, Size: 630 bytes --] On Tue, Apr 17, 2018 at 09:15:25PM +0200, Martin Ågren wrote: > This is a patch series to convert \-- to -- in our documentation. The > first patch is a reiteration of 1c262bb7b2 (doc: convert \--option to > --option, 2015-05-13) to fix some instances that have appeared since. > The other three patches deal with standalone "\--" which we can't > always turn into "--" since it can be rendered as an em dash. This series looked sane to me as well. I appreciate the work to keep our documentation working in both AsciiDoc and Asciidoctor. -- brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 867 bytes --] ^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2018-05-10 7:11 UTC | newest] Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-03-10 11:52 [PATCH 0/3] shortlog: do not accept revisions when run outside repo Martin Ågren 2018-03-10 11:52 ` [PATCH 1/3] git-shortlog.txt: reorder usages Martin Ågren 2018-03-13 19:19 ` Junio C Hamano 2018-03-10 11:52 ` [PATCH 2/3] shortlog: add usage-string for stdin-reading Martin Ågren 2018-03-10 11:52 ` [PATCH 3/3] shortlog: do not accept revisions when run outside repo Martin Ågren 2018-03-13 19:56 ` Jonathan Nieder 2018-03-13 20:47 ` Martin Ågren 2018-03-13 21:36 ` Jonathan Nieder 2018-03-13 21:46 ` Junio C Hamano 2018-03-14 5:06 ` Martin Ågren 2018-03-14 21:34 ` [PATCH v2 0/3] shortlog: disallow left-over arguments " Martin Ågren 2018-03-14 21:34 ` [PATCH v2 1/3] git-shortlog.txt: reorder usages Martin Ågren 2018-03-14 21:34 ` [PATCH v2 2/3] shortlog: add usage-string for stdin-reading Martin Ågren 2018-03-14 21:34 ` [PATCH v2 3/3] shortlog: disallow left-over arguments outside repo Martin Ågren 2018-03-28 8:48 ` [PATCH 0/3] shortlog: do not accept revisions when run " Jeff King 2018-03-28 12:24 ` Martin Ågren 2018-04-17 19:15 ` [PATCH 0/4] doc: cleaning up instances of \-- Martin Ågren 2018-04-17 19:15 ` [PATCH 1/4] doc: convert \--option to --option Martin Ågren 2018-04-17 19:15 ` [PATCH 2/4] doc: convert [\--] to [--] Martin Ågren 2018-04-17 19:15 ` [PATCH 3/4] git-[short]log.txt: unify quoted standalone -- Martin Ågren 2018-04-17 19:15 ` [PATCH 4/4] git-submodule.txt: quote usage in monospace, drop backslash Martin Ågren 2018-04-18 4:24 ` [PATCH 0/4] doc: cleaning up instances of \-- Junio C Hamano 2018-05-10 7:11 ` Jeff King 2018-04-19 1:25 ` brian m. carlson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).