[PATCH 0/5] gitk: make color preferences visually more pleasing and better usable

[PATCH 0/5] gitk: make color preferences visually more pleasing and better usable

[Johannes Sixt via GitGitGadget]

I find the user interface to set the color preferences a bit ugly for these
reasons:

 * The color samples are not clickable as one would expect who enters the
   dialog. Instead, the description is the button that must be clicked.

 * Since the descriptive texts are different for the preferences, the width
   of the buttons are different, too.

 * The descriptions themselves are not always natural language (read: they
   are nerdy) and use abbreviations.

This series makes the descriptions static text and turns the color samples
into the clickable buttons. It also makes the descriptions and dialog titles
more natural language.

This is the dialog before the change: Screenshot_color_buttons_before
[https://github.com/user-attachments/assets/910a647f-4e56-47b0-b4c4-e6e7bc966e9c]
And this is the dialog after the change: Screenshot_color_buttons_after
[https://github.com/user-attachments/assets/d985d837-1b1e-4b43-8bcd-76bca8d060ce]

Johannes Sixt (5):
  gitk: set intitial colors of swatches using the available helper
  gitk: condense repetitive code around color buttons into foreach loops
  gitk: show color preferences on the button instead of the label
  gitk: use more natural language for labels of color preferences
  gitk: avoid constructing dialog titles from text pieces

 gitk-git/gitk | 123 ++++++++++++++++++++++++--------------------------
 1 file changed, 59 insertions(+), 64 deletions(-)


base-commit: a97fcc37c2bc6340a8d7ce78dedf227aac4e9aa7
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2194%2Fj6t%2Fgitk-prefs-color-buttons-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2194/j6t/gitk-prefs-color-buttons-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2194
-- 
gitgitgadget

[PATCH 1/5] gitk: set intitial colors of swatches using the available helper

[Johannes Sixt via GitGitGadget]

From: Johannes Sixt <j6t@kdbg.org>

After the user has selected a color in the Preferences dialog, the
helper proc prefspage_set_colorswatches is used update the colors shown
in the Preferences dialog. Use this proc also after the Preferences
dialog is constructed to show the initial colors. This keeps the
procedure that gives the UI elements their colors in a single place.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 gitk-git/gitk | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index 0f3571050b..f5eb963b86 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -11801,8 +11801,6 @@ proc prefspage_general {notebook} {
 }
 
 proc prefspage_colors {notebook} {
-    global bgcolor fgcolor ctext diffcolors selectbgcolor markbgcolor
-    global diffbgcolors linkfgcolor
     global themeloader
 
     set page [create_prefs_page $notebook.colors]
@@ -11834,63 +11832,64 @@ 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 -background $bgcolor
+    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 -background $fgcolor
+    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 -background [lindex $diffcolors 0]
+    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 -background [lindex $diffbgcolors 0]
+    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 -background [lindex $diffcolors 1]
+    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 -background [lindex $diffbgcolors 1]
+    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 -background [lindex $diffcolors 2]
+    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 -background $markbgcolor
+    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 -background $selectbgcolor
+    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 -background $linkfgcolor
+    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
 
     grid columnconfigure $page 2 -weight 1
+    prefspage_set_colorswatches $page
 
     return $page
 }
 
 proc prefspage_set_colorswatches {page} {
-    global bgcolor fgcolor ctext diffcolors selectbgcolor markbgcolor
+    global bgcolor fgcolor diffcolors selectbgcolor markbgcolor
     global diffbgcolors linkfgcolor
 
     $page.bg configure -background $bgcolor
-- 
gitgitgadget

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

[Johannes Sixt via GitGitGadget]

From: Johannes Sixt <j6t@kdbg.org>

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 <j6t@kdbg.org>
---
 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

[PATCH 3/5] gitk: show color preferences on the button instead of the label

[Johannes Sixt via GitGitGadget]

From: Johannes Sixt <j6t@kdbg.org>

When the user goes to the color preferences, the eye is automatically
drawn to the color samples. However, clicking on the sample with the
intent to change it does nothing. Instead, the text label also acts as
a button and must be clicked to change the color.

Turn the color samples into clickable buttons and the text labels into
static text. Use non-themed buttons because their color can be changed
by simply specifying the color properties. (Themed buttons would need
a lot more elaboration.)

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 gitk-git/gitk | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index 137940defb..877b2ad27f 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -11867,10 +11867,10 @@ proc prefspage_colors {notebook} {
     ]
 
     foreach {uielem colorvar idx label title} $coloruielems {
-        label $page.$uielem -padx 40 -relief sunk
-        ttk::button $page.${uielem}btn -text $label \
+        ttk::label $page.$uielem -text $label
+        button $page.${uielem}btn -padx 40 -pady 0 \
             -command [list choosecolor $colorvar $idx $page $title]
-        grid x $page.${uielem}btn $page.$uielem -sticky w
+        grid x $page.$uielem $page.${uielem}btn -sticky w
     }
 
     grid columnconfigure $page 2 -weight 1
@@ -11896,7 +11896,7 @@ proc prefspage_set_colorswatches {page} {
         linkfg    $linkfgcolor \
     ]
     foreach {uielem color} $coloruielems {
-        $page.$uielem configure -background $color
+        $page.${uielem}btn configure -background $color -activebackground $color
     }
 }
 
-- 
gitgitgadget

[PATCH 4/5] gitk: use more natural language for labels of color preferences

[Johannes Sixt via GitGitGadget]

From: Johannes Sixt <j6t@kdbg.org>

The labels used to be clickable buttons. These would have looked funny
if they carried long texts. For this reason, abbreviations and "nerdy"
texts were used to keep them short. Since these labels are now static
text, bring them closer to natural language.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 gitk-git/gitk | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index 877b2ad27f..9e735d91bf 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -11838,31 +11838,31 @@ proc prefspage_colors {notebook} {
                     [mc "Background"] \
                     [mc "background"] \
         fg          fgcolor {} \
-                    [mc "Foreground"] \
+                    [mc "Main text"] \
                     [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 background"] \
                     [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 background"] \
                     [mc "diff new lines bg"] \
         hunksep     diffcolors 2 \
-                    [mc "Diff: hunk header"] \
+                    [mc "Diff: hunk headers"] \
                     [mc "diff hunk header"] \
         markbg      markbgcolor {} \
-                    [mc "Marked line bg"] \
+                    [mc "Marked lines background"] \
                     [mc "marked line background"] \
         selbg       selectbgcolor {} \
-                    [mc "Select bg"] \
+                    [mc "Selected text background"] \
                     [mc "background"] \
         linkfg      linkfgcolor {} \
-                    [mc "Link"] \
+                    [mc "Link text"] \
                     [mc "link"] \
     ]
 
-- 
gitgitgadget

[PATCH 5/5] gitk: avoid constructing dialog titles from text pieces

[Johannes Sixt via GitGitGadget]

From: Johannes Sixt <j6t@kdbg.org>

When the user clicks a color preference, a color selection dialog is
presented whose title is provided in parts by the caller. The dialog
implementation must supply the rest of the title. This is unfriendly
for translations. Provide the full title by the caller. Rewrite the
texts to be more natural language.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 gitk-git/gitk | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index 9e735d91bf..454f54dca8 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -11836,34 +11836,34 @@ proc prefspage_colors {notebook} {
     set coloruielems [list \
         bg          bgcolor {} \
                     [mc "Background"] \
-                    [mc "background"] \
+                    [mc "choose background color"] \
         fg          fgcolor {} \
                     [mc "Main text"] \
-                    [mc "foreground"] \
+                    [mc "choose main text color"] \
         diffold     diffcolors 0 \
                     [mc "Diff: old lines"] \
-                    [mc "diff old lines"] \
+                    [mc "choose text color of old lines"] \
         diffoldbg   diffbgcolors 0 \
                     [mc "Diff: old lines background"] \
-                    [mc "diff old lines bg"] \
+                    [mc "choose background color of old lines"] \
         diffnew     diffcolors 1 \
                     [mc "Diff: new lines"] \
-                    [mc "diff new lines"] \
+                    [mc "choose text color of new lines"] \
         diffnewbg   diffbgcolors 1 \
                     [mc "Diff: new lines background"] \
-                    [mc "diff new lines bg"] \
+                    [mc "choose background color of new lines"] \
         hunksep     diffcolors 2 \
                     [mc "Diff: hunk headers"] \
-                    [mc "diff hunk header"] \
+                    [mc "choose hunk header color"] \
         markbg      markbgcolor {} \
                     [mc "Marked lines background"] \
-                    [mc "marked line background"] \
+                    [mc "choose background color of marked lines"] \
         selbg       selectbgcolor {} \
                     [mc "Selected text background"] \
-                    [mc "background"] \
+                    [mc "choose background color of selected text"] \
         linkfg      linkfgcolor {} \
                     [mc "Link text"] \
-                    [mc "link"] \
+                    [mc "choose color of link text"] \
     ]
 
     foreach {uielem colorvar idx label title} $coloruielems {
@@ -12014,11 +12014,11 @@ proc choose_themeloader {prefspage} {
     }
 }
 
-proc choosecolor {v vi prefspage x} {
+proc choosecolor {v vi prefspage title} {
     global $v
 
     set c [tk_chooseColor -initialcolor [lindex [set $v] $vi] \
-               -title [mc "Gitk: choose color for %s" $x]]
+               -title "Gitk: $title"]
     if {$c eq {}} return
     lset $v $vi $c
     set_gui_colors
-- 
gitgitgadget

Re: [PATCH 1/5] gitk: set intitial colors of swatches using the available helper

[mark]

On 8/6/26 2:40 AM, Johannes Sixt via GitGitGadget wrote:
> From: Johannes Sixt <j6t@kdbg.org>
> 
> After the user has selected a color in the Preferences dialog, the
> helper proc prefspage_set_colorswatches is used update the colors shown
> in the Preferences dialog. Use this proc also after the Preferences
> dialog is constructed to show the initial colors. This keeps the
> procedure that gives the UI elements their colors in a single place.
> 
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---
This completes collecting the color settings to one place, Good!

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

[mark]

On 8/6/26 2:40 AM, Johannes Sixt via GitGitGadget wrote:
> From: Johannes Sixt <j6t@kdbg.org>
> 
> 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 <j6t@kdbg.org>
> ---
>  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} {


> -
> -    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"] \
The list is computed and used each time a dialog box is started from all static elements,
no real change. But this cleanly separates the per item data from the boiler plate,
definitely more readable. Good.

Re: [PATCH 3/5] gitk: show color preferences on the button instead of the label

[mark]

On 8/6/26 2:40 AM, Johannes Sixt via GitGitGadget wrote:
> From: Johannes Sixt <j6t@kdbg.org>
> 
> When the user goes to the color preferences, the eye is automatically
> drawn to the color samples. However, clicking on the sample with the
> intent to change it does nothing. Instead, the text label also acts as
> a button and must be clicked to change the color.
> 
> Turn the color samples into clickable buttons and the text labels into
> static text. Use non-themed buttons because their color can be changed
> by simply specifying the color properties. (Themed buttons would need
> a lot more elaboration.)
> 
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---
>  gitk-git/gitk | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/gitk-git/gitk b/gitk-git/gitk
> index 137940defb..877b2ad27f 100755
> --- a/gitk-git/gitk
> +++ b/gitk-git/gitk
> @@ -11867,10 +11867,10 @@ proc prefspage_colors {notebook} {
>      ]
>  
>      foreach {uielem colorvar idx label title} $coloruielems {
> -        label $page.$uielem -padx 40 -relief sunk
> -        ttk::button $page.${uielem}btn -text $label \
> +        ttk::label $page.$uielem -text $label
> +        button $page.${uielem}btn -padx 40 -pady 0 \
>              -command [list choosecolor $colorvar $idx $page $title]
> -        grid x $page.${uielem}btn $page.$uielem -sticky w
> +        grid x $page.$uielem $page.${uielem}btn -sticky w
>      }
>  
>      grid columnconfigure $page 2 -weight 1
> @@ -11896,7 +11896,7 @@ proc prefspage_set_colorswatches {page} {
>          linkfg    $linkfgcolor \
>      ]
>      foreach {uielem color} $coloruielems {
> -        $page.$uielem configure -background $color
> +        $page.${uielem}btn configure -background $color -activebackground $color
>      }
>  }
>  

The original approach has unthemed labels, allowing easy specification of color, and
themed buttons. With this change, the roles reverse, so swapping themed / unthemed makes
sense. Good.

But, the colored buttons do not stand out to my eye as much as in the prior approach.
Perhaps use -borderwidth 2? This is totally subjective, I'm ok if you leave this as-is.

Re: [PATCH 5/5] gitk: avoid constructing dialog titles from text pieces

[mark]

On 8/6/26 2:40 AM, Johannes Sixt via GitGitGadget wrote:
> From: Johannes Sixt <j6t@kdbg.org>
> 
> When the user clicks a color preference, a color selection dialog is
> presented whose title is provided in parts by the caller. The dialog
> implementation must supply the rest of the title. This is unfriendly
> for translations. Provide the full title by the caller. Rewrite the
> texts to be more natural language.
> 
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---
>  gitk-git/gitk | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/gitk-git/gitk b/gitk-git/gitk
> index 9e735d91bf..454f54dca8 100755
> --- a/gitk-git/gitk
> +++ b/gitk-git/gitk
> @@ -11836,34 +11836,34 @@ proc prefspage_colors {notebook} {
>      set coloruielems [list \
>          bg          bgcolor {} \
>                      [mc "Background"] \
> -                    [mc "background"] \
> +                    [mc "choose background color"] \
>          fg          fgcolor {} \
>                      [mc "Main text"] \
> -                    [mc "foreground"] \
> +                    [mc "choose main text color"] \

> @@ -12014,11 +12014,11 @@ proc choose_themeloader {prefspage} {
>      }
>  }
>  
> -proc choosecolor {v vi prefspage x} {
> +proc choosecolor {v vi prefspage title} {
>      global $v
>  
>      set c [tk_chooseColor -initialcolor [lindex [set $v] $vi] \
> -               -title [mc "Gitk: choose color for %s" $x]]
> +               -title "Gitk: $title"]
>      if {$c eq {}} return
>      lset $v $vi $c
>      set_gui_colors

This with the prior patch should help translation due to use of more common idioms and
complete phrases rather than jargon. Good.

Re: [PATCH 3/5] gitk: show color preferences on the button instead of the label

[Johannes Sixt]

Am 07.08.26 um 16:37 schrieb mark:
> But, the colored buttons do not stand out to my eye as much as in the prior approach.
> Perhaps use -borderwidth 2?

I agree. -borderwith 2 does help emphasize that these are clickable
buttons. I'll also insert a small space between the buttons like this:

@@ -11868,9 +11868,9 @@ proc prefspage_colors {notebook} {
 
     foreach {uielem colorvar idx label title} $coloruielems {
         ttk::label $page.$uielem -text $label
-        button $page.${uielem}btn -padx 40 -pady 0 \
+        button $page.${uielem}btn -padx 40 -pady 0 -borderwidth 2 \
             -command [list choosecolor $colorvar $idx $page $title]
-        grid x $page.$uielem $page.${uielem}btn -sticky w
+        grid x $page.$uielem $page.${uielem}btn -sticky w -pady 1
     }
 
     grid columnconfigure $page 2 -weight 1


-- Hannes

Re: [PATCH 5/5] gitk: avoid constructing dialog titles from text pieces

[Johannes Sixt]

Am 07.08.26 um 16:37 schrieb mark:
> This with the prior patch should help translation due to use of more common idioms and
> complete phrases rather than jargon. Good.

"Complete phrases" and "jargon" are words I need for the commit
messages! Thank you :-)

Now, with a fresh eye, I see that we have text like this:

	Diff: old lines background

This looks more like an assemblement of words and jargon than a phrase
in natural language. Any suggestions for improvement? How about

	Background of old lines in diffs

Sounds a bit clumsy.

(In this case I prefer to say "diffs" rather than e.g. "patch text",
because we are looking at differences between versions, "diffs", and not
patches. Also, we already have "diff" in multiple other places in the UI.)

-- Hannes

Re: [PATCH 5/5] gitk: avoid constructing dialog titles from text pieces

[Mark Levedahl]

On 8/8/26 6:00 AM, Johannes Sixt wrote:
> Am 07.08.26 um 16:37 schrieb mark:
>> This with the prior patch should help translation due to use of more common idioms and
>> complete phrases rather than jargon. Good.
> "Complete phrases" and "jargon" are words I need for the commit
> messages! Thank you :-)
>
> Now, with a fresh eye, I see that we have text like this:
>
> 	Diff: old lines background
>
> This looks more like an assemblement of words and jargon than a phrase
> in natural language. Any suggestions for improvement? How about
>
> 	Background of old lines in diffs
>
> Sounds a bit clumsy.
>
> (In this case I prefer to say "diffs" rather than e.g. "patch text",
> because we are looking at differences between versions, "diffs", and not
> patches. Also, we already have "diff" in multiple other places in the UI.)
>
> -- Hannes
>

I'm ok with using "Diff" as a common identifier for this here. gitk is parsing commit
diffs from git, showing each hunk separately. I'd also rather not use patch. The UI
already shows "new version" and "old version", so using new lines and old lines is consistent.

But, one thing missing, hinted at by your suggestion "Background of ..." is that the clear
definition of what is being affected is poorly expressed. A complete description of each
item would require a paragraph, probably with an example image. Trying to compress into a
few words will always exploit context and presume some common language we expect a
git/gitk user to understand or be able to learn in context.

Maybe some extra words and categorization would help. Consider using a longer category
description to provide common information, for instance:

  -General Canvas Area
                           Background
       Marked Lines Background
                              Main Text
                        Selected Text
                              Link Text
    -Commit Differences
                          old line text
                        new line text
             -old line background
       etc.

etc, where the lines beginning with "-" are just separators and do not have an associated
color selection button.

Mark

Mark

Re: [PATCH 5/5] gitk: avoid constructing dialog titles from text pieces

[Junio C Hamano]

Johannes Sixt <j6t@kdbg.org> writes:

> 	Diff: old lines background
> 	Background of old lines in diffs
>
> Sounds a bit clumsy.

Both lacking "color" is not a problem because it is easy to see that
this entire thing is about "color" from the context (like these
appear in "Color" menu or something)?