Hi, I have just downloaded and installed the version-4 branch, and
notice that (among other things) `modus-themes-mode-line' has been
deprecated. My understanding is that you want these changes to be made
via overrides (border-mode-line-active), right?
Unless I am missing something, this has do be done for every theme,
setting the ...-palette-overrides variable (which is not a user option),
individually? Would adding a common override variable make sense, to
make it easier if I e.g. always want to have a boxed border around the
mode line?
> From: Philip Kaludercic <philipk@posteo.net>> Date: Sat, 17 Dec 2022 15:01:50 +0000
Hello Philip,
> Hi, I have just downloaded and installed the version-4 branch, and> notice that (among other things) `modus-themes-mode-line' has been> deprecated. My understanding is that you want these changes to be made> via overrides (border-mode-line-active), right?
Correct!
> Unless I am missing something, this has do be done for every theme,> setting the ...-palette-overrides variable (which is not a user option),> individually? Would adding a common override variable make sense, to> make it easier if I e.g. always want to have a boxed border around the> mode line?
Yes, this makes perfect sense.
> diff --git a/modus-themes.el b/modus-themes.el> index e99085a..77cb63f 100644> --- a/modus-themes.el> +++ b/modus-themes.el> @@ -3652,6 +3652,9 @@ is a less intense variant of BG."> > ;;;; Instantiate a Modus theme> > +(defvar modus-operandi-common-overrides nil> + "Overrides for all modus themes.")> +> ;;;###autoload> (defmacro modus-themes-theme (name palette &optional overrides)> "Bind NAME's color PALETTE around face specs and variables.> @@ -3666,7 +3669,7 @@ corresponding entries."> (let ((sym (gensym))> (colors (mapcar #'car (symbol-value palette))))> `(let* ((c '((class color) (min-colors 256)))> - (,sym (append ,overrides ,palette))> + (,sym (append ,overrides modus-operandi-common-overrides ,palette))> ,@(mapcar (lambda (color)> (list color> `(let* ((value (car (alist-get ',color ,sym))))
Just replace "operandi" with themes and we are good to go. Maybe this
should be a 'defcustom'? For the others I thought about keeping them as
'defvar' so that they do not show up in the Custom UI. The users will
discover them by reading the manual or relevant material of mine.
> Another semi-related question, do you plan to add all the new variations> of the Modus Themes to emacs.git, or will the core themes only be the> current two?
My plan is to ask on emacs-devel and do whatever the maintainers want.
I do not know how things work in this case.
All the best,
Prot
--
Protesilaos Stavrou
https://protesilaos.com
Protesilaos Stavrou <info@protesilaos.com> writes:
>> From: Philip Kaludercic <philipk@posteo.net>>> Date: Sat, 17 Dec 2022 15:01:50 +0000>> Hello Philip,>>> Hi, I have just downloaded and installed the version-4 branch, and>> notice that (among other things) `modus-themes-mode-line' has been>> deprecated. My understanding is that you want these changes to be made>> via overrides (border-mode-line-active), right?>> Correct!>>> Unless I am missing something, this has do be done for every theme,>> setting the ...-palette-overrides variable (which is not a user option),>> individually? Would adding a common override variable make sense, to>> make it easier if I e.g. always want to have a boxed border around the>> mode line?>> Yes, this makes perfect sense.>>> diff --git a/modus-themes.el b/modus-themes.el>> index e99085a..77cb63f 100644>> --- a/modus-themes.el>> +++ b/modus-themes.el>> @@ -3652,6 +3652,9 @@ is a less intense variant of BG.">> >> ;;;; Instantiate a Modus theme>> >> +(defvar modus-operandi-common-overrides nil>> + "Overrides for all modus themes.")>> +>> ;;;###autoload>> (defmacro modus-themes-theme (name palette &optional overrides)>> "Bind NAME's color PALETTE around face specs and variables.>> @@ -3666,7 +3669,7 @@ corresponding entries.">> (let ((sym (gensym))>> (colors (mapcar #'car (symbol-value palette))))>> `(let* ((c '((class color) (min-colors 256)))>> - (,sym (append ,overrides ,palette))>> + (,sym (append ,overrides modus-operandi-common-overrides ,palette))>> ,@(mapcar (lambda (color)>> (list color>> `(let* ((value (car (alist-get ',color ,sym))))>> Just replace "operandi" with themes and we are good to go.
I have uninstalled 'version-4' and lost the changes. If you insist, I
can re-create the diff as a patch.
> Maybe this> should be a 'defcustom'? For the others I thought about keeping them as> 'defvar' so that they do not show up in the Custom UI. The users will> discover them by reading the manual or relevant material of mine.
I think it would make sense to make all of these user option, if only
because you can annotate them with types, and macros like setopt will
raise errors if you want to assign a value of the wrong type.
>> Another semi-related question, do you plan to add all the new variations>> of the Modus Themes to emacs.git, or will the core themes only be the>> current two?>> My plan is to ask on emacs-devel and do whatever the maintainers want.> I do not know how things work in this case.
Ok, great I think that is the best way forward.
> All the best,> Prot
> From: Philip Kaludercic <philipk@posteo.net>> Date: Sat, 17 Dec 2022 16:28:45 +0000> [... 47 lines elided]>> Just replace "operandi" with themes and we are good to go. >> I have uninstalled 'version-4' and lost the changes. If you insist, I> can re-create the diff as a patch.
Okay, don't worry. I will make the change on your behalf and use your
name+email here as the author.
>> Maybe this>> should be a 'defcustom'? For the others I thought about keeping them as>> 'defvar' so that they do not show up in the Custom UI. The users will>> discover them by reading the manual or relevant material of mine.>> I think it would make sense to make all of these user option, if only> because you can annotate them with types, and macros like setopt will> raise errors if you want to assign a value of the wrong type.
Good point! The type is tricky as we have either strings or symbols.
The latter must be part of the palette and cannot be recursive mappings.
--
Protesilaos Stavrou
https://protesilaos.com
Protesilaos Stavrou <info@protesilaos.com> writes:
>> From: Philip Kaludercic <philipk@posteo.net>>> Date: Sat, 17 Dec 2022 16:28:45 +0000>>> [... 47 lines elided]>>>> Just replace "operandi" with themes and we are good to go. >>>> I have uninstalled 'version-4' and lost the changes. If you insist, I>> can re-create the diff as a patch.>> Okay, don't worry. I will make the change on your behalf and use your> name+email here as the author.
I don't care either way, you decide if you care about attribution.
>>> Maybe this>>> should be a 'defcustom'? For the others I thought about keeping them as>>> 'defvar' so that they do not show up in the Custom UI. The users will>>> discover them by reading the manual or relevant material of mine.>>>> I think it would make sense to make all of these user option, if only>> because you can annotate them with types, and macros like setopt will>> raise errors if you want to assign a value of the wrong type.>> Good point! The type is tricky as we have either strings or symbols.> The latter must be part of the palette and cannot be recursive mappings.
Won't a choice do to describe one or the other?
> From: Philip Kaludercic <philipk@posteo.net>> Date: Sat, 17 Dec 2022 16:54:22 +0000> [... 13 lines elided]>> Okay, don't worry. I will make the change on your behalf and use your>> name+email here as the author.>> I don't care either way, you decide if you care about attribution.
I think it is better as a matter of principle just to raise awareness in
teh community: I mention it in the release notes and this way show that
all contributions matter.
> [... 9 lines elided]>> Good point! The type is tricky as we have either strings or symbols.>> The latter must be part of the palette and cannot be recursive mappings.>> Won't a choice do to describe one or the other?
Yes, though this way we do not catch mistakes such as a mapping to a
non-existent colour. For example:
(cursor reddd)
This is not a blocking issue. I just thought I would mention it.
--
Protesilaos Stavrou
https://protesilaos.com
Protesilaos Stavrou <info@protesilaos.com> writes:
>> From: Philip Kaludercic <philipk@posteo.net>>> Date: Sat, 17 Dec 2022 16:54:22 +0000>>> [... 13 lines elided]>>>> Okay, don't worry. I will make the change on your behalf and use your>>> name+email here as the author.>>>> I don't care either way, you decide if you care about attribution.>> I think it is better as a matter of principle just to raise awareness in> teh community: I mention it in the release notes and this way show that> all contributions matter.
True, that is laudable!
>> [... 9 lines elided]>>>> Good point! The type is tricky as we have either strings or symbols.>>> The latter must be part of the palette and cannot be recursive mappings.>>>> Won't a choice do to describe one or the other?>> Yes, though this way we do not catch mistakes such as a mapping to a> non-existent colour. For example:>> (cursor reddd)>> This is not a blocking issue. I just thought I would mention it.
Check the definition of color in wid-edit.el, you can define a custom
widget to validate input with a computation.
Protesilaos Stavrou <info@protesilaos.com> writes:
>> Another semi-related question, do you plan to add all the new variations>> of the Modus Themes to emacs.git, or will the core themes only be the>> current two?>> My plan is to ask on emacs-devel and do whatever the maintainers want.> I do not know how things work in this case.
While on the topic, I noticed that this breaks the flow of
`toggle-theme', since toggling now involves selecting a theme from the
family. Is this intentional? Should this be resolved upstream? Or
should the various themes be part of separate families?
> From: Philip Kaludercic <philipk@posteo.net>> Date: Sat, 17 Dec 2022 17:14:57 +0000> [... 26 lines elided]>> Yes, though this way we do not catch mistakes such as a mapping to a>> non-existent colour. For example:>>>> (cursor reddd)>>>> This is not a blocking issue. I just thought I would mention it.>> Check the definition of color in wid-edit.el, you can define a custom> widget to validate input with a computation.
Okay, I am taking note of this and will do it as soon as possible.
--
Protesilaos Stavrou
https://protesilaos.com
> From: Philip Kaludercic <philipk@posteo.net>> Date: Sat, 17 Dec 2022 17:17:18 +0000>> Protesilaos Stavrou <info@protesilaos.com> writes:>>>> Another semi-related question, do you plan to add all the new variations>>> of the Modus Themes to emacs.git, or will the core themes only be the>>> current two?>>>> My plan is to ask on emacs-devel and do whatever the maintainers want.>> I do not know how things work in this case.>> While on the topic, I noticed that this breaks the flow of> `toggle-theme', since toggling now involves selecting a theme from the> family. Is this intentional? Should this be resolved upstream? Or> should the various themes be part of separate families?
No, it is not intentional. They all belong to the same superfamily but
they clearly are pairs. Could the implementation upstream be revised to
have a :family and a :pair? Otherwise I will just make them different
families like 'modus', 'modus-tinted', 'modus-deuteranopia'.
--
Protesilaos Stavrou
https://protesilaos.com
Protesilaos Stavrou <info@protesilaos.com> writes:
>> From: Philip Kaludercic <philipk@posteo.net>>> Date: Sat, 17 Dec 2022 17:17:18 +0000>>>> Protesilaos Stavrou <info@protesilaos.com> writes:>>>>>> Another semi-related question, do you plan to add all the new variations>>>> of the Modus Themes to emacs.git, or will the core themes only be the>>>> current two?>>>>>> My plan is to ask on emacs-devel and do whatever the maintainers want.>>> I do not know how things work in this case.>>>> While on the topic, I noticed that this breaks the flow of>> `toggle-theme', since toggling now involves selecting a theme from the>> family. Is this intentional? Should this be resolved upstream? Or>> should the various themes be part of separate families?>> No, it is not intentional. They all belong to the same superfamily but> they clearly are pairs. Could the implementation upstream be revised to> have a :family and a :pair? Otherwise I will just make them different> families like 'modus', 'modus-tinted', 'modus-deuteranopia'.
It is possible, but I don't think Eli will allow for such a change to be
applied to Emacs 29, unless one can show that this is a bug. I can
write up a patch and send it to the bug tracker, and we'll see how it is
received.
> From: Philip Kaludercic <philipk@posteo.net>> Date: Sun, 18 Dec 2022 10:47:59 +0000>> [... 19 lines elided]>>> No, it is not intentional. They all belong to the same superfamily but>> they clearly are pairs. Could the implementation upstream be revised to>> have a :family and a :pair? Otherwise I will just make them different>> families like 'modus', 'modus-tinted', 'modus-deuteranopia'.>> It is possible, but I don't think Eli will allow for such a change to be> applied to Emacs 29, unless one can show that this is a bug. I can> write up a patch and send it to the bug tracker, and we'll see how it is> received.
Doing it just for Emacs 30 should be fine. I have not looked at how
this works, but I suppose something like this would work?
;;;###theme-autoload
(if (>= emacs-major-version 30)
;; :family and :pair
;; :family
)
--
Protesilaos Stavrou
https://protesilaos.com
Protesilaos Stavrou <info@protesilaos.com> writes:
>> From: Philip Kaludercic <philipk@posteo.net>>> Date: Sun, 18 Dec 2022 10:47:59 +0000>>>> [... 19 lines elided]>>>>> No, it is not intentional. They all belong to the same superfamily but>>> they clearly are pairs. Could the implementation upstream be revised to>>> have a :family and a :pair? Otherwise I will just make them different>>> families like 'modus', 'modus-tinted', 'modus-deuteranopia'.>>>> It is possible, but I don't think Eli will allow for such a change to be>> applied to Emacs 29, unless one can show that this is a bug. I can>> write up a patch and send it to the bug tracker, and we'll see how it is>> received.>> Doing it just for Emacs 30 should be fine. I have not looked at how> this works, but I suppose something like this would work?>> ;;;###theme-autoload> (if (>= emacs-major-version 30)> ;; :family and :pair> ;; :family> )
Yes, but the case distinction would be unnecessary, since :pair would
just be ignored by Emacs 29.
> From: Philip Kaludercic <philipk@posteo.net>> Date: Sun, 18 Dec 2022 14:54:31 +0000> [... 27 lines elided]> Yes, but the case distinction would be unnecessary, since :pair would> just be ignored by Emacs 29.
Okay, even better!
--
Protesilaos Stavrou
https://protesilaos.com