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

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

[trevor.m.murphy]

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

Tags: patch

Got a quick feature request.

Over the years I've put more and more information into the header line, and at some point I wanted to get that quick visual indicator of selected vs non-selected windows when I glanced at the header as I already got from the mode line.

This was surprisingly hard to achieve!  For a few years I used an ugly `buffer-list-update-hook` hack.  Then a few months ago I got adventurous and found that it actually wasn't as hard as I'd feared to hack these into the C source.

The new faces both inherit from the current `header-line` face, so there shouldn't be any visible impact if users don't customize the new faces.  For cases like mine it's easy to customize header-line-inactive to inherit from mode-line-inactive.

Thanks in advance for your attention and feedback,

Trevor


In GNU Emacs 29.4 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.43,
cairo version 1.18.0)
Repository revision: a92669f7ae7de3172cf883d15af9d7bb9bda7dd9
Repository branch: main
System Description: Arch Linux

Configured using:
 'configure --with-pgtk --with-native-compilation=aot --sysconfdir=/etc
 --prefix=/usr --libexecdir=/usr/lib --with-tree-sitter
 --localstatedir=/var --with-cairo --disable-build-details
 --with-harfbuzz --with-libsystemd --with-modules 'CFLAGS=-march=x86-64
 -mtune=generic -O2 -pipe -fno-plt -fexceptions -Wp,-D_FORTIFY_SOURCE=3
 -Wformat -Werror=format-security -fstack-clash-protection
 -fcf-protection -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -g
 -ffile-prefix-map=/home/trevor/pkgbuilds/abs/emacs/src=/usr/src/debug/emacs
 -flto=auto' 'LDFLAGS=-Wl,-O1 -Wl,--sort-common -Wl,--as-needed
 -Wl,-z,relro -Wl,-z,now -Wl,-z,pack-relative-relocs -flto=auto'
 'CXXFLAGS=-march=x86-64 -mtune=generic -O2 -pipe -fno-plt -fexceptions
 -Wp,-D_FORTIFY_SOURCE=3 -Wformat -Werror=format-security
 -fstack-clash-protection -fcf-protection -fno-omit-frame-pointer
 -mno-omit-leaf-frame-pointer -Wp,-D_GLIBCXX_ASSERTIONS -g
 -ffile-prefix-map=/home/trevor/pkgbuilds/abs/emacs/src=/usr/src/debug/emacs
 -flto=auto''


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-new-header-line-active-and-header-line-inactive-.patch --]
[-- Type: text/patch, Size: 12768 bytes --]

From 1000a0bac791dbe31da627389db9755fddf51243 Mon Sep 17 00:00:00 2001
From: Trevor Murphy <trevor.m.murphy@gmail.com>
Date: Thu, 17 Oct 2024 15:51:14 -0700
Subject: [PATCH] Add new `header-line-active` and `header-line-inactive`
 faces.

This is all intended to parallel the mode-line-active and
mode-line-inactive distinction.

* src/dispextern.h (CURRENT_HEADER_LINE_ACTIVE_FACE_ID_3,
CURRENT_HEADER_LINE_ACTIVE_FACE_ID): new macros based on mode line
equivalents
(face_id): new face IDs
* src/xdisp.c (window_box_height, pos_visible_p, init_iterator,
window_text_pixel_size, display_mode_lines, display_mode_line,
format-mode-line): replace all uses of HEADER_LINE_FACE_ID with either
a new macro or the new face IDs
* src/xfaces.c (syms_of_xfaces): new lisp symbols
(lookup_basic_face, realize_basic_faces): map new face IDs to their lisp
symbols
* lisp/faces.el (header-line-active, header-line-inactive): new deffaces
---
 lisp/faces.el    | 24 ++++++++++++++++++++++++
 src/dispextern.h | 33 +++++++++++++++++++++++++++++++--
 src/xdisp.c      | 45 ++++++++++++++++++++++++++-------------------
 src/xfaces.c     |  8 ++++++--
 4 files changed, 87 insertions(+), 23 deletions(-)

diff --git a/lisp/faces.el b/lisp/faces.el
index 21c3e663c6e..25a93d0ed7a 100644
--- a/lisp/faces.el
+++ b/lisp/faces.el
@@ -2821,6 +2821,30 @@ Use the face `mode-line-highlight' for features that can be selected."
   :version "28.1"
   :group 'basic-faces)
 
+(defface header-line-active
+  '((t :inherit header-line))
+  "Face for the selected header line.
+This inherits from the `header-line' face."
+  :version "29.5"
+  :group 'mode-line-faces
+  :group 'basic-faces)
+
+(defface header-line-inactive
+  '((default
+     :inherit header-line)
+    (((class color) (min-colors 88) (background light))
+     :weight light
+     :box (:line-width -1 :color "grey75" :style nil)
+     :foreground "grey20" :background "grey90")
+    (((class color) (min-colors 88) (background dark) )
+     :weight light
+     :box (:line-width -1 :color "grey40" :style nil)
+     :foreground "grey80" :background "grey30"))
+  "Basic header line face for non-selected windows."
+  :version "29.5"
+  :group 'mode-line-faces
+  :group 'basic-faces)
+
 (defface vertical-border
   '((((type tty)) :inherit mode-line-inactive))
   "Face used for vertical window dividers on ttys."
diff --git a/src/dispextern.h b/src/dispextern.h
index cc248a4472e..0d4c0a6c67e 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -1561,6 +1561,34 @@ struct glyph_string
 	 : estimate_mode_line_height					\
 	 (XFRAME ((W)->frame), CURRENT_MODE_LINE_ACTIVE_FACE_ID (W)))))
 
+/* Return the desired face id for the header line of a window, depending
+   on whether the window is selected or not, or if the window is the
+   scrolling window for the currently active minibuffer window.
+
+   Due to the way display_mode_lines manipulates with the contents of
+   selected_window, this macro needs three arguments: SELW which is
+   compared against the current value of selected_window, MBW which is
+   compared against minibuf_window (if SELW doesn't match), and SCRW
+   which is compared against minibuf_selected_window (if MBW matches).  */
+
+#define CURRENT_HEADER_LINE_ACTIVE_FACE_ID_3(SELW, MBW, SCRW)    	\
+     ((!mode_line_in_non_selected_windows			\
+       || (SELW) == XWINDOW (selected_window)			\
+       || (minibuf_level > 0					\
+           && !NILP (minibuf_selected_window)			\
+           && (MBW) == XWINDOW (minibuf_window)			\
+           && (SCRW) == XWINDOW (minibuf_selected_window)))	\
+      ? HEADER_LINE_ACTIVE_FACE_ID				\
+      : HEADER_LINE_INACTIVE_FACE_ID)
+
+
+/* Return the desired face id for the header line of window W.  */
+
+#define CURRENT_HEADER_LINE_ACTIVE_FACE_ID(W)		\
+	 CURRENT_HEADER_LINE_ACTIVE_FACE_ID_3(W, \
+					    XWINDOW (selected_window), \
+					    W)
+
 /* Return the current height of the header line of window W.  If not known
    from W->header_line_height, look at W's current glyph matrix, or return
    an estimation based on the height of the font of the face `header-line'.  */
@@ -1572,7 +1600,7 @@ struct glyph_string
       = (MATRIX_HEADER_LINE_HEIGHT ((W)->current_matrix)	\
 	 ? MATRIX_HEADER_LINE_HEIGHT ((W)->current_matrix)	\
 	 : estimate_mode_line_height				\
-	     (XFRAME ((W)->frame), HEADER_LINE_FACE_ID))))
+	 (XFRAME ((W)->frame), CURRENT_HEADER_LINE_ACTIVE_FACE_ID (W)))))
 
 /* Return the current height of the tab line of window W.  If not known
    from W->tab_line_height, look at W's current glyph matrix, or return
@@ -1893,7 +1921,8 @@ enum face_id
   MODE_LINE_INACTIVE_FACE_ID,
   TOOL_BAR_FACE_ID,
   FRINGE_FACE_ID,
-  HEADER_LINE_FACE_ID,
+  HEADER_LINE_ACTIVE_FACE_ID,
+  HEADER_LINE_INACTIVE_FACE_ID,
   SCROLL_BAR_FACE_ID,
   BORDER_FACE_ID,
   CURSOR_FACE_ID,
diff --git a/src/xdisp.c b/src/xdisp.c
index c74e81a3933..98bc157c789 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -1358,7 +1358,7 @@ window_box_height (struct window *w)
 	  if (hl_row && hl_row->mode_line_p)
 	    height -= hl_row->height;
 	  else
-	    height -= estimate_mode_line_height (f, HEADER_LINE_FACE_ID);
+	    height -= estimate_mode_line_height (f, CURRENT_HEADER_LINE_ACTIVE_FACE_ID (w));
 	}
     }
 
@@ -1753,7 +1753,7 @@ pos_visible_p (struct window *w, ptrdiff_t charpos, int *x, int *y,
 	= window_parameter (w, Qheader_line_format);
 
       w->header_line_height
-	= display_mode_line (w, HEADER_LINE_FACE_ID,
+	= display_mode_line (w, CURRENT_HEADER_LINE_ACTIVE_FACE_ID (w),
 			     NILP (window_header_line_format)
 			     ? BVAR (current_buffer, header_line_format)
 			     : window_header_line_format);
@@ -3197,13 +3197,14 @@ CHECK_WINDOW_END (struct window *w)
 
    BASE_FACE_ID is the id of a base face to use.  It must be one of
    DEFAULT_FACE_ID for normal text, MODE_LINE_ACTIVE_FACE_ID,
-   MODE_LINE_INACTIVE_FACE_ID, or HEADER_LINE_FACE_ID for displaying
-   mode lines, or TOOL_BAR_FACE_ID for displaying the tool-bar.
+   MODE_LINE_INACTIVE_FACE_ID, HEADER_LINE_ACTIVE_FACE_ID, or
+   HEADER_LINE_INACTIVE_FACE_ID for displaying mode lines, or
+   TOOL_BAR_FACE_ID for displaying the tool-bar.
 
    If ROW is null and BASE_FACE_ID is equal to MODE_LINE_ACTIVE_FACE_ID,
-   MODE_LINE_INACTIVE_FACE_ID, or HEADER_LINE_FACE_ID, the iterator
-   will be initialized to use the corresponding mode line glyph row of
-   the desired matrix of W.  */
+   MODE_LINE_INACTIVE_FACE_ID, HEADER_LINE_ACTIVE_FACE_ID, or
+   HEADER_LINE_INACTIVE_FACE_ID the iterator will be initialized to use
+   the corresponding mode line glyph row of the desired matrix of W.  */
 
 void
 init_iterator (struct it *it, struct window *w,
@@ -3251,7 +3252,8 @@ init_iterator (struct it *it, struct window *w,
 	row = MATRIX_MODE_LINE_ROW (w->desired_matrix);
       else if (base_face_id == TAB_LINE_FACE_ID)
 	row = MATRIX_TAB_LINE_ROW (w->desired_matrix);
-      else if (base_face_id == HEADER_LINE_FACE_ID)
+      else if (base_face_id == HEADER_LINE_ACTIVE_FACE_ID
+	       || base_face_id == HEADER_LINE_INACTIVE_FACE_ID)
 	{
 	  /* Header line row depends on whether tab line is enabled.  */
 	  w->desired_matrix->tab_line_p = window_wants_tab_line (w);
@@ -11854,7 +11856,7 @@ window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to,
       Lisp_Object window_header_line_format
 	= window_parameter (w, Qheader_line_format);
 
-      y = y + display_mode_line (w, HEADER_LINE_FACE_ID,
+      y = y + display_mode_line (w, CURRENT_HEADER_LINE_ACTIVE_FACE_ID (w),
 				 NILP (window_header_line_format)
 				 ? BVAR (current_buffer, header_line_format)
 				 : window_header_line_format);
@@ -27453,11 +27455,11 @@ display_mode_lines (struct window *w)
   line_number_displayed = false;
   w->column_number_displayed = -1;
 
+  struct window *sel_w = XWINDOW (old_selected_window);
   if (window_wants_mode_line (w))
     {
       Lisp_Object window_mode_line_format
 	= window_parameter (w, Qmode_line_format);
-      struct window *sel_w = XWINDOW (old_selected_window);
 
       /* Select mode line face based on the real selected window.  */
       display_mode_line (w,
@@ -27485,7 +27487,7 @@ display_mode_lines (struct window *w)
       Lisp_Object window_header_line_format
 	= window_parameter (w, Qheader_line_format);
 
-      display_mode_line (w, HEADER_LINE_FACE_ID,
+      display_mode_line (w, CURRENT_HEADER_LINE_ACTIVE_FACE_ID_3 (sel_w, sel_w, w),
 			 NILP (window_header_line_format)
 			 ? BVAR (current_buffer, header_line_format)
 			 : window_header_line_format);
@@ -27500,11 +27502,12 @@ display_mode_lines (struct window *w)
 }
 
 
-/* Display mode or header/tab line of window W.  FACE_ID specifies
-   which line to display; it is either MODE_LINE_ACTIVE_FACE_ID,
-   HEADER_LINE_FACE_ID or TAB_LINE_FACE_ID.  FORMAT is the
-   mode/header/tab line format to display.  Value is the pixel height
-   of the mode/header/tab line displayed.  */
+/* Display mode or header/tab line of window W.  FACE_ID specifies which
+   line to display; it is either MODE_LINE_ACTIVE_FACE_ID,
+   HEADER_LINE_ACTIVE_FACE_ID, HEADER_LINE_INACTIVE_FACE_ID, or
+   TAB_LINE_FACE_ID.  FORMAT is the mode/header/tab line format to
+   display.  Value is the pixel height of the mode/header/tab line
+   displayed.  */
 
 static int
 display_mode_line (struct window *w, enum face_id face_id, Lisp_Object format)
@@ -27525,7 +27528,8 @@ display_mode_line (struct window *w, enum face_id face_id, Lisp_Object format)
       it.glyph_row->tab_line_p = true;
       w->desired_matrix->tab_line_p = true;
     }
-  else if (face_id == HEADER_LINE_FACE_ID)
+  else if (face_id == HEADER_LINE_ACTIVE_FACE_ID
+	   || face_id == HEADER_LINE_INACTIVE_FACE_ID)
     w->desired_matrix->header_line_p = true;
 
   /* FIXME: This should be controlled by a user option.  But
@@ -27544,7 +27548,9 @@ display_mode_line (struct window *w, enum face_id face_id, Lisp_Object format)
   record_unwind_save_match_data ();
 
   if (NILP (Vmode_line_compact)
-      || face_id == HEADER_LINE_FACE_ID || face_id == TAB_LINE_FACE_ID)
+      || face_id == HEADER_LINE_ACTIVE_FACE_ID
+      || face_id == HEADER_LINE_INACTIVE_FACE_ID
+      || face_id == TAB_LINE_FACE_ID)
     {
       mode_line_target = MODE_LINE_DISPLAY;
       display_mode_element (&it, 0, 0, 0, format, Qnil, false);
@@ -28313,7 +28319,8 @@ are the selected window and the WINDOW's buffer).  */)
 		       ? MODE_LINE_ACTIVE_FACE_ID : MODE_LINE_INACTIVE_FACE_ID)
     : EQ (face, Qmode_line_active) ? MODE_LINE_ACTIVE_FACE_ID
     : EQ (face, Qmode_line_inactive) ? MODE_LINE_INACTIVE_FACE_ID
-    : EQ (face, Qheader_line) ? HEADER_LINE_FACE_ID
+    : EQ (face, Qheader_line_active) ? HEADER_LINE_ACTIVE_FACE_ID
+    : EQ (face, Qheader_line_inactive) ? HEADER_LINE_INACTIVE_FACE_ID
     : EQ (face, Qtab_line) ? TAB_LINE_FACE_ID
     : EQ (face, Qtab_bar) ? TAB_BAR_FACE_ID
     : EQ (face, Qtool_bar) ? TOOL_BAR_FACE_ID
diff --git a/src/xfaces.c b/src/xfaces.c
index e248279e9b7..f6264802fa4 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -5117,7 +5117,8 @@ lookup_basic_face (struct window *w, struct frame *f, int face_id)
     case DEFAULT_FACE_ID:		name = Qdefault;		break;
     case MODE_LINE_ACTIVE_FACE_ID:	name = Qmode_line_active;      	break;
     case MODE_LINE_INACTIVE_FACE_ID:	name = Qmode_line_inactive;	break;
-    case HEADER_LINE_FACE_ID:		name = Qheader_line;		break;
+    case HEADER_LINE_ACTIVE_FACE_ID:	name = Qheader_line_active;	break;
+    case HEADER_LINE_INACTIVE_FACE_ID:	name = Qheader_line_inactive;	break;
     case TAB_LINE_FACE_ID:		name = Qtab_line;		break;
     case TAB_BAR_FACE_ID:		name = Qtab_bar;		break;
     case TOOL_BAR_FACE_ID:		name = Qtool_bar;		break;
@@ -5867,7 +5868,8 @@ realize_basic_faces (struct frame *f)
       realize_named_face (f, Qmode_line_inactive, MODE_LINE_INACTIVE_FACE_ID);
       realize_named_face (f, Qtool_bar, TOOL_BAR_FACE_ID);
       realize_named_face (f, Qfringe, FRINGE_FACE_ID);
-      realize_named_face (f, Qheader_line, HEADER_LINE_FACE_ID);
+      realize_named_face (f, Qheader_line_active, HEADER_LINE_ACTIVE_FACE_ID);
+      realize_named_face (f, Qheader_line_inactive, HEADER_LINE_INACTIVE_FACE_ID);
       realize_named_face (f, Qscroll_bar, SCROLL_BAR_FACE_ID);
       realize_named_face (f, Qborder, BORDER_FACE_ID);
       realize_named_face (f, Qcursor, CURSOR_FACE_ID);
@@ -7438,6 +7440,8 @@ syms_of_xfaces (void)
   DEFSYM (Qfringe, "fringe");
   DEFSYM (Qtab_line, "tab-line");
   DEFSYM (Qheader_line, "header-line");
+  DEFSYM (Qheader_line_inactive, "header-line-inactive");
+  DEFSYM (Qheader_line_active, "header-line-active");
   DEFSYM (Qscroll_bar, "scroll-bar");
   DEFSYM (Qmenu, "menu");
   DEFSYM (Qcursor, "cursor");
-- 
2.46.2

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

[Eli Zaretskii]

> From: trevor.m.murphy@gmail.com
> Date: Fri, 18 Oct 2024 05:56:32 -0700
> 
> Over the years I've put more and more information into the header line, and at some point I wanted to get that quick visual indicator of selected vs non-selected windows when I glanced at the header as I already got from the mode line.
> 
> This was surprisingly hard to achieve!  For a few years I used an ugly `buffer-list-update-hook` hack.  Then a few months ago I got adventurous and found that it actually wasn't as hard as I'd feared to hack these into the C source.
> 
> The new faces both inherit from the current `header-line` face, so there shouldn't be any visible impact if users don't customize the new faces.  For cases like mine it's easy to customize header-line-inactive to inherit from mode-line-inactive.
> 
> Thanks in advance for your attention and feedback,

Thanks.  This changeset needs the appropriate changes in the Emacs
user manual (which lists the standard faces in "Standard Faces"), and
also a NEWS entry that announces the change.

Also, did you verify that modes which use header-line and customize
the header-line face still work as before after this change, both when
the window is selected and non-selected?

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

[Eli Zaretskii]

> Cc: 73862@debbugs.gnu.org
> Date: Sun, 27 Oct 2024 12:46:53 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > From: trevor.m.murphy@gmail.com
> > Date: Fri, 18 Oct 2024 05:56:32 -0700
> > 
> > Over the years I've put more and more information into the header line, and at some point I wanted to get that quick visual indicator of selected vs non-selected windows when I glanced at the header as I already got from the mode line.
> > 
> > This was surprisingly hard to achieve!  For a few years I used an ugly `buffer-list-update-hook` hack.  Then a few months ago I got adventurous and found that it actually wasn't as hard as I'd feared to hack these into the C source.
> > 
> > The new faces both inherit from the current `header-line` face, so there shouldn't be any visible impact if users don't customize the new faces.  For cases like mine it's easy to customize header-line-inactive to inherit from mode-line-inactive.
> > 
> > Thanks in advance for your attention and feedback,
> 
> Thanks.  This changeset needs the appropriate changes in the Emacs
> user manual (which lists the standard faces in "Standard Faces"), and
> also a NEWS entry that announces the change.
> 
> Also, did you verify that modes which use header-line and customize
> the header-line face still work as before after this change, both when
> the window is selected and non-selected?

Ping! Trevor, could you please answer the above question, and provide
an updated patch with documentation changes?  I'd like to install this
as soon as these issues are resolved.

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

[Trevor Murphy]

[-- Attachment #1.1: Type: text/plain, Size: 2318 bytes --]

Hi!  Thanks for your patience, attached please find the updated patch.
I've added documentation and a NEWS entry modeled off of similar commits.

When I put together the original patch I included a non-trivial value for
the default header-line-inactive (copied from mode-line-inactive) as a
visual indicator for the new face.  Happy to take it out.  In this updated
patch, header-line-inactive just inherits from header-line.  I tested by
looking at selected and not-selected Info buffers with emacs -Q both
patched and unpatched, in both graphical and terminal.

On Sat, Nov 9, 2024 at 1:37 AM Eli Zaretskii <eliz@gnu.org> wrote:

> > Cc: 73862@debbugs.gnu.org
> > Date: Sun, 27 Oct 2024 12:46:53 +0200
> > From: Eli Zaretskii <eliz@gnu.org>
> >
> > > From: trevor.m.murphy@gmail.com
> > > Date: Fri, 18 Oct 2024 05:56:32 -0700
> > >
> > > Over the years I've put more and more information into the header
> line, and at some point I wanted to get that quick visual indicator of
> selected vs non-selected windows when I glanced at the header as I already
> got from the mode line.
> > >
> > > This was surprisingly hard to achieve!  For a few years I used an ugly
> `buffer-list-update-hook` hack.  Then a few months ago I got adventurous
> and found that it actually wasn't as hard as I'd feared to hack these into
> the C source.
> > >
> > > The new faces both inherit from the current `header-line` face, so
> there shouldn't be any visible impact if users don't customize the new
> faces.  For cases like mine it's easy to customize header-line-inactive to
> inherit from mode-line-inactive.
> > >
> > > Thanks in advance for your attention and feedback,
> >
> > Thanks.  This changeset needs the appropriate changes in the Emacs
> > user manual (which lists the standard faces in "Standard Faces"), and
> > also a NEWS entry that announces the change.
> >
> > Also, did you verify that modes which use header-line and customize
> > the header-line face still work as before after this change, both when
> > the window is selected and non-selected?
>
> Ping! Trevor, could you please answer the above question, and provide
> an updated patch with documentation changes?  I'd like to install this
> as soon as these issues are resolved.
>


-- 
Trevor Murphy

[-- Attachment #1.2: Type: text/html, Size: 3151 bytes --]

[-- Attachment #2: 0001-Add-new-header-line-active-and-header-line-inactive-.patch --]
[-- Type: text/x-patch, Size: 14234 bytes --]

From fed179e66b521d30e9a7f393230ce10573302973 Mon Sep 17 00:00:00 2001
From: Trevor Murphy <trevor.m.murphy@gmail.com>
Date: Thu, 17 Oct 2024 15:51:14 -0700
Subject: [PATCH] Add new `header-line-active` and `header-line-inactive`
 faces.

This is all intended to parallel the mode-line-active and
mode-line-inactive distinction.

* doc/emacs/display.texi (Standard Faces): Document the new faces.

* lisp/faces.el (header-line-active, header-line-inactive): New faces.

* src/dispextern.h (CURRENT_HEADER_LINE_ACTIVE_FACE_ID_3,
CURRENT_HEADER_LINE_ACTIVE_FACE_ID): new macros based on mode line
equivalents
(face_id): new face IDs
* src/xdisp.c (window_box_height, pos_visible_p, init_iterator,
window_text_pixel_size, display_mode_lines, display_mode_line,
format-mode-line): replace all uses of HEADER_LINE_FACE_ID with either
a new macro or the new face IDs
* src/xfaces.c (syms_of_xfaces): new lisp symbols
(lookup_basic_face, realize_basic_faces): map new face IDs to their lisp
symbols
---
 doc/emacs/display.texi | 16 +++++++++++++++
 etc/NEWS               |  5 +++++
 lisp/faces.el          | 15 ++++++++++++++
 src/dispextern.h       | 33 +++++++++++++++++++++++++++++--
 src/xdisp.c            | 45 ++++++++++++++++++++++++------------------
 src/xfaces.c           |  8 ++++++--
 6 files changed, 99 insertions(+), 23 deletions(-)

diff --git a/doc/emacs/display.texi b/doc/emacs/display.texi
index 88520874c8e..22cd316f836 100644
--- a/doc/emacs/display.texi
+++ b/doc/emacs/display.texi
@@ -782,6 +782,22 @@ at the top of a window just as the mode line appears at the bottom.
 Most windows do not have a header line---only some special modes, such
 Info mode, create one.
 
+The @code{header-line-active} and @code{header-line-inactive} faces (which
+are the ones used on the header lines) inherit from this face.
+
+@item header-line-active
+@cindex faces for header lines
+Like @code{header-line}, but used for the header line of the currently
+selected window.  This face inherits from @code{header-line}, so changes
+in that face affect header lines in all windows.
+
+@item header-line-inactive
+@cindex @code{header-line-inactive} face
+Like @code{header-line}, but used for header lines of the windows other
+than the selected one (if those windows have a header line).  This face
+inherits from @code{header-line}, so changes in that face affect header
+lines in all windows.
+
 @item header-line-highlight
 @cindex @code{header-line-highlight} face
 Similar to @code{highlight} and @code{mode-line-highlight}, but used
diff --git a/etc/NEWS b/etc/NEWS
index a8ece5c3dc9..b0390e597d0 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -150,6 +150,11 @@ Killed buffers stored in a register using 'buffer-to-register' are
 automatically converted to a file-query value if the buffer was visiting
 a file.
 
+** New face 'header-line-active'.
+This inherits from the 'header-line' face, but is the face actually used
+on the header lines (along with 'header-line-inactive').
+
++++
 \f
 * Editing Changes in Emacs 31.1
 
diff --git a/lisp/faces.el b/lisp/faces.el
index 21c3e663c6e..dd85376de24 100644
--- a/lisp/faces.el
+++ b/lisp/faces.el
@@ -2821,6 +2821,21 @@ Use the face `mode-line-highlight' for features that can be selected."
   :version "28.1"
   :group 'basic-faces)
 
+(defface header-line-active
+  '((t :inherit header-line))
+  "Face for the selected header line.
+This inherits from the `header-line' face."
+  :version "29.5"
+  :group 'mode-line-faces
+  :group 'basic-faces)
+
+(defface header-line-inactive
+  '((t :inherit header-line))
+  "Basic header line face for non-selected windows."
+  :version "29.5"
+  :group 'mode-line-faces
+  :group 'basic-faces)
+
 (defface vertical-border
   '((((type tty)) :inherit mode-line-inactive))
   "Face used for vertical window dividers on ttys."
diff --git a/src/dispextern.h b/src/dispextern.h
index cc248a4472e..0d4c0a6c67e 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -1561,6 +1561,34 @@ struct glyph_string
 	 : estimate_mode_line_height					\
 	 (XFRAME ((W)->frame), CURRENT_MODE_LINE_ACTIVE_FACE_ID (W)))))
 
+/* Return the desired face id for the header line of a window, depending
+   on whether the window is selected or not, or if the window is the
+   scrolling window for the currently active minibuffer window.
+
+   Due to the way display_mode_lines manipulates with the contents of
+   selected_window, this macro needs three arguments: SELW which is
+   compared against the current value of selected_window, MBW which is
+   compared against minibuf_window (if SELW doesn't match), and SCRW
+   which is compared against minibuf_selected_window (if MBW matches).  */
+
+#define CURRENT_HEADER_LINE_ACTIVE_FACE_ID_3(SELW, MBW, SCRW)    	\
+     ((!mode_line_in_non_selected_windows			\
+       || (SELW) == XWINDOW (selected_window)			\
+       || (minibuf_level > 0					\
+           && !NILP (minibuf_selected_window)			\
+           && (MBW) == XWINDOW (minibuf_window)			\
+           && (SCRW) == XWINDOW (minibuf_selected_window)))	\
+      ? HEADER_LINE_ACTIVE_FACE_ID				\
+      : HEADER_LINE_INACTIVE_FACE_ID)
+
+
+/* Return the desired face id for the header line of window W.  */
+
+#define CURRENT_HEADER_LINE_ACTIVE_FACE_ID(W)		\
+	 CURRENT_HEADER_LINE_ACTIVE_FACE_ID_3(W, \
+					    XWINDOW (selected_window), \
+					    W)
+
 /* Return the current height of the header line of window W.  If not known
    from W->header_line_height, look at W's current glyph matrix, or return
    an estimation based on the height of the font of the face `header-line'.  */
@@ -1572,7 +1600,7 @@ struct glyph_string
       = (MATRIX_HEADER_LINE_HEIGHT ((W)->current_matrix)	\
 	 ? MATRIX_HEADER_LINE_HEIGHT ((W)->current_matrix)	\
 	 : estimate_mode_line_height				\
-	     (XFRAME ((W)->frame), HEADER_LINE_FACE_ID))))
+	 (XFRAME ((W)->frame), CURRENT_HEADER_LINE_ACTIVE_FACE_ID (W)))))
 
 /* Return the current height of the tab line of window W.  If not known
    from W->tab_line_height, look at W's current glyph matrix, or return
@@ -1893,7 +1921,8 @@ enum face_id
   MODE_LINE_INACTIVE_FACE_ID,
   TOOL_BAR_FACE_ID,
   FRINGE_FACE_ID,
-  HEADER_LINE_FACE_ID,
+  HEADER_LINE_ACTIVE_FACE_ID,
+  HEADER_LINE_INACTIVE_FACE_ID,
   SCROLL_BAR_FACE_ID,
   BORDER_FACE_ID,
   CURSOR_FACE_ID,
diff --git a/src/xdisp.c b/src/xdisp.c
index c74e81a3933..98bc157c789 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -1358,7 +1358,7 @@ window_box_height (struct window *w)
 	  if (hl_row && hl_row->mode_line_p)
 	    height -= hl_row->height;
 	  else
-	    height -= estimate_mode_line_height (f, HEADER_LINE_FACE_ID);
+	    height -= estimate_mode_line_height (f, CURRENT_HEADER_LINE_ACTIVE_FACE_ID (w));
 	}
     }
 
@@ -1753,7 +1753,7 @@ pos_visible_p (struct window *w, ptrdiff_t charpos, int *x, int *y,
 	= window_parameter (w, Qheader_line_format);
 
       w->header_line_height
-	= display_mode_line (w, HEADER_LINE_FACE_ID,
+	= display_mode_line (w, CURRENT_HEADER_LINE_ACTIVE_FACE_ID (w),
 			     NILP (window_header_line_format)
 			     ? BVAR (current_buffer, header_line_format)
 			     : window_header_line_format);
@@ -3197,13 +3197,14 @@ CHECK_WINDOW_END (struct window *w)
 
    BASE_FACE_ID is the id of a base face to use.  It must be one of
    DEFAULT_FACE_ID for normal text, MODE_LINE_ACTIVE_FACE_ID,
-   MODE_LINE_INACTIVE_FACE_ID, or HEADER_LINE_FACE_ID for displaying
-   mode lines, or TOOL_BAR_FACE_ID for displaying the tool-bar.
+   MODE_LINE_INACTIVE_FACE_ID, HEADER_LINE_ACTIVE_FACE_ID, or
+   HEADER_LINE_INACTIVE_FACE_ID for displaying mode lines, or
+   TOOL_BAR_FACE_ID for displaying the tool-bar.
 
    If ROW is null and BASE_FACE_ID is equal to MODE_LINE_ACTIVE_FACE_ID,
-   MODE_LINE_INACTIVE_FACE_ID, or HEADER_LINE_FACE_ID, the iterator
-   will be initialized to use the corresponding mode line glyph row of
-   the desired matrix of W.  */
+   MODE_LINE_INACTIVE_FACE_ID, HEADER_LINE_ACTIVE_FACE_ID, or
+   HEADER_LINE_INACTIVE_FACE_ID the iterator will be initialized to use
+   the corresponding mode line glyph row of the desired matrix of W.  */
 
 void
 init_iterator (struct it *it, struct window *w,
@@ -3251,7 +3252,8 @@ init_iterator (struct it *it, struct window *w,
 	row = MATRIX_MODE_LINE_ROW (w->desired_matrix);
       else if (base_face_id == TAB_LINE_FACE_ID)
 	row = MATRIX_TAB_LINE_ROW (w->desired_matrix);
-      else if (base_face_id == HEADER_LINE_FACE_ID)
+      else if (base_face_id == HEADER_LINE_ACTIVE_FACE_ID
+	       || base_face_id == HEADER_LINE_INACTIVE_FACE_ID)
 	{
 	  /* Header line row depends on whether tab line is enabled.  */
 	  w->desired_matrix->tab_line_p = window_wants_tab_line (w);
@@ -11854,7 +11856,7 @@ window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to,
       Lisp_Object window_header_line_format
 	= window_parameter (w, Qheader_line_format);
 
-      y = y + display_mode_line (w, HEADER_LINE_FACE_ID,
+      y = y + display_mode_line (w, CURRENT_HEADER_LINE_ACTIVE_FACE_ID (w),
 				 NILP (window_header_line_format)
 				 ? BVAR (current_buffer, header_line_format)
 				 : window_header_line_format);
@@ -27453,11 +27455,11 @@ display_mode_lines (struct window *w)
   line_number_displayed = false;
   w->column_number_displayed = -1;
 
+  struct window *sel_w = XWINDOW (old_selected_window);
   if (window_wants_mode_line (w))
     {
       Lisp_Object window_mode_line_format
 	= window_parameter (w, Qmode_line_format);
-      struct window *sel_w = XWINDOW (old_selected_window);
 
       /* Select mode line face based on the real selected window.  */
       display_mode_line (w,
@@ -27485,7 +27487,7 @@ display_mode_lines (struct window *w)
       Lisp_Object window_header_line_format
 	= window_parameter (w, Qheader_line_format);
 
-      display_mode_line (w, HEADER_LINE_FACE_ID,
+      display_mode_line (w, CURRENT_HEADER_LINE_ACTIVE_FACE_ID_3 (sel_w, sel_w, w),
 			 NILP (window_header_line_format)
 			 ? BVAR (current_buffer, header_line_format)
 			 : window_header_line_format);
@@ -27500,11 +27502,12 @@ display_mode_lines (struct window *w)
 }
 
 
-/* Display mode or header/tab line of window W.  FACE_ID specifies
-   which line to display; it is either MODE_LINE_ACTIVE_FACE_ID,
-   HEADER_LINE_FACE_ID or TAB_LINE_FACE_ID.  FORMAT is the
-   mode/header/tab line format to display.  Value is the pixel height
-   of the mode/header/tab line displayed.  */
+/* Display mode or header/tab line of window W.  FACE_ID specifies which
+   line to display; it is either MODE_LINE_ACTIVE_FACE_ID,
+   HEADER_LINE_ACTIVE_FACE_ID, HEADER_LINE_INACTIVE_FACE_ID, or
+   TAB_LINE_FACE_ID.  FORMAT is the mode/header/tab line format to
+   display.  Value is the pixel height of the mode/header/tab line
+   displayed.  */
 
 static int
 display_mode_line (struct window *w, enum face_id face_id, Lisp_Object format)
@@ -27525,7 +27528,8 @@ display_mode_line (struct window *w, enum face_id face_id, Lisp_Object format)
       it.glyph_row->tab_line_p = true;
       w->desired_matrix->tab_line_p = true;
     }
-  else if (face_id == HEADER_LINE_FACE_ID)
+  else if (face_id == HEADER_LINE_ACTIVE_FACE_ID
+	   || face_id == HEADER_LINE_INACTIVE_FACE_ID)
     w->desired_matrix->header_line_p = true;
 
   /* FIXME: This should be controlled by a user option.  But
@@ -27544,7 +27548,9 @@ display_mode_line (struct window *w, enum face_id face_id, Lisp_Object format)
   record_unwind_save_match_data ();
 
   if (NILP (Vmode_line_compact)
-      || face_id == HEADER_LINE_FACE_ID || face_id == TAB_LINE_FACE_ID)
+      || face_id == HEADER_LINE_ACTIVE_FACE_ID
+      || face_id == HEADER_LINE_INACTIVE_FACE_ID
+      || face_id == TAB_LINE_FACE_ID)
     {
       mode_line_target = MODE_LINE_DISPLAY;
       display_mode_element (&it, 0, 0, 0, format, Qnil, false);
@@ -28313,7 +28319,8 @@ are the selected window and the WINDOW's buffer).  */)
 		       ? MODE_LINE_ACTIVE_FACE_ID : MODE_LINE_INACTIVE_FACE_ID)
     : EQ (face, Qmode_line_active) ? MODE_LINE_ACTIVE_FACE_ID
     : EQ (face, Qmode_line_inactive) ? MODE_LINE_INACTIVE_FACE_ID
-    : EQ (face, Qheader_line) ? HEADER_LINE_FACE_ID
+    : EQ (face, Qheader_line_active) ? HEADER_LINE_ACTIVE_FACE_ID
+    : EQ (face, Qheader_line_inactive) ? HEADER_LINE_INACTIVE_FACE_ID
     : EQ (face, Qtab_line) ? TAB_LINE_FACE_ID
     : EQ (face, Qtab_bar) ? TAB_BAR_FACE_ID
     : EQ (face, Qtool_bar) ? TOOL_BAR_FACE_ID
diff --git a/src/xfaces.c b/src/xfaces.c
index e248279e9b7..f6264802fa4 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -5117,7 +5117,8 @@ lookup_basic_face (struct window *w, struct frame *f, int face_id)
     case DEFAULT_FACE_ID:		name = Qdefault;		break;
     case MODE_LINE_ACTIVE_FACE_ID:	name = Qmode_line_active;      	break;
     case MODE_LINE_INACTIVE_FACE_ID:	name = Qmode_line_inactive;	break;
-    case HEADER_LINE_FACE_ID:		name = Qheader_line;		break;
+    case HEADER_LINE_ACTIVE_FACE_ID:	name = Qheader_line_active;	break;
+    case HEADER_LINE_INACTIVE_FACE_ID:	name = Qheader_line_inactive;	break;
     case TAB_LINE_FACE_ID:		name = Qtab_line;		break;
     case TAB_BAR_FACE_ID:		name = Qtab_bar;		break;
     case TOOL_BAR_FACE_ID:		name = Qtool_bar;		break;
@@ -5867,7 +5868,8 @@ realize_basic_faces (struct frame *f)
       realize_named_face (f, Qmode_line_inactive, MODE_LINE_INACTIVE_FACE_ID);
       realize_named_face (f, Qtool_bar, TOOL_BAR_FACE_ID);
       realize_named_face (f, Qfringe, FRINGE_FACE_ID);
-      realize_named_face (f, Qheader_line, HEADER_LINE_FACE_ID);
+      realize_named_face (f, Qheader_line_active, HEADER_LINE_ACTIVE_FACE_ID);
+      realize_named_face (f, Qheader_line_inactive, HEADER_LINE_INACTIVE_FACE_ID);
       realize_named_face (f, Qscroll_bar, SCROLL_BAR_FACE_ID);
       realize_named_face (f, Qborder, BORDER_FACE_ID);
       realize_named_face (f, Qcursor, CURSOR_FACE_ID);
@@ -7438,6 +7440,8 @@ syms_of_xfaces (void)
   DEFSYM (Qfringe, "fringe");
   DEFSYM (Qtab_line, "tab-line");
   DEFSYM (Qheader_line, "header-line");
+  DEFSYM (Qheader_line_inactive, "header-line-inactive");
+  DEFSYM (Qheader_line_active, "header-line-active");
   DEFSYM (Qscroll_bar, "scroll-bar");
   DEFSYM (Qmenu, "menu");
   DEFSYM (Qcursor, "cursor");
-- 
2.47.0

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

[Eli Zaretskii]

> From: Trevor Murphy <trevor.m.murphy@gmail.com>
> Date: Sun, 10 Nov 2024 22:11:30 -0800
> Cc: 73862@debbugs.gnu.org
> 
> Hi!  Thanks for your patience, attached please find the updated patch.  I've added documentation and a
> NEWS entry modeled off of similar commits.  
> 
> When I put together the original patch I included a non-trivial value for the default header-line-inactive
> (copied from mode-line-inactive) as a visual indicator for the new face.  Happy to take it out.  In this updated
> patch, header-line-inactive just inherits from header-line.  I tested by looking at selected and not-selected
> Info buffers with emacs -Q both patched and unpatched, in both graphical and terminal.

Thanks, installed on the master branch, and closing the bug.

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

[Aaron Jensen]

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

I bisected and found that this patch caused a regression in header-line
formatting. I don't have the exact reproduction steps yet, but the gist is
that I use a header-line with multiple faces in one window, then open a new
popup window that has no header-line. The original window's header-line
will get its faces changed slightly, but it's hard to tell how.

Here's a video demonstrating it: https://share.cleanshot.com/CD9PtVv3

You can see that the padding on the left of the header line goes away and
the line numbers change to a variable pitch font. If I adjust my font size,
the header line redraws with the proper faces. This will happen randomly,
but I can force it with the font size change reliably.

I'll see if I can narrow reproduction steps, but it may be worth
considering a revert for now.

Thanks,


Aaron

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

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

[Aaron Jensen]

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

I'm able to reproduce this with the mode-line as well, so it appears that
there may be a bug there too in the code that was copied to implement the
active/inactive faces in the header line. It's entirely possible that
there's a bug in nano-modeline, but it seems suspect that code was added to
consider windows and now this issue triggers when new windows are created
(and possibly when selection changes, I haven't eliminated whether or not
that's a factor yet).


Aaron


On Tue, Dec 03, 2024 at 9:06 PM, Aaron Jensen <aaronjensen@gmail.com> wrote:

> I bisected and found that this patch caused a regression in header-line
> formatting. I don't have the exact reproduction steps yet, but the gist is
> that I use a header-line with multiple faces in one window, then open a new
> popup window that has no header-line. The original window's header-line
> will get its faces changed slightly, but it's hard to tell how.
>
> Here's a video demonstrating it: https://share.cleanshot.com/CD9PtVv3
>
> You can see that the padding on the left of the header line goes away and
> the line numbers change to a variable pitch font. If I adjust my font size,
> the header line redraws with the proper faces. This will happen randomly,
> but I can force it with the font size change reliably.
>
> I'll see if I can narrow reproduction steps, but it may be worth
> considering a revert for now.
>
> Thanks,
>
>
> Aaron
>

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

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

[Eli Zaretskii]

> From: Aaron Jensen <aaronjensen@gmail.com>
> Date: Tue, 3 Dec 2024 22:30:58 -0800

Aaron, it would have been more useful to CC Trevor, who is the author
of that changeset.  I've added him now.

> I'm able to reproduce this with the mode-line as well, so it appears that there may be a bug there too in the
> code that was copied to implement the active/inactive faces in the header line. It's entirely possible that
> there's a bug in nano-modeline, but it seems suspect that code was added to consider windows and now this
> issue triggers when new windows are created (and possibly when selection changes, I haven't eliminated
> whether or not that's a factor yet).

You were able to reproduce what?  I don't think you posted a recipe to
reproduce the problem.  Please do, if at all possible, preferably
starting from "emacs -Q".

> From: Eshel Yaron <me@eshelyaron.com>
> Cc: Trevor Murphy <trevor.m.murphy@gmail.com>
> Date: Wed, 04 Dec 2024 10:47:18 +0100
> 
> >     Add new `header-line-active' and `header-line-inactive' faces
> >     
> >     This is all intended to parallel the 'mode-line-active' and
> >     'mode-line-inactive' distinction.
> [...]
> 
> This seems to introduce a regression, consider the following recipe:
> 
> 1. emacs -Q
> 2. In the scratch buffer, evaluate:
>    (setq header-line-format "foobar")
>    (face-remap-add-relative 'header-line 'highlight)

Aren't you supposed to remap the two new faces instead of
'header-line'?

> 3. Type C-x C-M-= or something similar to force updating the header
>    line.  The header line in the scratch buffer now shows "foobar" and
>    uses the highlight face, as expected
> 4. Type C-x 4 b new RET to switch to another buffer in another window
> 5. In the new buffer evaluate (setq header-line-format "foobar")
> 6. Observe that the header line in the new buffer is also using the
>    highlight face.  That's unexpected!
> 7. Type C-x C-M-= while the new buffer is current
> 8. Observe that the header lines in both windows no longer have the
>    highlight face.  That's unexpected!
> 
> Before, remapping the header-line face with face-remap-add-relative
> would only affect the current buffer, as expected.  Now it seems like
> the face remapping "leaks" between buffers/windows somehow...

Trevor, could you please look into this?