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

From: Philippe Blain <levraiphilippeblain@gmail.com>
Date: Aug 24, 2022, 17:49 UTC
Message-ID: <145ddce5-8084-a024-39b5-9dea870d1afe@gmail.com>
In-Reply-To: <41781ceb-bfad-2534-2970-c74618c93f2a@gmail.com>

Hi Dscho,

Le 2022-08-24 à 09:21, Philippe Blain a écrit :

Show 28 quoted lines

> Hi Dscho,
> 
> Le 2022-08-24 à 02:27, Johannes Schindelin a écrit :
>> Hi Philippe,
>>
>> On Tue, 23 Aug 2022, Philippe Blain wrote:
>>
>>> 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. A submodule in "modified content" or "untracked content" state (i.e. no new commit) has no "index" line in its diff header. diff-so-fancy, with --patch, always *adds* a blank line before the diff-header because its modified diff header is 3 lines, whereas Git's is 4. So the count is off specifically for submodules in "modified/untracked content" state.

It seems the C version is less lenient than the Perl version, because the Perl version does not complain. But I'd say that it's more of a bug on the diff-so-fancy side, even if it is a regression of the builtin version... Delta does not alter the header (in its default configuration) and so it is not affected here.

For what it's worth, here is the updated reproducer script. It also downloads the 'delta' executable (for Linux) and runs it. With delta, both Perl and C versions work. With diff-so-fancy, the C version complains bout mismatched output.

```bash #!/bin/bash

set -euEo pipefail

repo=repro rm -rf $repo

TEST_AUTHOR_LOCALNAME=author TEST_AUTHOR_DOMAIN=example.com GIT_AUTHOR_EMAIL=${TEST_AUTHOR_LOCALNAME}@${TEST_AUTHOR_DOMAIN} GIT_AUTHOR_NAME='A U Thor' GIT_AUTHOR_DATE='1112354055 +0200' TEST_COMMITTER_LOCALNAME=committer TEST_COMMITTER_DOMAIN=example.com GIT_COMMITTER_EMAIL=${TEST_COMMITTER_LOCALNAME}@${TEST_COMMITTER_DOMAIN} GIT_COMMITTER_NAME='C O Mitter' GIT_COMMITTER_DATE='1112354055 +0200' export GIT_AUTHOR_EMAIL GIT_AUTHOR_NAME export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME export GIT_COMMITTER_DATE GIT_AUTHOR_DATE export HOME=

git -c init.defaultBranch=unimportant init $repo cd $repo date>file git add file git commit -m file git clone https://github.com/so-fancy/diff-so-fancy.git date>>file export PATH="diff-so-fancy:$PATH" git submodule add ./diff-so-fancy git commit -m "add sub" date>diff-so-fancy/README.md curl -sSLO https://github.com/dandavison/delta/releases/download/0.12.1/delta-0.12.1-x86_64-unknown-linux-gnu.tar.gz tar xzf delta-0.12.1-x86_64-unknown-linux-gnu.tar.gz export PATH="delta-0.12.1-x86_64-unknown-linux-gnu:$PATH" echo ----- git -c interactive.diffFilter="delta --color-only" -c add.interactive.useBuiltin=false add -p < <(echo n) echo ----- git -c interactive.diffFilter="delta --color-only" add -p < <(echo n) echo ----- git -c interactive.diffFilter="diff-so-fancy --patch" -c add.interactive.useBuiltin=false add -p < <(echo n) echo ----- git -c interactive.diffFilter="diff-so-fancy --patch" add -p < <(echo n) ```

Cheers,

Philippe.