{"thread":{"id":"34715","subject":"[PATCH] rebase -i: fix cases ignoring core.commentchar","startedAt":"2013-08-16T21:44:07Z","lastAt":"2013-08-18T20:36:08Z","messageCount":5,"participants":["Eric Sunshine","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"225352","messageId":"1376689447-78807-1-git-send-email-sunshine@sunshineco.com","threadId":"34715","inReplyTo":null,"subject":"[PATCH] rebase -i: fix cases ignoring core.commentchar","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-08-16T21:44:07Z","receivedAt":"2013-08-16T21:44:07Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"eff80a9fd990de36 (Allow custom \"comment char\", 2013-01-16) added general\ncore.commentchar support but forgot to update git-rebase--interactive to\nrespect it. 180bad3d10fe3a7f (rebase -i: respect core.commentchar,\n2013-02-11) addressed this oversight but missed one instance of\nhard-coded '#' comment character in skip_unnecessary_picks(). Fix this.\n\n9a46c25bdbf79744 (rebase: interactive: fix short SHA-1 collision,\n2013-08-12) added another instance of hard-coded '#' comment character\nin transform_todo_ids(). Fix this, as well.\n\nSigned-off-by: Eric Sunshine \n---\n\nWhile planning to work on a couple patches to address Junio's comments\non [1] (currently in 'next'), I realized that that series introduced a\nsmall regression by ignoring core.commentchar and assuming that the todo\nlist comment character is unconditionally '#'. This patch fixes that\nregression.\n\nWhile fixing the regression, I also noticed that the original patch\n(180bad3d10fe3a7f) which added core.commentchar support to rebase -i\nmissed one instance. This patch fixes that, as well.\n\n[1]: http://thread.gmane.org/gmane.comp.version-control.git/232146\n\n git-rebase--interactive.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex ea11e62..f246810 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -671,7 +671,7 @@ skip_unnecessary_picks () {\n \t\t\t\t;;\n \t\t\tesac\n \t\t\t;;\n-\t\t3,#*|3,)\n+\t\t3,\"$comment_char\"*|3,)\n \t\t\t# copy comments\n \t\t\t;;\n \t\t*)\n@@ -693,7 +693,7 @@ transform_todo_ids () {\n \twhile read -r command rest\n \tdo\n \t\tcase \"$command\" in\n-\t\t'#'* | exec)\n+\t\t\"$comment_char\"* | exec)\n \t\t\t# Be careful for oddball commands like 'exec'\n \t\t\t# that do not have a SHA-1 at the beginning of $rest.\n \t\t\t;;\n-- \n1.8.4.rc3.500.gc3113b0\n"},{"id":"225355","messageId":"CAPig+cTRc1-W7vJX52gb5S0ge4kZgKMBkHJjqWFRDgpuzTfM2g@mail.gmail.com","threadId":"34715","inReplyTo":"1376689447-78807-1-git-send-email-sunshine@sunshineco.com","subject":"Re: [PATCH] rebase -i: fix cases ignoring core.commentchar","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-08-16T21:55:33Z","receivedAt":"2013-08-16T21:55:33Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Aug 16, 2013 at 5:44 PM, Eric Sunshine wrote:\n> eff80a9fd990de36 (Allow custom \"comment char\", 2013-01-16) added general\n> core.commentchar support but forgot to update git-rebase--interactive to\n> respect it. 180bad3d10fe3a7f (rebase -i: respect core.commentchar,\n> 2013-02-11) addressed this oversight but missed one instance of\n> hard-coded '#' comment character in skip_unnecessary_picks(). Fix this.\n>\n> 9a46c25bdbf79744 (rebase: interactive: fix short SHA-1 collision,\n> 2013-08-12) added another instance of hard-coded '#' comment character\n> in transform_todo_ids(). Fix this, as well.\n>\n> Signed-off-by: Eric Sunshine \n\nI forgot to mention that I wanted to add tests to t3404 for these bugs\nbut couldn't figure out how to do it using the external behavior of\nrebase -i. I was able to verify the before and after behavior by\nadding temporary echo's to the code in order to observe the \"internal\"\nfunctioning.\n"},{"id":"225353","messageId":"CAPig+cSFoKb+v5fEXnwr=94B6uo2b57vgHTRgtFMj25M+R9dUw@mail.gmail.com","threadId":"34715","inReplyTo":"CAPig+cTRc1-W7vJX52gb5S0ge4kZgKMBkHJjqWFRDgpuzTfM2g@mail.gmail.com","subject":"Re: [PATCH] rebase -i: fix cases ignoring core.commentchar","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-08-16T22:19:58Z","receivedAt":"2013-08-16T22:19:58Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Aug 16, 2013 at 5:55 PM, Eric Sunshine wrote:\n> On Fri, Aug 16, 2013 at 5:44 PM, Eric Sunshine wrote:\n>> eff80a9fd990de36 (Allow custom \"comment char\", 2013-01-16) added general\n>> core.commentchar support but forgot to update git-rebase--interactive to\n>> respect it. 180bad3d10fe3a7f (rebase -i: respect core.commentchar,\n>> 2013-02-11) addressed this oversight but missed one instance of\n>> hard-coded '#' comment character in skip_unnecessary_picks(). Fix this.\n>>\n>> 9a46c25bdbf79744 (rebase: interactive: fix short SHA-1 collision,\n>> 2013-08-12) added another instance of hard-coded '#' comment character\n>> in transform_todo_ids(). Fix this, as well.\n>>\n>> Signed-off-by: Eric Sunshine \n>\n> I forgot to mention that I wanted to add tests to t3404 for these bugs\n> but couldn't figure out how to do it using the external behavior of\n> rebase -i. I was able to verify the before and after behavior by\n> adding temporary echo's to the code in order to observe the \"internal\"\n> functioning.\n\nOne of the fixes in this patch addresses an oversight in\n180bad3d10fe3a7f (rebase -i: respect core.commentchar, 2013-02-11)\nwhich is already in 'maint'. Should I split this patch in two so that\nthe one fix can be applied to 'maint'?\n\n(The other fix is for 9a46c25bdbf79744 in 'next'.)\n"},{"id":"225428","messageId":"7v38q6oi9a.fsf@alter.siamese.dyndns.org","threadId":"34715","inReplyTo":"CAPig+cSFoKb+v5fEXnwr=94B6uo2b57vgHTRgtFMj25M+R9dUw@mail.gmail.com","subject":"Re: [PATCH] rebase -i: fix cases ignoring core.commentchar","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-18T20:29:05Z","receivedAt":"2013-08-18T20:29:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> One of the fixes in this patch addresses an oversight in\n> 180bad3d10fe3a7f (rebase -i: respect core.commentchar, 2013-02-11)\n> which is already in 'maint'. Should I split this patch in two so that\n> the one fix can be applied to 'maint'?\n\nWe are so close to 1.8.4 final and I do not think it makes much\ndifference in practice, but if we were shooting for the ideal, yeah,\nas that part of the patch could make core.commentchar useful for\nrebase-i users who are still on 1.8.3.X maintenance track.\n"},{"id":"225429","messageId":"CAPig+cR0k5c1PLxpDYCFLH=9QchhYbeB1kPNk4uRkxv5Jy5cRQ@mail.gmail.com","threadId":"34715","inReplyTo":"7v38q6oi9a.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] rebase -i: fix cases ignoring core.commentchar","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-08-18T20:36:08Z","receivedAt":"2013-08-18T20:36:08Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Aug 18, 2013 at 4:29 PM, Junio C Hamano wrote:\n> Eric Sunshine writes:\n>\n>> One of the fixes in this patch addresses an oversight in\n>> 180bad3d10fe3a7f (rebase -i: respect core.commentchar, 2013-02-11)\n>> which is already in 'maint'. Should I split this patch in two so that\n>> the one fix can be applied to 'maint'?\n>\n> We are so close to 1.8.4 final and I do not think it makes much\n> difference in practice, but if we were shooting for the ideal, yeah,\n> as that part of the patch could make core.commentchar useful for\n> rebase-i users who are still on 1.8.3.X maintenance track.\n\nI sent a v2 [1] of this submission which does split the patch into two\nparts: one for 'maint' and one for 'next', with appropriate commit\nmessages. The actual changes to git-rebase--interactive.sh are\nidentical between v1 and v2.\n\n[1]: http://git.661346.n2.nabble.com/PATCH-v2-0-2-fix-cases-of-rebase-i-ignoring-core-commentchar-td7594009.html\n"}]}