threads / patch / 39838

patchcompletion: Add '--edit-todo' to rebase

Subject: [PATCH] completion: Add '--edit-todo' to rebase

## tl;dr

9 messages between Jul 13, 2015 and Aug 5, 2015. Diffs are folded; open one to read it.

replies: 8people: 3as markdown or json

Thomas Braun· Jul 13, 2015, 11:27 UTC · lore
Signed-off-by: Thomas Braun <thomas.braun@virtuell-zuhause.de>
---
 contrib/completion/git-completion.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to contrib/completion/git-completion.bash +1 −1
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index c97c648..2567a61 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1668,7 +1668,7 @@ _git_rebase ()
 {
 	local dir="$(__gitdir)"
 	if [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then
-		__gitcomp "--continue --skip --abort"
+		__gitcomp "--continue --skip --abort --edit-todo"
 		return
 	fi
 	__git_complete_strategy && return
John Keeping· Jul 13, 2015, 13:11 UTC · re: Thomas Braun · lore

Re: [PATCH] completion: Add '--edit-todo' to rebase

On Mon, Jul 13, 2015 at 01:27:56PM +0200, Thomas Braun wrote:
Show 15 quoted lines
> Signed-off-by: Thomas Braun <thomas.braun@virtuell-zuhause.de>
> ---
>  contrib/completion/git-completion.bash | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index c97c648..2567a61 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1668,7 +1668,7 @@ _git_rebase ()
>  {
>  	local dir="$(__gitdir)"
>  	if [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then
> -		__gitcomp "--continue --skip --abort"
> +		__gitcomp "--continue --skip --abort --edit-todo"
git-rebase.sh contains:
	if test "$action" = "edit-todo" && test "$type" != "interactive"
	then
		die "$(gettext "The --edit-todo action can only be used during interactive rebase.")"
	fi

I wonder if it's worth doing a similar check here, which presumably means testing if "$dir"/interactive exists.

Show 5 quoted lines
>  		return
>  	fi
>  	__git_complete_strategy && return
> 
> 
Thomas Braun· Jul 27, 2015, 17:54 UTC · re: John Keeping · lore

[PATCH v2] completion: Add '--edit-todo' to rebase

Signed-off-by: Thomas Braun <thomas.braun@virtuell-zuhause.de>
---
Show 11 quoted lines
> John Keeping <john@keeping.me.uk> hat am 13. Juli 2015 um 15:11 geschrieben:
> git-rebase.sh contains:
> 
> 	if test "$action" = "edit-todo" && test "$type" != "interactive"
> 	then
> 		die "$(gettext "The --edit-todo action can only be used during interactive
> rebase.")"
> 	fi
> 
> I wonder if it's worth doing a similar check here, which presumably
> means testing if "$dir"/interactive exists.
Good point. Thanks for the hint.
 contrib/completion/git-completion.bash | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
Show changes to contrib/completion/git-completion.bash +5 −1
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index c97c648..b03050e 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1668,7 +1668,11 @@ _git_rebase ()
 {
 	local dir="$(__gitdir)"
 	if [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then
-		__gitcomp "--continue --skip --abort"
+		if [ -d "$dir"/interactive ]; then
+			__gitcomp "--continue --skip --abort --edit-todo"
+		else
+			__gitcomp "--continue --skip --abort"
+		fi
 		return
 	fi
 	__git_complete_strategy && return
-- 
2.4.5.windows.1
SZEDER Gábor· Jul 30, 2015, 11:24 UTC · re: Thomas Braun · lore

Re: [PATCH v2] completion: Add '--edit-todo' to rebase

Quoting Thomas Braun <thomas.braun@virtuell-zuhause.de>:
Show 15 quoted lines
> Signed-off-by: Thomas Braun <thomas.braun@virtuell-zuhause.de>
> ---
>> John Keeping <john@keeping.me.uk> hat am 13. Juli 2015 um 15:11 geschrieben:
>> git-rebase.sh contains:
>>
>> 	if test "$action" = "edit-todo" && test "$type" != "interactive"
>> 	then
>> 		die "$(gettext "The --edit-todo action can only be used during interactive
>> rebase.")"
>> 	fi
>>
>> I wonder if it's worth doing a similar check here, which presumably
>> means testing if "$dir"/interactive exists.
>
> Good point. Thanks for the hint.

Perhaps the subject line could say "completion: offer '--edit-todo' during interactive rebase" to be a bit more specific.

Show 14 quoted lines
> contrib/completion/git-completion.bash | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/contrib/completion/git-completion.bash
> b/contrib/completion/git-completion.bash
> index c97c648..b03050e 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1668,7 +1668,11 @@ _git_rebase ()
> {
> 	local dir="$(__gitdir)"
> 	if [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then
> -		__gitcomp "--continue --skip --abort"
> +		if [ -d "$dir"/interactive ]; then

This doesn't work for me, I think it looks for the right file at the wrong place. During an interactive rebase I have no '.git/interactive' file but a '.git/rebase-merge/interactive', so I never get '--edit-todo'.

After some playing around and a cursory look at the source it seems to me that I have '.git/rebase-apply' during a "regular" rebase and '.git/rebase-merge' during an interactive rebase, and git-rebase.sh checks the presence of the 'interactive' file only in '.git/rebase-merge'. It's not clear to me yet whether it's possible to have a '.git/rebase-merge' without the file 'interactive' in it. If it is possible, then I'd like to know with which commands and under what circumstances. If it isn't, then we wouldn't have to look for the file at all, because checking the presence of the directory would be enough.

Best, Gábor

Show 9 quoted lines
> +			__gitcomp "--continue --skip --abort --edit-todo"
> +		else
> +			__gitcomp "--continue --skip --abort"
> +		fi
> 		return
> 	fi
> 	__git_complete_strategy && return
> --
> 2.4.5.windows.1
John Keeping· Jul 30, 2015, 11:29 UTC · re: SZEDER Gábor · lore

Re: [PATCH v2] completion: Add '--edit-todo' to rebase

On Thu, Jul 30, 2015 at 01:24:03PM +0200, SZEDER Gábor wrote:
Show 52 quoted lines
> 
> Quoting Thomas Braun <thomas.braun@virtuell-zuhause.de>:
> 
> > Signed-off-by: Thomas Braun <thomas.braun@virtuell-zuhause.de>
> > ---
> >> John Keeping <john@keeping.me.uk> hat am 13. Juli 2015 um 15:11 geschrieben:
> >> git-rebase.sh contains:
> >>
> >> 	if test "$action" = "edit-todo" && test "$type" != "interactive"
> >> 	then
> >> 		die "$(gettext "The --edit-todo action can only be used during interactive
> >> rebase.")"
> >> 	fi
> >>
> >> I wonder if it's worth doing a similar check here, which presumably
> >> means testing if "$dir"/interactive exists.
> >
> > Good point. Thanks for the hint.
> 
> Perhaps the subject line could say "completion: offer '--edit-todo'  
> during interactive rebase" to be a bit more specific.
> 
> > contrib/completion/git-completion.bash | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/contrib/completion/git-completion.bash
> > b/contrib/completion/git-completion.bash
> > index c97c648..b03050e 100644
> > --- a/contrib/completion/git-completion.bash
> > +++ b/contrib/completion/git-completion.bash
> > @@ -1668,7 +1668,11 @@ _git_rebase ()
> > {
> > 	local dir="$(__gitdir)"
> > 	if [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then
> > -		__gitcomp "--continue --skip --abort"
> > +		if [ -d "$dir"/interactive ]; then
> 
> This doesn't work for me, I think it looks for the right file at the  
> wrong place.  During an interactive rebase I have no  
> '.git/interactive' file but a '.git/rebase-merge/interactive', so I  
> never get '--edit-todo'.
> 
> After some playing around and a cursory look at the source it seems to  
> me that I have '.git/rebase-apply' during a "regular" rebase and  
> '.git/rebase-merge' during an interactive rebase, and git-rebase.sh  
> checks the presence of the 'interactive' file only in  
> '.git/rebase-merge'.  It's not clear to me yet whether it's possible  
> to have a '.git/rebase-merge' without the file 'interactive' in it.   
> If it is possible, then I'd like to know with which commands and under  
> what circumstances.  If it isn't, then we wouldn't have to look for  
> the file at all, because checking the presence of the directory would  
> be enough.

"git rebase --merge" will use ".git/rebase-merge" without creating the "interactive" flag.

SZEDER Gábor· Jul 31, 2015, 10:16 UTC · re: John Keeping · lore

Re: [PATCH v2] completion: Add '--edit-todo' to rebase

Quoting John Keeping <john@keeping.me.uk>:
Show 44 quoted lines
> On Thu, Jul 30, 2015 at 01:24:03PM +0200, SZEDER Gábor wrote:
>>
>> Quoting Thomas Braun <thomas.braun@virtuell-zuhause.de>:
>>
>>> Signed-off-by: Thomas Braun <thomas.braun@virtuell-zuhause.de>
>>> ---
>>>> John Keeping <john@keeping.me.uk> hat am 13. Juli 2015 um 15:11
>> geschrieben:
>>>> git-rebase.sh contains:
>>>>
>>>> 	if test "$action" = "edit-todo" && test "$type" != "interactive"
>>>> 	then
>>>> 		die "$(gettext "The --edit-todo action can only be used during
>> interactive
>>>> rebase.")"
>>>> 	fi
>>>>
>>>> I wonder if it's worth doing a similar check here, which presumably
>>>> means testing if "$dir"/interactive exists.
>>>
>>> Good point. Thanks for the hint.
>>
>> Perhaps the subject line could say "completion: offer '--edit-todo'
>> during interactive rebase" to be a bit more specific.
>>
>>> contrib/completion/git-completion.bash | 6 +++++-
>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/contrib/completion/git-completion.bash
>>> b/contrib/completion/git-completion.bash
>>> index c97c648..b03050e 100644
>>> --- a/contrib/completion/git-completion.bash
>>> +++ b/contrib/completion/git-completion.bash
>>> @@ -1668,7 +1668,11 @@ _git_rebase ()
>>> {
>>> 	local dir="$(__gitdir)"
>>> 	if [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then
>>> -		__gitcomp "--continue --skip --abort"
>>> +		if [ -d "$dir"/interactive ]; then
>>
>> This doesn't work for me, I think it looks for the right file at the
>> wrong place.  During an interactive rebase I have no
>> '.git/interactive' file but a '.git/rebase-merge/interactive', so I
>> never get '--edit-todo'.

Just noticed another issue here: it looks for a directory, though it should look for a file.

Show 13 quoted lines
>> After some playing around and a cursory look at the source it seems to
>> me that I have '.git/rebase-apply' during a "regular" rebase and
>> '.git/rebase-merge' during an interactive rebase, and git-rebase.sh
>> checks the presence of the 'interactive' file only in
>> '.git/rebase-merge'.  It's not clear to me yet whether it's possible
>> to have a '.git/rebase-merge' without the file 'interactive' in it.
>> If it is possible, then I'd like to know with which commands and under
>> what circumstances.  If it isn't, then we wouldn't have to look for
>> the file at all, because checking the presence of the directory would
>> be enough.
>
> "git rebase --merge" will use ".git/rebase-merge" without creating the
> "interactive" flag.

Oh, right, thanks. I should have remembered, I wrote the test of the prompt script for that case... (On a related note: is it possible to have a '.git/rebase-apply' directory, but neither 'rebasing' or 'applying' files within? The prompt script has a long if-elif chain with such a branch, and I remember wondering how I could trigger it for testing.)

Anyway, so this could be something like (modulo likely whitespace damage):
Show changes to contrib/completion/git-completion.bash +4 −1
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 07c34ef913..fac01d6985 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1667,7 +1667,10 @@ _git_push ()
   _git_rebase ()
   {
   	local dir="$(__gitdir)"
-	if [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then
+	if [ -f "$dir"/rebase-merge/interactive ]; then
+		__gitcomp "--continue --skip --abort --edit-todo"
+		return
+	elif [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then
   		__gitcomp "--continue --skip --abort"
   		return
   	fi


Best,
Gábor
Thomas Braun· Jul 31, 2015, 17:37 UTC · re: SZEDER Gábor · lore

Re: [PATCH v2] completion: Add '--edit-todo' to rebase

Am 31.07.2015 um 12:16 schrieb SZEDER Gábor:
Show 92 quoted lines
> 
> Quoting John Keeping <john@keeping.me.uk>:
> 
>> On Thu, Jul 30, 2015 at 01:24:03PM +0200, SZEDER Gábor wrote:
>>>
>>> Quoting Thomas Braun <thomas.braun@virtuell-zuhause.de>:
>>>
>>>> Signed-off-by: Thomas Braun <thomas.braun@virtuell-zuhause.de>
>>>> ---
>>>>> John Keeping <john@keeping.me.uk> hat am 13. Juli 2015 um 15:11
>>> geschrieben:
>>>>> git-rebase.sh contains:
>>>>>
>>>>>     if test "$action" = "edit-todo" && test "$type" != "interactive"
>>>>>     then
>>>>>         die "$(gettext "The --edit-todo action can only be used during
>>> interactive
>>>>> rebase.")"
>>>>>     fi
>>>>>
>>>>> I wonder if it's worth doing a similar check here, which presumably
>>>>> means testing if "$dir"/interactive exists.
>>>>
>>>> Good point. Thanks for the hint.
>>>
>>> Perhaps the subject line could say "completion: offer '--edit-todo'
>>> during interactive rebase" to be a bit more specific.
>>>
>>>> contrib/completion/git-completion.bash | 6 +++++-
>>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/contrib/completion/git-completion.bash
>>>> b/contrib/completion/git-completion.bash
>>>> index c97c648..b03050e 100644
>>>> --- a/contrib/completion/git-completion.bash
>>>> +++ b/contrib/completion/git-completion.bash
>>>> @@ -1668,7 +1668,11 @@ _git_rebase ()
>>>> {
>>>>     local dir="$(__gitdir)"
>>>>     if [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then
>>>> -        __gitcomp "--continue --skip --abort"
>>>> +        if [ -d "$dir"/interactive ]; then
>>>
>>> This doesn't work for me, I think it looks for the right file at the
>>> wrong place.  During an interactive rebase I have no
>>> '.git/interactive' file but a '.git/rebase-merge/interactive', so I
>>> never get '--edit-todo'.
> 
> Just noticed another issue here: it looks for a directory, though it
> should look for a file.
> 
> 
>>> After some playing around and a cursory look at the source it seems to
>>> me that I have '.git/rebase-apply' during a "regular" rebase and
>>> '.git/rebase-merge' during an interactive rebase, and git-rebase.sh
>>> checks the presence of the 'interactive' file only in
>>> '.git/rebase-merge'.  It's not clear to me yet whether it's possible
>>> to have a '.git/rebase-merge' without the file 'interactive' in it.
>>> If it is possible, then I'd like to know with which commands and under
>>> what circumstances.  If it isn't, then we wouldn't have to look for
>>> the file at all, because checking the presence of the directory would
>>> be enough.
>>
>> "git rebase --merge" will use ".git/rebase-merge" without creating the
>> "interactive" flag.
> 
> Oh, right, thanks.  I should have remembered, I wrote the test of the
> prompt script for that case...
> (On a related note: is it possible to have a '.git/rebase-apply'
> directory, but neither 'rebasing' or 'applying' files within?  The
> prompt script has a long if-elif chain with such a branch, and I
> remember wondering how I could trigger it for testing.)
> 
> Anyway, so this could be something like (modulo likely whitespace damage):
> 
> diff --git a/contrib/completion/git-completion.bash
> b/contrib/completion/git-completion.bash
> index 07c34ef913..fac01d6985 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1667,7 +1667,10 @@ _git_push ()
>   _git_rebase ()
>   {
>       local dir="$(__gitdir)"
> -    if [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then
> +    if [ -f "$dir"/rebase-merge/interactive ]; then
> +        __gitcomp "--continue --skip --abort --edit-todo"
> +        return
> +    elif [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then
>           __gitcomp "--continue --skip --abort"
>           return
>       fi
This looks much better than my attempt. Thanks.
How is the protocol now? Do I reroll and add 
Helped-By: John Keeping <john@keeping.me.uk>
Completely-Overhauled-And-Properly-Implemented: SZEDER Gábor <szeder@ira.uka.de>
?
Thomas
SZEDER Gábor· Aug 1, 2015, 09:25 UTC · re: Thomas Braun · lore

Re: [PATCH v2] completion: Add '--edit-todo' to rebase

Quoting Thomas Braun <thomas.braun@virtuell-zuhause.de>:
Show 27 quoted lines
> Am 31.07.2015 um 12:16 schrieb SZEDER Gábor:
>> Anyway, so this could be something like (modulo likely whitespace damage):
>>
>> diff --git a/contrib/completion/git-completion.bash
>> b/contrib/completion/git-completion.bash
>> index 07c34ef913..fac01d6985 100644
>> --- a/contrib/completion/git-completion.bash
>> +++ b/contrib/completion/git-completion.bash
>> @@ -1667,7 +1667,10 @@ _git_push ()
>>   _git_rebase ()
>>   {
>>       local dir="$(__gitdir)"
>> -    if [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then
>> +    if [ -f "$dir"/rebase-merge/interactive ]; then
>> +        __gitcomp "--continue --skip --abort --edit-todo"
>> +        return
>> +    elif [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then
>>           __gitcomp "--continue --skip --abort"
>>           return
>>       fi
>
> This looks much better than my attempt. Thanks.
>
> How is the protocol now? Do I reroll and add
> Helped-By: John Keeping <john@keeping.me.uk>
> Completely-Overhauled-And-Properly-Implemented: SZEDER Gábor  
> <szeder@ira.uka.de>

Ugh :) I'm quite happy with Helped-by, if you do a proper reroll after trying it out to see that it indeed does what it should.

Thanks, Gábor

Thomas Braun· Aug 5, 2015, 13:40 UTC · re: SZEDER Gábor · lore

[PATCH v3 1/1] completion: offer '--edit-todo' during interactive rebase

Signed-off-by: Thomas Braun <thomas.braun@virtuell-zuhause.de>
Helped-by: John Keeping <john@keeping.me.uk>
Helped-by: SZEDER Gábor <szeder@ira.uka.de>
---
Tested by:
- ensuring I'm in a bash shell
- source git-completion.bash
- git rebase -i HEAD~1, choose edit instead of pick in the editor
- on entering "git rebase" you should be offered "--edit-todo"
 contrib/completion/git-completion.bash | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
Show changes to contrib/completion/git-completion.bash +4 −1
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index c97c648..087771b 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1667,7 +1667,10 @@ _git_push ()
 _git_rebase ()
 {
 	local dir="$(__gitdir)"
-	if [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then
+	if [ -f "$dir"/rebase-merge/interactive ]; then
+		__gitcomp "--continue --skip --abort --edit-todo"
+		return
+	elif [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then
 		__gitcomp "--continue --skip --abort"
 		return
 	fi
-- 
2.4.3.413.ga5fe668

← back to recent threads