On Tue, Mar 28, 2017 at 01:05:40PM -0700, Jacob Keller wrote:
> On Tue, Mar 28, 2017 at 12:03 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> > Jacob Keller <jacob.e.keller@xxxxxxxxx> 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