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

[Philippe Blain <levraiphilippeblain@gmail.com>]

Hi Dscho,

Le 2022-08-24 à 09:21, Philippe Blain a écrit :
> 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.