Re: js/add-p-diff-parsing-fix (was: What's cooking in git.git (Aug 2022, #10; Tue, 30))

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Aug 31, 2022, 09:31 UTC
Message-ID: <b0a368a1-a6cc-1940-4804-3cbcae1db235@gmail.com>
In-Reply-To: <220831.86bks0ajy7.gmgdl@evledraar.gmail.com>

On 31/08/2022 09:29, Ævar Arnfjörð Bjarmason wrote:

Show 17 quoted lines

> 
> On Tue, Aug 30 2022, Junio C Hamano wrote:
> 
>> * js/add-p-diff-parsing-fix (2022-08-29) 5 commits
>>   - add -p: ignore dirty submodules
>>   - add -p: handle `diff-so-fancy`'s hunk headers better
>>   - add -p: insert space in colored hunk header as needed
>>   - add -p: gracefully ignore unparseable hunk headers in colored diffs
>>   - t3701: redefine what is "bogus" output of a diff filter
>>
>>   Those who use diff-so-fancy as the diff-filter noticed a regression
>>   or two in the reimplemented "add -p", which has been corrected.
>>
>>   Will merge to 'next'?
>>   source: <pull.1336.v3.git.1661785916.gitgitgadget@gmail.com>
> 
> This LGTM.

I'm not sure that the different output from the builtin version compared to the perl version when using interactive.diffFilter is an improvement. The perl version leaves the filtered output alone but the builtin version has started prepending @@ -a,b +d,c @@ to the "hunk header" lines produced by the filter.

Best Wishes

Phillip

Show 21 quoted lines

> For the release notes you might want to tweak this. While this series
> fixes an issue that some users ran into with "diff-so-fancy" it's really
> just a generic set of fixes for "git add -p". The issue just happens to
> have been discovered with that tool.
> 
> E.g. on master now you can try this in git.git:
> 
> 	echo hi >README.mda
> 	./git -c interactive.diffFilter=tac add -p
> 	error: could not parse colored hunk header '?[31m-[Documentation/gitcvs-migration.txt]: Documentation/gitcvs-migration.txt?[m'
> 
> I.e. just reversing the output with tac(1) is enough to trigger the
> failure fixed in this series.
> 
> Maybe?:
> 
> 	'git-add -p' can now parse diffs that don't contain the line
> 	information that "git diff" itself would emit within "@@
> 	... @@". As a result the reimplemented "add -p" can now be used
> 	with the popular "diff-so-fancy" utility.
>