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
