bug#28033: [PATCH] Add new face 'header-line-highlight'

bug#28033: [PATCH] Add new face 'header-line-highlight'

[Alex]

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

Some header-line configurations don't interact nicely with the
'highlight' face, particularly when they use the :box attribute.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: header-line-highlight --]
[-- Type: text/x-diff, Size: 3675 bytes --]

From 6ac60c53180c87d1b5c5dd2ebdfda259938a8e32 Mon Sep 17 00:00:00 2001
From: Alexander Gramiak <agrambot@gmail.com>
Date: Wed, 9 Aug 2017 17:14:06 -0600
Subject: [PATCH] Add new face 'header-line-highlight'

* doc/emacs/display.texi (Standard Faces):
* etc/NEWS: Document the face.
* lisp/emacs-lisp/tabulated-list.el (tabulated-list-init-header):
* lisp/info.el (Info-fontify-node): Use the face.
* lisp/faces.el: Define the face.
---
 doc/emacs/display.texi            | 4 ++++
 etc/NEWS                          | 5 +++++
 lisp/emacs-lisp/tabulated-list.el | 2 +-
 lisp/faces.el                     | 5 +++++
 lisp/info.el                      | 2 +-
 5 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/doc/emacs/display.texi b/doc/emacs/display.texi
index 083fcdf97a..440aab1055 100644
--- a/doc/emacs/display.texi
+++ b/doc/emacs/display.texi
@@ -711,6 +711,10 @@ Standard Faces
 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.
+@item header-line-highlight
+@cindex header-line-highlight face
+Similar to @code{highlight} and @code{mode-line-highlight}, but used
+for mouse-sensitive portions of text on header lines.
 @item vertical-border
 @cindex vertical-border face
 This face is used for the vertical divider between windows on text
diff --git a/etc/NEWS b/etc/NEWS
index 2b789be3c8..dfb7ca3fca 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -254,6 +254,11 @@ face instead of the 'escape-glyph' face.
 ** Approximations to quotes are now displayed with the new 'homoglyph'
 face instead of the 'escape-glyph' face.
 
++++
+** The new face 'header-line-highlight' has been introduced as the
+header line analogue of 'mode-line-highlight'.  This should be the
+preferred mouse-face for mouse-sensitive elements in the header line.
+
 ---
 ** 'C-x h' ('mark-whole-buffer') will now avoid marking the prompt
 part of minibuffers.
diff --git a/lisp/emacs-lisp/tabulated-list.el b/lisp/emacs-lisp/tabulated-list.el
index 8ff5cdf18e..b91532f7e8 100644
--- a/lisp/emacs-lisp/tabulated-list.el
+++ b/lisp/emacs-lisp/tabulated-list.el
@@ -191,7 +191,7 @@ tabulated-list-init-header
   ;; FIXME: Should share code with tabulated-list-print-col!
   (let ((x (max tabulated-list-padding 0))
 	(button-props `(help-echo "Click to sort by column"
-			mouse-face highlight
+			mouse-face header-line-highlight
 			keymap ,tabulated-list-sort-button-map))
 	(cols nil))
     (if display-line-numbers
diff --git a/lisp/faces.el b/lisp/faces.el
index 5ed11d11ce..01d94d7aae 100644
--- a/lisp/faces.el
+++ b/lisp/faces.el
@@ -2628,6 +2628,11 @@ header-line
   :version "21.1"
   :group 'basic-faces)
 
+(defface header-line-highlight '((t :inherit highlight))
+  "Basic header line face for highlighting."
+  :version "26.1"
+  :group 'basic-faces)
+
 (defface vertical-border
   '((((type tty)) :inherit mode-line-inactive))
   "Face used for vertical window dividers on ttys."
diff --git a/lisp/info.el b/lisp/info.el
index c7f0bbf08d..45a9116e06 100644
--- a/lisp/info.el
+++ b/lisp/info.el
@@ -4654,7 +4654,7 @@ Info-fontify-node
             (if (string-equal (downcase tag) "node")
                 (put-text-property nbeg nend 'font-lock-face 'info-header-node)
               (put-text-property nbeg nend 'font-lock-face 'info-header-xref)
-              (put-text-property tbeg nend 'mouse-face 'highlight)
+              (put-text-property tbeg nend 'mouse-face 'header-line-highlight)
               (put-text-property tbeg nend
                                  'help-echo
                                  (concat "mouse-2: Go to node "
-- 
2.13.2

bug#28033: [PATCH] Add new face 'header-line-highlight'

[Eli Zaretskii]

> From: Alex <agrambot@gmail.com>
> Date: Wed, 09 Aug 2017 17:29:19 -0600
> 
> Some header-line configurations don't interact nicely with the
> 'highlight' face, particularly when they use the :box attribute.

Can you elaborate about the problem and why introducing a new face is
the solution for it?

Thanks.

bug#28033: [PATCH] Add new face 'header-line-highlight'

[Alex]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Alex <agrambot@gmail.com>
>> Date: Wed, 09 Aug 2017 17:29:19 -0600
>> 
>> Some header-line configurations don't interact nicely with the
>> 'highlight' face, particularly when they use the :box attribute.
>
> Can you elaborate about the problem and why introducing a new face is
> the solution for it?
>
> Thanks.

The problem is that the highlight face, which is used for highlighting
links and other mouse-sensitive buffer text frequently, may not always
look nice with the header-line face. I've attached a picture
demonstrating one possible issue. In it, the header-line face has a :box
attribute with line-width 4, but the highlight face does not. This
results in the characters at the ends being clipped.

A new face allows for users to customize it to match the customization
of the header-line face while not affecting all other uses of the
highlight face.

I believe the use case of this face is very similar to
'mode-line-highlight', which was added quite a while ago.


[-- Attachment #2: Screenshot_2017-08-09_23-20-30.png --]
[-- Type: image/png, Size: 10388 bytes --]

bug#28033: [PATCH] Add new face 'header-line-highlight'

[Eli Zaretskii]

> From: Alex <agrambot@gmail.com>
> Cc: 28033@debbugs.gnu.org
> Date: Wed, 09 Aug 2017 23:36:19 -0600
> 
> The problem is that the highlight face, which is used for highlighting
> links and other mouse-sensitive buffer text frequently, may not always
> look nice with the header-line face. I've attached a picture
> demonstrating one possible issue. In it, the header-line face has a :box
> attribute with line-width 4, but the highlight face does not. This
> results in the characters at the ends being clipped.
> 
> A new face allows for users to customize it to match the customization
> of the header-line face while not affecting all other uses of the
> highlight face.
> 
> I believe the use case of this face is very similar to
> 'mode-line-highlight', which was added quite a while ago.

Thanks, this makes sense.  But please add some of this rationale to
the documentation.

Also, it is preferable to have the first line of a NEWS item be a full
sentence, if possible.  In this case, I would just say

  ** New face 'header-line-highlight'.

and then follow that by the details.

bug#28033: [PATCH] Add new face 'header-line-highlight'

[Alex]

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

Eli Zaretskii <eliz@gnu.org> writes:

> Thanks, this makes sense.  But please add some of this rationale to
> the documentation.

I'm not sure exactly what you're looking for, but I added a brief
explanation to the doc.

> Also, it is preferable to have the first line of a NEWS item be a full
> sentence, if possible.  In this case, I would just say
>
>   ** New face 'header-line-highlight'.
>
> and then follow that by the details.

Sure. It seems that a lot of nearby entries don't follow that style,
though...


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: header-line 2 --]
[-- Type: text/x-diff, Size: 3759 bytes --]

From d0ed559373afa9c1922cb40eed156d68e5c1f344 Mon Sep 17 00:00:00 2001
From: Alexander Gramiak <agrambot@gmail.com>
Date: Wed, 9 Aug 2017 17:14:06 -0600
Subject: [PATCH] Add new face 'header-line-highlight'

* doc/emacs/display.texi (Standard Faces):
* etc/NEWS: Document the face.
* lisp/emacs-lisp/tabulated-list.el (tabulated-list-init-header):
* lisp/info.el (Info-fontify-node): Use the face.
* lisp/faces.el: Define the face.
---
 doc/emacs/display.texi            | 6 ++++++
 etc/NEWS                          | 6 ++++++
 lisp/emacs-lisp/tabulated-list.el | 2 +-
 lisp/faces.el                     | 5 +++++
 lisp/info.el                      | 2 +-
 5 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/doc/emacs/display.texi b/doc/emacs/display.texi
index 083fcdf97a..832629c733 100644
--- a/doc/emacs/display.texi
+++ b/doc/emacs/display.texi
@@ -711,6 +711,12 @@ Standard Faces
 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.
+@item header-line-highlight
+@cindex header-line-highlight face
+Similar to @code{highlight} and @code{mode-line-highlight}, but used
+for mouse-sensitive portions of text on header lines.  This face is
+useful for when @code{header-line} doesn't interact well with
+@code{highlight}.
 @item vertical-border
 @cindex vertical-border face
 This face is used for the vertical divider between windows on text
diff --git a/etc/NEWS b/etc/NEWS
index 2b789be3c8..50499f2f09 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -254,6 +254,12 @@ face instead of the 'escape-glyph' face.
 ** Approximations to quotes are now displayed with the new 'homoglyph'
 face instead of the 'escape-glyph' face.
 
++++
+** New face 'header-line-highlight'.
+This face is the header line analogue of 'mode-line-highlight'; it
+should be the preferred mouse-face for mouse-sensitive elements in the
+header line.
+
 ---
 ** 'C-x h' ('mark-whole-buffer') will now avoid marking the prompt
 part of minibuffers.
diff --git a/lisp/emacs-lisp/tabulated-list.el b/lisp/emacs-lisp/tabulated-list.el
index 8ff5cdf18e..b91532f7e8 100644
--- a/lisp/emacs-lisp/tabulated-list.el
+++ b/lisp/emacs-lisp/tabulated-list.el
@@ -191,7 +191,7 @@ tabulated-list-init-header
   ;; FIXME: Should share code with tabulated-list-print-col!
   (let ((x (max tabulated-list-padding 0))
 	(button-props `(help-echo "Click to sort by column"
-			mouse-face highlight
+			mouse-face header-line-highlight
 			keymap ,tabulated-list-sort-button-map))
 	(cols nil))
     (if display-line-numbers
diff --git a/lisp/faces.el b/lisp/faces.el
index 5ed11d11ce..01d94d7aae 100644
--- a/lisp/faces.el
+++ b/lisp/faces.el
@@ -2628,6 +2628,11 @@ header-line
   :version "21.1"
   :group 'basic-faces)
 
+(defface header-line-highlight '((t :inherit highlight))
+  "Basic header line face for highlighting."
+  :version "26.1"
+  :group 'basic-faces)
+
 (defface vertical-border
   '((((type tty)) :inherit mode-line-inactive))
   "Face used for vertical window dividers on ttys."
diff --git a/lisp/info.el b/lisp/info.el
index c7f0bbf08d..45a9116e06 100644
--- a/lisp/info.el
+++ b/lisp/info.el
@@ -4654,7 +4654,7 @@ Info-fontify-node
             (if (string-equal (downcase tag) "node")
                 (put-text-property nbeg nend 'font-lock-face 'info-header-node)
               (put-text-property nbeg nend 'font-lock-face 'info-header-xref)
-              (put-text-property tbeg nend 'mouse-face 'highlight)
+              (put-text-property tbeg nend 'mouse-face 'header-line-highlight)
               (put-text-property tbeg nend
                                  'help-echo
                                  (concat "mouse-2: Go to node "
-- 
2.13.2

bug#28033: [PATCH] Add new face 'header-line-highlight'

[Eli Zaretskii]

> From: Alex <agrambot@gmail.com>
> Cc: 28033@debbugs.gnu.org
> Date: Fri, 11 Aug 2017 16:10:02 -0600
> 
> > Thanks, this makes sense.  But please add some of this rationale to
> > the documentation.
> 
> I'm not sure exactly what you're looking for, but I added a brief
> explanation to the doc.

That's what I was looking for (although I made minor wording changes
in the actual commit).

> > Also, it is preferable to have the first line of a NEWS item be a full
> > sentence, if possible.  In this case, I would just say
> >
> >   ** New face 'header-line-highlight'.
> >
> > and then follow that by the details.
> 
> Sure. It seems that a lot of nearby entries don't follow that style,
> though...

Yes, something to work on.  Patches welcome.

I've pushed the changes.  A couple of minor comments for the future:

 . Please mention the bug number in the log message.
 . The order of the references to various parts of the changes in the
   log message should assume the reading order of top to bottom, so
   the log message might need some minor reordering.  In this case,
   your original order:

> * doc/emacs/display.texi (Standard Faces):
> * etc/NEWS: Document the face.
> * lisp/emacs-lisp/tabulated-list.el (tabulated-list-init-header):
> * lisp/info.el (Info-fontify-node): Use the face.
> * lisp/faces.el: Define the face.

   refers to "the face" before it was defined.  I've reordered it to
   put the reference to the lisp/faces.el change before all the rest.

Thanks again for working on this.

bug#28033: [PATCH] Add new face 'header-line-highlight'

[Alex]

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

Eli Zaretskii <eliz@gnu.org> writes:

>  . The order of the references to various parts of the changes in the
>    log message should assume the reading order of top to bottom, so
>    the log message might need some minor reordering.  In this case,
>    your original order:

I was going for a mostly alphabetical ordering. I take it that doesn't
matter?

>> * doc/emacs/display.texi (Standard Faces):
>> * etc/NEWS: Document the face.
>> * lisp/emacs-lisp/tabulated-list.el (tabulated-list-init-header):
>> * lisp/info.el (Info-fontify-node): Use the face.
>> * lisp/faces.el: Define the face.
>
>    refers to "the face" before it was defined.  I've reordered it to
>    put the reference to the lisp/faces.el change before all the rest.

I don't really understand this part (though I don't mind following it).
Only the commit summary line references the face by name, and the
summary is already at the top. Why does it matter that, in the actual
program execution, the face has to be defined first?

P.S. I happened upon two more places to add this face to. Would you
please push this as well?

Thanks.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Use-header-line-highlight-in-proced-and-erc.patch --]
[-- Type: text/x-diff, Size: 1548 bytes --]

From 4aaff22bf46ad72922cd72a01eb841b60fb2eace Mon Sep 17 00:00:00 2001
From: Alexander Gramiak <agrambot@gmail.com>
Date: Sat, 12 Aug 2017 20:45:33 -0600
Subject: [PATCH] Use 'header-line-highlight' in proced and erc

* lisp/erc/erc-list.el (erc-list-button):
* lisp/proced.el (proced-format): Use the face.
---
 lisp/erc/erc-list.el | 2 +-
 lisp/proced.el       | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lisp/erc/erc-list.el b/lisp/erc/erc-list.el
index 5110239f61..7d6413ee7f 100644
--- a/lisp/erc/erc-list.el
+++ b/lisp/erc/erc-list.el
@@ -145,7 +145,7 @@ erc-list-button
   (erc-propertize title
 		  'column-number column
 		  'help-echo "mouse-1: sort by column"
-		  'mouse-face 'highlight
+		  'mouse-face 'header-line-highlight
 		  'keymap erc-list-menu-sort-button-map))
 
 (define-derived-mode erc-list-menu-mode special-mode "ERC-List"
diff --git a/lisp/proced.el b/lisp/proced.el
index be3b7c41a6..18693f4556 100644
--- a/lisp/proced.el
+++ b/lisp/proced.el
@@ -1438,7 +1438,7 @@ proced-format
              (hprops
               (if (nth 4 grammar)
                   (let ((descend (if (eq key sort-key) proced-descend (nth 5 grammar))))
-                    `(proced-key ,key mouse-face highlight
+                    `(proced-key ,key mouse-face header-line-highlight
                                  help-echo ,(format proced-header-help-echo
                                                     (if descend "-" "+")
                                                     (nth 1 grammar)
-- 
2.13.2

bug#28033: [PATCH] Add new face 'header-line-highlight'

[Eli Zaretskii]

> From: Alex <agrambot@gmail.com>
> Cc: 28033-done@debbugs.gnu.org
> Date: Sat, 12 Aug 2017 21:05:42 -0600
> 
> >  . The order of the references to various parts of the changes in the
> >    log message should assume the reading order of top to bottom, so
> >    the log message might need some minor reordering.  In this case,
> >    your original order:
> 
> I was going for a mostly alphabetical ordering. I take it that doesn't
> matter?
> 
> >> * doc/emacs/display.texi (Standard Faces):
> >> * etc/NEWS: Document the face.
> >> * lisp/emacs-lisp/tabulated-list.el (tabulated-list-init-header):
> >> * lisp/info.el (Info-fontify-node): Use the face.
> >> * lisp/faces.el: Define the face.
> >
> >    refers to "the face" before it was defined.  I've reordered it to
> >    put the reference to the lisp/faces.el change before all the rest.
> 
> I don't really understand this part (though I don't mind following it).
> Only the commit summary line references the face by name, and the
> summary is already at the top. Why does it matter that, in the actual
> program execution, the face has to be defined first?

Well, the logical order is: first you introduce the face, then you
use it, then you document it.  So it'd be nice to have the log message
read this way, top to bottom.  In your case, it was in the reverse
order, probably because "C-x 4 a" puts the entries in LIFO order.

Admittedly, this is a very minor aesthetic issue.

> P.S. I happened upon two more places to add this face to. Would you
> please push this as well?

Will do, thanks.

bug#28033: [PATCH] Add new face 'header-line-highlight'

[Eli Zaretskii]

> Date: Sun, 13 Aug 2017 17:27:04 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 28033@debbugs.gnu.org
> 
> > P.S. I happened upon two more places to add this face to. Would you
> > please push this as well?
> 
> Will do, thanks.

Done.

Once again, please always mention the bug number on the log message.