git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Philippe Blain <levraiphilippeblain@gmail.com>
Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org
Subject: Re: [PATCH 0/3] built-in add -p: support diff-so-fancy better
Date: Wed, 24 Aug 2022 11:24:02 -0700	[thread overview]
Message-ID: <xmqqmtbt7b5p.fsf@gitster.g> (raw)
In-Reply-To: <145ddce5-8084-a024-39b5-9dea870d1afe@gmail.com> (Philippe Blain's message of "Wed, 24 Aug 2022 13:49:30 -0400")

Philippe Blain <levraiphilippeblain@gmail.com> writes:

>>>> However, I've tried it on a more "real-life" example, and then I get:
>>>>
>>>>     error: mismatched output from interactive.diffFilter
>>>>     hint: Your filter must maintain a one-to-one correspondence
>>>>     hint: between its input and output lines.
>>>>
>>>> This is despite using "diff-so-fancy --patch" as interactive.diffFilter, which should
>>>> keep the number of lines the same.
>>>
>>> Would you mind sharing the example with me?
>>>
>>> Thanks,
>>> Dscho
>> 
>> In trying to write a reproducer, I realize that this is in fact a separate "bug".
>> I happened to have a submodule in the repository I was testing, and it had modified 
>> content. This is what was causing the "mismatched output" error, although I'm not
>> sure why. If I use a pathspec that does not include the submodule when invoking 
>> 'git add -p', it works correctly. 
>> 
>> I'm a bit out of time now but I'll try to write a separate
>> reproducer for this later today.
>
> So in trying to write a new reproducer, I found the cause of the
> bug. ...

I somehow had an impression that that the C reimplementation was
designed to be a bug-for-bug compatible faithful rewrite of the
original scripted version, but looking at the way how this bug is
handled, I am not sure (it looks as if the initial fix was to patch
the code to match the symptom, not to make the code to behave more
faithfully to the scripted version).  As with any big rewrite, a
complete re-implementation always has risks to introduce unintended
regressions and that is why starting with faithful rewrite with the
staged transition plan is valuable.

In this case, the staged transition plan, the step to flip the
default without before remove the old one, is working beautifully.
And it was only made possible with the actual users reporting issues
before they say "the new one is broken, so let's use the knob to
retain the old implementation".  Please thank those in diff-so-fancy
land for reporting the issue.

Thanks, you two, for working on this.

  reply	other threads:[~2022-08-24 18:24 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-23 18:04 [PATCH 0/3] built-in add -p: support diff-so-fancy better Johannes Schindelin via GitGitGadget
2022-08-23 18:04 ` [PATCH 1/3] t3701: redefine what is "bogus" output of a diff filter Johannes Schindelin via GitGitGadget
2022-08-23 18:04 ` [PATCH 2/3] add -p: gracefully ignore unparseable hunk headers in colored diffs Johannes Schindelin via GitGitGadget
2022-08-23 18:04 ` [PATCH 3/3] add -p: handle `diff-so-fancy`'s hunk headers better Johannes Schindelin via GitGitGadget
2022-08-24  3:49 ` [PATCH 0/3] built-in add -p: support diff-so-fancy better Philippe Blain
2022-08-24  6:27   ` Johannes Schindelin
2022-08-24 13:21     ` Philippe Blain
2022-08-24 17:49       ` Philippe Blain
2022-08-24 18:24         ` Junio C Hamano [this message]
2022-08-24 21:05           ` Johannes Schindelin
2022-08-24 21:37             ` Junio C Hamano
2022-08-24 21:21 ` [PATCH v2 0/4] " Johannes Schindelin via GitGitGadget
2022-08-24 21:21   ` [PATCH v2 1/4] t3701: redefine what is "bogus" output of a diff filter Johannes Schindelin via GitGitGadget
2022-08-24 21:21   ` [PATCH v2 2/4] add -p: gracefully ignore unparseable hunk headers in colored diffs Johannes Schindelin via GitGitGadget
2022-08-29  7:56     ` Junio C Hamano
2022-08-24 21:21   ` [PATCH v2 3/4] add -p: handle `diff-so-fancy`'s hunk headers better Johannes Schindelin via GitGitGadget
2022-08-29  8:06     ` Junio C Hamano
2022-08-29 13:32       ` Johannes Schindelin
2022-08-29 17:19         ` Junio C Hamano
2022-08-30 14:14           ` Johannes Schindelin
2022-08-24 21:21   ` [PATCH v2 4/4] add -p: ignore dirty submodules Johannes Schindelin via GitGitGadget
2022-08-24 22:11   ` [PATCH v2 0/4] built-in add -p: support diff-so-fancy better Junio C Hamano
2022-08-25  0:18   ` Philippe Blain
2022-08-26 11:43     ` Johannes Schindelin
2022-08-26 23:15       ` Philippe Blain
2022-08-29 15:11   ` [PATCH v3 0/5] " Johannes Schindelin via GitGitGadget
2022-08-29 15:11     ` [PATCH v3 1/5] t3701: redefine what is "bogus" output of a diff filter Johannes Schindelin via GitGitGadget
2022-08-30 13:17       ` Phillip Wood
2022-08-30 21:36         ` Junio C Hamano
2022-08-31  9:26           ` Phillip Wood
2022-08-31 15:36             ` Jeff King
2022-08-31 15:47               ` Jeff King
2022-08-31 19:57                 ` Johannes Schindelin
2022-08-29 15:11     ` [PATCH v3 2/5] add -p: gracefully ignore unparseable hunk headers in colored diffs Johannes Schindelin via GitGitGadget
2022-08-29 15:11     ` [PATCH v3 3/5] add -p: insert space in colored hunk header as needed Johannes Schindelin via GitGitGadget
2022-08-29 15:11     ` [PATCH v3 4/5] add -p: handle `diff-so-fancy`'s hunk headers better Johannes Schindelin via GitGitGadget
2022-08-30 13:23       ` Phillip Wood
2022-08-29 15:11     ` [PATCH v3 5/5] add -p: ignore dirty submodules Johannes Schindelin via GitGitGadget
2022-08-30 13:26       ` Phillip Wood
2022-08-31 20:05         ` Johannes Schindelin
2022-08-31 20:19           ` Junio C Hamano
2022-08-31 20:38             ` Johannes Schindelin
2022-08-29 18:01     ` [PATCH v3 0/5] built-in add -p: support diff-so-fancy better Junio C Hamano
2022-08-30 14:22       ` Johannes Schindelin
2022-08-30 13:29     ` Phillip Wood
2022-08-31 20:44       ` Johannes Schindelin
2022-08-31 20:31     ` [PATCH v4 0/3] " Johannes Schindelin via GitGitGadget
2022-08-31 20:31       ` [PATCH v4 1/3] add -p: detect more mismatches between plain vs colored diffs Johannes Schindelin via GitGitGadget
2022-09-01 13:19         ` Phillip Wood
2022-08-31 20:31       ` [PATCH v4 2/3] add -p: gracefully handle unparseable hunk headers in " Johannes Schindelin via GitGitGadget
2022-09-01 13:53         ` Phillip Wood
2022-09-01 15:09           ` Johannes Schindelin
2022-08-31 20:31       ` [PATCH v4 3/3] add -p: ignore dirty submodules Johannes Schindelin via GitGitGadget
2022-09-01 15:45         ` Jeff King
2022-09-01 15:49           ` Jeff King
2022-09-01 16:17         ` Junio C Hamano
2022-09-02  8:53           ` Johannes Schindelin
2022-09-01 13:55       ` [PATCH v4 0/3] built-in add -p: support diff-so-fancy better Phillip Wood
2022-09-01 16:19         ` Junio C Hamano
2022-09-01 15:42       ` [PATCH v5 " Johannes Schindelin via GitGitGadget
2022-09-01 15:42         ` [PATCH v5 1/3] add -p: detect more mismatches between plain vs colored diffs Johannes Schindelin via GitGitGadget
2022-09-01 15:42         ` [PATCH v5 2/3] add -p: gracefully handle unparseable hunk headers in " Johannes Schindelin via GitGitGadget
2022-09-01 16:03           ` Phillip Wood
2022-09-01 15:42         ` [PATCH v5 3/3] add -p: ignore dirty submodules Johannes Schindelin via GitGitGadget
2022-09-01 16:55           ` Junio C Hamano
2022-09-01 16:04         ` [PATCH v5 0/3] built-in add -p: support diff-so-fancy better Phillip Wood
2022-09-01 16:54           ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqmtbt7b5p.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=levraiphilippeblain@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).