[PATCH] gitk: use config settings for head/tag colors

[PATCH] gitk: use config settings for head/tag colors

[Shannon Barber via GitGitGadget]

From: Shannon Barber <sbarber@dataspeedinc.com>

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.

Signed-off-by: Shannon Barber <sgbarber@gmail.com>
---
    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(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index 7f62c8041d..0415abd873 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -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
-- 
gitgitgadget

Re: [PATCH] gitk: use config settings for head/tag colors

[Johannes Sixt]

Thank you for your contribution!

Am 13.01.26 um 07:28 schrieb Shannon Barber via GitGitGadget:
> From: Shannon Barber <sbarber@dataspeedinc.com>
> 
> 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.
> 
> Signed-off-by: Shannon Barber <sgbarber@gmail.com>

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

Re: [PATCH] gitk: use config settings for head/tag colors

[Johannes Sixt]

Am 15.01.26 um 07:03 schrieb Shannon Barber:
> I think I can simplify it to :
>>  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