{"thread":{"id":"54124","subject":"[PATCH] git-gui: Basic dark mode support","startedAt":"2020-08-24T15:50:47Z","lastAt":"2020-12-18T19:47:04Z","messageCount":37,"participants":["Serg Tereshchenko","Matthias Aßhauer","Pratyush Yadav","Stefan Haller","serg.partizan@gmail.com"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"404326","messageId":"20200824154835.160749-1-serg.partizan@gmail.com","threadId":"54124","inReplyTo":null,"subject":"[PATCH] git-gui: Basic dark mode support","fromName":"Serg Tereshchenko","fromEmail":"serg.partizan@gmail.com","sentAt":"2020-08-24T15:48:35Z","receivedAt":"2020-08-24T15:50:47Z","isPatch":true,"sender":{"key":"serg.partizan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/301015?v=4"},"body":"Hi all.\n\nI want to use dark themes with git citool, and here is my first attempt\nto do so.\n\nI am new to tcl, so i happily accept any tips on how to improve code.\n\nFirst things first: to properly support colors, would be nice to have\nthem separated from app code, so i created new file lib/colored.tcl. Name\nis selected to be consistent with \"lib/themed.tcl\".\n\nThen, i extract hardcoded colors from git-gui.sh into namespace Color.\nThen, if option use_ttk is true, i update default colors for\nbackground/foreground from current theme.\n\nHow it was looking before:\n - Dark theme (awdark): https://i.imgur.com/0lrfHyq.png\n - Light theme (clam): https://i.imgur.com/1fsfayJ.png\n\nNow looks like this:\n - Dark theme (awdark): https://i.imgur.com/BISllEH.png\n - Light theme (clam): https://i.imgur.com/WclSTa4.png\n\nOne problem that i can't yet fix: gray background for files in\nchangelists. Any advice on this?\n\n\nI would be happy to move color definitions from git-gui.sh to\nthemed.tcl, so we can set it once, and not for each ttext call. Do you\nthink this is a good idea now or in the future?\n\nI see some work is already done in that direction, like lib/themed.tcl:gold_frame.\n\n\nKind Regards.\n\nSigned-off-by: Serg Tereshchenko \n---\n git-gui.sh | 33 +++++++++++++++++++--------------\n lib/colored.tcl | 23 +++++++++++++++++++++++\n 2 files changed, 42 insertions(+), 14 deletions(-)\n create mode 100644 lib/colored.tcl\n\ndiff --git a/git-gui.sh b/git-gui.sh\nindex ca66a8e..cffd106 100755\n--- a/git-gui.sh\n+++ b/git-gui.sh\n@@ -861,6 +861,7 @@ proc apply_config {} {\n \t\t\tset NS ttk\n \t\t\tbind [winfo class .] <> [list InitTheme]\n \t\t\tpave_toplevel .\n+\t\t\tColor::syncColorsWithTheme\n \t\t}\n \t}\n }\n@@ -3273,9 +3274,13 @@ pack .vpane -anchor n -side top -fill both -expand 1\n # -- Working Directory File List\n \n textframe .vpane.files.workdir -height 100 -width 200\n-tlabel .vpane.files.workdir.title -text [mc \"Unstaged Changes\"] \\\n-\t-background lightsalmon -foreground black\n-ttext $ui_workdir -background white -foreground black \\\n+tlabel .vpane.files.workdir.title \\\n+\t-text [mc \"Unstaged Changes\"] \\\n+\t-background $Color::lightRed \\\n+\t-foreground $Color::textOnLight\n+ttext $ui_workdir \\\n+\t-background $Color::textBg \\\n+\t-foreground $Color::textColor \\\n \t-borderwidth 0 \\\n \t-width 20 -height 10 \\\n \t-wrap none \\\n@@ -3296,8 +3301,8 @@ pack $ui_workdir -side left -fill both -expand 1\n textframe .vpane.files.index -height 100 -width 200\n tlabel .vpane.files.index.title \\\n \t-text [mc \"Staged Changes (Will Commit)\"] \\\n-\t-background lightgreen -foreground black\n-ttext $ui_index -background white -foreground black \\\n+\t-background $Color::lightGreen -foreground $Color::textOnLight\n+ttext $ui_index -background $Color::textBg -foreground $Color::textColor \\\n \t-borderwidth 0 \\\n \t-width 20 -height 10 \\\n \t-wrap none \\\n@@ -3432,7 +3437,7 @@ if {![is_enabled nocommit]} {\n }\n \n textframe .vpane.lower.commarea.buffer.frame\n-ttext $ui_comm -background white -foreground black \\\n+ttext $ui_comm -background $Color::textBg -foreground $Color::textColor \\\n \t-borderwidth 1 \\\n \t-undo true \\\n \t-maxundo 20 \\\n@@ -3519,19 +3524,19 @@ trace add variable current_diff_path write trace_current_diff_path\n \n gold_frame .vpane.lower.diff.header\n tlabel .vpane.lower.diff.header.status \\\n-\t-background gold \\\n-\t-foreground black \\\n+\t-background $Color::lightGold \\\n+\t-foreground $Color::textOnLight \\\n \t-width $max_status_desc \\\n \t-anchor w \\\n \t-justify left\n tlabel .vpane.lower.diff.header.file \\\n-\t-background gold \\\n-\t-foreground black \\\n+\t-background $Color::lightGold \\\n+\t-foreground $Color::textOnLight \\\n \t-anchor w \\\n \t-justify left\n tlabel .vpane.lower.diff.header.path \\\n-\t-background gold \\\n-\t-foreground blue \\\n+\t-background $Color::lightGold \\\n+\t-foreground $Color::lightBlue \\\n \t-anchor w \\\n \t-justify left \\\n \t-font [eval font create [font configure font_ui] -underline 1] \\\n@@ -3561,7 +3566,7 @@ bind .vpane.lower.diff.header.path {do_file_open $current_diff_path}\n #\n textframe .vpane.lower.diff.body\n set ui_diff .vpane.lower.diff.body.t\n-ttext $ui_diff -background white -foreground black \\\n+ttext $ui_diff -background $Color::textBg -foreground $Color::textColor \\\n \t-borderwidth 0 \\\n \t-width 80 -height 5 -wrap none \\\n \t-font font_diff \\\n@@ -3589,7 +3594,7 @@ foreach {n c} {0 black 1 red4 2 green4 3 yellow4 4 blue4 5 magenta4 6 cyan4 7 gr\n $ui_diff tag configure clr1 -font font_diffbold\n $ui_diff tag configure clr4 -underline 1\n \n-$ui_diff tag conf d_info -foreground blue -font font_diffbold\n+$ui_diff tag conf d_info -foreground $Color::lightBlue -font font_diffbold\n \n $ui_diff tag conf d_cr -elide true\n $ui_diff tag conf d_@ -font font_diffbold\ndiff --git a/lib/colored.tcl b/lib/colored.tcl\nnew file mode 100644\nindex 0000000..fdb3f9c\n--- /dev/null\n+++ b/lib/colored.tcl\n@@ -0,0 +1,23 @@\n+# Color configuration support for git-gui.\n+\n+namespace eval Color {\n+\t# static colors\n+\tvariable lightRed\t\tlightsalmon\n+\tvariable lightGreen\t\tgreen\n+\tvariable lightGold\t\tgold\n+\tvariable lightBlue\t\tblue\n+\tvariable textOnLight\tblack\n+\tvariable textOnDark\t\twhite\n+\t# theme colors\n+\tvariable interfaceBg\tlightgray\n+\tvariable textBg\t\t\twhite\n+\tvariable textColor\t\tblack\n+\n+\tproc syncColorsWithTheme {} {\n+\t\tset Color::interfaceBg\t[ttk::style lookup Entry -background]\n+\t\tset Color::textBg\t\t[ttk::style lookup Treeview -background]\n+\t\tset Color::textColor\t[ttk::style lookup Treeview -foreground]\n+\n+\t\ttk_setPalette $Color::interfaceBg\n+\t}\n+}\n-- \n2.28.0\n\n"},{"id":"404470","messageId":"AM0PR04MB4771FE6F6A9284489A3D5660A5570@AM0PR04MB4771.eurprd04.prod.outlook.com","threadId":"54124","inReplyTo":"20200824154835.160749-1-serg.partizan@gmail.com","subject":"Re: [PATCH] git-gui: Basic dark mode support","fromName":"Matthias Aßhauer","fromEmail":"mha1993@live.de","sentAt":"2020-08-25T19:01:56Z","receivedAt":"2020-08-25T19:02:00Z","isPatch":true,"sender":{"key":"mha1993@live.de","avatar":"https://avatars.githubusercontent.com/u/6178234?v=4"},"body":" >One problem that i can't yet fix: gray background for files \ninchangelists. Any advice on this?\n\nIf I understand the issue and the tcl/tk docs correctly, you can change \nthat color using -highlightcolor on the ttext widget.\n\nI've cc'ed in Pratyush, the current git-gui maintainer. He's probably \nbest suited to review this patch.\n\n\nBest regards\n\n\nMatthias\n\n\n"},{"id":"406121","messageId":"20200922110419.ymqj4ol76kg6qshf@yadavpratyush.com","threadId":"54124","inReplyTo":"20200824154835.160749-1-serg.partizan@gmail.com","subject":"Re: [PATCH] git-gui: Basic dark mode support","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2020-09-22T11:04:19Z","receivedAt":"2020-09-22T11:04:37Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"Hi Serg,\n\nThanks for the patch and sorry for taking so long in getting to it.\n\nOn 24/08/20 06:48PM, Serg Tereshchenko wrote:\n> Hi all.\n> \n> I want to use dark themes with git citool, and here is my first attempt\n> to do so.\n\nLike I said in the previous email this part doesn't really belong in the \ncommit message. In fact, while this entire text is a good description of \nthe patch and your efforts, it is not a good commit message.\n \n> I am new to tcl, so i happily accept any tips on how to improve code.\n> \n> First things first: to properly support colors, would be nice to have\n> them separated from app code, so i created new file lib/colored.tcl. Name\n> is selected to be consistent with \"lib/themed.tcl\".\n\nWouldn't having the contents of colored.tcl in themed.tcl be a good \nidea? The way I see it, colors are part of the theming of the \napplication.\n \n> Then, i extract hardcoded colors from git-gui.sh into namespace Color.\n> Then, if option use_ttk is true, i update default colors for\n> background/foreground from current theme.\n> \n> How it was looking before:\n> - Dark theme (awdark): https://i.imgur.com/0lrfHyq.png\n> - Light theme (clam): https://i.imgur.com/1fsfayJ.png\n> \n> Now looks like this:\n> - Dark theme (awdark): https://i.imgur.com/BISllEH.png\n> - Light theme (clam): https://i.imgur.com/WclSTa4.png\n\nThis is quite an improvement :-)\n \n> One problem that i can't yet fix: gray background for files in\n> changelists. Any advice on this?\n\nYou can set that in the function `rmsel_tag` in git-gui.sh on the line\n\n $text tag conf in_sel -background lightgray\n \n> I would be happy to move color definitions from git-gui.sh to\n> themed.tcl, so we can set it once, and not for each ttext call. Do you\n> think this is a good idea now or in the future?\n\nDo you mean to put the `-foreground` and `-background` options in the \nfunction ttext in themed.tcl? If so how can a widget specify if it wants \na dark text or light for example?\n \n> I see some work is already done in that direction, like lib/themed.tcl:gold_frame.\n> \n> \n> Kind Regards.\n> \n> Signed-off-by: Serg Tereshchenko \n> ---\n> git-gui.sh | 33 +++++++++++++++++++--------------\n> lib/colored.tcl | 23 +++++++++++++++++++++++\n> 2 files changed, 42 insertions(+), 14 deletions(-)\n> create mode 100644 lib/colored.tcl\n> \n> diff --git a/git-gui.sh b/git-gui.sh\n> index ca66a8e..cffd106 100755\n> --- a/git-gui.sh\n> +++ b/git-gui.sh\n> @@ -861,6 +861,7 @@ proc apply_config {} {\n> \t\t\tset NS ttk\n> \t\t\tbind [winfo class .] <> [list InitTheme]\n> \t\t\tpave_toplevel .\n> +\t\t\tColor::syncColorsWithTheme\n> \t\t}\n> \t}\n> }\n> @@ -3273,9 +3274,13 @@ pack .vpane -anchor n -side top -fill both -expand 1\n> # -- Working Directory File List\n> \n> textframe .vpane.files.workdir -height 100 -width 200\n> -tlabel .vpane.files.workdir.title -text [mc \"Unstaged Changes\"] \\\n> -\t-background lightsalmon -foreground black\n> -ttext $ui_workdir -background white -foreground black \\\n> +tlabel .vpane.files.workdir.title \\\n> +\t-text [mc \"Unstaged Changes\"] \\\n> +\t-background $Color::lightRed \\\n> +\t-foreground $Color::textOnLight\n> +ttext $ui_workdir \\\n> +\t-background $Color::textBg \\\n> +\t-foreground $Color::textColor \\\n> \t-borderwidth 0 \\\n> \t-width 20 -height 10 \\\n> \t-wrap none \\\n> @@ -3296,8 +3301,8 @@ pack $ui_workdir -side left -fill both -expand 1\n> textframe .vpane.files.index -height 100 -width 200\n> tlabel .vpane.files.index.title \\\n> \t-text [mc \"Staged Changes (Will Commit)\"] \\\n> -\t-background lightgreen -foreground black\n> -ttext $ui_index -background white -foreground black \\\n> +\t-background $Color::lightGreen -foreground $Color::textOnLight\n> +ttext $ui_index -background $Color::textBg -foreground $Color::textColor \\\n> \t-borderwidth 0 \\\n> \t-width 20 -height 10 \\\n> \t-wrap none \\\n> @@ -3432,7 +3437,7 @@ if {![is_enabled nocommit]} {\n> }\n> \n> textframe .vpane.lower.commarea.buffer.frame\n> -ttext $ui_comm -background white -foreground black \\\n> +ttext $ui_comm -background $Color::textBg -foreground $Color::textColor \\\n> \t-borderwidth 1 \\\n> \t-undo true \\\n> \t-maxundo 20 \\\n> @@ -3519,19 +3524,19 @@ trace add variable current_diff_path write trace_current_diff_path\n> \n> gold_frame .vpane.lower.diff.header\n> tlabel .vpane.lower.diff.header.status \\\n> -\t-background gold \\\n> -\t-foreground black \\\n> +\t-background $Color::lightGold \\\n> +\t-foreground $Color::textOnLight \\\n> \t-width $max_status_desc \\\n> \t-anchor w \\\n> \t-justify left\n> tlabel .vpane.lower.diff.header.file \\\n> -\t-background gold \\\n> -\t-foreground black \\\n> +\t-background $Color::lightGold \\\n> +\t-foreground $Color::textOnLight \\\n> \t-anchor w \\\n> \t-justify left\n> tlabel .vpane.lower.diff.header.path \\\n> -\t-background gold \\\n> -\t-foreground blue \\\n> +\t-background $Color::lightGold \\\n> +\t-foreground $Color::lightBlue \\\n> \t-anchor w \\\n> \t-justify left \\\n> \t-font [eval font create [font configure font_ui] -underline 1] \\\n> @@ -3561,7 +3566,7 @@ bind .vpane.lower.diff.header.path {do_file_open $current_diff_path}\n> #\n> textframe .vpane.lower.diff.body\n> set ui_diff .vpane.lower.diff.body.t\n> -ttext $ui_diff -background white -foreground black \\\n> +ttext $ui_diff -background $Color::textBg -foreground $Color::textColor \\\n> \t-borderwidth 0 \\\n> \t-width 80 -height 5 -wrap none \\\n> \t-font font_diff \\\n> @@ -3589,7 +3594,7 @@ foreach {n c} {0 black 1 red4 2 green4 3 yellow4 4 blue4 5 magenta4 6 cyan4 7 gr\n> $ui_diff tag configure clr1 -font font_diffbold\n> $ui_diff tag configure clr4 -underline 1\n> \n> -$ui_diff tag conf d_info -foreground blue -font font_diffbold\n> +$ui_diff tag conf d_info -foreground $Color::lightBlue -font font_diffbold\n> \n> $ui_diff tag conf d_cr -elide true\n> $ui_diff tag conf d_@ -font font_diffbold\n> diff --git a/lib/colored.tcl b/lib/colored.tcl\n> new file mode 100644\n> index 0000000..fdb3f9c\n> --- /dev/null\n> +++ b/lib/colored.tcl\n> @@ -0,0 +1,23 @@\n> +# Color configuration support for git-gui.\n> +\n> +namespace eval Color {\n\nFWIW I don't mind if you just put all this in the global namespace, but \nI'll leave it up to you.\n\n> +\t# static colors\n> +\tvariable lightRed\t\tlightsalmon\n> +\tvariable lightGreen\t\tgreen\n> +\tvariable lightGold\t\tgold\n> +\tvariable lightBlue\t\tblue\n> +\tvariable textOnLight\tblack\n> +\tvariable textOnDark\t\twhite\n\nWhy have `textOnLight`, `textOnDark` and `textColor` separately? My \nguess is that it is for when you want to force light colors regardless \nof the theme? Am I right?\n\n> +\t# theme colors\n> +\tvariable interfaceBg\tlightgray\n> +\tvariable textBg\t\t\twhite\n> +\tvariable textColor\t\tblack\n\nNitpick: please use snake_case for variable names like the rest of the \ncode does. Same for the function name below and the namespace name \nabove.\n\n> +\n> +\tproc syncColorsWithTheme {} {\n> +\t\tset Color::interfaceBg\t[ttk::style lookup Entry -background]\n> +\t\tset Color::textBg\t\t[ttk::style lookup Treeview -background]\n> +\t\tset Color::textColor\t[ttk::style lookup Treeview -foreground]\n> +\n> +\t\ttk_setPalette $Color::interfaceBg\n> +\t}\n> +}\n\nMost of the patch looks good to me apart from my small suggestions. \nThanks for working on this.\n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"406428","messageId":"20200926145443.15423-1-serg.partizan@gmail.com","threadId":"54124","inReplyTo":"20200922110419.ymqj4ol76kg6qshf@yadavpratyush.com","subject":"[PATCH v2] git-gui: Basic dark mode support","fromName":"Serg Tereshchenko","fromEmail":"serg.partizan@gmail.com","sentAt":"2020-09-26T14:54:43Z","receivedAt":"2020-09-26T14:55:14Z","isPatch":true,"sender":{"key":"serg.partizan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/301015?v=4"},"body":"Hi Pratyush.\n\n> Wouldn't having the contents of colored.tcl in themed.tcl be a good \n> idea? The way I see it, colors are part of the theming of the \n> application.\n\nYou are right, fixed this.\n\n> You can set that in the function `rmsel_tag` in git-gui.sh on the line\n\nThanks, it worked!\n\n>> I would be happy to move color definitions from git-gui.sh to\n>> themed.tcl, so we can set it once, and not for each ttext call. Do you\n>> think this is a good idea now or in the future?\n>\n>Do you mean to put the `-foreground` and `-background` options in the \n>function ttext in themed.tcl? If so how can a widget specify if it wants \n>a dark text or light for example?\n\nTurns out ttext was always using black/white colors, so i just removed\nit from ttext calls and used `option add` to set default colors.\n\nAnd if some widget needs to different, it can be implemented like\nexisting gold_frame.\n\nOr like theoretical `ttext_inverse`, which just calls ttext with\n-background -foreground swapped. Or maybe we can come up with something\nbetter. Main idea is to keep all theme-related code in themed.tcl.\n\n> Why have `textOnLight`, `textOnDark` and `textColor` separately? My \n> guess is that it is for when you want to force light colors regardless \n> of the theme? Am I right?\n\nSomething like that, i was using it for tlabel like this:\n> tlabel ... -background $Color::lightGreen -foreground $Color::textOnLight\n\nBut, it was actually not related to current task, so i just reverted\nthat changes and focused only on getting basic dark theme support.\n\n> Nitpick: please use snake_case for variable names like the rest of the \n> code does. Same for the function name below and the namespace name \n> above.\n\nFixed. I was confused by InitTheme and InitEntryFrame.\n\n--\nRegargs,\nSerg Tereshchenko\n\n--- 8< ---\nRemoved forced colors in ttext widget calls,\ninstead using Text.Background/Foreground options.\nThis way colors can be configured dependent on current theme, and even\noverriden by user via .Xresources.\n\nExtracted colors for in_sel/in_diff tags into colors:: namespace,\nwhere they can be configured from current theme colors.\n\nSigned-off-by: Serg Tereshchenko \n---\n git-gui.sh | 17 +++++++++++------\n lib/themed.tcl | 38 ++++++++++++++++++++++++++++++++++++++\n 2 files changed, 49 insertions(+), 6 deletions(-)\n\ndiff --git a/git-gui.sh b/git-gui.sh\nindex d18b902..867b8ce 100755\n--- a/git-gui.sh\n+++ b/git-gui.sh\n@@ -720,7 +720,9 @@ proc rmsel_tag {text} {\n \t\t-background [$text cget -background] \\\n \t\t-foreground [$text cget -foreground] \\\n \t\t-borderwidth 0\n-\t$text tag conf in_sel -background lightgray\n+\t$text tag conf in_sel\\\n+\t\t-background $color::select_bg \\\n+\t\t-foreground $color::select_fg\n \tbind $text break\n \treturn $text\n }\n@@ -863,6 +865,7 @@ proc apply_config {} {\n \t\t\tset NS ttk\n \t\t\tbind [winfo class .] <> [list InitTheme]\n \t\t\tpave_toplevel .\n+\t\t\tcolor::sync_with_theme\n \t\t}\n \t}\n }\n@@ -3272,7 +3275,7 @@ pack .vpane -anchor n -side top -fill both -expand 1\n textframe .vpane.files.workdir -height 100 -width 200\n tlabel .vpane.files.workdir.title -text [mc \"Unstaged Changes\"] \\\n \t-background lightsalmon -foreground black\n-ttext $ui_workdir -background white -foreground black \\\n+ttext $ui_workdir \\\n \t-borderwidth 0 \\\n \t-width 20 -height 10 \\\n \t-wrap none \\\n@@ -3294,7 +3297,7 @@ textframe .vpane.files.index -height 100 -width 200\n tlabel .vpane.files.index.title \\\n \t-text [mc \"Staged Changes (Will Commit)\"] \\\n \t-background lightgreen -foreground black\n-ttext $ui_index -background white -foreground black \\\n+ttext $ui_index \\\n \t-borderwidth 0 \\\n \t-width 20 -height 10 \\\n \t-wrap none \\\n@@ -3321,7 +3324,9 @@ if {!$use_ttk} {\n \n foreach i [list $ui_index $ui_workdir] {\n \trmsel_tag $i\n-\t$i tag conf in_diff -background [$i tag cget in_sel -background]\n+\t$i tag conf in_diff \\\n+\t\t-background $color::select_bg \\\n+\t\t-foreground $color::select_fg\n }\n unset i\n \n@@ -3429,7 +3434,7 @@ if {![is_enabled nocommit]} {\n }\n \n textframe .vpane.lower.commarea.buffer.frame\n-ttext $ui_comm -background white -foreground black \\\n+ttext $ui_comm \\\n \t-borderwidth 1 \\\n \t-undo true \\\n \t-maxundo 20 \\\n@@ -3558,7 +3563,7 @@ bind .vpane.lower.diff.header.path {do_file_open $current_diff_path}\n #\n textframe .vpane.lower.diff.body\n set ui_diff .vpane.lower.diff.body.t\n-ttext $ui_diff -background white -foreground black \\\n+ttext $ui_diff \\\n \t-borderwidth 0 \\\n \t-width 80 -height 5 -wrap none \\\n \t-font font_diff \\\ndiff --git a/lib/themed.tcl b/lib/themed.tcl\nindex 88b3119..83e3ac7 100644\n--- a/lib/themed.tcl\n+++ b/lib/themed.tcl\n@@ -1,6 +1,44 @@\n # Functions for supporting the use of themed Tk widgets in git-gui.\n # Copyright (C) 2009 Pat Thoyts \n \n+\n+namespace eval color {\n+\t# Variable colors\n+\t# Preffered way to set widget colors is using add_option.\n+\t# In some cases, like with tags in_diff/in_sel, we use these colors.\n+\tvariable select_bg\t\tlightgray\n+\tvariable select_fg\t\tblack\n+\n+\tproc sync_with_theme {} {\n+\t\tset base_bg\t\t[ttk::style lookup . -background]\n+\t\tset base_fg\t\t[ttk::style lookup . -foreground]\n+\t\tset text_bg\t\t[ttk::style lookup Treeview -background]\n+\t\tset text_fg\t\t[ttk::style lookup Treeview -foreground]\n+\t\tset select_bg\t[ttk::style lookup Default -selectbackground]\n+\t\tset select_fg\t[ttk::style lookup Default -selectforeground]\n+\n+\t\tset color::select_bg $select_bg\n+\t\tset color::select_fg $select_fg\n+\n+\t\tproc add_option {key val} {\n+\t\t\toption add $key $val widgetDefault\n+\t\t}\n+\t\t# Add options for plain Tk widgets\n+\t\t# Using `option add` instead of tk_setPalette to avoid unintended\n+\t\t# consequences.\n+\t\tif {![is_MacOSX]} {\n+\t\t\tadd_option *Menu.Background $base_bg\n+\t\t\tadd_option *Menu.Foreground $base_fg\n+\t\t\tadd_option *Menu.activeBackground $select_bg\n+\t\t\tadd_option *Menu.activeForeground $select_fg\n+\t\t}\n+\t\tadd_option *Text.Background $text_bg\n+\t\tadd_option *Text.Foreground $text_fg\n+\t\tadd_option *Text.HighlightBackground $base_bg\n+\t\tadd_option *Text.HighlightColor $select_bg\n+\t}\n+}\n+\n proc ttk_get_current_theme {} {\n \t# Handle either current Tk or older versions of 8.5\n \tif {[catch {set theme [ttk::style theme use]}]} {\n-- \n2.28.0\n\n"},{"id":"407044","messageId":"20201007110751.237kem2mlnb7hbrk@yadavpratyush.com","threadId":"54124","inReplyTo":"20200926145443.15423-1-serg.partizan@gmail.com","subject":"Re: [PATCH v2] git-gui: Basic dark mode support","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2020-10-07T11:07:51Z","receivedAt":"2020-10-07T11:08:02Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"Hi Serg,\n\nOn 26/09/20 05:54PM, Serg Tereshchenko wrote:\n> Hi Pratyush.\n> \n> > Wouldn't having the contents of colored.tcl in themed.tcl be a good \n> > idea? The way I see it, colors are part of the theming of the \n> > application.\n> \n> You are right, fixed this.\n> \n> > You can set that in the function `rmsel_tag` in git-gui.sh on the line\n> \n> Thanks, it worked!\n> \n> >> I would be happy to move color definitions from git-gui.sh to\n> >> themed.tcl, so we can set it once, and not for each ttext call. Do you\n> >> think this is a good idea now or in the future?\n> >\n> >Do you mean to put the `-foreground` and `-background` options in the \n> >function ttext in themed.tcl? If so how can a widget specify if it wants \n> >a dark text or light for example?\n> \n> Turns out ttext was always using black/white colors, so i just removed\n> it from ttext calls and used `option add` to set default colors.\n\nOk. Sounds like a good idea.\n \n> And if some widget needs to different, it can be implemented like\n> existing gold_frame.\n> \n> Or like theoretical `ttext_inverse`, which just calls ttext with\n> -background -foreground swapped. Or maybe we can come up with something\n> better. Main idea is to keep all theme-related code in themed.tcl.\n> \n> > Why have `textOnLight`, `textOnDark` and `textColor` separately? My \n> > guess is that it is for when you want to force light colors regardless \n> > of the theme? Am I right?\n> \n> Something like that, i was using it for tlabel like this:\n> > tlabel ... -background $Color::lightGreen -foreground $Color::textOnLight\n> \n> But, it was actually not related to current task, so i just reverted\n> that changes and focused only on getting basic dark theme support.\n\nOk.\n \n> > Nitpick: please use snake_case for variable names like the rest of the \n> > code does. Same for the function name below and the namespace name \n> > above.\n> \n> Fixed. I was confused by InitTheme and InitEntryFrame.\n> \n> --\n> Regargs,\n> Serg Tereshchenko\n> \n> --- 8< ---\n> Removed forced colors in ttext widget calls,\n> instead using Text.Background/Foreground options.\n> This way colors can be configured dependent on current theme, and even\n> overriden by user via .Xresources.\n> \n> Extracted colors for in_sel/in_diff tags into colors:: namespace,\n> where they can be configured from current theme colors.\n\nThe commit message could be improved. It should first describe the \nproblem it is trying to solve, why it is worth solving, and then tell \nthe codebase to fix it. The details of how it is done can be learned \nfrom the contents of the patch, so they are not as important.\n\nHow about the message below?\n\n The colors of some ttext widgets are hard-coded. These hard-coded \n colors are okay with a light theme but with a dark theme some widgets \n are dark colored and the hard-coded ones are still light. This defeats \n the purpose of applying the theme and makes the UI look very awkward.\n\n Remove the hard-coded colors in ttext calls and use colors from the \n theme for those widgets via Text.Background and Text.Foreground from \n the option database.\n\n Similarly, the highlighting for the currently selected file(s) in the \n \"Staged Files\" and \"Unstaged Files\" sections is also hard-coded. Pull \n the colors for that from the current theme to make sure it is in line \n with the rest of the theme colors.\n\nNo need to resend. I'll use this message when applying unless you have \nany suggestions or objections.\n \n> Signed-off-by: Serg Tereshchenko \n> ---\n> git-gui.sh | 17 +++++++++++------\n> lib/themed.tcl | 38 ++++++++++++++++++++++++++++++++++++++\n> 2 files changed, 49 insertions(+), 6 deletions(-)\n\nThe rest of the patch looks good. Will apply. Thanks.\n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"407045","messageId":"20201007111333.iojf5dbwoxbnie3z@yadavpratyush.com","threadId":"54124","inReplyTo":"20200824154835.160749-1-serg.partizan@gmail.com","subject":"Re: [PATCH] git-gui: Basic dark mode support","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2020-10-07T11:13:33Z","receivedAt":"2020-10-07T11:13:41Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"On 24/08/20 06:48PM, Serg Tereshchenko wrote:\n> Hi all.\n> \n> I want to use dark themes with git citool, and here is my first attempt\n> to do so.\n> \n> I am new to tcl, so i happily accept any tips on how to improve code.\n> \n> First things first: to properly support colors, would be nice to have\n> them separated from app code, so i created new file lib/colored.tcl. Name\n> is selected to be consistent with \"lib/themed.tcl\".\n> \n> Then, i extract hardcoded colors from git-gui.sh into namespace Color.\n> Then, if option use_ttk is true, i update default colors for\n> background/foreground from current theme.\n> \n> How it was looking before:\n> - Dark theme (awdark): https://i.imgur.com/0lrfHyq.png\n> - Light theme (clam): https://i.imgur.com/1fsfayJ.png\n> \n> Now looks like this:\n> - Dark theme (awdark): https://i.imgur.com/BISllEH.png\n> - Light theme (clam): https://i.imgur.com/WclSTa4.png\n\nHow do you tell git-gui which theme to use? I had some trouble setting \nthe theme and ended up adding code to source the theme files and then \nset the theme via `ttk::style theme use`. I hope there is a better way \nthan that.\n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"407128","messageId":"20201008082004.5643-1-serg.partizan@gmail.com","threadId":"54124","inReplyTo":"20201007111333.iojf5dbwoxbnie3z@yadavpratyush.com","subject":"Re: [PATCH] git-gui: Basic dark mode support","fromName":"Serg Tereshchenko","fromEmail":"serg.partizan@gmail.com","sentAt":"2020-10-08T08:20:04Z","receivedAt":"2020-10-08T08:20:34Z","isPatch":true,"sender":{"key":"serg.partizan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/301015?v=4"},"body":"> How do you tell git-gui which theme to use? I had some trouble setting \n> the theme and ended up adding code to source the theme files and then \n> set the theme via `ttk::style theme use`. I hope there is a better way \n> than that.\n\nYes, there is. To change theme on the fly use:\n\n echo '*TkTheme: clam' | xrdb -merge -\n\nTo set theme, add \"*TkTheme: clam\" to ~/.Xresources and run\n\n xrdb -merge ~/.Xresources\n\nThere is lack of dark themes in default tk installs right now,\ni'm using awdark: https://sourceforge.net/projects/tcl-awthemes/\n\nTo install theme you need to unpack it somewhere like ~/.local/share/tk-themes/awthemes\nAnd tell tcl where to find it.\n\n export TCLLIBPATH=$HOME/.local/share/tk-themes\n\nI had to modify version numbers inside awthemes package to make in work,\nbut hope it'll be fixed upstream.\n\nHere is blog post which explains this in greater detail:\nhttp://blog.serindu.com/2019/03/07/applying-tk-themes-to-git-gui/\n\n--\nRegards,\nSerg Tereshchenko\n"},{"id":"407129","messageId":"20201008082439.6013-1-serg.partizan@gmail.com","threadId":"54124","inReplyTo":"20201007110751.237kem2mlnb7hbrk@yadavpratyush.com","subject":"Re: [PATCH] git-gui: Basic dark mode support","fromName":"Serg Tereshchenko","fromEmail":"serg.partizan@gmail.com","sentAt":"2020-10-08T08:24:39Z","receivedAt":"2020-10-08T08:24:52Z","isPatch":true,"sender":{"key":"serg.partizan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/301015?v=4"},"body":"> The commit message could be improved. It should first describe the \n> problem it is trying to solve, why it is worth solving, and then tell \n> the codebase to fix it. The details of how it is done can be learned \n> from the contents of the patch, so they are not as important.\n> \n> How about the message below?\n> \n> The colors of some ttext widgets are hard-coded. These hard-coded \n> colors are okay with a light theme but with a dark theme some widgets \n> are dark colored and the hard-coded ones are still light. This defeats \n> the purpose of applying the theme and makes the UI look very awkward.\n> \n> Remove the hard-coded colors in ttext calls and use colors from the \n> theme for those widgets via Text.Background and Text.Foreground from \n> the option database.\n> \n> Similarly, the highlighting for the currently selected file(s) in the \n> \"Staged Files\" and \"Unstaged Files\" sections is also hard-coded. Pull \n> the colors for that from the current theme to make sure it is in line \n> with the rest of the theme colors.\n> \n> No need to resend. I'll use this message when applying unless you have \n> any suggestions or objections.\n\nI have no objections, please use this message.\n\nI'll try to write better commit messages in the future.\n\n--\nRegards,\nSerg Tereshchenko\n"},{"id":"407131","messageId":"20201008082829.h4wno3mntb4kk6oo@yadavpratyush.com","threadId":"54124","inReplyTo":"20201008082004.5643-1-serg.partizan@gmail.com","subject":"Re: [PATCH] git-gui: Basic dark mode support","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2020-10-08T08:28:29Z","receivedAt":"2020-10-08T08:28:35Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"On 08/10/20 11:20AM, Serg Tereshchenko wrote:\n> > How do you tell git-gui which theme to use? I had some trouble setting \n> > the theme and ended up adding code to source the theme files and then \n> > set the theme via `ttk::style theme use`. I hope there is a better way \n> > than that.\n> \n> Yes, there is. To change theme on the fly use:\n> \n> echo '*TkTheme: clam' | xrdb -merge -\n> \n> To set theme, add \"*TkTheme: clam\" to ~/.Xresources and run\n> \n> xrdb -merge ~/.Xresources\n> \n> There is lack of dark themes in default tk installs right now,\n> i'm using awdark: https://sourceforge.net/projects/tcl-awthemes/\n> \n> To install theme you need to unpack it somewhere like ~/.local/share/tk-themes/awthemes\n> And tell tcl where to find it.\n> \n> export TCLLIBPATH=$HOME/.local/share/tk-themes\n> \n> I had to modify version numbers inside awthemes package to make in work,\n> but hope it'll be fixed upstream.\n> \n> Here is blog post which explains this in greater detail:\n> http://blog.serindu.com/2019/03/07/applying-tk-themes-to-git-gui/\n\nThanks. This is a bit complicated to be honest. I don't think we can do \nmuch about the \"installing Tk themes\" part, but we can certainly make it \neasier to select an installed theme in git-gui. A config option like \ngui.tktheme would be good. Something to consider in the future...\n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"407134","messageId":"20201008084406.7494-1-serg.partizan@gmail.com","threadId":"54124","inReplyTo":"20201008082829.h4wno3mntb4kk6oo@yadavpratyush.com","subject":"Re: [PATCH] git-gui: Basic dark mode support","fromName":"Serg Tereshchenko","fromEmail":"serg.partizan@gmail.com","sentAt":"2020-10-08T08:44:06Z","receivedAt":"2020-10-08T08:44:21Z","isPatch":true,"sender":{"key":"serg.partizan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/301015?v=4"},"body":"> Thanks. This is a bit complicated to be honest. I don't think we can do \n> much about the \"installing Tk themes\" part, but we can certainly make it \n> easier to select an installed theme in git-gui. A config option like \n> gui.tktheme would be good. Something to consider in the future...\n\nI think application should not be responsible for setting theme.\n\nOf course, it is simpler for user to set git-gui theme in app config,\nbut right way to do it - is to set theme on system level.\n\nOn mac it is already using aqua (with dark colors if set), and user even\ndon't know about it.\n\nOn windows it uses some windows-like theme by default.\n\nOn linux, yes, we must change ~/.Xresources, but this is just how we set themes\nfor tk apps. It's systemwide, and all tk apps will resect this choice.\n\n--\nRegards,\nSerg Tereshchenko\n"},{"id":"407147","messageId":"20201008130741.mz7k3uy65xdbdkeh@yadavpratyush.com","threadId":"54124","inReplyTo":"20200926145443.15423-1-serg.partizan@gmail.com","subject":"Re: [PATCH v2] git-gui: Basic dark mode support","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2020-10-08T13:07:41Z","receivedAt":"2020-10-08T13:24:10Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"On 26/09/20 05:54PM, Serg Tereshchenko wrote:\n> Removed forced colors in ttext widget calls,\n> instead using Text.Background/Foreground options.\n> This way colors can be configured dependent on current theme, and even\n> overriden by user via .Xresources.\n> \n> Extracted colors for in_sel/in_diff tags into colors:: namespace,\n> where they can be configured from current theme colors.\n> \n> Signed-off-by: Serg Tereshchenko \n> ---\n> git-gui.sh | 17 +++++++++++------\n> lib/themed.tcl | 38 ++++++++++++++++++++++++++++++++++++++\n> 2 files changed, 49 insertions(+), 6 deletions(-)\n\nMerged to git-gui/master. Thanks.\n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"410461","messageId":"7553c99f-1dea-0c1d-e5b0-2103333a76b7@haller-berlin.de","threadId":"54124","inReplyTo":"20201008130741.mz7k3uy65xdbdkeh@yadavpratyush.com","subject":"Re: [PATCH v2] git-gui: Basic dark mode support","fromName":"Stefan Haller","fromEmail":"stefan@haller-berlin.de","sentAt":"2020-11-21T17:47:37Z","receivedAt":"2020-11-21T17:47:41Z","isPatch":true,"sender":{"key":"stefan@haller-berlin.de","avatar":"https://avatars.githubusercontent.com/u/1225667?v=4"},"body":"On 08.10.20 15:07, Pratyush Yadav wrote:\n> On 26/09/20 05:54PM, Serg Tereshchenko wrote:\n>> Removed forced colors in ttext widget calls,\n>> instead using Text.Background/Foreground options.\n>> This way colors can be configured dependent on current theme, and even\n>> overriden by user via .Xresources.\n>>\n>> Extracted colors for in_sel/in_diff tags into colors:: namespace,\n>> where they can be configured from current theme colors.\n>>\n>> Signed-off-by: Serg Tereshchenko \n>> ---\n>> git-gui.sh | 17 +++++++++++------\n>> lib/themed.tcl | 38 ++++++++++++++++++++++++++++++++++++++\n>> 2 files changed, 49 insertions(+), 6 deletions(-)\n> \n> Merged to git-gui/master. Thanks.\n\nThis caused a regression: when selecting text in the diff pane or in the\ncommit message window, the selected text now has a black background (on\nMac and on Windows, I don't have a Linux system to test this). This\nlooks quite ugly; it used to be light blue on both of these systems.\n\nWhen setting gui.usettk to 0, it is light blue as before (as expected).\n\nI'm sorry that I can't give any suggestions how to fix this, because I\nhave trouble understanding the code related to themes, even after\nstaring at it for quite a while this afternoon.\n\nBest,\nStefan\n"},{"id":"410488","messageId":"6R67KQ.86UAEA0ZJLWH2@gmail.com","threadId":"54124","inReplyTo":"7553c99f-1dea-0c1d-e5b0-2103333a76b7@haller-berlin.de","subject":"Re: [PATCH v2] git-gui: Basic dark mode support","fromName":"","fromEmail":"serg.partizan@gmail.com","sentAt":"2020-11-22T12:30:42Z","receivedAt":"2020-11-22T12:30:56Z","isPatch":true,"sender":{"key":"serg.partizan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/301015?v=4"},"body":"\n\nOn Sat, Nov 21, 2020 at 18:47, Stefan Haller \nwrote:\n> This caused a regression: when selecting text in the diff pane or in \n> the\n> commit message window, the selected text now has a black background \n> (on\n> Mac and on Windows, I don't have a Linux system to test this). This\n> looks quite ugly; it used to be light blue on both of these systems.\n> \n> When setting gui.usettk to 0, it is light blue as before (as \n> expected).\n> \n> I'm sorry that I can't give any suggestions how to fix this, because I\n> have trouble understanding the code related to themes, even after\n> staring at it for quite a while this afternoon.\n> \n> Best,\n> Stefan\n\nLooks like it uses inversed text colors for select colors. It works the \nsame way on linux too.\n\nI'll try to figure out why and how it can be fixed.\n\n\n\n"},{"id":"410491","messageId":"20201122133233.7077-1-serg.partizan@gmail.com","threadId":"54124","inReplyTo":"7553c99f-1dea-0c1d-e5b0-2103333a76b7@haller-berlin.de","subject":"[PATCH] git-gui: Fix selected text colors","fromName":"Serg Tereshchenko","fromEmail":"serg.partizan@gmail.com","sentAt":"2020-11-22T13:32:33Z","receivedAt":"2020-11-22T13:33:02Z","isPatch":true,"sender":{"key":"serg.partizan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/301015?v=4"},"body":"Stefan, please check if this fixes select colors for you.\n\n--- 8< ---\n\nAdded selected state colors for text widget.\n\nSame colors for active and inactive selection, to match previous\nbehaviour.\n\nSigned-off-by: Serg Tereshchenko \n---\n lib/themed.tcl | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/lib/themed.tcl b/lib/themed.tcl\nindex 83e3ac7..eda5f8c 100644\n--- a/lib/themed.tcl\n+++ b/lib/themed.tcl\n@@ -34,8 +34,10 @@ namespace eval color {\n \t\t}\n \t\tadd_option *Text.Background $text_bg\n \t\tadd_option *Text.Foreground $text_fg\n-\t\tadd_option *Text.HighlightBackground $base_bg\n-\t\tadd_option *Text.HighlightColor $select_bg\n+\t\tadd_option *Text.selectBackground $select_bg\n+\t\tadd_option *Text.selectForeground $select_fg\n+\t\tadd_option *Text.inactiveSelectBackground $select_bg\n+\t\tadd_option *Text.inactiveSelectForeground $select_fg\n \t}\n }\n \n-- \n2.29.2\n\n"},{"id":"410492","messageId":"2436cd2e-26b9-a7cc-722a-7f27212f58f4@haller-berlin.de","threadId":"54124","inReplyTo":"20201122133233.7077-1-serg.partizan@gmail.com","subject":"Re: [PATCH] git-gui: Fix selected text colors","fromName":"Stefan Haller","fromEmail":"stefan@haller-berlin.de","sentAt":"2020-11-22T15:41:44Z","receivedAt":"2020-11-22T15:42:09Z","isPatch":true,"sender":{"key":"stefan@haller-berlin.de","avatar":"https://avatars.githubusercontent.com/u/1225667?v=4"},"body":"On 22.11.20 14:32, Serg Tereshchenko wrote:\n> Stefan, please check if this fixes select colors for you.\n\nYes, this works. Thanks for the quick fix! I tested on Mac in both light\nand dark mode, and on Windows.\n\n> --- 8< ---\n> \n> Added selected state colors for text widget.\n> \n> Same colors for active and inactive selection, to match previous\n> behaviour.\n\nPreserving the previous behavior is probably a good idea when fixing a\nregression.\n\nHowever, it would actually be nice to have different colors for active\nand inactive selection (could be a follow-up patch). In native Mac and\nWindows applications the active selection background is usually light\nblue, and the inactive one is light grey. This would not just be a\ncosmetic improvement that looks prettier (that wouldn't be worth it),\nbut it would be a real usability improvement because it would make it\nmuch easier to tell which of the four main views has the keyboard focus.\n\nI couldn't find a way to query the inactive selection colors, though. Do\nyou know if there's a way to do that? If not, I guess one way to do this\nis to numerically calculate a grey color with a similar brightness from\nthe active selection background. I could work on a patch if you think\nthis is an approach that makes sense.\n\n-Stefan\n\n\n> Signed-off-by: Serg Tereshchenko \n> ---\n> lib/themed.tcl | 6 ++++--\n> 1 file changed, 4 insertions(+), 2 deletions(-)\n> \n> diff --git a/lib/themed.tcl b/lib/themed.tcl\n> index 83e3ac7..eda5f8c 100644\n> --- a/lib/themed.tcl\n> +++ b/lib/themed.tcl\n> @@ -34,8 +34,10 @@ namespace eval color {\n> \t\t}\n> \t\tadd_option *Text.Background $text_bg\n> \t\tadd_option *Text.Foreground $text_fg\n> -\t\tadd_option *Text.HighlightBackground $base_bg\n> -\t\tadd_option *Text.HighlightColor $select_bg\n> +\t\tadd_option *Text.selectBackground $select_bg\n> +\t\tadd_option *Text.selectForeground $select_fg\n> +\t\tadd_option *Text.inactiveSelectBackground $select_bg\n> +\t\tadd_option *Text.inactiveSelectForeground $select_fg\n> \t}\n> }\n> \n> \n"},{"id":"410494","messageId":"DZJ7KQ.UXACXR9SWDQI3@gmail.com","threadId":"54124","inReplyTo":"2436cd2e-26b9-a7cc-722a-7f27212f58f4@haller-berlin.de","subject":"Re: [PATCH] git-gui: Fix selected text colors","fromName":"","fromEmail":"serg.partizan@gmail.com","sentAt":"2020-11-22T17:16:25Z","receivedAt":"2020-11-22T17:16:36Z","isPatch":true,"sender":{"key":"serg.partizan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/301015?v=4"},"body":"\nOn Sun, Nov 22, 2020 at 16:41, Stefan Haller \nwrote:\n> Preserving the previous behavior is probably a good idea when fixing a\n> regression.\n> \n> However, it would actually be nice to have different colors for active\n> and inactive selection (could be a follow-up patch). In native Mac and\n> Windows applications the active selection background is usually light\n> blue, and the inactive one is light grey. This would not just be a\n> cosmetic improvement that looks prettier (that wouldn't be worth it),\n> but it would be a real usability improvement because it would make it\n> much easier to tell which of the four main views has the keyboard \n> focus.\n> \n> I couldn't find a way to query the inactive selection colors, though. \n> Do\n> you know if there's a way to do that? If not, I guess one way to do \n> this\n> is to numerically calculate a grey color with a similar brightness \n> from\n> the active selection background. I could work on a patch if you think\n> this is an approach that makes sense.\n\nI'm using this code in `wish` to query widget for available options:\n\n > text .t\n > .t configure\n\nAnd it shows this widget has `-inactiveselectbackground` option. \nHowever, it doesn't have `-inactiveselectforeground` as I was thinking \nin previous patch.\n\n > .t configure -inactiveselectbackground\n-inactiveselectbackground inactiveSelectBackground Foreground #c3c3c3 \n#c3c3c3\n\nBut I have no idea how to get this colors from ttk::style. Looking at \nawdark theme, it set's inactiveselectbackground in function \nsetTextColors, which is used on text widget directly. And we cannot use \nit here.\n\nI think calculating that gray color from current selection bg is too \nmuch work for just one color.\n\nWe can just set inactiveSelectBackground to some neutral gray color \nlike #707070 or #909090 which will work fine with both dark and light \nthemes.\n\nAnd, because we're using \"widgetDefault\" priority - themes can override \nthis, when they want to explicitly set this color.\n\n\n"},{"id":"410520","messageId":"20201123114805.48800-1-stefan@haller-berlin.de","threadId":"54124","inReplyTo":"DZJ7KQ.UXACXR9SWDQI3@gmail.com","subject":"[PATCH] git-gui: use gray selection background for inactive text widgets","fromName":"Stefan Haller","fromEmail":"stefan@haller-berlin.de","sentAt":"2020-11-23T11:48:05Z","receivedAt":"2020-11-23T11:48:38Z","isPatch":true,"sender":{"key":"stefan@haller-berlin.de","avatar":"https://avatars.githubusercontent.com/u/1225667?v=4"},"body":"On 22.11.20 18:16, serg.partizan@gmail.com wrote:\n> I think calculating that gray color from current selection bg is too much work\n> for just one color.\n>\n> We can just set inactiveSelectBackground to some neutral gray color like\n> #707070 or #909090 which will work fine with both dark and light themes.\n\nOK, fine with me. Here's a patch that does this (it sits on top of yours). It\nalmost works, except for one problem: on Mac, the inactive selection background\nis white instead of lightgray, but only for the diff view; for the commit editor\nit's correct. On Windows it's also correct for both views. I can't figure out\nwhat's the difference on Mac; do you have an idea what could be wrong?\n\n--- 8< ---\n\nThis makes it easier to see at a glance which of the four main views has the\nkeyboard focus.\n---\n git-gui.sh | 25 +++++++++++++++++++++----\n lib/themed.tcl | 13 +++++++++----\n 2 files changed, 30 insertions(+), 8 deletions(-)\n\ndiff --git a/git-gui.sh b/git-gui.sh\nindex 867b8ce..a8c5cad 100755\n--- a/git-gui.sh\n+++ b/git-gui.sh\n@@ -721,8 +721,8 @@ proc rmsel_tag {text} {\n \t\t-foreground [$text cget -foreground] \\\n \t\t-borderwidth 0\n \t$text tag conf in_sel\\\n-\t\t-background $color::select_bg \\\n-\t\t-foreground $color::select_fg\n+\t\t-background $color::inactive_select_bg \\\n+\t\t-foreground $color::inactive_select_fg\n \tbind $text break\n \treturn $text\n }\n@@ -3325,8 +3325,25 @@ if {!$use_ttk} {\n foreach i [list $ui_index $ui_workdir] {\n \trmsel_tag $i\n \t$i tag conf in_diff \\\n-\t\t-background $color::select_bg \\\n-\t\t-foreground $color::select_fg\n+\t\t-background $color::inactive_select_bg \\\n+\t\t-foreground $color::inactive_select_fg\n+\n+\tif {$use_ttk} {\n+\t\tbind $i {\n+\t\t\tforeach tag [list in_diff in_sel] {\n+\t\t\t\t%W tag conf $tag \\\n+\t\t\t\t\t-background $color::select_bg \\\n+\t\t\t\t\t-foreground $color::select_fg\n+\t\t\t}\n+\t\t}\n+\t\tbind $i {\n+\t\t\tforeach tag [list in_diff in_sel] {\n+\t\t\t\t%W tag conf $tag \\\n+\t\t\t\t\t-background $color::inactive_select_bg \\\n+\t\t\t\t\t-foreground $color::inactive_select_fg\n+\t\t\t}\n+\t\t}\n+\t}\n }\n unset i\n\ndiff --git a/lib/themed.tcl b/lib/themed.tcl\nindex eda5f8c..02b15f2 100644\n--- a/lib/themed.tcl\n+++ b/lib/themed.tcl\n@@ -6,8 +6,10 @@ namespace eval color {\n \t# Variable colors\n \t# Preffered way to set widget colors is using add_option.\n \t# In some cases, like with tags in_diff/in_sel, we use these colors.\n-\tvariable select_bg\t\tlightgray\n-\tvariable select_fg\t\tblack\n+\tvariable select_bg\t\t\t\tlightblue\n+\tvariable select_fg\t\t\t\tblack\n+\tvariable inactive_select_bg\t\tlightgray\n+\tvariable inactive_select_fg\t\tblack\n\n \tproc sync_with_theme {} {\n \t\tset base_bg\t\t[ttk::style lookup . -background]\n@@ -16,6 +18,9 @@ namespace eval color {\n \t\tset text_fg\t\t[ttk::style lookup Treeview -foreground]\n \t\tset select_bg\t[ttk::style lookup Default -selectbackground]\n \t\tset select_fg\t[ttk::style lookup Default -selectforeground]\n+\t\t# We keep inactive_select_bg as the hard-coded light gray above, as\n+\t\t# there doesn't seem to be a way to get it from the theme. Light gray\n+\t\t# should work well for light and dark themes.\n\n \t\tset color::select_bg $select_bg\n \t\tset color::select_fg $select_fg\n@@ -36,8 +41,8 @@ namespace eval color {\n \t\tadd_option *Text.Foreground $text_fg\n \t\tadd_option *Text.selectBackground $select_bg\n \t\tadd_option *Text.selectForeground $select_fg\n-\t\tadd_option *Text.inactiveSelectBackground $select_bg\n-\t\tadd_option *Text.inactiveSelectForeground $select_fg\n+\t\tadd_option *Text.inactiveSelectBackground $color::inactive_select_bg\n+\t\tadd_option *Text.inactiveSelectForeground $color::inactive_select_fg\n \t}\n }\n\n--\n2.29.0.18.gf8c967e53c\n\n"},{"id":"410525","messageId":"JE39KQ.T4FA61XF34XT3@gmail.com","threadId":"54124","inReplyTo":"20201123114805.48800-1-stefan@haller-berlin.de","subject":"Re: [PATCH] git-gui: use gray selection background for inactive text widgets","fromName":"","fromEmail":"serg.partizan@gmail.com","sentAt":"2020-11-23T13:13:31Z","receivedAt":"2020-11-23T13:14:13Z","isPatch":true,"sender":{"key":"serg.partizan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/301015?v=4"},"body":"\n\nOn Mon, Nov 23, 2020 at 12:48, Stefan Haller \nwrote:\n> On 22.11.20 18:16, serg.partizan@gmail.com wrote:\n>> I think calculating that gray color from current selection bg is \n>> too much work\n>> for just one color.\n>> \n>> We can just set inactiveSelectBackground to some neutral gray color \n>> like\n>> #707070 or #909090 which will work fine with both dark and light \n>> themes.\n> \n> OK, fine with me. Here's a patch that does this (it sits on top of \n> yours). It\n> almost works, except for one problem: on Mac, the inactive selection \n> background\n> is white instead of lightgray, but only for the diff view; for the \n> commit editor\n> it's correct. On Windows it's also correct for both views. I can't \n> figure out\n> what's the difference on Mac; do you have an idea what could be wrong?\n> \nI have no idea. Can confirm on linux it works as expected.\n> --- 8< ---\n> \n> This makes it easier to see at a glance which of the four main views \n> has the\n> keyboard focus.\n> ---\n> git-gui.sh | 25 +++++++++++++++++++++----\n> lib/themed.tcl | 13 +++++++++----\n> 2 files changed, 30 insertions(+), 8 deletions(-)\n> \n> diff --git a/git-gui.sh b/git-gui.sh\n> index 867b8ce..a8c5cad 100755\n> --- a/git-gui.sh\n> +++ b/git-gui.sh\n> @@ -721,8 +721,8 @@ proc rmsel_tag {text} {\n> \t\t-foreground [$text cget -foreground] \\\n> \t\t-borderwidth 0\n> \t$text tag conf in_sel\\\n> -\t\t-background $color::select_bg \\\n> -\t\t-foreground $color::select_fg\n> +\t\t-background $color::inactive_select_bg \\\n> +\t\t-foreground $color::inactive_select_fg\n> \tbind $text break\n> \treturn $text\n> }\n> @@ -3325,8 +3325,25 @@ if {!$use_ttk} {\n> foreach i [list $ui_index $ui_workdir] {\n> \trmsel_tag $i\n> \t$i tag conf in_diff \\\n> -\t\t-background $color::select_bg \\\n> -\t\t-foreground $color::select_fg\n> +\t\t-background $color::inactive_select_bg \\\n> +\t\t-foreground $color::inactive_select_fg\n> +\n> +\tif {$use_ttk} {\n\nI think this check can be safely removed. This is all standard tk \nwidgets, and select_bg/fg only changed if use_ttk is true.\n\n> +\t\tbind $i {\n> +\t\t\tforeach tag [list in_diff in_sel] {\n> +\t\t\t\t%W tag conf $tag \\\n> +\t\t\t\t\t-background $color::select_bg \\\n> +\t\t\t\t\t-foreground $color::select_fg\n> +\t\t\t}\n> +\t\t}\n> +\t\tbind $i {\n> +\t\t\tforeach tag [list in_diff in_sel] {\n\nThis two `foreach` can be combined into one?\n\n> +\t\t\t\t%W tag conf $tag \\\n\nAnd this `%W`, probably should be `$i`?\n\n> +\t\t\t\t\t-background $color::inactive_select_bg \\\n> +\t\t\t\t\t-foreground $color::inactive_select_fg\n> +\t\t\t}\n> +\t\t}\n> +\t}\n\nAnd maybe this new code should be grouped into function like \n\"bind_tag_selection_handlers\" to improve readability?\n\n> }\n> unset i\n> \n> diff --git a/lib/themed.tcl b/lib/themed.tcl\n> index eda5f8c..02b15f2 100644\n> --- a/lib/themed.tcl\n> +++ b/lib/themed.tcl\n> @@ -6,8 +6,10 @@ namespace eval color {\n> \t# Variable colors\n> \t# Preffered way to set widget colors is using add_option.\n> \t# In some cases, like with tags in_diff/in_sel, we use these colors.\n> -\tvariable select_bg\t\tlightgray\n> -\tvariable select_fg\t\tblack\n> +\tvariable select_bg\t\t\t\tlightblue\n> +\tvariable select_fg\t\t\t\tblack\n> +\tvariable inactive_select_bg\t\tlightgray\n> +\tvariable inactive_select_fg\t\tblack\n> \n> \tproc sync_with_theme {} {\n> \t\tset base_bg\t\t[ttk::style lookup . -background]\n> @@ -16,6 +18,9 @@ namespace eval color {\n> \t\tset text_fg\t\t[ttk::style lookup Treeview -foreground]\n> \t\tset select_bg\t[ttk::style lookup Default -selectbackground]\n> \t\tset select_fg\t[ttk::style lookup Default -selectforeground]\n> +\t\t# We keep inactive_select_bg as the hard-coded light gray above, as\n> +\t\t# there doesn't seem to be a way to get it from the theme. Light \n> gray\n> +\t\t# should work well for light and dark themes.\n> \n> \t\tset color::select_bg $select_bg\n> \t\tset color::select_fg $select_fg\n> @@ -36,8 +41,8 @@ namespace eval color {\n> \t\tadd_option *Text.Foreground $text_fg\n> \t\tadd_option *Text.selectBackground $select_bg\n> \t\tadd_option *Text.selectForeground $select_fg\n> -\t\tadd_option *Text.inactiveSelectBackground $select_bg\n> -\t\tadd_option *Text.inactiveSelectForeground $select_fg\n> +\t\tadd_option *Text.inactiveSelectBackground \n> $color::inactive_select_bg\n> +\t\tadd_option *Text.inactiveSelectForeground \n> $color::inactive_select_fg\n> \t}\n> }\n> \n> --\n> 2.29.0.18.gf8c967e53c\n> \n\n\n"},{"id":"410547","messageId":"23d6eb6c-4c7b-b9dd-d0df-fe0feaa0dc17@haller-berlin.de","threadId":"54124","inReplyTo":"JE39KQ.T4FA61XF34XT3@gmail.com","subject":"Re: [PATCH] git-gui: use gray selection background for inactive text widgets","fromName":"Stefan Haller","fromEmail":"stefan@haller-berlin.de","sentAt":"2020-11-23T19:03:12Z","receivedAt":"2020-11-23T19:03:35Z","isPatch":true,"sender":{"key":"stefan@haller-berlin.de","avatar":"https://avatars.githubusercontent.com/u/1225667?v=4"},"body":"On 23.11.20 14:13, serg.partizan@gmail.com wrote:\n> \n> \n> On Mon, Nov 23, 2020 at 12:48, Stefan Haller \n> wrote:\n>> On 22.11.20 18:16, serg.partizan@gmail.com wrote:\n>>>  I think calculating that gray color from current selection bg is too much work\n>>>  for just one color.\n>>>\n>>>  We can just set inactiveSelectBackground to some neutral gray color like\n>>>  #707070 or #909090 which will work fine with both dark and light themes.\n>>\n>> OK, fine with me. Here's a patch that does this (it sits on top of\n>> yours). It almost works, except for one problem: on Mac, the\n>> inactive selection background is white instead of lightgray, but\n>> only for the diff view; for the commit editor it's correct. On\n>> Windows it's also correct for both views. I can't figure out what's\n>> the difference on Mac; do you have an idea what could be wrong?\n>>\n> I have no idea. Can confirm on linux it works as expected.\n\nThat's too bad, as I don't think the patch is acceptable with this\ndefect. I could maybe see if I can find something by reading the Tk\nsources, but I'm not really sure where to start, to be honest. Any\nsuggestions appreciated.\n\n\n>> diff --git a/git-gui.sh b/git-gui.sh\n>> index 867b8ce..a8c5cad 100755\n>> --- a/git-gui.sh\n>> +++ b/git-gui.sh\n>> @@ -3325,8 +3325,25 @@ if {!$use_ttk} {\n>>  foreach i [list $ui_index $ui_workdir] {\n>>      rmsel_tag $i\n>>      $i tag conf in_diff \\\n>> -        -background $color::select_bg \\\n>> -        -foreground $color::select_fg\n>> +        -background $color::inactive_select_bg \\\n>> +        -foreground $color::inactive_select_fg\n>> +\n>> +    if {$use_ttk} {\n> \n> I think this check can be safely removed. This is all standard tk\n> widgets, and select_bg/fg only changed if use_ttk is true.\n\nI only added this check because I initialize the select_fg color to\nlightblue in non-ttk mode, so the file lists would switch color even\nthough the text fields don't, and I wanted to avoid that. Of course, if\nI initialize select_fg to lightgray as before, this is not an issue, and\nthe behavior is unchanged in non-ttk mode. I'll change that in v2.\n\n>> +        bind $i {\n>> +            foreach tag [list in_diff in_sel] {\n>> +                %W tag conf $tag \\\n>> +                    -background $color::select_bg \\\n>> +                    -foreground $color::select_fg\n>> +            }\n>> +        }\n>> +        bind $i {\n>> +            foreach tag [list in_diff in_sel] {\n> \n> This two `foreach` can be combined into one?\n\nI don't see how; any concrete suggestions? But I have other ideas how to\nsimplify the code (by using one function set_selection_colors that takes\na has_focus bool and is used for both bindings).\n\n>> +                %W tag conf $tag \\\n> \n> And this `%W`, probably should be `$i`?\n\nNo, $i wouldn't work because we're inside curly braces, so $i wouldn't\nget expanded. It would be possible to work around this by using \"\"\ninstead of {}, but why? Using %W seems to be the idiomatic way in\nbindings, we do this everywhere else too.\n"},{"id":"410548","messageId":"b4571217-ea98-a282-48d3-e9679c600f4c@haller-berlin.de","threadId":"54124","inReplyTo":"DZJ7KQ.UXACXR9SWDQI3@gmail.com","subject":"Re: [PATCH] git-gui: Fix selected text colors","fromName":"Stefan Haller","fromEmail":"stefan@haller-berlin.de","sentAt":"2020-11-23T19:03:32Z","receivedAt":"2020-11-23T19:03:35Z","isPatch":true,"sender":{"key":"stefan@haller-berlin.de","avatar":"https://avatars.githubusercontent.com/u/1225667?v=4"},"body":"On 22.11.20 18:16, serg.partizan@gmail.com wrote:\n> I think calculating that gray color from current selection bg is too\n> much work for just one color.\n> \n> We can just set inactiveSelectBackground to some neutral gray color like\n> #707070 or #909090 which will work fine with both dark and light themes.\n\nI tested this, but it doesn't work well enough, in my opinion. An\n#888888 gray is too dark for normal mode, but too bright for dark mode\non Mac.\n\nCalculating a gray color is not really so difficult, so I'll just do\nthat in v2. The problem is that it needs to be recalculated when the\ntheme changes, and I have trouble testing that because the\n<> event doesn't appear to be sent on Mac, as far as I can\nsee.\n\n-Stefan\n"},{"id":"410562","messageId":"HLM9KQ.QZTOWNY8EICB1@gmail.com","threadId":"54124","inReplyTo":"23d6eb6c-4c7b-b9dd-d0df-fe0feaa0dc17@haller-berlin.de","subject":"Re: [PATCH] git-gui: use gray selection background for inactive text widgets","fromName":"","fromEmail":"serg.partizan@gmail.com","sentAt":"2020-11-23T20:08:05Z","receivedAt":"2020-11-23T20:09:01Z","isPatch":true,"sender":{"key":"serg.partizan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/301015?v=4"},"body":"\n\nOn Mon, Nov 23, 2020 at 20:03, Stefan Haller \nwrote:\n>>> + bind $i {\n>>> + foreach tag [list in_diff in_sel] {\n>>> + %W tag conf $tag \\\n>>> + -background $color::select_bg \\\n>>> + -foreground $color::select_fg\n>>> + }\n>>> + }\n>>> + bind $i {\n>>> + foreach tag [list in_diff in_sel] {\n>> \n>> This two `foreach` can be combined into one?\n> \n> I don't see how; any concrete suggestions? But I have other ideas how \n> to\n> simplify the code (by using one function set_selection_colors that \n> takes\n> a has_focus bool and is used for both bindings).\n\nI tried to do this, and now i understand why my suggestion was wrong, i \nwas looking at this as \"cycle inside cycle\", but it's actually \"cycle \ninside event handler\".\n\n> \n>>> + %W tag conf $tag \\\n>> \n>> And this `%W`, probably should be `$i`?\n> \n> No, $i wouldn't work because we're inside curly braces, so $i wouldn't\n> get expanded. It would be possible to work around this by using \"\"\n> instead of {}, but why? Using %W seems to be the idiomatic way in\n> bindings, we do this everywhere else too.\n\nOh, now i see it's used in the same way in other places!\n\n > %W The path name of the window to which the event was reported (the \nwindow field from the event).\n\nNow I understand it.\n\n\n\n"},{"id":"410569","messageId":"NKO9KQ.ECZZ8I6WPK063@gmail.com","threadId":"54124","inReplyTo":"b4571217-ea98-a282-48d3-e9679c600f4c@haller-berlin.de","subject":"Re: [PATCH] git-gui: Fix selected text colors","fromName":"","fromEmail":"serg.partizan@gmail.com","sentAt":"2020-11-23T20:50:47Z","receivedAt":"2020-11-23T20:51:17Z","isPatch":true,"sender":{"key":"serg.partizan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/301015?v=4"},"body":"\n\nOn Mon, Nov 23, 2020 at 20:03, Stefan Haller \nwrote:\n> The problem is that it needs to be recalculated when the\n> theme changes, and I have trouble testing that because the\n> <> event doesn't appear to be sent on Mac, as far as I \n> can\n> see.\n\nHow are you testing this?\n\nIf I put `puts \"InitTheme\"` into InitTheme which is called on \nThemeChanged, i can see it being called multiple times after git-gui \nstarts, but when I change theme using \"echo '*TkTheme: awdark' | xrdb \n-merge -\", it is not called.\n\nI suppose that signal is called only when theme is changed inside app. \nYes, i just tested this, and that event is sent when you change theme \nfrom the app.\n\nSo you can safely put your code inside \"color::sync_with_theme\".\n\nAnd We should move call to sync_with_theme from git-gui.sh into \nInitTheme. I don't know why I have not put it there before.\n\n\n"},{"id":"410714","messageId":"55348fbb-95bb-1dd2-4e17-4fc622ae7603@haller-berlin.de","threadId":"54124","inReplyTo":"NKO9KQ.ECZZ8I6WPK063@gmail.com","subject":"Re: [PATCH] git-gui: Fix selected text colors","fromName":"Stefan Haller","fromEmail":"stefan@haller-berlin.de","sentAt":"2020-11-24T21:19:59Z","receivedAt":"2020-11-24T21:20:22Z","isPatch":true,"sender":{"key":"stefan@haller-berlin.de","avatar":"https://avatars.githubusercontent.com/u/1225667?v=4"},"body":"On 23.11.20 21:50, serg.partizan@gmail.com wrote:\n> \n> \n> On Mon, Nov 23, 2020 at 20:03, Stefan Haller \n> wrote:\n>> The problem is that it needs to be recalculated when the\n>> theme changes, and I have trouble testing that because the\n>> <> event doesn't appear to be sent on Mac, as far as I can\n>> see.\n> \n> How are you testing this?\n\nBy changing the Appearance setting from Light to Dark or back in Mac's\npreferences window. The git gui window does update dynamically when you\ndo this.\n\nHowever, I think I was wrong when I assumed that this would change the\ntheme; there's only one theme on Mac, the \"aqua\" theme. It just changes\nits colors, it seems.\n\n> So you can safely put your code inside \"color::sync_with_theme\".\n\nWill do; I'll send out v2 in a moment.\n\n> And We should move call to sync_with_theme from git-gui.sh into\n> InitTheme. I don't know why I have not put it there before.\n\nYes, I was wondering this too. But as it doesn't seem to make a\ndifference in practice, I'll leave this for someone else to fix at some\npoint.\n"},{"id":"410715","messageId":"20201124212333.80040-1-stefan@haller-berlin.de","threadId":"54124","inReplyTo":"55348fbb-95bb-1dd2-4e17-4fc622ae7603@haller-berlin.de","subject":"[PATCH v2] git-gui: use gray background for inactive text widgets","fromName":"Stefan Haller","fromEmail":"stefan@haller-berlin.de","sentAt":"2020-11-24T21:23:33Z","receivedAt":"2020-11-24T21:24:05Z","isPatch":true,"sender":{"key":"stefan@haller-berlin.de","avatar":"https://avatars.githubusercontent.com/u/1225667?v=4"},"body":"Second version; it simplifies the code to initialize and update the colors in\nthe two file list views a bit, and it calculates a gray color for the inactive\nselection from the active selection. This looks a lot better in the themes I\nhave tried.\n\nThe bug with the inactive diff selection background on Mac is still there,\nhowever.\n\n--- 8< ---\n\nThis makes it easier to see at a glance which of the four main views has the\nkeyboard focus.\n---\n git-gui.sh | 18 ++++++++++++------\n lib/themed.tcl | 21 +++++++++++++++++----\n 2 files changed, 29 insertions(+), 10 deletions(-)\n\ndiff --git a/git-gui.sh b/git-gui.sh\nindex 867b8ce..e818caa 100755\n--- a/git-gui.sh\n+++ b/git-gui.sh\n@@ -720,9 +720,6 @@ proc rmsel_tag {text} {\n \t\t-background [$text cget -background] \\\n \t\t-foreground [$text cget -foreground] \\\n \t\t-borderwidth 0\n-\t$text tag conf in_sel\\\n-\t\t-background $color::select_bg \\\n-\t\t-foreground $color::select_fg\n \tbind $text break\n \treturn $text\n }\n@@ -3322,11 +3319,20 @@ if {!$use_ttk} {\n \t.vpane.files paneconfigure .vpane.files.index -sticky news\n }\n\n+proc set_selection_colors {w has_focus} {\n+\tforeach tag [list in_diff in_sel] {\n+\t\t$w tag conf $tag \\\n+\t\t\t-background [expr {$has_focus ? $color::select_bg : $color::inactive_select_bg}] \\\n+\t\t\t-foreground [expr {$has_focus ? $color::select_fg : $color::inactive_select_fg}]\n+\t}\n+}\n+\n foreach i [list $ui_index $ui_workdir] {\n \trmsel_tag $i\n-\t$i tag conf in_diff \\\n-\t\t-background $color::select_bg \\\n-\t\t-foreground $color::select_fg\n+\n+\tset_selection_colors $i 0\n+\tbind $i \t{ set_selection_colors %W 1 }\n+\tbind $i \t{ set_selection_colors %W 0 }\n }\n unset i\n\ndiff --git a/lib/themed.tcl b/lib/themed.tcl\nindex eda5f8c..db49085 100644\n--- a/lib/themed.tcl\n+++ b/lib/themed.tcl\n@@ -6,8 +6,10 @@ namespace eval color {\n \t# Variable colors\n \t# Preffered way to set widget colors is using add_option.\n \t# In some cases, like with tags in_diff/in_sel, we use these colors.\n-\tvariable select_bg\t\tlightgray\n-\tvariable select_fg\t\tblack\n+\tvariable select_bg\t\t\t\tlightgray\n+\tvariable select_fg\t\t\t\tblack\n+\tvariable inactive_select_bg\t\tlightgray\n+\tvariable inactive_select_fg\t\tblack\n\n \tproc sync_with_theme {} {\n \t\tset base_bg\t\t[ttk::style lookup . -background]\n@@ -19,6 +21,8 @@ namespace eval color {\n\n \t\tset color::select_bg $select_bg\n \t\tset color::select_fg $select_fg\n+\t\tset color::inactive_select_bg [convert_rgb_to_gray $select_bg]\n+\t\tset color::inactive_select_fg $select_fg\n\n \t\tproc add_option {key val} {\n \t\t\toption add $key $val widgetDefault\n@@ -36,11 +40,20 @@ namespace eval color {\n \t\tadd_option *Text.Foreground $text_fg\n \t\tadd_option *Text.selectBackground $select_bg\n \t\tadd_option *Text.selectForeground $select_fg\n-\t\tadd_option *Text.inactiveSelectBackground $select_bg\n-\t\tadd_option *Text.inactiveSelectForeground $select_fg\n+\t\tadd_option *Text.inactiveSelectBackground $color::inactive_select_bg\n+\t\tadd_option *Text.inactiveSelectForeground $color::inactive_select_fg\n \t}\n }\n\n+proc convert_rgb_to_gray {rgb} {\n+\t# Simply take the average of red, green and blue. This wouldn't be good\n+\t# enough for, say, converting a photo to grayscale, but for this simple\n+\t# purpose of approximating the brightness of a color it's good enough.\n+\tlassign [winfo rgb . $rgb] r g b\n+\tset gray [expr {($r / 256 + $g / 256 + $b / 256) / 3}]\n+\treturn [format \"#%2.2X%2.2X%2.2X\" $gray $gray $gray]\n+}\n+\n proc ttk_get_current_theme {} {\n \t# Handle either current Tk or older versions of 8.5\n \tif {[catch {set theme [ttk::style theme use]}]} {\n--\n2.29.0.21.g59e7d82785.dirty\n\n"},{"id":"410981","messageId":"1a42781d-0e4c-6478-f26d-5eccbd9c6205@haller-berlin.de","threadId":"54124","inReplyTo":"20201123114805.48800-1-stefan@haller-berlin.de","subject":"Re: [PATCH] git-gui: use gray selection background for inactive text widgets","fromName":"Stefan Haller","fromEmail":"stefan@haller-berlin.de","sentAt":"2020-11-29T17:40:33Z","receivedAt":"2020-11-29T17:41:48Z","isPatch":true,"sender":{"key":"stefan@haller-berlin.de","avatar":"https://avatars.githubusercontent.com/u/1225667?v=4"},"body":"On 23.11.20 12:48, Stefan Haller wrote:\n> On 22.11.20 18:16, serg.partizan@gmail.com wrote:\n>> I think calculating that gray color from current selection bg is too much work\n>> for just one color.\n>>\n>> We can just set inactiveSelectBackground to some neutral gray color like\n>> #707070 or #909090 which will work fine with both dark and light themes.\n> \n> OK, fine with me. Here's a patch that does this (it sits on top of yours). It\n> almost works, except for one problem: on Mac, the inactive selection background\n> is white instead of lightgray, but only for the diff view; for the commit editor\n> it's correct. On Windows it's also correct for both views. I can't figure out\n> what's the difference on Mac; do you have an idea what could be wrong?\n\nAfter spending quite a while single-stepping through lots of Tk code, I\nfound the reason. On Mac, disabled text widgets simply don't draw the\nselection background. [1]\n\nI can see three options for solving this:\n\n1) Don't use \"state focus\" and \"state !focus\" on the text widgets, but\n instead set the selection color manually using \"text conf sel\n -background\". Disadvantage: have to calculate the disabled color\n using a heuristic like I did for the file lists in my v2 patch.\n\n2) Don't use \"configure -state disabled\" to make the diff text widget\n read-only; instead, use one of the other methods from [2].\n Disadvantage: quite a big change, and seems complex to me.\n\n3) Enable the the diff widget when it loses focus, and disable it again\n when it gets focus. I tried this in a quick prototype, and it works\n very well. It just *feels* wrong to enable a read-only text widget\n while it is unfocused; but I couldn't find any situation where it\n would behave wrong, because as soon as you try to interact with it,\n the first thing that happens is that it gets disabled again.\n\nI tend towards option 3, because it's reasonably simple and works. I'll\nwork on a patch tomorrow unless anybody has objections.\n\n-Stefan\n\n[1] https://github.com/tcltk/tk/blob/main/generic/tkTextDisp.c#L847\n[2] https://wiki.tcl-lang.org/page/Read-only+text+widget\n"},{"id":"410996","messageId":"WC3MKQ.KLJ4EJGGRQYY2@gmail.com","threadId":"54124","inReplyTo":"1a42781d-0e4c-6478-f26d-5eccbd9c6205@haller-berlin.de","subject":"Re: [PATCH] git-gui: use gray selection background for inactive text widgets","fromName":"","fromEmail":"serg.partizan@gmail.com","sentAt":"2020-11-30T13:41:20Z","receivedAt":"2020-11-30T13:42:11Z","isPatch":true,"sender":{"key":"serg.partizan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/301015?v=4"},"body":"\n\nOn Sun, Nov 29, 2020 at 18:40, Stefan Haller \nwrote:\n> After spending quite a while single-stepping through lots of Tk code, \n> I\n> found the reason. On Mac, disabled text widgets simply don't draw the\n> selection background. [1]\n> \n> I can see three options for solving this:\n> \n> 1) Don't use \"state focus\" and \"state !focus\" on the text widgets, but\n> instead set the selection color manually using \"text conf sel\n> -background\". Disadvantage: have to calculate the disabled color\n> using a heuristic like I did for the file lists in my v2 patch.\n> \n> 2) Don't use \"configure -state disabled\" to make the diff text widget\n> read-only; instead, use one of the other methods from [2].\n> Disadvantage: quite a big change, and seems complex to me.\n> \n> 3) Enable the the diff widget when it loses focus, and disable it \n> again\n> when it gets focus. I tried this in a quick prototype, and it works\n> very well. It just *feels* wrong to enable a read-only text widget\n> while it is unfocused; but I couldn't find any situation where it\n> would behave wrong, because as soon as you try to interact with it,\n> the first thing that happens is that it gets disabled again.\n> \n> I tend towards option 3, because it's reasonably simple and works. \n> I'll\n> work on a patch tomorrow unless anybody has objections.\n> \n\nI don't like any of this options, as it makes code complicated. I \npersonally would prefer to not implement this feature at all, but \nthat's just me.\n\nMaybe Pratyush can say something reasonable about this, as maintainer.\n\nI propose to wait a week or two for other opinions, before starting to \nwrite a patch.\n\n\n"},{"id":"411003","messageId":"20201130180827.2oimhr3vmjq7tzaq@yadavpratyush.com","threadId":"54124","inReplyTo":"WC3MKQ.KLJ4EJGGRQYY2@gmail.com","subject":"Re: [PATCH] git-gui: use gray selection background for inactive text?? widgets","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2020-11-30T18:08:27Z","receivedAt":"2020-11-30T18:09:43Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"Hi,\n\nI have not had the time to go through these patches. I'll try to do it \nin a couple days.\n\nOn 30/11/20 03:41PM, serg.partizan@gmail.com wrote:\n> \n> \n> On Sun, Nov 29, 2020 at 18:40, Stefan Haller \n> wrote:\n> > After spending quite a while single-stepping through lots of Tk code, I\n> > found the reason. On Mac, disabled text widgets simply don't draw the\n> > selection background. [1]\n> > \n> > I can see three options for solving this:\n> > \n> > 1) Don't use \"state focus\" and \"state !focus\" on the text widgets, but\n> > instead set the selection color manually using \"text conf sel\n> > -background\". Disadvantage: have to calculate the disabled color\n> > using a heuristic like I did for the file lists in my v2 patch.\n> > \n> > 2) Don't use \"configure -state disabled\" to make the diff text widget\n> > read-only; instead, use one of the other methods from [2].\n> > Disadvantage: quite a big change, and seems complex to me.\n> > \n> > 3) Enable the the diff widget when it loses focus, and disable it again\n> > when it gets focus. I tried this in a quick prototype, and it works\n> > very well. It just *feels* wrong to enable a read-only text widget\n> > while it is unfocused; but I couldn't find any situation where it\n> > would behave wrong, because as soon as you try to interact with it,\n> > the first thing that happens is that it gets disabled again.\n> > \n> > I tend towards option 3, because it's reasonably simple and works. I'll\n> > work on a patch tomorrow unless anybody has objections.\n> > \n> \n> I don't like any of this options, as it makes code complicated. I personally\n> would prefer to not implement this feature at all, but that's just me.\n\nThat is my first thought as well. All 3 alternatives are less than \nideal. I don't think the problem is big enough to warrant adding hacks \nlike this. They will come back to bite us sooner or later.\n\nIf you _really_ want to fix this, maybe try convincing the Tk devs about \nfixing it.\n \n> Maybe Pratyush can say something reasonable about this, as maintainer.\n> \n> I propose to wait a week or two for other opinions, before starting to write\n> a patch.\n> \n> \n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"411010","messageId":"20201130201837.19091-1-stefan@haller-berlin.de","threadId":"54124","inReplyTo":"20201130180827.2oimhr3vmjq7tzaq@yadavpratyush.com","subject":"Re: [PATCH] git-gui: use gray selection background for inactive text widgets","fromName":"Stefan Haller","fromEmail":"stefan@haller-berlin.de","sentAt":"2020-11-30T20:18:36Z","receivedAt":"2020-11-30T20:19:57Z","isPatch":true,"sender":{"key":"stefan@haller-berlin.de","avatar":"https://avatars.githubusercontent.com/u/1225667?v=4"},"body":"On 30.11.20 19:08, Pratyush Yadav wrote:\n> On 30/11/20 03:41PM, serg.partizan@gmail.com wrote:\n>> On Sun, Nov 29, 2020 at 18:40, Stefan Haller \n>> wrote:\n>>> After spending quite a while single-stepping through lots of Tk code, I\n>>> found the reason. On Mac, disabled text widgets simply don't draw the\n>>> selection background. [1]\n>>>\n>>> I can see three options for solving this:\n>>>\n>>> 1) Don't use \"state focus\" and \"state !focus\" on the text widgets, but\n>>> instead set the selection color manually using \"text conf sel\n>>> -background\". Disadvantage: have to calculate the disabled color\n>>> using a heuristic like I did for the file lists in my v2 patch.\n>>>\n>>> 2) Don't use \"configure -state disabled\" to make the diff text widget\n>>> read-only; instead, use one of the other methods from [2].\n>>> Disadvantage: quite a big change, and seems complex to me.\n>>>\n>>> 3) Enable the the diff widget when it loses focus, and disable it again\n>>> when it gets focus. I tried this in a quick prototype, and it works\n>>> very well. It just *feels* wrong to enable a read-only text widget\n>>> while it is unfocused; but I couldn't find any situation where it\n>>> would behave wrong, because as soon as you try to interact with it,\n>>> the first thing that happens is that it gets disabled again.\n>>>\n>>> I tend towards option 3, because it's reasonably simple and works. I'll\n>>> work on a patch tomorrow unless anybody has objections.\n>>>\n>>\n>> I don't like any of this options, as it makes code complicated. I personally\n>> would prefer to not implement this feature at all, but that's just me.\n>\n> That is my first thought as well. All 3 alternatives are less than\n> ideal. I don't think the problem is big enough to warrant adding hacks\n> like this. They will come back to bite us sooner or later.\n>\n> If you _really_ want to fix this, maybe try convincing the Tk devs about\n> fixing it.\n\nYeah, maybe. Just for the record, here's a patch that does it (in the next\nmessage), and frankly, I don't think it's so bad. I do think it's enough of an\nimprovement that it's worth having. I guess I'll have to maintain it in my local\nbuild if you don't like it.\n\n-Stefan\n"},{"id":"411011","messageId":"20201130201837.19091-2-stefan@haller-berlin.de","threadId":"54124","inReplyTo":"20201130201837.19091-1-stefan@haller-berlin.de","subject":"[PATCH] git-gui: keep showing selection when diff view gets deactivated on Mac","fromName":"Stefan Haller","fromEmail":"stefan@haller-berlin.de","sentAt":"2020-11-30T20:18:37Z","receivedAt":"2020-11-30T20:19:58Z","isPatch":true,"sender":{"key":"stefan@haller-berlin.de","avatar":"https://avatars.githubusercontent.com/u/1225667?v=4"},"body":"On Mac, Tk text widgets don't draw the selection when they are inactive and\ndisabled [1]. This causes the diff selection to disappear on Mac when the diff\nview loses focus.\n\nTo work around that, we configure text views to be enabled when they become\ndeactivated. While this feels wrong, there's not problem with it because as soon\nas the user tries to interact with the view, the first thing that happens is\nthat it gets disabled again.\n\n[1] https://github.com/tcltk/tk/blob/main/generic/tkTextDisp.c#L847\n\nSigned-off-by: Stefan Haller \n---\n lib/themed.tcl | 36 ++++++++++++++++++++++++++++++++++--\n 1 file changed, 34 insertions(+), 2 deletions(-)\n\ndiff --git a/lib/themed.tcl b/lib/themed.tcl\nindex db49085..eeb5bf8 100644\n--- a/lib/themed.tcl\n+++ b/lib/themed.tcl\n@@ -302,6 +302,38 @@ proc tspinbox {w args} {\n \t}\n }\n\n+proc focus_text {w} {\n+\tglobal text_states\n+\n+\t[winfo parent $w] state focus\n+\n+\tif {[is_MacOSX]} {\n+\t\t# Restore the disabled state that we remembered for this widget when it\n+\t\t# got deactivated last. If there's no remembered state, then this is the\n+\t\t# first time we are being activated right after construction, and\n+\t\t# there's no need to change the state.\n+\t\tif {[info exists text_states($w)]} {\n+\t\t\t$w configure -state $text_states($w)\n+\t\t\tunset text_states($w)\n+\t\t}\n+\t}\n+}\n+\n+proc unfocus_text {w} {\n+\tglobal text_states\n+\n+\t[winfo parent $w] state !focus\n+\n+\tif {[is_MacOSX]} {\n+\t\t# On Mac, the selection is not drawn when a text widget is inactive and\n+\t\t# disabled. To work around that, set the disabled state back to normal\n+\t\t# when deactivating the widget. Remember the disabled state so that we\n+\t\t# can restore it when we become active again.\n+\t\tset text_states($w) [lindex [$w configure -state] end]\n+\t\t$w configure -state normal\n+\t}\n+}\n+\n # Create a text widget with any theme specific properties.\n proc ttext {w args} {\n \tglobal use_ttk\n@@ -315,8 +347,8 @@ proc ttext {w args} {\n \tset w [eval [linsert $args 0 text $w]]\n \tif {$use_ttk} {\n \t\tif {[winfo class [winfo parent $w]] eq \"EntryFrame\"} {\n-\t\t\tbind $w {[winfo parent %W] state focus}\n-\t\t\tbind $w {[winfo parent %W] state !focus}\n+\t\t\tbind $w {focus_text %W}\n+\t\t\tbind $w {unfocus_text %W}\n \t\t}\n \t}\n \treturn $w\n--\n2.29.0.21.g59e7d82785.dirty\n\n"},{"id":"412504","messageId":"20201217202352.oia64d35wtwi3cce@yadavpratyush.com","threadId":"54124","inReplyTo":"20201122133233.7077-1-serg.partizan@gmail.com","subject":"Re: [PATCH] git-gui: Fix selected text colors","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2020-12-17T20:23:52Z","receivedAt":"2020-12-17T20:24:38Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"On 22/11/20 03:32PM, Serg Tereshchenko wrote:\n> Stefan, please check if this fixes select colors for you.\n> \n> --- 8< ---\n> \n> Added selected state colors for text widget.\n> \n> Same colors for active and inactive selection, to match previous\n> behaviour.\n> \n> Signed-off-by: Serg Tereshchenko \n> ---\n> lib/themed.tcl | 6 ++++--\n> 1 file changed, 4 insertions(+), 2 deletions(-)\n\nApplied to git-gui/master. Thanks.\n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"412505","messageId":"20201217214912.ycp7bidcyqwzslxy@yadavpratyush.com","threadId":"54124","inReplyTo":"20201124212333.80040-1-stefan@haller-berlin.de","subject":"Re: [PATCH v2] git-gui: use gray background for inactive text widgets","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2020-12-17T21:49:12Z","receivedAt":"2020-12-17T21:49:59Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"Hi,\n\nOn 24/11/20 10:23PM, Stefan Haller wrote:\n> Second version; it simplifies the code to initialize and update the colors in\n> the two file list views a bit, and it calculates a gray color for the inactive\n> selection from the active selection. This looks a lot better in the themes I\n> have tried.\n> \n> The bug with the inactive diff selection background on Mac is still there,\n> however.\n> \n> --- 8< ---\n> \n> This makes it easier to see at a glance which of the four main views has the\n> keyboard focus.\n\nMissing Signed-off-by.\n\n> ---\n> git-gui.sh | 18 ++++++++++++------\n> lib/themed.tcl | 21 +++++++++++++++++----\n> 2 files changed, 29 insertions(+), 10 deletions(-)\n> \n> diff --git a/git-gui.sh b/git-gui.sh\n> index 867b8ce..e818caa 100755\n> --- a/git-gui.sh\n> +++ b/git-gui.sh\n> @@ -720,9 +720,6 @@ proc rmsel_tag {text} {\n> \t\t-background [$text cget -background] \\\n> \t\t-foreground [$text cget -foreground] \\\n> \t\t-borderwidth 0\n> -\t$text tag conf in_sel\\\n> -\t\t-background $color::select_bg \\\n> -\t\t-foreground $color::select_fg\n> \tbind $text break\n> \treturn $text\n> }\n> @@ -3322,11 +3319,20 @@ if {!$use_ttk} {\n> \t.vpane.files paneconfigure .vpane.files.index -sticky news\n> }\n> \n> +proc set_selection_colors {w has_focus} {\n> +\tforeach tag [list in_diff in_sel] {\n> +\t\t$w tag conf $tag \\\n> +\t\t\t-background [expr {$has_focus ? $color::select_bg : $color::inactive_select_bg}] \\\n> +\t\t\t-foreground [expr {$has_focus ? $color::select_fg : $color::inactive_select_fg}]\n> +\t}\n> +}\n> +\n> foreach i [list $ui_index $ui_workdir] {\n> \trmsel_tag $i\n> -\t$i tag conf in_diff \\\n> -\t\t-background $color::select_bg \\\n> -\t\t-foreground $color::select_fg\n> +\n> +\tset_selection_colors $i 0\n> +\tbind $i \t{ set_selection_colors %W 1 }\n> +\tbind $i \t{ set_selection_colors %W 0 }\n> }\n> unset i\n> \n> diff --git a/lib/themed.tcl b/lib/themed.tcl\n> index eda5f8c..db49085 100644\n> --- a/lib/themed.tcl\n> +++ b/lib/themed.tcl\n> @@ -6,8 +6,10 @@ namespace eval color {\n> \t# Variable colors\n> \t# Preffered way to set widget colors is using add_option.\n> \t# In some cases, like with tags in_diff/in_sel, we use these colors.\n> -\tvariable select_bg\t\tlightgray\n> -\tvariable select_fg\t\tblack\n> +\tvariable select_bg\t\t\t\tlightgray\n> +\tvariable select_fg\t\t\t\tblack\n> +\tvariable inactive_select_bg\t\tlightgray\n> +\tvariable inactive_select_fg\t\tblack\n> \n> \tproc sync_with_theme {} {\n> \t\tset base_bg\t\t[ttk::style lookup . -background]\n> @@ -19,6 +21,8 @@ namespace eval color {\n> \n> \t\tset color::select_bg $select_bg\n> \t\tset color::select_fg $select_fg\n> +\t\tset color::inactive_select_bg [convert_rgb_to_gray $select_bg]\n> +\t\tset color::inactive_select_fg $select_fg\n> \n> \t\tproc add_option {key val} {\n> \t\t\toption add $key $val widgetDefault\n> @@ -36,11 +40,20 @@ namespace eval color {\n> \t\tadd_option *Text.Foreground $text_fg\n> \t\tadd_option *Text.selectBackground $select_bg\n> \t\tadd_option *Text.selectForeground $select_fg\n> -\t\tadd_option *Text.inactiveSelectBackground $select_bg\n> -\t\tadd_option *Text.inactiveSelectForeground $select_fg\n> +\t\tadd_option *Text.inactiveSelectBackground $color::inactive_select_bg\n> +\t\tadd_option *Text.inactiveSelectForeground $color::inactive_select_fg\n\nNitpick: Do what is being done for select_bg and select_fg and create a \nlocal variable for it.\n\n> \t}\n> }\n> \n> +proc convert_rgb_to_gray {rgb} {\n> +\t# Simply take the average of red, green and blue. This wouldn't be good\n> +\t# enough for, say, converting a photo to grayscale, but for this simple\n> +\t# purpose of approximating the brightness of a color it's good enough.\n> +\tlassign [winfo rgb . $rgb] r g b\n\nIs there no simpler way to extract r, g, and b? This is a little cryptic \nto be honest.\n\n> +\tset gray [expr {($r / 256 + $g / 256 + $b / 256) / 3}]\n> +\treturn [format \"#%2.2X%2.2X%2.2X\" $gray $gray $gray]\n> +}\n> +\n> proc ttk_get_current_theme {} {\n> \t# Handle either current Tk or older versions of 8.5\n> \tif {[catch {set theme [ttk::style theme use]}]} {\n\nThe patch looks good for the most part. I can fix the above nitpick \nlocally and merge it in tomorrow if you send me your signoff by then. I \ndon't want to hold off the PR to Junio too much longer. A simple reply \ncontaining your Signed-off-by should be fine. Thanks.\n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"412506","messageId":"63356203-8af0-64e5-4694-f47d31bcdee6@haller-berlin.de","threadId":"54124","inReplyTo":"20201217214912.ycp7bidcyqwzslxy@yadavpratyush.com","subject":"Re: [PATCH v2] git-gui: use gray background for inactive text widgets","fromName":"Stefan Haller","fromEmail":"stefan@haller-berlin.de","sentAt":"2020-12-17T22:14:00Z","receivedAt":"2020-12-17T22:14:44Z","isPatch":true,"sender":{"key":"stefan@haller-berlin.de","avatar":"https://avatars.githubusercontent.com/u/1225667?v=4"},"body":"On 17.12.20 22:49, Pratyush Yadav wrote:\n> Hi,\n> \n> On 24/11/20 10:23PM, Stefan Haller wrote:\n>> Second version; it simplifies the code to initialize and update the colors in\n>> the two file list views a bit, and it calculates a gray color for the inactive\n>> selection from the active selection. This looks a lot better in the themes I\n>> have tried.\n>>\n>> The bug with the inactive diff selection background on Mac is still there,\n>> however.\n>>\n>> --- 8< ---\n>>\n>> This makes it easier to see at a glance which of the four main views has the\n>> keyboard focus.\n> \n> Missing Signed-off-by.\n\nIf you are willing to add this in yourself, that would be nice, I don't\nhave time to send another patch today:\n\nSigned-off-by: Stefan Haller \n\n>> ---\n>> git-gui.sh | 18 ++++++++++++------\n>> lib/themed.tcl | 21 +++++++++++++++++----\n>> 2 files changed, 29 insertions(+), 10 deletions(-)\n>>\n>> diff --git a/git-gui.sh b/git-gui.sh\n>> index 867b8ce..e818caa 100755\n>> --- a/git-gui.sh\n>> +++ b/git-gui.sh\n>> @@ -720,9 +720,6 @@ proc rmsel_tag {text} {\n>> \t\t-background [$text cget -background] \\\n>> \t\t-foreground [$text cget -foreground] \\\n>> \t\t-borderwidth 0\n>> -\t$text tag conf in_sel\\\n>> -\t\t-background $color::select_bg \\\n>> -\t\t-foreground $color::select_fg\n>> \tbind $text break\n>> \treturn $text\n>> }\n>> @@ -3322,11 +3319,20 @@ if {!$use_ttk} {\n>> \t.vpane.files paneconfigure .vpane.files.index -sticky news\n>> }\n>>\n>> +proc set_selection_colors {w has_focus} {\n>> +\tforeach tag [list in_diff in_sel] {\n>> +\t\t$w tag conf $tag \\\n>> +\t\t\t-background [expr {$has_focus ? $color::select_bg : $color::inactive_select_bg}] \\\n>> +\t\t\t-foreground [expr {$has_focus ? $color::select_fg : $color::inactive_select_fg}]\n>> +\t}\n>> +}\n>> +\n>> foreach i [list $ui_index $ui_workdir] {\n>> \trmsel_tag $i\n>> -\t$i tag conf in_diff \\\n>> -\t\t-background $color::select_bg \\\n>> -\t\t-foreground $color::select_fg\n>> +\n>> +\tset_selection_colors $i 0\n>> +\tbind $i \t{ set_selection_colors %W 1 }\n>> +\tbind $i \t{ set_selection_colors %W 0 }\n>> }\n>> unset i\n>>\n>> diff --git a/lib/themed.tcl b/lib/themed.tcl\n>> index eda5f8c..db49085 100644\n>> --- a/lib/themed.tcl\n>> +++ b/lib/themed.tcl\n>> @@ -6,8 +6,10 @@ namespace eval color {\n>> \t# Variable colors\n>> \t# Preffered way to set widget colors is using add_option.\n>> \t# In some cases, like with tags in_diff/in_sel, we use these colors.\n>> -\tvariable select_bg\t\tlightgray\n>> -\tvariable select_fg\t\tblack\n>> +\tvariable select_bg\t\t\t\tlightgray\n>> +\tvariable select_fg\t\t\t\tblack\n>> +\tvariable inactive_select_bg\t\tlightgray\n>> +\tvariable inactive_select_fg\t\tblack\n>>\n>> \tproc sync_with_theme {} {\n>> \t\tset base_bg\t\t[ttk::style lookup . -background]\n>> @@ -19,6 +21,8 @@ namespace eval color {\n>>\n>> \t\tset color::select_bg $select_bg\n>> \t\tset color::select_fg $select_fg\n>> +\t\tset color::inactive_select_bg [convert_rgb_to_gray $select_bg]\n>> +\t\tset color::inactive_select_fg $select_fg\n>>\n>> \t\tproc add_option {key val} {\n>> \t\t\toption add $key $val widgetDefault\n>> @@ -36,11 +40,20 @@ namespace eval color {\n>> \t\tadd_option *Text.Foreground $text_fg\n>> \t\tadd_option *Text.selectBackground $select_bg\n>> \t\tadd_option *Text.selectForeground $select_fg\n>> -\t\tadd_option *Text.inactiveSelectBackground $select_bg\n>> -\t\tadd_option *Text.inactiveSelectForeground $select_fg\n>> +\t\tadd_option *Text.inactiveSelectBackground $color::inactive_select_bg\n>> +\t\tadd_option *Text.inactiveSelectForeground $color::inactive_select_fg\n> \n> Nitpick: Do what is being done for select_bg and select_fg and create a \n> local variable for it.\n> \n>> \t}\n>> }\n>>\n>> +proc convert_rgb_to_gray {rgb} {\n>> +\t# Simply take the average of red, green and blue. This wouldn't be good\n>> +\t# enough for, say, converting a photo to grayscale, but for this simple\n>> +\t# purpose of approximating the brightness of a color it's good enough.\n>> +\tlassign [winfo rgb . $rgb] r g b\n> \n> Is there no simpler way to extract r, g, and b? This is a little cryptic \n> to be honest.\n\nActually, I find this pretty elegant, and from what I have seen, it's\nidiomatic Tcl. A less cryptic way would be (untested):\n\n set components [winfo rgb . $rgb]\n set r [lindex $components 0]\n set g [lindex $components 1]\n set b [lindex $components 2]\n\nBut I much prefer the one-line version.\n\n>> +\tset gray [expr {($r / 256 + $g / 256 + $b / 256) / 3}]\n>> +\treturn [format \"#%2.2X%2.2X%2.2X\" $gray $gray $gray]\n>> +}\n>> +\n>> proc ttk_get_current_theme {} {\n>> \t# Handle either current Tk or older versions of 8.5\n>> \tif {[catch {set theme [ttk::style theme use]}]} {\n> \n> The patch looks good for the most part. I can fix the above nitpick \n> locally and merge it in tomorrow if you send me your signoff by then. I \n> don't want to hold off the PR to Junio too much longer. A simple reply \n> containing your Signed-off-by should be fine. Thanks.\n\nAwesome, thanks for that.\n\n-Stefan\n"},{"id":"412545","messageId":"20201218094314.22138-1-stefan@haller-berlin.de","threadId":"54124","inReplyTo":"20201217214912.ycp7bidcyqwzslxy@yadavpratyush.com","subject":"[PATCH v3] git-gui: use gray background for inactive text widgets","fromName":"Stefan Haller","fromEmail":"stefan@haller-berlin.de","sentAt":"2020-12-18T09:43:14Z","receivedAt":"2020-12-18T09:44:48Z","isPatch":true,"sender":{"key":"stefan@haller-berlin.de","avatar":"https://avatars.githubusercontent.com/u/1225667?v=4"},"body":"This makes it easier to see at a glance which of the four main views has the\nkeyboard focus.\n\nSigned-off-by: Stefan Haller \n---\nHi Pratyush,\n\nhere's a rerolled version with added Signed-off-by and using local variables for\ninactive_select_bg and inactive_select_bg, hoping this might save you some time.\nLet me know if this is what you had in mind, I wasn't totally sure.\n\n git-gui.sh | 18 ++++++++++++------\n lib/themed.tcl | 35 +++++++++++++++++++++++++----------\n 2 files changed, 37 insertions(+), 16 deletions(-)\n\ndiff --git a/git-gui.sh b/git-gui.sh\nindex cc6c2aa..201524c 100755\n--- a/git-gui.sh\n+++ b/git-gui.sh\n@@ -720,9 +720,6 @@ proc rmsel_tag {text} {\n \t\t-background [$text cget -background] \\\n \t\t-foreground [$text cget -foreground] \\\n \t\t-borderwidth 0\n-\t$text tag conf in_sel\\\n-\t\t-background $color::select_bg \\\n-\t\t-foreground $color::select_fg\n \tbind $text break\n \treturn $text\n }\n@@ -3328,11 +3325,20 @@ if {!$use_ttk} {\n \t.vpane.files paneconfigure .vpane.files.index -sticky news\n }\n\n+proc set_selection_colors {w has_focus} {\n+\tforeach tag [list in_diff in_sel] {\n+\t\t$w tag conf $tag \\\n+\t\t\t-background [expr {$has_focus ? $color::select_bg : $color::inactive_select_bg}] \\\n+\t\t\t-foreground [expr {$has_focus ? $color::select_fg : $color::inactive_select_fg}]\n+\t}\n+}\n+\n foreach i [list $ui_index $ui_workdir] {\n \trmsel_tag $i\n-\t$i tag conf in_diff \\\n-\t\t-background $color::select_bg \\\n-\t\t-foreground $color::select_fg\n+\n+\tset_selection_colors $i 0\n+\tbind $i \t{ set_selection_colors %W 1 }\n+\tbind $i \t{ set_selection_colors %W 0 }\n }\n unset i\n\ndiff --git a/lib/themed.tcl b/lib/themed.tcl\nindex 244c061..0f6575a 100644\n--- a/lib/themed.tcl\n+++ b/lib/themed.tcl\n@@ -6,19 +6,25 @@ namespace eval color {\n \t# Variable colors\n \t# Preffered way to set widget colors is using add_option.\n \t# In some cases, like with tags in_diff/in_sel, we use these colors.\n-\tvariable select_bg\t\tlightgray\n-\tvariable select_fg\t\tblack\n+\tvariable select_bg\t\t\t\tlightgray\n+\tvariable select_fg\t\t\t\tblack\n+\tvariable inactive_select_bg\t\tlightgray\n+\tvariable inactive_select_fg\t\tblack\n\n \tproc sync_with_theme {} {\n-\t\tset base_bg\t\t[ttk::style lookup . -background]\n-\t\tset base_fg\t\t[ttk::style lookup . -foreground]\n-\t\tset text_bg\t\t[ttk::style lookup Treeview -background]\n-\t\tset text_fg\t\t[ttk::style lookup Treeview -foreground]\n-\t\tset select_bg\t[ttk::style lookup Default -selectbackground]\n-\t\tset select_fg\t[ttk::style lookup Default -selectforeground]\n+\t\tset base_bg\t\t [ttk::style lookup . -background]\n+\t\tset base_fg\t\t [ttk::style lookup . -foreground]\n+\t\tset text_bg\t\t [ttk::style lookup Treeview -background]\n+\t\tset text_fg\t\t [ttk::style lookup Treeview -foreground]\n+\t\tset select_bg\t [ttk::style lookup Default -selectbackground]\n+\t\tset select_fg\t [ttk::style lookup Default -selectforeground]\n+\t\tset inactive_select_bg\t[convert_rgb_to_gray $select_bg]\n+\t\tset inactive_select_fg \t$select_fg\n\n \t\tset color::select_bg $select_bg\n \t\tset color::select_fg $select_fg\n+\t\tset color::inactive_select_bg $inactive_select_bg\n+\t\tset color::inactive_select_fg $inactive_select_fg\n\n \t\tproc add_option {key val} {\n \t\t\toption add $key $val widgetDefault\n@@ -36,11 +42,20 @@ namespace eval color {\n \t\tadd_option *Text.Foreground $text_fg\n \t\tadd_option *Text.selectBackground $select_bg\n \t\tadd_option *Text.selectForeground $select_fg\n-\t\tadd_option *Text.inactiveSelectBackground $select_bg\n-\t\tadd_option *Text.inactiveSelectForeground $select_fg\n+\t\tadd_option *Text.inactiveSelectBackground $inactive_select_bg\n+\t\tadd_option *Text.inactiveSelectForeground $inactive_select_fg\n \t}\n }\n\n+proc convert_rgb_to_gray {rgb} {\n+\t# Simply take the average of red, green and blue. This wouldn't be good\n+\t# enough for, say, converting a photo to grayscale, but for this simple\n+\t# purpose of approximating the brightness of a color it's good enough.\n+\tlassign [winfo rgb . $rgb] r g b\n+\tset gray [expr {($r / 256 + $g / 256 + $b / 256) / 3}]\n+\treturn [format \"#%2.2X%2.2X%2.2X\" $gray $gray $gray]\n+}\n+\n proc ttk_get_current_theme {} {\n \t# Handle either current Tk or older versions of 8.5\n \tif {[catch {set theme [ttk::style theme use]}]} {\n--\n2.29.0.21.g59e7d82785.dirty\n\n"},{"id":"412557","messageId":"20201218125025.gj277jwxrblqp45b@yadavpratyush.com","threadId":"54124","inReplyTo":"63356203-8af0-64e5-4694-f47d31bcdee6@haller-berlin.de","subject":"Re: [PATCH v2] git-gui: use gray background for inactive text widgets","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2020-12-18T12:50:25Z","receivedAt":"2020-12-18T12:51:19Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"On 17/12/20 11:14PM, Stefan Haller wrote:\n> On 17.12.20 22:49, Pratyush Yadav wrote:\n> > Hi,\n> > \n> > On 24/11/20 10:23PM, Stefan Haller wrote:\n> >> Second version; it simplifies the code to initialize and update the colors in\n> >> the two file list views a bit, and it calculates a gray color for the inactive\n> >> selection from the active selection. This looks a lot better in the themes I\n> >> have tried.\n> >>\n> >> The bug with the inactive diff selection background on Mac is still there,\n> >> however.\n> >>\n> >> --- 8< ---\n> >>\n> >> This makes it easier to see at a glance which of the four main views has the\n> >> keyboard focus.\n> > \n> > Missing Signed-off-by.\n> \n> If you are willing to add this in yourself, that would be nice, I don't\n> have time to send another patch today:\n> \n> Signed-off-by: Stefan Haller \n> \n> >> ---\n> >> git-gui.sh | 18 ++++++++++++------\n> >> lib/themed.tcl | 21 +++++++++++++++++----\n> >> 2 files changed, 29 insertions(+), 10 deletions(-)\n> >>\n> >> diff --git a/git-gui.sh b/git-gui.sh\n> >> index 867b8ce..e818caa 100755\n> >> --- a/git-gui.sh\n> >> +++ b/git-gui.sh\n> >> @@ -720,9 +720,6 @@ proc rmsel_tag {text} {\n> >> \t\t-background [$text cget -background] \\\n> >> \t\t-foreground [$text cget -foreground] \\\n> >> \t\t-borderwidth 0\n> >> -\t$text tag conf in_sel\\\n> >> -\t\t-background $color::select_bg \\\n> >> -\t\t-foreground $color::select_fg\n> >> \tbind $text break\n> >> \treturn $text\n> >> }\n> >> @@ -3322,11 +3319,20 @@ if {!$use_ttk} {\n> >> \t.vpane.files paneconfigure .vpane.files.index -sticky news\n> >> }\n> >>\n> >> +proc set_selection_colors {w has_focus} {\n> >> +\tforeach tag [list in_diff in_sel] {\n> >> +\t\t$w tag conf $tag \\\n> >> +\t\t\t-background [expr {$has_focus ? $color::select_bg : $color::inactive_select_bg}] \\\n> >> +\t\t\t-foreground [expr {$has_focus ? $color::select_fg : $color::inactive_select_fg}]\n> >> +\t}\n> >> +}\n> >> +\n> >> foreach i [list $ui_index $ui_workdir] {\n> >> \trmsel_tag $i\n> >> -\t$i tag conf in_diff \\\n> >> -\t\t-background $color::select_bg \\\n> >> -\t\t-foreground $color::select_fg\n> >> +\n> >> +\tset_selection_colors $i 0\n> >> +\tbind $i \t{ set_selection_colors %W 1 }\n> >> +\tbind $i \t{ set_selection_colors %W 0 }\n> >> }\n> >> unset i\n> >>\n> >> diff --git a/lib/themed.tcl b/lib/themed.tcl\n> >> index eda5f8c..db49085 100644\n> >> --- a/lib/themed.tcl\n> >> +++ b/lib/themed.tcl\n> >> @@ -6,8 +6,10 @@ namespace eval color {\n> >> \t# Variable colors\n> >> \t# Preffered way to set widget colors is using add_option.\n> >> \t# In some cases, like with tags in_diff/in_sel, we use these colors.\n> >> -\tvariable select_bg\t\tlightgray\n> >> -\tvariable select_fg\t\tblack\n> >> +\tvariable select_bg\t\t\t\tlightgray\n> >> +\tvariable select_fg\t\t\t\tblack\n> >> +\tvariable inactive_select_bg\t\tlightgray\n> >> +\tvariable inactive_select_fg\t\tblack\n> >>\n> >> \tproc sync_with_theme {} {\n> >> \t\tset base_bg\t\t[ttk::style lookup . -background]\n> >> @@ -19,6 +21,8 @@ namespace eval color {\n> >>\n> >> \t\tset color::select_bg $select_bg\n> >> \t\tset color::select_fg $select_fg\n> >> +\t\tset color::inactive_select_bg [convert_rgb_to_gray $select_bg]\n> >> +\t\tset color::inactive_select_fg $select_fg\n> >>\n> >> \t\tproc add_option {key val} {\n> >> \t\t\toption add $key $val widgetDefault\n> >> @@ -36,11 +40,20 @@ namespace eval color {\n> >> \t\tadd_option *Text.Foreground $text_fg\n> >> \t\tadd_option *Text.selectBackground $select_bg\n> >> \t\tadd_option *Text.selectForeground $select_fg\n> >> -\t\tadd_option *Text.inactiveSelectBackground $select_bg\n> >> -\t\tadd_option *Text.inactiveSelectForeground $select_fg\n> >> +\t\tadd_option *Text.inactiveSelectBackground $color::inactive_select_bg\n> >> +\t\tadd_option *Text.inactiveSelectForeground $color::inactive_select_fg\n> > \n> > Nitpick: Do what is being done for select_bg and select_fg and create a \n> > local variable for it.\n> > \n> >> \t}\n> >> }\n> >>\n> >> +proc convert_rgb_to_gray {rgb} {\n> >> +\t# Simply take the average of red, green and blue. This wouldn't be good\n> >> +\t# enough for, say, converting a photo to grayscale, but for this simple\n> >> +\t# purpose of approximating the brightness of a color it's good enough.\n> >> +\tlassign [winfo rgb . $rgb] r g b\n> > \n> > Is there no simpler way to extract r, g, and b? This is a little cryptic \n> > to be honest.\n> \n> Actually, I find this pretty elegant, and from what I have seen, it's\n> idiomatic Tcl. A less cryptic way would be (untested):\n> \n> set components [winfo rgb . $rgb]\n> set r [lindex $components 0]\n> set g [lindex $components 1]\n> set b [lindex $components 2]\n> \n> But I much prefer the one-line version.\n\nI agree. Using lassign is much neater. But that is not my point. I am \ntalking about the \"[winfo rgb . $rgb]\". This call generates the list \nthat is then assigned to the 3 variables. This part is a little cryptic. \nIs there no simpler way to separate out the r, g, and b values?\n \n> >> +\tset gray [expr {($r / 256 + $g / 256 + $b / 256) / 3}]\n> >> +\treturn [format \"#%2.2X%2.2X%2.2X\" $gray $gray $gray]\n> >> +}\n> >> +\n> >> proc ttk_get_current_theme {} {\n> >> \t# Handle either current Tk or older versions of 8.5\n> >> \tif {[catch {set theme [ttk::style theme use]}]} {\n> > \n> > The patch looks good for the most part. I can fix the above nitpick \n> > locally and merge it in tomorrow if you send me your signoff by then. I \n> > don't want to hold off the PR to Junio too much longer. A simple reply \n> > containing your Signed-off-by should be fine. Thanks.\n> \n> Awesome, thanks for that.\n> \n> -Stefan\n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"412558","messageId":"20201218125151.zbuye67xbka7qphe@yadavpratyush.com","threadId":"54124","inReplyTo":"20201218094314.22138-1-stefan@haller-berlin.de","subject":"Re: [PATCH v3] git-gui: use gray background for inactive text widgets","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2020-12-18T12:51:51Z","receivedAt":"2020-12-18T12:52:37Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"On 18/12/20 10:43AM, Stefan Haller wrote:\n> This makes it easier to see at a glance which of the four main views has the\n> keyboard focus.\n> \n> Signed-off-by: Stefan Haller \n> ---\n> Hi Pratyush,\n> \n> here's a rerolled version with added Signed-off-by and using local variables for\n> inactive_select_bg and inactive_select_bg, hoping this might save you some time.\n> Let me know if this is what you had in mind, I wasn't totally sure.\n\nYes, this is what I wanted. Thanks. Will apply.\n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"412559","messageId":"90975583-17ad-0d53-5919-fe1c8400291c@haller-berlin.de","threadId":"54124","inReplyTo":"20201218125025.gj277jwxrblqp45b@yadavpratyush.com","subject":"Re: [PATCH v2] git-gui: use gray background for inactive text widgets","fromName":"Stefan Haller","fromEmail":"stefan@haller-berlin.de","sentAt":"2020-12-18T13:01:11Z","receivedAt":"2020-12-18T13:02:11Z","isPatch":true,"sender":{"key":"stefan@haller-berlin.de","avatar":"https://avatars.githubusercontent.com/u/1225667?v=4"},"body":"On 18.12.20 13:50, Pratyush Yadav wrote:\n> On 17/12/20 11:14PM, Stefan Haller wrote:\n>> On 17.12.20 22:49, Pratyush Yadav wrote:\n>>> Hi,\n>>>\n>>> On 24/11/20 10:23PM, Stefan Haller wrote>>>> +proc convert_rgb_to_gray {rgb} {\n>>>> +\t# Simply take the average of red, green and blue. This wouldn't be good\n>>>> +\t# enough for, say, converting a photo to grayscale, but for this simple\n>>>> +\t# purpose of approximating the brightness of a color it's good enough.\n>>>> +\tlassign [winfo rgb . $rgb] r g b\n>>>\n>>> Is there no simpler way to extract r, g, and b? This is a little cryptic \n>>> to be honest.\n>>\n>> Actually, I find this pretty elegant, and from what I have seen, it's\n>> idiomatic Tcl. A less cryptic way would be (untested):\n>>\n>> set components [winfo rgb . $rgb]\n>> set r [lindex $components 0]\n>> set g [lindex $components 1]\n>> set b [lindex $components 2]\n>>\n>> But I much prefer the one-line version.\n> \n> I agree. Using lassign is much neater. But that is not my point. I am \n> talking about the \"[winfo rgb . $rgb]\". This call generates the list \n> that is then assigned to the 3 variables. This part is a little cryptic. \n> Is there no simpler way to separate out the r, g, and b values?\nAccording to the winfo man page this is the only way to do this;\nsee [1].\n\nInstead of the lassign, you can also do\n\n foreach {r g b} [winfo rgb . $rgb] {}\n\nbut I don't think that's better.\n\n-Stefan\n\n\n[1] https://www.tcl.tk/man/tcl8.6/TkCmd/winfo.htm\n"},{"id":"412579","messageId":"20201218194618.ehwijoyiboluonzf@yadavpratyush.com","threadId":"54124","inReplyTo":"20201218094314.22138-1-stefan@haller-berlin.de","subject":"Re: [PATCH v3] git-gui: use gray background for inactive text widgets","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2020-12-18T19:46:18Z","receivedAt":"2020-12-18T19:47:04Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"On 18/12/20 10:43AM, Stefan Haller wrote:\n> This makes it easier to see at a glance which of the four main views has the\n> keyboard focus.\n> \n> Signed-off-by: Stefan Haller \n> ---\n> Hi Pratyush,\n> \n> here's a rerolled version with added Signed-off-by and using local variables for\n> inactive_select_bg and inactive_select_bg, hoping this might save you some time.\n> Let me know if this is what you had in mind, I wasn't totally sure.\n> \n> git-gui.sh | 18 ++++++++++++------\n> lib/themed.tcl | 35 +++++++++++++++++++++++++----------\n> 2 files changed, 37 insertions(+), 16 deletions(-)\n> \n> diff --git a/git-gui.sh b/git-gui.sh\n> index cc6c2aa..201524c 100755\n> --- a/git-gui.sh\n> +++ b/git-gui.sh\n> @@ -720,9 +720,6 @@ proc rmsel_tag {text} {\n> \t\t-background [$text cget -background] \\\n> \t\t-foreground [$text cget -foreground] \\\n> \t\t-borderwidth 0\n> -\t$text tag conf in_sel\\\n> -\t\t-background $color::select_bg \\\n> -\t\t-foreground $color::select_fg\n> \tbind $text break\n> \treturn $text\n> }\n> @@ -3328,11 +3325,20 @@ if {!$use_ttk} {\n> \t.vpane.files paneconfigure .vpane.files.index -sticky news\n> }\n> \n> +proc set_selection_colors {w has_focus} {\n> +\tforeach tag [list in_diff in_sel] {\n> +\t\t$w tag conf $tag \\\n> +\t\t\t-background [expr {$has_focus ? $color::select_bg : $color::inactive_select_bg}] \\\n> +\t\t\t-foreground [expr {$has_focus ? $color::select_fg : $color::inactive_select_fg}]\n> +\t}\n> +}\n> +\n> foreach i [list $ui_index $ui_workdir] {\n> \trmsel_tag $i\n> -\t$i tag conf in_diff \\\n> -\t\t-background $color::select_bg \\\n> -\t\t-foreground $color::select_fg\n> +\n> +\tset_selection_colors $i 0\n> +\tbind $i \t{ set_selection_colors %W 1 }\n> +\tbind $i \t{ set_selection_colors %W 0 }\n> }\n> unset i\n> \n> diff --git a/lib/themed.tcl b/lib/themed.tcl\n> index 244c061..0f6575a 100644\n> --- a/lib/themed.tcl\n> +++ b/lib/themed.tcl\n> @@ -6,19 +6,25 @@ namespace eval color {\n> \t# Variable colors\n> \t# Preffered way to set widget colors is using add_option.\n> \t# In some cases, like with tags in_diff/in_sel, we use these colors.\n> -\tvariable select_bg\t\tlightgray\n> -\tvariable select_fg\t\tblack\n> +\tvariable select_bg\t\t\t\tlightgray\n> +\tvariable select_fg\t\t\t\tblack\n> +\tvariable inactive_select_bg\t\tlightgray\n> +\tvariable inactive_select_fg\t\tblack\n> \n> \tproc sync_with_theme {} {\n> -\t\tset base_bg\t\t[ttk::style lookup . -background]\n> -\t\tset base_fg\t\t[ttk::style lookup . -foreground]\n> -\t\tset text_bg\t\t[ttk::style lookup Treeview -background]\n> -\t\tset text_fg\t\t[ttk::style lookup Treeview -foreground]\n> -\t\tset select_bg\t[ttk::style lookup Default -selectbackground]\n> -\t\tset select_fg\t[ttk::style lookup Default -selectforeground]\n> +\t\tset base_bg\t\t [ttk::style lookup . -background]\n> +\t\tset base_fg\t\t [ttk::style lookup . -foreground]\n> +\t\tset text_bg\t\t [ttk::style lookup Treeview -background]\n> +\t\tset text_fg\t\t [ttk::style lookup Treeview -foreground]\n> +\t\tset select_bg\t [ttk::style lookup Default -selectbackground]\n> +\t\tset select_fg\t [ttk::style lookup Default -selectforeground]\n> +\t\tset inactive_select_bg\t[convert_rgb_to_gray $select_bg]\n> +\t\tset inactive_select_fg \t$select_fg\n\nThis has mixed tabs and spaces for alignment. Changed to all tabs \nlocally. Applied to git-gui/master with this change. Thanks.\n\n-- \nRegards,\nPratyush Yadav\n"}]}