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

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 28, 2017, 19:03 UTC
Message-ID: <xmqqk279cjfs.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170328122209.4861-2-jacob.e.keller@intel.com>

Jacob Keller <jacob.e.keller@intel.com> writes:

Show 5 quoted lines

> 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.

Show 5 quoted lines

> 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.