Re: Patch text in git-add patch mode lacks whitespace highlighting

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Peff,

On Mon, 3 Feb 2020, Jeff King wrote:

> On Mon, Feb 03, 2020 at 01:37:48PM +0100, Johannes Schindelin wrote:
>
> > > The short of my answer is that I think the color-moved stuff might be a
> > > candidate, but it's sufficiently different that I think it should be
> > > decided on as a separate patch.
> >
> > I actually wonder whether we should do something completely different. The
> > problem with `git diff-files --color`, after all, is that `diff-files` was
> > never intended to produce user-facing output, so the `--color` is somewhat
> > of a contradiction here. The fact that `diff-files` is a low-level (or
> > "plumbing") command means that by nature, it wants to control a lot more
> > what the user-provided config can change (typically, scripts calling
> > `diff-files` expect a certain format, and it would not do at all to heed
> > `diff.noPrefix`, for example.
> >
> > In that respect, `git add -p` using `diff-files` is kind of wrong: we
> > _want_ to show the result to the user, with no processing at all (execpt
> > the user-provided `diffFilter`).
>
> Sort of. The problem is that we need two matching copies of the diff:
> one to apply, and one to show the user. They don't need to be
> byte-for-byte identical, but they should correlate at the level of
> individual lines. And the "one to apply" can take on some user-selected
> options, as long as the result can still be applied.
>
> > So why not introduce a new option to `diff-files` and `diff-index` to ask
> > it _specifically_ to heed diff UI config settings? I.e. a command-line
> > option that makes it call
> >
> >         git_config(git_diff_ui_config, NULL);
> >
> > instead of
> >
> >         git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
>
> Would you pass that option to both of the diff calls, or just the one
> generating the human-readable input?
>
> If just the human-readable one, then many options that change the line
> count would be problems: diff.context, diff.interhunkcontext,
> diff.orderfile, etc.

Darn, you have a very valid point :-(

> If both, then some options would be problematic for applying. Just
> looking over the list, these jump out at me:
>
>   - color.diff=always would obviously be an issue (though TBH I think
>     anybody doing that is inviting a lot of breakages anyway)
>
>   - diff.external would be a problem if it kicked in, though I think it
>     would require --ext-diff to actually do anything
>
>   - diff.submodule would generate diffs that can't be applied

So I guess it is back to your patch, maybe amended by a move of the
color-moved setting.

Ciao,
Dscho