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

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

On Wed, Jun 21, 2017 at 06:31:34PM +0200, Christian Rösch 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.

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

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

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

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(+)

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(-)

Thanks for the quick reaction and fix!

On 21/06/17 21:28, Jeff King wrote:

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

Best wishes, Christian