GNU bug report logs - #77928
[PATCH] use-package :custom-face is meant to behave like custom-set-face

Previous Next

Full log

Message #61 received at 77928 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Steven Allen <steven <at> stebalien.com>
Cc: 77928 <at> debbugs.gnu.org, johnw <at> gnu.org, shields <at> msrl.com
Subject: Re: bug#77928: 31.0.50; ebcde0f90f6 (bug#77928) breaks :custom-face
 for themed faces
Date: Wed, 28 May 2025 14:10:37 +0300

> From: Steven Allen <steven <at> stebalien.com>
> Cc: johnw <at> gnu.org, shields <at> msrl.com, 77928 <at> debbugs.gnu.org
> Date: Tue, 27 May 2025 09:38:28 -0700
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> From: Steven Allen <steven <at> stebalien.com>
> >> Cc: Michael Shields <shields <at> msrl.com>, 77928 <at> debbugs.gnu.org
> >> Date: Mon, 26 May 2025 18:26:24 -0700
> >>
> >> John Wiegley <johnw <at> gnu.org> writes:
> >>
> >> >>>>>> Eli Zaretskii <eliz <at> gnu.org> writes:
> >> >
> >> >>> From: Michael Shields <shields <at> msrl.com>
> >> >>> Date: Mon, 19 May 2025 22:51:29 -0700
> >> >>> Cc: steven <at> stebalien.com, 77928 <at> debbugs.gnu.org
> >> >>>
> >> >>> I think I understand the issue, but not its solution. There are more or less three standard ways to change
> >> >>> faces now: Customize, themes, and use-package :custom-face. But there is no clear way to resolve the
> >> >>> ordering between them, or whether they replace or inherit at each level. The behavior I was surprised by,
> >> >>> and that has been reported previously, is that there was no way to specify a replacement for face settings.
> >> >>> But Steven is surprised by the fact that things now happen at a level below theme application, and the theme
> >> >>> replaces instead of inherits the defface spec.
> >> >
> >> >> John, could you perhaps help us out here?  Bug#77928 was about
> >> >> :custom-face, which is a use-package feature.  I don't really
> >> >> understand how this feature is supposed to work, given that
> >> >> use-package produces macros whose effect happens when a package is
> >> >> loaded, and given that other methods of face definition are available.
> >> >
> >> > All that use-package is doing here is expanding the ‘:custom-face‘ declaration
> >> > into this call:
> >> >
> >> >   (apply #'face-spec-set '(font-lock-keyword-face ((t :italic t))))
> >> >
> >> > Should it be calling something other than `face-spec-set' now?
> >>
> >> That's what it used to be before the change in question. Now it expands
> >> to:
> >>
> >>      (apply #'face-spec-set '(font-lock-keyword-face ((t :italic t)) face-defface-spec))
> >>
> >> The issue here is that the themed face spec has a higher priority than
> >> the "defface" spec.
> >
> > But AFAIU, face-defface-spec records the theme customizations of the
> > face as well, no?  I'm afraid I don't quite understand what you are
> > saying here, could you please explain and elaborate?
> 
> face-defface-spec is only used when declaring faces, not when
> customizing/theming them. The theme-face property is used to store the
> themed face, customized-face is used to store customizations through the
> "custom" system, and face-override-spec is used otherwise.
> 
> You can look at a face's plist to get a better idea of how this works:
> 
>     (symbol-plist 'default)
> 
> Also look at the documentation for facep-spec-recalc: theme and/or user
> customizations override the "defface" spec, causing the bug I'm seeing here.
> 
> >> IMO, this should be reverted to just:
> >>
> >>      (apply #'face-spec-set '(font-lock-keyword-face ((t :italic t))))
> >>
> >> (leaving the part of the patch that marks the face as "modified" intact).
> >>
> >> >From what I can tell, the patch in question was attempting to solve an
> >> issue where overriding faces get "merged" with "lower" priority face
> >> specs but, IMO, that isn't really a bug.
> >
> > Why don't you consider that a bug?
> 
> That's how face customization works. E.g.:
> 
>     (custom-set-faces '(font-lock-keyword-face ((t :italic t)) t))
> 
> Will make keywords italic but won't override the theme/defaults unless
> requested. E.g.:
> 
>     (custom-set-faces
>       '(font-lock-keyword-face ((t :italic t :foreground unspecified)) t))
> 
> Will "unset" the foreground (it'll inherit from the face's parent). And:
> 
>     (custom-set-faces
>       '(font-lock-keyword-face ((t :italic t :foreground reset)) t))
> 
> Will "reset" the foreground to the default face's foreground.
> 
> This makes it possible to compose/modify faces without completely
> overriding them. E.g., if you want a specific face to be bold/italic but
> otherwise want the underlying theme to apply. This makes it possible to,
> e.g., customize certain aspects of faces across themes.

Thanks.

Unfortunately, what you say above seems to assume the level of
detailed knowledge in themes, use-package, and their interactions with
Customize that I don't have.  I think I know how faces work and how
face merging works, but after reading your explanations and examples
several times I'm sorry to say that I'm utterly confused by what you
say and don't have any clear idea about the problem, its consequences,
and your analysis of the original issue and its fix.

So either someone explains this to me in terms I can understand and
reason about, or we will need to wait for someone more knowledgeable
in this stuff to pick up the gauntlet.

Alternatively, if you convince John and Michael that the changes you
think should be done are indeed the way to go, I will gladly delegate
the decision to you three.

Sorry I couldn't be of more help.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.