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

From: Jeff King <peff@peff.net>
Date: Feb 3, 2020, 14:51 UTC
Message-ID: <20200203145155.GA272077@coredump.intra.peff.net>
In-Reply-To: <nycvar.QRO.7.76.6.2002031320180.46@tvgsbejvaqbjf.bet>

On Mon, Feb 03, 2020 at 01:37:48PM +0100, Johannes Schindelin wrote:

Show 16 quoted lines

> > 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.

Show 9 quoted lines

> 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.

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

-Peff