Re: [PATCH] git-gui: support core.whitespace rules in diff view

From: Tor Arvid Lund <torarvid@gmail.com>
Date: Oct 21, 2010, 12:36 UTC
Message-ID: <AANLkTikG0_9+Gjnhehf8436Kw089Zd-XaSdSOP-J6_yD@mail.gmail.com>
In-Reply-To: <7vy69stop2.fsf@alter.siamese.dyndns.org>

On Thu, Oct 21, 2010 at 1:43 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 16 quoted lines

> Pat Thoyts <patthoyts@users.sourceforge.net> writes:
>
>> This is a rather more complete implementation of whitespace highlighting
>> according to the core.whitespace setting. The diff view whitespace
>> highlights should match what you see with 'git diff' when color is
>> enabled for all the whitespace rules except cr-at-eol where there is
>> currently a rule to hide these.
>>
>> Suggested-by: Tor Arvid Lund <torarvid@gmail.com>
>> Signed-off-by: Pat Thoyts <patthoyts@users.sourceforge.net>
>> ---
>
> This might be a very stupid question, but isn't it an easier-to-maintain
> option to let underlying "git diff" color its output and convert the ANSI
> coloring to whatever Tcl wants to use, especially in the long run, instead
> of trying to replicate the logic to check whitespace breakages here?

Hi Pat, and thanks again for doing the Tcl hacking :)

I did a quick test (with core.whitespace unset, so the defaults should be used). I wonder if there is a little bug somewhere else in git-gui. If I have committed a line like this:

............Hello world

and change it to:

........*Hello world

I used '.' to indicate Space, and '*' to indicate Tab, so I changed 4 spaces to one tab character. As I understand space-before-tab, this should render 8 red spaces, 1 white Tab, and then "Hello world". But instead I get 9 white spaces... So the tab gets converted to a space (I verified that it was not my editor that converted it - it really is 8 SP and 1 TAB).

If I change it to ***Hello world so that there are no leading spaces, then the Tabs do not get converted.

Can you reproduce this? I have applied your patch on top of Junio's master and build on msysgit on Vista 64 (if that matters).

Btw, I almost feel bad saying this after you implemented my suggestion, but Junio's suggestion to parse the ANSI escapes seems quite clever, doesn't it?

Have a good day.

-Tor Arvid-