git add -p does not work with custom comment char currently

git add -p does not work with custom comment char currently

[Christian Rösch]

Hi,

my ~/.gitconfig contains

[core]
    commentchar = $

as far as I can see with 2.13.0 and 2.13.1.516.g05ec6e1 (built from
source) if I do

$ git add -p

and edit the hunk manually the comment char is not parsed correctly:

Stage this hunk [y,n,q,a,d,/,s,e,?]? e
warning: recount: unexpected line: $ Manual hunk edit mode -- see bottom
for a quick guide.

error: corrupt patch at line 6
Your edited hunk does not apply. Edit again (saying "no" discards!) [y/n]? n


As far as I can tell this is a bug but it would be nice if you could let
me know if it works for you with a custom comment char.


Thanks in advance,
Christian

Re: git add -p does not work with custom comment char currently

[Jeff King]

On Wed, Jun 21, 2017 at 06:31:34PM +0200, Christian Rösch wrote:

> [core]
>     commentchar = $
> 
> as far as I can see with 2.13.0 and 2.13.1.516.g05ec6e1 (built from
> source) if I do
> 
> $ git add -p
> 
> and edit the hunk manually the comment char is not parsed correctly:
> 
> Stage this hunk [y,n,q,a,d,/,s,e,?]? e
> warning: recount: unexpected line: $ Manual hunk edit mode -- see bottom
> for a quick guide.
> 
> error: corrupt patch at line 6
> Your edited hunk does not apply. Edit again (saying "no" discards!) [y/n]? n
> 
> 
> As far as I can tell this is a bug but it would be nice if you could let
> me know if it works for you with a custom comment char.

I can reproduce easily here with the script below. It looks like a
regression in c9d961647 (i18n: add--interactive: mark edit_hunk_manually
message for translation, 2016-12-14), which is in v2.12.0. It taught the
script to write out with the comment char, but reading it back does not
seem to work.

Here's my reproduction script:

-- >8 --
rm -rf repo

git init repo
cd repo
git config core.commentchar '$'

seq 10 >file
git add file
git commit -m base
perl -pi -e 'print "new\n" if $. == 5' file
(echo e; echo n) | GIT_EDITOR=true git.compile add -p
# look for "corrupt patch" in the output
-- 8< --

I think there's another bug, too, where the "patch did not apply
cleanly" prompt goes into an infinite loop if it gets EOF.

-Peff

[PATCH 0/2] Re: git add -p does not work with custom comment char currently

[Jeff King]

On Wed, Jun 21, 2017 at 02:20:21PM -0400, Jeff King wrote:

> I can reproduce easily here with the script below. It looks like a
> regression in c9d961647 (i18n: add--interactive: mark edit_hunk_manually
> message for translation, 2016-12-14), which is in v2.12.0. It taught the
> script to write out with the comment char, but reading it back does not
> seem to work.
>
> [...]
>
> I think there's another bug, too, where the "patch did not apply
> cleanly" prompt goes into an infinite loop if it gets EOF.

Here are fixes for both.

  [1/2]: add--interactive: handle EOF in prompt_yesno
  [2/2]: add--interactive: quote commentChar regex

 git-add--interactive.perl  | 3 ++-
 t/t3701-add-interactive.sh | 8 ++++++++
 2 files changed, 10 insertions(+), 1 deletion(-)

-Peff

[PATCH 1/2] add--interactive: handle EOF in prompt_yesno

[Jeff King]

The prompt_yesno function loops indefinitely waiting for a
"y" or "n" response. But it doesn't handle EOF, meaning
that we can end up in an infinite loop of reading EOF from
stdin. One way to simulate that is with:

  echo e | GIT_EDITOR='echo corrupt >' git add -p

Let's break out of the loop and propagate the undef to the
caller. Without modifying the callers that effectively turns
it into a "no" response. This is reasonable for both of the
current callers, and it leaves room for any future caller to
check for undef explicitly.

Signed-off-by: Jeff King <peff@peff.net>
---
 git-add--interactive.perl | 1 +
 1 file changed, 1 insertion(+)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 79d675b5b..fc722fe8a 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -1136,6 +1136,7 @@ sub prompt_yesno {
 	while (1) {
 		print colored $prompt_color, $prompt;
 		my $line = prompt_single_character;
+		return undef unless defined $line;
 		return 0 if $line =~ /^n/i;
 		return 1 if $line =~ /^y/i;
 	}
-- 
2.13.1.792.g159074dab

[PATCH 2/2] add--interactive: quote commentChar regex

[Jeff King]

Since c9d961647 (i18n: add--interactive: mark
edit_hunk_manually message for translation, 2016-12-14),
when the user asks to edit a hunk manually, we respect
core.commentChar in generating the edit instructions.
However, when we then strip out comment lines, we use a
simple regex like:

  /^$commentChar/

If your chosen comment character is a regex metacharacter,
then that will behave in a confusing manner ("$", for
instance, would only eliminate blank lines, not actual
comment lines).

We can fix that by telling perl not to respect
metacharacters.

Reported-by: Christian Rösch <christian@croesch.de>
Signed-off-by: Jeff King <peff@peff.net>
---
I did this on the current master. It also applies cleanly on top of
'maint'. The fix applies back as far as c9d961647, but t3701 grew some
new tests in the interim, so there's a minor conflict applying it there.

 git-add--interactive.perl  | 2 +-
 t/t3701-add-interactive.sh | 8 ++++++++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index fc722fe8a..0e8543c86 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -1081,7 +1081,7 @@ EOF2
 
 	open $fh, '<', $hunkfile
 		or die sprintf(__("failed to open hunk edit file for reading: %s"), $!);
-	my @newtext = grep { !/^$comment_line_char/ } <$fh>;
+	my @newtext = grep { !/^\Q$comment_line_char\E/ } <$fh>;
 	close $fh;
 	unlink $hunkfile;
 
diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index 2ecb43a61..2f3e7cea6 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -477,4 +477,12 @@ test_expect_success 'add -p does not expand argument lists' '
 	! grep not-changed trace.out
 '
 
+test_expect_success 'hunk-editing handles custom comment char' '
+	git reset --hard &&
+	echo change >>file &&
+	test_config core.commentChar "\$" &&
+	echo e | GIT_EDITOR=true git add -p &&
+	git diff --exit-code
+'
+
 test_done
-- 
2.13.1.792.g159074dab

Re: [PATCH 2/2] add--interactive: quote commentChar regex

[Christian Rösch]

Thanks for the quick reaction and fix!

On 21/06/17 21:28, Jeff King wrote:
> If your chosen comment character is a regex metacharacter,
> then that will behave in a confusing manner ("$", for
> instance, would only eliminate blank lines, not actual
> comment lines).

So things would get worse when using * as comment char?! Maybe I should
consider a more uncommon character as comment char ;)

Best wishes,
Christian