# [PATCH] completion: suggest sequencer commands for revert 10 messages from 2015-05-25 to 2015-06-01. Participants: Thomas Braun, Junio C Hamano, Ramkumar Ramachandra, SZEDER Gábor. Thread: https://gitlist.dev/t/39417 ## Thomas Braun, 2015-05-25 09:59 Subject: [PATCH] completion: suggest sequencer commands for revert Message-ID: <1432547975.4644.5.camel@virtuell-zuhause.de> URL: https://gitlist.dev/e/1432547975.4644.5.camel%40virtuell-zuhause.de ``` Signed-off-by: Thomas Braun --- Hi, I added the sequencer commands for git revert. These are handy in case a git revert needs manual intervention. Thanks, Thomas contrib/completion/git-completion.bash | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index bfc74e9..3c00acd 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -2282,6 +2282,11 @@ _git_reset () _git_revert () { + local dir="$(__gitdir)" + if [ -f "$dir"/REVERT_HEAD ]; then + __gitcomp "--continue --quit --abort" + return + fi case "$cur" in --*) __gitcomp "--edit --mainline --no-edit --no-commit --signoff" ``` ## Junio C Hamano, 2015-05-29 19:50 Subject: Re: [PATCH] completion: suggest sequencer commands for revert Message-ID: URL: https://gitlist.dev/e/xmqq1thzcgde.fsf%40gitster.dls.corp.google.com In-Reply-To: <1432547975.4644.5.camel@virtuell-zuhause.de> ``` Thomas Braun writes: > Signed-off-by: Thomas Braun > --- > > Hi, > > I added the sequencer commands for git revert. These are handy in case a git > revert needs manual intervention. This looks OK from a cursory read to me; asking opinions from those who have touched the file in the recent past (Ram also happens to be one of the people who were heavily involved in sequencer work). Thanks. > > Thanks, > Thomas > > contrib/completion/git-completion.bash | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash > index bfc74e9..3c00acd 100644 > --- a/contrib/completion/git-completion.bash > +++ b/contrib/completion/git-completion.bash > @@ -2282,6 +2282,11 @@ _git_reset () > > _git_revert () > { > + local dir="$(__gitdir)" > + if [ -f "$dir"/REVERT_HEAD ]; then > + __gitcomp "--continue --quit --abort" > + return > + fi > case "$cur" in > --*) > __gitcomp "--edit --mainline --no-edit --no-commit --signoff" ``` ## Ramkumar Ramachandra, 2015-05-29 23:13 Subject: Re: [PATCH] completion: suggest sequencer commands for revert Message-ID: URL: https://gitlist.dev/e/CALkWK0m9Fj_3AnJcENL-kAFSbYjW4fKpN76Tftrt672hQj4qww%40mail.gmail.com In-Reply-To: ``` Junio C Hamano wrote: > > > contrib/completion/git-completion.bash | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash > > index bfc74e9..3c00acd 100644 > > --- a/contrib/completion/git-completion.bash > > +++ b/contrib/completion/git-completion.bash > > @@ -2282,6 +2282,11 @@ _git_reset () > > > > _git_revert () > > { > > + local dir="$(__gitdir)" > > + if [ -f "$dir"/REVERT_HEAD ]; then > > + __gitcomp "--continue --quit --abort" > > + return > > + fi > > case "$cur" in > > --*) > > __gitcomp "--edit --mainline --no-edit --no-commit --signoff" This corresponds exactly to what we do for git-cherry-pick: local dir="$(__gitdir)" if [ -f "$dir"/CHERRY_PICK_HEAD ]; then __gitcomp "--continue --quit --abort" return fi Perhaps _git_revert() and _git_cherry_pick() should call into the same function with different arguments. This looks fine though. ``` ## Thomas Braun, 2015-05-30 15:57 Subject: [PATCH v2 0/2] completion: sequencer commands Message-ID: <1433001452.5113.4.camel@virtuell-zuhause.de> URL: https://gitlist.dev/e/1433001452.5113.4.camel%40virtuell-zuhause.de In-Reply-To: ``` Ramkumar Ramachandra wrote: > Junio C Hamano wrote: > > > > > contrib/completion/git-completion.bash | 5 +++++ > > > 1 file changed, 5 insertions(+) > > > > > > diff --git a/contrib/completion/git-completion.bash > b/contrib/completion/git-completion.bash > > > index bfc74e9..3c00acd 100644 > > > --- a/contrib/completion/git-completion.bash > > > +++ b/contrib/completion/git-completion.bash > > > @@ -2282,6 +2282,11 @@ _git_reset () > > > > > > _git_revert () > > > { > > > + local dir="$(__gitdir)" > > > + if [ -f "$dir"/REVERT_HEAD ]; then > > > + __gitcomp "--continue --quit --abort" > > > + return > > > + fi > > > case "$cur" in > > > --*) > > > __gitcomp "--edit --mainline --no-edit --no-commit > --signoff" > > This corresponds exactly to what we do for git-cherry-pick: > > local dir="$(__gitdir)" > if [ -f "$dir"/CHERRY_PICK_HEAD ]; then > __gitcomp "--continue --quit --abort" > return > fi > > Perhaps _git_revert() and _git_cherry_pick() should call into the same > function with different arguments. Good idea. I created a new function __git_complete_sequencer which is now used to complete all commands with active sequencer. ``` ## Thomas Braun, 2015-05-30 16:01 Subject: [PATCH v2 2/2] completion: suggest sequencer commands for revert Message-ID: <1433001691.5113.7.camel@virtuell-zuhause.de> URL: https://gitlist.dev/e/1433001691.5113.7.camel%40virtuell-zuhause.de In-Reply-To: ``` Signed-off-by: Thomas Braun --- contrib/completion/git-completion.bash | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index f6e5bf6..486c61b 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -868,6 +868,12 @@ __git_complete_sequencer () return 0 fi ;; + revert) + if [ -f "$dir"/REVERT_HEAD ]; then + __gitcomp "--continue --quit --abort" + return 0 + fi + ;; rebase) if [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then __gitcomp "--continue --skip --abort" @@ -2300,6 +2306,8 @@ _git_reset () _git_revert () { + __git_complete_sequencer "revert" && return + case "$cur" in --*) __gitcomp "--edit --mainline --no-edit --no-commit --signoff" ``` ## Thomas Braun, 2015-05-30 16:01 Subject: [PATCH v2 1/2] completion: Add sequencer function Message-ID: <1433001710.5113.8.camel@virtuell-zuhause.de> URL: https://gitlist.dev/e/1433001710.5113.8.camel%40virtuell-zuhause.de In-Reply-To: ``` Signed-off-by: Thomas Braun --- contrib/completion/git-completion.bash | 48 +++++++++++++++++++++++----------- 1 file changed, 33 insertions(+), 15 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index bfc74e9..f6e5bf6 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -851,15 +851,40 @@ __git_count_arguments () printf "%d" $c } +__git_complete_sequencer () +{ + local dir="$(__gitdir)" + + case "$1" in + am) + if [ -d "$dir"/rebase-apply ]; then + __gitcomp "--skip --continue --resolved --abort" + return 0 + fi + ;; + cherry-pick) + if [ -f "$dir"/CHERRY_PICK_HEAD ]; then + __gitcomp "--continue --quit --abort" + return 0 + fi + ;; + rebase) + if [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then + __gitcomp "--continue --skip --abort" + return 0 + fi + ;; + esac + + return 1 +} + __git_whitespacelist="nowarn warn error error-all fix" _git_am () { - local dir="$(__gitdir)" - if [ -d "$dir"/rebase-apply ]; then - __gitcomp "--skip --continue --resolved --abort" - return - fi + __git_complete_sequencer "am" && return + case "$cur" in --whitespace=*) __gitcomp "$__git_whitespacelist" "" "${cur##--whitespace=}" @@ -1044,11 +1069,8 @@ _git_cherry () _git_cherry_pick () { - local dir="$(__gitdir)" - if [ -f "$dir"/CHERRY_PICK_HEAD ]; then - __gitcomp "--continue --quit --abort" - return - fi + __git_complete_sequencer "cherry-pick" && return + case "$cur" in --*) __gitcomp "--edit --no-commit --signoff --strategy= --mainline" @@ -1666,11 +1688,7 @@ _git_push () _git_rebase () { - local dir="$(__gitdir)" - if [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then - __gitcomp "--continue --skip --abort" - return - fi + __git_complete_sequencer "rebase" && return __git_complete_strategy && return case "$cur" in --whitespace=*) ``` ## Thomas Braun, 2015-05-30 16:02 Subject: [PATCH v2 1/2] completion: Add sequencer function Message-ID: <1433001725.5113.9.camel@virtuell-zuhause.de> URL: https://gitlist.dev/e/1433001725.5113.9.camel%40virtuell-zuhause.de In-Reply-To: ``` Signed-off-by: Thomas Braun --- contrib/completion/git-completion.bash | 48 +++++++++++++++++++++++----------- 1 file changed, 33 insertions(+), 15 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index bfc74e9..f6e5bf6 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -851,15 +851,40 @@ __git_count_arguments () printf "%d" $c } +__git_complete_sequencer () +{ + local dir="$(__gitdir)" + + case "$1" in + am) + if [ -d "$dir"/rebase-apply ]; then + __gitcomp "--skip --continue --resolved --abort" + return 0 + fi + ;; + cherry-pick) + if [ -f "$dir"/CHERRY_PICK_HEAD ]; then + __gitcomp "--continue --quit --abort" + return 0 + fi + ;; + rebase) + if [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then + __gitcomp "--continue --skip --abort" + return 0 + fi + ;; + esac + + return 1 +} + __git_whitespacelist="nowarn warn error error-all fix" _git_am () { - local dir="$(__gitdir)" - if [ -d "$dir"/rebase-apply ]; then - __gitcomp "--skip --continue --resolved --abort" - return - fi + __git_complete_sequencer "am" && return + case "$cur" in --whitespace=*) __gitcomp "$__git_whitespacelist" "" "${cur##--whitespace=}" @@ -1044,11 +1069,8 @@ _git_cherry () _git_cherry_pick () { - local dir="$(__gitdir)" - if [ -f "$dir"/CHERRY_PICK_HEAD ]; then - __gitcomp "--continue --quit --abort" - return - fi + __git_complete_sequencer "cherry-pick" && return + case "$cur" in --*) __gitcomp "--edit --no-commit --signoff --strategy= --mainline" @@ -1666,11 +1688,7 @@ _git_push () _git_rebase () { - local dir="$(__gitdir)" - if [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then - __gitcomp "--continue --skip --abort" - return - fi + __git_complete_sequencer "rebase" && return __git_complete_strategy && return case "$cur" in --whitespace=*) ``` ## SZEDER Gábor, 2015-05-30 19:01 Subject: Re: [PATCH v2 1/2] completion: Add sequencer function Message-ID: <20150530210103.Horde.YoJthfdQJy5oq9wT_oOl3Q1@webmail.informatik.kit.edu> URL: https://gitlist.dev/e/20150530210103.Horde.YoJthfdQJy5oq9wT_oOl3Q1%40webmail.informatik.kit.edu In-Reply-To: <1433001710.5113.8.camel@virtuell-zuhause.de> ``` Quoting Thomas Braun : > Signed-off-by: Thomas Braun > --- > contrib/completion/git-completion.bash | 48 > +++++++++++++++++++++++----------- > 1 file changed, 33 insertions(+), 15 deletions(-) I don't see the benefits of this change. This patch adds more than twice as many lines as it removes, and patch 2/2 adds 8 new lines although it could get away with only 5 without this function. To offer sequencer options we currently go through a single if statement, with this patch we'd go through a case statement, an if statement and finally an &&. Gábor > diff --git a/contrib/completion/git-completion.bash > b/contrib/completion/git-completion.bash > index bfc74e9..f6e5bf6 100644 > --- a/contrib/completion/git-completion.bash > +++ b/contrib/completion/git-completion.bash > @@ -851,15 +851,40 @@ __git_count_arguments () > printf "%d" $c > } > > +__git_complete_sequencer () > +{ > + local dir="$(__gitdir)" > + > + case "$1" in > + am) > + if [ -d "$dir"/rebase-apply ]; then > + __gitcomp "--skip --continue --resolved --abort" > + return 0 > + fi > + ;; > + cherry-pick) > + if [ -f "$dir"/CHERRY_PICK_HEAD ]; then > + __gitcomp "--continue --quit --abort" > + return 0 > + fi > + ;; > + rebase) > + if [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then > + __gitcomp "--continue --skip --abort" > + return 0 > + fi > + ;; > + esac > + > + return 1 > +} > + > __git_whitespacelist="nowarn warn error error-all fix" > > _git_am () > { > - local dir="$(__gitdir)" > - if [ -d "$dir"/rebase-apply ]; then > - __gitcomp "--skip --continue --resolved --abort" > - return > - fi > + __git_complete_sequencer "am" && return > + > case "$cur" in > --whitespace=*) > __gitcomp "$__git_whitespacelist" "" "${cur##--whitespace=}" > @@ -1044,11 +1069,8 @@ _git_cherry () > > _git_cherry_pick () > { > - local dir="$(__gitdir)" > - if [ -f "$dir"/CHERRY_PICK_HEAD ]; then > - __gitcomp "--continue --quit --abort" > - return > - fi > + __git_complete_sequencer "cherry-pick" && return > + > case "$cur" in > --*) > __gitcomp "--edit --no-commit --signoff --strategy= --mainline" > @@ -1666,11 +1688,7 @@ _git_push () > > _git_rebase () > { > - local dir="$(__gitdir)" > - if [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then > - __gitcomp "--continue --skip --abort" > - return > - fi > + __git_complete_sequencer "rebase" && return > __git_complete_strategy && return > case "$cur" in > --whitespace=*) ``` ## Junio C Hamano, 2015-06-01 14:38 Subject: Re: [PATCH v2 1/2] completion: Add sequencer function Message-ID: URL: https://gitlist.dev/e/xmqq6177a3zk.fsf%40gitster.dls.corp.google.com In-Reply-To: <20150530210103.Horde.YoJthfdQJy5oq9wT_oOl3Q1@webmail.informatik.kit.edu> ``` SZEDER Gábor writes: > I don't see the benefits of this change. This patch adds more than > twice as many lines as it removes, and patch 2/2 adds 8 new lines > although it could get away with only 5 without this function. To > offer sequencer options we currently go through a single if statement, > with this patch we'd go through a case statement, an if statement and > finally an &&. > > Gábor Perhaps, especially given that I'd imagine we won't be adding 47 new commands that drive the sequencer in the near future ;-) I presume that you are OK with Thomas's original version, then? ``` ## SZEDER Gábor, 2015-06-01 15:06 Subject: Re: [PATCH v2 1/2] completion: Add sequencer function Message-ID: <20150601170603.Horde.iiRpDlRNsOoCFFaHVSBOwg1@webmail.informatik.kit.edu> URL: https://gitlist.dev/e/20150601170603.Horde.iiRpDlRNsOoCFFaHVSBOwg1%40webmail.informatik.kit.edu In-Reply-To: ``` Quoting Junio C Hamano : > SZEDER Gábor writes: > >> I don't see the benefits of this change. This patch adds more than >> twice as many lines as it removes, and patch 2/2 adds 8 new lines >> although it could get away with only 5 without this function. To >> offer sequencer options we currently go through a single if statement, >> with this patch we'd go through a case statement, an if statement and >> finally an &&. >> >> Gábor > > Perhaps, especially given that I'd imagine we won't be adding 47 new > commands that drive the sequencer in the near future ;-) > > I presume that you are OK with Thomas's original version, then? Yes, definitely. It's a shame all these sequencing commands have different sets of sequencer options. Perhaps something to clean up for, say, v3.0 :) Gábor ```