Question about customising faces

Question about customising faces

[Joost Kremers]

Hi list,

I wanted to customise the faces used by isearch / lazy-highlight, and I ended up
having to do the following in order to get it to work for both the initial frame
and any additional frames created during the Emacs session (with
`make-frame-command`):

```
  ;; We need to set these faces twice: once for the initial frame...
  (set-face-attribute 'isearch         nil :background nil             :foreground "#d33682")
  (set-face-attribute 'isearch-group-1 nil :background "lightskyblue1" :foreground "#d33682")
  (set-face-attribute 'isearch-group-2 nil :background "DarkSeaGreen1" :foreground "#d33682")
  (set-face-attribute 'lazy-highlight  nil :background nil             :foreground nil :box "#d33682")
  ;; ...and once for all future frames:
  (set-face-attribute 'isearch         t :background 'unspecified    :foreground "#d33682")
  (set-face-attribute 'isearch-group-1 t :background "lightskyblue1" :foreground "#d33682")
  (set-face-attribute 'isearch-group-2 t :background "DarkSeaGreen1" :foreground "#d33682")
  (set-face-attribute 'lazy-highlight  t :background 'unspecified    :foreground 'unspecified :box "#d33682")
```

So basically, I need two calls to `set-face-attribute` for each face, I haven't
found a way to reduce these to one call per face.

Am I overlooking something here? Is there a way to do this better?

I should perhaps point out that I use a custom theme (solarized-theme) and that
this invocation is placed inside the `use-package` call that loads this theme
and activates it:

```
(use-package solarized-theme
  :ensure t
  :config
  (setq solarized-scale-org-headlines nil
        solarized-use-variable-pitch nil)
  (load-theme 'solarized-light t)
  ;; We need to set these faces twice: once for the current frame...
  (set-face-attribute 'isearch         nil :background nil             :foreground "#d33682")
  (set-face-attribute 'isearch-group-1 nil :background "lightskyblue1" :foreground "#d33682")
  (set-face-attribute 'isearch-group-2 nil :background "DarkSeaGreen1" :foreground "#d33682")
  (set-face-attribute 'lazy-highlight  nil :background nil             :foreground nil :box "#d33682")
  ;; And once for all future frames.
  (set-face-attribute 'isearch         t :background 'unspecified    :foreground "#d33682")
  (set-face-attribute 'isearch-group-1 t :background "lightskyblue1" :foreground "#d33682")
  (set-face-attribute 'isearch-group-2 t :background "DarkSeaGreen1" :foreground "#d33682")
  (set-face-attribute 'lazy-highlight  t :background 'unspecified    :foreground 'unspecified :box "#d33682"))
```

TIA

Joost


-- 
Joost Kremers
Life has its moments

Re: Question about customising faces

[Eli Zaretskii]

> From: Joost Kremers <joostkremers@fastmail.fm>
> Date: Mon, 29 Aug 2022 13:30:21 +0200
> 
> 
> I wanted to customise the faces used by isearch / lazy-highlight, and I ended up
> having to do the following in order to get it to work for both the initial frame
> and any additional frames created during the Emacs session (with
> `make-frame-command`):
> 
> ```
>   ;; We need to set these faces twice: once for the initial frame...
>   (set-face-attribute 'isearch         nil :background nil             :foreground "#d33682")
>   (set-face-attribute 'isearch-group-1 nil :background "lightskyblue1" :foreground "#d33682")
>   (set-face-attribute 'isearch-group-2 nil :background "DarkSeaGreen1" :foreground "#d33682")
>   (set-face-attribute 'lazy-highlight  nil :background nil             :foreground nil :box "#d33682")
>   ;; ...and once for all future frames:
>   (set-face-attribute 'isearch         t :background 'unspecified    :foreground "#d33682")
>   (set-face-attribute 'isearch-group-1 t :background "lightskyblue1" :foreground "#d33682")
>   (set-face-attribute 'isearch-group-2 t :background "DarkSeaGreen1" :foreground "#d33682")
>   (set-face-attribute 'lazy-highlight  t :background 'unspecified    :foreground 'unspecified :box "#d33682")
> ```
> 
> So basically, I need two calls to `set-face-attribute` for each face, I haven't
> found a way to reduce these to one call per face.

The call with FRAME nil should have been enough.  And in my testing,
it is: it sets the attributes for both the first and the subsequent
frames.

> I should perhaps point out that I use a custom theme (solarized-theme) and that
> this invocation is placed inside the `use-package` call that loads this theme
> and activates it:

I don't know what that means in terms of effect on Emacs internals.
Does the single call with FRAME nil work if you do it outside the
use-package thing?  If so, perhaps use-package is the reason it
doesn't work for you.

Re: Question about customising faces

[Joost Kremers]

On Mon, Aug 29 2022, Eli Zaretskii wrote:
> The call with FRAME nil should have been enough.  And in my testing,
> it is: it sets the attributes for both the first and the subsequent
> frames.

I just tried with emacs -Q, executing only those `set-face-attribute` calls and
the problem persists. I need to execute both sets in order to get it to work. 

Note that the distinction isn't really initial frame vs. subsequent frames.
Rather, it's frames created before executing the calls to `set-face-attributes`
and after executing those calls.

I'm assuming that this is the case mentioned in the doc string of
set-face-attribute:

"[...] to reset the value
of some attribute to ‘unspecified’ in a way that overrides the
non-‘unspecified’ value defined by the face’s spec in ‘defface’,
for new frames, you must explicitly call this function with FRAME
set to t and the attribute’s value set to ‘unspecified’; just
using FRAME of nil will not affect new frames in this case.to reset the value
of some attribute to ‘unspecified’ in a way that overrides the
non-‘unspecified’ value defined by the face’s spec in ‘defface’,
for new frames, you must explicitly call this function with FRAME
set to t and the attribute’s value set to ‘unspecified’; just
using FRAME of nil will not affect new frames in this case."


-- 
Joost Kremers
Life has its moments

Re: Question about customising faces

[Stefan Monnier via Users list for the GNU Emacs text editor]

> I just tried with emacs -Q, executing only those `set-face-attribute`
> calls and the problem persists. I need to execute both sets in order
> to get it to work. 

Is there a particular reason why you use `set-face-attribute` rather
than a declarative solution like M-x customize-face?


        Stefan

Re: Question about customising faces

[Eli Zaretskii]

> From: Joost Kremers <joostkremers@fastmail.fm>
> Cc: help-gnu-emacs@gnu.org
> Date: Mon, 29 Aug 2022 22:07:41 +0200
> 
> On Mon, Aug 29 2022, Eli Zaretskii wrote:
> > The call with FRAME nil should have been enough.  And in my testing,
> > it is: it sets the attributes for both the first and the subsequent
> > frames.
> 
> I just tried with emacs -Q, executing only those `set-face-attribute` calls and
> the problem persists. I need to execute both sets in order to get it to work. 

Please show a recipe, starting from "emacs -Q", to reproduce the issue.

> Note that the distinction isn't really initial frame vs. subsequent frames.
> Rather, it's frames created before executing the calls to `set-face-attributes`
> and after executing those calls.

I don't think I understand what you are saying, but FRAME nil is
documented as affecting both all the existing frames and the future
frames.

> I'm assuming that this is the case mentioned in the doc string of
> set-face-attribute:
> 
> "[...] to reset the value
> of some attribute to ‘unspecified’ in a way that overrides the
> non-‘unspecified’ value defined by the face’s spec in ‘defface’,
> for new frames, you must explicitly call this function with FRAME
> set to t and the attribute’s value set to ‘unspecified’; just
> using FRAME of nil will not affect new frames in this case.to reset the value
> of some attribute to ‘unspecified’ in a way that overrides the
> non-‘unspecified’ value defined by the face’s spec in ‘defface’,
> for new frames, you must explicitly call this function with FRAME
> set to t and the attribute’s value set to ‘unspecified’; just
> using FRAME of nil will not affect new frames in this case."

I don't think this is related: you only used 'unspecified' in the call
with FRAME t, not in the calls with FRAME nil (which should have taken
care of everything).

Re: Question about customising faces

[Joost Kremers]

On Tue, Aug 30 2022, Eli Zaretskii wrote:
>> From: Joost Kremers <joostkremers@fastmail.fm>
>> Cc: help-gnu-emacs@gnu.org
>> Date: Mon, 29 Aug 2022 22:07:41 +0200
>> 
>> On Mon, Aug 29 2022, Eli Zaretskii wrote:
>> > The call with FRAME nil should have been enough.  And in my testing,
>> > it is: it sets the attributes for both the first and the subsequent
>> > frames.
>> 
>> I just tried with emacs -Q, executing only those `set-face-attribute` calls
>> and
>> the problem persists. I need to execute both sets in order to get it to work. 
>
> Please show a recipe, starting from "emacs -Q", to reproduce the issue.

I do the following, starting from `emacs -Q`:

- Evaluate the following `set-face-attribute` calls (with FRAME set to nil, no
  use of `unspecified`):

  (set-face-attribute 'isearch nil :background nil :foreground "#d33682")
  (set-face-attribute 'isearch-group-1 nil :background "lightskyblue1" :foreground "#d33682")
  (set-face-attribute 'isearch-group-2 nil :background "DarkSeaGreen1" :foreground "#d33682")
  (set-face-attribute 'lazy-highlight  nil :background nil :foreground nil :box "#d33682")

- Start isearch in any buffer and observe the changed faces.

- Create a new frame with `C-x 5 2`.

- Start isearch again and observe that the faces are a mix of old an new. Most
  notably are the background colors, which I'm trying to set to nil but which
  are present in the newly created frame. The same for the foreground of
  lazy-highlight. The box of lazy-highlight does appear, though.

- Now evaluate the other four calls to `set-face-attribute`:

  (set-face-attribute 'isearch t :background 'unspecified :foreground "#d33682")
  (set-face-attribute 'isearch-group-1 t :background "lightskyblue1" :foreground "#d33682")
  (set-face-attribute 'isearch-group-2 t :background "DarkSeaGreen1" :foreground "#d33682")
  (set-face-attribute 'lazy-highlight  t :background 'unspecified :foreground 'unspecified :box "#d33682")

- Create another frame with `C-x 5 2`.

- Start isearch; now the faces in the newly created frame are what I expect them
  to be, i.e., the same as in the initial frame.

Tested on Emacs 28.1.

-- 
Joost Kremers
Life has its moments

Re: Question about customising faces

[Joost Kremers]

On Mon, Aug 29 2022, Stefan Monnier via Users list for the GNU Emacs text editor wrote:
>> I just tried with emacs -Q, executing only those `set-face-attribute`
>> calls and the problem persists. I need to execute both sets in order
>> to get it to work. 
>
> Is there a particular reason why you use `set-face-attribute` rather
> than a declarative solution like M-x customize-face?

Well, a while ago I decided to stop using Customize because of
`package-selected-packages` being written to my `custome-file`. I couldn't find
a hassle-free way to sync my Emacs config between machines. I kept having to
resolve merge conflicts caused by `package-selected-packages`, so I took
`custom-file` out of version control. Now it's only used for
`package-selected-packages` and everything else is in init.el and files loaded
from there.


-- 
Joost Kremers
Life has its moments

Re: Question about customising faces

[Eli Zaretskii]

> From: Joost Kremers <joostkremers@fastmail.fm>
> Cc: help-gnu-emacs@gnu.org
> Date: Tue, 30 Aug 2022 08:44:53 +0200
> 
> > Please show a recipe, starting from "emacs -Q", to reproduce the issue.
> 
> I do the following, starting from `emacs -Q`:
> 
> - Evaluate the following `set-face-attribute` calls (with FRAME set to nil, no
>   use of `unspecified`):
> 
>   (set-face-attribute 'isearch nil :background nil :foreground "#d33682")
>   (set-face-attribute 'isearch-group-1 nil :background "lightskyblue1" :foreground "#d33682")
>   (set-face-attribute 'isearch-group-2 nil :background "DarkSeaGreen1" :foreground "#d33682")
>   (set-face-attribute 'lazy-highlight  nil :background nil :foreground nil :box "#d33682")
> 
> - Start isearch in any buffer and observe the changed faces.
> 
> - Create a new frame with `C-x 5 2`.
> 
> - Start isearch again and observe that the faces are a mix of old an new. Most
>   notably are the background colors, which I'm trying to set to nil but which
>   are present in the newly created frame. The same for the foreground of
>   lazy-highlight. The box of lazy-highlight does appear, though.

Well, you should have said explicitly that the problem only happens
with colors that you set to nil.  If I missed that, I apologize for
not spotting that important detail.

What did you expect to happen as result of setting the color of a face
to nil?

And yes, for what (I think) you wanted to achieve, two calls that you
show in the OP are really necessary.

Re: Question about customising faces

[Stefan Monnier]

>> Is there a particular reason why you use `set-face-attribute` rather
>> than a declarative solution like M-x customize-face?
>
> Well, a while ago I decided to stop using Customize because of
> `package-selected-packages` being written to my
> `custome-file`.  I couldn't find a hassle-free way to sync my Emacs
> config between machines.  I kept having to resolve merge conflicts
> caused by `package-selected-packages`, so I took `custom-file` out of
> version control.  Now it's only used for `package-selected-packages`
> and everything else is in init.el and files loaded from there.

Too bad, because in my experience `set-face-attribute` is more often
a problem than a solution.  Maybe you should try to use
`face-spec-set` instead.

Regarding your problem with `package-selected-packages`, I plead guilty.
Not sure how best to address that problem.  In the short term, you can try
something like

    (advice-add 'package--save-selected-packages :override #'ignore)

tho it probably comes with other undesirable side-effects.  I suggest
you file a bug report (and send me the bug#nb) so we can discuss
alternatives (and if you filed one already, just send me the bug#nb).



        Stefan

Re: Question about customising faces

[Gregory Heytings]

>
> - Evaluate the following `set-face-attribute` calls (with FRAME set to nil, no use of `unspecified`):
>
> (set-face-attribute 'isearch nil :background nil :foreground "#d33682")
> (set-face-attribute 'isearch-group-1 nil :background "lightskyblue1" :foreground "#d33682")
> (set-face-attribute 'isearch-group-2 nil :background "DarkSeaGreen1" :foreground "#d33682")
> (set-face-attribute 'lazy-highlight  nil :background nil :foreground nil :box "#d33682")
>

Why don't use use :background 'unspecified here?

Re: Question about customising faces

[Eli Zaretskii]

> Date: Tue, 30 Aug 2022 15:52:18 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: Eli Zaretskii <eliz@gnu.org>, help-gnu-emacs@gnu.org
> 
> Why don't use use :background 'unspecified here?

That would still require 2 calls, see the doc string of
set-face-attribute.

Resetting a face attribute is a rare enough (and unusual) thing to do,
so we didn't bother optimizing that for all the current and future
frames.

Re: Question about customising faces

[Gregory Heytings]

>> Why don't use use :background 'unspecified here?
>
> That would still require 2 calls, see the doc string of 
> set-face-attribute.
>

Is the docstring right?  With that recipe, using 'unspecified together 
with frame nil affects all current and future frames here.

Re: Question about customising faces

[Eli Zaretskii]

> Date: Tue, 30 Aug 2022 17:50:45 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: help-gnu-emacs@gnu.org
> 
> 
> >> Why don't use use :background 'unspecified here?
> >
> > That would still require 2 calls, see the doc string of 
> > set-face-attribute.
> >
> 
> Is the docstring right?  With that recipe, using 'unspecified together 
> with frame nil affects all current and future frames here.

Which recipe? what did you try, exactly?

Re: Question about customising faces

[Gregory Heytings]

>
> Which recipe? what did you try, exactly?
>

emacs -Q
C-x 5 2
M-: (set-face-attribute 'isearch nil :background 'unspecified :foreground "#d33682") RET
C-s buffer
C-x 5 2
C-s buffer

I see exactly the exact same colors with the two isearch commands.

But perhaps I misunderstand what Joost is trying to do.

Re: Question about customising faces

[Eli Zaretskii]

> Date: Tue, 30 Aug 2022 18:31:22 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: help-gnu-emacs@gnu.org
> 
> emacs -Q
> C-x 5 2
> M-: (set-face-attribute 'isearch nil :background 'unspecified :foreground "#d33682") RET
> C-s buffer
> C-x 5 2
> C-s buffer
> 
> I see exactly the exact same colors with the two isearch commands.

Joost used nil, not 'unspecified, for the background attribute.

Re: Question about customising faces

[Gregory Heytings]

>> emacs -Q
>> C-x 5 2
>> M-: (set-face-attribute 'isearch nil :background 'unspecified :foreground "#d33682") RET
>> C-s buffer
>> C-x 5 2
>> C-s buffer
>>
>> I see exactly the exact same colors with the two isearch commands.
>
> Joost used nil, not 'unspecified, for the background attribute.
>

Which is why I asked why he did not use 'unspecified instead of nil 😉

What does nil do that 'unspecified doesn't?

(And after using nil, describe-face displays 'unspecified.)

Re: Question about customising faces

[Eli Zaretskii]

> Date: Tue, 30 Aug 2022 18:48:47 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: help-gnu-emacs@gnu.org
> 
> > Joost used nil, not 'unspecified, for the background attribute.
> >
> 
> Which is why I asked why he did not use 'unspecified instead of nil 😉
> 
> What does nil do that 'unspecified doesn't?

Actually, nil is not valid as a color, but people still use it.

See the discussion of bug#54156.

Re: Question about customising faces

[Gregory Heytings]

>
> Actually, nil is not valid as a color, but people still use it.
>
> See the discussion of bug#54156.
>

I remember that discussion indeed.  So according to what you said there 
Joost should use

(set-face-attribute 'isearch nil :background 'unspecified :foreground "#d33682")
(set-face-attribute 'isearch t :background 'unspecified :foreground "#d33682")

What I don't understand is why the second call to set-face-attribute is 
needed: here at least, the first call also affects future frames.

Re: Question about customising faces

[Joost Kremers]

On Tue, Aug 30 2022, Gregory Heytings wrote:
>>
>> Actually, nil is not valid as a color, but people still use it.
>>
>> See the discussion of bug#54156.
>>
>
> I remember that discussion indeed.  So according to what you said there Joost
> should use
>
> (set-face-attribute 'isearch nil :background 'unspecified :foreground "#d33682")
> (set-face-attribute 'isearch t :background 'unspecified :foreground "#d33682")
>
> What I don't understand is why the second call to set-face-attribute is needed:
> here at least, the first call also affects future frames.

Yeah, it works for me as well if I use 'unspecified. The second call (with the
FRAME argument set to t) is not necessary.

Thanks!

-- 
Joost Kremers
Life has its moments

Re: Question about customising faces

[Joost Kremers]

On Tue, Aug 30 2022, Eli Zaretskii wrote:
> Well, you should have said explicitly that the problem only happens
> with colors that you set to nil.  If I missed that, I apologize for
> not spotting that important detail.

No, I apologise. I didn't say that explicitly.

> What did you expect to happen as result of setting the color of a face
> to nil?

That the face would use the background color defined for the default face.

> And yes, for what (I think) you wanted to achieve, two calls that you
> show in the OP are really necessary.

It seems that a single call, with FRAME set to t and the color set to
'unspecified, works for both the existing frame and for new frames.

[quoting from another email in the thread]
> Resetting a face attribute is a rare enough (and unusual) thing to do,
> so we didn't bother optimizing that for all the current and future
> frames.

Basically, what I wanted to do was to customise a face without using the
Customize interface. Honestly, I don't know if `set-face-attribute` is the
correct way of doing that, but I have similar customisations in my init file for
mode-line-related faces which I found on the net somewhere and they've always
worked.

-- 
Joost Kremers
Life has its moments

Re: Question about customising faces

[Joost Kremers]

On Tue, Aug 30 2022, Stefan Monnier wrote:
> Too bad, because in my experience `set-face-attribute` is more often
> a problem than a solution.  Maybe you should try to use
> `face-spec-set` instead.

Didn't know about `face-spec-set`. Is there a recommended way to customise a
face in one's init file without using Customize?

> Regarding your problem with `package-selected-packages`, I plead guilty.
> Not sure how best to address that problem.  In the short term, you can try
> something like
>
>     (advice-add 'package--save-selected-packages :override #'ignore)
>
> tho it probably comes with other undesirable side-effects.  I suggest
> you file a bug report (and send me the bug#nb) so we can discuss
> alternatives (and if you filed one already, just send me the bug#nb).

I can file a bug report, but note that this has been discussed before:

https://lists.gnu.org/archive/html/emacs-devel/2016-02/msg00958.html

(The discussion at the time morphed into a discussion about persistent storage,
which I think is also still an open issue...)

-- 
Joost Kremers
Life has its moments

Re: Question about customising faces

[Stefan Monnier]

>> Too bad, because in my experience `set-face-attribute` is more often
>> a problem than a solution.  Maybe you should try to use
>> `face-spec-set` instead.
> Didn't know about `face-spec-set`. Is there a recommended way to customise a
> face in one's init file without using Customize?

I recommend `custom-set-faces` first (i.e. Customize).
And `face-spec-set` as second best.
I can't recommend `set-face-attribute` at all.

>> Regarding your problem with `package-selected-packages`, I plead guilty.
>> Not sure how best to address that problem.  In the short term, you can try
>> something like
>>
>>     (advice-add 'package--save-selected-packages :override #'ignore)
>>
>> tho it probably comes with other undesirable side-effects.  I suggest
>> you file a bug report (and send me the bug#nb) so we can discuss
>> alternatives (and if you filed one already, just send me the bug#nb).
>
> I can file a bug report, but note that this has been discussed before:

Yes, I know, but I'm hoping we can make this discussion a bit more focused.
I think it really needs a way out.


        Stefan

RE: [External] : Re: Question about customising faces

[Drew Adams]

> See the discussion of bug#54156.

Which here, for those interested:

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=54156

Re: Question about customising faces

[Eli Zaretskii]

> Date: Tue, 30 Aug 2022 19:15:59 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: help-gnu-emacs@gnu.org
> 
> > Actually, nil is not valid as a color, but people still use it.
> >
> > See the discussion of bug#54156.
> >
> 
> I remember that discussion indeed.  So according to what you said there 
> Joost should use
> 
> (set-face-attribute 'isearch nil :background 'unspecified :foreground "#d33682")
> (set-face-attribute 'isearch t :background 'unspecified :foreground "#d33682")
> 
> What I don't understand is why the second call to set-face-attribute is 
> needed: here at least, the first call also affects future frames.

According to the discussion there, if all the calls use 'unspecified,
only the first one is needed.  But if nil is used, something that
people think they can do, both calls are needed.

Re: Question about customising faces

[Gregory Heytings]

>
> According to the discussion there, if all the calls use 'unspecified, 
> only the first one is needed.  But if nil is used, something that people 
> think they can do, both calls are needed.
>

Okay, I opened a bug report about this (bug#57499).

Re: Question about customising faces

[Joost Kremers]

On Tue, Aug 30 2022, Stefan Monnier wrote:
[package-selected-packages]
> I suggest
> you file a bug report (and send me the bug#nb)

I filed a bug report but forgot to send you the bug number. It's 57503.

-- 
Joost Kremers
Life has its moments