{"thread":{"id":"46695","subject":"[PATCH] doc/for-each-ref: explicitly specify option names","startedAt":"2017-09-01T14:50:45Z","lastAt":"2017-09-12T04:53:18Z","messageCount":9,"participants":["Kevin Daudt","Jeff King","Johannes Schindelin","Jonathan Nieder","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"327503","messageId":"20170901144931.26114-1-me@ikke.info","threadId":"46695","inReplyTo":null,"subject":"[PATCH] doc/for-each-ref: explicitly specify option names","fromName":"Kevin Daudt","fromEmail":"me@ikke.info","sentAt":"2017-09-01T14:49:31Z","receivedAt":"2017-09-01T14:50:45Z","isPatch":true,"sender":{"key":"me@ikke.info","avatar":"https://avatars.githubusercontent.com/u/135698?v=4"},"body":"For count, sort and format, only the argument names were listed under\nOPTIONS, not the option names.\n\nAdd the option names to make it clear the options exist\n\nSigned-off-by: Kevin Daudt \n---\n Documentation/git-for-each-ref.txt | 18 +++++++++---------\n 1 file changed, 9 insertions(+), 9 deletions(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex bb370c9c7..0c2032855 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -25,19 +25,25 @@ host language allowing their direct evaluation in that language.\n \n OPTIONS\n -------\n-::\n+...::\n+\tIf one or more patterns are given, only refs are shown that\n+\tmatch against at least one pattern, either using fnmatch(3) or\n+\tliterally, in the latter case matching completely or from the\n+\tbeginning up to a slash.\n+\n+--count ::\n \tBy default the command shows all refs that match\n \t``. This option makes it stop after showing\n \tthat many refs.\n \n-::\n+--sort ::\n \tA field name to sort on. Prefix `-` to sort in\n \tdescending order of the value. When unspecified,\n \t`refname` is used. You may use the --sort= option\n \tmultiple times, in which case the last key becomes the primary\n \tkey.\n \n-::\n+--format ::\n \tA string that interpolates `%(fieldname)` from a ref being shown\n \tand the object it points at. If `fieldname`\n \tis prefixed with an asterisk (`*`) and the ref points\n@@ -50,12 +56,6 @@ OPTIONS\n \t`xx`; for example `%00` interpolates to `\\0` (NUL),\n \t`%09` to `\\t` (TAB) and `%0a` to `\\n` (LF).\n \n-...::\n-\tIf one or more patterns are given, only refs are shown that\n-\tmatch against at least one pattern, either using fnmatch(3) or\n-\tliterally, in the latter case matching completely or from the\n-\tbeginning up to a slash.\n-\n --shell::\n --perl::\n --python::\n-- \n2.14.1.459.g238e487ea9\n\n"},{"id":"327513","messageId":"20170901220322.stzgclld56r5lbt3@sigill.intra.peff.net","threadId":"46695","inReplyTo":"20170901144931.26114-1-me@ikke.info","subject":"Re: [PATCH] doc/for-each-ref: explicitly specify option names","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-09-01T22:03:23Z","receivedAt":"2017-09-01T22:03:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Sep 01, 2017 at 04:49:31PM +0200, Kevin Daudt wrote:\n\n> For count, sort and format, only the argument names were listed under\n> OPTIONS, not the option names.\n> \n> Add the option names to make it clear the options exist\n\nYeah, this looks much better. I could see having a general \"\"\nsection if it was used as the argument to multiple options, but that's\nnot what's going on here. And even if it were, the right thing is to\nhave individual \"--foo=\" and \"--bar=\" items in the list,\nand let them refer to a more general section on formats.\n\n-Peff\n"},{"id":"327516","messageId":"alpine.DEB.2.21.1.1709020102490.4132@virtualbox","threadId":"46695","inReplyTo":"20170901144931.26114-1-me@ikke.info","subject":"Re: [PATCH] doc/for-each-ref: explicitly specify option names","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-09-01T23:03:10Z","receivedAt":"2017-09-01T23:03:23Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Kevin,\n\nOn Fri, 1 Sep 2017, Kevin Daudt wrote:\n\n> For count, sort and format, only the argument names were listed under\n> OPTIONS, not the option names.\n> \n> Add the option names to make it clear the options exist\n> \n> Signed-off-by: Kevin Daudt \n> ---\n\nThis patch makes sense to me.\n\nThanks,\nJohannes\n"},{"id":"327518","messageId":"20170901231933.GC143138@aiede.mtv.corp.google.com","threadId":"46695","inReplyTo":"20170901144931.26114-1-me@ikke.info","subject":"Re: [PATCH] doc/for-each-ref: explicitly specify option names","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2017-09-01T23:19:33Z","receivedAt":"2017-09-01T23:19:41Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Kevin Daudt wrote:\n\n> For count, sort and format, only the argument names were listed under\n> OPTIONS, not the option names.\n>\n> Add the option names to make it clear the options exist\n\nnit: missing full-stop (.) at end of sentence.\n\n> Signed-off-by: Kevin Daudt \n> ---\n> Documentation/git-for-each-ref.txt | 18 +++++++++---------\n> 1 file changed, 9 insertions(+), 9 deletions(-)\n\nMakes sense.\n\n> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\n> index bb370c9c7..0c2032855 100644\n> --- a/Documentation/git-for-each-ref.txt\n> +++ b/Documentation/git-for-each-ref.txt\n> @@ -25,19 +25,25 @@ host language allowing their direct evaluation in that language.\n> \n> OPTIONS\n> -------\n> +...::\n> +\tIf one or more patterns are given, only refs are shown that\n> +\tmatch against at least one pattern, either using fnmatch(3) or\n> +\tliterally, in the latter case matching completely or from the\n> +\tbeginning up to a slash.\n> +\n> -::\n> +--count ::\n\nnit: the usage string (and \"git help cli\") recommends --count=\nwith equal-sign, so it probably makes sense to match that (and likewise\nfor the other options).\n\nLooking closer reveals more problems with the manpage:\n\n* the synopsis mixes the style using = and the style using space,\n without a clear reason for doing so\n\n* the synopsis implies that I can run\n \"git for-each-ref --merged --contains HEAD\". But that produces\n\n\tfatal: malformed object name --contains\n\n An exact grammar would be harder to read than what is here, but\n perhaps it's worth a word or two on that subject in the description\n section.\n\n The description of --contains in git-branch.txt has the same problem.\n It's tempting to treat the argument to --contains as\n non-optional in the synopsis, since it's always harmless for a user\n to specify it. The OPTIONS section can still explain what happens\n when isn't specified.\n\n* by the way, the synopsis and options sections use where\n they mean .\n\nHow about something like this patch, for squashing in? It focuses on\njust the ordering and option name issues described in the commit\nmessage --- it doesn't make any of the more aggressive changes\ndescribed above.\n\nThanks,\nJonathan\n\ndiff --git i/Documentation/git-for-each-ref.txt w/Documentation/git-for-each-ref.txt\nindex 0c20328555..66b4e0a405 100644\n--- i/Documentation/git-for-each-ref.txt\n+++ w/Documentation/git-for-each-ref.txt\n@@ -10,8 +10,9 @@ SYNOPSIS\n [verse]\n 'git for-each-ref' [--count=] [--shell|--perl|--python|--tcl]\n \t\t [(--sort=)...] [--format=] [...]\n-\t\t [--points-at ] [(--merged | --no-merged) []]\n-\t\t [--contains []] [--no-contains []]\n+\t\t [--points-at=]\n+\t\t (--merged[=] | --no-merged[=])\n+\t\t [--contains[=]] [--no-contains[=]]\n \n DESCRIPTION\n -----------\n@@ -31,19 +32,19 @@ OPTIONS\n \tliterally, in the latter case matching completely or from the\n \tbeginning up to a slash.\n \n---count ::\n+--count=::\n \tBy default the command shows all refs that match\n \t``. This option makes it stop after showing\n \tthat many refs.\n \n---sort ::\n+--sort=::\n \tA field name to sort on. Prefix `-` to sort in\n \tdescending order of the value. When unspecified,\n \t`refname` is used. You may use the --sort= option\n \tmultiple times, in which case the last key becomes the primary\n \tkey.\n \n---format ::\n+--format=::\n \tA string that interpolates `%(fieldname)` from a ref being shown\n \tand the object it points at. If `fieldname`\n \tis prefixed with an asterisk (`*`) and the ref points\n@@ -65,24 +66,24 @@ OPTIONS\n \tthe specified host language. This is meant to produce\n \ta scriptlet that can directly be `eval`ed.\n \n---points-at ::\n+--points-at=::\n \tOnly list refs which points at the given object.\n \n---merged []::\n+--merged[=]::\n \tOnly list refs whose tips are reachable from the\n \tspecified commit (HEAD if not specified),\n \tincompatible with `--no-merged`.\n \n---no-merged []::\n+--no-merged[=]::\n \tOnly list refs whose tips are not reachable from the\n \tspecified commit (HEAD if not specified),\n \tincompatible with `--merged`.\n \n---contains []::\n+--contains[=]::\n \tOnly list refs which contain the specified commit (HEAD if not\n \tspecified).\n \n---no-contains []::\n+--no-contains[=]::\n \tOnly list refs which don't contain the specified commit (HEAD\n \tif not specified).\n \n"},{"id":"327523","messageId":"xmqqk21hj1qw.fsf@gitster.mtv.corp.google.com","threadId":"46695","inReplyTo":"20170901144931.26114-1-me@ikke.info","subject":"Re: [PATCH] doc/for-each-ref: explicitly specify option names","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-09-02T02:03:19Z","receivedAt":"2017-09-02T02:03:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kevin Daudt writes:\n\n> For count, sort and format, only the argument names were listed under\n> OPTIONS, not the option names.\n>\n> Add the option names to make it clear the options exist\n>\n> Signed-off-by: Kevin Daudt \n> ---\n> Documentation/git-for-each-ref.txt | 18 +++++++++---------\n> 1 file changed, 9 insertions(+), 9 deletions(-)\n\nSounds sensible. First I thought this was done deliberately because\nthese placeholder can apply to more than one option and we wanted to\nexplain each thing only once (e.g. \"\" appears in 5 places,\nand having to repeat something like \"you can spell by the\nunique prefix of the object name, or the name of a ref that points\nat it, or ...\" in each option would be awkward), but that is not the\ncase here.\n\nWhile we are at it, I just noticed that the SYNOPSIS section makes\nit look as if ... must come before points-at, merged, and\ntheir friends, but I do not think that should be the case. I also\nnotice that unlike --sort/--format/... the last four/five options\nare spelled with \"--option \" syntax; we should consistently\nuse \"--option=\" instead there.\n\nBut these two are separate issues that can be fixed in a patch\nseparate from this one I am responding to.\n\np.s. I am still mostly offline and won't be doing any reviews or\nqueuing that requires me to look at anything beyond the patch\ncontext. Please do not get disappointed if you do not see your\npatch in my tree until later next week.\n"},{"id":"327863","messageId":"20170911193338.25985-1-me@ikke.info","threadId":"46695","inReplyTo":"20170901144931.26114-1-me@ikke.info","subject":"[PATCH v2 1/2] doc/for-each-ref: consistently use '=' to between argument names and values","fromName":"Kevin Daudt","fromEmail":"me@ikke.info","sentAt":"2017-09-11T19:33:37Z","receivedAt":"2017-09-11T19:34:59Z","isPatch":true,"sender":{"key":"me@ikke.info","avatar":"https://avatars.githubusercontent.com/u/135698?v=4"},"body":"The synopsis and description inconsistently add a '=' between the\nargument name and it's value. Make this consistent.\n\nSigned-off-by: Kevin Daudt \n---\n Documentation/git-for-each-ref.txt | 15 ++++++++-------\n 1 file changed, 8 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex bb370c9c7..1015c88f6 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -10,8 +10,9 @@ SYNOPSIS\n [verse]\n 'git for-each-ref' [--count=] [--shell|--perl|--python|--tcl]\n \t\t [(--sort=)...] [--format=] [...]\n-\t\t [--points-at ] [(--merged | --no-merged) []]\n-\t\t [--contains []] [--no-contains []]\n+\t\t [--points-at=]\n+\t\t (--merged[=] | --no-merged[=])\n+\t\t [--contains[=]] [--no-contains[=]]\n \n DESCRIPTION\n -----------\n@@ -65,24 +66,24 @@ OPTIONS\n \tthe specified host language. This is meant to produce\n \ta scriptlet that can directly be `eval`ed.\n \n---points-at ::\n+--points-at=::\n \tOnly list refs which points at the given object.\n \n---merged []::\n+--merged[=]::\n \tOnly list refs whose tips are reachable from the\n \tspecified commit (HEAD if not specified),\n \tincompatible with `--no-merged`.\n \n---no-merged []::\n+--no-merged[=]::\n \tOnly list refs whose tips are not reachable from the\n \tspecified commit (HEAD if not specified),\n \tincompatible with `--merged`.\n \n---contains []::\n+--contains[=]::\n \tOnly list refs which contain the specified commit (HEAD if not\n \tspecified).\n \n---no-contains []::\n+--no-contains[=]::\n \tOnly list refs which don't contain the specified commit (HEAD\n \tif not specified).\n \n-- \n2.14.1.459.g238e487ea9\n\n"},{"id":"327864","messageId":"20170911193338.25985-2-me@ikke.info","threadId":"46695","inReplyTo":"20170911193338.25985-1-me@ikke.info","subject":"[PATCH v2 2/2] doc/for-each-ref: explicitly specify option names","fromName":"Kevin Daudt","fromEmail":"me@ikke.info","sentAt":"2017-09-11T19:33:38Z","receivedAt":"2017-09-11T19:35:08Z","isPatch":true,"sender":{"key":"me@ikke.info","avatar":"https://avatars.githubusercontent.com/u/135698?v=4"},"body":"For count, sort and format, only the argument names were listed under\nOPTIONS, not the option names.\n\nAdd the option names to make it clear the options exist\n\nSigned-off-by: Kevin Daudt \n---\n Documentation/git-for-each-ref.txt | 18 +++++++++---------\n 1 file changed, 9 insertions(+), 9 deletions(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex 1015c88f6..66b4e0a40 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -26,19 +26,25 @@ host language allowing their direct evaluation in that language.\n \n OPTIONS\n -------\n-::\n+...::\n+\tIf one or more patterns are given, only refs are shown that\n+\tmatch against at least one pattern, either using fnmatch(3) or\n+\tliterally, in the latter case matching completely or from the\n+\tbeginning up to a slash.\n+\n+--count=::\n \tBy default the command shows all refs that match\n \t``. This option makes it stop after showing\n \tthat many refs.\n \n-::\n+--sort=::\n \tA field name to sort on. Prefix `-` to sort in\n \tdescending order of the value. When unspecified,\n \t`refname` is used. You may use the --sort= option\n \tmultiple times, in which case the last key becomes the primary\n \tkey.\n \n-::\n+--format=::\n \tA string that interpolates `%(fieldname)` from a ref being shown\n \tand the object it points at. If `fieldname`\n \tis prefixed with an asterisk (`*`) and the ref points\n@@ -51,12 +57,6 @@ OPTIONS\n \t`xx`; for example `%00` interpolates to `\\0` (NUL),\n \t`%09` to `\\t` (TAB) and `%0a` to `\\n` (LF).\n \n-...::\n-\tIf one or more patterns are given, only refs are shown that\n-\tmatch against at least one pattern, either using fnmatch(3) or\n-\tliterally, in the latter case matching completely or from the\n-\tbeginning up to a slash.\n-\n --shell::\n --perl::\n --python::\n-- \n2.14.1.459.g238e487ea9\n\n"},{"id":"327871","messageId":"xmqq8thk3b2n.fsf@gitster.mtv.corp.google.com","threadId":"46695","inReplyTo":"20170911193338.25985-1-me@ikke.info","subject":"Re: [PATCH v2 1/2] doc/for-each-ref: consistently use '=' to between argument names and values","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-09-12T02:28:00Z","receivedAt":"2017-09-12T02:28:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kevin Daudt writes:\n\n> The synopsis and description inconsistently add a '=' between the\n> argument name and it's value. Make this consistent.\n>\n> Signed-off-by: Kevin Daudt \n> ---\n> Documentation/git-for-each-ref.txt | 15 ++++++++-------\n> 1 file changed, 8 insertions(+), 7 deletions(-)\n\nGood idea, and I think it is in line with an earlier suggestion by\nJonathan (cc'ed).\n\nThanks.\n\n> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\n> index bb370c9c7..1015c88f6 100644\n> --- a/Documentation/git-for-each-ref.txt\n> +++ b/Documentation/git-for-each-ref.txt\n> @@ -10,8 +10,9 @@ SYNOPSIS\n> [verse]\n> 'git for-each-ref' [--count=] [--shell|--perl|--python|--tcl]\n> \t\t [(--sort=)...] [--format=] [...]\n> -\t\t [--points-at ] [(--merged | --no-merged) []]\n> -\t\t [--contains []] [--no-contains []]\n> +\t\t [--points-at=]\n> +\t\t (--merged[=] | --no-merged[=])\n> +\t\t [--contains[=]] [--no-contains[=]]\n> \n> DESCRIPTION\n> -----------\n> @@ -65,24 +66,24 @@ OPTIONS\n> \tthe specified host language. This is meant to produce\n> \ta scriptlet that can directly be `eval`ed.\n> \n> ---points-at ::\n> +--points-at=::\n> \tOnly list refs which points at the given object.\n> \n> ---merged []::\n> +--merged[=]::\n> \tOnly list refs whose tips are reachable from the\n> \tspecified commit (HEAD if not specified),\n> \tincompatible with `--no-merged`.\n> \n> ---no-merged []::\n> +--no-merged[=]::\n> \tOnly list refs whose tips are not reachable from the\n> \tspecified commit (HEAD if not specified),\n> \tincompatible with `--merged`.\n> \n> ---contains []::\n> +--contains[=]::\n> \tOnly list refs which contain the specified commit (HEAD if not\n> \tspecified).\n> \n> ---no-contains []::\n> +--no-contains[=]::\n> \tOnly list refs which don't contain the specified commit (HEAD\n> \tif not specified).\n"},{"id":"327873","messageId":"20170912043815.GA23062@alpha.vpn.ikke.info","threadId":"46695","inReplyTo":"xmqq8thk3b2n.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 1/2] doc/for-each-ref: consistently use '=' to between argument names and values","fromName":"Kevin Daudt","fromEmail":"me@ikke.info","sentAt":"2017-09-12T04:38:15Z","receivedAt":"2017-09-12T04:53:18Z","isPatch":true,"sender":{"key":"me@ikke.info","avatar":"https://avatars.githubusercontent.com/u/135698?v=4"},"body":"On Tue, Sep 12, 2017 at 11:28:00AM +0900, Junio C Hamano wrote:\n> Kevin Daudt writes:\n> \n> > The synopsis and description inconsistently add a '=' between the\n> > argument name and it's value. Make this consistent.\n> >\n> > Signed-off-by: Kevin Daudt \n> > ---\n> > Documentation/git-for-each-ref.txt | 15 ++++++++-------\n> > 1 file changed, 8 insertions(+), 7 deletions(-)\n> \n> Good idea, and I think it is in line with an earlier suggestion by\n> Jonathan (cc'ed).\n> \n> Thanks.\n> \n\nYeah, this is his diff applied. Forgot to CC him.\n"}]}