threads / discuss / 25475

Colored whitespace in git gui

Subject: Colored whitespace in git gui

## tl;dr

14 messages between Oct 18, 2010 and Oct 22, 2010.

replies: 13people: 4as markdown or json

Tor Arvid Lund· Oct 18, 2010, 23:00 UTC · lore

Hi, all! When doing "git diff", whitespaces before EOL, for instance, are marked with red background in my terminal.

Is it possible to see this coloring in git gui too?
-Tor Arvid-
Pat Thoyts· Oct 19, 2010, 22:59 UTC · re: Tor Arvid Lund · lore

[PATCH] git-gui: highlight trailing whitespace in diff view

Highlight any trailing whitespace in the diff view using a red background as is done in the terminal when color is enabled.

Suggested-by: Tor Arvid Lund <torarvid@gmail.com>
Signed-off-by: Pat Thoyts <patthoyts@users.sourceforge.net>
---
Tor Arvid Lund <torarvid@gmail.com> writes:
Show 6 quoted lines
>Hi, all! When doing "git diff", whitespaces before EOL, for instance,
>are marked with red background in my terminal.
>
>Is it possible to see this coloring in git gui too?
>
>-Tor Arvid-

This patch should do the job. It probably should get some configuration item to control this though.

 git-gui.sh   |    1 +
 lib/diff.tcl |    9 ++++++++-
 2 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/git-gui.sh b/git-gui.sh
index 25229a4..8d652f0 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -3314,6 +3314,7 @@ pack .vpane.lower.diff.header -side top -fill x
 pack .vpane.lower.diff.body -side bottom -fill both -expand 1
 
 $ui_diff tag conf d_cr -elide true
+$ui_diff tag conf ws -background red
 $ui_diff tag conf d_@ -foreground blue -font font_diffbold
 $ui_diff tag conf d_+ -foreground {#00a000}
 $ui_diff tag conf d_- -foreground red
diff --git a/lib/diff.tcl b/lib/diff.tcl
index c628750..83e3f6d 100644
--- a/lib/diff.tcl
+++ b/lib/diff.tcl
@@ -434,7 +434,14 @@ proc read_diff {fd cont_info} {
 			}
 			}
 		}
-		$ui_diff insert end $line $tags
+		if {[regexp -indices {^.*\S(\s+)$} $line -> ndx]} {
+		    set ndx [expr {[lindex $ndx 0] - 1}]
+			set nonws [string range $line 0 $ndx]
+			$ui_diff insert end $nonws $tags \
+				[string range $line [incr ndx] end] [concat $tags ws]
+		} else {
+			$ui_diff insert end $line $tags
+		}
 		if {[string index $line end] eq "\r"} {
 			$ui_diff tag add d_cr {end - 2c}
 		}

-- 
1.7.3.1.msysgit.0
Tor Arvid Lund· Oct 20, 2010, 15:30 UTC · re: Pat Thoyts · lore

Re: [PATCH] git-gui: highlight trailing whitespace in diff view

On Wed, Oct 20, 2010 at 12:59 AM, Pat Thoyts <patthoyts@users.sourceforge.net> wrote:

Show 5 quoted lines
> Highlight any trailing whitespace in the diff view using a red background
> as is done in the terminal when color is enabled.
>
> Suggested-by: Tor Arvid Lund <torarvid@gmail.com>
> Signed-off-by: Pat Thoyts <patthoyts@users.sourceforge.net>

Hey, Pat, and thanks for this. It solves my initial question. But in the time since I asked that question, I started thinking that it probably should honor the core.whitespace settings, and also work for, say, 'space-before-tab'. (You seem to say the same thing in your commit msg, I see)

I tried hacking it for a bit, but my Tcl knowledge is Absolute Zero, so it got frustrating pretty fast :)

Show 46 quoted lines
> ---
>
> Tor Arvid Lund <torarvid@gmail.com> writes:
>>Hi, all! When doing "git diff", whitespaces before EOL, for instance,
>>are marked with red background in my terminal.
>>
>>Is it possible to see this coloring in git gui too?
>>
>>-Tor Arvid-
>
> This patch should do the job. It probably should get some configuration
> item to control this though.
>
>  git-gui.sh   |    1 +
>  lib/diff.tcl |    9 ++++++++-
>  2 files changed, 9 insertions(+), 1 deletions(-)
>
> diff --git a/git-gui.sh b/git-gui.sh
> index 25229a4..8d652f0 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -3314,6 +3314,7 @@ pack .vpane.lower.diff.header -side top -fill x
>  pack .vpane.lower.diff.body -side bottom -fill both -expand 1
>
>  $ui_diff tag conf d_cr -elide true
> +$ui_diff tag conf ws -background red
>  $ui_diff tag conf d_@ -foreground blue -font font_diffbold
>  $ui_diff tag conf d_+ -foreground {#00a000}
>  $ui_diff tag conf d_- -foreground red
> diff --git a/lib/diff.tcl b/lib/diff.tcl
> index c628750..83e3f6d 100644
> --- a/lib/diff.tcl
> +++ b/lib/diff.tcl
> @@ -434,7 +434,14 @@ proc read_diff {fd cont_info} {
>                        }
>                        }
>                }
> -               $ui_diff insert end $line $tags
> +               if {[regexp -indices {^.*\S(\s+)$} $line -> ndx]} {
> +                   set ndx [expr {[lindex $ndx 0] - 1}]
> +                       set nonws [string range $line 0 $ndx]
> +                       $ui_diff insert end $nonws $tags \
> +                               [string range $line [incr ndx] end] [concat $tags ws]
> +               } else {
> +                       $ui_diff insert end $line $tags
> +               }
<snip>

So - what I tried to do was expand on this (to include a 'space-before-tab' filter). I can't make a simple "elseif" for regexp "( +)\t" (or whatever the right regexp may be).

I mean that currently you can only match one ws-filter per line. So a line with both space-before-tab and blank-at-eol would not be printed correctly.

What I wanted to do, in pseudo-code, was:
read git config core.whitespace
for each diff-line #diff-line is any line starting with '+' or '-'
  set temp-ui-line to $line $tags
  if option_enabled core.whitespace.space-before-tab
    if regexp_match space-before-tab-pattern -> indices
      temp-ui-line.replace(indices[0], indices[1]) with-red-bg
  if option_enabled core.whitespace.blank-at-eol
    ...
  ..
end for

#and then $ui_diff insert end $temp-ui-line ------------------

Maybe this is the easiest thing in the world to do in Tcl, but I don't have more time to fiddle with it today :)

Thanks for the patch anyways.
-Tor Arvid-
Pat Thoyts· Oct 20, 2010, 22:05 UTC · re: Tor Arvid Lund · lore

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

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>
---
 git-gui.sh   |   27 +++++++++++++++++++++++++++
 lib/diff.tcl |   58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 83 insertions(+), 2 deletions(-)
diff --git a/git-gui.sh b/git-gui.sh
index 5e8378f..134afba 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -806,6 +806,7 @@ proc apply_config {} {
 }
 
 set default_config(branch.autosetupmerge) true
+set default_config(core.whitespace) ""
 set default_config(merge.tool) {}
 set default_config(mergetool.keepbackup) true
 set default_config(merge.diffstat) true
@@ -836,6 +837,13 @@ set font_descs {
 	{fontdiff font_diff {mc "Diff/Console Font"}}
 }
 
+set whitespace_config(blank-at-eol) 1
+set whitespace_config(space-before-tab) 1
+set whitespace_config(indent-with-non-tab) 0
+set whitespace_config(tab-in-indent) 0
+set whitespace_config(blank-at-eof) 1
+set whitespace_config(cr-at-eol) 0
+
 ######################################################################
 ##
 ## find git
@@ -1060,6 +1068,7 @@ git-version proc _parse_config {arr_name args} {
 
 proc load_config {include_global} {
 	global repo_config global_config system_config default_config
+	global whitespace_config
 
 	if {$include_global} {
 		_parse_config system_config --system
@@ -1080,6 +1089,23 @@ proc load_config {include_global} {
 			set repo_config($name) $system_config($name)
 		}
 	}
+	set whitespace ""
+	foreach cf {default_config system_config global_config repo_config} {
+		upvar #0 $cf config
+		if {[info exists config(core.whitespace)]} {
+			set whitespace $config(core.whitespace)
+		}
+	}
+	foreach var [split $whitespace ,] {
+		set state [expr {![string match -* $var]}]
+		set var [string trimleft $var -]
+		if {[string match "trailing-space" $var]} {
+			set whitespace_config(blank-at-eol) $state
+			set whitespace_config(blank-at-eof) $state
+		} else {
+			set whitespace_config($var) $state
+		}
+	}
 }
 
 ######################################################################
@@ -3323,6 +3349,7 @@ pack .vpane.lower.diff.header -side top -fill x
 pack .vpane.lower.diff.body -side bottom -fill both -expand 1
 
 $ui_diff tag conf d_cr -elide true
+$ui_diff tag conf ws -background red
 $ui_diff tag conf d_@ -foreground blue -font font_diffbold
 $ui_diff tag conf d_+ -foreground {#00a000}
 $ui_diff tag conf d_- -foreground red
diff --git a/lib/diff.tcl b/lib/diff.tcl
index c628750..e174faf 100644
--- a/lib/diff.tcl
+++ b/lib/diff.tcl
@@ -332,11 +332,17 @@ proc start_show_diff {cont_info {add_opts {}}} {
 	fileevent $fd readable [list read_diff $fd $cont_info]
 }
 
+proc add_tag {w tag pos ndx} {
+	set begin "$pos + [lindex $ndx 0] chars"
+	set end "$pos + [expr {[lindex $ndx 1] + 1}] chars"
+	$w tag add $tag $begin $end
+}
+
 proc read_diff {fd cont_info} {
 	global ui_diff diff_active is_submodule_diff
 	global is_3way_diff is_conflict_diff current_diff_header
 	global current_diff_queue
-	global diff_empty_count
+	global diff_empty_count whitespace_config
 
 	$ui_diff conf -state normal
 	while {[gets $fd line] >= 0} {
@@ -434,7 +440,38 @@ proc read_diff {fd cont_info} {
 			}
 			}
 		}
-		$ui_diff insert end $line $tags
+		set insertion [list $line $tags]
+		if {$whitespace_config(blank-at-eol)} {
+			if {[regexp -indices {^[+-](?:.*\S)?(\s+)$} $line -> ndx]} {
+				set ndx [expr {[lindex $ndx 0] - 1}]
+				set insertion [list [string range $line 0 $ndx] $tags\
+								   [string range $line [incr ndx] end]\
+								   [concat $tags ws]]
+			}
+		}
+		set pos [$ui_diff index "end - 1 line linestart"]
+		eval [linsert $insertion 0 $ui_diff insert end]
+		if {[regexp {^[+-](\s*)\S} [lindex $insertion 0] -> initial]} {
+			if {$whitespace_config(indent-with-non-tab)} {
+				if {[regexp -indices {^x( {8,})} x$initial -> ndx]} {
+					add_tag $ui_diff ws $pos $ndx
+				}
+			}
+			if {$whitespace_config(space-before-tab)} {
+				set start 0
+				while {[regexp -indices -start $start {( +)\t} x$initial -> ndx]} {
+					add_tag $ui_diff ws $pos $ndx
+					set start [expr {[lindex $ndx 1] + 1}]
+				}
+			}
+			if {$whitespace_config(tab-in-indent)} {
+				set start 0
+				while {[regexp -indices -start $start {\t} x$initial ndx]} {
+					add_tag $ui_diff ws $pos $ndx
+					set start [expr {[lindex $ndx 1] + 1}]
+				}
+			}
+		}
 		if {[string index $line end] eq "\r"} {
 			$ui_diff tag add d_cr {end - 2c}
 		}
@@ -450,6 +487,16 @@ proc read_diff {fd cont_info} {
 			return
 		}
 
+		if {$whitespace_config(blank-at-eof)} {
+			set n 2
+			$ui_diff configure -state normal
+			while {[regexp {^[+-]$} [$ui_diff get "end - $n line linestart" "end - $n line lineend"]]} {
+				$ui_diff tag add ws "end - $n line linestart" "end - $n line lineend"
+				incr n
+			}
+			$ui_diff configure -state disabled
+		}
+
 		set diff_active 0
 		unlock_index
 		set scroll_pos [lindex $cont_info 0]
@@ -731,3 +778,10 @@ proc apply_range_or_line {x y} {
 
 	unlock_index
 }
+
+# Local variables:
+# mode: tcl
+# indent-tabs-mode: t
+# tcl-indent-level: 4
+# tab-width: 4
+# End:

-- 
1.7.3.1.msysgit.0
Junio C Hamano· Oct 20, 2010, 23:43 UTC · re: Pat Thoyts · lore

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

Pat Thoyts <patthoyts@users.sourceforge.net> writes:
Show 9 quoted lines
> 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?

Tor Arvid Lund· Oct 21, 2010, 12:36 UTC · re: Junio C Hamano · lore

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

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-
Pat Thoyts· Oct 21, 2010, 18:58 UTC · re: Tor Arvid Lund · lore

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

Tor Arvid Lund <torarvid@gmail.com> writes:
Show 36 quoted lines
>On Thu, Oct 21, 2010 at 1:43 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> 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).

This is caused by the tabstyle being defaulted to 'tabular'. This means the first tab on a line extends to the first tab position and if that lies to the left then the width of a single space is added. Looks like this widget should be using -tabstyle wordprocessor which would do what you expect and move to the next tab position.

It is still a tab - just the display width looks wrong.
Show 11 quoted lines
>
>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?
Indeed - see my other post for an implementation.
-- 
Pat Thoyts                            http://www.patthoyts.tk/
PGP fingerprint 2C 6E 98 07 2C 59 C8 97  10 CE 11 E6 04 E0 B9 DD
Tor Arvid Lund· Oct 22, 2010, 12:00 UTC · re: Pat Thoyts · lore

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

On Thu, Oct 21, 2010 at 8:58 PM, Pat Thoyts <patthoyts@users.sourceforge.net> wrote:

> Tor Arvid Lund <torarvid@gmail.com> writes:
<snip>
Show 24 quoted lines
>>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).
>
> This is caused by the tabstyle being defaulted to 'tabular'. This means
> the first tab on a line extends to the first tab position and if that
> lies to the left then the width of a single space is added. Looks like
> this widget should be using -tabstyle wordprocessor which would do what
> you expect and move to the next tab position.
>
> It is still a tab - just the display width looks wrong.
<snap>
Ok. I tried this, which seems to make it better:
diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index 31ca47d..65d5f2a 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -3283,6 +3283,7 @@ text $ui_diff -background white -foreground black \
        -borderwidth 0 \
        -width 80 -height 5 -wrap none \
        -font font_diff \
+       -tabstyle wordprocessor \
        -xscrollcommand {.vpane.lower.diff.body.sbx set} \
        -yscrollcommand {.vpane.lower.diff.body.sby set} \
        -state disabled

(this goes on top of your most recent patch from an hour ago)

-Tor Arvid-
Pat Thoyts· Oct 22, 2010, 15:18 UTC · re: Tor Arvid Lund · lore

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

Tor Arvid Lund <torarvid@gmail.com> writes:
Show 46 quoted lines
>On Thu, Oct 21, 2010 at 8:58 PM, Pat Thoyts
><patthoyts@users.sourceforge.net> wrote:
>> Tor Arvid Lund <torarvid@gmail.com> writes:
><snip>
>>>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).
>>
>> This is caused by the tabstyle being defaulted to 'tabular'. This means
>> the first tab on a line extends to the first tab position and if that
>> lies to the left then the width of a single space is added. Looks like
>> this widget should be using -tabstyle wordprocessor which would do what
>> you expect and move to the next tab position.
>>
>> It is still a tab - just the display width looks wrong.
><snap>
>
>Ok. I tried this, which seems to make it better:
>
>diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
>index 31ca47d..65d5f2a 100755
>--- a/git-gui/git-gui.sh
>+++ b/git-gui/git-gui.sh
>@@ -3283,6 +3283,7 @@ text $ui_diff -background white -foreground black \
>        -borderwidth 0 \
>        -width 80 -height 5 -wrap none \
>        -font font_diff \
>+       -tabstyle wordprocessor \
>        -xscrollcommand {.vpane.lower.diff.body.sbx set} \
>        -yscrollcommand {.vpane.lower.diff.body.sby set} \
>        -state disabled
>
>(this goes on top of your most recent patch from an hour ago)

I guess I should have mentioned - I already pushed such a patch to git-gui.git's master.

cdd321a git-gui: use wordprocessor tab style to ensure tabs work as expected
Thanks anyway :)
-- 
Pat Thoyts                            http://www.patthoyts.tk/
PGP fingerprint 2C 6E 98 07 2C 59 C8 97  10 CE 11 E6 04 E0 B9 DD
Pat Thoyts· Oct 21, 2010, 15:22 UTC · re: Junio C Hamano · lore

[PATCH] git-gui: apply color information from git diff

This patch extracts the ansi color sequences from git diff output and applies these to the diff view window. This ensures that the gui view makes use of the current git configuration for whitespace display.

Suggested-by: Tor Arvid Lund <torarvid@gmail.com>
Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Pat Thoyts <patthoyts@users.sourceforge.net>
---
Junio C Hamano <gitster@pobox.com> writes:
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?
Seems like a fine plan. Here is an implementation to try out.
 git-gui.sh   |    7 ++++++-
 lib/diff.tcl |   27 ++++++++++++++++++++++++++-
 2 files changed, 32 insertions(+), 2 deletions(-)
diff --git a/git-gui.sh b/git-gui.sh
index 5e8378f..8fba57c 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -3322,8 +3322,13 @@ pack $ui_diff -side left -fill both -expand 1
 pack .vpane.lower.diff.header -side top -fill x
 pack .vpane.lower.diff.body -side bottom -fill both -expand 1
 
+foreach {n c} {0 black 1 red4 2 green4 3 yellow4 4 blue4 5 magenta4 6 cyan4 7 grey60} {
+	$ui_diff tag configure clr4$n -background $c
+	$ui_diff tag configure clr3$n -foreground $c
+}
+
 $ui_diff tag conf d_cr -elide true
-$ui_diff tag conf d_@ -foreground blue -font font_diffbold
+$ui_diff tag conf d_@ -font font_diffbold
 $ui_diff tag conf d_+ -foreground {#00a000}
 $ui_diff tag conf d_- -foreground red
 
diff --git a/lib/diff.tcl b/lib/diff.tcl
index c628750..7625cb8 100644
--- a/lib/diff.tcl
+++ b/lib/diff.tcl
@@ -294,7 +294,7 @@ proc start_show_diff {cont_info {add_opts {}}} {
 	}
 
 	lappend cmd -p
-	lappend cmd --no-color
+	lappend cmd --color
 	if {$repo_config(gui.diffcontext) >= 1} {
 		lappend cmd "-U$repo_config(gui.diffcontext)"
 	}
@@ -332,6 +332,21 @@ proc start_show_diff {cont_info {add_opts {}}} {
 	fileevent $fd readable [list read_diff $fd $cont_info]
 }
 
+proc parse_color_line {line} {
+	set start 0
+	set result ""
+	set markup [list]
+	while {[regexp -indices -start $start "\033\\\[(\\d+)?m" $line match code]} {
+		foreach {begin end} $match break
+		append result [string range $line $start [expr {$begin - 1}]]
+		lappend markup [string length $result] [eval [linsert $code 0 string range $line]]
+		set start [incr end]
+	}
+	append result [string range $line $start end]
+	if {[llength $markup] < 4} {set markup {}}
+	return [list $result $markup]
+}
+
 proc read_diff {fd cont_info} {
 	global ui_diff diff_active is_submodule_diff
 	global is_3way_diff is_conflict_diff current_diff_header
@@ -340,6 +355,9 @@ proc read_diff {fd cont_info} {
 
 	$ui_diff conf -state normal
 	while {[gets $fd line] >= 0} {
+		foreach {line markup} [parse_color_line $line] break
+		set line [string map {\033 ^} $line]
+
 		# -- Cleanup uninteresting diff header lines.
 		#
 		if {$::current_diff_inheader} {
@@ -434,11 +452,18 @@ proc read_diff {fd cont_info} {
 			}
 			}
 		}
+		set mark [$ui_diff index "end - 1 line linestart"]
 		$ui_diff insert end $line $tags
 		if {[string index $line end] eq "\r"} {
 			$ui_diff tag add d_cr {end - 2c}
 		}
 		$ui_diff insert end "\n" $tags
+
+		foreach {posbegin colbegin posend colend} $markup {
+			set a "$mark linestart + $posbegin chars"
+			set b "$mark linestart + $posend chars"
+			catch {$ui_diff tag add clr$colbegin $a $b}
+		}
 	}
 	$ui_diff conf -state disabled
 

-- 
1.7.3.1.msysgit.0
Kevin Ballard· Oct 21, 2010, 20:59 UTC · re: Pat Thoyts · lore

Re: [PATCH] git-gui: apply color information from git diff

On Oct 21, 2010, at 8:22 AM, Pat Thoyts wrote:
> +	while {[regexp -indices -start $start "\033\\\[(\\d+)?m" $line match code]} {
Git currently doesn't emit combined escapes (e.g. \e[0;31m to reset and then turn on red text), but I can imagine it being enhanced to do this in the future. I would recommend handling it here if you can.
-Kevin Ballard
Pat Thoyts· Oct 21, 2010, 23:42 UTC · re: Kevin Ballard · lore

Re: [PATCH] git-gui: apply color information from git diff

Kevin Ballard <kevin@sb.org> writes:
Show 5 quoted lines
>On Oct 21, 2010, at 8:22 AM, Pat Thoyts wrote:
>
>> +	while {[regexp -indices -start $start "\033\\\[(\\d+)?m" $line match code]} {
>
>Git currently doesn't emit combined escapes (e.g. \e[0;31m to reset and then turn on red text), but I can imagine it being enhanced to do this in the future. I would recommend handling it here if you can.
In fact that can be configured even now:
 % git config color.diff.whitespace "bold white bold cyan"
 % git diff --color | cat.exe --show-all
 ^[[1;37;46m+^[[m$
so thanks for the warning.
-- 
Pat Thoyts                            http://www.patthoyts.tk/
PGP fingerprint 2C 6E 98 07 2C 59 C8 97  10 CE 11 E6 04 E0 B9 DD
Pat Thoyts· Oct 22, 2010, 10:10 UTC · re: Kevin Ballard · lore

[PATCH] git-gui: apply color information from git diff output

This patch extracts the ANSI color sequences from git diff output and applies these to the diff view window. This ensures that the gui view makes use of the current git configuration for whitespace display.

ANSI codes may include attributes, foreground and background in a single sequence. Handle this and support bold and reverse attributes. Ignore all other attributes.

Suggested-by: Tor Arvid Lund <torarvid@gmail.com>
Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Pat Thoyts <patthoyts@users.sourceforge.net>
---
Kevin Ballard <kevin@sb.org> writes:
Show 7 quoted lines
>On Oct 21, 2010, at 8:22 AM, Pat Thoyts wrote:
>
>> +	while {[regexp -indices -start $start "\033\\\[(\\d+)?m" $line match code]} {
>
>Git currently doesn't emit combined escapes (e.g. \e[0;31m to reset and then turn on red text), but I can imagine it being enhanced to do this in the future. I would recommend handling it here if you can.
>
>-Kevin Ballard

It turns out that such sequences will be generated by git if the user configures the color.diff.whitespace (eg: bold cyan magenta). This patch handles these cases. I don't see any point trying to handle blink. I could add underline but I don't see that being so appropriate for a GUI. It seems more like something that is configured for a monochrome terminal.

 git-gui.sh   |   10 +++++++++-
 lib/diff.tcl |   34 +++++++++++++++++++++++++++++++++-
 2 files changed, 42 insertions(+), 2 deletions(-)
diff --git a/git-gui.sh b/git-gui.sh
index 1ccaba1..1fb0254 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -3322,8 +3322,16 @@ pack $ui_diff -side left -fill both -expand 1
 pack .vpane.lower.diff.header -side top -fill x
 pack .vpane.lower.diff.body -side bottom -fill both -expand 1
 
+foreach {n c} {0 black 1 red4 2 green4 3 yellow4 4 blue4 5 magenta4 6 cyan4 7 grey60} {
+	$ui_diff tag configure clr4$n -background $c
+	$ui_diff tag configure clri4$n -foreground $c
+	$ui_diff tag configure clr3$n -foreground $c
+	$ui_diff tag configure clri3$n -background $c
+}
+$ui_diff tag configure clr1 -font font_diffbold
+
 $ui_diff tag conf d_cr -elide true
-$ui_diff tag conf d_@ -foreground blue -font font_diffbold
+$ui_diff tag conf d_@ -font font_diffbold
 $ui_diff tag conf d_+ -foreground {#00a000}
 $ui_diff tag conf d_- -foreground red
 
diff --git a/lib/diff.tcl b/lib/diff.tcl
index c628750..dcf0711 100644
--- a/lib/diff.tcl
+++ b/lib/diff.tcl
@@ -294,7 +294,7 @@ proc start_show_diff {cont_info {add_opts {}}} {
 	}
 
 	lappend cmd -p
-	lappend cmd --no-color
+	lappend cmd --color
 	if {$repo_config(gui.diffcontext) >= 1} {
 		lappend cmd "-U$repo_config(gui.diffcontext)"
 	}
@@ -332,6 +332,23 @@ proc start_show_diff {cont_info {add_opts {}}} {
 	fileevent $fd readable [list read_diff $fd $cont_info]
 }
 
+proc parse_color_line {line} {
+	set start 0
+	set result ""
+	set markup [list]
+	set regexp {\033\[((?:\d+;)*\d+)?m}
+	while {[regexp -indices -start $start $regexp $line match code]} {
+		foreach {begin end} $match break
+		append result [string range $line $start [expr {$begin - 1}]]
+		lappend markup [string length $result] \
+			[eval [linsert $code 0 string range $line]]
+		set start [incr end]
+	}
+	append result [string range $line $start end]
+	if {[llength $markup] < 4} {set markup {}}
+	return [list $result $markup]
+}
+
 proc read_diff {fd cont_info} {
 	global ui_diff diff_active is_submodule_diff
 	global is_3way_diff is_conflict_diff current_diff_header
@@ -340,6 +357,9 @@ proc read_diff {fd cont_info} {
 
 	$ui_diff conf -state normal
 	while {[gets $fd line] >= 0} {
+		foreach {line markup} [parse_color_line $line] break
+		set line [string map {\033 ^} $line]
+
 		# -- Cleanup uninteresting diff header lines.
 		#
 		if {$::current_diff_inheader} {
@@ -434,11 +454,23 @@ proc read_diff {fd cont_info} {
 			}
 			}
 		}
+		set mark [$ui_diff index "end - 1 line linestart"]
 		$ui_diff insert end $line $tags
 		if {[string index $line end] eq "\r"} {
 			$ui_diff tag add d_cr {end - 2c}
 		}
 		$ui_diff insert end "\n" $tags
+
+		foreach {posbegin colbegin posend colend} $markup {
+			set prefix clr
+			foreach style [split $colbegin ";"] {
+				if {$style eq "7"} {append prefix i; continue}
+				if {$style < 30 || $style > 47} {continue}
+				set a "$mark linestart + $posbegin chars"
+				set b "$mark linestart + $posend chars"
+				catch {$ui_diff tag add $prefix$style $a $b}
+			}
+		}
 	}
 	$ui_diff conf -state disabled
 

-- 
1.7.3.1.msysgit.0
Tor Arvid Lund· Oct 22, 2010, 14:40 UTC · re: Pat Thoyts · lore

Re: [PATCH] git-gui: apply color information from git diff output

On Fri, Oct 22, 2010 at 12:10 PM, Pat Thoyts <patthoyts@users.sourceforge.net> wrote:

Show 12 quoted lines
> This patch extracts the ANSI color sequences from git diff output and
> applies these to the diff view window. This ensures that the gui view
> makes use of the current git configuration for whitespace display.
>
> ANSI codes may include attributes, foreground and background in a single
> sequence. Handle this and support bold and reverse attributes. Ignore
> all other attributes.
>
> Suggested-by: Tor Arvid Lund <torarvid@gmail.com>
> Suggested-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Pat Thoyts <patthoyts@users.sourceforge.net>
> ---
Tested-by: Tor Arvid Lund <torarvid@gmail.com>

It seems to work well for me. I'm just using the default (unset) core.whitespace settings, so space-before-tab and blank-at-eol both show up with red background, just like in the console.

-Tor Arvid-
Show 119 quoted lines
> Kevin Ballard <kevin@sb.org> writes:
>
>>On Oct 21, 2010, at 8:22 AM, Pat Thoyts wrote:
>>
>>> +    while {[regexp -indices -start $start "\033\\\[(\\d+)?m" $line match code]} {
>>
>>Git currently doesn't emit combined escapes (e.g. \e[0;31m to reset and then turn on red text), but I can imagine it being enhanced to do this in the future. I would recommend handling it here if you can.
>>
>>-Kevin Ballard
>
> It turns out that such sequences will be generated by git if the user
> configures the color.diff.whitespace (eg: bold cyan magenta). This patch
> handles these cases. I don't see any point trying to handle blink. I
> could add underline but I don't see that being so appropriate for a
> GUI. It seems more like something that is configured for a monochrome
> terminal.
>
>  git-gui.sh   |   10 +++++++++-
>  lib/diff.tcl |   34 +++++++++++++++++++++++++++++++++-
>  2 files changed, 42 insertions(+), 2 deletions(-)
>
> diff --git a/git-gui.sh b/git-gui.sh
> index 1ccaba1..1fb0254 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -3322,8 +3322,16 @@ pack $ui_diff -side left -fill both -expand 1
>  pack .vpane.lower.diff.header -side top -fill x
>  pack .vpane.lower.diff.body -side bottom -fill both -expand 1
>
> +foreach {n c} {0 black 1 red4 2 green4 3 yellow4 4 blue4 5 magenta4 6 cyan4 7 grey60} {
> +       $ui_diff tag configure clr4$n -background $c
> +       $ui_diff tag configure clri4$n -foreground $c
> +       $ui_diff tag configure clr3$n -foreground $c
> +       $ui_diff tag configure clri3$n -background $c
> +}
> +$ui_diff tag configure clr1 -font font_diffbold
> +
>  $ui_diff tag conf d_cr -elide true
> -$ui_diff tag conf d_@ -foreground blue -font font_diffbold
> +$ui_diff tag conf d_@ -font font_diffbold
>  $ui_diff tag conf d_+ -foreground {#00a000}
>  $ui_diff tag conf d_- -foreground red
>
> diff --git a/lib/diff.tcl b/lib/diff.tcl
> index c628750..dcf0711 100644
> --- a/lib/diff.tcl
> +++ b/lib/diff.tcl
> @@ -294,7 +294,7 @@ proc start_show_diff {cont_info {add_opts {}}} {
>        }
>
>        lappend cmd -p
> -       lappend cmd --no-color
> +       lappend cmd --color
>        if {$repo_config(gui.diffcontext) >= 1} {
>                lappend cmd "-U$repo_config(gui.diffcontext)"
>        }
> @@ -332,6 +332,23 @@ proc start_show_diff {cont_info {add_opts {}}} {
>        fileevent $fd readable [list read_diff $fd $cont_info]
>  }
>
> +proc parse_color_line {line} {
> +       set start 0
> +       set result ""
> +       set markup [list]
> +       set regexp {\033\[((?:\d+;)*\d+)?m}
> +       while {[regexp -indices -start $start $regexp $line match code]} {
> +               foreach {begin end} $match break
> +               append result [string range $line $start [expr {$begin - 1}]]
> +               lappend markup [string length $result] \
> +                       [eval [linsert $code 0 string range $line]]
> +               set start [incr end]
> +       }
> +       append result [string range $line $start end]
> +       if {[llength $markup] < 4} {set markup {}}
> +       return [list $result $markup]
> +}
> +
>  proc read_diff {fd cont_info} {
>        global ui_diff diff_active is_submodule_diff
>        global is_3way_diff is_conflict_diff current_diff_header
> @@ -340,6 +357,9 @@ proc read_diff {fd cont_info} {
>
>        $ui_diff conf -state normal
>        while {[gets $fd line] >= 0} {
> +               foreach {line markup} [parse_color_line $line] break
> +               set line [string map {\033 ^} $line]
> +
>                # -- Cleanup uninteresting diff header lines.
>                #
>                if {$::current_diff_inheader} {
> @@ -434,11 +454,23 @@ proc read_diff {fd cont_info} {
>                        }
>                        }
>                }
> +               set mark [$ui_diff index "end - 1 line linestart"]
>                $ui_diff insert end $line $tags
>                if {[string index $line end] eq "\r"} {
>                        $ui_diff tag add d_cr {end - 2c}
>                }
>                $ui_diff insert end "\n" $tags
> +
> +               foreach {posbegin colbegin posend colend} $markup {
> +                       set prefix clr
> +                       foreach style [split $colbegin ";"] {
> +                               if {$style eq "7"} {append prefix i; continue}
> +                               if {$style < 30 || $style > 47} {continue}
> +                               set a "$mark linestart + $posbegin chars"
> +                               set b "$mark linestart + $posend chars"
> +                               catch {$ui_diff tag add $prefix$style $a $b}
> +                       }
> +               }
>        }
>        $ui_diff conf -state disabled
>
> --
> 1.7.3.1.msysgit.0
>
>
>

← back to recent threads