threads / patch / 34722

v2, 2 partsfix cases of rebase -i ignoring core.commentchar

Subject: [PATCH v2 0/2] fix cases of rebase -i ignoring core.commentchar

## tl;dr

5 messages between Aug 18, 2013 and Aug 19, 2013. Diffs are folded; open one to read it.

replies: 4people: 2as markdown or json

Eric Sunshine· Aug 18, 2013, 17:12 UTC · lore

This is a reroll of [1] which fixes a couple cases where rebase -i ignores core.commentchar. One case was an oversight from the original patch which added core.commentchar support to rebase -i, and is already in 'maint'. The other is a regression introduced by [2] which is still in 'next'.

In this reroll, the single patch from v1 is split into two: patch 1/2 is suitable for 'maint'; and patch 2/2 is for 'next'. There are no other changes.

I wanted to add tests to t3404 for these bugs but couldn't figure out how to do it using the external interface of rebase -i. I was able to verify before and after behavior by adding temporary echo's to the code in order to observe the "internal" functioning.

[1]: http://git.661346.n2.nabble.com/PATCH-rebase-i-fix-cases-ignoring-core-commentchar-td7593965.html [2]: http://git.661346.n2.nabble.com/PATCH-0-3-fix-interactive-rebase-short-SHA-1-collision-bug-td7593673.html#a7593676

Eric Sunshine (2):
  rebase -i: fix skip_unnecessary_picks() to respect core.commentchar
  rebase -i: fix core.commentchar regression

 git-rebase--interactive.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
1.8.4.rc3.500.gc3113b0

Eric Sunshine· Aug 18, 2013, 17:12 UTC · re: Eric Sunshine · lore

[PATCH v2 1/2] rebase -i: fix skip_unnecessary_picks() to respect core.commentchar

eff80a9fd990de36 (Allow custom "comment char", 2013-01-16) added general core.commentchar support but forgot to update git-rebase--interactive to respect it. 180bad3d10fe3a7f (rebase -i: respect core.commentchar, 2013-02-11) addressed this oversight but missed one instance of hard-coded '#' comment character in skip_unnecessary_picks(). Fix this.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---

Suitable for 'maint'.

 git-rebase--interactive.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Show changes to git-rebase--interactive.sh +1 −1

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index ea11e62..3733312 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -671,7 +671,7 @@ skip_unnecessary_picks () {
 				;;
 			esac
 			;;
-		3,#*|3,)
+		3,"$comment_char"*|3,)
 			# copy comments
 			;;
 		*)

-- 
1.8.4.rc3.500.gc3113b0

Eric Sunshine· Aug 18, 2013, 17:12 UTC · re: Eric Sunshine · lore

[PATCH v2 2/2] rebase -i: fix core.commentchar regression

9a46c25bdbf79744 (rebase: interactive: fix short SHA-1 collision, 2013-08-12) added transform_todo_ids() which assumes incorrectly that the comment character is always '#', however, this has not been the case since 180bad3d10fe3a7f (rebase -i: respect core.commentchar, 2013-02-11) patched rebase -i to respect core.commentchar. Fix this regression.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---

Intended for 'next'.

 git-rebase--interactive.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Show changes to git-rebase--interactive.sh +1 −1

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 3733312..f246810 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -693,7 +693,7 @@ transform_todo_ids () {
 	while read -r command rest
 	do
 		case "$command" in
-		'#'* | exec)
+		"$comment_char"* | exec)
 			# Be careful for oddball commands like 'exec'
 			# that do not have a SHA-1 at the beginning of $rest.
 			;;

-- 
1.8.4.rc3.500.gc3113b0

Junio C Hamano· Aug 18, 2013, 21:14 UTC · re: Eric Sunshine · lore

Re: [PATCH v2 2/2] rebase -i: fix core.commentchar regression

Eric Sunshine <sunshine@sunshineco.com> writes:

> Intended for 'next'.

Thanks.

Will queue on top of es/rebase-i-no-abbrev, but we have a chance to rebuild 'next' after 1.8.4 release, so we may want to squash this into the problematic commit when it happens.

Show 17 quoted lines

>
>  git-rebase--interactive.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 3733312..f246810 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -693,7 +693,7 @@ transform_todo_ids () {
>  	while read -r command rest
>  	do
>  		case "$command" in
> -		'#'* | exec)
> +		"$comment_char"* | exec)
>  			# Be careful for oddball commands like 'exec'
>  			# that do not have a SHA-1 at the beginning of $rest.
>  			;;

Eric Sunshine· Aug 19, 2013, 00:29 UTC · re: Junio C Hamano · lore

Re: [PATCH v2 2/2] rebase -i: fix core.commentchar regression

On Sun, Aug 18, 2013 at 5:14 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 9 quoted lines

> Eric Sunshine <sunshine@sunshineco.com> writes:
>
>> Intended for 'next'.
>
> Thanks.
>
> Will queue on top of es/rebase-i-no-abbrev, but we have a chance to
> rebuild 'next' after 1.8.4 release, so we may want to squash this
> into the problematic commit when it happens.

That would be fine. Time permitting, I also would like to send out a couple incremental patches atop 'next' which address your comments [1] about es/rebase-i-no-abbrev. Those new patches should be suitable for squashing if you want to go that route.

[1]: http://thread.gmane.org/gmane.comp.version-control.git/232146

Show 17 quoted lines

>>
>>  git-rebase--interactive.sh | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
>> index 3733312..f246810 100644
>> --- a/git-rebase--interactive.sh
>> +++ b/git-rebase--interactive.sh
>> @@ -693,7 +693,7 @@ transform_todo_ids () {
>>       while read -r command rest
>>       do
>>               case "$command" in
>> -             '#'* | exec)
>> +             "$comment_char"* | exec)
>>                       # Be careful for oddball commands like 'exec'
>>                       # that do not have a SHA-1 at the beginning of $rest.
>>                       ;;