threads / patch / 39417

patchcompletion: suggest sequencer commands for revert

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

## tl;dr

10 messages between May 25, 2015 and Jun 1, 2015. Diffs are folded; open one to read it.

replies: 9people: 4as markdown or json

Thomas Braun· May 25, 2015, 09:59 UTC · lore
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(+)
Show changes to contrib/completion/git-completion.bash +5 −0
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· May 29, 2015, 19:50 UTC · re: Thomas Braun · lore

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

Thomas Braun <thomas.braun@virtuell-zuhause.de> writes:
Show 7 quoted lines
> 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.
Show 23 quoted lines
>
> 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· May 29, 2015, 23:13 UTC · re: Junio C Hamano · lore

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

Junio C Hamano wrote:
Show 20 quoted lines
>
> >  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· May 30, 2015, 15:57 UTC · re: Ramkumar Ramachandra · lore

[PATCH v2 0/2] completion: sequencer commands

Ramkumar Ramachandra wrote:
Show 34 quoted lines
> 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· May 30, 2015, 16:01 UTC · re: Ramkumar Ramachandra · lore

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

Signed-off-by: Thomas Braun <thomas.braun@virtuell-zuhause.de>
---
 contrib/completion/git-completion.bash | 8 ++++++++
 1 file changed, 8 insertions(+)
Show changes to contrib/completion/git-completion.bash +8 −0
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· May 30, 2015, 16:01 UTC · re: Ramkumar Ramachandra · lore

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

Signed-off-by: Thomas Braun <thomas.braun@virtuell-zuhause.de>
---
 contrib/completion/git-completion.bash | 48 +++++++++++++++++++++++-----------
 1 file changed, 33 insertions(+), 15 deletions(-)
Show changes to contrib/completion/git-completion.bash +33 −15
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· May 30, 2015, 19:01 UTC · re: Thomas Braun · lore

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

Quoting Thomas Braun <thomas.braun@virtuell-zuhause.de>:
Show 5 quoted lines
> 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
Show 78 quoted lines
> 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· Jun 1, 2015, 14:38 UTC · re: SZEDER Gábor · lore

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

SZEDER Gábor <szeder@ira.uka.de> writes:
Show 8 quoted lines
> 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· Jun 1, 2015, 15:06 UTC · re: Junio C Hamano · lore

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

Quoting Junio C Hamano <gitster@pobox.com>:
Show 15 quoted lines
> 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
Thomas Braun· May 30, 2015, 16:02 UTC · re: Ramkumar Ramachandra · lore

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

Signed-off-by: Thomas Braun <thomas.braun@virtuell-zuhause.de>
---
 contrib/completion/git-completion.bash | 48 +++++++++++++++++++++++-----------
 1 file changed, 33 insertions(+), 15 deletions(-)
Show changes to contrib/completion/git-completion.bash +33 −15
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=*)

← back to recent threads