{"thread":{"id":"37167","subject":"[PATCH 2/4] completion: complete \"unstuck\" `git push --recurse-submodules`","startedAt":"2014-07-19T09:45:54Z","lastAt":"2014-07-23T21:31:22Z","messageCount":18,"participants":["John Keeping","Johannes Sixt","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"246360","messageId":"e9576136c09dbf65c5e614f9272d2c2afa96f5b6.1405763157.git.john@keeping.me.uk","threadId":"37167","inReplyTo":null,"subject":"[PATCH 1/4] completion: add missing terminator in case statement","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2014-07-19T09:45:54Z","receivedAt":"2014-07-19T09:45:54Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"Signed-off-by: John Keeping <john@keeping.me.uk>\n---\n contrib/completion/git-completion.bash | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 7a6e1d7..d0b2895 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1627,6 +1627,7 @@ _git_push ()\n \t--repo)\n \t\t__gitcomp_nl \"$(__git_remotes)\"\n \t\treturn\n+\t\t;;\n \tesac\n \tcase \"$cur\" in\n \t--repo=*)\n-- \n2.0.1.472.g6f92e5f.dirty\n"},{"id":"246359","messageId":"a3317a7692675483f60ee10ff98e1e95a3dc4b5a.1405763157.git.john@keeping.me.uk","threadId":"37167","inReplyTo":"e9576136c09dbf65c5e614f9272d2c2afa96f5b6.1405763157.git.john@keeping.me.uk","subject":"[PATCH 2/4] completion: complete \"unstuck\" `git push --recurse-submodules`","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2014-07-19T09:45:55Z","receivedAt":"2014-07-19T09:45:55Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"Since the argument to `--recurse-submodules` is mandatory, it does not\nneed to be stuck to the option with `=`.\n\nSigned-off-by: John Keeping <john@keeping.me.uk>\n---\n contrib/completion/git-completion.bash | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex d0b2895..06211a6 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1624,6 +1624,10 @@ __git_push_recurse_submodules=\"check on-demand\"\n _git_push ()\n {\n \tcase \"$prev\" in\n+\t--recurse_submodules)\n+\t\t__gitcomp \"$__git_push_recurse_submodules\"\n+\t\treturn\n+\t\t;;\n \t--repo)\n \t\t__gitcomp_nl \"$(__git_remotes)\"\n \t\treturn\n-- \n2.0.1.472.g6f92e5f.dirty\n"},{"id":"246362","messageId":"b7908d70349de0c542c44b4dc0dae5e10c60a2de.1405763157.git.john@keeping.me.uk","threadId":"37167","inReplyTo":"e9576136c09dbf65c5e614f9272d2c2afa96f5b6.1405763157.git.john@keeping.me.uk","subject":"[PATCH 3/4] completion: add some missing options to `git push`","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2014-07-19T09:45:56Z","receivedAt":"2014-07-19T09:45:56Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"Signed-off-by: John Keeping <john@keeping.me.uk>\n---\n contrib/completion/git-completion.bash | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 06211a6..9dcc222 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1645,6 +1645,7 @@ _git_push ()\n \t--*)\n \t\t__gitcomp \"\n \t\t\t--all --mirror --tags --dry-run --force --verbose\n+\t\t\t--quiet --prune --delete --follow-tags\n \t\t\t--receive-pack= --repo= --set-upstream\n \t\t\t--recurse-submodules=\n \t\t\"\n-- \n2.0.1.472.g6f92e5f.dirty\n"},{"id":"246361","messageId":"ead0ead00ac48f0a8fc316c0e5fa0b8fd04477c3.1405763157.git.john@keeping.me.uk","threadId":"37167","inReplyTo":"e9576136c09dbf65c5e614f9272d2c2afa96f5b6.1405763157.git.john@keeping.me.uk","subject":"[PATCH 4/4] completion: complete `git push --force-with-lease=`","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2014-07-19T09:45:57Z","receivedAt":"2014-07-19T09:45:57Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"Signed-off-by: John Keeping <john@keeping.me.uk>\n---\n contrib/completion/git-completion.bash | 22 +++++++++++++++++++++-\n 1 file changed, 21 insertions(+), 1 deletion(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 9dcc222..ad0e75c 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1621,6 +1621,22 @@ _git_pull ()\n \n __git_push_recurse_submodules=\"check on-demand\"\n \n+__git_complete_force_with_lease ()\n+{\n+\tlocal cur_=$1\n+\n+\tcase \"$cur_\" in\n+\t--*=)\n+\t\t;;\n+\t*:*)\n+\t\t__gitcomp_nl \"$(__git_refs)\" \"\" \"${cur_#*:}\"\n+\t\t;;\n+\t*)\n+\t\t__gitcomp_nl \"$(__git_refs)\" \"\" \"$cur_\"\n+\t\t;;\n+\tesac\n+}\n+\n _git_push ()\n {\n \tcase \"$prev\" in\n@@ -1634,6 +1650,10 @@ _git_push ()\n \t\t;;\n \tesac\n \tcase \"$cur\" in\n+\t--force-with-lease=*)\n+\t\t__git_complete_force_with_lease \"${cur##--force-with-lease=}\"\n+\t\treturn\n+\t\t;;\n \t--repo=*)\n \t\t__gitcomp_nl \"$(__git_remotes)\" \"\" \"${cur##--repo=}\"\n \t\treturn\n@@ -1647,7 +1667,7 @@ _git_push ()\n \t\t\t--all --mirror --tags --dry-run --force --verbose\n \t\t\t--quiet --prune --delete --follow-tags\n \t\t\t--receive-pack= --repo= --set-upstream\n-\t\t\t--recurse-submodules=\n+\t\t\t--force-with-lease --force-with-lease= --recurse-submodules=\n \t\t\"\n \t\treturn\n \t\t;;\n-- \n2.0.1.472.g6f92e5f.dirty\n"},{"id":"246364","messageId":"53CA5876.40007@kdbg.org","threadId":"37167","inReplyTo":"a3317a7692675483f60ee10ff98e1e95a3dc4b5a.1405763157.git.john@keeping.me.uk","subject":"Re: [PATCH 2/4] completion: complete \"unstuck\" `git push --recurse-submodules`","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2014-07-19T11:37:26Z","receivedAt":"2014-07-19T11:37:26Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 19.07.2014 11:45, schrieb John Keeping:\n> Since the argument to `--recurse-submodules` is mandatory, it does not\n> need to be stuck to the option with `=`.\n> \n> Signed-off-by: John Keeping <john@keeping.me.uk>\n> ---\n>  contrib/completion/git-completion.bash | 4 ++++\n>  1 file changed, 4 insertions(+)\n> \n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index d0b2895..06211a6 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -1624,6 +1624,10 @@ __git_push_recurse_submodules=\"check on-demand\"\n>  _git_push ()\n>  {\n>  \tcase \"$prev\" in\n> +\t--recurse_submodules)\n\nThis would need a dash, not an underscore.\n\n> +\t\t__gitcomp \"$__git_push_recurse_submodules\"\n> +\t\treturn\n> +\t\t;;\n>  \t--repo)\n>  \t\t__gitcomp_nl \"$(__git_remotes)\"\n>  \t\treturn\n> \n"},{"id":"246371","messageId":"20140719151014.GA26927@serenity.lan","threadId":"37167","inReplyTo":"53CA5876.40007@kdbg.org","subject":"Re: [PATCH 2/4] completion: complete \"unstuck\" `git push --recurse-submodules`","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2014-07-19T15:10:14Z","receivedAt":"2014-07-19T15:10:14Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Sat, Jul 19, 2014 at 01:37:26PM +0200, Johannes Sixt wrote:\n> Am 19.07.2014 11:45, schrieb John Keeping:\n> > Since the argument to `--recurse-submodules` is mandatory, it does not\n> > need to be stuck to the option with `=`.\n> > \n> > Signed-off-by: John Keeping <john@keeping.me.uk>\n> > ---\n> >  contrib/completion/git-completion.bash | 4 ++++\n> >  1 file changed, 4 insertions(+)\n> > \n> > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> > index d0b2895..06211a6 100644\n> > --- a/contrib/completion/git-completion.bash\n> > +++ b/contrib/completion/git-completion.bash\n> > @@ -1624,6 +1624,10 @@ __git_push_recurse_submodules=\"check on-demand\"\n> >  _git_push ()\n> >  {\n> >  \tcase \"$prev\" in\n> > +\t--recurse_submodules)\n> \n> This would need a dash, not an underscore.\n\nThanks.  I'll wait to see if there are any more comments before\nre-rolling.\n\n> > +\t\t__gitcomp \"$__git_push_recurse_submodules\"\n> > +\t\treturn\n> > +\t\t;;\n> >  \t--repo)\n> >  \t\t__gitcomp_nl \"$(__git_remotes)\"\n> >  \t\treturn\n> > \n> \n"},{"id":"246466","messageId":"xmqqmwc2k4h4.fsf@gitster.dls.corp.google.com","threadId":"37167","inReplyTo":"e9576136c09dbf65c5e614f9272d2c2afa96f5b6.1405763157.git.john@keeping.me.uk","subject":"Re: [PATCH 1/4] completion: add missing terminator in case statement","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-07-21T17:26:47Z","receivedAt":"2014-07-21T17:26:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"John Keeping <john@keeping.me.uk> writes:\n\n> Signed-off-by: John Keeping <john@keeping.me.uk>\n> ---\n\nAs these ;; are separators not terminators, this is not strictly\nnecessary.  Squashing it into a change that adds more case arms to\nthis case statement is of course not just good but necessary,\nthough.\n\n>  contrib/completion/git-completion.bash | 1 +\n>  1 file changed, 1 insertion(+)\n>\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index 7a6e1d7..d0b2895 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -1627,6 +1627,7 @@ _git_push ()\n>  \t--repo)\n>  \t\t__gitcomp_nl \"$(__git_remotes)\"\n>  \t\treturn\n> +\t\t;;\n>  \tesac\n>  \tcase \"$cur\" in\n>  \t--repo=*)\n"},{"id":"246478","messageId":"xmqqk376iidy.fsf@gitster.dls.corp.google.com","threadId":"37167","inReplyTo":"xmqqmwc2k4h4.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 1/4] completion: add missing terminator in case statement","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-07-21T20:09:13Z","receivedAt":"2014-07-21T20:09:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> John Keeping <john@keeping.me.uk> writes:\n>\n>> Signed-off-by: John Keeping <john@keeping.me.uk>\n>> ---\n>\n> As these ;; are separators not terminators, this is not strictly\n> necessary.  Squashing it into a change that adds more case arms to\n> this case statement is of course not just good but necessary,\n> though.\n\ns/necessary/may be &/; if you add new arms before this one, you\nwon't need it.  But if you add one after this, you would ;-).\n\n>>  contrib/completion/git-completion.bash | 1 +\n>>  1 file changed, 1 insertion(+)\n>>\n>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n>> index 7a6e1d7..d0b2895 100644\n>> --- a/contrib/completion/git-completion.bash\n>> +++ b/contrib/completion/git-completion.bash\n>> @@ -1627,6 +1627,7 @@ _git_push ()\n>>  \t--repo)\n>>  \t\t__gitcomp_nl \"$(__git_remotes)\"\n>>  \t\treturn\n>> +\t\t;;\n>>  \tesac\n>>  \tcase \"$cur\" in\n>>  \t--repo=*)\n"},{"id":"246516","messageId":"20140722182207.GC26927@serenity.lan","threadId":"37167","inReplyTo":"xmqqk376iidy.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 1/4] completion: add missing terminator in case statement","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2014-07-22T18:22:07Z","receivedAt":"2014-07-22T18:22:07Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Mon, Jul 21, 2014 at 01:09:13PM -0700, Junio C Hamano wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n> \n> > John Keeping <john@keeping.me.uk> writes:\n> >\n> >> Signed-off-by: John Keeping <john@keeping.me.uk>\n> >> ---\n> >\n> > As these ;; are separators not terminators, this is not strictly\n> > necessary.  Squashing it into a change that adds more case arms to\n> > this case statement is of course not just good but necessary,\n> > though.\n> \n> s/necessary/may be &/; if you add new arms before this one, you\n> won't need it.  But if you add one after this, you would ;-).\n\nHmm... POSIX describes them as terminators :-)\n\n\tThe compound-list for each list of patterns, with the possible\n\texception of the last, shall be terminated with \";;\".\n\nAlthough, bash.info is inaccurate here (clearly Bash does implement the\nPOSIX behaviour otherwise the existing code wouldn't work):\n\n\tEach clause must be terminated with `;;', `;&', or `;;&'.  The\n\tWORD undergoes tilde expansion, parameter expansion, command\n\tsubstitution, arithmetic expansion, and quote removal before\n\tmatching is attempted.  Each PATTERN undergoes tilde expansion,\n\tparameter expansion, command substitution, and arithmetic\n\texpansion.\n\n\tThere may be an arbitrary number of `case' clauses, each terminated\n\tby a `;;', `;&', or `;;&'.  The first pattern that matches\n\tdetermines the command-list that is executed.\n\n\nI'll drop this patch in the re-roll since it isn't necessary.\n\n> >>  contrib/completion/git-completion.bash | 1 +\n> >>  1 file changed, 1 insertion(+)\n> >>\n> >> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> >> index 7a6e1d7..d0b2895 100644\n> >> --- a/contrib/completion/git-completion.bash\n> >> +++ b/contrib/completion/git-completion.bash\n> >> @@ -1627,6 +1627,7 @@ _git_push ()\n> >>  \t--repo)\n> >>  \t\t__gitcomp_nl \"$(__git_remotes)\"\n> >>  \t\treturn\n> >> +\t\t;;\n> >>  \tesac\n> >>  \tcase \"$cur\" in\n> >>  \t--repo=*)\n"},{"id":"246512","messageId":"8303c95dd3e3ee8d77d4b994bb2d33b6a155a17f.1406053442.git.john@keeping.me.uk","threadId":"37167","inReplyTo":"e9576136c09dbf65c5e614f9272d2c2afa96f5b6.1405763157.git.john@keeping.me.uk","subject":"[PATCH v2 1/3] completion: complete \"unstuck\" `git push --recurse-submodules`","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2014-07-22T18:24:56Z","receivedAt":"2014-07-22T18:24:56Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"Since the argument to `--recurse-submodules` is mandatory, it does not\nneed to be stuck to the option with `=`.\n\nSigned-off-by: John Keeping <john@keeping.me.uk>\n---\nChange since v1:\n    - Fix typo --recurse{_ => -}submodules\n    - Dropped previous patch 1/4 adding \";;\" at the end of the \"--repo\" case\n\n contrib/completion/git-completion.bash | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 7a6e1d7..bed3665 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1624,6 +1624,10 @@ __git_push_recurse_submodules=\"check on-demand\"\n _git_push ()\n {\n \tcase \"$prev\" in\n+\t--recurse-submodules)\n+\t\t__gitcomp \"$__git_push_recurse_submodules\"\n+\t\treturn\n+\t\t;;\n \t--repo)\n \t\t__gitcomp_nl \"$(__git_remotes)\"\n \t\treturn\n-- \n2.0.1.472.g6f92e5f.dirty\n"},{"id":"246513","messageId":"648e0ae1853932105fb5fb7f718010d62a2d8bac.1406053442.git.john@keeping.me.uk","threadId":"37167","inReplyTo":"8303c95dd3e3ee8d77d4b994bb2d33b6a155a17f.1406053442.git.john@keeping.me.uk","subject":"[PATCH v2 2/3] completion: add some missing options to `git push`","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2014-07-22T18:24:57Z","receivedAt":"2014-07-22T18:24:57Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"Signed-off-by: John Keeping <john@keeping.me.uk>\n---\n contrib/completion/git-completion.bash | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex bed3665..33a4962 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1644,6 +1644,7 @@ _git_push ()\n \t--*)\n \t\t__gitcomp \"\n \t\t\t--all --mirror --tags --dry-run --force --verbose\n+\t\t\t--quiet --prune --delete --follow-tags\n \t\t\t--receive-pack= --repo= --set-upstream\n \t\t\t--recurse-submodules=\n \t\t\"\n-- \n2.0.1.472.g6f92e5f.dirty\n"},{"id":"246514","messageId":"90f116e5dc385ed5e85c499d855682720b0e91c7.1406053442.git.john@keeping.me.uk","threadId":"37167","inReplyTo":"8303c95dd3e3ee8d77d4b994bb2d33b6a155a17f.1406053442.git.john@keeping.me.uk","subject":"[PATCH v2 3/3] completion: complete `git push --force-with-lease=`","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2014-07-22T18:24:58Z","receivedAt":"2014-07-22T18:24:58Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"Signed-off-by: John Keeping <john@keeping.me.uk>\n---\n contrib/completion/git-completion.bash | 22 +++++++++++++++++++++-\n 1 file changed, 21 insertions(+), 1 deletion(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 33a4962..293868a 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1621,6 +1621,22 @@ _git_pull ()\n \n __git_push_recurse_submodules=\"check on-demand\"\n \n+__git_complete_force_with_lease ()\n+{\n+\tlocal cur_=$1\n+\n+\tcase \"$cur_\" in\n+\t--*=)\n+\t\t;;\n+\t*:*)\n+\t\t__gitcomp_nl \"$(__git_refs)\" \"\" \"${cur_#*:}\"\n+\t\t;;\n+\t*)\n+\t\t__gitcomp_nl \"$(__git_refs)\" \"\" \"$cur_\"\n+\t\t;;\n+\tesac\n+}\n+\n _git_push ()\n {\n \tcase \"$prev\" in\n@@ -1633,6 +1649,10 @@ _git_push ()\n \t\treturn\n \tesac\n \tcase \"$cur\" in\n+\t--force-with-lease=*)\n+\t\t__git_complete_force_with_lease \"${cur##--force-with-lease=}\"\n+\t\treturn\n+\t\t;;\n \t--repo=*)\n \t\t__gitcomp_nl \"$(__git_remotes)\" \"\" \"${cur##--repo=}\"\n \t\treturn\n@@ -1646,7 +1666,7 @@ _git_push ()\n \t\t\t--all --mirror --tags --dry-run --force --verbose\n \t\t\t--quiet --prune --delete --follow-tags\n \t\t\t--receive-pack= --repo= --set-upstream\n-\t\t\t--recurse-submodules=\n+\t\t\t--force-with-lease --force-with-lease= --recurse-submodules=\n \t\t\"\n \t\treturn\n \t\t;;\n-- \n2.0.1.472.g6f92e5f.dirty\n"},{"id":"246518","messageId":"xmqqvbqpf8qd.fsf@gitster.dls.corp.google.com","threadId":"37167","inReplyTo":"20140722182207.GC26927@serenity.lan","subject":"Re: [PATCH 1/4] completion: add missing terminator in case statement","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-07-22T20:18:17Z","receivedAt":"2014-07-22T20:18:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"John Keeping <john@keeping.me.uk> writes:\n\n> On Mon, Jul 21, 2014 at 01:09:13PM -0700, Junio C Hamano wrote:\n>> Junio C Hamano <gitster@pobox.com> writes:\n>> \n>> > John Keeping <john@keeping.me.uk> writes:\n>> >\n>> >> Signed-off-by: John Keeping <john@keeping.me.uk>\n>> >> ---\n>> >\n>> > As these ;; are separators not terminators, this is not strictly\n>> > necessary.  Squashing it into a change that adds more case arms to\n>> > this case statement is of course not just good but necessary,\n>> > though.\n>> \n>> s/necessary/may be &/; if you add new arms before this one, you\n>> won't need it.  But if you add one after this, you would ;-).\n>\n> Hmm... POSIX describes them as terminators :-)\n>\n> \tThe compound-list for each list of patterns, with the possible\n> \texception of the last, shall be terminated with \";;\".\n\nA terminator that is optional at the end is a separator ;-).\n\nHaving ';;' immediately before 'esac' is not wrong, but omitting it\nis exactly equally correct as having one, so it is not something we\nwould want a patch to churn.\n\n> I'll drop this patch in the re-roll since it isn't necessary.\n\nThis round looked good from a cursory read, except that the first\none still makes me wonder why you chose to put it there _before_\nwhere we handle --repo, where the corresponding case on \"$cur\"\nhandles --repo= first and then --recurse-submodules= next.\n\nWouldn't the end result easier to follow if you stuck to the same\norder?\n"},{"id":"246519","messageId":"xmqqr41df8hu.fsf@gitster.dls.corp.google.com","threadId":"37167","inReplyTo":"8303c95dd3e3ee8d77d4b994bb2d33b6a155a17f.1406053442.git.john@keeping.me.uk","subject":"Re: [PATCH v2 1/3] completion: complete \"unstuck\" `git push --recurse-submodules`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-07-22T20:23:25Z","receivedAt":"2014-07-22T20:23:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"John Keeping <john@keeping.me.uk> writes:\n\n> Since the argument to `--recurse-submodules` is mandatory, it does not\n> need to be stuck to the option with `=`.\n>\n> Signed-off-by: John Keeping <john@keeping.me.uk>\n> ---\n> Change since v1:\n>     - Fix typo --recurse{_ => -}submodules\n>     - Dropped previous patch 1/4 adding \";;\" at the end of the \"--repo\" case\n>\n>  contrib/completion/git-completion.bash | 4 ++++\n>  1 file changed, 4 insertions(+)\n>\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index 7a6e1d7..bed3665 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -1624,6 +1624,10 @@ __git_push_recurse_submodules=\"check on-demand\"\n>  _git_push ()\n>  {\n>  \tcase \"$prev\" in\n> +\t--recurse-submodules)\n> +\t\t__gitcomp \"$__git_push_recurse_submodules\"\n> +\t\treturn\n> +\t\t;;\n>  \t--repo)\n>  \t\t__gitcomp_nl \"$(__git_remotes)\"\n>  \t\treturn\n\nIf you mimick the order they are handled in the case on \"$cur\", it\nwould also let us sneak in the missing-optional \";;\" to case/esac to\nkeep symmetry between the two ;-)\n\nIn other words, like this, perhaps?\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 019026e..b27f385 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1617,6 +1617,11 @@ _git_push ()\n \t--repo)\n \t\t__gitcomp_nl \"$(__git_remotes)\"\n \t\treturn\n+\t\t;;\n+\t--recurse-submodules)\n+\t\t__gitcomp \"$__git_push_recurse_submodules\"\n+\t\treturn\n+\t\t;;\n \tesac\n \tcase \"$cur\" in\n \t--repo=*)\n-- \n2.0.2-892-g223db29\n"},{"id":"246522","messageId":"20140722205730.GD26927@serenity.lan","threadId":"37167","inReplyTo":"xmqqr41df8hu.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 1/3] completion: complete \"unstuck\" `git push --recurse-submodules`","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2014-07-22T20:57:30Z","receivedAt":"2014-07-22T20:57:30Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Tue, Jul 22, 2014 at 01:23:25PM -0700, Junio C Hamano wrote:\n> John Keeping <john@keeping.me.uk> writes:\n> \n> > Since the argument to `--recurse-submodules` is mandatory, it does not\n> > need to be stuck to the option with `=`.\n> >\n> > Signed-off-by: John Keeping <john@keeping.me.uk>\n> > ---\n> > Change since v1:\n> >     - Fix typo --recurse{_ => -}submodules\n> >     - Dropped previous patch 1/4 adding \";;\" at the end of the \"--repo\" case\n> >\n> >  contrib/completion/git-completion.bash | 4 ++++\n> >  1 file changed, 4 insertions(+)\n> >\n> > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> > index 7a6e1d7..bed3665 100644\n> > --- a/contrib/completion/git-completion.bash\n> > +++ b/contrib/completion/git-completion.bash\n> > @@ -1624,6 +1624,10 @@ __git_push_recurse_submodules=\"check on-demand\"\n> >  _git_push ()\n> >  {\n> >  \tcase \"$prev\" in\n> > +\t--recurse-submodules)\n> > +\t\t__gitcomp \"$__git_push_recurse_submodules\"\n> > +\t\treturn\n> > +\t\t;;\n> >  \t--repo)\n> >  \t\t__gitcomp_nl \"$(__git_remotes)\"\n> >  \t\treturn\n> \n> If you mimick the order they are handled in the case on \"$cur\", it\n> would also let us sneak in the missing-optional \";;\" to case/esac to\n> keep symmetry between the two ;-)\n> \n> In other words, like this, perhaps?\n\nMakes sense.  I don't think I noted the order in the \"$cur\" case, I just\nput the new one in here so that they were sorted lexicographically.\n\nDo you want me to re-roll with this change or can you replace the patch\nwhile applying?\n\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index 019026e..b27f385 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -1617,6 +1617,11 @@ _git_push ()\n>  \t--repo)\n>  \t\t__gitcomp_nl \"$(__git_remotes)\"\n>  \t\treturn\n> +\t\t;;\n> +\t--recurse-submodules)\n> +\t\t__gitcomp \"$__git_push_recurse_submodules\"\n> +\t\treturn\n> +\t\t;;\n>  \tesac\n>  \tcase \"$cur\" in\n>  \t--repo=*)\n> -- \n> 2.0.2-892-g223db29\n"},{"id":"246524","messageId":"xmqqha29f606.fsf@gitster.dls.corp.google.com","threadId":"37167","inReplyTo":"20140722205730.GD26927@serenity.lan","subject":"Re: [PATCH v2 1/3] completion: complete \"unstuck\" `git push --recurse-submodules`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-07-22T21:17:13Z","receivedAt":"2014-07-22T21:17:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"John Keeping <john@keeping.me.uk> writes:\n\n> Makes sense.  I don't think I noted the order in the \"$cur\" case, I just\n> put the new one in here so that they were sorted lexicographically.\n\nUnless there is particular reason, consistently using lexicographic\norder in all related places is one good way to organize things, but\ndoing so would involve flipping orders in the other case statement\nfor this particular patch.  In general, I prefer to see people add\nnew things at the end, when there is no particular reason to do\notherwise.\n\n> Do you want me to re-roll with this change or can you replace the patch\n> while applying?\n\nI think I had to flip the third one to adjust to the change I\nsuggested to this; the result will be on 'pu', so please double\ncheck when I push it out.\n\nThanks.\n"},{"id":"246596","messageId":"20140723193536.GE26927@serenity.lan","threadId":"37167","inReplyTo":"xmqqha29f606.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 1/3] completion: complete \"unstuck\" `git push --recurse-submodules`","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2014-07-23T19:35:36Z","receivedAt":"2014-07-23T19:35:36Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Tue, Jul 22, 2014 at 02:17:13PM -0700, Junio C Hamano wrote:\n> John Keeping <john@keeping.me.uk> writes:\n> \n> > Do you want me to re-roll with this change or can you replace the patch\n> > while applying?\n> \n> I think I had to flip the third one to adjust to the change I\n> suggested to this; the result will be on 'pu', so please double\n> check when I push it out.\n\nThe result on jk/more-push-completion looks good.  Thanks.\n"},{"id":"246610","messageId":"xmqqr41bdaol.fsf@gitster.dls.corp.google.com","threadId":"37167","inReplyTo":"20140723193536.GE26927@serenity.lan","subject":"Re: [PATCH v2 1/3] completion: complete \"unstuck\" `git push --recurse-submodules`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-07-23T21:31:22Z","receivedAt":"2014-07-23T21:31:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"John Keeping <john@keeping.me.uk> writes:\n\n> On Tue, Jul 22, 2014 at 02:17:13PM -0700, Junio C Hamano wrote:\n>> John Keeping <john@keeping.me.uk> writes:\n>> \n>> > Do you want me to re-roll with this change or can you replace the patch\n>> > while applying?\n>> \n>> I think I had to flip the third one to adjust to the change I\n>> suggested to this; the result will be on 'pu', so please double\n>> check when I push it out.\n>\n> The result on jk/more-push-completion looks good.  Thanks.\n\nThanks.\n"}]}