| Message ID | 20200824154835.160749-1-serg.partizan@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | git-gui: Basic dark mode support | expand |
>One problem that i can't yet fix: gray background for files
inchangelists. Any advice on this?
If I understand the issue and the tcl/tk docs correctly, you can change
that color using -highlightcolor on the ttext widget.
I've cc'ed in Pratyush, the current git-gui maintainer. He's probably
best suited to review this patch.
Best regards
Matthias
Hi Serg, Thanks for the patch and sorry for taking so long in getting to it. On 24/08/20 06:48PM, Serg Tereshchenko wrote: > Hi all. > > I want to use dark themes with git citool, and here is my first attempt > to do so. Like I said in the previous email this part doesn't really belong in the commit message. In fact, while this entire text is a good description of the patch and your efforts, it is not a good commit message. > I am new to tcl, so i happily accept any tips on how to improve code. > > First things first: to properly support colors, would be nice to have > them separated from app code, so i created new file lib/colored.tcl. Name > is selected to be consistent with "lib/themed.tcl". Wouldn't having the contents of colored.tcl in themed.tcl be a good idea? The way I see it, colors are part of the theming of the application. > Then, i extract hardcoded colors from git-gui.sh into namespace Color. > Then, if option use_ttk is true, i update default colors for > background/foreground from current theme. > > How it was looking before: > - Dark theme (awdark): https://i.imgur.com/0lrfHyq.png > - Light theme (clam): https://i.imgur.com/1fsfayJ.png > > Now looks like this: > - Dark theme (awdark): https://i.imgur.com/BISllEH.png > - Light theme (clam): https://i.imgur.com/WclSTa4.png This is quite an improvement :-) > One problem that i can't yet fix: gray background for files in > changelists. Any advice on this? You can set that in the function `rmsel_tag` in git-gui.sh on the line $text tag conf in_sel -background lightgray > I would be happy to move color definitions from git-gui.sh to > themed.tcl, so we can set it once, and not for each ttext call. Do you > think this is a good idea now or in the future? Do you mean to put the `-foreground` and `-background` options in the function ttext in themed.tcl? If so how can a widget specify if it wants a dark text or light for example? > I see some work is already done in that direction, like lib/themed.tcl:gold_frame. > > > Kind Regards. > > Signed-off-by: Serg Tereshchenko <serg.partizan@gmail.com> > --- > git-gui.sh | 33 +++++++++++++++++++-------------- > lib/colored.tcl | 23 +++++++++++++++++++++++ > 2 files changed, 42 insertions(+), 14 deletions(-) > create mode 100644 lib/colored.tcl > > diff --git a/git-gui.sh b/git-gui.sh > index ca66a8e..cffd106 100755 > --- a/git-gui.sh > +++ b/git-gui.sh > @@ -861,6 +861,7 @@ proc apply_config {} { > set NS ttk > bind [winfo class .] <<ThemeChanged>> [list InitTheme] > pave_toplevel . > + Color::syncColorsWithTheme > } > } > } > @@ -3273,9 +3274,13 @@ pack .vpane -anchor n -side top -fill both -expand 1 > # -- Working Directory File List > > textframe .vpane.files.workdir -height 100 -width 200 > -tlabel .vpane.files.workdir.title -text [mc "Unstaged Changes"] \ > - -background lightsalmon -foreground black > -ttext $ui_workdir -background white -foreground black \ > +tlabel .vpane.files.workdir.title \ > + -text [mc "Unstaged Changes"] \ > + -background $Color::lightRed \ > + -foreground $Color::textOnLight > +ttext $ui_workdir \ > + -background $Color::textBg \ > + -foreground $Color::textColor \ > -borderwidth 0 \ > -width 20 -height 10 \ > -wrap none \ > @@ -3296,8 +3301,8 @@ pack $ui_workdir -side left -fill both -expand 1 > textframe .vpane.files.index -height 100 -width 200 > tlabel .vpane.files.index.title \ > -text [mc "Staged Changes (Will Commit)"] \ > - -background lightgreen -foreground black > -ttext $ui_index -background white -foreground black \ > + -background $Color::lightGreen -foreground $Color::textOnLight > +ttext $ui_index -background $Color::textBg -foreground $Color::textColor \ > -borderwidth 0 \ > -width 20 -height 10 \ > -wrap none \ > @@ -3432,7 +3437,7 @@ if {![is_enabled nocommit]} { > } > > textframe .vpane.lower.commarea.buffer.frame > -ttext $ui_comm -background white -foreground black \ > +ttext $ui_comm -background $Color::textBg -foreground $Color::textColor \ > -borderwidth 1 \ > -undo true \ > -maxundo 20 \ > @@ -3519,19 +3524,19 @@ trace add variable current_diff_path write trace_current_diff_path > > gold_frame .vpane.lower.diff.header > tlabel .vpane.lower.diff.header.status \ > - -background gold \ > - -foreground black \ > + -background $Color::lightGold \ > + -foreground $Color::textOnLight \ > -width $max_status_desc \ > -anchor w \ > -justify left > tlabel .vpane.lower.diff.header.file \ > - -background gold \ > - -foreground black \ > + -background $Color::lightGold \ > + -foreground $Color::textOnLight \ > -anchor w \ > -justify left > tlabel .vpane.lower.diff.header.path \ > - -background gold \ > - -foreground blue \ > + -background $Color::lightGold \ > + -foreground $Color::lightBlue \ > -anchor w \ > -justify left \ > -font [eval font create [font configure font_ui] -underline 1] \ > @@ -3561,7 +3566,7 @@ bind .vpane.lower.diff.header.path <Button-1> {do_file_open $current_diff_path} > # > textframe .vpane.lower.diff.body > set ui_diff .vpane.lower.diff.body.t > -ttext $ui_diff -background white -foreground black \ > +ttext $ui_diff -background $Color::textBg -foreground $Color::textColor \ > -borderwidth 0 \ > -width 80 -height 5 -wrap none \ > -font font_diff \ > @@ -3589,7 +3594,7 @@ foreach {n c} {0 black 1 red4 2 green4 3 yellow4 4 blue4 5 magenta4 6 cyan4 7 gr > $ui_diff tag configure clr1 -font font_diffbold > $ui_diff tag configure clr4 -underline 1 > > -$ui_diff tag conf d_info -foreground blue -font font_diffbold > +$ui_diff tag conf d_info -foreground $Color::lightBlue -font font_diffbold > > $ui_diff tag conf d_cr -elide true > $ui_diff tag conf d_@ -font font_diffbold > diff --git a/lib/colored.tcl b/lib/colored.tcl > new file mode 100644 > index 0000000..fdb3f9c > --- /dev/null > +++ b/lib/colored.tcl > @@ -0,0 +1,23 @@ > +# Color configuration support for git-gui. > + > +namespace eval Color { FWIW I don't mind if you just put all this in the global namespace, but I'll leave it up to you. > + # static colors > + variable lightRed lightsalmon > + variable lightGreen green > + variable lightGold gold > + variable lightBlue blue > + variable textOnLight black > + variable textOnDark white Why have `textOnLight`, `textOnDark` and `textColor` separately? My guess is that it is for when you want to force light colors regardless of the theme? Am I right? > + # theme colors > + variable interfaceBg lightgray > + variable textBg white > + variable textColor black Nitpick: please use snake_case for variable names like the rest of the code does. Same for the function name below and the namespace name above. > + > + proc syncColorsWithTheme {} { > + set Color::interfaceBg [ttk::style lookup Entry -background] > + set Color::textBg [ttk::style lookup Treeview -background] > + set Color::textColor [ttk::style lookup Treeview -foreground] > + > + tk_setPalette $Color::interfaceBg > + } > +} Most of the patch looks good to me apart from my small suggestions. Thanks for working on this.
On 24/08/20 06:48PM, Serg Tereshchenko wrote: > Hi all. > > I want to use dark themes with git citool, and here is my first attempt > to do so. > > I am new to tcl, so i happily accept any tips on how to improve code. > > First things first: to properly support colors, would be nice to have > them separated from app code, so i created new file lib/colored.tcl. Name > is selected to be consistent with "lib/themed.tcl". > > Then, i extract hardcoded colors from git-gui.sh into namespace Color. > Then, if option use_ttk is true, i update default colors for > background/foreground from current theme. > > How it was looking before: > - Dark theme (awdark): https://i.imgur.com/0lrfHyq.png > - Light theme (clam): https://i.imgur.com/1fsfayJ.png > > Now looks like this: > - Dark theme (awdark): https://i.imgur.com/BISllEH.png > - Light theme (clam): https://i.imgur.com/WclSTa4.png How do you tell git-gui which theme to use? I had some trouble setting the theme and ended up adding code to source the theme files and then set the theme via `ttk::style theme use`. I hope there is a better way than that.
> How do you tell git-gui which theme to use? I had some trouble setting > the theme and ended up adding code to source the theme files and then > set the theme via `ttk::style theme use`. I hope there is a better way > than that. Yes, there is. To change theme on the fly use: echo '*TkTheme: clam' | xrdb -merge - To set theme, add "*TkTheme: clam" to ~/.Xresources and run xrdb -merge ~/.Xresources There is lack of dark themes in default tk installs right now, i'm using awdark: https://sourceforge.net/projects/tcl-awthemes/ To install theme you need to unpack it somewhere like ~/.local/share/tk-themes/awthemes And tell tcl where to find it. export TCLLIBPATH=$HOME/.local/share/tk-themes I had to modify version numbers inside awthemes package to make in work, but hope it'll be fixed upstream. Here is blog post which explains this in greater detail: http://blog.serindu.com/2019/03/07/applying-tk-themes-to-git-gui/ -- Regards, Serg Tereshchenko
On 08/10/20 11:20AM, Serg Tereshchenko wrote: > > How do you tell git-gui which theme to use? I had some trouble setting > > the theme and ended up adding code to source the theme files and then > > set the theme via `ttk::style theme use`. I hope there is a better way > > than that. > > Yes, there is. To change theme on the fly use: > > echo '*TkTheme: clam' | xrdb -merge - > > To set theme, add "*TkTheme: clam" to ~/.Xresources and run > > xrdb -merge ~/.Xresources > > There is lack of dark themes in default tk installs right now, > i'm using awdark: https://sourceforge.net/projects/tcl-awthemes/ > > To install theme you need to unpack it somewhere like ~/.local/share/tk-themes/awthemes > And tell tcl where to find it. > > export TCLLIBPATH=$HOME/.local/share/tk-themes > > I had to modify version numbers inside awthemes package to make in work, > but hope it'll be fixed upstream. > > Here is blog post which explains this in greater detail: > http://blog.serindu.com/2019/03/07/applying-tk-themes-to-git-gui/ Thanks. This is a bit complicated to be honest. I don't think we can do much about the "installing Tk themes" part, but we can certainly make it easier to select an installed theme in git-gui. A config option like gui.tktheme would be good. Something to consider in the future...
> Thanks. This is a bit complicated to be honest. I don't think we can do > much about the "installing Tk themes" part, but we can certainly make it > easier to select an installed theme in git-gui. A config option like > gui.tktheme would be good. Something to consider in the future... I think application should not be responsible for setting theme. Of course, it is simpler for user to set git-gui theme in app config, but right way to do it - is to set theme on system level. On mac it is already using aqua (with dark colors if set), and user even don't know about it. On windows it uses some windows-like theme by default. On linux, yes, we must change ~/.Xresources, but this is just how we set themes for tk apps. It's systemwide, and all tk apps will resect this choice. -- Regards, Serg Tereshchenko
diff --git a/git-gui.sh b/git-gui.sh index ca66a8e..cffd106 100755 --- a/git-gui.sh +++ b/git-gui.sh @@ -861,6 +861,7 @@ proc apply_config {} { set NS ttk bind [winfo class .] <<ThemeChanged>> [list InitTheme] pave_toplevel . + Color::syncColorsWithTheme } } } @@ -3273,9 +3274,13 @@ pack .vpane -anchor n -side top -fill both -expand 1 # -- Working Directory File List textframe .vpane.files.workdir -height 100 -width 200 -tlabel .vpane.files.workdir.title -text [mc "Unstaged Changes"] \ - -background lightsalmon -foreground black -ttext $ui_workdir -background white -foreground black \ +tlabel .vpane.files.workdir.title \ + -text [mc "Unstaged Changes"] \ + -background $Color::lightRed \ + -foreground $Color::textOnLight +ttext $ui_workdir \ + -background $Color::textBg \ + -foreground $Color::textColor \ -borderwidth 0 \ -width 20 -height 10 \ -wrap none \ @@ -3296,8 +3301,8 @@ pack $ui_workdir -side left -fill both -expand 1 textframe .vpane.files.index -height 100 -width 200 tlabel .vpane.files.index.title \ -text [mc "Staged Changes (Will Commit)"] \ - -background lightgreen -foreground black -ttext $ui_index -background white -foreground black \ + -background $Color::lightGreen -foreground $Color::textOnLight +ttext $ui_index -background $Color::textBg -foreground $Color::textColor \ -borderwidth 0 \ -width 20 -height 10 \ -wrap none \ @@ -3432,7 +3437,7 @@ if {![is_enabled nocommit]} { } textframe .vpane.lower.commarea.buffer.frame -ttext $ui_comm -background white -foreground black \ +ttext $ui_comm -background $Color::textBg -foreground $Color::textColor \ -borderwidth 1 \ -undo true \ -maxundo 20 \ @@ -3519,19 +3524,19 @@ trace add variable current_diff_path write trace_current_diff_path gold_frame .vpane.lower.diff.header tlabel .vpane.lower.diff.header.status \ - -background gold \ - -foreground black \ + -background $Color::lightGold \ + -foreground $Color::textOnLight \ -width $max_status_desc \ -anchor w \ -justify left tlabel .vpane.lower.diff.header.file \ - -background gold \ - -foreground black \ + -background $Color::lightGold \ + -foreground $Color::textOnLight \ -anchor w \ -justify left tlabel .vpane.lower.diff.header.path \ - -background gold \ - -foreground blue \ + -background $Color::lightGold \ + -foreground $Color::lightBlue \ -anchor w \ -justify left \ -font [eval font create [font configure font_ui] -underline 1] \ @@ -3561,7 +3566,7 @@ bind .vpane.lower.diff.header.path <Button-1> {do_file_open $current_diff_path} # textframe .vpane.lower.diff.body set ui_diff .vpane.lower.diff.body.t -ttext $ui_diff -background white -foreground black \ +ttext $ui_diff -background $Color::textBg -foreground $Color::textColor \ -borderwidth 0 \ -width 80 -height 5 -wrap none \ -font font_diff \ @@ -3589,7 +3594,7 @@ foreach {n c} {0 black 1 red4 2 green4 3 yellow4 4 blue4 5 magenta4 6 cyan4 7 gr $ui_diff tag configure clr1 -font font_diffbold $ui_diff tag configure clr4 -underline 1 -$ui_diff tag conf d_info -foreground blue -font font_diffbold +$ui_diff tag conf d_info -foreground $Color::lightBlue -font font_diffbold $ui_diff tag conf d_cr -elide true $ui_diff tag conf d_@ -font font_diffbold diff --git a/lib/colored.tcl b/lib/colored.tcl new file mode 100644 index 0000000..fdb3f9c --- /dev/null +++ b/lib/colored.tcl @@ -0,0 +1,23 @@ +# Color configuration support for git-gui. + +namespace eval Color { + # static colors + variable lightRed lightsalmon + variable lightGreen green + variable lightGold gold + variable lightBlue blue + variable textOnLight black + variable textOnDark white + # theme colors + variable interfaceBg lightgray + variable textBg white + variable textColor black + + proc syncColorsWithTheme {} { + set Color::interfaceBg [ttk::style lookup Entry -background] + set Color::textBg [ttk::style lookup Treeview -background] + set Color::textColor [ttk::style lookup Treeview -foreground] + + tk_setPalette $Color::interfaceBg + } +}
Hi all. I want to use dark themes with git citool, and here is my first attempt to do so. I am new to tcl, so i happily accept any tips on how to improve code. First things first: to properly support colors, would be nice to have them separated from app code, so i created new file lib/colored.tcl. Name is selected to be consistent with "lib/themed.tcl". Then, i extract hardcoded colors from git-gui.sh into namespace Color. Then, if option use_ttk is true, i update default colors for background/foreground from current theme. How it was looking before: - Dark theme (awdark): https://i.imgur.com/0lrfHyq.png - Light theme (clam): https://i.imgur.com/1fsfayJ.png Now looks like this: - Dark theme (awdark): https://i.imgur.com/BISllEH.png - Light theme (clam): https://i.imgur.com/WclSTa4.png One problem that i can't yet fix: gray background for files in changelists. Any advice on this? I would be happy to move color definitions from git-gui.sh to themed.tcl, so we can set it once, and not for each ttext call. Do you think this is a good idea now or in the future? I see some work is already done in that direction, like lib/themed.tcl:gold_frame. Kind Regards. Signed-off-by: Serg Tereshchenko <serg.partizan@gmail.com> --- git-gui.sh | 33 +++++++++++++++++++-------------- lib/colored.tcl | 23 +++++++++++++++++++++++ 2 files changed, 42 insertions(+), 14 deletions(-) create mode 100644 lib/colored.tcl