bug#73862: [PATCH] Add `header-line-active` and `header-line-inactive` faces.

[Aaron Jensen <aaronjensen@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 8760 bytes --]

On Thu, Dec 05, 2024 at 12:42 PM, Eli Zaretskii <eliz@gnu.org> wrote:

> From: Aaron Jensen <aaronjensen@gmail.com>
> Date: Thu, 5 Dec 2024 08:02:06 -0800
> Cc: trevor.m.murphy@gmail.com, me@eshelyaron.com, 73862@debbugs.gnu.org
>
> On Wed, Dec 04, 2024 at 11:51 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>
> Once again, please show some simple Lisp to reproduce the phenomena you
> are observing. It is hard to discuss these highly technical issues on this
> abstract level.
>
> Happy to. Sorry, I didn't think it was necessary given Eshel's providing
> of the recipe.
>
> I asked a recipe from you because you seemed to be reporting something
> different: the happening once" part.
>
> 1. (setq header-line-format "Some Header")
> 2. (face-remap-set-base 'header-line 'highlight)
> 3. (set-face-attribute 'default nil :height (+ (face-attribute 'default
> :height) 10))
> 4. (switch-to-buffer-other-window "new")
> 5. From within new buffer/window: (set-face-attribute 'default nil
> :height (+ (face-attribute 'default :height) 10))
>
> You'll see after step 3 the header line switches to the highlight face.
> After step 5, the header line in the original buffer switches away from the
> highlight face, which is unexpected.
>
> Why are you changing the attributes of the default face in step 3 and step
> 5 when what you want is to change the header-line face? And do you
> understand why step 2 didn't change the way header line is displayed, but
> step 3 did?
>

Only to force a redisplay in a way that actually causes the inheritance to
be considered. Note that when you reproduce this same thing with the
mode-line, force-mode-line-update doesn't work to cause the inheritance to
be considered. There's something "special" about changing the font or the
geometry of the frame or something that's causing the consideration of
inheritance.


> If you then switch back to the original buffer and change the font size
> again, the highlight will display again.
>
> How come the font of the default face affects the colors of the
> header-line faces? Doesn't that already look strange?
>

The strange thing is that when and how the inheritance is taken into
account is not the same as for other faces. So yes, it already looks
strange — I shouldn't need to force a redisplay. Affecting the font of the
default face is just one means by which I can trigger the redisplay in a
way that causes the issue. The specifics of it are a red herring. When I
first noticed the issue it was as a result of a vterm popup window being
shown. No default font modifications were made. I don't know what the
specific trigger was, however.


> Switching back to the new buffer and changing the font size causes it to
> disappear again, so I was mistaken about it only happening once.
>
> OK, so that mystery doesn't exist. Good.
>
> On the very basic level of the display code, when a display iterator is
> initialized, the window for whose display the iterator is used must be
> given to init_iterator as its argument, and the buffer of that window must
> be temporarily made to be the current buffer. So this cannot be the problem
> in this case. If init_iterator would be passed an incorrect window, we'd
> have much more grave display problems than this minor issue.
>
> Understood, but that's not what I was attempting to describe. I was trying
> to say that the window passed to init_interator IS the correct window. And
> that that is used to resolve the remap for the inactive/active faces.
> However, it does not appear that that same window is used to resolve
> remapping of faces that are inherited by the inactive/active faces.
>
> Face remapping is local to buffers, not to windows. So which window is
> passed to init_interator is not relevant to face remapping; the buffer of
> that window is.
>

My mistake. Even so, the buffer isn't being taken into account
consistently, but you speak to that below.


> From what I can tell, this only applies to header-line-active and
> header-line-inactive (and their mode line equivalents) and it does not
> apply to other faces in the header line that may inherit from remapped
> faces, so that's good.
>
> Also, I understand what you're saying that remapping mode-line was decided
> in Emacs 29 to not be supported and the conclusion may be the same for
> header-line. If that's what it needs to be, that's fine. As Eshel said, it
> will be a breaking change because people have relied on this behavior.
>
> I think there's no way to support what you expect cleanly. The reason is
> simple: inheritance in so-called "basic faces" is not taken into account
> when face-remapping is considered for them. Face inheritance is considered
> when faces are merged, but basic faces are not merged when they are used by
> the display engine for the respective parts of the Emacs display (i.e., the
> header-line faces for the header line, as opposed to just using the
> header-line faces for buffer text). That changing the attributes of the
> default faces has the effect of propagating the inheritance through
> face-remapping is an accident, the side effect of the particular
> implementation of when we recompute all the basic faces anew. It could be
> considered a bug on its own.
>
> We could perhaps add special-purpose code that would consider face
> inheritance for mode-line and header-line faces when remapping them, but
> that would get is in a different mess:
>
> . mode-line-inactive inherits from mode-line only by default, on color
> displays it doesn't; so if you remap mode-line, mode-line-inactive will
> only be affected on monochrome displays
> . header-line also inherits from mode-line, albeit only by default, so
> remapping mode-line would _sometimes_ affect header-line (and also
> header-line-active and header-line-inactive) and sometimes not
> . if the user customizes some of the mode-line or header-line faces to not
> inherit or to always inherit, remapping mode-line or header-line will
> produce different results for different faces
>
> So basically it's a royal mess, all of it caused by the fact that people
> somehow expect that remapping the header-line face will affect
> header-line-active and header-line-inactive, and similarly for mode-line.
> That was never supported for basic faces, and I think it makes little sense
> to support it in the future, because the results will certainly surprise
> someone.
>
> So I tend to close this bug as wontfix, and just mention in the
> documentation (NEWS at least) that people who remap header-line or
> mode-line need now to remap the new -active and -inactive faces.
>

Ok, I get what you're saying here, but I don't know that I agree with this:
"...people somehow expect that remapping the header-line face will affect
header-line-active and header-line-inactive." I'm not sure why it wouldn't
be expected. It's reasonable to expect that inherit and face remapping
would work the same way always. I didn't even know there was a notion of
"basic faces" and what that meant. If that's somehow a special designation
that confers different behavior, then we shouldn't be surprised if people
are confused by its differences. I do not have these differences memorized,
and ultimately that's what it sounds like we are being asked to do. Yes,
you can remap faces, yes, you can inherit faces, and yes those two work
together EXCEPT in this one case. Even with documentation, this will be
confusing because we cannot rely on people finding that documentation.

If basic faces do not fully support inheritance, we probably shouldn't
allow them to have inheritance. That would mean that header-line-inactive
would be defined explicitly, without inheritance, and if someone attempted
to add an inherit property, it would fail.

This would mean that the base faces, header-line and mode-line would be
eliminated. Is it too much to ask for someone to explicitly configure the
four faces (mode-line-active, mode-line-inactive, header-line-active, and
header-line-inactive) any time they want to apply a theme to them? It
sounds like that's the expectation for remapping, so why not extend that to
customizing them?

If there were no header-line or mode-line face then I would not have run
into this. I would have been required to update my theme accordingly and
the code that I use to remap header line faces, but that would have been
apparent and seems like a reasonable change to make in a major version, but
I can't recommend any of this because I don't know enough about the version
criteria, release process, internals, etc, so please consider this just a
perspective.

Thanks,

[-- Attachment #2: Type: text/html, Size: 12196 bytes --]