# [PATCH] rebase -i: fix cases ignoring core.commentchar 5 messages from 2013-08-16 to 2013-08-18. Participants: Eric Sunshine, Junio C Hamano. Thread: https://gitlist.dev/t/34715 ## Eric Sunshine, 2013-08-16 21:44 Subject: [PATCH] rebase -i: fix cases ignoring core.commentchar Message-ID: <1376689447-78807-1-git-send-email-sunshine@sunshineco.com> URL: https://gitlist.dev/e/1376689447-78807-1-git-send-email-sunshine%40sunshineco.com ``` 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. 9a46c25bdbf79744 (rebase: interactive: fix short SHA-1 collision, 2013-08-12) added another instance of hard-coded '#' comment character in transform_todo_ids(). Fix this, as well. Signed-off-by: Eric Sunshine --- While planning to work on a couple patches to address Junio's comments on [1] (currently in 'next'), I realized that that series introduced a small regression by ignoring core.commentchar and assuming that the todo list comment character is unconditionally '#'. This patch fixes that regression. While fixing the regression, I also noticed that the original patch (180bad3d10fe3a7f) which added core.commentchar support to rebase -i missed one instance. This patch fixes that, as well. [1]: http://thread.gmane.org/gmane.comp.version-control.git/232146 git-rebase--interactive.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index ea11e62..f246810 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 ;; *) @@ -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 ``` ## Eric Sunshine, 2013-08-16 21:55 Subject: Re: [PATCH] rebase -i: fix cases ignoring core.commentchar Message-ID: URL: https://gitlist.dev/e/CAPig%2BcTRc1-W7vJX52gb5S0ge4kZgKMBkHJjqWFRDgpuzTfM2g%40mail.gmail.com In-Reply-To: <1376689447-78807-1-git-send-email-sunshine@sunshineco.com> ``` On Fri, Aug 16, 2013 at 5:44 PM, Eric Sunshine wrote: > 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. > > 9a46c25bdbf79744 (rebase: interactive: fix short SHA-1 collision, > 2013-08-12) added another instance of hard-coded '#' comment character > in transform_todo_ids(). Fix this, as well. > > Signed-off-by: Eric Sunshine I forgot to mention that I wanted to add tests to t3404 for these bugs but couldn't figure out how to do it using the external behavior of rebase -i. I was able to verify the before and after behavior by adding temporary echo's to the code in order to observe the "internal" functioning. ``` ## Eric Sunshine, 2013-08-16 22:19 Subject: Re: [PATCH] rebase -i: fix cases ignoring core.commentchar Message-ID: URL: https://gitlist.dev/e/CAPig%2BcSFoKb%2Bv5fEXnwr%3D94B6uo2b57vgHTRgtFMj25M%2BR9dUw%40mail.gmail.com In-Reply-To: ``` On Fri, Aug 16, 2013 at 5:55 PM, Eric Sunshine wrote: > On Fri, Aug 16, 2013 at 5:44 PM, Eric Sunshine wrote: >> 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. >> >> 9a46c25bdbf79744 (rebase: interactive: fix short SHA-1 collision, >> 2013-08-12) added another instance of hard-coded '#' comment character >> in transform_todo_ids(). Fix this, as well. >> >> Signed-off-by: Eric Sunshine > > I forgot to mention that I wanted to add tests to t3404 for these bugs > but couldn't figure out how to do it using the external behavior of > rebase -i. I was able to verify the before and after behavior by > adding temporary echo's to the code in order to observe the "internal" > functioning. One of the fixes in this patch addresses an oversight in 180bad3d10fe3a7f (rebase -i: respect core.commentchar, 2013-02-11) which is already in 'maint'. Should I split this patch in two so that the one fix can be applied to 'maint'? (The other fix is for 9a46c25bdbf79744 in 'next'.) ``` ## Junio C Hamano, 2013-08-18 20:29 Subject: Re: [PATCH] rebase -i: fix cases ignoring core.commentchar Message-ID: <7v38q6oi9a.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7v38q6oi9a.fsf%40alter.siamese.dyndns.org In-Reply-To: ``` Eric Sunshine writes: > One of the fixes in this patch addresses an oversight in > 180bad3d10fe3a7f (rebase -i: respect core.commentchar, 2013-02-11) > which is already in 'maint'. Should I split this patch in two so that > the one fix can be applied to 'maint'? We are so close to 1.8.4 final and I do not think it makes much difference in practice, but if we were shooting for the ideal, yeah, as that part of the patch could make core.commentchar useful for rebase-i users who are still on 1.8.3.X maintenance track. ``` ## Eric Sunshine, 2013-08-18 20:36 Subject: Re: [PATCH] rebase -i: fix cases ignoring core.commentchar Message-ID: URL: https://gitlist.dev/e/CAPig%2BcR0k5c1PLxpDYCFLH%3D9QchhYbeB1kPNk4uRkxv5Jy5cRQ%40mail.gmail.com In-Reply-To: <7v38q6oi9a.fsf@alter.siamese.dyndns.org> ``` On Sun, Aug 18, 2013 at 4:29 PM, Junio C Hamano wrote: > Eric Sunshine writes: > >> One of the fixes in this patch addresses an oversight in >> 180bad3d10fe3a7f (rebase -i: respect core.commentchar, 2013-02-11) >> which is already in 'maint'. Should I split this patch in two so that >> the one fix can be applied to 'maint'? > > We are so close to 1.8.4 final and I do not think it makes much > difference in practice, but if we were shooting for the ideal, yeah, > as that part of the patch could make core.commentchar useful for > rebase-i users who are still on 1.8.3.X maintenance track. I sent a v2 [1] of this submission which does split the patch into two parts: one for 'maint' and one for 'next', with appropriate commit messages. The actual changes to git-rebase--interactive.sh are identical between v1 and v2. [1]: http://git.661346.n2.nabble.com/PATCH-v2-0-2-fix-cases-of-rebase-i-ignoring-core-commentchar-td7594009.html ```