# [PATCH 2/4] completion: complete "unstuck" `git push --recurse-submodules` 18 messages from 2014-07-19 to 2014-07-23. Participants: John Keeping, Johannes Sixt, Junio C Hamano. Thread: https://gitlist.dev/t/37167 ## John Keeping, 2014-07-19 09:45 Subject: [PATCH 1/4] completion: add missing terminator in case statement Message-ID: URL: https://gitlist.dev/e/e9576136c09dbf65c5e614f9272d2c2afa96f5b6.1405763157.git.john%40keeping.me.uk ``` Signed-off-by: John Keeping --- contrib/completion/git-completion.bash | 1 + 1 file changed, 1 insertion(+) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 7a6e1d7..d0b2895 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1627,6 +1627,7 @@ _git_push () --repo) __gitcomp_nl "$(__git_remotes)" return + ;; esac case "$cur" in --repo=*) -- 2.0.1.472.g6f92e5f.dirty ``` ## John Keeping, 2014-07-19 09:45 Subject: [PATCH 2/4] completion: complete "unstuck" `git push --recurse-submodules` Message-ID: URL: https://gitlist.dev/e/a3317a7692675483f60ee10ff98e1e95a3dc4b5a.1405763157.git.john%40keeping.me.uk In-Reply-To: ``` Since the argument to `--recurse-submodules` is mandatory, it does not need to be stuck to the option with `=`. Signed-off-by: John Keeping --- contrib/completion/git-completion.bash | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index d0b2895..06211a6 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1624,6 +1624,10 @@ __git_push_recurse_submodules="check on-demand" _git_push () { case "$prev" in + --recurse_submodules) + __gitcomp "$__git_push_recurse_submodules" + return + ;; --repo) __gitcomp_nl "$(__git_remotes)" return -- 2.0.1.472.g6f92e5f.dirty ``` ## John Keeping, 2014-07-19 09:45 Subject: [PATCH 3/4] completion: add some missing options to `git push` Message-ID: URL: https://gitlist.dev/e/b7908d70349de0c542c44b4dc0dae5e10c60a2de.1405763157.git.john%40keeping.me.uk In-Reply-To: ``` Signed-off-by: John Keeping --- contrib/completion/git-completion.bash | 1 + 1 file changed, 1 insertion(+) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 06211a6..9dcc222 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1645,6 +1645,7 @@ _git_push () --*) __gitcomp " --all --mirror --tags --dry-run --force --verbose + --quiet --prune --delete --follow-tags --receive-pack= --repo= --set-upstream --recurse-submodules= " -- 2.0.1.472.g6f92e5f.dirty ``` ## John Keeping, 2014-07-19 09:45 Subject: [PATCH 4/4] completion: complete `git push --force-with-lease=` Message-ID: URL: https://gitlist.dev/e/ead0ead00ac48f0a8fc316c0e5fa0b8fd04477c3.1405763157.git.john%40keeping.me.uk In-Reply-To: ``` Signed-off-by: John Keeping --- contrib/completion/git-completion.bash | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 9dcc222..ad0e75c 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1621,6 +1621,22 @@ _git_pull () __git_push_recurse_submodules="check on-demand" +__git_complete_force_with_lease () +{ + local cur_=$1 + + case "$cur_" in + --*=) + ;; + *:*) + __gitcomp_nl "$(__git_refs)" "" "${cur_#*:}" + ;; + *) + __gitcomp_nl "$(__git_refs)" "" "$cur_" + ;; + esac +} + _git_push () { case "$prev" in @@ -1634,6 +1650,10 @@ _git_push () ;; esac case "$cur" in + --force-with-lease=*) + __git_complete_force_with_lease "${cur##--force-with-lease=}" + return + ;; --repo=*) __gitcomp_nl "$(__git_remotes)" "" "${cur##--repo=}" return @@ -1647,7 +1667,7 @@ _git_push () --all --mirror --tags --dry-run --force --verbose --quiet --prune --delete --follow-tags --receive-pack= --repo= --set-upstream - --recurse-submodules= + --force-with-lease --force-with-lease= --recurse-submodules= " return ;; -- 2.0.1.472.g6f92e5f.dirty ``` ## Johannes Sixt, 2014-07-19 11:37 Subject: Re: [PATCH 2/4] completion: complete "unstuck" `git push --recurse-submodules` Message-ID: <53CA5876.40007@kdbg.org> URL: https://gitlist.dev/e/53CA5876.40007%40kdbg.org In-Reply-To: ``` Am 19.07.2014 11:45, schrieb John Keeping: > Since the argument to `--recurse-submodules` is mandatory, it does not > need to be stuck to the option with `=`. > > Signed-off-by: John Keeping > --- > contrib/completion/git-completion.bash | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash > index d0b2895..06211a6 100644 > --- a/contrib/completion/git-completion.bash > +++ b/contrib/completion/git-completion.bash > @@ -1624,6 +1624,10 @@ __git_push_recurse_submodules="check on-demand" > _git_push () > { > case "$prev" in > + --recurse_submodules) This would need a dash, not an underscore. > + __gitcomp "$__git_push_recurse_submodules" > + return > + ;; > --repo) > __gitcomp_nl "$(__git_remotes)" > return > ``` ## John Keeping, 2014-07-19 15:10 Subject: Re: [PATCH 2/4] completion: complete "unstuck" `git push --recurse-submodules` Message-ID: <20140719151014.GA26927@serenity.lan> URL: https://gitlist.dev/e/20140719151014.GA26927%40serenity.lan In-Reply-To: <53CA5876.40007@kdbg.org> ``` On Sat, Jul 19, 2014 at 01:37:26PM +0200, Johannes Sixt wrote: > Am 19.07.2014 11:45, schrieb John Keeping: > > Since the argument to `--recurse-submodules` is mandatory, it does not > > need to be stuck to the option with `=`. > > > > Signed-off-by: John Keeping > > --- > > contrib/completion/git-completion.bash | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash > > index d0b2895..06211a6 100644 > > --- a/contrib/completion/git-completion.bash > > +++ b/contrib/completion/git-completion.bash > > @@ -1624,6 +1624,10 @@ __git_push_recurse_submodules="check on-demand" > > _git_push () > > { > > case "$prev" in > > + --recurse_submodules) > > This would need a dash, not an underscore. Thanks. I'll wait to see if there are any more comments before re-rolling. > > + __gitcomp "$__git_push_recurse_submodules" > > + return > > + ;; > > --repo) > > __gitcomp_nl "$(__git_remotes)" > > return > > > ``` ## Junio C Hamano, 2014-07-21 17:26 Subject: Re: [PATCH 1/4] completion: add missing terminator in case statement Message-ID: URL: https://gitlist.dev/e/xmqqmwc2k4h4.fsf%40gitster.dls.corp.google.com In-Reply-To: ``` John Keeping writes: > Signed-off-by: John Keeping > --- As these ;; are separators not terminators, this is not strictly necessary. Squashing it into a change that adds more case arms to this case statement is of course not just good but necessary, though. > contrib/completion/git-completion.bash | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash > index 7a6e1d7..d0b2895 100644 > --- a/contrib/completion/git-completion.bash > +++ b/contrib/completion/git-completion.bash > @@ -1627,6 +1627,7 @@ _git_push () > --repo) > __gitcomp_nl "$(__git_remotes)" > return > + ;; > esac > case "$cur" in > --repo=*) ``` ## Junio C Hamano, 2014-07-21 20:09 Subject: Re: [PATCH 1/4] completion: add missing terminator in case statement Message-ID: URL: https://gitlist.dev/e/xmqqk376iidy.fsf%40gitster.dls.corp.google.com In-Reply-To: ``` Junio C Hamano writes: > John Keeping writes: > >> Signed-off-by: John Keeping >> --- > > As these ;; are separators not terminators, this is not strictly > necessary. Squashing it into a change that adds more case arms to > this case statement is of course not just good but necessary, > though. s/necessary/may be &/; if you add new arms before this one, you won't need it. But if you add one after this, you would ;-). >> contrib/completion/git-completion.bash | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash >> index 7a6e1d7..d0b2895 100644 >> --- a/contrib/completion/git-completion.bash >> +++ b/contrib/completion/git-completion.bash >> @@ -1627,6 +1627,7 @@ _git_push () >> --repo) >> __gitcomp_nl "$(__git_remotes)" >> return >> + ;; >> esac >> case "$cur" in >> --repo=*) ``` ## John Keeping, 2014-07-22 18:22 Subject: Re: [PATCH 1/4] completion: add missing terminator in case statement Message-ID: <20140722182207.GC26927@serenity.lan> URL: https://gitlist.dev/e/20140722182207.GC26927%40serenity.lan In-Reply-To: ``` On Mon, Jul 21, 2014 at 01:09:13PM -0700, Junio C Hamano wrote: > Junio C Hamano writes: > > > John Keeping writes: > > > >> Signed-off-by: John Keeping > >> --- > > > > As these ;; are separators not terminators, this is not strictly > > necessary. Squashing it into a change that adds more case arms to > > this case statement is of course not just good but necessary, > > though. > > s/necessary/may be &/; if you add new arms before this one, you > won't need it. But if you add one after this, you would ;-). Hmm... POSIX describes them as terminators :-) The compound-list for each list of patterns, with the possible exception of the last, shall be terminated with ";;". Although, bash.info is inaccurate here (clearly Bash does implement the POSIX behaviour otherwise the existing code wouldn't work): Each clause must be terminated with `;;', `;&', or `;;&'. The WORD undergoes tilde expansion, parameter expansion, command substitution, arithmetic expansion, and quote removal before matching is attempted. Each PATTERN undergoes tilde expansion, parameter expansion, command substitution, and arithmetic expansion. There may be an arbitrary number of `case' clauses, each terminated by a `;;', `;&', or `;;&'. The first pattern that matches determines the command-list that is executed. I'll drop this patch in the re-roll since it isn't necessary. > >> contrib/completion/git-completion.bash | 1 + > >> 1 file changed, 1 insertion(+) > >> > >> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash > >> index 7a6e1d7..d0b2895 100644 > >> --- a/contrib/completion/git-completion.bash > >> +++ b/contrib/completion/git-completion.bash > >> @@ -1627,6 +1627,7 @@ _git_push () > >> --repo) > >> __gitcomp_nl "$(__git_remotes)" > >> return > >> + ;; > >> esac > >> case "$cur" in > >> --repo=*) ``` ## John Keeping, 2014-07-22 18:24 Subject: [PATCH v2 1/3] completion: complete "unstuck" `git push --recurse-submodules` Message-ID: <8303c95dd3e3ee8d77d4b994bb2d33b6a155a17f.1406053442.git.john@keeping.me.uk> URL: https://gitlist.dev/e/8303c95dd3e3ee8d77d4b994bb2d33b6a155a17f.1406053442.git.john%40keeping.me.uk In-Reply-To: ``` Since the argument to `--recurse-submodules` is mandatory, it does not need to be stuck to the option with `=`. Signed-off-by: John Keeping --- Change since v1: - Fix typo --recurse{_ => -}submodules - Dropped previous patch 1/4 adding ";;" at the end of the "--repo" case contrib/completion/git-completion.bash | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 7a6e1d7..bed3665 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1624,6 +1624,10 @@ __git_push_recurse_submodules="check on-demand" _git_push () { case "$prev" in + --recurse-submodules) + __gitcomp "$__git_push_recurse_submodules" + return + ;; --repo) __gitcomp_nl "$(__git_remotes)" return -- 2.0.1.472.g6f92e5f.dirty ``` ## John Keeping, 2014-07-22 18:24 Subject: [PATCH v2 2/3] completion: add some missing options to `git push` Message-ID: <648e0ae1853932105fb5fb7f718010d62a2d8bac.1406053442.git.john@keeping.me.uk> URL: https://gitlist.dev/e/648e0ae1853932105fb5fb7f718010d62a2d8bac.1406053442.git.john%40keeping.me.uk In-Reply-To: <8303c95dd3e3ee8d77d4b994bb2d33b6a155a17f.1406053442.git.john@keeping.me.uk> ``` Signed-off-by: John Keeping --- contrib/completion/git-completion.bash | 1 + 1 file changed, 1 insertion(+) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index bed3665..33a4962 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1644,6 +1644,7 @@ _git_push () --*) __gitcomp " --all --mirror --tags --dry-run --force --verbose + --quiet --prune --delete --follow-tags --receive-pack= --repo= --set-upstream --recurse-submodules= " -- 2.0.1.472.g6f92e5f.dirty ``` ## John Keeping, 2014-07-22 18:24 Subject: [PATCH v2 3/3] completion: complete `git push --force-with-lease=` Message-ID: <90f116e5dc385ed5e85c499d855682720b0e91c7.1406053442.git.john@keeping.me.uk> URL: https://gitlist.dev/e/90f116e5dc385ed5e85c499d855682720b0e91c7.1406053442.git.john%40keeping.me.uk In-Reply-To: <8303c95dd3e3ee8d77d4b994bb2d33b6a155a17f.1406053442.git.john@keeping.me.uk> ``` Signed-off-by: John Keeping --- contrib/completion/git-completion.bash | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 33a4962..293868a 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1621,6 +1621,22 @@ _git_pull () __git_push_recurse_submodules="check on-demand" +__git_complete_force_with_lease () +{ + local cur_=$1 + + case "$cur_" in + --*=) + ;; + *:*) + __gitcomp_nl "$(__git_refs)" "" "${cur_#*:}" + ;; + *) + __gitcomp_nl "$(__git_refs)" "" "$cur_" + ;; + esac +} + _git_push () { case "$prev" in @@ -1633,6 +1649,10 @@ _git_push () return esac case "$cur" in + --force-with-lease=*) + __git_complete_force_with_lease "${cur##--force-with-lease=}" + return + ;; --repo=*) __gitcomp_nl "$(__git_remotes)" "" "${cur##--repo=}" return @@ -1646,7 +1666,7 @@ _git_push () --all --mirror --tags --dry-run --force --verbose --quiet --prune --delete --follow-tags --receive-pack= --repo= --set-upstream - --recurse-submodules= + --force-with-lease --force-with-lease= --recurse-submodules= " return ;; -- 2.0.1.472.g6f92e5f.dirty ``` ## Junio C Hamano, 2014-07-22 20:18 Subject: Re: [PATCH 1/4] completion: add missing terminator in case statement Message-ID: URL: https://gitlist.dev/e/xmqqvbqpf8qd.fsf%40gitster.dls.corp.google.com In-Reply-To: <20140722182207.GC26927@serenity.lan> ``` John Keeping writes: > On Mon, Jul 21, 2014 at 01:09:13PM -0700, Junio C Hamano wrote: >> Junio C Hamano writes: >> >> > John Keeping writes: >> > >> >> Signed-off-by: John Keeping >> >> --- >> > >> > As these ;; are separators not terminators, this is not strictly >> > necessary. Squashing it into a change that adds more case arms to >> > this case statement is of course not just good but necessary, >> > though. >> >> s/necessary/may be &/; if you add new arms before this one, you >> won't need it. But if you add one after this, you would ;-). > > Hmm... POSIX describes them as terminators :-) > > The compound-list for each list of patterns, with the possible > exception of the last, shall be terminated with ";;". A terminator that is optional at the end is a separator ;-). Having ';;' immediately before 'esac' is not wrong, but omitting it is exactly equally correct as having one, so it is not something we would want a patch to churn. > I'll drop this patch in the re-roll since it isn't necessary. This round looked good from a cursory read, except that the first one still makes me wonder why you chose to put it there _before_ where we handle --repo, where the corresponding case on "$cur" handles --repo= first and then --recurse-submodules= next. Wouldn't the end result easier to follow if you stuck to the same order? ``` ## Junio C Hamano, 2014-07-22 20:23 Subject: Re: [PATCH v2 1/3] completion: complete "unstuck" `git push --recurse-submodules` Message-ID: URL: https://gitlist.dev/e/xmqqr41df8hu.fsf%40gitster.dls.corp.google.com In-Reply-To: <8303c95dd3e3ee8d77d4b994bb2d33b6a155a17f.1406053442.git.john@keeping.me.uk> ``` John Keeping writes: > Since the argument to `--recurse-submodules` is mandatory, it does not > need to be stuck to the option with `=`. > > Signed-off-by: John Keeping > --- > Change since v1: > - Fix typo --recurse{_ => -}submodules > - Dropped previous patch 1/4 adding ";;" at the end of the "--repo" case > > contrib/completion/git-completion.bash | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash > index 7a6e1d7..bed3665 100644 > --- a/contrib/completion/git-completion.bash > +++ b/contrib/completion/git-completion.bash > @@ -1624,6 +1624,10 @@ __git_push_recurse_submodules="check on-demand" > _git_push () > { > case "$prev" in > + --recurse-submodules) > + __gitcomp "$__git_push_recurse_submodules" > + return > + ;; > --repo) > __gitcomp_nl "$(__git_remotes)" > return If you mimick the order they are handled in the case on "$cur", it would also let us sneak in the missing-optional ";;" to case/esac to keep symmetry between the two ;-) In other words, like this, perhaps? diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 019026e..b27f385 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1617,6 +1617,11 @@ _git_push () --repo) __gitcomp_nl "$(__git_remotes)" return + ;; + --recurse-submodules) + __gitcomp "$__git_push_recurse_submodules" + return + ;; esac case "$cur" in --repo=*) -- 2.0.2-892-g223db29 ``` ## John Keeping, 2014-07-22 20:57 Subject: Re: [PATCH v2 1/3] completion: complete "unstuck" `git push --recurse-submodules` Message-ID: <20140722205730.GD26927@serenity.lan> URL: https://gitlist.dev/e/20140722205730.GD26927%40serenity.lan In-Reply-To: ``` On Tue, Jul 22, 2014 at 01:23:25PM -0700, Junio C Hamano wrote: > John Keeping writes: > > > Since the argument to `--recurse-submodules` is mandatory, it does not > > need to be stuck to the option with `=`. > > > > Signed-off-by: John Keeping > > --- > > Change since v1: > > - Fix typo --recurse{_ => -}submodules > > - Dropped previous patch 1/4 adding ";;" at the end of the "--repo" case > > > > contrib/completion/git-completion.bash | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash > > index 7a6e1d7..bed3665 100644 > > --- a/contrib/completion/git-completion.bash > > +++ b/contrib/completion/git-completion.bash > > @@ -1624,6 +1624,10 @@ __git_push_recurse_submodules="check on-demand" > > _git_push () > > { > > case "$prev" in > > + --recurse-submodules) > > + __gitcomp "$__git_push_recurse_submodules" > > + return > > + ;; > > --repo) > > __gitcomp_nl "$(__git_remotes)" > > return > > If you mimick the order they are handled in the case on "$cur", it > would also let us sneak in the missing-optional ";;" to case/esac to > keep symmetry between the two ;-) > > In other words, like this, perhaps? Makes sense. I don't think I noted the order in the "$cur" case, I just put the new one in here so that they were sorted lexicographically. Do you want me to re-roll with this change or can you replace the patch while applying? > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash > index 019026e..b27f385 100644 > --- a/contrib/completion/git-completion.bash > +++ b/contrib/completion/git-completion.bash > @@ -1617,6 +1617,11 @@ _git_push () > --repo) > __gitcomp_nl "$(__git_remotes)" > return > + ;; > + --recurse-submodules) > + __gitcomp "$__git_push_recurse_submodules" > + return > + ;; > esac > case "$cur" in > --repo=*) > -- > 2.0.2-892-g223db29 ``` ## Junio C Hamano, 2014-07-22 21:17 Subject: Re: [PATCH v2 1/3] completion: complete "unstuck" `git push --recurse-submodules` Message-ID: URL: https://gitlist.dev/e/xmqqha29f606.fsf%40gitster.dls.corp.google.com In-Reply-To: <20140722205730.GD26927@serenity.lan> ``` John Keeping writes: > Makes sense. I don't think I noted the order in the "$cur" case, I just > put the new one in here so that they were sorted lexicographically. Unless there is particular reason, consistently using lexicographic order in all related places is one good way to organize things, but doing so would involve flipping orders in the other case statement for this particular patch. In general, I prefer to see people add new things at the end, when there is no particular reason to do otherwise. > Do you want me to re-roll with this change or can you replace the patch > while applying? I think I had to flip the third one to adjust to the change I suggested to this; the result will be on 'pu', so please double check when I push it out. Thanks. ``` ## John Keeping, 2014-07-23 19:35 Subject: Re: [PATCH v2 1/3] completion: complete "unstuck" `git push --recurse-submodules` Message-ID: <20140723193536.GE26927@serenity.lan> URL: https://gitlist.dev/e/20140723193536.GE26927%40serenity.lan In-Reply-To: ``` On Tue, Jul 22, 2014 at 02:17:13PM -0700, Junio C Hamano wrote: > John Keeping writes: > > > Do you want me to re-roll with this change or can you replace the patch > > while applying? > > I think I had to flip the third one to adjust to the change I > suggested to this; the result will be on 'pu', so please double > check when I push it out. The result on jk/more-push-completion looks good. Thanks. ``` ## Junio C Hamano, 2014-07-23 21:31 Subject: Re: [PATCH v2 1/3] completion: complete "unstuck" `git push --recurse-submodules` Message-ID: URL: https://gitlist.dev/e/xmqqr41bdaol.fsf%40gitster.dls.corp.google.com In-Reply-To: <20140723193536.GE26927@serenity.lan> ``` John Keeping writes: > On Tue, Jul 22, 2014 at 02:17:13PM -0700, Junio C Hamano wrote: >> John Keeping writes: >> >> > Do you want me to re-roll with this change or can you replace the patch >> > while applying? >> >> I think I had to flip the third one to adjust to the change I >> suggested to this; the result will be on 'pu', so please double >> check when I push it out. > > The result on jk/more-push-completion looks good. Thanks. Thanks. ```