bug#49162: 28.0.50; [PATCH] Use named face for apropos-face-button

bug#49162: 28.0.50; [PATCH] Use named face for apropos-face-button

[Protesilaos Stavrou]

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

Dear maintainers,

The attached patch defines a new face for the Apropos buffer which
applies to buttons that indicate faces.

Before the attributes were hardcoded and could thus not be edited by the
user/theme.

All the best,
Protesilaos

-- 
Protesilaos Stavrou
https://protesilaos.com

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Use-named-face-for-apropos-face-button.patch --]
[-- Type: text/x-patch, Size: 1933 bytes --]

From fed7e750cb8a6b0057c8c036c6113335f3ebeb44 Mon Sep 17 00:00:00 2001
Message-Id: <fed7e750cb8a6b0057c8c036c6113335f3ebeb44.1624306326.git.info@protesilaos.com>
From: Protesilaos Stavrou <info@protesilaos.com>
Date: Mon, 21 Jun 2021 23:11:35 +0300
Subject: [PATCH] Use named face for apropos-face-button

* etc/NEWS: Document new face.
* lisp/apropos.el (apropos-face): Define new face.
(apropos-face-button): Specify face instead of hardcoding attributes.
---
 etc/NEWS        | 3 +++
 lisp/apropos.el | 7 ++++++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/etc/NEWS b/etc/NEWS
index 3c7d5ca10b..4cbd87b917 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1606,6 +1606,9 @@ These new navigation commands are bound to 'n' and 'p' in
 *** New command 'apropos-function'.
 This works like 'C-u M-x apropos-command' but is more discoverable.
 
+*** New face 'apropos-face-button'.
+Applies to buttons that indicate a face.
+
 ** CC Mode
 
 *** Added support for Doxygen documentation style.
diff --git a/lisp/apropos.el b/lisp/apropos.el
index 17665a725a..f24448599f 100644
--- a/lisp/apropos.el
+++ b/lisp/apropos.el
@@ -96,6 +96,11 @@ (defface apropos-property
   "Face for property name in Apropos output, or nil for none."
   :version "24.3")
 
+(defface apropos-face-button
+  '((t (:inherit (font-lock-variable-name-face button))))
+  "Face for buttons that indicate a face in Apropos."
+  :version "28.1")
+
 (defface apropos-function-button
   '((t (:inherit (font-lock-function-name-face button))))
   "Button face indicating a function, macro, or command in Apropos."
@@ -276,7 +281,7 @@ (define-button-type 'apropos-user-option
 (define-button-type 'apropos-face
   'apropos-label "Face"
   'apropos-short-label "F"
-  'face '(font-lock-variable-name-face button)
+  'face 'apropos-face-button
   'help-echo "mouse-2, RET: Display more help on this face"
   'follow-link t
   'action (lambda (button)
-- 
2.32.0

bug#49162: 28.0.50; [PATCH] Use named face for apropos-face-button

[Eli Zaretskii]

> From: Protesilaos Stavrou <info@protesilaos.com>
> Date: Mon, 21 Jun 2021 23:17:32 +0300
> 
> +*** New face 'apropos-face-button'.

The name should be apropos-button-face, IMO.

Thanks.

bug#49162: 28.0.50; [PATCH] Use named face for apropos-face-button

[martin rudalics]

 >> +*** New face 'apropos-face-button'.
 >
 > The name should be apropos-button-face, IMO.

`apropos-button' IMO.

martin

bug#49162: 28.0.50; [PATCH] Use named face for apropos-face-button

[Protesilaos Stavrou]

On 2021-06-22, 14:02 +0200, martin rudalics <rudalics@gmx.at> wrote:

>  >> +*** New face 'apropos-face-button'.
>  >
>  > The name should be apropos-button-face, IMO.
>
> `apropos-button' IMO.

Thank you Martin and Eli!  I picked that symbol because it was already
given by the implicit face.

You can try it with emacs -Q:

- M-x apropos RET font-lock-variable RET
- Place point over the underlined "Face" text and M-x describe-char
- In the resulting Help buffer, you will find the reference to
  'apropos-face-button', whose specification is defined thus:

    Category apropos-face-button:

      action               [Show]
      apropos-label        "Face"
      apropos-short-label  "F"
      evaporate            t
      face                 (font-lock-variable-name-face button)
      follow-link          t
      help-echo            "mouse-2, RET: Display more help on this face"
      keymap               [Show]
      mouse-face           highlight
      rear-nonsticky       t
      supertype            button
      type                 apropos-face


What do you think?

-- 
Protesilaos Stavrou
https://protesilaos.com

bug#49162: 28.0.50; [PATCH] Use named face for apropos-face-button

[Andreas Schwab]

On Jun 22 2021, Protesilaos Stavrou wrote:

> On 2021-06-22, 14:02 +0200, martin rudalics <rudalics@gmx.at> wrote:
>
>>  >> +*** New face 'apropos-face-button'.
>>  >
>>  > The name should be apropos-button-face, IMO.
>>
>> `apropos-button' IMO.
>
> Thank you Martin and Eli!  I picked that symbol because it was already
> given by the implicit face.
>
> You can try it with emacs -Q:
>
> - M-x apropos RET font-lock-variable RET
> - Place point over the underlined "Face" text and M-x describe-char
> - In the resulting Help buffer, you will find the reference to
>   'apropos-face-button', whose specification is defined thus:
>
>     Category apropos-face-button:
>
>       action               [Show]
>       apropos-label        "Face"
>       apropos-short-label  "F"
>       evaporate            t
>       face                 (font-lock-variable-name-face button)
>       follow-link          t
>       help-echo            "mouse-2, RET: Display more help on this face"
>       keymap               [Show]
>       mouse-face           highlight
>       rear-nonsticky       t
>       supertype            button
>       type                 apropos-face
>
>
> What do you think?

It's the face for the apropos-face-button, so the name looks right.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

bug#49162: 28.0.50; [PATCH] Use named face for apropos-face-button

[Eli Zaretskii]

> From: Protesilaos Stavrou <info@protesilaos.com>
> Cc: 49162@debbugs.gnu.org
> Date: Tue, 22 Jun 2021 15:11:33 +0300
> 
> Thank you Martin and Eli!  I picked that symbol because it was already
> given by the implicit face.
> 
> You can try it with emacs -Q:
> 
> - M-x apropos RET font-lock-variable RET
> - Place point over the underlined "Face" text and M-x describe-char
> - In the resulting Help buffer, you will find the reference to
>   'apropos-face-button', whose specification is defined thus:
> 
>     Category apropos-face-button:

That's the _button_, not its _face_.  It is cumbersome to call a face
by a name that ends with "-button", IMO.

bug#49162: 28.0.50; [PATCH] Use named face for apropos-face-button

[Protesilaos Stavrou]

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

On 2021-06-22, 15:37 +0300, Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Protesilaos Stavrou <info@protesilaos.com>
>> Cc: 49162@debbugs.gnu.org
>> Date: Tue, 22 Jun 2021 15:11:33 +0300
>> 
>> Thank you Martin and Eli!  I picked that symbol because it was already
>> given by the implicit face.
>> 
>> You can try it with emacs -Q:
>> 
>> - M-x apropos RET font-lock-variable RET
>> - Place point over the underlined "Face" text and M-x describe-char
>> - In the resulting Help buffer, you will find the reference to
>>   'apropos-face-button', whose specification is defined thus:
>> 
>>     Category apropos-face-button:
>
> That's the _button_, not its _face_.  It is cumbersome to call a face
> by a name that ends with "-button", IMO.

I see.  Thank you!  Please find attached the revised patch for
'apropos-button-face'

-- 
Protesilaos Stavrou
https://protesilaos.com

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Use-named-face-for-apropos-button-face.patch --]
[-- Type: text/x-patch, Size: 1933 bytes --]

From 8183d254688f2f37404c0669e0a7636dd4c18c7e Mon Sep 17 00:00:00 2001
Message-Id: <8183d254688f2f37404c0669e0a7636dd4c18c7e.1624367239.git.info@protesilaos.com>
From: Protesilaos Stavrou <info@protesilaos.com>
Date: Mon, 21 Jun 2021 23:11:35 +0300
Subject: [PATCH] Use named face for apropos-button-face

* etc/NEWS: Document new face.
* lisp/apropos.el (apropos-face): Define new face.
(apropos-button-face): Specify face instead of hardcoding attributes.
---
 etc/NEWS        | 3 +++
 lisp/apropos.el | 7 ++++++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/etc/NEWS b/etc/NEWS
index 3c7d5ca10b..1bf8801566 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1606,6 +1606,9 @@ These new navigation commands are bound to 'n' and 'p' in
 *** New command 'apropos-function'.
 This works like 'C-u M-x apropos-command' but is more discoverable.
 
+*** New face 'apropos-button-face'.
+Applies to buttons that indicate a face.
+
 ** CC Mode
 
 *** Added support for Doxygen documentation style.
diff --git a/lisp/apropos.el b/lisp/apropos.el
index 17665a725a..c6c86f6db0 100644
--- a/lisp/apropos.el
+++ b/lisp/apropos.el
@@ -96,6 +96,11 @@ (defface apropos-property
   "Face for property name in Apropos output, or nil for none."
   :version "24.3")
 
+(defface apropos-button-face
+  '((t (:inherit (font-lock-variable-name-face button))))
+  "Face for buttons that indicate a face in Apropos."
+  :version "28.1")
+
 (defface apropos-function-button
   '((t (:inherit (font-lock-function-name-face button))))
   "Button face indicating a function, macro, or command in Apropos."
@@ -276,7 +281,7 @@ (define-button-type 'apropos-user-option
 (define-button-type 'apropos-face
   'apropos-label "Face"
   'apropos-short-label "F"
-  'face '(font-lock-variable-name-face button)
+  'face 'apropos-button-face
   'help-echo "mouse-2, RET: Display more help on this face"
   'follow-link t
   'action (lambda (button)
-- 
2.32.0

bug#49162: 28.0.50; [PATCH] Use named face for apropos-face-button

[Lars Ingebrigtsen]

Protesilaos Stavrou <info@protesilaos.com> writes:

> I see.  Thank you!  Please find attached the revised patch for
> 'apropos-button-face'

Thanks; applied to Emacs 28 (but I changed the name of the face to
`apropos-button' -- we don't call new faces `-face' these days).

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#49162: 28.0.50; [PATCH] Use named face for apropos-face-button

[Andreas Schwab]

On Jun 22 2021, Eli Zaretskii wrote:

>> From: Protesilaos Stavrou <info@protesilaos.com>
>> Cc: 49162@debbugs.gnu.org
>> Date: Tue, 22 Jun 2021 15:11:33 +0300
>> 
>> Thank you Martin and Eli!  I picked that symbol because it was already
>> given by the implicit face.
>> 
>> You can try it with emacs -Q:
>> 
>> - M-x apropos RET font-lock-variable RET
>> - Place point over the underlined "Face" text and M-x describe-char
>> - In the resulting Help buffer, you will find the reference to
>>   'apropos-face-button', whose specification is defined thus:
>> 
>>     Category apropos-face-button:
>
> That's the _button_, not its _face_.

But the new face is the face for the button.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

bug#49162: 28.0.50; [PATCH] Use named face for apropos-face-button

[Eli Zaretskii]

> From: Andreas Schwab <schwab@linux-m68k.org>
> Cc: Protesilaos Stavrou <info@protesilaos.com>,  49162@debbugs.gnu.org
> Date: Tue, 22 Jun 2021 16:00:06 +0200
> 
> >>     Category apropos-face-button:
> >
> > That's the _button_, not its _face_.
> 
> But the new face is the face for the button.

I don't have anything about naming the face apropos-button.  It's
apropos-face-button that I don't like: it implies that it's a button
of a face, or something that is apropos button-face; both are wrong.

bug#49162: 28.0.50; [PATCH] Use named face for apropos-face-button

[Andreas Schwab]

On Jun 22 2021, Eli Zaretskii wrote:

>> From: Andreas Schwab <schwab@linux-m68k.org>
>> Cc: Protesilaos Stavrou <info@protesilaos.com>,  49162@debbugs.gnu.org
>> Date: Tue, 22 Jun 2021 16:00:06 +0200
>> 
>> >>     Category apropos-face-button:
>> >
>> > That's the _button_, not its _face_.
>> 
>> But the new face is the face for the button.
>
> I don't have anything about naming the face apropos-button.  It's
> apropos-face-button that I don't like: it implies that it's a button
> of a face, or something that is apropos button-face; both are wrong.

But that happens to be the name of the button that the face is for, just
like the apropos-variable-button face is the face for the
apropos-variable-button button.  Since face names have no qualifiers,
such duplication is unavoidable.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

bug#49162: 28.0.50; [PATCH] Use named face for apropos-face-button

[Eli Zaretskii]

> From: Andreas Schwab <schwab@linux-m68k.org>
> Cc: info@protesilaos.com,  49162@debbugs.gnu.org
> Date: Tue, 22 Jun 2021 17:44:25 +0200
> 
> > I don't have anything about naming the face apropos-button.  It's
> > apropos-face-button that I don't like: it implies that it's a button
> > of a face, or something that is apropos button-face; both are wrong.
> 
> But that happens to be the name of the button that the face is for, just
> like the apropos-variable-button face is the face for the
> apropos-variable-button button.

And they are all unfortunate, IMO.  We don't need to keep some kind of
consistency here, I think.