git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Mike McGranahan <mike@mcgwiz.com>
Date
Feb 1, 2020, 00:48 UTC
Message-ID
<CAK7jxYjLoXZhRzUWERmJg9ADNhvJt5SLLymPVktiWcT4RC6VpA@mail.gmail.com>
In-Reply-To
<20200131091917.GC2857810@coredump.intra.peff.net>

I can confirm this fixes the unexpected behavior I reported. (Thanks!) I can't speak directly to the plumbing as I don't regularly use thos tools.

-Mike
On Fri, Jan 31, 2020 at 1:19 AM Jeff King <peff@peff.net> wrote:
Show 64 quoted lines
>
> 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
Previous: Jeff KingNext: Johannes Schindelin
Message 4 of 14 in “Patch text in git-add patch mode lacks whitespace highlighting”
  1. Mike McGranahanJan 30, 2020
  2. Jeff KingJan 31, 2020
  3. diff: move diff.wsErrorHighlight to "basic" configJeff King, Jan 31, 2020
  4. Mike McGranahanFeb 1, 2020
  5. Johannes SchindelinJan 31, 2020
  6. Mike McGranahanFeb 1, 2020
  7. Jeff KingFeb 1, 2020
  8. Johannes SchindelinFeb 1, 2020
  9. Jeff KingFeb 3, 2020
  10. Johannes SchindelinFeb 3, 2020
  11. Jeff KingFeb 3, 2020
  12. Junio C HamanoFeb 4, 2020
  13. Jeff KingFeb 4, 2020
  14. Johannes SchindelinFeb 4, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.