Hi,
I just recently noticed that modus-themes were setting
vc-annotate-background-mode to nil, even if I configured it to be t (by
custom.el is loaded before loading modus). Is there a particular reason
for doing so? Personally, I find the default a lot more intuitive to
reason about what code is old and what is new. It is easy to fix, but
one might consider a theme changing a user option like this to be
invasive.
> From: Philip Kaludercic <philipk@posteo.net>
> Date: Sun, 05 Jun 2022 14:18:21 +0000
>
>
> Hi,
Hello Philip,
> I just recently noticed that modus-themes were setting
> vc-annotate-background-mode to nil, even if I configured it to be t (by
> custom.el is loaded before loading modus). Is there a particular reason
> for doing so? Personally, I find the default a lot more intuitive to
> reason about what code is old and what is new. It is easy to fix, but
> one might consider a theme changing a user option like this to be
> invasive.
It is invasive and I want to remove it. The reason I never touched it
is because no-one ever provided any feedback about it.
I am now thinking about removing all those variables:
modus-themes.el | 23 -----------------------
1 file changed, 23 deletions(-)
diff --git a/modus-themes.el b/modus-themes.el
index f068e4a..860f03e 100644
--- a/modus-themes.el+++ b/modus-themes.el
@@ -7504,29 +7504,6 @@ ;;;; mini-modeline
;;;; pdf-tools
`(pdf-view-midnight-colors
'(,fg-main . ,bg-dim))
-;;;; vc-annotate (C-x v g)- `(vc-annotate-background nil)- `(vc-annotate-background-mode nil)- `(vc-annotate-color-map- '((20 . ,red)- (40 . ,magenta)- (60 . ,magenta-alt)- (80 . ,red-alt)- (100 . ,yellow)- (120 . ,yellow-alt)- (140 . ,fg-special-warm)- (160 . ,fg-special-mild)- (180 . ,green)- (200 . ,green-alt)- (220 . ,cyan-alt-other)- (240 . ,cyan-alt)- (260 . ,cyan)- (280 . ,fg-special-cold)- (300 . ,blue)- (320 . ,blue-alt)- (340 . ,blue-alt-other)- (360 . ,magenta-alt-other)))- `(vc-annotate-very-old-color nil);;;; wid-edit
`(widget-link-prefix ,(if (memq 'all-buttons modus-themes-box-buttons)
" "
What do you think? They don't do the right thing and things don't work
properly anyway. I will update the documentation instead.
An aside here: I remember in previous versions of Emacs where the
vc-annotate-color-map could be controlled by the themes by setting it
this way, but this is not possible in the last few months. The return
value I get is the default:
vc-annotate-color-map
;; => ((20 . "#FF3F3F") (40 . "#FF6C3F") (60 . "#FF993F") (80
;; . "#FFC63F") (100 . "#FFF33F") (120 . "#DDFF3F") (140 . "#B0FF3F")
;; (160 . "#83FF3F") (180 . "#56FF3F") (200 . "#3FFF56") (220
;; . "#3FFF83") (240 . "#3FFFB0") (260 . "#3FFFDD") (280 . "#3FF3FF")
;; (300 . "#3FC6FF") (320 . "#3F99FF") (340 . "#3F6CFF") (360
;; . "#3F3FFF"))
--
Protesilaos Stavrou
https://protesilaos.com
Protesilaos Stavrou <info@protesilaos.com> writes:
>> From: Philip Kaludercic <philipk@posteo.net>>> Date: Sun, 05 Jun 2022 14:18:21 +0000>>>>>> Hi,>> Hello Philip,>>> I just recently noticed that modus-themes were setting>> vc-annotate-background-mode to nil, even if I configured it to be t (by>> custom.el is loaded before loading modus). Is there a particular reason>> for doing so? Personally, I find the default a lot more intuitive to>> reason about what code is old and what is new. It is easy to fix, but>> one might consider a theme changing a user option like this to be>> invasive.>> It is invasive and I want to remove it. The reason I never touched it> is because no-one ever provided any feedback about it.
I love being the first one to do so :).
> I am now thinking about removing all those variables:>> modus-themes.el | 23 -----------------------> 1 file changed, 23 deletions(-)>> diff --git a/modus-themes.el b/modus-themes.el> index f068e4a..860f03e 100644> --- a/modus-themes.el> +++ b/modus-themes.el> @@ -7504,29 +7504,6 @@ ;;;; mini-modeline> ;;;; pdf-tools> `(pdf-view-midnight-colors> '(,fg-main . ,bg-dim))> -;;;; vc-annotate (C-x v g)> - `(vc-annotate-background nil)
Why is this option set to what already was the default value?
> - `(vc-annotate-background-mode nil)> - `(vc-annotate-color-map> - '((20 . ,red)> - (40 . ,magenta)> - (60 . ,magenta-alt)> - (80 . ,red-alt)> - (100 . ,yellow)> - (120 . ,yellow-alt)> - (140 . ,fg-special-warm)> - (160 . ,fg-special-mild)> - (180 . ,green)> - (200 . ,green-alt)> - (220 . ,cyan-alt-other)> - (240 . ,cyan-alt)> - (260 . ,cyan)> - (280 . ,fg-special-cold)> - (300 . ,blue)> - (320 . ,blue-alt)> - (340 . ,blue-alt-other)> - (360 . ,magenta-alt-other)))
I think keeping `vc-annotate-color-map' would be just fine. If you
remove it the default colors are used, and while it works, it wouldn't
match the colour scheme.
> - `(vc-annotate-very-old-color nil)
It might be interesting if this could be set to some sensible value, but
as far as I see it has to be a colour spec. Can an entry in
modus-themes-custom-variables depend on the specific Modus theme
> ;;;; wid-edit> `(widget-link-prefix ,(if (memq 'all-buttons modus-themes-box-buttons)> " ">> What do you think? They don't do the right thing and things don't work> properly anyway. I will update the documentation instead.>> An aside here: I remember in previous versions of Emacs where the> vc-annotate-color-map could be controlled by the themes by setting it> this way, but this is not possible in the last few months. The return> value I get is the default:>> vc-annotate-color-map> ;; => ((20 . "#FF3F3F") (40 . "#FF6C3F") (60 . "#FF993F") (80> ;; . "#FFC63F") (100 . "#FFF33F") (120 . "#DDFF3F") (140 . "#B0FF3F")> ;; (160 . "#83FF3F") (180 . "#56FF3F") (200 . "#3FFF56") (220> ;; . "#3FFF83") (240 . "#3FFFB0") (260 . "#3FFFDD") (280 . "#3FF3FF")> ;; (300 . "#3FC6FF") (320 . "#3F99FF") (340 . "#3F6CFF") (360> ;; . "#3F3FFF"))
> From: Philip Kaludercic <philipk@posteo.net>> Date: Sun, 05 Jun 2022 15:15:04 +0000>>> It is invasive and I want to remove it. The reason I never touched it>> is because no-one ever provided any feedback about it.>> I love being the first one to do so :).
And thanks for doing it!
>> I am now thinking about removing all those variables:>>>> modus-themes.el | 23 ----------------------->> 1 file changed, 23 deletions(-)>>>> diff --git a/modus-themes.el b/modus-themes.el>> index f068e4a..860f03e 100644>> --- a/modus-themes.el>> +++ b/modus-themes.el>> @@ -7504,29 +7504,6 @@ ;;;; mini-modeline>> ;;;; pdf-tools>> `(pdf-view-midnight-colors>> '(,fg-main . ,bg-dim))>> -;;;; vc-annotate (C-x v g)>> - `(vc-annotate-background nil)>> Why is this option set to what already was the default value?
The idea was to prevent the user from setting it. As for why: because
it is not possible to know if the colour the user picks will lead to
accessible combinations.
For the record, I also prefer backgrounds for diffs and related.
>> - `(vc-annotate-background-mode nil)>> - `(vc-annotate-color-map>> - '((20 . ,red)>> - (40 . ,magenta)>> - (60 . ,magenta-alt)>> - (80 . ,red-alt)>> - (100 . ,yellow)>> - (120 . ,yellow-alt)>> - (140 . ,fg-special-warm)>> - (160 . ,fg-special-mild)>> - (180 . ,green)>> - (200 . ,green-alt)>> - (220 . ,cyan-alt-other)>> - (240 . ,cyan-alt)>> - (260 . ,cyan)>> - (280 . ,fg-special-cold)>> - (300 . ,blue)>> - (320 . ,blue-alt)>> - (340 . ,blue-alt-other)>> - (360 . ,magenta-alt-other)))>> I think keeping `vc-annotate-color-map' would be just fine. If you> remove it the default colors are used, and while it works, it wouldn't> match the colour scheme.
The problem is that it does not work properly. If I keep those values
and the user changes vc-annotate-background-mode to t, then they will
get unusable colours. That mode takes those colours and applies them to
the background. With modus-operandi, this results in intense
backgrounds with black text---virtually unreadable.
Also, I have noticed that these user options generally don't behave as
expected. For example, I am using the following to experiment
with/without backgrounds and with switching between modus-operandi and
modus-vivendi. For some reason, modus-operandi now always uses
backgrounds while modus-vivendi doesn't (vc-annotate-background-mode is
nil)... I know that I need to re-run M-x vc-annotate for changes to
take effect.
(setq vc-annotate-background-mode nil)
(defun my-modus-themes-vc-annotate ()
;; Actual values are for demo purposes
(modus-themes-with-colors
(if vc-annotate-background-mode
(setq vc-annotate-background bg-alt
vc-annotate-color-map
`((20 . ,red-intense-bg)
(40 . ,red-subtle-bg)
(60 . ,red-refine-bg)
(80 . ,yellow-intense-bg)
(100 . ,yellow-subtle-bg)
(120 . ,yellow-refine-bg)
(140 . ,magenta-intense-bg)
(160 . ,magenta-subtle-bg)
(180 . ,magenta-refine-bg)
(200 . ,cyan-intense-bg)
(220 . ,cyan-subtle-bg)
(240 . ,cyan-refine-bg)
(260 . ,green-intense-bg)
(280 . ,green-subtle-bg)
(300 . ,green-refine-bg)
(320 . ,blue-intense-bg)
(340 . ,blue-subtle-bg)
(360 . ,blue-refine-bg)))
(setq vc-annotate-background nil
vc-annotate-color-map
`((20 . ,red)
(40 . ,magenta)
(60 . ,magenta-alt)
(80 . ,red-alt)
(100 . ,yellow)
(120 . ,yellow-alt)
(140 . ,fg-special-warm)
(160 . ,fg-special-mild)
(180 . ,green)
(200 . ,green-alt)
(220 . ,cyan-alt-other)
(240 . ,cyan-alt)
(260 . ,cyan)
(280 . ,fg-special-cold)
(300 . ,blue)
(320 . ,blue-alt)
(340 . ,blue-alt-other)
(360 . ,magenta-alt-other))))))
(add-hook 'modus-themes-after-load-theme-hook #'my-modus-themes-vc-annotate)
>> - `(vc-annotate-very-old-color nil)>> It might be interesting if this could be set to some sensible value, but> as far as I see it has to be a colour spec. Can an entry in> modus-themes-custom-variables depend on the specific Modus theme
In principle, yes. Though we could apply an appropriate gray background
since the rest of the colour space is occupied.
--
Protesilaos Stavrou
https://protesilaos.com
Protesilaos Stavrou <info@protesilaos.com> writes:
>>> - `(vc-annotate-background-mode nil)>>> - `(vc-annotate-color-map>>> - '((20 . ,red)>>> - (40 . ,magenta)>>> - (60 . ,magenta-alt)>>> - (80 . ,red-alt)>>> - (100 . ,yellow)>>> - (120 . ,yellow-alt)>>> - (140 . ,fg-special-warm)>>> - (160 . ,fg-special-mild)>>> - (180 . ,green)>>> - (200 . ,green-alt)>>> - (220 . ,cyan-alt-other)>>> - (240 . ,cyan-alt)>>> - (260 . ,cyan)>>> - (280 . ,fg-special-cold)>>> - (300 . ,blue)>>> - (320 . ,blue-alt)>>> - (340 . ,blue-alt-other)>>> - (360 . ,magenta-alt-other)))>>>> I think keeping `vc-annotate-color-map' would be just fine. If you>> remove it the default colors are used, and while it works, it wouldn't>> match the colour scheme.>> The problem is that it does not work properly. If I keep those values> and the user changes vc-annotate-background-mode to t, then they will> get unusable colours. That mode takes those colours and applies them to> the background. With modus-operandi, this results in intense> backgrounds with black text---virtually unreadable.
This could be solved in the future by allowing vc-annotate-color-map to
map numbers to faces. But of course it would take a while until this
feature would trickle down to everyday users.
> Also, I have noticed that these user options generally don't behave as> expected. For example, I am using the following to experiment> with/without backgrounds and with switching between modus-operandi and> modus-vivendi. For some reason, modus-operandi now always uses> backgrounds while modus-vivendi doesn't (vc-annotate-background-mode is> nil)... I know that I need to re-run M-x vc-annotate for changes to> take effect.
Hmm, that is peculiar. In that case it might make sense to apply the
changes you propose for now.
> (setq vc-annotate-background-mode nil)>> (defun my-modus-themes-vc-annotate ()> ;; Actual values are for demo purposes> (modus-themes-with-colors> (if vc-annotate-background-mode> (setq vc-annotate-background bg-alt> vc-annotate-color-map> `((20 . ,red-intense-bg)> (40 . ,red-subtle-bg)> (60 . ,red-refine-bg)> (80 . ,yellow-intense-bg)> (100 . ,yellow-subtle-bg)> (120 . ,yellow-refine-bg)> (140 . ,magenta-intense-bg)> (160 . ,magenta-subtle-bg)> (180 . ,magenta-refine-bg)> (200 . ,cyan-intense-bg)> (220 . ,cyan-subtle-bg)> (240 . ,cyan-refine-bg)> (260 . ,green-intense-bg)> (280 . ,green-subtle-bg)> (300 . ,green-refine-bg)> (320 . ,blue-intense-bg)> (340 . ,blue-subtle-bg)> (360 . ,blue-refine-bg)))> (setq vc-annotate-background nil> vc-annotate-color-map> `((20 . ,red)> (40 . ,magenta)> (60 . ,magenta-alt)> (80 . ,red-alt)> (100 . ,yellow)> (120 . ,yellow-alt)> (140 . ,fg-special-warm)> (160 . ,fg-special-mild)> (180 . ,green)> (200 . ,green-alt)> (220 . ,cyan-alt-other)> (240 . ,cyan-alt)> (260 . ,cyan)> (280 . ,fg-special-cold)> (300 . ,blue)> (320 . ,blue-alt)> (340 . ,blue-alt-other)> (360 . ,magenta-alt-other))))))>> (add-hook 'modus-themes-after-load-theme-hook #'my-modus-themes-vc-annotate)>>>> - `(vc-annotate-very-old-color nil)>>>> It might be interesting if this could be set to some sensible value, but>> as far as I see it has to be a colour spec. Can an entry in>> modus-themes-custom-variables depend on the specific Modus theme>> In principle, yes. Though we could apply an appropriate gray background> since the rest of the colour space is occupied.
> From: Philip Kaludercic <philipk@posteo.net>> Date: Sun, 05 Jun 2022 16:21:45 +0000>> Hmm, that is peculiar. In that case it might make sense to apply the> changes you propose for now.
Done in commit 6f2b048. I elaborated in the commit message. Thank you!
By the way, this was in the modus-themes.org (now removed):
** Note on vc-annotate-background-mode
:properties:
:custom_id: h:5095cbd1-e17a-419c-93e8-951c186362a3
:end:
Due to the unique way ~vc-annotate~ ({{{kbd(C-x v g)}}}) applies colors, support
for its background mode (~vc-annotate-background-mode~) is disabled at the
theme level.
Normally, such a drastic measure should not belong in a theme: assuming
the user's preferences is bad practice. However, it has been deemed
necessary in the interest of preserving color contrast accessibility
while still supporting a useful built-in tool.
If there actually is a way to avoid such a course of action, without
prejudice to the accessibility standard of this project, then please
report as much or send patches ([[#h:9c3cd842-14b7-44d7-84b2-a5c8bc3fc3b1][Contributing]]).
--
Protesilaos Stavrou
https://protesilaos.com
Protesilaos Stavrou <info@protesilaos.com> writes:
>> From: Philip Kaludercic <philipk@posteo.net>>> Date: Sun, 05 Jun 2022 16:21:45 +0000>>>> Hmm, that is peculiar. In that case it might make sense to apply the>> changes you propose for now.>> Done in commit 6f2b048. I elaborated in the commit message. Thank you!
Great, thank you!