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

Signed-off-by: Thomas Braun <redacted>
---
 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"
 		return
 	fi
 	__git_complete_strategy && return

On Mon, Jul 13, 2015 at 01:27:56PM +0200, Thomas Braun wrote:

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.

 		return
 	fi
 	__git_complete_strategy && return

Signed-off-by: Thomas Braun <redacted>
---

John Keeping [off-list ref] 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(-)

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

Quoting Thomas Braun [off-list ref]:

Signed-off-by: Thomas Braun <redacted>
---

quoted

John Keeping [off-list ref] 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.

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

+			__gitcomp "--continue --skip --abort --edit-todo"
+		else
+			__gitcomp "--continue --skip --abort"
+		fi
		return
	fi
	__git_complete_strategy && return
--
2.4.5.windows.1

On Thu, Jul 30, 2015 at 01:24:03PM +0200, SZEDER Gábor wrote:

Quoting Thomas Braun [off-list ref]:

quoted

Signed-off-by: Thomas Braun <redacted>
---

quoted

John Keeping [off-list ref] 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.

quoted

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.

Quoting John Keeping [off-list ref]:

On Thu, Jul 30, 2015 at 01:24:03PM +0200, SZEDER Gábor wrote:

quoted

Quoting Thomas Braun [off-list ref]:

quoted

Signed-off-by: Thomas Braun <redacted>
---

quoted

John Keeping [off-list ref] hat am 13. Juli 2015 um 15:11

geschrieben:

quoted

quoted

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

quoted

quoted

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.

quoted

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.

quoted

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

Best,
Gábor

Am 31.07.2015 um 12:16 schrieb SZEDER Gábor:

This looks much better than my attempt. Thanks.

How is the protocol now? Do I reroll and add 
Helped-By: John Keeping [off-list ref]
Completely-Overhauled-And-Properly-Implemented: SZEDER Gábor [off-list ref]
?

Thomas

Quoting Thomas Braun [off-list ref]:

Am 31.07.2015 um 12:16 schrieb SZEDER Gábor:

quoted

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 [off-list ref]
Completely-Overhauled-And-Properly-Implemented: SZEDER Gábor  
[off-list ref]

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

Signed-off-by: Thomas Braun <redacted>
Helped-by: John Keeping [off-list ref]
Helped-by: SZEDER Gábor [off-list ref]
---
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(-)

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