Re: [PATCH RFC 2/2] diff: teach diff to expand tabs in output

From: Jeff King <peff@peff.net>
Date: Mar 28, 2017, 20:38 UTC
Message-ID: <20170328203814.36aiocj53wtfcjz5@sigill.intra.peff.net>
In-Reply-To: <CA+P7+xqA9ZOfaeVg0wjgXVs=N=OJUZjjY1pahUn2xbvSQohfUQ@mail.gmail.com>

On Tue, Mar 28, 2017 at 01:05:40PM -0700, Jacob Keller wrote:

Show 30 quoted lines

> On Tue, Mar 28, 2017 at 12:03 PM, Junio C Hamano <gitster@pobox.com> wrote:
> > Jacob Keller <jacob.e.keller@intel.com> writes:
> >
> >> I'm really not a fan of how the ws code ended up. It seems pretty ugly
> >> and weird to hack in the expand_tabs stuff here. However, I'm really not
> >> sure how else I could handle this. Additionally, I'm not 100% sure
> >> whether this interacts with format-patch or other machinery which may
> >> well want some way to be excluded. Thoughts?
> >
> > As long as you do the same as "do we color the output?  no, no, we
> > are format-patch and must not color" logic to refrain from expanding
> > the tabs, you should be OK.
> >
> >> I think there also may be some wonky bits when performing the tab
> >> expansion during whitespace checks, due to the way we expand, because I
> >> don't think that the tabexpand function takes into account the "current"
> >> location when adding a string, so it very well may not be correct.... I
> >> am unsure if there is a good way to fix this.
> >
> > This "feature" is limited to the diff output, so one way may be to
> > leave the code as-is and pipe the output to a filter that is similar
> > to /usr/bin/expand but knows that the first column is special (this
> > is the part that "this is limited to diff" kicks in).  You may even
> > be able to implement it as a new option to "expand(1)" and then
> > people who aren't Git users would also benefit.
> >
> 
> That makes more sense. I'll take a look at that. It might even be
> possible to modify the pager setup so that it does that as part of its
> process.

This is similar to how I use diff-highlight, which is basically:

  [pager]
  log = diff-highlight | less
  show = diff-highlight | less
  diff = diff-highlight | less

I've wanted to move diff-highlight inside git, but ran into ugly conflicts with the whitespace-marking code as well. Something like:

  git show b16a991c1be5681b4b673d4343dfcc0c2f5ad498 |
  perl -pe 's/^(.)(\t+)/$1 . (" " x (length($2) * 8))/e'

But sticking it in your pager pipeline is tough, because you actually need to skip over the color bytes when they are present. You can see in diff-highlight how this is handled.

That also means an option to something like "expand" is tough, because "first character" really means "first non-ANSI-code character".

-Peff