{"thread":{"id":"51826","subject":"[PATCH] git-submodule.txt: fix AsciiDoc formatting error","startedAt":"2019-09-10T19:09:24Z","lastAt":"2019-09-16T19:25:52Z","messageCount":6,"participants":["Denton Liu","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"382144","messageId":"796a25ee1e9a9c0421d42ab6644e81d23a9bd99b.1568142486.git.liu.denton@gmail.com","threadId":"51826","inReplyTo":null,"subject":"[PATCH] git-submodule.txt: fix AsciiDoc formatting error","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-09-10T19:09:18Z","receivedAt":"2019-09-10T19:09:24Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"In b57e8119e6 (submodule: teach set-branch subcommand, 2019-02-08), the\n`set-branch` subcommand was added for submodules. When the documentation\nwas written, the syntax for a \"index term\" in AsciiDoc was\naccidentally used. This caused the documentation to be rendered as\n\n\tset-branch -d|--default)|(-b|--branch [--] \n\ninstead of\n\n\tset-branch (-d|--default)|(-b|--branch ) [--] \n\nRemove surrounding parentheses so that the \"index term\" syntax is not\ntriggered (and because it looks nicer without them anyway ;) ).\n\nSigned-off-by: Denton Liu \n---\n Documentation/git-submodule.txt | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt\nindex 0ed5c24dc1..e349442f4c 100644\n--- a/Documentation/git-submodule.txt\n+++ b/Documentation/git-submodule.txt\n@@ -173,7 +173,7 @@ submodule with the `--init` option.\n If `--recursive` is specified, this command will recurse into the\n registered submodules, and update any nested submodules within.\n --\n-set-branch ((-d|--default)|(-b|--branch )) [--] ::\n+set-branch (-d|--default)|(-b|--branch ) [--] ::\n \tSets the default remote tracking branch for the submodule. The\n \t`--branch` option allows the remote branch to be specified. The\n \t`--default` option removes the submodule..branch configuration\n-- \n2.23.0.163.g796a25ee1e\n\n"},{"id":"382316","messageId":"xmqqk1acdqf8.fsf@gitster-ct.c.googlers.com","threadId":"51826","inReplyTo":"796a25ee1e9a9c0421d42ab6644e81d23a9bd99b.1568142486.git.liu.denton@gmail.com","subject":"Re: [PATCH] git-submodule.txt: fix AsciiDoc formatting error","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-09-13T18:03:39Z","receivedAt":"2019-09-13T18:03:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Denton Liu writes:\n\n> Remove surrounding parentheses so that the \"index term\" syntax is not\n> triggered (and because it looks nicer without them anyway ;) ).\n\n\"Correct\" always trumps \"nicer\", though ;-)\n\nThe $USAGE string in the script describes the available options to\nthis subcommand like so:\n\n or: $dashless [--quiet] set-branch (--default|--branch ) [--] \n\nwhich is, \"you can give either --default or --branch , but\nnot both\". What the original, if there were no special meaning in\ndouble-paren, meant to say seems to (almost) match that.\n\n> -set-branch ((-d|--default)|(-b|--branch )) [--] ::\n\nI say \"almost\", as it gives a wrong impression that you can give\n\"-b\" without \"\" X-<.\n\nNow what does the updated text say to us?\n\n> +set-branch (-d|--default)|(-b|--branch ) [--] ::\n\nI think the attempt to cram the short-form is unnecessarily\ncluttering and making the result incorrect. How about doing\nsomething like this instead?\n\n Documentation/git-submodule.txt | 8 +++++---\n 1 file changed, 5 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt\nindex 0ed5c24dc1..816baa7dd0 100644\n--- a/Documentation/git-submodule.txt\n+++ b/Documentation/git-submodule.txt\n@@ -173,10 +173,12 @@ submodule with the `--init` option.\n If `--recursive` is specified, this command will recurse into the\n registered submodules, and update any nested submodules within.\n --\n-set-branch ((-d|--default)|(-b|--branch )) [--] ::\n+set-branch (--default|--branch ) [--] ::\n \tSets the default remote tracking branch for the submodule. The\n-\t`--branch` option allows the remote branch to be specified. The\n-\t`--default` option removes the submodule..branch configuration\n+\t`--branch` option (or its short-form, `-b `)\n+\tallows the remote branch to be specified. The\n+\t`--default` option (or its short-form, `-d`)\n+\tremoves the submodule..branch configuration\n \tkey, which causes the tracking branch to default to 'master'.\n \n summary [--cached|--files] [(-n|--summary-limit) ] [commit] [--] [...]::\n"},{"id":"382344","messageId":"20190913220902.GA93452@dentonliu-ltm.internal.salesforce.com","threadId":"51826","inReplyTo":"xmqqk1acdqf8.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] git-submodule.txt: fix AsciiDoc formatting error","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-09-13T22:09:02Z","receivedAt":"2019-09-13T22:09:06Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"On Fri, Sep 13, 2019 at 11:03:39AM -0700, Junio C Hamano wrote:\n> Denton Liu writes:\n> \n> > Remove surrounding parentheses so that the \"index term\" syntax is not\n> > triggered (and because it looks nicer without them anyway ;) ).\n> \n> \"Correct\" always trumps \"nicer\", though ;-)\n> \n> The $USAGE string in the script describes the available options to\n> this subcommand like so:\n> \n> or: $dashless [--quiet] set-branch (--default|--branch ) [--] \n> \n> which is, \"you can give either --default or --branch , but\n> not both\". What the original, if there were no special meaning in\n> double-paren, meant to say seems to (almost) match that.\n> \n> > -set-branch ((-d|--default)|(-b|--branch )) [--] ::\n> \n> I say \"almost\", as it gives a wrong impression that you can give\n> \"-b\" without \"\" X-<.\n> \n> Now what does the updated text say to us?\n> \n> > +set-branch (-d|--default)|(-b|--branch ) [--] ::\n> \n> I think the attempt to cram the short-form is unnecessarily\n> cluttering and making the result incorrect. How about doing\n> something like this instead?\n> \n> Documentation/git-submodule.txt | 8 +++++---\n> 1 file changed, 5 insertions(+), 3 deletions(-)\n> \n> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt\n> index 0ed5c24dc1..816baa7dd0 100644\n> --- a/Documentation/git-submodule.txt\n> +++ b/Documentation/git-submodule.txt\n> @@ -173,10 +173,12 @@ submodule with the `--init` option.\n> If `--recursive` is specified, this command will recurse into the\n> registered submodules, and update any nested submodules within.\n> --\n> -set-branch ((-d|--default)|(-b|--branch )) [--] ::\n> +set-branch (--default|--branch ) [--] ::\n> \tSets the default remote tracking branch for the submodule. The\n> -\t`--branch` option allows the remote branch to be specified. The\n> -\t`--default` option removes the submodule..branch configuration\n> +\t`--branch` option (or its short-form, `-b `)\n> +\tallows the remote branch to be specified. The\n> +\t`--default` option (or its short-form, `-d`)\n> +\tremoves the submodule..branch configuration\n> \tkey, which causes the tracking branch to default to 'master'.\n> \n> summary [--cached|--files] [(-n|--summary-limit) ] [commit] [--] [...]::\n\nHmm, I don't really like this since with every other subcommand, the\nshort-forms are in the command summary so it's obvious to the reader\nin a quick glance which options are available. With this change, a\nreader would have to not only read the summary line but also scan the\nblurb below.\n\nIn the context line above, we see `[(-n|--summary-limit) ]` as a\npossible way of notating a short and long option with argument. What do\nyou think about the following potential output?\n\n\tset-branch (-d|--default)|((-b|--branch) ) [--] ::\n\nOf course, we reintroduce the double paren problem but I can dig into\nasciidoc syntax and figure out how to escape it properly.\n"},{"id":"382349","messageId":"xmqq36gzdc30.fsf@gitster-ct.c.googlers.com","threadId":"51826","inReplyTo":"20190913220902.GA93452@dentonliu-ltm.internal.salesforce.com","subject":"Re: [PATCH] git-submodule.txt: fix AsciiDoc formatting error","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-09-13T23:13:23Z","receivedAt":"2019-09-13T23:13:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Denton Liu writes:\n\n>> > -set-branch ((-d|--default)|(-b|--branch )) [--] ::\n>> \n>> I say \"almost\", as it gives a wrong impression that you can give\n>> \"-b\" without \"\" X-<.\n>> \n>> Now what does the updated text say to us?\n>> \n>> > +set-branch (-d|--default)|(-b|--branch ) [--] ::\n>> \n>> I think the attempt to cram the short-form is unnecessarily\n>> cluttering and making the result incorrect.\n>> ...\n>> \n> Hmm, I don't really like this since with every other subcommand, the\n> short-forms are in the command summary so it's obvious to the reader\n> in a quick glance which options are available.\n\nI actually do not think it adds that much value. Once a user learns\nwhat --branch does and is told -b is its short form, it is much less\nimportant that -b and --branch are both available than --default and\n--branch are possibilities, and you cannot use both at the same time.\n\nIf anything, perhaps other subcommands' description may benefit if\nwe unclutter by reducing the prominence we give to their short form.\n\n> In the context line above, we see `[(-n|--summary-limit) ]` as a\n> possible way of notating a short and long option with argument. What do\n> you think about the following potential output?\n>\n> \tset-branch (-d|--default)|((-b|--branch) ) [--] ::\n>\n> Of course, we reintroduce the double paren problem but I can dig into\n> asciidoc syntax and figure out how to escape it properly.\n\nThat's much less important than the fact that you are losing \"-b and\n-d cannot be used together\", which is what the usage string gives us\n(and which is what I tried to express with my rewrite).\n"},{"id":"382436","messageId":"2ae16375ba185bcaceb2ba3e8843db495f0044a6.1568657575.git.liu.denton@gmail.com","threadId":"51826","inReplyTo":"796a25ee1e9a9c0421d42ab6644e81d23a9bd99b.1568142486.git.liu.denton@gmail.com","subject":"[PATCH v2] git-submodule.txt: fix AsciiDoc formatting error","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-09-16T18:19:17Z","receivedAt":"2019-09-16T18:19:24Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"In b57e8119e6 (submodule: teach set-branch subcommand, 2019-02-08), the\n`set-branch` subcommand was added for submodules. When the documentation\nwas written, the syntax for a \"index term\" in AsciiDoc was\naccidentally used. This caused the documentation to be rendered as\n\n\tset-branch -d|--default)|(-b|--branch [--] \n\ninstead of\n\n\tset-branch ((-d|--default)|(-b|--branch )) [--] \n\nIn addition to this, the original documentation was possibly confusing\nas it made it seem as if the `-b` option didn't accept a ``\nargument.\n\nBreak `--default` and `--branch` into their own separate invocations to\nmake it obvious that these options are mutually exclusive. Also, this\nremoves the surrounding parentheses so that the \"index term\" syntax is\nnot triggered.\n\nSigned-off-by: Denton Liu \n---\n\nThanks for reviewing, Junio.\n\nI thought about it over the weekend and I hope that this is a good\ncompromise.\n\nI looked through other options in the documentation and I couldn't find\nany other places where a short-form was documented in the body instead\nof in the summary line so I didn't want to introduce that with this\ncommit. I feel like it would be unfamiliar to someone who's used to\nreading other Git documentation.\n\nAlso, I opted to separate the the options onto their own lines because\nthis makes it obvious that these two arguments are incompatible.\nHopefully, this assuages your concerns.\n\nRange-diff against v1:\n1: 796a25ee1e ! 1: 2ae16375ba git-submodule.txt: fix AsciiDoc formatting error\n @@ Commit message\n \n instead of\n \n - set-branch (-d|--default)|(-b|--branch ) [--] \n + set-branch ((-d|--default)|(-b|--branch )) [--] \n \n - Remove surrounding parentheses so that the \"index term\" syntax is not\n - triggered (and because it looks nicer without them anyway ;) ).\n + In addition to this, the original documentation was possibly confusing\n + as it made it seem as if the `-b` option didn't accept a ``\n + argument.\n +\n + Break `--default` and `--branch` into their own separate invocations to\n + make it obvious that these options are mutually exclusive. Also, this\n + removes the surrounding parentheses so that the \"index term\" syntax is\n + not triggered.\n \n Signed-off-by: Denton Liu \n \n @@ Documentation/git-submodule.txt: submodule with the `--init` option.\n registered submodules, and update any nested submodules within.\n --\n -set-branch ((-d|--default)|(-b|--branch )) [--] ::\n -+set-branch (-d|--default)|(-b|--branch ) [--] ::\n ++set-branch (-b|--branch) [--] ::\n ++set-branch (-d|--default) [--] ::\n \tSets the default remote tracking branch for the submodule. The\n \t`--branch` option allows the remote branch to be specified. The\n \t`--default` option removes the submodule..branch configuration\n\n Documentation/git-submodule.txt | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt\nindex 0ed5c24dc1..1f46380af2 100644\n--- a/Documentation/git-submodule.txt\n+++ b/Documentation/git-submodule.txt\n@@ -173,7 +173,8 @@ submodule with the `--init` option.\n If `--recursive` is specified, this command will recurse into the\n registered submodules, and update any nested submodules within.\n --\n-set-branch ((-d|--default)|(-b|--branch )) [--] ::\n+set-branch (-b|--branch) [--] ::\n+set-branch (-d|--default) [--] ::\n \tSets the default remote tracking branch for the submodule. The\n \t`--branch` option allows the remote branch to be specified. The\n \t`--default` option removes the submodule..branch configuration\n-- \n2.23.0\n\n"},{"id":"382448","messageId":"xmqq4l1cavr9.fsf@gitster-ct.c.googlers.com","threadId":"51826","inReplyTo":"2ae16375ba185bcaceb2ba3e8843db495f0044a6.1568657575.git.liu.denton@gmail.com","subject":"Re: [PATCH v2] git-submodule.txt: fix AsciiDoc formatting error","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-09-16T19:25:46Z","receivedAt":"2019-09-16T19:25:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Denton Liu writes:\n\n> Break `--default` and `--branch` into their own separate invocations to\n> make it obvious that these options are mutually exclusive.\n\nThat sounds even clearer than what the original wanted to do. Very\ngood idea.\n\n> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt\n> index 0ed5c24dc1..1f46380af2 100644\n> --- a/Documentation/git-submodule.txt\n> +++ b/Documentation/git-submodule.txt\n> @@ -173,7 +173,8 @@ submodule with the `--init` option.\n> If `--recursive` is specified, this command will recurse into the\n> registered submodules, and update any nested submodules within.\n> --\n> -set-branch ((-d|--default)|(-b|--branch )) [--] ::\n> +set-branch (-b|--branch) [--] ::\n> +set-branch (-d|--default) [--] ::\n> \tSets the default remote tracking branch for the submodule. The\n> \t`--branch` option allows the remote branch to be specified. The\n> \t`--default` option removes the submodule..branch configuration\n"}]}