Subject: Re: [PATCH 0/3] built-in add -p: support diff-so-fancy better

From: Junio C Hamano <[email-masked]>
Date: Wed, 24 Aug 2022 11:24:02 -0700

Philippe Blain <[email-masked]> 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.