git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] gitk: add diff lines background colors
@ 2020-02-11 21:24 Stefan Dotterweich
  2020-04-07 16:42 ` Johannes Sixt
  0 siblings, 1 reply; 4+ messages in thread
From: Stefan Dotterweich @ 2020-02-11 21:24 UTC (permalink / raw)
  To: git; +Cc: Paul Mackerras, Stefan Dotterweich

Not using colored background for added and removed lines is a missed
opportunity to make diff lines easier to grasp visually.

Use a subtle red/green background by default. Make the font slightly darker
to improve contrast.

Signed-off-by: Stefan Dotterweich <stefandotterweich@gmx.de>
---
The variable diffcolors seems like a fitting place for the two new colors.
However, adding them to that list causes problems if diffcolors saved in
.gitk then contains three instead of five colors. This could be solved by
modifying the variable after loading .gitk. I'm not sure if that would be
the preferred approach or where to implement a special case like that. To
avoid the problem, I introduced a new variable diffbgcolors.

 gitk | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/gitk b/gitk
index da84e22..66c237a 100755
--- a/gitk
+++ b/gitk
@@ -2073,7 +2073,7 @@ proc makewindow {} {
     global rowctxmenu fakerowmenu mergemax wrapcomment
     global highlight_files gdttype
     global searchstring sstring
-    global bgcolor fgcolor bglist fglist diffcolors selectbgcolor
+    global bgcolor fgcolor bglist fglist diffcolors diffbgcolors selectbgcolor
     global uifgcolor uifgdisabledcolor
     global filesepbgcolor filesepfgcolor
     global mergecolors foundbgcolor currentsearchhitbgcolor
@@ -2434,7 +2434,9 @@ proc makewindow {} {
     $ctext tag conf filesep -font textfontbold -fore $filesepfgcolor -back $filesepbgcolor
     $ctext tag conf hunksep -fore [lindex $diffcolors 2]
     $ctext tag conf d0 -fore [lindex $diffcolors 0]
+    $ctext tag conf d0 -back [lindex $diffbgcolors 0]
     $ctext tag conf dresult -fore [lindex $diffcolors 1]
+    $ctext tag conf dresult -back [lindex $diffbgcolors 1]
     $ctext tag conf m0 -fore [lindex $mergecolors 0]
     $ctext tag conf m1 -fore [lindex $mergecolors 1]
     $ctext tag conf m2 -fore [lindex $mergecolors 2]
@@ -11607,6 +11609,7 @@ proc prefspage_general {notebook} {

 proc prefspage_colors {notebook} {
     global NS uicolor bgcolor fgcolor ctext diffcolors selectbgcolor markbgcolor
+    global diffbgcolors

     set page [create_prefs_page $notebook.colors]

@@ -11629,11 +11632,23 @@ proc prefspage_colors {notebook} {
 	-command [list choosecolor diffcolors 0 $page.diffold [mc "diff old lines"] \
 		      [list $ctext tag conf d0 -foreground]]
     grid x $page.diffoldbut $page.diffold -sticky w
+    label $page.diffoldbg -padx 40 -relief sunk -background [lindex $diffbgcolors 0]
+    ${NS}::button $page.diffoldbgbut -text [mc "Diff: old lines bg"] \
+	-command [list choosecolor diffbgcolors 0 $page.diffoldbg \
+		      [mc "diff old lines bg"] \
+		      [list $ctext tag conf d0 -background]]
+    grid x $page.diffoldbgbut $page.diffoldbg -sticky w
     label $page.diffnew -padx 40 -relief sunk -background [lindex $diffcolors 1]
     ${NS}::button $page.diffnewbut -text [mc "Diff: new lines"] \
 	-command [list choosecolor diffcolors 1 $page.diffnew [mc "diff new lines"] \
 		      [list $ctext tag conf dresult -foreground]]
     grid x $page.diffnewbut $page.diffnew -sticky w
+    label $page.diffnewbg -padx 40 -relief sunk -background [lindex $diffbgcolors 1]
+    ${NS}::button $page.diffnewbgbut -text [mc "Diff: new lines bg"] \
+	-command [list choosecolor diffbgcolors 1 $page.diffnewbg \
+		      [mc "diff new lines bg"] \
+		      [list $ctext tag conf dresult -background]]
+    grid x $page.diffnewbgbut $page.diffnewbg -sticky w
     label $page.hunksep -padx 40 -relief sunk -background [lindex $diffcolors 2]
     ${NS}::button $page.hunksepbut -text [mc "Diff: hunk header"] \
 	-command [list choosecolor diffcolors 2 $page.hunksep \
@@ -12377,7 +12392,8 @@ if {[tk windowingsystem] eq "win32"} {
 	set web_browser "xdg-open"
     }
 }
-set diffcolors {red "#00a000" blue}
+set diffcolors {"#c30000" "#009800" blue}
+set diffbgcolors {"#fff3f3" "#f0fff0"}
 set diffcontext 3
 set mergecolors {red blue "#00ff00" purple brown "#009090" magenta "#808000" "#009000" "#ff0080" cyan "#b07070" "#70b0f0" "#70f0b0" "#f0b070" "#ff70b0"}
 set ignorespace 0
@@ -12448,7 +12464,7 @@ set config_variables {
     remotebgcolor tagbgcolor tagfgcolor tagoutlinecolor reflinecolor
     filesepbgcolor filesepfgcolor linehoverbgcolor linehoverfgcolor
     linehoveroutlinecolor mainheadcirclecolor workingfilescirclecolor
-    indexcirclecolor circlecolors linkfgcolor circleoutlinecolor
+    indexcirclecolor circlecolors linkfgcolor circleoutlinecolor diffbgcolors
     web_browser
 }
 foreach var $config_variables {
--
2.24.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] gitk: add diff lines background colors
  2020-02-11 21:24 [PATCH] gitk: add diff lines background colors Stefan Dotterweich
@ 2020-04-07 16:42 ` Johannes Sixt
  2020-04-08 18:36   ` [PATCH] gitk: Un-hide selection in added and removed text and search results Johannes Sixt
  0 siblings, 1 reply; 4+ messages in thread
From: Johannes Sixt @ 2020-04-07 16:42 UTC (permalink / raw)
  To: Stefan Dotterweich; +Cc: git, Paul Mackerras

Am 11.02.20 um 22:24 schrieb Stefan Dotterweich:
> Not using colored background for added and removed lines is a missed
> opportunity to make diff lines easier to grasp visually.
> 
> Use a subtle red/green background by default. Make the font slightly darker
> to improve contrast.

I've been using gitk with this patch for a while, and I find the new
appearance *very* pleasing! It gives gitk a fresh, modern look.

There is one major gripe, though: the new background color overrides the
selection background and makes the selection invisible. This is a
showstopper.

Note that the search result highlight does not become invisible.

-- Hannes

> 
> Signed-off-by: Stefan Dotterweich <stefandotterweich@gmx.de>
> ---
> The variable diffcolors seems like a fitting place for the two new colors.
> However, adding them to that list causes problems if diffcolors saved in
> .gitk then contains three instead of five colors. This could be solved by
> modifying the variable after loading .gitk. I'm not sure if that would be
> the preferred approach or where to implement a special case like that. To
> avoid the problem, I introduced a new variable diffbgcolors.
> 
>  gitk | 22 +++++++++++++++++++---
>  1 file changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/gitk b/gitk
> index da84e22..66c237a 100755
> --- a/gitk
> +++ b/gitk
> @@ -2073,7 +2073,7 @@ proc makewindow {} {
>      global rowctxmenu fakerowmenu mergemax wrapcomment
>      global highlight_files gdttype
>      global searchstring sstring
> -    global bgcolor fgcolor bglist fglist diffcolors selectbgcolor
> +    global bgcolor fgcolor bglist fglist diffcolors diffbgcolors selectbgcolor
>      global uifgcolor uifgdisabledcolor
>      global filesepbgcolor filesepfgcolor
>      global mergecolors foundbgcolor currentsearchhitbgcolor
> @@ -2434,7 +2434,9 @@ proc makewindow {} {
>      $ctext tag conf filesep -font textfontbold -fore $filesepfgcolor -back $filesepbgcolor
>      $ctext tag conf hunksep -fore [lindex $diffcolors 2]
>      $ctext tag conf d0 -fore [lindex $diffcolors 0]
> +    $ctext tag conf d0 -back [lindex $diffbgcolors 0]
>      $ctext tag conf dresult -fore [lindex $diffcolors 1]
> +    $ctext tag conf dresult -back [lindex $diffbgcolors 1]
>      $ctext tag conf m0 -fore [lindex $mergecolors 0]
>      $ctext tag conf m1 -fore [lindex $mergecolors 1]
>      $ctext tag conf m2 -fore [lindex $mergecolors 2]
> @@ -11607,6 +11609,7 @@ proc prefspage_general {notebook} {
> 
>  proc prefspage_colors {notebook} {
>      global NS uicolor bgcolor fgcolor ctext diffcolors selectbgcolor markbgcolor
> +    global diffbgcolors
> 
>      set page [create_prefs_page $notebook.colors]
> 
> @@ -11629,11 +11632,23 @@ proc prefspage_colors {notebook} {
>  	-command [list choosecolor diffcolors 0 $page.diffold [mc "diff old lines"] \
>  		      [list $ctext tag conf d0 -foreground]]
>      grid x $page.diffoldbut $page.diffold -sticky w
> +    label $page.diffoldbg -padx 40 -relief sunk -background [lindex $diffbgcolors 0]
> +    ${NS}::button $page.diffoldbgbut -text [mc "Diff: old lines bg"] \
> +	-command [list choosecolor diffbgcolors 0 $page.diffoldbg \
> +		      [mc "diff old lines bg"] \
> +		      [list $ctext tag conf d0 -background]]
> +    grid x $page.diffoldbgbut $page.diffoldbg -sticky w
>      label $page.diffnew -padx 40 -relief sunk -background [lindex $diffcolors 1]
>      ${NS}::button $page.diffnewbut -text [mc "Diff: new lines"] \
>  	-command [list choosecolor diffcolors 1 $page.diffnew [mc "diff new lines"] \
>  		      [list $ctext tag conf dresult -foreground]]
>      grid x $page.diffnewbut $page.diffnew -sticky w
> +    label $page.diffnewbg -padx 40 -relief sunk -background [lindex $diffbgcolors 1]
> +    ${NS}::button $page.diffnewbgbut -text [mc "Diff: new lines bg"] \
> +	-command [list choosecolor diffbgcolors 1 $page.diffnewbg \
> +		      [mc "diff new lines bg"] \
> +		      [list $ctext tag conf dresult -background]]
> +    grid x $page.diffnewbgbut $page.diffnewbg -sticky w
>      label $page.hunksep -padx 40 -relief sunk -background [lindex $diffcolors 2]
>      ${NS}::button $page.hunksepbut -text [mc "Diff: hunk header"] \
>  	-command [list choosecolor diffcolors 2 $page.hunksep \
> @@ -12377,7 +12392,8 @@ if {[tk windowingsystem] eq "win32"} {
>  	set web_browser "xdg-open"
>      }
>  }
> -set diffcolors {red "#00a000" blue}
> +set diffcolors {"#c30000" "#009800" blue}
> +set diffbgcolors {"#fff3f3" "#f0fff0"}
>  set diffcontext 3
>  set mergecolors {red blue "#00ff00" purple brown "#009090" magenta "#808000" "#009000" "#ff0080" cyan "#b07070" "#70b0f0" "#70f0b0" "#f0b070" "#ff70b0"}
>  set ignorespace 0
> @@ -12448,7 +12464,7 @@ set config_variables {
>      remotebgcolor tagbgcolor tagfgcolor tagoutlinecolor reflinecolor
>      filesepbgcolor filesepfgcolor linehoverbgcolor linehoverfgcolor
>      linehoveroutlinecolor mainheadcirclecolor workingfilescirclecolor
> -    indexcirclecolor circlecolors linkfgcolor circleoutlinecolor
> +    indexcirclecolor circlecolors linkfgcolor circleoutlinecolor diffbgcolors
>      web_browser
>  }
>  foreach var $config_variables {
> --
> 2.24.1
> 
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH] gitk: Un-hide selection in added and removed text and search results
  2020-04-07 16:42 ` Johannes Sixt
@ 2020-04-08 18:36   ` Johannes Sixt
  2020-04-09 17:48     ` [PATCH v2] gitk: Un-hide selection in areas with non-default background color Johannes Sixt
  0 siblings, 1 reply; 4+ messages in thread
From: Johannes Sixt @ 2020-04-08 18:36 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: Stefan Dotterweich, git

The recently introduced background for the tags that highlight
added and removed text takes precedence over the background color
of the selection. But selected text is more important than the
highlighted text. Make the highlighting tags the lowest priority.

The same argument holds for the highlight of search results. But
search results are a bit more important. Therefore, make them
also low-priority, but just above the added-and-removed tags.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
Am 07.04.20 um 18:42 schrieb Johannes Sixt:
> There is one major gripe, though: the new background color overrides the
> selection background and makes the selection invisible. This is a
> showstopper.

Here is a fix for that on top of Stefan's patch. The patch text works
without Stefan's patch (and would un-hide the selection in search
results), but the commit message would have to be adjusted.

-- Hannes

 gitk | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/gitk b/gitk
index 4129c0ba95..d4dd9aca64 100755
--- a/gitk
+++ b/gitk
@@ -2468,6 +2468,11 @@ proc makewindow {} {
     $ctext tag conf currentsearchhit -back $currentsearchhitbgcolor
     $ctext tag conf wwrap -wrap word -lmargin2 1c
     $ctext tag conf bold -font textfontbold
+    # set these to the lowest priority:
+    $ctext tag lower currentsearchhit
+    $ctext tag lower found
+    $ctext tag lower dresult
+    $ctext tag lower d0
 
     .pwbottom add .bleft
     if {!$use_ttk} {
-- 
2.26.0.207.gdeb9c6cae9

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v2] gitk: Un-hide selection in areas with non-default background color
  2020-04-08 18:36   ` [PATCH] gitk: Un-hide selection in added and removed text and search results Johannes Sixt
@ 2020-04-09 17:48     ` Johannes Sixt
  0 siblings, 0 replies; 4+ messages in thread
From: Johannes Sixt @ 2020-04-09 17:48 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: Stefan Dotterweich, git

The recently introduced background for the tags that highlight
added and removed text takes precedence over the background color
of the selection. But selected text is more important than the
highlighted text. Make the highlighting tags the lowest priority.

The same argument holds for the file separator and the highlight
of search results. Therefore, make them also low-priority. But
search results are a bit more important; therefore, keep them
above the other tags.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
Here is an update for my earlier patch as I found that the file
separator also overrides the selection. This fixes it.

1:  7c4c862 ! 1:  3a0e64c gitk: Un-hide selection in added and removed text and search results
    @@ Metadata
     Author: Johannes Sixt <j6t@kdbg.org>
     
      ## Commit message ##
    -    gitk: Un-hide selection in added and removed text and search results
    +    gitk: Un-hide selection in areas with non-default background color
     
         The recently introduced background for the tags that highlight
         added and removed text takes precedence over the background color
         of the selection. But selected text is more important than the
         highlighted text. Make the highlighting tags the lowest priority.
     
    -    The same argument holds for the highlight of search results. But
    -    search results are a bit more important. Therefore, make them
    -    also low-priority, but just above the added-and-removed tags.
    +    The same argument holds for the file separator and the highlight
    +    of search results. Therefore, make them also low-priority. But
    +    search results are a bit more important; therefore, keep them
    +    above the other tags.
     
         Signed-off-by: Johannes Sixt <j6t@kdbg.org>
     
    @@ gitk: proc makewindow {} {
     +    # set these to the lowest priority:
     +    $ctext tag lower currentsearchhit
     +    $ctext tag lower found
    ++    $ctext tag lower filesep
     +    $ctext tag lower dresult
     +    $ctext tag lower d0
      
 gitk | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/gitk b/gitk
index 4129c0b..9ac6e5b 100755
--- a/gitk
+++ b/gitk
@@ -2468,6 +2468,12 @@ proc makewindow {} {
     $ctext tag conf currentsearchhit -back $currentsearchhitbgcolor
     $ctext tag conf wwrap -wrap word -lmargin2 1c
     $ctext tag conf bold -font textfontbold
+    # set these to the lowest priority:
+    $ctext tag lower currentsearchhit
+    $ctext tag lower found
+    $ctext tag lower filesep
+    $ctext tag lower dresult
+    $ctext tag lower d0
 
     .pwbottom add .bleft
     if {!$use_ttk} {
-- 
2.26.0.207.gdeb9c6cae9


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-04-09 17:48 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-11 21:24 [PATCH] gitk: add diff lines background colors Stefan Dotterweich
2020-04-07 16:42 ` Johannes Sixt
2020-04-08 18:36   ` [PATCH] gitk: Un-hide selection in added and removed text and search results Johannes Sixt
2020-04-09 17:48     ` [PATCH v2] gitk: Un-hide selection in areas with non-default background color Johannes Sixt

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