[PATCH 2/5] gitk: condense repetitive code around color buttons into foreach loops

"Johannes Sixt via GitGitGadget" <[email protected]> Thu, 06 Aug 2026 06:40:15 +0000
Newsgroups org.kernel.vger.git
Message-ID <edb567608e1bda16efbdb8d4ec359318cf105284.1785998419.git.gitgitgadget@gmail.com>
From: Johannes Sixt <[email protected]>

The color selection elements in the Preferences dialog are set up in a
very uniform manner. The code doing that has grown in the past by simply
modifying a copy of a paragraph. Extract the varying parts into a list
and then operate the repeating parts in a foreach loop. This helps a
later change where we want to set up the UI elements in a different way.

Change the UI names "markbgsep" and "selbgsep" to drop the "sep", which
has obviously been left over by accident when "hunksep" was copied.

Change the suffix "but" to "btn", which resonates a bit better when
reading the code.

Signed-off-by: Johannes Sixt <[email protected]>
---
 gitk-git/gitk | 114 ++++++++++++++++++++++++--------------------------
 1 file changed, 55 insertions(+), 59 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index f5eb963b86..137940defb 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -11832,55 +11832,46 @@ proc prefspage_colors {notebook} {
 
     ttk::label $page.cdisp -text [mc "Colors: press to choose"] -font mainfontbold
     grid $page.cdisp - -sticky w -pady 10
-    label $page.bg -padx 40 -relief sunk
-    ttk::button $page.bgbut -text [mc "Background"] \
-        -command [list choosecolor bgcolor {} $page [mc "background"]]
-    grid x $page.bgbut $page.bg -sticky w
-
-    label $page.fg -padx 40 -relief sunk
-    ttk::button $page.fgbut -text [mc "Foreground"] \
-        -command [list choosecolor fgcolor {} $page [mc "foreground"]]
-    grid x $page.fgbut $page.fg -sticky w
-
-    label $page.diffold -padx 40 -relief sunk
-    ttk::button $page.diffoldbut -text [mc "Diff: old lines"] \
-        -command [list choosecolor diffcolors 0 $page [mc "diff old lines"]]
-    grid x $page.diffoldbut $page.diffold -sticky w
-
-    label $page.diffoldbg -padx 40 -relief sunk
-    ttk::button $page.diffoldbgbut -text [mc "Diff: old lines bg"] \
-        -command [list choosecolor diffbgcolors 0 $page [mc "diff old lines bg"]]
-    grid x $page.diffoldbgbut $page.diffoldbg -sticky w
-
-    label $page.diffnew -padx 40 -relief sunk
-    ttk::button $page.diffnewbut -text [mc "Diff: new lines"] \
-        -command [list choosecolor diffcolors 1 $page [mc "diff new lines"]]
-    grid x $page.diffnewbut $page.diffnew -sticky w
-
-    label $page.diffnewbg -padx 40 -relief sunk
-    ttk::button $page.diffnewbgbut -text [mc "Diff: new lines bg"] \
-        -command [list choosecolor diffbgcolors 1 $page [mc "diff new lines bg"]]
-    grid x $page.diffnewbgbut $page.diffnewbg -sticky w
-
-    label $page.hunksep -padx 40 -relief sunk
-    ttk::button $page.hunksepbut -text [mc "Diff: hunk header"] \
-        -command [list choosecolor diffcolors 2 $page [mc "diff hunk header"]]
-    grid x $page.hunksepbut $page.hunksep -sticky w
-
-    label $page.markbgsep -padx 40 -relief sunk
-    ttk::button $page.markbgbut -text [mc "Marked line bg"] \
-        -command [list choosecolor markbgcolor {} $page [mc "marked line background"]]
-    grid x $page.markbgbut $page.markbgsep -sticky w
-
-    label $page.selbgsep -padx 40 -relief sunk
-    ttk::button $page.selbgbut -text [mc "Select bg"] \
-        -command [list choosecolor selectbgcolor {} $page [mc "background"]]
-    grid x $page.selbgbut $page.selbgsep -sticky w
-
-    label $page.linkfg -padx 40 -relief sunk
-    ttk::button $page.linkfgbut -text [mc "Link"] \
-        -command [list choosecolor linkfgcolor {} $page [mc "link"]]
-    grid x $page.linkfgbut $page.linkfg -sticky w
+
+    set coloruielems [list \
+        bg          bgcolor {} \
+                    [mc "Background"] \
+                    [mc "background"] \
+        fg          fgcolor {} \
+                    [mc "Foreground"] \
+                    [mc "foreground"] \
+        diffold     diffcolors 0 \
+                    [mc "Diff: old lines"] \
+                    [mc "diff old lines"] \
+        diffoldbg   diffbgcolors 0 \
+                    [mc "Diff: old lines bg"] \
+                    [mc "diff old lines bg"] \
+        diffnew     diffcolors 1 \
+                    [mc "Diff: new lines"] \
+                    [mc "diff new lines"] \
+        diffnewbg   diffbgcolors 1 \
+                    [mc "Diff: new lines bg"] \
+                    [mc "diff new lines bg"] \
+        hunksep     diffcolors 2 \
+                    [mc "Diff: hunk header"] \
+                    [mc "diff hunk header"] \
+        markbg      markbgcolor {} \
+                    [mc "Marked line bg"] \
+                    [mc "marked line background"] \
+        selbg       selectbgcolor {} \
+                    [mc "Select bg"] \
+                    [mc "background"] \
+        linkfg      linkfgcolor {} \
+                    [mc "Link"] \
+                    [mc "link"] \
+    ]
+
+    foreach {uielem colorvar idx label title} $coloruielems {
+        label $page.$uielem -padx 40 -relief sunk
+        ttk::button $page.${uielem}btn -text $label \
+            -command [list choosecolor $colorvar $idx $page $title]
+        grid x $page.${uielem}btn $page.$uielem -sticky w
+    }
 
     grid columnconfigure $page 2 -weight 1
     prefspage_set_colorswatches $page
@@ -11892,16 +11883,21 @@ proc prefspage_set_colorswatches {page} {
     global bgcolor fgcolor diffcolors selectbgcolor markbgcolor
     global diffbgcolors linkfgcolor
 
-    $page.bg configure -background $bgcolor
-    $page.fg configure -background $fgcolor
-    $page.diffold configure -background [lindex $diffcolors 0]
-    $page.diffoldbg configure -background [lindex $diffbgcolors 0]
-    $page.diffnew configure -background [lindex $diffcolors 1]
-    $page.diffnewbg configure -background [lindex $diffbgcolors 1]
-    $page.hunksep configure -background [lindex $diffcolors 2]
-    $page.markbgsep configure -background $markbgcolor
-    $page.selbgsep configure -background $selectbgcolor
-    $page.linkfg configure -background $linkfgcolor
+    set coloruielems [list \
+        bg        $bgcolor \
+        fg        $fgcolor \
+        diffold   [lindex $diffcolors 0] \
+        diffoldbg [lindex $diffbgcolors 0] \
+        diffnew   [lindex $diffcolors 1] \
+        diffnewbg [lindex $diffbgcolors 1] \
+        hunksep   [lindex $diffcolors 2] \
+        markbg    $markbgcolor \
+        selbg     $selectbgcolor \
+        linkfg    $linkfgcolor \
+    ]
+    foreach {uielem color} $coloruielems {
+        $page.$uielem configure -background $color
+    }
 }
 
 proc prefspage_fonts {notebook} {
-- 
gitgitgadget