Subject: Re: [PATCH] gitk: support config the color of linkfgcolor via Gitk Preferences

From: Junio C Hamano <[email-masked]>
Date: Fri, 27 Feb 2026 08:49:40 -0800

Johannes Sixt <[email-masked]> writes:

> Am 26.02.26 um 13:33 schrieb Gary Wang via GitGitGadget:
>> From: Wang Zichong <[email-masked]>
>> 
>> As a dark-theme user, I use the Preferences dialog to set colors
>> for gitk, the only color I cannot change via that dialog is the
>> link foreground color, which will lead me to use the default link
>> color on a dark background that make it not really readable.
>> 
>> This patch makes the link foreground color also configurable in the
>> Gitk Preferences dialog's Color tab, so user won't need to dig into
>> the code/manual to know if the link color is configurable and can
>> simply set the color there.
>
> Makes sense.
>
> Our usual style is to not say "This patch does X to...", but to write in
> imperative mood "Do X to...".

A bit of tangent, but I wonder if it would help new comers if we add
what I sometimes send (e.g., a recent one found in

    https://lore.kernel.org/git/[email-masked]/

) somewhere more prominent, like MyFirstContribution?

>> CC: Mark Levedahl <[email-masked]>, Paul Mackerras <[email-masked]>

It is unusual to see multiple people listed on a single Cc: trailer.

>> Signed-off-by: Wang Zichong <[email-masked]>
>> ---
>>     gitk: support config the color of linkfgcolor via Gitk Preferences
>
> In the subject line, perhaps:
>
>    gitk: support link color in the Preferences dialog
>
>> +    label $page.linkfg -padx 40 -relief sunk -background $linkfgcolor
>> +    ttk::button $page.linkfgbut -text [mc "Link color"] \
>> +        -command [list choosecolor linkfgcolor {} $page [mc "link color"]]
>
> This text "link color" is used in the title of the color selection
> dialog. It then reads awkwardly "Gitk: choose color for link color".
> Let's just use the text "links" for this purpose, and then also just
> "Links" as the label on the button.
>
>> @@ -11891,6 +11896,7 @@ proc prefspage_set_colorswatches {page} {
>>      $page.hunksep configure -background [lindex $diffcolors 2]
>>      $page.markbgsep configure -background $markbgcolor
>>      $page.selbgsep configure -background $selectbgcolor
>> +    $page.linkfg configure -background $linkfgcolor
>
> The variable's value is already serialized in the configuration and
> needs no additional treatment. Good.
>
> -- Hannes