bug#37802: 27.0.50; Feature request: hook for theme enabling/disabling

bug#37802: 27.0.50; Feature request: hook for theme enabling/disabling

[Óscar Fuentes]

I'm experimenting with some code that uses face-remap-add-relative to
modify colors on specific windows. It takes the attribute of an existing
face, alters it and uses :filtered to apply the change conditionally. On
my specific case I use that technique for darkening the default
background of the windows that have no keyboard focus.

However, when I switch themes I have to manually force the recalculation
of the "darked" background to adapt it to the new theme. If I had a hook
that fires *after* a theme is enabled or disabled, I could automatize
the recalculation.

I looked at custom.el and the change seems simple enough so I wonder if
there is any reason for not implementing such hook.


In GNU Emacs 27.0.50 (build 1, x86_64-pc-linux-gnu, X toolkit)
 of 2019-09-12 built on sky
Repository revision: 421084d2cb160261b259bddb687bb2c234f8f1ef
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12004000
System Description: Debian GNU/Linux bullseye/sid

bug#37802: 27.0.50; Feature request: hook for theme enabling/disabling

[Lars Ingebrigtsen]

Óscar Fuentes <ofv@wanadoo.es> writes:

> I'm experimenting with some code that uses face-remap-add-relative to
> modify colors on specific windows. It takes the attribute of an existing
> face, alters it and uses :filtered to apply the change conditionally. On
> my specific case I use that technique for darkening the default
> background of the windows that have no keyboard focus.
>
> However, when I switch themes I have to manually force the recalculation
> of the "darked" background to adapt it to the new theme. If I had a hook
> that fires *after* a theme is enabled or disabled, I could automatize
> the recalculation.
>
> I looked at custom.el and the change seems simple enough so I wonder if
> there is any reason for not implementing such hook.

(I'm going through old bug reports that unfortunately weren't resolved
at the time.)

If this is for a package, I think it might make more sense to use
`advice-add' on `enable-theme'?  But I wouldn't mind adding a an
`enable-theme-hook' here, if that makes things substantially easier for
your use case.


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#37802: 27.0.50; Feature request: hook for theme enabling/disabling

[Óscar Fuentes]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Óscar Fuentes <ofv@wanadoo.es> writes:
>
>> I'm experimenting with some code that uses face-remap-add-relative to
>> modify colors on specific windows. It takes the attribute of an existing
>> face, alters it and uses :filtered to apply the change conditionally. On
>> my specific case I use that technique for darkening the default
>> background of the windows that have no keyboard focus.
>>
>> However, when I switch themes I have to manually force the recalculation
>> of the "darked" background to adapt it to the new theme. If I had a hook
>> that fires *after* a theme is enabled or disabled, I could automatize
>> the recalculation.
>>
>> I looked at custom.el and the change seems simple enough so I wonder if
>> there is any reason for not implementing such hook.
>
> (I'm going through old bug reports that unfortunately weren't resolved
> at the time.)
>
> If this is for a package, I think it might make more sense to use
> `advice-add' on `enable-theme'?  But I wouldn't mind adding a an
> `enable-theme-hook' here, if that makes things substantially easier for
> your use case.

Currently I've solved my requirement by using defadvice on enable-theme
*and* disable-theme for adapting my custom faces to changes on the
active theme(s). Using a hook would be a little bit cleaner. I'm also
surprised that no hook was implemented for this case, as most changes on
user-visible state has an associated hook.

Whatever you decide, it is fine with me.

bug#37802: 27.0.50; Feature request: hook for theme enabling/disabling

[Lars Ingebrigtsen]

Óscar Fuentes <ofv@wanadoo.es> writes:

> Currently I've solved my requirement by using defadvice on enable-theme
> *and* disable-theme for adapting my custom faces to changes on the
> active theme(s). Using a hook would be a little bit cleaner. I'm also
> surprised that no hook was implemented for this case, as most changes on
> user-visible state has an associated hook.

Yes, true.  So I've now added these two hooks to Emacs 29.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#37802: 27.0.50; Feature request: hook for theme enabling/disabling

[Eli Zaretskii]

> Resent-From: Lars Ingebrigtsen <larsi@gnus.org>
> Original-Sender: "Debbugs-submit" <debbugs-submit-bounces@debbugs.gnu.org>
> Resent-CC: bug-gnu-emacs@gnu.org
> Resent-Sender: help-debbugs@gnu.org
> Cc: 37802@debbugs.gnu.org
> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Sat, 21 May 2022 14:56:34 +0200
> 
> Óscar Fuentes <ofv@wanadoo.es> writes:
> 
> > Currently I've solved my requirement by using defadvice on enable-theme
> > *and* disable-theme for adapting my custom faces to changes on the
> > active theme(s). Using a hook would be a little bit cleaner. I'm also
> > surprised that no hook was implemented for this case, as most changes on
> > user-visible state has an associated hook.
> 
> Yes, true.  So I've now added these two hooks to Emacs 29.

Thanks, but is there any reason these hooks aren't called
enable-theme-function and disable-theme-function?  Hooks by such
names are already documented as "abnormal hooks", which would avoid
the need of saying these new hooks are "atypical", something that we
never had in our terminology.

bug#37802: 27.0.50; Feature request: hook for theme enabling/disabling

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> Thanks, but is there any reason these hooks aren't called
> enable-theme-function and disable-theme-function?  Hooks by such
> names are already documented as "abnormal hooks", which would avoid
> the need of saying these new hooks are "atypical", something that we
> never had in our terminology.

"Abnormal" was the word I was looking for.

As for the name, I don't really have much of an opinion -- we have more
of these abnormal hooks called -hook than -function(s), a short grepping
for run-hook-with-args shows.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#37802: 27.0.50; Feature request: hook for theme enabling/disabling

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: ofv@wanadoo.es,  37802@debbugs.gnu.org
> Date: Sat, 21 May 2022 15:21:29 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Thanks, but is there any reason these hooks aren't called
> > enable-theme-function and disable-theme-function?  Hooks by such
> > names are already documented as "abnormal hooks", which would avoid
> > the need of saying these new hooks are "atypical", something that we
> > never had in our terminology.
> 
> "Abnormal" was the word I was looking for.
> 
> As for the name, I don't really have much of an opinion -- we have more
> of these abnormal hooks called -hook than -function(s), a short grepping
> for run-hook-with-args shows.

Maybe so, and we cannot do much about bad names that are already in
the wild.  But why not use the documented names for new hooks we
introduce now?

I can rename them myself, I just wanted to make sure you didn't have
any particular reason for using those names.

bug#37802: 27.0.50; Feature request: hook for theme enabling/disabling

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> I can rename them myself, I just wanted to make sure you didn't have
> any particular reason for using those names.

Nope.  Please go ahead.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#37802: 27.0.50; Feature request: hook for theme enabling/disabling

[Óscar Fuentes]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Yes, true.  So I've now added these two hooks to Emacs 29.

Thank you.

As it seems that the discussion about the hook's name is resolved, I'm
closing the bug report.