{"thread":{"id":"46223","subject":"git add -p does not work with custom comment char currently","startedAt":"2017-06-21T16:52:58Z","lastAt":"2017-06-21T21:13:11Z","messageCount":6,"participants":["Christian Rösch","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"322819","messageId":"89d4e348-167e-b201-d243-63b1156b9ed0@croesch.de","threadId":"46223","inReplyTo":null,"subject":"git add -p does not work with custom comment char currently","fromName":"Christian Rösch","fromEmail":"christian@croesch.de","sentAt":"2017-06-21T16:31:34Z","receivedAt":"2017-06-21T16:52:58Z","isPatch":false,"sender":{"key":"christian@croesch.de","avatar":null},"body":"Hi,\n\nmy ~/.gitconfig contains\n\n[core]\n    commentchar = $\n\nas far as I can see with 2.13.0 and 2.13.1.516.g05ec6e1 (built from\nsource) if I do\n\n$ git add -p\n\nand edit the hunk manually the comment char is not parsed correctly:\n\nStage this hunk [y,n,q,a,d,/,s,e,?]? e\nwarning: recount: unexpected line: $ Manual hunk edit mode -- see bottom\nfor a quick guide.\n\nerror: corrupt patch at line 6\nYour edited hunk does not apply. Edit again (saying \"no\" discards!) [y/n]? n\n\n\nAs far as I can tell this is a bug but it would be nice if you could let\nme know if it works for you with a custom comment char.\n\n\nThanks in advance,\nChristian\n\n"},{"id":"322835","messageId":"20170621182020.ywopkykzdfhlxi5o@sigill.intra.peff.net","threadId":"46223","inReplyTo":"89d4e348-167e-b201-d243-63b1156b9ed0@croesch.de","subject":"Re: git add -p does not work with custom comment char currently","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-06-21T18:20:21Z","receivedAt":"2017-06-21T18:20:57Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 21, 2017 at 06:31:34PM +0200, Christian Rösch wrote:\n\n> [core]\n>     commentchar = $\n> \n> as far as I can see with 2.13.0 and 2.13.1.516.g05ec6e1 (built from\n> source) if I do\n> \n> $ git add -p\n> \n> and edit the hunk manually the comment char is not parsed correctly:\n> \n> Stage this hunk [y,n,q,a,d,/,s,e,?]? e\n> warning: recount: unexpected line: $ Manual hunk edit mode -- see bottom\n> for a quick guide.\n> \n> error: corrupt patch at line 6\n> Your edited hunk does not apply. Edit again (saying \"no\" discards!) [y/n]? n\n> \n> \n> As far as I can tell this is a bug but it would be nice if you could let\n> me know if it works for you with a custom comment char.\n\nI can reproduce easily here with the script below. It looks like a\nregression in c9d961647 (i18n: add--interactive: mark edit_hunk_manually\nmessage for translation, 2016-12-14), which is in v2.12.0. It taught the\nscript to write out with the comment char, but reading it back does not\nseem to work.\n\nHere's my reproduction script:\n\n-- >8 --\nrm -rf repo\n\ngit init repo\ncd repo\ngit config core.commentchar '$'\n\nseq 10 >file\ngit add file\ngit commit -m base\nperl -pi -e 'print \"new\\n\" if $. == 5' file\n(echo e; echo n) | GIT_EDITOR=true git.compile add -p\n# look for \"corrupt patch\" in the output\n-- 8< --\n\nI think there's another bug, too, where the \"patch did not apply\ncleanly\" prompt goes into an infinite loop if it gets EOF.\n\n-Peff\n"},{"id":"322847","messageId":"20170621192315.3umx4lszvzwjffnb@sigill.intra.peff.net","threadId":"46223","inReplyTo":"20170621182020.ywopkykzdfhlxi5o@sigill.intra.peff.net","subject":"[PATCH 0/2] Re: git add -p does not work with custom comment char currently","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-06-21T19:23:15Z","receivedAt":"2017-06-21T19:23:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 21, 2017 at 02:20:21PM -0400, Jeff King wrote:\n\n> I can reproduce easily here with the script below. It looks like a\n> regression in c9d961647 (i18n: add--interactive: mark edit_hunk_manually\n> message for translation, 2016-12-14), which is in v2.12.0. It taught the\n> script to write out with the comment char, but reading it back does not\n> seem to work.\n>\n> [...]\n>\n> I think there's another bug, too, where the \"patch did not apply\n> cleanly\" prompt goes into an infinite loop if it gets EOF.\n\nHere are fixes for both.\n\n  [1/2]: add--interactive: handle EOF in prompt_yesno\n  [2/2]: add--interactive: quote commentChar regex\n\n git-add--interactive.perl  | 3 ++-\n t/t3701-add-interactive.sh | 8 ++++++++\n 2 files changed, 10 insertions(+), 1 deletion(-)\n\n-Peff\n"},{"id":"322848","messageId":"20170621192636.e5odne2lojuq63gm@sigill.intra.peff.net","threadId":"46223","inReplyTo":"20170621192315.3umx4lszvzwjffnb@sigill.intra.peff.net","subject":"[PATCH 1/2] add--interactive: handle EOF in prompt_yesno","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-06-21T19:26:36Z","receivedAt":"2017-06-21T19:26:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The prompt_yesno function loops indefinitely waiting for a\n\"y\" or \"n\" response. But it doesn't handle EOF, meaning\nthat we can end up in an infinite loop of reading EOF from\nstdin. One way to simulate that is with:\n\n  echo e | GIT_EDITOR='echo corrupt >' git add -p\n\nLet's break out of the loop and propagate the undef to the\ncaller. Without modifying the callers that effectively turns\nit into a \"no\" response. This is reasonable for both of the\ncurrent callers, and it leaves room for any future caller to\ncheck for undef explicitly.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n git-add--interactive.perl | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 79d675b5b..fc722fe8a 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -1136,6 +1136,7 @@ sub prompt_yesno {\n \twhile (1) {\n \t\tprint colored $prompt_color, $prompt;\n \t\tmy $line = prompt_single_character;\n+\t\treturn undef unless defined $line;\n \t\treturn 0 if $line =~ /^n/i;\n \t\treturn 1 if $line =~ /^y/i;\n \t}\n-- \n2.13.1.792.g159074dab\n\n"},{"id":"322849","messageId":"20170621192859.udejhkl66ylsbg24@sigill.intra.peff.net","threadId":"46223","inReplyTo":"20170621192315.3umx4lszvzwjffnb@sigill.intra.peff.net","subject":"[PATCH 2/2] add--interactive: quote commentChar regex","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-06-21T19:28:59Z","receivedAt":"2017-06-21T19:29:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Since c9d961647 (i18n: add--interactive: mark\nedit_hunk_manually message for translation, 2016-12-14),\nwhen the user asks to edit a hunk manually, we respect\ncore.commentChar in generating the edit instructions.\nHowever, when we then strip out comment lines, we use a\nsimple regex like:\n\n  /^$commentChar/\n\nIf your chosen comment character is a regex metacharacter,\nthen that will behave in a confusing manner (\"$\", for\ninstance, would only eliminate blank lines, not actual\ncomment lines).\n\nWe can fix that by telling perl not to respect\nmetacharacters.\n\nReported-by: Christian Rösch <christian@croesch.de>\nSigned-off-by: Jeff King <peff@peff.net>\n---\nI did this on the current master. It also applies cleanly on top of\n'maint'. The fix applies back as far as c9d961647, but t3701 grew some\nnew tests in the interim, so there's a minor conflict applying it there.\n\n git-add--interactive.perl  | 2 +-\n t/t3701-add-interactive.sh | 8 ++++++++\n 2 files changed, 9 insertions(+), 1 deletion(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex fc722fe8a..0e8543c86 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -1081,7 +1081,7 @@ EOF2\n \n \topen $fh, '<', $hunkfile\n \t\tor die sprintf(__(\"failed to open hunk edit file for reading: %s\"), $!);\n-\tmy @newtext = grep { !/^$comment_line_char/ } <$fh>;\n+\tmy @newtext = grep { !/^\\Q$comment_line_char\\E/ } <$fh>;\n \tclose $fh;\n \tunlink $hunkfile;\n \ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 2ecb43a61..2f3e7cea6 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -477,4 +477,12 @@ test_expect_success 'add -p does not expand argument lists' '\n \t! grep not-changed trace.out\n '\n \n+test_expect_success 'hunk-editing handles custom comment char' '\n+\tgit reset --hard &&\n+\techo change >>file &&\n+\ttest_config core.commentChar \"\\$\" &&\n+\techo e | GIT_EDITOR=true git add -p &&\n+\tgit diff --exit-code\n+'\n+\n test_done\n-- \n2.13.1.792.g159074dab\n"},{"id":"322865","messageId":"0bd23975-2283-fb98-c685-9adc0a9f0216@croesch.de","threadId":"46223","inReplyTo":"20170621192859.udejhkl66ylsbg24@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] add--interactive: quote commentChar regex","fromName":"Christian Rösch","fromEmail":"christian@croesch.de","sentAt":"2017-06-21T21:12:56Z","receivedAt":"2017-06-21T21:13:11Z","isPatch":true,"sender":{"key":"christian@croesch.de","avatar":null},"body":"Thanks for the quick reaction and fix!\n\nOn 21/06/17 21:28, Jeff King wrote:\n> If your chosen comment character is a regex metacharacter,\n> then that will behave in a confusing manner (\"$\", for\n> instance, would only eliminate blank lines, not actual\n> comment lines).\n\nSo things would get worse when using * as comment char?! Maybe I should\nconsider a more uncommon character as comment char ;)\n\nBest wishes,\nChristian\n"}]}