threads / patch / 44735

patch, 13 partsgitk: when processing tag labels, don't use `marks` as scratch space

Subject: [PATCH 01/13] gitk: when processing tag labels, don't use `marks` as scratch space

## tl;dr

24 messages between Dec 19, 2016 and Feb 6, 2017. Diffs are folded; open one to read it.

replies: 23people: 4as markdown or json

Michael Haggerty· Dec 19, 2016, 16:44 UTC · lore

[PATCH 00/13] gitk: tweak rendering of remote-tracking references

This patch series changes a bunch of details about how remote-tracking references are rendered in the commit list of gitk:

* Omit the "remote/" prefix on normal remote-tracking references. They
  are already distinguished via their two-tone rendering and (usually)
  longer names, and this change saves a lot of visual clutter and
  horizontal space.
* Render remote-tracking references that have more than the usual
  three slashes like
      origin/foo/bar
      ^^^^^^^
  rather than
      origin/foo/bar (formerly remotes/origin/foo/bar)
      ^^^^^^^^^^^              ^^^^^^^^^^^^^^^^^^^
  , where the indicated part is the prefix that is rendered in a
  different color. Usually, such a reference represents a remote
  branch that contains a slash in its name, so the new split more
  accurately portrays the separation between remote name and remote
  branch name.
* Introduce a separate constant to specify the background color used
  for the branch name part of remote-tracking references, to allow it
  to differ from the color used for local branches (which by default
  is bright green).
* Change the default background colors for remote-tracking branches to
  light brown and brown (formerly they were pale orange and bright
  green).

I understand that the colors of pixels on computer screens is an even more emotional topic that that of bikesheds, so I implemented the last change as a separate commit, the last one in the series. Feel free to drop it if you don't want the default color change.

Along the way, I did a bunch of refactoring in the area to make these changes easier, and introduced a constant for the background color of "other" references so that it can also be adjusted by users.

(Unfortunately, these colors can only be adjusted by editing the configuration file. Someday it would be nice to allow them to be configured via the preferences dialog.)

It's been a while since I've written any Tcl code, so I apologize in advance for any howlers :-)

This branch applies against the `master` branch in git://ozlabs.org/~paulus/gitk.

Michael
Michael Haggerty (13):
  gitk: when processing tag labels, don't use `marks` as scratch space
  gitk: keep track of tag types in a separate `types` array
  gitk: use a type "tags" to indicate abbreviated tags
  gitk: use a type "mainhead" to indicate the main HEAD branch
  gitk: fill in `wvals` as the tags are first processed
  gitk: simplify regexp
  gitk: extract a method `remotereftext`
  gitk: only change the color of the "remote" part of remote refs
  gitk: shorten labels displayed for modern remote-tracking refs
  gitk: use type "remote" for remote-tracking references
  gitk: introduce a constant otherrefbgcolor
  gitk: add a configuration setting `remoterefbgcolor`
  gitk: change the default colors for remote-tracking references
 gitk | 114 ++++++++++++++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 76 insertions(+), 38 deletions(-)
-- 
2.9.3
Michael Haggerty· Dec 19, 2016, 16:45 UTC · re: Michael Haggerty · lore
Instead, just append to it as necessary.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 gitk | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)
Show changes to gitk +8 −6
diff --git a/gitk b/gitk
index a14d7a1..296efb3 100755
--- a/gitk
+++ b/gitk
@@ -6570,18 +6570,20 @@ proc drawtags {id x xt y1} {
     set extra [expr {$delta + $lthickness + $linespc}]
 
     if {[info exists idtags($id)]} {
-	set marks $idtags($id)
-	set ntags [llength $marks]
+	set tags $idtags($id)
+	set ntags [llength $tags]
 	if {$ntags > $maxtags ||
-	    [totalwidth $marks mainfont $extra] > $maxwidth} {
+	    [totalwidth $tags mainfont $extra] > $maxwidth} {
 	    # show just a single "n tags..." tag
 	    set singletag 1
 	    if {$ntags == 1} {
-		set marks [list "tag..."]
+		lappend marks "tag..."
 	    } else {
-		set marks [list [format "%d tags..." $ntags]]
+		lappend marks [format "%d tags..." $ntags]
+		set ntags 1
 	    }
-	    set ntags 1
+	} else {
+	    set marks [concat $marks $tags]
 	}
     }
     if {[info exists idheads($id)]} {
-- 
2.9.3
Michael Haggerty· Dec 19, 2016, 16:45 UTC · re: Michael Haggerty · lore

[PATCH 02/13] gitk: keep track of tag types in a separate `types` array

The resulting code is easier to follow than the old counting-based code, plus in a moment we will add some more specific types.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 gitk | 37 +++++++++++++++++++++----------------
 1 file changed, 21 insertions(+), 16 deletions(-)
Show changes to gitk +21 −16
diff --git a/gitk b/gitk
index 296efb3..7c830c3 100755
--- a/gitk
+++ b/gitk
@@ -6560,8 +6560,7 @@ proc drawtags {id x xt y1} {
     global reflinecolor
 
     set marks {}
-    set ntags 0
-    set nheads 0
+    set types {}
     set singletag 0
     set maxtags 3
     set maxtagpct 25
@@ -6576,22 +6575,30 @@ proc drawtags {id x xt y1} {
 	    [totalwidth $tags mainfont $extra] > $maxwidth} {
 	    # show just a single "n tags..." tag
 	    set singletag 1
+	    lappend types tag
 	    if {$ntags == 1} {
 		lappend marks "tag..."
 	    } else {
 		lappend marks [format "%d tags..." $ntags]
-		set ntags 1
 	    }
 	} else {
-	    set marks [concat $marks $tags]
+	    foreach tag $tags {
+		lappend types tag
+		lappend marks $tag
+	    }
 	}
     }
     if {[info exists idheads($id)]} {
-	set marks [concat $marks $idheads($id)]
-	set nheads [llength $idheads($id)]
+	foreach head $idheads($id) {
+	    lappend types head
+	    lappend marks $head
+	}
     }
     if {[info exists idotherrefs($id)]} {
-	set marks [concat $marks $idotherrefs($id)]
+	foreach other $idotherrefs($id) {
+	    lappend types other
+	    lappend marks $other
+	}
     }
     if {$marks eq {}} {
 	return $xt
@@ -6601,10 +6608,8 @@ proc drawtags {id x xt y1} {
     set yb [expr {$yt + $linespc - 1}]
     set xvals {}
     set wvals {}
-    set i -1
-    foreach tag $marks {
-	incr i
-	if {$i >= $ntags && $i < $ntags + $nheads && $tag eq $mainhead} {
+    foreach tag $marks type $types {
+	if {$type eq "head" && $tag eq $mainhead} {
 	    set wid [font measure mainfontbold $tag]
 	} else {
 	    set wid [font measure mainfont $tag]
@@ -6616,12 +6621,12 @@ proc drawtags {id x xt y1} {
     set t [$canv create line $x $y1 [lindex $xvals end] $y1 \
 	       -width $lthickness -fill $reflinecolor -tags tag.$id]
     $canv lower $t
-    foreach tag $marks x $xvals wid $wvals {
+    foreach tag $marks type $types x $xvals wid $wvals {
 	set tag_quoted [string map {% %%} $tag]
 	set xl [expr {$x + $delta}]
 	set xr [expr {$x + $delta + $wid + $lthickness}]
 	set font mainfont
-	if {[incr ntags -1] >= 0} {
+	if {$type eq "tag"} {
 	    # draw a tag
 	    set t [$canv create polygon $x [expr {$yt + $delta}] $xl $yt \
 		       $xr $yt $xr $yb $xl $yb $x [expr {$yb - $delta}] \
@@ -6636,7 +6641,7 @@ proc drawtags {id x xt y1} {
 	    set rowtextx([rowofcommit $id]) [expr {$xr + $linespc}]
 	} else {
 	    # draw a head or other ref
-	    if {[incr nheads -1] >= 0} {
+	    if {$type eq "head"} {
 		set col $headbgcolor
 		if {$tag eq $mainhead} {
 		    set font mainfontbold
@@ -6658,9 +6663,9 @@ proc drawtags {id x xt y1} {
 	}
 	set t [$canv create text $xl $y1 -anchor w -text $tag -fill $headfgcolor \
 		   -font $font -tags [list tag.$id text]]
-	if {$ntags >= 0} {
+	if {$type eq "tag"} {
 	    $canv bind $t <1> $tagclick
-	} elseif {$nheads >= 0} {
+	} elseif {$type eq "head"} {
 	    $canv bind $t $ctxbut [list headmenu %X %Y $id $tag_quoted]
 	}
     }
-- 
2.9.3
Michael Haggerty· Dec 19, 2016, 16:45 UTC · re: Michael Haggerty · lore

[PATCH 12/13] gitk: add a configuration setting `remoterefbgcolor`

Remote-tracking references are very different than local branches, so it would be nice to make it possible to distinguish then visually. So allow the remote reference background color to be configured separately from the branch reference background color by introducing a new constant, `remoterefbgcolor`. For the moment, leave it set to the old default `headbgcolor`.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 gitk | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
Show changes to gitk +6 −3
diff --git a/gitk b/gitk
index 0bd4aa5..cb5c715 100755
--- a/gitk
+++ b/gitk
@@ -6574,7 +6574,7 @@ proc drawtags {id x xt y1} {
     global linespc lthickness
     global canv rowtextx curview fgcolor bgcolor ctxbut
     global headbgcolor headfgcolor headoutlinecolor
-    global remotebgcolor otherrefbgcolor
+    global remotebgcolor remoterefbgcolor otherrefbgcolor
     global tagbgcolor tagfgcolor tagoutlinecolor
     global reflinecolor
 
@@ -6669,8 +6669,10 @@ proc drawtags {id x xt y1} {
 	    if {$type eq "mainhead"} {
 		set col $headbgcolor
 		set font mainfontbold
-	    } elseif {$type eq "head" || $type eq "remote"} {
+	    } elseif {$type eq "head"} {
 		set col $headbgcolor
+	    } elseif {$type eq "remote"} {
+		set col $remoterefbgcolor
 	    } else {
 		set col $otherrefbgcolor
 	    }
@@ -12362,6 +12364,7 @@ set headbgcolor "#00ff00"
 set headfgcolor black
 set headoutlinecolor black
 set remotebgcolor #ffddaa
+set remoterefbgcolor #00ff00
 set otherrefbgcolor #ddddff
 set tagbgcolor yellow
 set tagfgcolor black
@@ -12420,7 +12423,7 @@ set config_variables {
     bgcolor fgcolor uifgcolor uifgdisabledcolor colors diffcolors mergecolors
     markbgcolor diffcontext selectbgcolor foundbgcolor currentsearchhitbgcolor
     extdifftool perfile_attrs headbgcolor headfgcolor headoutlinecolor
-    remotebgcolor otherrefbgcolor
+    remotebgcolor remoterefbgcolor otherrefbgcolor
     tagbgcolor tagfgcolor tagoutlinecolor reflinecolor
     filesepbgcolor filesepfgcolor linehoverbgcolor linehoverfgcolor
     linehoveroutlinecolor mainheadcirclecolor workingfilescirclecolor
-- 
2.9.3
Michael Haggerty· Dec 19, 2016, 16:45 UTC · re: Michael Haggerty · lore

[PATCH 07/13] gitk: extract a method `remotereftext`

The text value that it also provides to its caller is not used yet, but it will be soon.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 gitk | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)
Show changes to gitk +17 −1
diff --git a/gitk b/gitk
index 84a5326..51ebaf5 100755
--- a/gitk
+++ b/gitk
@@ -6551,6 +6551,22 @@ proc totalwidth {l font extra} {
     return $tot
 }
 
+# Set textName to the text that should be shown in the label for the
+# specified head and prefixName to the prefix text that should be
+# highlighted in $remotebgcolor. Return 1 iff $head is a remote head.
+proc remotereftext {head textName prefixName} {
+    upvar $textName text
+    upvar $prefixName prefix
+
+    if {[regexp {^((remotes/(.*/|)).*)} $head match text prefix]} {
+	return 1
+    } else {
+	set text $head
+	set prefix ""
+	return 0
+    }
+}
+
 proc drawtags {id x xt y1} {
     global idtags idheads idotherrefs mainhead
     global linespc lthickness
@@ -6654,7 +6670,7 @@ proc drawtags {id x xt y1} {
 	    set xl [expr {$xl - $delta/2}]
 	    $canv create polygon $x $yt $xr $yt $xr $yb $x $yb \
 		-width 1 -outline black -fill $col -tags tag.$id
-	    if {[regexp {^(remotes/(.*/|))} $tag match remoteprefix]} {
+	    if {[remotereftext $tag text remoteprefix]} {
 	        set rwid [font measure mainfont $remoteprefix]
 		set xi [expr {$x + 1}]
 		set yti [expr {$yt + 1}]
-- 
2.9.3
Michael Haggerty· Dec 19, 2016, 16:45 UTC · re: Michael Haggerty · lore

[PATCH 06/13] gitk: simplify regexp

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 gitk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to gitk +1 −1
diff --git a/gitk b/gitk
index fb2f653..84a5326 100755
--- a/gitk
+++ b/gitk
@@ -6654,7 +6654,7 @@ proc drawtags {id x xt y1} {
 	    set xl [expr {$xl - $delta/2}]
 	    $canv create polygon $x $yt $xr $yt $xr $yb $x $yb \
 		-width 1 -outline black -fill $col -tags tag.$id
-	    if {[regexp {^(remotes/.*/|remotes/)} $tag match remoteprefix]} {
+	    if {[regexp {^(remotes/(.*/|))} $tag match remoteprefix]} {
 	        set rwid [font measure mainfont $remoteprefix]
 		set xi [expr {$x + 1}]
 		set yti [expr {$yt + 1}]
-- 
2.9.3
Michael Haggerty· Dec 19, 2016, 16:45 UTC · re: Michael Haggerty · lore

[PATCH 04/13] gitk: use a type "mainhead" to indicate the main HEAD branch

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 gitk | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)
Show changes to gitk +11 −7
diff --git a/gitk b/gitk
index 502f0aa..d2e3803 100755
--- a/gitk
+++ b/gitk
@@ -6588,7 +6588,11 @@ proc drawtags {id x xt y1} {
     }
     if {[info exists idheads($id)]} {
 	foreach head $idheads($id) {
-	    lappend types head
+	    if {$head eq $mainhead} {
+		lappend types mainhead
+	    } else {
+		lappend types head
+	    }
 	    lappend marks $head
 	}
     }
@@ -6607,7 +6611,7 @@ proc drawtags {id x xt y1} {
     set xvals {}
     set wvals {}
     foreach tag $marks type $types {
-	if {$type eq "head" && $tag eq $mainhead} {
+	if {$type eq "mainhead"} {
 	    set wid [font measure mainfontbold $tag]
 	} else {
 	    set wid [font measure mainfont $tag]
@@ -6639,11 +6643,11 @@ proc drawtags {id x xt y1} {
 	    set rowtextx([rowofcommit $id]) [expr {$xr + $linespc}]
 	} else {
 	    # draw a head or other ref
-	    if {$type eq "head"} {
+	    if {$type eq "mainhead"} {
+		set col $headbgcolor
+		set font mainfontbold
+	    } elseif {$type eq "head"} {
 		set col $headbgcolor
-		if {$tag eq $mainhead} {
-		    set font mainfontbold
-		}
 	    } else {
 		set col "#ddddff"
 	    }
@@ -6663,7 +6667,7 @@ proc drawtags {id x xt y1} {
 		   -font $font -tags [list tag.$id text]]
 	if {$type eq "tag" || $type eq "tags"} {
 	    $canv bind $t <1> $tagclick
-	} elseif {$type eq "head"} {
+	} elseif {$type eq "head" || $type eq "mainhead"} {
 	    $canv bind $t $ctxbut [list headmenu %X %Y $id $tag_quoted]
 	}
     }
-- 
2.9.3
Michael Haggerty· Dec 19, 2016, 16:45 UTC · re: Michael Haggerty · lore

[PATCH 08/13] gitk: only change the color of the "remote" part of remote refs

If a remote-tracking reference has the form
    refs/remotes/origin/foo/bar

, then the part of the reference that tells which remote it comes from is `refs/remotes/origin`. So display such a reference as

    remotes/origin/foo/bar
    ^^^^^^^^^^^^^^^

, where the indicated part is displayed in `$remotebgcolor`. The old code always split the reference name at its last slash, thus rendering the above remote-tracking reference as

    remotes/origin/foo/bar
    ^^^^^^^^^^^^^^^^^^^
, which makes no sense at all.

Note that this commit doesn't change the rendering of remote-tracking references with only two slashes (e.g., `refs/remotes/foo`). Such references were created by `git-svn` when using its old naming scheme. They are still rendered like

    remotes/foo
    ^^^^^^^^
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 gitk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to gitk +1 −1
diff --git a/gitk b/gitk
index 51ebaf5..c146a15 100755
--- a/gitk
+++ b/gitk
@@ -6558,7 +6558,7 @@ proc remotereftext {head textName prefixName} {
     upvar $textName text
     upvar $prefixName prefix
 
-    if {[regexp {^((remotes/(.*/|)).*)} $head match text prefix]} {
+    if {[regexp {^((remotes/([^/]+/|)).*)} $head match text prefix]} {
 	return 1
     } else {
 	set text $head
-- 
2.9.3
Michael Haggerty· Dec 19, 2016, 16:45 UTC · re: Michael Haggerty · lore

[PATCH 05/13] gitk: fill in `wvals` as the tags are first processed

It's no longer, and a little bit more direct, to fill in `wvals` at the same time as we determine the tag's type.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 gitk | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)
Show changes to gitk +10 −10
diff --git a/gitk b/gitk
index d2e3803..fb2f653 100755
--- a/gitk
+++ b/gitk
@@ -6561,6 +6561,7 @@ proc drawtags {id x xt y1} {
 
     set marks {}
     set types {}
+    set wvals {}
     set maxtags 3
     set maxtagpct 25
     set maxwidth [expr {[graph_pane_width] * $maxtagpct / 100}]
@@ -6575,14 +6576,17 @@ proc drawtags {id x xt y1} {
 	    # show just a single "n tags..." tag
 	    lappend types tags
 	    if {$ntags == 1} {
-		lappend marks "tag..."
+		set text "tag..."
 	    } else {
-		lappend marks [format "%d tags..." $ntags]
+		set text [format "%d tags..." $ntags]
 	    }
+	    lappend marks $text
+	    lappend wvals [font measure mainfont $text]
 	} else {
 	    foreach tag $tags {
 		lappend types tag
 		lappend marks $tag
+		lappend wvals [font measure mainfont $tag]
 	    }
 	}
     }
@@ -6590,8 +6594,10 @@ proc drawtags {id x xt y1} {
 	foreach head $idheads($id) {
 	    if {$head eq $mainhead} {
 		lappend types mainhead
+		lappend wvals [font measure mainfontbold $head]
 	    } else {
 		lappend types head
+		lappend wvals [font measure mainfont $head]
 	    }
 	    lappend marks $head
 	}
@@ -6600,6 +6606,7 @@ proc drawtags {id x xt y1} {
 	foreach other $idotherrefs($id) {
 	    lappend types other
 	    lappend marks $other
+	    lappend wvals [font measure mainfont $other]
 	}
     }
     if {$marks eq {}} {
@@ -6609,15 +6616,8 @@ proc drawtags {id x xt y1} {
     set yt [expr {$y1 - 0.5 * $linespc}]
     set yb [expr {$yt + $linespc - 1}]
     set xvals {}
-    set wvals {}
-    foreach tag $marks type $types {
-	if {$type eq "mainhead"} {
-	    set wid [font measure mainfontbold $tag]
-	} else {
-	    set wid [font measure mainfont $tag]
-	}
+    foreach wid $wvals {
 	lappend xvals $xt
-	lappend wvals $wid
 	set xt [expr {$xt + $wid + $extra}]
     }
     set t [$canv create line $x $y1 [lindex $xvals end] $y1 \
-- 
2.9.3
Michael Haggerty· Dec 19, 2016, 16:45 UTC · re: Michael Haggerty · lore

[PATCH 09/13] gitk: shorten labels displayed for modern remote-tracking refs

If a reference has the form
    refs/remotes/origin/foo
, then use the label
    origin/foo
    ^^^^^^^

, where the indicated part is displayed in `$remotebgcolor`. The old code would have displayed such a reference as

    remotes/origin/foo
    ^^^^^^^^^^^^^^^

, which wastes horizontal space displaying `remote/` for every remote reference. However, if a remote-tracking reference has only two slashes, like

    refs/remotes/baz
, render the label the same as before, namely
    remotes/baz
    ^^^^^^^^
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 gitk | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)
Show changes to gitk +7 −3
diff --git a/gitk b/gitk
index c146a15..d22ce5f 100755
--- a/gitk
+++ b/gitk
@@ -6558,7 +6558,9 @@ proc remotereftext {head textName prefixName} {
     upvar $textName text
     upvar $prefixName prefix
 
-    if {[regexp {^((remotes/([^/]+/|)).*)} $head match text prefix]} {
+    if {[regexp {^remotes/(([^/]+/).*)} $head match text prefix]} {
+	return 1
+    } elseif {[regexp {^((remotes/).*)} $head match text prefix]} {
 	return 1
     } else {
 	set text $head
@@ -6613,7 +6615,8 @@ proc drawtags {id x xt y1} {
 		lappend wvals [font measure mainfontbold $head]
 	    } else {
 		lappend types head
-		lappend wvals [font measure mainfont $head]
+		remotereftext $head text remoteprefix
+		lappend wvals [font measure mainfont $text]
 	    }
 	    lappend marks $head
 	}
@@ -6644,6 +6647,7 @@ proc drawtags {id x xt y1} {
 	set xl [expr {$x + $delta}]
 	set xr [expr {$x + $delta + $wid + $lthickness}]
 	set font mainfont
+	set text $tag
 	if {$type eq "tag" || $type eq "tags"} {
 	    # draw a tag
 	    set t [$canv create polygon $x [expr {$yt + $delta}] $xl $yt \
@@ -6679,7 +6683,7 @@ proc drawtags {id x xt y1} {
 			-width 0 -fill $remotebgcolor -tags tag.$id
 	    }
 	}
-	set t [$canv create text $xl $y1 -anchor w -text $tag -fill $headfgcolor \
+	set t [$canv create text $xl $y1 -anchor w -text $text -fill $headfgcolor \
 		   -font $font -tags [list tag.$id text]]
 	if {$type eq "tag" || $type eq "tags"} {
 	    $canv bind $t <1> $tagclick
-- 
2.9.3
Michael Haggerty· Dec 19, 2016, 16:45 UTC · re: Michael Haggerty · lore

[PATCH 10/13] gitk: use type "remote" for remote-tracking references

This is clearer, and also lets us avoid calling `remotereftext` a second time for normal branches.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 gitk | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
Show changes to gitk +6 −3
diff --git a/gitk b/gitk
index d22ce5f..44f4d70 100755
--- a/gitk
+++ b/gitk
@@ -6613,9 +6613,11 @@ proc drawtags {id x xt y1} {
 	    if {$head eq $mainhead} {
 		lappend types mainhead
 		lappend wvals [font measure mainfontbold $head]
+	    } elseif {[remotereftext $head text remoteprefix]} {
+		lappend types remote
+		lappend wvals [font measure mainfont $text]
 	    } else {
 		lappend types head
-		remotereftext $head text remoteprefix
 		lappend wvals [font measure mainfont $text]
 	    }
 	    lappend marks $head
@@ -6666,7 +6668,7 @@ proc drawtags {id x xt y1} {
 	    if {$type eq "mainhead"} {
 		set col $headbgcolor
 		set font mainfontbold
-	    } elseif {$type eq "head"} {
+	    } elseif {$type eq "head" || $type eq "remote"} {
 		set col $headbgcolor
 	    } else {
 		set col "#ddddff"
@@ -6674,7 +6676,8 @@ proc drawtags {id x xt y1} {
 	    set xl [expr {$xl - $delta/2}]
 	    $canv create polygon $x $yt $xr $yt $xr $yb $x $yb \
 		-width 1 -outline black -fill $col -tags tag.$id
-	    if {[remotereftext $tag text remoteprefix]} {
+	    if {$type eq "remote"} {
+		remotereftext $tag text remoteprefix
 	        set rwid [font measure mainfont $remoteprefix]
 		set xi [expr {$x + 1}]
 		set yti [expr {$yt + 1}]
-- 
2.9.3
Michael Haggerty· Dec 19, 2016, 16:45 UTC · re: Michael Haggerty · lore

[PATCH 11/13] gitk: introduce a constant otherrefbgcolor

This is the value used for references other than tags, heads, and remote-tracking references (e.g., `refs/stash`).

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 gitk | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
Show changes to gitk +6 −3
diff --git a/gitk b/gitk
index 44f4d70..0bd4aa5 100755
--- a/gitk
+++ b/gitk
@@ -6573,7 +6573,8 @@ proc drawtags {id x xt y1} {
     global idtags idheads idotherrefs mainhead
     global linespc lthickness
     global canv rowtextx curview fgcolor bgcolor ctxbut
-    global headbgcolor headfgcolor headoutlinecolor remotebgcolor
+    global headbgcolor headfgcolor headoutlinecolor
+    global remotebgcolor otherrefbgcolor
     global tagbgcolor tagfgcolor tagoutlinecolor
     global reflinecolor
 
@@ -6671,7 +6672,7 @@ proc drawtags {id x xt y1} {
 	    } elseif {$type eq "head" || $type eq "remote"} {
 		set col $headbgcolor
 	    } else {
-		set col "#ddddff"
+		set col $otherrefbgcolor
 	    }
 	    set xl [expr {$xl - $delta/2}]
 	    $canv create polygon $x $yt $xr $yt $xr $yb $x $yb \
@@ -12361,6 +12362,7 @@ set headbgcolor "#00ff00"
 set headfgcolor black
 set headoutlinecolor black
 set remotebgcolor #ffddaa
+set otherrefbgcolor #ddddff
 set tagbgcolor yellow
 set tagfgcolor black
 set tagoutlinecolor black
@@ -12418,7 +12420,8 @@ set config_variables {
     bgcolor fgcolor uifgcolor uifgdisabledcolor colors diffcolors mergecolors
     markbgcolor diffcontext selectbgcolor foundbgcolor currentsearchhitbgcolor
     extdifftool perfile_attrs headbgcolor headfgcolor headoutlinecolor
-    remotebgcolor tagbgcolor tagfgcolor tagoutlinecolor reflinecolor
+    remotebgcolor otherrefbgcolor
+    tagbgcolor tagfgcolor tagoutlinecolor reflinecolor
     filesepbgcolor filesepfgcolor linehoverbgcolor linehoverfgcolor
     linehoveroutlinecolor mainheadcirclecolor workingfilescirclecolor
     indexcirclecolor circlecolors linkfgcolor circleoutlinecolor
-- 
2.9.3
Michael Haggerty· Dec 19, 2016, 16:45 UTC · re: Michael Haggerty · lore

[PATCH 13/13] gitk: change the default colors for remote-tracking references

Instead of coloring the reference name part of remote-tracking references the same bright green as used for local branches, change it to a subtler brown. Change the remote name color to go better with the brown.

I chose these colors because
* Remote-tracking references are less important than local branches, so
  it is appropriate that their coloring be subtler.
* Brown isn't used elsewhere.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 gitk | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Show changes to gitk +2 −2
diff --git a/gitk b/gitk
index cb5c715..be97640 100755
--- a/gitk
+++ b/gitk
@@ -12363,8 +12363,8 @@ set markbgcolor "#e0e0ff"
 set headbgcolor "#00ff00"
 set headfgcolor black
 set headoutlinecolor black
-set remotebgcolor #ffddaa
-set remoterefbgcolor #00ff00
+set remotebgcolor #d9d4b2
+set remoterefbgcolor #c1a677
 set otherrefbgcolor #ddddff
 set tagbgcolor yellow
 set tagfgcolor black
-- 
2.9.3
Michael Haggerty· Dec 19, 2016, 16:45 UTC · re: Michael Haggerty · lore

[PATCH 03/13] gitk: use a type "tags" to indicate abbreviated tags

This replaces the functionality of the old `singletag` variable.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 gitk | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)
Show changes to gitk +4 −6
diff --git a/gitk b/gitk
index 7c830c3..502f0aa 100755
--- a/gitk
+++ b/gitk
@@ -6561,7 +6561,6 @@ proc drawtags {id x xt y1} {
 
     set marks {}
     set types {}
-    set singletag 0
     set maxtags 3
     set maxtagpct 25
     set maxwidth [expr {[graph_pane_width] * $maxtagpct / 100}]
@@ -6574,8 +6573,7 @@ proc drawtags {id x xt y1} {
 	if {$ntags > $maxtags ||
 	    [totalwidth $tags mainfont $extra] > $maxwidth} {
 	    # show just a single "n tags..." tag
-	    set singletag 1
-	    lappend types tag
+	    lappend types tags
 	    if {$ntags == 1} {
 		lappend marks "tag..."
 	    } else {
@@ -6626,13 +6624,13 @@ proc drawtags {id x xt y1} {
 	set xl [expr {$x + $delta}]
 	set xr [expr {$x + $delta + $wid + $lthickness}]
 	set font mainfont
-	if {$type eq "tag"} {
+	if {$type eq "tag" || $type eq "tags"} {
 	    # draw a tag
 	    set t [$canv create polygon $x [expr {$yt + $delta}] $xl $yt \
 		       $xr $yt $xr $yb $xl $yb $x [expr {$yb - $delta}] \
 		       -width 1 -outline $tagoutlinecolor -fill $tagbgcolor \
 		       -tags tag.$id]
-	    if {$singletag} {
+	    if {$type eq "tags"} {
 		set tagclick [list showtags $id 1]
 	    } else {
 		set tagclick [list showtag $tag_quoted 1]
@@ -6663,7 +6661,7 @@ proc drawtags {id x xt y1} {
 	}
 	set t [$canv create text $xl $y1 -anchor w -text $tag -fill $headfgcolor \
 		   -font $font -tags [list tag.$id text]]
-	if {$type eq "tag"} {
+	if {$type eq "tag" || $type eq "tags"} {
 	    $canv bind $t <1> $tagclick
 	} elseif {$type eq "head"} {
 	    $canv bind $t $ctxbut [list headmenu %X %Y $id $tag_quoted]
-- 
2.9.3
Philip Oakley· Dec 19, 2016, 22:54 UTC · re: Michael Haggerty · lore

Re: [PATCH 00/13] gitk: tweak rendering of remote-tracking references

From: "Michael Haggerty" <mhagger@alum.mit.edu>
> This patch series changes a bunch of details about how remote-tracking
> references are rendered in the commit list of gitk:
>
[...]
Show 14 quoted lines
> * Introduce a separate constant to specify the background color used
>  for the branch name part of remote-tracking references, to allow it
>  to differ from the color used for local branches (which by default
>  is bright green).
>
> * Change the default background colors for remote-tracking branches to
>  light brown and brown (formerly they were pale orange and bright
>  green).
>
> I understand that the colors of pixels on computer screens is an even
> more emotional topic that that of bikesheds, so I implemented the last
> change as a separate commit, the last one in the series. Feel free to
> drop it if you don't want the default color change.
>

Just to say that there was an issue with the bright green (lime) a while back when 'green' changed its colour.

dscho reported in (https://github.com/git-for-windows/git/issues/300#issuecomment-133702654 26 Aug 2015)

"[T]his is a change in Tk 8.6 described here (http://wiki.tcl.tk/1424): From Tcl/Tk 8.6 on, Tk uses Web colours instead of X11 ones, where they conflict."

In particular the old bright green version of 'green' became a darker green, with the old colour becoming named lime.

For me, I needed to change my colour scheme (to a lime) as I could not read the refs against the darker colour.

Anyway, that's the background as I know it.

-- Philip

Michael Haggerty· Dec 20, 2016, 08:03 UTC · re: Philip Oakley · lore

Re: [PATCH 00/13] gitk: tweak rendering of remote-tracking references

On 12/19/2016 11:53 PM, Philip Oakley wrote:
Show 39 quoted lines
> From: "Michael Haggerty" <mhagger@alum.mit.edu>
>> This patch series changes a bunch of details about how remote-tracking
>> references are rendered in the commit list of gitk:
>>
> [...]
>> * Introduce a separate constant to specify the background color used
>>  for the branch name part of remote-tracking references, to allow it
>>  to differ from the color used for local branches (which by default
>>  is bright green).
>>
>> * Change the default background colors for remote-tracking branches to
>>  light brown and brown (formerly they were pale orange and bright
>>  green).
>>
>> I understand that the colors of pixels on computer screens is an even
>> more emotional topic that that of bikesheds, so I implemented the last
>> change as a separate commit, the last one in the series. Feel free to
>> drop it if you don't want the default color change.
> 
> Just to say that there was an issue with the bright green (lime) a while
> back when 'green' changed its colour.
> 
> dscho reported in
> (https://github.com/git-for-windows/git/issues/300#issuecomment-133702654
> 26 Aug 2015)
> 
> "[T]his is a change in Tk 8.6 described here (http://wiki.tcl.tk/1424):
> From
> Tcl/Tk 8.6 on, Tk uses Web colours instead of X11 ones, where they
> conflict."
> 
> In particular the old bright green version of 'green' became a darker
> green,
> with the old colour becoming named lime.
> 
> For me, I needed to change my colour scheme (to a lime) as I could not read
> the refs against the darker colour.
> 
> Anyway, that's the background as I know it.

Thanks for the context. In fact, it was this color change that got me looking at this code in the first place.

For anyone still suffering from the "green background of branch labels in gitk got too dark" problem:

The default color in gitk was made lighter again in
    66db14c "gitk: Color name update", 2015-10-25

But your own configuration might still have "green" written in it, from the days when "green" was still bright. If so, change the value for `headbgcolor` to `lime` in `~/.config/git/gitk`.

It would be nice if these colors were adjustable via the "Preferences -> Colors" dialog.

Michael
Marc Branchaud· Dec 20, 2016, 15:01 UTC · re: Michael Haggerty · lore

Re: [PATCH 00/13] gitk: tweak rendering of remote-tracking references

On 2016-12-19 11:44 AM, Michael Haggerty wrote:
> This patch series changes a bunch of details about how remote-tracking
> references are rendered in the commit list of gitk:
Thanks for this!  I like the new, compact look very much!

That said, I remember when I was a new git user and I leaned heavily on gitk to understand how references worked. It was particularly illuminating to see the remote references distinctly labeled, and the fact that they were "remotes/origin/foo" gave me an Aha! moment where I came to understand that the refs hierarchy is more flexible than just the conventions coded into git itself. I eventually felt free to create my own, private ref hierarchies.

I am in no way opposed to this series. I just wanted to point out that there was some utility in those labels. It makes me think that it might be worthwhile for gitk to have a "raw-refs" mode, that shows the full "refs/foo/bar/baz" paths of all the heads, tags, and whatever else. It could be a useful teaching tool for git.

> * Omit the "remote/" prefix on normal remote-tracking references. They
If you re-roll, s:remote/:remotes/:.
Show 20 quoted lines
>   are already distinguished via their two-tone rendering and (usually)
>   longer names, and this change saves a lot of visual clutter and
>   horizontal space.
>
> * Render remote-tracking references that have more than the usual
>   three slashes like
>
>       origin/foo/bar
>       ^^^^^^^
>
>   rather than
>
>       origin/foo/bar (formerly remotes/origin/foo/bar)
>       ^^^^^^^^^^^              ^^^^^^^^^^^^^^^^^^^
>
>   , where the indicated part is the prefix that is rendered in a
>   different color. Usually, such a reference represents a remote
>   branch that contains a slash in its name, so the new split more
>   accurately portrays the separation between remote name and remote
>   branch name.
*Love* this change!  :)
Show 8 quoted lines
> * Introduce a separate constant to specify the background color used
>   for the branch name part of remote-tracking references, to allow it
>   to differ from the color used for local branches (which by default
>   is bright green).
>
> * Change the default background colors for remote-tracking branches to
>   light brown and brown (formerly they were pale orange and bright
>   green).
Please don't change the remotebgcolor default.
Also, perhaps the default remoterefbgcolor should be
	set remoterefbgcolor $headbgcolor
?

I say this because when I applied the series, without the last patch, I was miffed that the remote/ref colour had changed.

Plus I think there's utility in having local and remote branch names in the same colour, again especially for new users. It helps emphasize their similarities, and demystify some of git's internal magic.

		M.
Show 39 quoted lines
> I understand that the colors of pixels on computer screens is an even
> more emotional topic that that of bikesheds, so I implemented the last
> change as a separate commit, the last one in the series. Feel free to
> drop it if you don't want the default color change.
>
> Along the way, I did a bunch of refactoring in the area to make these
> changes easier, and introduced a constant for the background color of
> "other" references so that it can also be adjusted by users.
>
> (Unfortunately, these colors can only be adjusted by editing the
> configuration file. Someday it would be nice to allow them to be
> configured via the preferences dialog.)
>
> It's been a while since I've written any Tcl code, so I apologize in
> advance for any howlers :-)
>
> This branch applies against the `master` branch in
> git://ozlabs.org/~paulus/gitk.
>
> Michael
>
> Michael Haggerty (13):
>   gitk: when processing tag labels, don't use `marks` as scratch space
>   gitk: keep track of tag types in a separate `types` array
>   gitk: use a type "tags" to indicate abbreviated tags
>   gitk: use a type "mainhead" to indicate the main HEAD branch
>   gitk: fill in `wvals` as the tags are first processed
>   gitk: simplify regexp
>   gitk: extract a method `remotereftext`
>   gitk: only change the color of the "remote" part of remote refs
>   gitk: shorten labels displayed for modern remote-tracking refs
>   gitk: use type "remote" for remote-tracking references
>   gitk: introduce a constant otherrefbgcolor
>   gitk: add a configuration setting `remoterefbgcolor`
>   gitk: change the default colors for remote-tracking references
>
>  gitk | 114 ++++++++++++++++++++++++++++++++++++++++++++-----------------------
>  1 file changed, 76 insertions(+), 38 deletions(-)
>
Paul Mackerras· Dec 20, 2016, 22:17 UTC · re: Marc Branchaud · lore

Re: [PATCH 00/13] gitk: tweak rendering of remote-tracking references

On Tue, Dec 20, 2016 at 10:01:15AM -0500, Marc Branchaud wrote:
Show 18 quoted lines
> On 2016-12-19 11:44 AM, Michael Haggerty wrote:
> >This patch series changes a bunch of details about how remote-tracking
> >references are rendered in the commit list of gitk:
> 
> Thanks for this!  I like the new, compact look very much!
> 
> That said, I remember when I was a new git user and I leaned heavily on gitk
> to understand how references worked.  It was particularly illuminating to
> see the remote references distinctly labeled, and the fact that they were
> "remotes/origin/foo" gave me an Aha! moment where I came to understand that
> the refs hierarchy is more flexible than just the conventions coded into git
> itself.  I eventually felt free to create my own, private ref hierarchies.
> 
> I am in no way opposed to this series.  I just wanted to point out that
> there was some utility in those labels.  It makes me think that it might be
> worthwhile for gitk to have a "raw-refs" mode, that shows the full
> "refs/foo/bar/baz" paths of all the heads, tags, and whatever else.  It
> could be a useful teaching tool for git.

Do you think we should have a checkbox in the preferences dialog to select whether to display the long form or the short form?

Paul.
Marc Branchaud· Dec 20, 2016, 22:53 UTC · re: Paul Mackerras · lore

Re: [PATCH 00/13] gitk: tweak rendering of remote-tracking references

On 2016-12-20 05:17 PM, Paul Mackerras wrote:
Show 22 quoted lines
> On Tue, Dec 20, 2016 at 10:01:15AM -0500, Marc Branchaud wrote:
>> On 2016-12-19 11:44 AM, Michael Haggerty wrote:
>>> This patch series changes a bunch of details about how remote-tracking
>>> references are rendered in the commit list of gitk:
>>
>> Thanks for this!  I like the new, compact look very much!
>>
>> That said, I remember when I was a new git user and I leaned heavily on gitk
>> to understand how references worked.  It was particularly illuminating to
>> see the remote references distinctly labeled, and the fact that they were
>> "remotes/origin/foo" gave me an Aha! moment where I came to understand that
>> the refs hierarchy is more flexible than just the conventions coded into git
>> itself.  I eventually felt free to create my own, private ref hierarchies.
>>
>> I am in no way opposed to this series.  I just wanted to point out that
>> there was some utility in those labels.  It makes me think that it might be
>> worthwhile for gitk to have a "raw-refs" mode, that shows the full
>> "refs/foo/bar/baz" paths of all the heads, tags, and whatever else.  It
>> could be a useful teaching tool for git.
>
> Do you think we should have a checkbox in the preferences dialog to
> select whether to display the long form or the short form?
Mmmm, more knobs!

No, I don't think that's necessary. Such a setting would probably just confuse people. Plus it's not something anyone is likely to want to change anyway. Maybe if there were an "Advanced" tab in the settings dialog, but even then it seems like overkill.

I suspect there are better ways to teach people about the refs hierarchy than cluttering up gitk. These may even already exist -- I've been a git user for 8 years now, so I'm sure my perspective of the learning curve is skewed.

		M.
Michael Haggerty· Dec 21, 2016, 00:05 UTC · re: Marc Branchaud · lore

Re: [PATCH 00/13] gitk: tweak rendering of remote-tracking references

On 12/20/2016 04:01 PM, Marc Branchaud wrote:
Show 19 quoted lines
> On 2016-12-19 11:44 AM, Michael Haggerty wrote:
>> This patch series changes a bunch of details about how remote-tracking
>> references are rendered in the commit list of gitk:
> 
> Thanks for this!  I like the new, compact look very much!
> 
> That said, I remember when I was a new git user and I leaned heavily on
> gitk to understand how references worked.  It was particularly
> illuminating to see the remote references distinctly labeled, and the
> fact that they were "remotes/origin/foo" gave me an Aha! moment where I
> came to understand that the refs hierarchy is more flexible than just
> the conventions coded into git itself.  I eventually felt free to create
> my own, private ref hierarchies.
> 
> I am in no way opposed to this series.  I just wanted to point out that
> there was some utility in those labels.  It makes me think that it might
> be worthwhile for gitk to have a "raw-refs" mode, that shows the full
> "refs/foo/bar/baz" paths of all the heads, tags, and whatever else.  It
> could be a useful teaching tool for git.

Yes, I understand that the longer names might be useful for beginners, and the full names even more so. However, I think once a user has that "aha!" moment, the space wasted on all the redundant words is a real impediment to gitk's usability. It is common to have a few references on a single commit (especially if you pull from multiple remotes), in which case the summary line is completely invisible (and therefore its context menu is unreachable). I don't like the idea of dumbing down the UI permanently based on what users need at the very beginning of their Git usage.

Would it be possible to use the short names in the UI but to add the full names to a tooltip or to the context menu?

>> * Omit the "remote/" prefix on normal remote-tracking references. They
> 
> If you re-roll, s:remote/:remotes/:.
Thanks.
Show 40 quoted lines
>>   are already distinguished via their two-tone rendering and (usually)
>>   longer names, and this change saves a lot of visual clutter and
>>   horizontal space.
>>
>> * Render remote-tracking references that have more than the usual
>>   three slashes like
>>
>>       origin/foo/bar
>>       ^^^^^^^
>>
>>   rather than
>>
>>       origin/foo/bar (formerly remotes/origin/foo/bar)
>>       ^^^^^^^^^^^              ^^^^^^^^^^^^^^^^^^^
>>
>>   , where the indicated part is the prefix that is rendered in a
>>   different color. Usually, such a reference represents a remote
>>   branch that contains a slash in its name, so the new split more
>>   accurately portrays the separation between remote name and remote
>>   branch name.
> 
> *Love* this change!  :)
> 
>> * Introduce a separate constant to specify the background color used
>>   for the branch name part of remote-tracking references, to allow it
>>   to differ from the color used for local branches (which by default
>>   is bright green).
>>
>> * Change the default background colors for remote-tracking branches to
>>   light brown and brown (formerly they were pale orange and bright
>>   green).
> 
> Please don't change the remotebgcolor default.
> 
> Also, perhaps the default remoterefbgcolor should be
>     set remoterefbgcolor $headbgcolor
> ?
> 
> I say this because when I applied the series, without the last patch, I
> was miffed that the remote/ref colour had changed.

This is a one-time inconvenience that gitk developers will experience. I doubt that users jump backwards and forwards in gitk versions very often.

I do find it strange that gitk writes a color selection to its configuration file *even if the user has left it at its default*. Normally I would expect only user-changed settings to be written to the config file, and other values to use the default that is built into the program. For example, such an approach would have made the transition from "green" to "lime" easier.

> [...]

Thanks for your feedback! Michael

Marc Branchaud· Dec 21, 2016, 19:07 UTC · re: Michael Haggerty · lore

Re: [PATCH 00/13] gitk: tweak rendering of remote-tracking references

On 2016-12-20 07:05 PM, Michael Haggerty wrote:
Show 30 quoted lines
> On 12/20/2016 04:01 PM, Marc Branchaud wrote:
>> On 2016-12-19 11:44 AM, Michael Haggerty wrote:
>>> This patch series changes a bunch of details about how remote-tracking
>>> references are rendered in the commit list of gitk:
>>
>> Thanks for this!  I like the new, compact look very much!
>>
>> That said, I remember when I was a new git user and I leaned heavily on
>> gitk to understand how references worked.  It was particularly
>> illuminating to see the remote references distinctly labeled, and the
>> fact that they were "remotes/origin/foo" gave me an Aha! moment where I
>> came to understand that the refs hierarchy is more flexible than just
>> the conventions coded into git itself.  I eventually felt free to create
>> my own, private ref hierarchies.
>>
>> I am in no way opposed to this series.  I just wanted to point out that
>> there was some utility in those labels.  It makes me think that it might
>> be worthwhile for gitk to have a "raw-refs" mode, that shows the full
>> "refs/foo/bar/baz" paths of all the heads, tags, and whatever else.  It
>> could be a useful teaching tool for git.
>
> Yes, I understand that the longer names might be useful for beginners,
> and the full names even more so. However, I think once a user has that
> "aha!" moment, the space wasted on all the redundant words is a real
> impediment to gitk's usability. It is common to have a few references on
> a single commit (especially if you pull from multiple remotes), in which
> case the summary line is completely invisible (and therefore its context
> menu is unreachable). I don't like the idea of dumbing down the UI
> permanently based on what users need at the very beginning of their Git
> usage.
I agree.
> Would it be possible to use the short names in the UI but to add the
> full names to a tooltip or to the context menu?
I don't know -- my Tk-fu is weak.
Show 49 quoted lines
>>> * Omit the "remote/" prefix on normal remote-tracking references. They
>>
>> If you re-roll, s:remote/:remotes/:.
>
> Thanks.
>
>>>   are already distinguished via their two-tone rendering and (usually)
>>>   longer names, and this change saves a lot of visual clutter and
>>>   horizontal space.
>>>
>>> * Render remote-tracking references that have more than the usual
>>>   three slashes like
>>>
>>>       origin/foo/bar
>>>       ^^^^^^^
>>>
>>>   rather than
>>>
>>>       origin/foo/bar (formerly remotes/origin/foo/bar)
>>>       ^^^^^^^^^^^              ^^^^^^^^^^^^^^^^^^^
>>>
>>>   , where the indicated part is the prefix that is rendered in a
>>>   different color. Usually, such a reference represents a remote
>>>   branch that contains a slash in its name, so the new split more
>>>   accurately portrays the separation between remote name and remote
>>>   branch name.
>>
>> *Love* this change!  :)
>>
>>> * Introduce a separate constant to specify the background color used
>>>   for the branch name part of remote-tracking references, to allow it
>>>   to differ from the color used for local branches (which by default
>>>   is bright green).
>>>
>>> * Change the default background colors for remote-tracking branches to
>>>   light brown and brown (formerly they were pale orange and bright
>>>   green).
>>
>> Please don't change the remotebgcolor default.
>>
>> Also, perhaps the default remoterefbgcolor should be
>>     set remoterefbgcolor $headbgcolor
>> ?
>>
>> I say this because when I applied the series, without the last patch, I
>> was miffed that the remote/ref colour had changed.
>
> This is a one-time inconvenience that gitk developers will experience. I
> doubt that users jump backwards and forwards in gitk versions very often.
In what way do you mean it's restricted to gitk developers?

Patch 12 introduces remoterefbgcolor, with a specific default value. Prior to that, the "ref part" of remote refs was rendered with headbgcolor. Users who changed their headbgcolor are used to seeing the "ref part" of remote refs in the same color as their local heads. Applying patch 12 changes the "ref part" color of remote refs, for such users.

All I'm saying is that make the remoterefbgcolor default be $headbgcolor avoids this.

But, honestly, I don't feel strongly about it.
		M.
Michael Haggerty· Dec 22, 2016, 08:15 UTC · re: Marc Branchaud · lore

Re: [PATCH 00/13] gitk: tweak rendering of remote-tracking references

On 12/21/2016 08:07 PM, Marc Branchaud wrote:
Show 16 quoted lines
> On 2016-12-20 07:05 PM, Michael Haggerty wrote:
>> On 12/20/2016 04:01 PM, Marc Branchaud wrote:
>>> [...]
>>> Please don't change the remotebgcolor default.
>>>
>>> Also, perhaps the default remoterefbgcolor should be
>>>     set remoterefbgcolor $headbgcolor
>>> ?
>>>
>>> I say this because when I applied the series, without the last patch, I
>>> was miffed that the remote/ref colour had changed.
>>
>> This is a one-time inconvenience that gitk developers will experience. I
>> doubt that users jump backwards and forwards in gitk versions very often.
> 
> In what way do you mean it's restricted to gitk developers?
Maybe I misunderstood your objection.

While developing this, I realized that the very first time your run gitk (i.e., when you don't already have a configuration file), it writes the then-current default colors into your configuration file. If you later switch gitk versions to a version with different default colors, the colors from the first-run version are preserved (unless the names of the variables used to hold the colors are changed).

So if you would run the tip version of my branch first, then the parent of that version, you would continue to see the colors as modified by the final commit. If you then switch to the master version, the remote branch names go back to green (because it goes back to using `headbgcolor` again), but the remote prefix stays light brown. I thought you were unhappy about some form of this unexpected persistence problem. But this problem will mostly be experienced by gitk developers who jump back and forth between versions.

A normal user probably mostly moves forward through version numbers, and only occasionally. Such a user, jumping from master to the tip of my branch (assuming they haven't customized anything), would see

* local branches stay lime
* remote branch prefixes stay pale orange (they don't change to light
brown because the pale orange color from master is stored in their
configuration file)
* remote branch names change from lime to brown (because the
`remoterefbgcolor` configuration setting didn't exist before)
Show 9 quoted lines
> Patch 12 introduces remoterefbgcolor, with a specific default value.
> Prior to that, the "ref part" of remote refs was rendered with
> headbgcolor.  Users who changed their headbgcolor are used to seeing the
> "ref part" of remote refs in the same color as their local heads.
> Applying patch 12 changes the "ref part" color of remote refs, for such
> users.
> 
> All I'm saying is that make the remoterefbgcolor default be $headbgcolor
> avoids this.

For somebody who thinks that most people will want local and remote-tracking branch names to be rendered in the same color, your suggestion would be an improvement.

But for somebody like me who thinks it is a good idea to render remote-tracking branch names in a different color than local branch names, this is a feature, not a bug. Even a user who explicitly configured `headbgcolor` to, say, purple, wasn't necessarily expressing a wish to have remote-tracking branch names rendered in purple. Until now they had no choice to set these colors separately!

So even for somebody who configured this setting before, I think that having the remote-tracking branch names change color when the user upgrades to this version is a good thing, because it lets the user know that these two things can now be colored independently. If they don't like the new default brown color, such a user knows where to change it (even to make it agree with `headbgcolor` if they want that).

But I understand that this is a matter of personal preference. I have but one "vote" :-)

Michael
Marc Branchaud· Jan 9, 2017, 22:03 UTC · re: Michael Haggerty · lore

Re: [PATCH 00/13] gitk: tweak rendering of remote-tracking references

On 2016-12-22 03:15 AM, Michael Haggerty wrote:
Show 77 quoted lines
> On 12/21/2016 08:07 PM, Marc Branchaud wrote:
>> On 2016-12-20 07:05 PM, Michael Haggerty wrote:
>>> On 12/20/2016 04:01 PM, Marc Branchaud wrote:
>>>> [...]
>>>> Please don't change the remotebgcolor default.
>>>>
>>>> Also, perhaps the default remoterefbgcolor should be
>>>>     set remoterefbgcolor $headbgcolor
>>>> ?
>>>>
>>>> I say this because when I applied the series, without the last patch, I
>>>> was miffed that the remote/ref colour had changed.
>>>
>>> This is a one-time inconvenience that gitk developers will experience. I
>>> doubt that users jump backwards and forwards in gitk versions very often.
>>
>> In what way do you mean it's restricted to gitk developers?
>
> Maybe I misunderstood your objection.
>
> While developing this, I realized that the very first time your run gitk
> (i.e., when you don't already have a configuration file), it writes the
> then-current default colors into your configuration file. If you later
> switch gitk versions to a version with different default colors, the
> colors from the first-run version are preserved (unless the names of the
> variables used to hold the colors are changed).
>
> So if you would run the tip version of my branch first, then the parent
> of that version, you would continue to see the colors as modified by the
> final commit. If you then switch to the master version, the remote
> branch names go back to green (because it goes back to using
> `headbgcolor` again), but the remote prefix stays light brown. I thought
> you were unhappy about some form of this unexpected persistence problem.
> But this problem will mostly be experienced by gitk developers who jump
> back and forth between versions.
>
> A normal user probably mostly moves forward through version numbers, and
> only occasionally. Such a user, jumping from master to the tip of my
> branch (assuming they haven't customized anything), would see
>
> * local branches stay lime
> * remote branch prefixes stay pale orange (they don't change to light
> brown because the pale orange color from master is stored in their
> configuration file)
> * remote branch names change from lime to brown (because the
> `remoterefbgcolor` configuration setting didn't exist before)
>
>> Patch 12 introduces remoterefbgcolor, with a specific default value.
>> Prior to that, the "ref part" of remote refs was rendered with
>> headbgcolor.  Users who changed their headbgcolor are used to seeing the
>> "ref part" of remote refs in the same color as their local heads.
>> Applying patch 12 changes the "ref part" color of remote refs, for such
>> users.
>>
>> All I'm saying is that make the remoterefbgcolor default be $headbgcolor
>> avoids this.
>
> For somebody who thinks that most people will want local and
> remote-tracking branch names to be rendered in the same color, your
> suggestion would be an improvement.
>
> But for somebody like me who thinks it is a good idea to render
> remote-tracking branch names in a different color than local branch
> names, this is a feature, not a bug. Even a user who explicitly
> configured `headbgcolor` to, say, purple, wasn't necessarily expressing
> a wish to have remote-tracking branch names rendered in purple. Until
> now they had no choice to set these colors separately!
>
> So even for somebody who configured this setting before, I think that
> having the remote-tracking branch names change color when the user
> upgrades to this version is a good thing, because it lets the user know
> that these two things can now be colored independently. If they don't
> like the new default brown color, such a user knows where to change it
> (even to make it agree with `headbgcolor` if they want that).
>
> But I understand that this is a matter of personal preference. I have
> but one "vote" :-)

I think we understand each other, and that we disagree. I don't feel strongly about it though, so if you want to go this way that's fine by me.

Your case would be helped if the various ref colours were exposed in the preferences dialog. Someone who upgrades to your gitk and doesn't like the default remoterefbgcolor has to track down the right setting, close all running gitk instances, and hand-edit ~/.gitk.

But I think improving gitk's colour-preferences dialog is a bit beyond the scope of your topic...

		M.
Marc Branchaud· Feb 6, 2017, 17:13 UTC · re: Michael Haggerty · lore

Re: [PATCH 00/13] gitk: tweak rendering of remote-tracking references

On 2016-12-19 11:44 AM, Michael Haggerty wrote:
> This patch series changes a bunch of details about how remote-tracking
> references are rendered in the commit list of gitk:
I don't see this series in git v2.12.0-rc0, nor in Paul's gitk repo.

I hope this is an oversight, and not that the series got dropped for some reason.

		M.
Show 70 quoted lines
> * Omit the "remote/" prefix on normal remote-tracking references. They
>   are already distinguished via their two-tone rendering and (usually)
>   longer names, and this change saves a lot of visual clutter and
>   horizontal space.
>
> * Render remote-tracking references that have more than the usual
>   three slashes like
>
>       origin/foo/bar
>       ^^^^^^^
>
>   rather than
>
>       origin/foo/bar (formerly remotes/origin/foo/bar)
>       ^^^^^^^^^^^              ^^^^^^^^^^^^^^^^^^^
>
>   , where the indicated part is the prefix that is rendered in a
>   different color. Usually, such a reference represents a remote
>   branch that contains a slash in its name, so the new split more
>   accurately portrays the separation between remote name and remote
>   branch name.
>
> * Introduce a separate constant to specify the background color used
>   for the branch name part of remote-tracking references, to allow it
>   to differ from the color used for local branches (which by default
>   is bright green).
>
> * Change the default background colors for remote-tracking branches to
>   light brown and brown (formerly they were pale orange and bright
>   green).
>
> I understand that the colors of pixels on computer screens is an even
> more emotional topic that that of bikesheds, so I implemented the last
> change as a separate commit, the last one in the series. Feel free to
> drop it if you don't want the default color change.
>
> Along the way, I did a bunch of refactoring in the area to make these
> changes easier, and introduced a constant for the background color of
> "other" references so that it can also be adjusted by users.
>
> (Unfortunately, these colors can only be adjusted by editing the
> configuration file. Someday it would be nice to allow them to be
> configured via the preferences dialog.)
>
> It's been a while since I've written any Tcl code, so I apologize in
> advance for any howlers :-)
>
> This branch applies against the `master` branch in
> git://ozlabs.org/~paulus/gitk.
>
> Michael
>
> Michael Haggerty (13):
>   gitk: when processing tag labels, don't use `marks` as scratch space
>   gitk: keep track of tag types in a separate `types` array
>   gitk: use a type "tags" to indicate abbreviated tags
>   gitk: use a type "mainhead" to indicate the main HEAD branch
>   gitk: fill in `wvals` as the tags are first processed
>   gitk: simplify regexp
>   gitk: extract a method `remotereftext`
>   gitk: only change the color of the "remote" part of remote refs
>   gitk: shorten labels displayed for modern remote-tracking refs
>   gitk: use type "remote" for remote-tracking references
>   gitk: introduce a constant otherrefbgcolor
>   gitk: add a configuration setting `remoterefbgcolor`
>   gitk: change the default colors for remote-tracking references
>
>  gitk | 114 ++++++++++++++++++++++++++++++++++++++++++++-----------------------
>  1 file changed, 76 insertions(+), 38 deletions(-)
>

← back to recent threads