[PATCH] completion: suggest sequencer commands for revert

[PATCH] completion: suggest sequencer commands for revert

[Thomas Braun]

Signed-off-by: Thomas Braun <thomas.braun@virtuell-zuhause.de>
---

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"

Re: [PATCH] completion: suggest sequencer commands for revert

[Junio C Hamano]

Thomas Braun <thomas.braun@virtuell-zuhause.de> writes:

> Signed-off-by: Thomas Braun <thomas.braun@virtuell-zuhause.de>
> ---
>
> 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"

Re: [PATCH] completion: suggest sequencer commands for revert

[Ramkumar Ramachandra]

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.

[PATCH v2 0/2] completion: sequencer commands

[Thomas Braun]

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.

[PATCH v2 2/2] completion: suggest sequencer commands for revert

[Thomas Braun]

Signed-off-by: Thomas Braun <thomas.braun@virtuell-zuhause.de>
---
 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"

[PATCH v2 1/2] completion: Add sequencer function

[Thomas Braun]

Signed-off-by: Thomas Braun <thomas.braun@virtuell-zuhause.de>
---
 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=*)

[PATCH v2 1/2] completion: Add sequencer function

[Thomas Braun]

Signed-off-by: Thomas Braun <thomas.braun@virtuell-zuhause.de>
---
 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=*)

Re: [PATCH v2 1/2] completion: Add sequencer function

[SZEDER Gábor]

Quoting Thomas Braun <thomas.braun@virtuell-zuhause.de>:

> Signed-off-by: Thomas Braun <thomas.braun@virtuell-zuhause.de>
> ---
>  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=*)

Re: [PATCH v2 1/2] completion: Add sequencer function

[Junio C Hamano]

SZEDER Gábor <szeder@ira.uka.de> 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?

Re: [PATCH v2 1/2] completion: Add sequencer function

[SZEDER Gábor]

Quoting Junio C Hamano <gitster@pobox.com>:

> SZEDER Gábor <szeder@ira.uka.de> 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