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

On Tue, Aug 30 2022, Junio C Hamano wrote:

quoted

* 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: [ref]

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

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.