* Colored whitespace in git gui @ 2010-10-18 23:00 Tor Arvid Lund 2010-10-19 22:59 ` [PATCH] git-gui: highlight trailing whitespace in diff view Pat Thoyts 0 siblings, 1 reply; 14+ messages in thread From: Tor Arvid Lund @ 2010-10-18 23:00 UTC (permalink / raw) To: Git mailing list 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- ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] git-gui: highlight trailing whitespace in diff view 2010-10-18 23:00 Colored whitespace in git gui Tor Arvid Lund @ 2010-10-19 22:59 ` Pat Thoyts 2010-10-20 15:30 ` Tor Arvid Lund 0 siblings, 1 reply; 14+ messages in thread From: Pat Thoyts @ 2010-10-19 22:59 UTC (permalink / raw) To: Tor Arvid Lund; +Cc: Git mailing list 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: >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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] git-gui: highlight trailing whitespace in diff view 2010-10-19 22:59 ` [PATCH] git-gui: highlight trailing whitespace in diff view Pat Thoyts @ 2010-10-20 15:30 ` Tor Arvid Lund 2010-10-20 22:05 ` [PATCH] git-gui: support core.whitespace rules " Pat Thoyts 0 siblings, 1 reply; 14+ messages in thread From: Tor Arvid Lund @ 2010-10-20 15:30 UTC (permalink / raw) To: Pat Thoyts; +Cc: Git mailing list On Wed, Oct 20, 2010 at 12:59 AM, Pat Thoyts <patthoyts@users.sourceforge.net> wrote: > 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 :) > --- > > 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- ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] git-gui: support core.whitespace rules in diff view 2010-10-20 15:30 ` Tor Arvid Lund @ 2010-10-20 22:05 ` Pat Thoyts 2010-10-20 23:43 ` Junio C Hamano 0 siblings, 1 reply; 14+ messages in thread From: Pat Thoyts @ 2010-10-20 22:05 UTC (permalink / raw) To: Tor Arvid Lund; +Cc: Git mailing list 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] git-gui: support core.whitespace rules in diff view 2010-10-20 22:05 ` [PATCH] git-gui: support core.whitespace rules " Pat Thoyts @ 2010-10-20 23:43 ` Junio C Hamano 2010-10-21 12:36 ` Tor Arvid Lund 2010-10-21 15:22 ` [PATCH] git-gui: apply color information from git diff Pat Thoyts 0 siblings, 2 replies; 14+ messages in thread From: Junio C Hamano @ 2010-10-20 23:43 UTC (permalink / raw) To: Pat Thoyts; +Cc: Tor Arvid Lund, Git mailing list 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? ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] git-gui: support core.whitespace rules in diff view 2010-10-20 23:43 ` Junio C Hamano @ 2010-10-21 12:36 ` Tor Arvid Lund 2010-10-21 18:58 ` Pat Thoyts 2010-10-21 15:22 ` [PATCH] git-gui: apply color information from git diff Pat Thoyts 1 sibling, 1 reply; 14+ messages in thread From: Tor Arvid Lund @ 2010-10-21 12:36 UTC (permalink / raw) To: Pat Thoyts; +Cc: Junio C Hamano, Git mailing list 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). 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- ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] git-gui: support core.whitespace rules in diff view 2010-10-21 12:36 ` Tor Arvid Lund @ 2010-10-21 18:58 ` Pat Thoyts 2010-10-22 12:00 ` Tor Arvid Lund 0 siblings, 1 reply; 14+ messages in thread From: Pat Thoyts @ 2010-10-21 18:58 UTC (permalink / raw) To: Tor Arvid Lund; +Cc: Junio C Hamano, Git mailing list Tor Arvid Lund <torarvid@gmail.com> writes: >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. > >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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] git-gui: support core.whitespace rules in diff view 2010-10-21 18:58 ` Pat Thoyts @ 2010-10-22 12:00 ` Tor Arvid Lund 2010-10-22 15:18 ` Pat Thoyts 0 siblings, 1 reply; 14+ messages in thread From: Tor Arvid Lund @ 2010-10-22 12:00 UTC (permalink / raw) To: Pat Thoyts; +Cc: Junio C Hamano, Git mailing list 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) -Tor Arvid- ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] git-gui: support core.whitespace rules in diff view 2010-10-22 12:00 ` Tor Arvid Lund @ 2010-10-22 15:18 ` Pat Thoyts 0 siblings, 0 replies; 14+ messages in thread From: Pat Thoyts @ 2010-10-22 15:18 UTC (permalink / raw) To: Tor Arvid Lund; +Cc: Junio C Hamano, Git mailing list Tor Arvid Lund <torarvid@gmail.com> writes: >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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] git-gui: apply color information from git diff 2010-10-20 23:43 ` Junio C Hamano 2010-10-21 12:36 ` Tor Arvid Lund @ 2010-10-21 15:22 ` Pat Thoyts 2010-10-21 20:59 ` Kevin Ballard 1 sibling, 1 reply; 14+ messages in thread From: Pat Thoyts @ 2010-10-21 15:22 UTC (permalink / raw) To: Junio C Hamano; +Cc: Tor Arvid Lund, Git mailing list 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: >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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] git-gui: apply color information from git diff 2010-10-21 15:22 ` [PATCH] git-gui: apply color information from git diff Pat Thoyts @ 2010-10-21 20:59 ` Kevin Ballard 2010-10-21 23:42 ` Pat Thoyts 2010-10-22 10:10 ` [PATCH] git-gui: apply color information from git diff output Pat Thoyts 0 siblings, 2 replies; 14+ messages in thread From: Kevin Ballard @ 2010-10-21 20:59 UTC (permalink / raw) To: Pat Thoyts; +Cc: Junio C Hamano, Tor Arvid Lund, Git mailing list 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] git-gui: apply color information from git diff 2010-10-21 20:59 ` Kevin Ballard @ 2010-10-21 23:42 ` Pat Thoyts 2010-10-22 10:10 ` [PATCH] git-gui: apply color information from git diff output Pat Thoyts 1 sibling, 0 replies; 14+ messages in thread From: Pat Thoyts @ 2010-10-21 23:42 UTC (permalink / raw) To: Kevin Ballard; +Cc: Junio C Hamano, Tor Arvid Lund, Git mailing list 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. 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] git-gui: apply color information from git diff output 2010-10-21 20:59 ` Kevin Ballard 2010-10-21 23:42 ` Pat Thoyts @ 2010-10-22 10:10 ` Pat Thoyts 2010-10-22 14:40 ` Tor Arvid Lund 1 sibling, 1 reply; 14+ messages in thread From: Pat Thoyts @ 2010-10-22 10:10 UTC (permalink / raw) To: Kevin Ballard; +Cc: Junio C Hamano, Tor Arvid Lund, Git mailing list 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: >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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] git-gui: apply color information from git diff output 2010-10-22 10:10 ` [PATCH] git-gui: apply color information from git diff output Pat Thoyts @ 2010-10-22 14:40 ` Tor Arvid Lund 0 siblings, 0 replies; 14+ messages in thread From: Tor Arvid Lund @ 2010-10-22 14:40 UTC (permalink / raw) To: Pat Thoyts; +Cc: Kevin Ballard, Junio C Hamano, Git mailing list On Fri, Oct 22, 2010 at 12:10 PM, Pat Thoyts <patthoyts@users.sourceforge.net> wrote: > 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- > 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 > > > ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2010-10-22 15:18 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-10-18 23:00 Colored whitespace in git gui Tor Arvid Lund 2010-10-19 22:59 ` [PATCH] git-gui: highlight trailing whitespace in diff view Pat Thoyts 2010-10-20 15:30 ` Tor Arvid Lund 2010-10-20 22:05 ` [PATCH] git-gui: support core.whitespace rules " Pat Thoyts 2010-10-20 23:43 ` Junio C Hamano 2010-10-21 12:36 ` Tor Arvid Lund 2010-10-21 18:58 ` Pat Thoyts 2010-10-22 12:00 ` Tor Arvid Lund 2010-10-22 15:18 ` Pat Thoyts 2010-10-21 15:22 ` [PATCH] git-gui: apply color information from git diff Pat Thoyts 2010-10-21 20:59 ` Kevin Ballard 2010-10-21 23:42 ` Pat Thoyts 2010-10-22 10:10 ` [PATCH] git-gui: apply color information from git diff output Pat Thoyts 2010-10-22 14:40 ` Tor Arvid Lund
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).