On Thu, Jan 30, 2020 at 01:53:52PM -0800, Mike McGranahan wrote:
> I'm using version 2.25.0.windows.1. If I set config "wsErrorHighlight"
> to "old", and run `git diff -p`, the resulting patch text highlights
> whitespace differences in the old text.
>
> If I then run git-add in interactive patch mode, I expect the diff to
> include the whitespace highlights. But actually, it does not.
>
> Is this a bug? Thanks for your help.
Sort of. It's more of a feature that has not yet been implemented. ;)
Like the rest of the color config, diff.wsErrorHighlight is not enabled
for scriptable plumbing commands like git-diff-index, etc; it is only
used for the user-facing porcelain commands like git-diff.
So scripts like git-add--interactive, which use the plumbing under the
hood, are responsible for deciding which config options should be
respected and passing the correct command-line options on to the
plumbing. We do that for color.diff, diff.algorithm, and a few others.
But nobody has yet taught it that diff.wsErrorHighlight is safe to pass
(for the user-facing "color" version of the diff).
The solution for the existing perl script would be something like this:
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index c4b75bcb7f..fac1d1e63e 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -46,6 +46,7 @@
my $normal_color = $repo->get_color("", "reset");
my $diff_algorithm = $repo->config('diff.algorithm');
+my $diff_error_highlight = $repo->config('diff.wsErrorHighlight');
my $diff_filter = $repo->config('interactive.difffilter');
my $use_readkey = 0;
@@ -727,7 +728,11 @@ sub parse_diff {
my @diff = run_cmd_pipe("git", @diff_cmd, "--", $path);
my @colored = ();
if ($diff_use_color) {
- my @display_cmd = ("git", @diff_cmd, qw(--color --), $path);
+ my @display_cmd = ("git", @diff_cmd, qw(--color),
+ $diff_error_highlight ?
+ "--ws-error-highlight=$diff_error_highlight" :
+ (),
+ qw(--), $path);
if (defined $diff_filter) {
# quotemeta is overkill, but sufficient for shell-quoting
my $diff = join(' ', map { quotemeta } @display_cmd);
but this is all being rewritten in C. I'm not sure how the multiple
diffs are being handled there.
All that said, I kind of wonder if diff.wsErrorHighlight ought to be
respected by even plumbing commands. After all, it does nothing unless
color is enabled anyway, so I don't think it runs the risk of confusing
a user of the plumbing. It's no different than color.diff.*, which we
respect even in the plumbing commands (if the caller has manually asked
for color, of course).
-Peff
