Skip to contentSkip to conversation

gitk: use config settings for head/tag colors

PATCH In review Shannon Barber via GitGitGadget 2 participants 3 messages Jan 13 – Jan 20 1 message not in the archive

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 gitk: use config settings for head/tag colors

From: Shannon Barber <[email protected]>

The drawtags procedure currently uses headfgcolor for all label text,
ignoring the tagfgcolor setting.

The call to create the outline polygon for (non-tag) heads currently
has the color for headoutlinecolor hardcoded to black.

This patch maintains the variables for the non-tag refs so that heads
are colored differently from non-head (non-tag) refs.

The outline and fill colors for the non-head refs remain hardcoded to
the prior values, black & #ddddff.

---
gitk: use headoutlinecolor, headbgcolor, and tagfgcolor config settings to change the color of the tags and head outlines and fill

This patch makes the config settings, headoutlinecolor and tagfgcolor, work. These are existing config settings but not used in the code.

(I noticed that tagfgcolor was not working while making an extension for vscode that exports its color theme to gitk.) (Upon review of the code I found that headoutlinecolor was also not used and investigated and fixed it.)

This simple patch has them affect the target UI elements while drawing refs and their outlined boxes. Tags are drawn in their own code branch then heads and non-heads are drawn in another one. Because both head and non-head refs are drawn in the same loop the colors used to draw (and fill) toggle between the head colors and (still hard-coded) non-head ref colors.

Image showing it working, reviewing the patch that made it work. gitk-colors
[https://github.com/user-attachments/assets/0eea207d-0891-41c8-8c6d-1464a96a1c76]

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2030%2FMagmaiKH%2Fsbarber%2Fuse_color_config_variables-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2030/MagmaiKH/sbarber/use_color_config_variables-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2030

 gitk-git/gitk | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)
gitk-git/gitk +7 -4
index 7f62c8041d..0415abd873 100755
@@ -6831,16 +6831,18 @@ proc drawtags {id x xt y1} {
         } else {
             # draw a head or other ref
             if {[incr nheads -1] >= 0} {
-                set col $headbgcolor
+                set refoutlinecol $headoutlinecolor
+                set reffillcol $headbgcolor
                 if {$tag eq $mainhead} {
                     set font mainfontbold
                 }
             } else {
-                set col "#ddddff"
+                set refoutlinecol black
+                set reffillcol "#ddddff"
             }
             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
+                -width 1 -outline $refoutlinecol -fill $reffillcol -tags tag.$id
             if {[regexp {^(remotes/.*/|remotes/)} $tag match remoteprefix]} {
                 set rwid [font measure mainfont $remoteprefix]
                 set xi [expr {$x + 1}]
@@ -6850,7 +6852,8 @@ 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 textfgcolor [expr {$ntags >= 0 ? $tagfgcolor : $headfgcolor}]
+        set t [$canv create text $xl $y1 -anchor w -text $tag -fill $textfgcolor \
                    -font $font -tags [list tag.$id text]]
         if {$ntags >= 0} {
             $canv bind $t <1> $tagclick

base-commit: d529f3a197364881746f558e5652f0236131eb86

Signature · 1 line
gitgitgadget
#1 lore raw

Thank you for your contribution!

Am 13.01.26 um 07:28 schrieb Shannon Barber via GitGitGadget:

Quoted 8 lines

From: Shannon Barber <[email protected]>

The drawtags procedure currently uses headfgcolor for all label text,
ignoring the tagfgcolor setting.

The call to create the outline polygon for (non-tag) heads currently
has the color for headoutlinecolor hardcoded to black.

Quoted 7 lines

This patch maintains the variables for the non-tag refs so that heads
are colored differently from non-head (non-tag) refs.

The outline and fill colors for the non-head refs remain hardcoded to
the prior values, black & #ddddff.

In this project, the author and signer-off should be identical. Please choose one identity for both.

It was very hard to figure out what the patch attempts to do. The commit message wasn't very helpful, I am afraid. I would have appreciated if a short summary of the status quo at a high level had been given. For example:

--- 8< ---
Gitk draws ref names with 4 different styles depending on the type of ref:

- ...
...

The styles use variables that can be set in the configuration file for ..., but hard-codes the style for ... But there do exist configuration entries for ... but they are not used. Replace the hard-coded values for these latter ones, but leave the remaining styles unchanged.

...
--- 8< ---

What is also missing is what the implications for users are after the change. Clearly, the settings stored in the configuration file are now heeded. But what happens for users who are unaware that there are settings (since they are not accessible via the UI). Are any observable changes intentional? If yes, what is the possible impact?

BTW, the paragraph indentation is a bit odd.

The patch text looks good.

-- Hannes

A message that is not in the archive (sent off-list or not yet received): SJ1SPRMB0003BF77E3500DF7C96E3042D18CA@SJ1SPRMB0003.namprd20.prod.outlook.com

#3 lore raw

Am 15.01.26 um 07:03 schrieb Shannon Barber:

Quoted 11 lines

I think I can simplify it to :

Quoted 1 line

 gitk: honor the headoutlinecolor and tagfgcolor config settings

I pushed a fix with a corrected sign-off.

These settings already exist but the code ignored them.

I do not understand your question about a high-level summary. There are no structural changes.
There are no functional changes.
This is a cosmetic change to how the head and tag refs are drawn, to use already existing color configurations (that were inadvertently ignored.)

While the effect of the change is just cosmetic (in the sense that the visual appearance of the graph labels is changed), it is not a "no functional change".

Consider a user who has experimented with the configuration file. They may have found that changing the value of these variables in the file doesn't work, and then forgot about it, leaving the modified value in the file. With this change, the value that was so far ignored, now suddenly has an effect. It is worthwhile to analyze such behavior and document it at least in the commit message, so that later readers of the code and history know that the case was considered.

You should think about such effects and note them in the commit message. Include the expected behavior in such edge cases. That's what I meant by
high-level summary.

-- Hannes