Re: [PATCH] gitk: support config the color of linkfgcolor via Gitk Preferences
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 27, 2026, 16:49 UTC
- Message-ID
- <xmqq8qcet9ln.fsf@gitster.g>
- In-Reply-To
- <ffd58cd5-811b-4ebf-8572-cca681ec9bf7@kdbg.org>
Johannes Sixt <j6t@kdbg.org> writes:
Show 17 quoted lines
> Am 26.02.26 um 13:33 schrieb Gary Wang via GitGitGadget: >> From: Wang Zichong <wangzichong@deepin.org> >> >> 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/xmqq343ehu4o.fsf@gitster.g/
) somewhere more prominent, like MyFirstContribution?
>> CC: Mark Levedahl <mlevedahl@gmail.com>, Paul Mackerras <paulus@samba.org>
It is unusual to see multiple people listed on a single Cc: trailer.
Show 27 quoted lines
>> Signed-off-by: Wang Zichong <wangzichong@deepin.org>
>> ---
>> 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