unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / Atom feed
* Proposal for an improved `help-for-help'
@ 2021-02-21 12:06 Stefan Kangas
  2021-02-21 16:46 ` [External] : " Drew Adams
                   ` (4 more replies)
  0 siblings, 5 replies; 48+ messages in thread
From: Stefan Kangas @ 2021-02-21 12:06 UTC (permalink / raw)
  To: emacs-devel

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

One of the usability complaints I've heard is that `help-for-help'
(`C-h C-h') could be better organized.  I also think some colors could
really help in making it easier to navigate.

I have attached a rough draft for what an improved version of might look
like.  It would be great to get some feedback.

I have also attached a patch, but it's very rough.  I'm specifically
having some trouble with font-locking: It works if I copy the contents
of the " *Metahelp*" buffer and invoke the mode manually, but it will
not work when I say `C-h C-h'.  Help with this is also welcome -- I'm
probably missing something obvious.

[-- Attachment #2: help-for-help-new.png --]
[-- Type: image/png, Size: 229054 bytes --]

[-- Attachment #3: 0001-Help-for-help-mode.patch --]
[-- Type: text/x-diff, Size: 7844 bytes --]

From 25604343235028d606bdd0b26bc0eae6a76e281c Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefan@marxist.se>
Date: Sat, 20 Feb 2021 17:07:23 +0100
Subject: [PATCH] Help for help mode

---
 lisp/help-macro.el | 13 +++++++---
 lisp/help-mode.el  | 36 +++++++++++++++++++++++++++
 lisp/help.el       | 61 +++++++++++++++++++++++++++++-----------------
 3 files changed, 84 insertions(+), 26 deletions(-)

diff --git a/lisp/help-macro.el b/lisp/help-macro.el
index 791b10a878..6d4d901acf 100644
--- a/lisp/help-macro.el
+++ b/lisp/help-macro.el
@@ -83,15 +83,21 @@ three-step-help
   :type 'boolean
   :group 'help)
 
-(defmacro make-help-screen (fname help-line help-text helped-map)
+(defmacro make-help-screen (fname help-line help-text helped-map &optional mode)
   "Construct help-menu function name FNAME.
 When invoked, FNAME shows HELP-LINE and reads a command using HELPED-MAP.
+
 If the command is the help character, FNAME displays HELP-TEXT
 and continues trying to read a command using HELPED-MAP.
+
 If HELP-TEXT contains the sequence `%THIS-KEY%', that is replaced
 with the key sequence that invoked FNAME.
+
 When FNAME finally does get a command, it executes that command
-and then returns."
+and then returns.
+
+If optional argument MODE is non-nil, use that mode instead of
+`help-mode'."
   (let ((doc-fn (intern (concat (symbol-name fname) "-doc"))))
     `(progn
        (defun ,doc-fn () ,help-text nil)
@@ -145,7 +151,8 @@ make-help-screen
 		       (erase-buffer)
 		       (insert help-screen))
 		     (let ((minor-mode-map-alist new-minor-mode-map-alist))
-		       (help-mode)
+                       ,(or (and mode `(,mode))
+                            '(help-mode))
 		       (setq new-minor-mode-map-alist minor-mode-map-alist))
 		     (goto-char (point-min))
 		     (while (or (memq char (append help-event-list
diff --git a/lisp/help-mode.el b/lisp/help-mode.el
index 30a1ce053c..7dd9317aea 100644
--- a/lisp/help-mode.el
+++ b/lisp/help-mode.el
@@ -788,6 +788,42 @@ help-insert-string
   (with-output-to-temp-buffer (help-buffer)
     (insert string)))
 
+\f
+;; `help-for-help' syntax highlighting
+
+(defface help-for-help-header '((t :weight bold :foreground "darkblue"
+                               :height 1.2))
+  "Face used for headers in the `help-for-help' buffer."
+  :group 'help)
+
+(defface help-for-help-binding '((t :weight bold :foreground "SpringGreen4"))
+  "Face used for headers in the `help-for-help' buffer."
+  :group 'help)
+
+(defvar help-for-help-mode-font-lock-keywords
+  `((,(rx bol (or "Basic Help"
+                  "Documentation"
+                  "Searching and packages"
+                  "Other")
+          eol)
+     0 'help-for-help-header)
+    (,(rx bol (? (any "MCs") "-") (any letter ".")
+      " "
+      (? (one-or-more (or alnum "-"))))
+     0 'help-for-help-binding)
+    (,(rx word-start (group (or "C-h" "F1" "SPC" "DEL")) word-end)
+     (1 'help-for-help-binding))
+    (,(rx (seq "Type " (group "q") " to")) (1 'help-for-help-binding)))
+  "Font lock keywords for `help-for-help-mode'.")
+
+;;;###autoload
+(define-derived-mode help-for-help-mode help-mode "Help X"
+  "Major mode used by `help-for-help'."
+  :interactive t
+  (setq-local font-lock-defaults `(,help-for-help-mode-font-lock-keywords t))
+  ;; FIXME: Why is this needed?
+  (font-lock-fontify-buffer))
+
 \f
 ;; Bookmark support
 
diff --git a/lisp/help.el b/lisp/help.el
index 084e941549..e842c600aa 100644
--- a/lisp/help.el
+++ b/lisp/help.el
@@ -199,39 +199,53 @@ 'help-for-help
   "You have typed %THIS-KEY%, the help character.  Type a Help option:
 \(Use SPC or DEL to scroll through this text.  Type \\<help-map>\\[help-quit] to exit the Help command.)
 
-a PATTERN   Show commands whose name matches the PATTERN (a list of words
-              or a regexp).  See also the `apropos' command.
+Basic Help
+
+m           Display documentation of current minor modes and current major mode,
+              including their special commands.
 b           Display all key bindings.
+x COMMAND   Display documentation for the given command.
+k KEYS      Display the full documentation for the key sequence.
+
+f FUNCTION  Display documentation for the given function.
+o SYMBOL    Display the given function or variable's documentation and value.
+v VARIABLE  Display the given variable's documentation and value.
+
 c KEYS      Display the command name run by the given key sequence.
-C CODING    Describe the given coding system, or RET for current ones.
+w COMMAND   Display which keystrokes invoke the given command (where-is).
+.           Display any available local help at point in the echo area.
+
+Documentation
+
+r           Display the Emacs manual in Info mode.
+i           Start the Info documentation reader: read included manuals.
+t           Start the Emacs learn-by-doing tutorial.
+F COMMAND   Show the Emacs manual's section that describes the command.
+K KEYS      Show the Emacs manual's section for the command bound to KEYS.
+S SYMBOL    Show the section for the given symbol in the Info manual
+              for the programming language used in this buffer.
+R           Prompt for a manual and then display it in Info mode.
+
+Searching and packages
+
+a PATTERN   Show commands whose name matches the PATTERN (a list of words
+              or a regexp).  See also the `apropos' command.
 d PATTERN   Show a list of functions, variables, and other items whose
               documentation matches the PATTERN (a list of words or a regexp).
+P PACKAGE   Describe the given Emacs Lisp package.
+p TOPIC     Find packages matching a given topic keyword.
+
+Other
+
+C CODING    Describe the given coding system, or RET for current ones.
 e           Go to the *Messages* buffer which logs echo-area messages.
-f FUNCTION  Display documentation for the given function.
-F COMMAND   Show the Emacs manual's section that describes the command.
 g           Display information about the GNU project.
 h           Display the HELLO file which illustrates various scripts.
-i           Start the Info documentation reader: read included manuals.
 I METHOD    Describe a specific input method, or RET for current.
-k KEYS      Display the full documentation for the key sequence.
-K KEYS      Show the Emacs manual's section for the command bound to KEYS.
-l           Show last 300 input keystrokes (lossage).
 L LANG-ENV  Describes a specific language environment, or RET for current.
-m           Display documentation of current minor modes and current major mode,
-              including their special commands.
+l           Show last 300 input keystrokes (lossage).
 n           Display news of recent Emacs changes.
-o SYMBOL    Display the given function or variable's documentation and value.
-p TOPIC     Find packages matching a given topic keyword.
-P PACKAGE   Describe the given Emacs Lisp package.
-r           Display the Emacs manual in Info mode.
-R           Prompt for a manual and then display it in Info mode.
 s           Display contents of current syntax table, plus explanations.
-S SYMBOL    Show the section for the given symbol in the Info manual
-              for the programming language used in this buffer.
-t           Start the Emacs learn-by-doing tutorial.
-v VARIABLE  Display the given variable's documentation and value.
-w COMMAND   Display which keystrokes invoke the given command (where-is).
-.           Display any available local help at point in the echo area.
 
 C-a         Information about Emacs.
 C-c         Emacs copying permission (GNU General Public License).
@@ -245,7 +259,8 @@ 'help-for-help
 C-s         Search forward \"help window\".
 C-t         Emacs TODO list.
 C-w         Information on absence of warranty for GNU Emacs."
-  help-map)
+  help-map
+  help-for-help-mode)
 
 \f
 
-- 
2.30.0


^ permalink raw reply	[flat|nested] 48+ messages in thread

* RE: [External] : Proposal for an improved `help-for-help'
  2021-02-21 12:06 Proposal for an improved `help-for-help' Stefan Kangas
@ 2021-02-21 16:46 ` Drew Adams
  2021-02-21 17:31   ` Stefan Kangas
  2021-02-21 17:42 ` Lars Ingebrigtsen
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 48+ messages in thread
From: Drew Adams @ 2021-02-21 16:46 UTC (permalink / raw)
  To: Stefan Kangas, emacs-devel

It's good to discuss possible improvements to `C-h'.

(I do hope that, this time however, changes aren't
made before a wide discussion here, and preferably
some consensus is reached.  And at least this isn't
happening only in a bug thread.)
___

FWIW, I've never been a big fan of the help-for-help UI.

At a minimum, keys and commands mentioned should
have links to their full doc (or other info).

(In particular, if only keys are shown, they should
be linked to doc that shows their command bindings.)

Not as a _substitute_, but just as an example, using
`describe-keymap help-map' shows you each `C-h' key,
together with its command.  And the command, at least,
is linked to its doc string.

Again, I'm not saying that this is a _substitute_
for help-for-help.  I'm just saying that suggestions
such as yours should have links on things, at a
minimum.

^ permalink raw reply	[flat|nested] 48+ messages in thread

* RE: [External] : Proposal for an improved `help-for-help'
  2021-02-21 16:46 ` [External] : " Drew Adams
@ 2021-02-21 17:31   ` Stefan Kangas
  2021-02-21 18:17     ` Drew Adams
  0 siblings, 1 reply; 48+ messages in thread
From: Stefan Kangas @ 2021-02-21 17:31 UTC (permalink / raw)
  To: Drew Adams, emacs-devel

Drew Adams <drew.adams@oracle.com> writes:

> FWIW, I've never been a big fan of the help-for-help UI.
>
> At a minimum, keys and commands mentioned should
> have links to their full doc (or other info).

I don't see the need for such links in `help-for-help' in particular,
but I'm open to being convinced.  It might make sense if we can find a
good UI that is easy to understand and does not overwhelm.

What do you imagine this would look like?  For example, which keys would
a user press after `C-h C-h' to follow these added links?

(This might also be outside the scope of the reformatting I attempt
here.)



^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: Proposal for an improved `help-for-help'
  2021-02-21 12:06 Proposal for an improved `help-for-help' Stefan Kangas
  2021-02-21 16:46 ` [External] : " Drew Adams
@ 2021-02-21 17:42 ` Lars Ingebrigtsen
  2021-02-21 18:18   ` [External] : " Drew Adams
                     ` (2 more replies)
  2021-02-21 17:45 ` Proposal for an improved `help-for-help' Eli Zaretskii
                   ` (2 subsequent siblings)
  4 siblings, 3 replies; 48+ messages in thread
From: Lars Ingebrigtsen @ 2021-02-21 17:42 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: emacs-devel

Stefan Kangas <stefan@marxist.se> writes:

> One of the usability complaints I've heard is that `help-for-help'
> (`C-h C-h') could be better organized.  I also think some colors could
> really help in making it easier to navigate.

Looks lot better to me.

> I have also attached a patch, but it's very rough.  I'm specifically
> having some trouble with font-locking: It works if I copy the contents
> of the " *Metahelp*" buffer and invoke the mode manually, but it will
> not work when I say `C-h C-h'.  Help with this is also welcome -- I'm
> probably missing something obvious.

Is using font locking the best option here, though?  What about just
inserting the text with the text properties already applied?

(Drew responds here that he doesn't much like how `C-h C-h' works -- and
I don't much like it either.  But that's a different discussion.)

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



^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: Proposal for an improved `help-for-help'
  2021-02-21 12:06 Proposal for an improved `help-for-help' Stefan Kangas
  2021-02-21 16:46 ` [External] : " Drew Adams
  2021-02-21 17:42 ` Lars Ingebrigtsen
@ 2021-02-21 17:45 ` Eli Zaretskii
  2021-02-21 18:20   ` [External] : " Drew Adams
  2021-02-21 18:48   ` Stefan Kangas
  2021-02-21 19:27 ` Howard Melman
  2021-02-22 10:01 ` Yuri Khan
  4 siblings, 2 replies; 48+ messages in thread
From: Eli Zaretskii @ 2021-02-21 17:45 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: emacs-devel

> From: Stefan Kangas <stefan@marxist.se>
> Date: Sun, 21 Feb 2021 06:06:58 -0600
> 
> One of the usability complaints I've heard is that `help-for-help'
> (`C-h C-h') could be better organized.  I also think some colors could
> really help in making it easier to navigate.
> 
> I have attached a rough draft for what an improved version of might look
> like.  It would be great to get some feedback.

Please explain the rationale for the order.  It is different from the
order in the manual and in the menus, so we should at least discuss
it, and that needs some rationale.



^ permalink raw reply	[flat|nested] 48+ messages in thread

* RE: [External] : Proposal for an improved `help-for-help'
  2021-02-21 17:31   ` Stefan Kangas
@ 2021-02-21 18:17     ` Drew Adams
  0 siblings, 0 replies; 48+ messages in thread
From: Drew Adams @ 2021-02-21 18:17 UTC (permalink / raw)
  To: Stefan Kangas, emacs-devel

> > FWIW, I've never been a big fan of the help-for-help UI.
> >
> > At a minimum, keys and commands mentioned should
> > have links to their full doc (or other info).
> 
> I don't see the need for such links in `help-for-help' in particular,
> but I'm open to being convinced.  It might make sense if we can find a
> good UI that is easy to understand and does not overwhelm.
> 
> What do you imagine this would look like?  For example, which keys
> would a user press after `C-h C-h' to follow these added links?
> 
> (This might also be outside the scope of the reformatting I attempt
> here.)

I really haven't thought about it.  At all.

Off the top of my head, `C-h C-h' wouldn't
put you in a semi-modal dialog, showing you
some keys you can press to get particular
info (or keys to scroll for more such).

Instead, it could show you, and perhaps
select, a *Help* buffer that describes the
help system - keys with their descriptions,
and with links.

That is, after all, how you will interact
with help generally, once you get into it.
It's what each help command does: show you
some help info and (usually) provide some
links to more info.

The `C-h C-h' dialog seems a bit weird,
to me; that's all.  (I don't claim it's
weird for others.)
___

But again, I'm not proposing anything
particular as a replacement for the
`C-h C-h' dialog.  My point about that
was just that I've never been a big fan
of it.

That comment is really separate from my
feedback about your proposal, which was
to consider adding links.

^ permalink raw reply	[flat|nested] 48+ messages in thread

* RE: [External] : Re: Proposal for an improved `help-for-help'
  2021-02-21 17:42 ` Lars Ingebrigtsen
@ 2021-02-21 18:18   ` Drew Adams
  2021-02-21 19:49   ` Stefan Kangas
  2021-02-24  1:40   ` Consistent face for keys in *Help* and `substitute-command-keys' Stefan Kangas
  2 siblings, 0 replies; 48+ messages in thread
From: Drew Adams @ 2021-02-21 18:18 UTC (permalink / raw)
  To: Lars Ingebrigtsen, Stefan Kangas; +Cc: emacs-devel

> (Drew responds here that he doesn't much like how `C-h C-h' works --
> and I don't much like it either.  But that's a different discussion.)

It's definitely a different discussion.
Apologies for possibly muddying the waters.



^ permalink raw reply	[flat|nested] 48+ messages in thread

* RE: [External] : Re: Proposal for an improved `help-for-help'
  2021-02-21 17:45 ` Proposal for an improved `help-for-help' Eli Zaretskii
@ 2021-02-21 18:20   ` Drew Adams
  2021-02-21 18:48   ` Stefan Kangas
  1 sibling, 0 replies; 48+ messages in thread
From: Drew Adams @ 2021-02-21 18:20 UTC (permalink / raw)
  To: Eli Zaretskii, Stefan Kangas; +Cc: emacs-devel

> Please explain the rationale for the order.  It is different from the
> order in the manual and in the menus, so we should at least discuss
> it, and that needs some rationale.

Agreed, the order is important - should be
considered carefully.

(I don't have any suggestions about that,
at least not yet.)




^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: Proposal for an improved `help-for-help'
  2021-02-21 17:45 ` Proposal for an improved `help-for-help' Eli Zaretskii
  2021-02-21 18:20   ` [External] : " Drew Adams
@ 2021-02-21 18:48   ` Stefan Kangas
  2021-02-21 19:19     ` Eli Zaretskii
  1 sibling, 1 reply; 48+ messages in thread
From: Stefan Kangas @ 2021-02-21 18:48 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

> Please explain the rationale for the order.  It is different from the
> order in the manual and in the menus, so we should at least discuss
> it, and that needs some rationale.

This was mostly to give a quick example of how it could look.
Sorry for not being more clear.

I agree that we need to think seriously about what the best ordering is,
also compared to the manual.

If the general direction here is okay, I will provide a full proposal,
including thinking through the order.



^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: Proposal for an improved `help-for-help'
  2021-02-21 18:48   ` Stefan Kangas
@ 2021-02-21 19:19     ` Eli Zaretskii
  2021-02-21 20:04       ` Stefan Kangas
  0 siblings, 1 reply; 48+ messages in thread
From: Eli Zaretskii @ 2021-02-21 19:19 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: emacs-devel

> From: Stefan Kangas <stefan@marxist.se>
> Date: Sun, 21 Feb 2021 12:48:20 -0600
> Cc: emacs-devel@gnu.org
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Please explain the rationale for the order.  It is different from the
> > order in the manual and in the menus, so we should at least discuss
> > it, and that needs some rationale.
> 
> This was mostly to give a quick example of how it could look.
> Sorry for not being more clear.

Ah, okay.  You said you didn't care for the current order, so I
thought the order you suggested was based on some principles yhou
could describe.

> I agree that we need to think seriously about what the best ordering is,
> also compared to the manual.
> 
> If the general direction here is okay, I will provide a full proposal,
> including thinking through the order.

If the order is not the aspect you wanted us to consider, then which
aspects are?  I admit that I didn't give a thought to any other
aspects yet.



^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: Proposal for an improved `help-for-help'
  2021-02-21 12:06 Proposal for an improved `help-for-help' Stefan Kangas
                   ` (2 preceding siblings ...)
  2021-02-21 17:45 ` Proposal for an improved `help-for-help' Eli Zaretskii
@ 2021-02-21 19:27 ` Howard Melman
  2021-02-22 15:25   ` Stefan Kangas
  2021-02-22 10:01 ` Yuri Khan
  4 siblings, 1 reply; 48+ messages in thread
From: Howard Melman @ 2021-02-21 19:27 UTC (permalink / raw)
  To: emacs-devel

Stefan Kangas <stefan@marxist.se> writes:

> I have attached a rough draft for what an improved version of might look
> like.  It would be great to get some feedback.

I think this is a good improvement.  I'd like to see some of
the help text normalized to be more consistent.  Here's an
attempt at that, though it could us another pass too.


Basic Help

m           Display documentation of current major and minor modes and their commands.
x COMMAND   Display documentation of the command.
k KEYS      Display documentation of the key sequence.
f FUNCTION  Display documentation of the function.
v VARIABLE  Display documentation and value of the variable.
o SYMBOL    Display documentation and value of the function or variable.
c KEYS      Display the command name bound to the key sequence.
b           Display all bindings of commands to keys.
w COMMAND   Display which keys are bound to the command.
.           Display any available local help at point.

Documentation

t           Start the Emacs learn-by-doing tutorial.
r           Show the Emacs manual in Info mode.
F COMMAND   Show the Emacs manual's section for the command.
K KEYS      Show the Emacs manual's section for the command bound to KEYS.
i           Show the Info documentation reader listing all available manuals.
S SYMBOL    Show the section of the relevant Info manual for the symbol.

SEARCHING AND PACKAGES

a PATTERN   Show commands whose name matches the PATTERN (a list of words or a regexp).  See also the `apropos' command.
d PATTERN   Show a list of functions, variables, and other items whose documentation matches the PATTERN (a list of words or a regexp).
P PACKAGE   Describe the given Emacs Lisp package.
p TOPIC     Find packages matching a given topic keyword.

OTHER

e           Go to the *Messages* buffer which logs echo-area messages.
l           Show last 300 input keystrokes (lossage).
C CODING    Describe the given coding system, or RET for current ones.
I METHOD    Describe a specific input method, or RET for current.
L LANG-ENV  Describes a specific language environment, or RET for current.
s           Display contents of current syntax table, plus explanations.

g           Display information about the GNU project.
n           Display news of recent Emacs changes.
h           Display the HELLO file which illustrates various scripts.

C-a         Information about Emacs.
C-c         Emacs copying permission (GNU General Public License).
C-d         Instructions for debugging GNU Emacs.
C-e         External packages and information about Emacs.
C-f         Emacs FAQ.
C-m         How to order printed Emacs manuals.
C-n         News of recent Emacs changes.
C-o         Emacs ordering and distribution information.
C-p         Info about known Emacs problems.
C-s         Search forward \"help window\".
C-t         Emacs TODO list.
C-w         Information on absence of warranty for GNU Emacs."


-- 

Howard




^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: Proposal for an improved `help-for-help'
  2021-02-21 17:42 ` Lars Ingebrigtsen
  2021-02-21 18:18   ` [External] : " Drew Adams
@ 2021-02-21 19:49   ` Stefan Kangas
  2021-02-24  1:40   ` Consistent face for keys in *Help* and `substitute-command-keys' Stefan Kangas
  2 siblings, 0 replies; 48+ messages in thread
From: Stefan Kangas @ 2021-02-21 19:49 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: emacs-devel

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Is using font locking the best option here, though?  What about just
> inserting the text with the text properties already applied?

Yes, that would probably be better.

It's a little bit more work, but shouldn't be too bad.



^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: Proposal for an improved `help-for-help'
  2021-02-21 19:19     ` Eli Zaretskii
@ 2021-02-21 20:04       ` Stefan Kangas
  2021-02-21 20:16         ` Eli Zaretskii
  0 siblings, 1 reply; 48+ messages in thread
From: Stefan Kangas @ 2021-02-21 20:04 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

> Ah, okay.  You said you didn't care for the current order, so I
> thought the order you suggested was based on some principles yhou
> could describe.
>
> If the order is not the aspect you wanted us to consider, then which
> aspects are?  I admit that I didn't give a thought to any other
> aspects yet.

Well, it is roughly, but only very roughly here, based on an idea to put
more important things higher up.  (The thing that makes it "rough" is
that I didn't think through carefully the exact ordering that makes
sense.)  This goes for both the sections and the commands themselves.

Today we just have them in alphabetical order.

In the final section (currently named "Other") I'm not sure we can find
an "order of importance".  Whereas `describe-function' clearly is more
important `describe-local-help'.

I think it would be useful also to get feedback on introducing titled
sections, and the overall visual appearance (including coloring the
keybindings).



^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: Proposal for an improved `help-for-help'
  2021-02-21 20:04       ` Stefan Kangas
@ 2021-02-21 20:16         ` Eli Zaretskii
  2021-02-21 23:27           ` Stefan Kangas
  0 siblings, 1 reply; 48+ messages in thread
From: Eli Zaretskii @ 2021-02-21 20:16 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: emacs-devel

> From: Stefan Kangas <stefan@marxist.se>
> Date: Sun, 21 Feb 2021 14:04:51 -0600
> Cc: emacs-devel@gnu.org
> 
> > If the order is not the aspect you wanted us to consider, then which
> > aspects are?  I admit that I didn't give a thought to any other
> > aspects yet.
> 
> Well, it is roughly, but only very roughly here, based on an idea to put
> more important things higher up.  (The thing that makes it "rough" is
> that I didn't think through carefully the exact ordering that makes
> sense.)  This goes for both the sections and the commands themselves.

So it _is_ about the order.  But then I think we should discuss the
underlying principles, and then order the commands accordingly.

The rationale for the order in the manual was to show the discovery
commands first, and the commands for focused help later.

> I think it would be useful also to get feedback on introducing titled
> sections, and the overall visual appearance (including coloring the
> keybindings).

If the aggregation is meaningful, I think having sections should be a
no-brainer.



^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: Proposal for an improved `help-for-help'
  2021-02-21 20:16         ` Eli Zaretskii
@ 2021-02-21 23:27           ` Stefan Kangas
  2021-02-22 16:12             ` Eli Zaretskii
  0 siblings, 1 reply; 48+ messages in thread
From: Stefan Kangas @ 2021-02-21 23:27 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

> So it _is_ about the order.  But then I think we should discuss the
> underlying principles, and then order the commands accordingly.
>
> The rationale for the order in the manual was to show the discovery
> commands first, and the commands for focused help later.

To be honest, I didn't yet think very hard about this problem.  But
sure, we can discuss it.  Any suggestions or ideas are welcome.

My preliminary thinking here is that we should probably start with
something like `describe-mode' and perhaps `describe-bindings' -- things
that are about the basic commands to find help about the task that the
user is engaged in right now.  Maybe together with `describe-key', and
`describe-command'.

Ideally, we would just copy the ordering in the manual.  It would save a
lot of work.  But the Info node `(emacs) Help' puts the Emacs FAQ and
`C-h p' very early, even before `describe-mode'.  I don't think that is
exactly what we want for the `help-for-help'.

So we probably need to think a bit more about this, and how that screen
will be used.

>> I think it would be useful also to get feedback on introducing titled
>> sections, and the overall visual appearance (including coloring the
>> keybindings).
>
> If the aggregation is meaningful, I think having sections should be a
> no-brainer.

That is good to know, thanks.



^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: Proposal for an improved `help-for-help'
  2021-02-21 12:06 Proposal for an improved `help-for-help' Stefan Kangas
                   ` (3 preceding siblings ...)
  2021-02-21 19:27 ` Howard Melman
@ 2021-02-22 10:01 ` Yuri Khan
  2021-02-22 15:25   ` Stefan Kangas
  4 siblings, 1 reply; 48+ messages in thread
From: Yuri Khan @ 2021-02-22 10:01 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: Emacs developers

On Sun, 21 Feb 2021 at 19:08, Stefan Kangas <stefan@marxist.se> wrote:

> One of the usability complaints I've heard is that `help-for-help'
> (`C-h C-h') could be better organized.  I also think some colors could
> really help in making it easier to navigate.

There is some discussion going in this thread concerning the order and
sectioning. I’d like to make a few usability suggestions, too.

Is there a reason that help-for-help is page-scrollable with
Space/Backspace and C-v/M-v but not PageUp/PageDown, and
line-scrollable with the mouse wheel but not Up/Down arrow keys? A
newcomer will try those and be surprised by the help buffer silently
disappearing.

Shift+Space should probably also act as “scroll a page back” on
terminals where this key is distinguishable.



^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: Proposal for an improved `help-for-help'
  2021-02-21 19:27 ` Howard Melman
@ 2021-02-22 15:25   ` Stefan Kangas
  0 siblings, 0 replies; 48+ messages in thread
From: Stefan Kangas @ 2021-02-22 15:25 UTC (permalink / raw)
  To: Howard Melman, emacs-devel

Howard Melman <hmelman@gmail.com> writes:

> Stefan Kangas <stefan@marxist.se> writes:
>
>> I have attached a rough draft for what an improved version of might look
>> like.  It would be great to get some feedback.
>
> I think this is a good improvement.  I'd like to see some of
> the help text normalized to be more consistent.  Here's an
> attempt at that, though it could us another pass too.

Yes, I agree it could use some reformulating.  I will have a look at your
suggestions, thanks!



^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: Proposal for an improved `help-for-help'
  2021-02-22 10:01 ` Yuri Khan
@ 2021-02-22 15:25   ` Stefan Kangas
  0 siblings, 0 replies; 48+ messages in thread
From: Stefan Kangas @ 2021-02-22 15:25 UTC (permalink / raw)
  To: Yuri Khan; +Cc: Emacs developers

Yuri Khan <yuri.v.khan@gmail.com> writes:

> Is there a reason that help-for-help is page-scrollable with
> Space/Backspace and C-v/M-v but not PageUp/PageDown, and
> line-scrollable with the mouse wheel but not Up/Down arrow keys? A
> newcomer will try those and be surprised by the help buffer silently
> disappearing.
>
> Shift+Space should probably also act as “scroll a page back” on
> terminals where this key is distinguishable.

I agree with all your points here.  Fixing the above would be a good
improvement on what we have.



^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: Proposal for an improved `help-for-help'
  2021-02-21 23:27           ` Stefan Kangas
@ 2021-02-22 16:12             ` Eli Zaretskii
  0 siblings, 0 replies; 48+ messages in thread
From: Eli Zaretskii @ 2021-02-22 16:12 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: emacs-devel

> From: Stefan Kangas <stefan@marxist.se>
> Date: Sun, 21 Feb 2021 17:27:51 -0600
> Cc: emacs-devel@gnu.org
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > So it _is_ about the order.  But then I think we should discuss the
> > underlying principles, and then order the commands accordingly.
> >
> > The rationale for the order in the manual was to show the discovery
> > commands first, and the commands for focused help later.
> 
> To be honest, I didn't yet think very hard about this problem.  But
> sure, we can discuss it.  Any suggestions or ideas are welcome.
> 
> My preliminary thinking here is that we should probably start with
> something like `describe-mode' and perhaps `describe-bindings' -- things
> that are about the basic commands to find help about the task that the
> user is engaged in right now.  Maybe together with `describe-key', and
> `describe-command'.

Since there can be different use cases, and each one could favor a
different order, perhaps we should precede a short guide to the list?
Like

  If you look for some feature, try "discovery" group.
  If you look for documentation of a command or a variable or a mode,
  try ...

etc.



^ permalink raw reply	[flat|nested] 48+ messages in thread

* Consistent face for keys in *Help* and `substitute-command-keys'
  2021-02-21 17:42 ` Lars Ingebrigtsen
  2021-02-21 18:18   ` [External] : " Drew Adams
  2021-02-21 19:49   ` Stefan Kangas
@ 2021-02-24  1:40   ` Stefan Kangas
  2021-02-24  2:24     ` [External] : " Drew Adams
                       ` (4 more replies)
  2 siblings, 5 replies; 48+ messages in thread
From: Stefan Kangas @ 2021-02-24  1:40 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: emacs-devel

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

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Stefan Kangas <stefan@marxist.se> writes:
>
>> One of the usability complaints I've heard is that `help-for-help'
>> (`C-h C-h') could be better organized.  I also think some colors could
>> really help in making it easier to navigate.
>
[...]
>
> Is using font locking the best option here, though?  What about just
> inserting the text with the text properties already applied?

OK, so here's what I came up with so far.

The first step was to insert the keybindings with the new face.  But
rather than doing that in just `help-for-help', and perhaps improve
other things later, I realized that it is better to decide on a
consistent look for keybindings in _all_ help commands.

This means introducing a new face `help-key-binding' and then using that
any time we output a keybinding for use in the help system.

But going even further than that, I realized it would be very useful if
this face applies to any key in any message we output, by convention.
This would be a useful improvement in consistency, in the same way it
helps to consistently use `C-' to mean the Control modifier.  It makes
it easier for users to see that, hey, this text is different in the same
way that other keybindings have been, so it must also be referring to a
keybinding.

So I have made `substitute-command-keys' add this face unconditionally
to keys.

Having looked over the 430 matches for s-c-k in our tree, I couldn't
find any location where we would not benefit from this consistency.  On
the contrary, it would be quite nice to see that the same face used in
*Help* also shows up in messages such as these:

    "Use \\[shadow-copy-files] to update shadows."
    "Press \\[wdired-finish-edit] when finished or \\[wdired-abort-changes] ..."
    "\\[scroll-up] scrolls up, \\[scroll-down] scrolls down, ..."

Please find attached a patch.  It is a little bit rough around the edges
still so needs polishing up and documentation.  For testing, try e.g.
`C-h C-h', `C-h C-h', `C-h C-c', or even `C-s C-h ?'.

Thoughts welcome.

[-- Attachment #2: 0001-Colorize-keybindings-in-help.patch --]
[-- Type: text/x-diff, Size: 28125 bytes --]

From b7c2db02f46a44fac317a6c81efa001ecfea93a0 Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefan@marxist.se>
Date: Wed, 24 Feb 2021 01:18:13 +0100
Subject: [PATCH] Colorize keybindings in help

---
 lisp/faces.el      |   4 +
 lisp/help-fns.el   |  17 ++--
 lisp/help-macro.el | 226 ++++++++++++++++++++++-----------------------
 lisp/help.el       | 120 +++++++++++++-----------
 lisp/isearch.el    |   8 +-
 src/keymap.c       |  13 ++-
 6 files changed, 208 insertions(+), 180 deletions(-)

diff --git a/lisp/faces.el b/lisp/faces.el
index 90f11bbe3b..d6bab5e732 100644
--- a/lisp/faces.el
+++ b/lisp/faces.el
@@ -2815,6 +2815,10 @@ help-argument-name
   "Face to highlight argument names in *Help* buffers."
   :group 'help)
 
+(defface help-key-binding '((t :weight semi-bold :foreground "ForestGreen"))
+  "Face for keybindings in *Help* buffers."
+  :group 'help)
+
 (defface glyphless-char
   '((((type tty)) :inherit underline)
     (((type pc)) :inherit escape-glyph)
diff --git a/lisp/help-fns.el b/lisp/help-fns.el
index 7244695094..8277bbdad1 100644
--- a/lisp/help-fns.el
+++ b/lisp/help-fns.el
@@ -466,12 +466,15 @@ help-fns--key-bindings
               ;; If lots of ordinary text characters run this command,
               ;; don't mention them one by one.
               (if (< (length non-modified-keys) 10)
-                  (princ (mapconcat #'key-description keys ", "))
+                  (with-current-buffer standard-output
+                    (insert (mapconcat #'help--key-description-fontified
+                                       keys ", ")))
                 (dolist (key non-modified-keys)
                   (setq keys (delq key keys)))
                 (if keys
                     (progn
-                      (princ (mapconcat #'key-description keys ", "))
+                      (princ (mapconcat #'help--key-description-fontified
+                                        keys ", "))
                       (princ ", and many ordinary text characters"))
                   (princ "many ordinary text characters"))))
             (when (or remapped keys non-modified-keys)
@@ -1825,10 +1828,12 @@ describe-mode
 	      (save-excursion
 		(re-search-backward (substitute-command-keys "`\\([^`']+\\)'")
                                     nil t)
-		(help-xref-button 1 'help-function-def mode file-name)))))
-	(princ ":\n")
-	(princ (help-split-fundoc (documentation major-mode) nil 'doc))
-        (princ (help-fns--list-local-commands)))))
+                (help-xref-button 1 'help-function-def mode file-name)))))
+        (let ((fundoc (help-split-fundoc (documentation major-mode) nil 'doc)))
+          (with-current-buffer standard-output
+            (insert ":\n")
+            (insert fundoc)
+            (insert (help-fns--list-local-commands)))))))
   ;; For the sake of IELM and maybe others
   nil)
 
diff --git a/lisp/help-macro.el b/lisp/help-macro.el
index 791b10a878..4bb5e00906 100644
--- a/lisp/help-macro.el
+++ b/lisp/help-macro.el
@@ -92,119 +92,119 @@ make-help-screen
 with the key sequence that invoked FNAME.
 When FNAME finally does get a command, it executes that command
 and then returns."
-  (let ((doc-fn (intern (concat (symbol-name fname) "-doc"))))
-    `(progn
-       (defun ,doc-fn () ,help-text nil)
-       (defun ,fname ()
-	 "Help command."
-	 (interactive)
-	 (let ((line-prompt
-		(substitute-command-keys ,help-line)))
-	   (when three-step-help
-	     (message "%s" line-prompt))
-	   (let* ((help-screen (documentation (quote ,doc-fn)))
-		  ;; We bind overriding-local-map for very small
-		  ;; sections, *excluding* where we switch buffers
-		  ;; and where we execute the chosen help command.
-		  (local-map (make-sparse-keymap))
-		  (new-minor-mode-map-alist minor-mode-map-alist)
-		  (prev-frame (selected-frame))
-		  config new-frame key char)
-	     (when (string-match "%THIS-KEY%" help-screen)
-	       (setq help-screen
-		     (replace-match (key-description
-				     (substring (this-command-keys) 0 -1))
-				    t t help-screen)))
-	     (unwind-protect
-		 (let ((minor-mode-map-alist nil))
-		   (setcdr local-map ,helped-map)
-		   (define-key local-map [t] 'undefined)
-		   ;; Make the scroll bar keep working normally.
-		   (define-key local-map [vertical-scroll-bar]
-		     (lookup-key global-map [vertical-scroll-bar]))
-		   (if three-step-help
-		       (progn
-			 (setq key (let ((overriding-local-map local-map))
-				     (read-key-sequence nil)))
-			 ;; Make the HELP key translate to C-h.
-			 (if (lookup-key function-key-map key)
-			     (setq key (lookup-key function-key-map key)))
-			 (setq char (aref key 0)))
-		     (setq char ??))
-		   (when (or (eq char ??) (eq char help-char)
-			     (memq char help-event-list))
-		     (setq config (current-window-configuration))
-		     (pop-to-buffer " *Metahelp*" nil t)
-		     (and (fboundp 'make-frame)
-			  (not (eq (window-frame)
-				   prev-frame))
-			  (setq new-frame (window-frame)
-				config nil))
-		     (setq buffer-read-only nil)
-		     (let ((inhibit-read-only t))
-		       (erase-buffer)
-		       (insert help-screen))
-		     (let ((minor-mode-map-alist new-minor-mode-map-alist))
-		       (help-mode)
-		       (setq new-minor-mode-map-alist minor-mode-map-alist))
-		     (goto-char (point-min))
-		     (while (or (memq char (append help-event-list
-						   (cons help-char '(?? ?\C-v ?\s ?\177 delete backspace vertical-scroll-bar ?\M-v))))
-				(eq (car-safe char) 'switch-frame)
-				(equal key "\M-v"))
-		       (condition-case nil
-			   (cond
-			    ((eq (car-safe char) 'switch-frame)
-			     (handle-switch-frame char))
-			    ((memq char '(?\C-v ?\s))
-			     (scroll-up))
-			    ((or (memq char '(?\177 ?\M-v delete backspace))
-				 (equal key "\M-v"))
-			     (scroll-down)))
-			 (error nil))
-		       (let ((cursor-in-echo-area t)
-			     (overriding-local-map local-map))
-			 (setq key (read-key-sequence
-				    (format "Type one of the options listed%s: "
-					    (if (pos-visible-in-window-p
-						 (point-max))
-						"" ", or SPACE or DEL to scroll")))
-			       char (aref key 0)))
-
-		       ;; If this is a scroll bar command, just run it.
-		       (when (eq char 'vertical-scroll-bar)
-			 (command-execute (lookup-key local-map key) nil key))))
-		   ;; We don't need the prompt any more.
-		   (message "")
-		   ;; Mouse clicks are not part of the help feature,
-		   ;; so reexecute them in the standard environment.
-		   (if (listp char)
-		       (setq unread-command-events
-			     (cons char unread-command-events)
-			     config nil)
-		     (let ((defn (lookup-key local-map key)))
-		       (if defn
-			   (progn
-			     (when config
-			       (set-window-configuration config)
-			       (setq config nil))
-			     ;; Temporarily rebind `minor-mode-map-alist'
-			     ;; to `new-minor-mode-map-alist' (Bug#10454).
-			     (let ((minor-mode-map-alist new-minor-mode-map-alist))
-			       ;; `defn' must make sure that its frame is
-			       ;; selected, so we won't iconify it below.
-			       (call-interactively defn))
-			     (when new-frame
-			       ;; Do not iconify the selected frame.
-			       (unless (eq new-frame (selected-frame))
-				 (iconify-frame new-frame))
-			       (setq new-frame nil)))
-			 (ding)))))
-	       (when config
-		 (set-window-configuration config))
-	       (when new-frame
-		 (iconify-frame new-frame))
-	       (setq minor-mode-map-alist new-minor-mode-map-alist))))))))
+  `(defun ,fname ()
+     "Help command."
+     (interactive)
+     (let ((line-prompt
+            (substitute-command-keys ,help-line)))
+       (when three-step-help
+         (message "%s" line-prompt))
+       (let* ((help-screen ,help-text)
+              ;; We bind overriding-local-map for very small
+              ;; sections, *excluding* where we switch buffers
+              ;; and where we execute the chosen help command.
+              (local-map (make-sparse-keymap))
+              (new-minor-mode-map-alist minor-mode-map-alist)
+              (prev-frame (selected-frame))
+              config new-frame key char)
+         (when (string-match "%THIS-KEY%" help-screen)
+           (setq help-screen
+                 (replace-match (propertize
+                                 (key-description
+                                  (substring (this-command-keys) 0 -1))
+                                 'font-lock-face 'help-key-binding
+                                 'face 'help-key-binding)
+                                t t help-screen)))
+         (unwind-protect
+             (let ((minor-mode-map-alist nil))
+               (setcdr local-map ,helped-map)
+               (define-key local-map [t] 'undefined)
+               ;; Make the scroll bar keep working normally.
+               (define-key local-map [vertical-scroll-bar]
+                 (lookup-key global-map [vertical-scroll-bar]))
+               (if three-step-help
+                   (progn
+                     (setq key (let ((overriding-local-map local-map))
+                                 (read-key-sequence nil)))
+                     ;; Make the HELP key translate to C-h.
+                     (if (lookup-key function-key-map key)
+                         (setq key (lookup-key function-key-map key)))
+                     (setq char (aref key 0)))
+                 (setq char ??))
+               (when (or (eq char ??) (eq char help-char)
+                         (memq char help-event-list))
+                 (setq config (current-window-configuration))
+                 (pop-to-buffer " *Metahelp*" nil t)
+                 (and (fboundp 'make-frame)
+                      (not (eq (window-frame)
+                               prev-frame))
+                      (setq new-frame (window-frame)
+                            config nil))
+                 (setq buffer-read-only nil)
+                 (let ((inhibit-read-only t))
+                   (erase-buffer)
+                   (insert (substitute-command-keys help-screen)))
+                 (let ((minor-mode-map-alist new-minor-mode-map-alist))
+                   (help-mode)
+                   (setq new-minor-mode-map-alist minor-mode-map-alist))
+                 (goto-char (point-min))
+                 (while (or (memq char (append help-event-list
+                                               (cons help-char '(?? ?\C-v ?\s ?\177 delete backspace vertical-scroll-bar ?\M-v))))
+                            (eq (car-safe char) 'switch-frame)
+                            (equal key "\M-v"))
+                   (condition-case nil
+                       (cond
+                        ((eq (car-safe char) 'switch-frame)
+                         (handle-switch-frame char))
+                        ((memq char '(?\C-v ?\s))
+                         (scroll-up))
+                        ((or (memq char '(?\177 ?\M-v delete backspace))
+                             (equal key "\M-v"))
+                         (scroll-down)))
+                     (error nil))
+                   (let ((cursor-in-echo-area t)
+                         (overriding-local-map local-map))
+                     (setq key (read-key-sequence
+                                (format "Type one of the options listed%s: "
+                                        (if (pos-visible-in-window-p
+                                             (point-max))
+                                            "" ", or SPACE or DEL to scroll")))
+                           char (aref key 0)))
+
+                   ;; If this is a scroll bar command, just run it.
+                   (when (eq char 'vertical-scroll-bar)
+                     (command-execute (lookup-key local-map key) nil key))))
+               ;; We don't need the prompt any more.
+               (message "")
+               ;; Mouse clicks are not part of the help feature,
+               ;; so reexecute them in the standard environment.
+               (if (listp char)
+                   (setq unread-command-events
+                         (cons char unread-command-events)
+                         config nil)
+                 (let ((defn (lookup-key local-map key)))
+                   (if defn
+                       (progn
+                         (when config
+                           (set-window-configuration config)
+                           (setq config nil))
+                         ;; Temporarily rebind `minor-mode-map-alist'
+                         ;; to `new-minor-mode-map-alist' (Bug#10454).
+                         (let ((minor-mode-map-alist new-minor-mode-map-alist))
+                           ;; `defn' must make sure that its frame is
+                           ;; selected, so we won't iconify it below.
+                           (call-interactively defn))
+                         (when new-frame
+                           ;; Do not iconify the selected frame.
+                           (unless (eq new-frame (selected-frame))
+                             (iconify-frame new-frame))
+                           (setq new-frame nil)))
+                     (ding)))))
+           (when config
+             (set-window-configuration config))
+           (when new-frame
+             (iconify-frame new-frame))
+           (setq minor-mode-map-alist new-minor-mode-map-alist))))))
 
 (provide 'help-macro)
 
diff --git a/lisp/help.el b/lisp/help.el
index 084e941549..0247f08706 100644
--- a/lisp/help.el
+++ b/lisp/help.el
@@ -104,8 +104,8 @@ help-map
     (define-key map "R" 'info-display-manual)
     (define-key map "s" 'describe-syntax)
     (define-key map "t" 'help-with-tutorial)
-    (define-key map "w" 'where-is)
     (define-key map "v" 'describe-variable)
+    (define-key map "w" 'where-is)
     (define-key map "q" 'help-quit)
     map)
   "Keymap for characters following the Help key.")
@@ -199,52 +199,52 @@ 'help-for-help
   "You have typed %THIS-KEY%, the help character.  Type a Help option:
 \(Use SPC or DEL to scroll through this text.  Type \\<help-map>\\[help-quit] to exit the Help command.)
 
-a PATTERN   Show commands whose name matches the PATTERN (a list of words
+\\[apropos-command] PATTERN   Show commands whose name matches the PATTERN (a list of words
               or a regexp).  See also the `apropos' command.
-b           Display all key bindings.
-c KEYS      Display the command name run by the given key sequence.
-C CODING    Describe the given coding system, or RET for current ones.
-d PATTERN   Show a list of functions, variables, and other items whose
+\\[describe-bindings]           Display all key bindings.
+\\[describe-key-briefly] KEYS      Display the command name run by the given key sequence.
+\\[describe-coding-system] CODING    Describe the given coding system, or RET for current ones.
+\\[apropos-documentation] PATTERN   Show a list of functions, variables, and other items whose
               documentation matches the PATTERN (a list of words or a regexp).
-e           Go to the *Messages* buffer which logs echo-area messages.
-f FUNCTION  Display documentation for the given function.
-F COMMAND   Show the Emacs manual's section that describes the command.
-g           Display information about the GNU project.
-h           Display the HELLO file which illustrates various scripts.
-i           Start the Info documentation reader: read included manuals.
-I METHOD    Describe a specific input method, or RET for current.
-k KEYS      Display the full documentation for the key sequence.
-K KEYS      Show the Emacs manual's section for the command bound to KEYS.
-l           Show last 300 input keystrokes (lossage).
-L LANG-ENV  Describes a specific language environment, or RET for current.
-m           Display documentation of current minor modes and current major mode,
-              including their special commands.
-n           Display news of recent Emacs changes.
-o SYMBOL    Display the given function or variable's documentation and value.
-p TOPIC     Find packages matching a given topic keyword.
-P PACKAGE   Describe the given Emacs Lisp package.
-r           Display the Emacs manual in Info mode.
-R           Prompt for a manual and then display it in Info mode.
-s           Display contents of current syntax table, plus explanations.
-S SYMBOL    Show the section for the given symbol in the Info manual
+\\[view-echo-area-messages]           Go to the *Messages* buffer which logs echo-area messages.
+\\[describe-function] FUNCTION  Display documentation for the given function.
+\\[Info-goto-emacs-command-node] COMMAND   Show the Emacs manual's section that describes the command.
+\\[describe-gnu-project]           Display information about the GNU project.
+\\[view-hello-file]           Display the HELLO file which illustrates various scripts.
+\\[info]           Start the Info documentation reader: read included manuals.
+\\[describe-input-method] METHOD    Describe a specific input method, or RET for current.
+\\[describe-key] KEYS      Display the full documentation for the key sequence.
+\\[Info-goto-emacs-key-command-node] KEYS      Show the Emacs manual's section for the command bound to KEYS.
+\\[view-lossage]           Show last 300 input keystrokes (lossage).
+\\[describe-language-environment] LANG-ENV  Describes a specific language environment, or RET for current.
+\\[describe-mode]           Display documentation of current minor modes and current major mode,
+             including their special commands.
+\\[view-emacs-news]           Display news of recent Emacs changes.
+\\[describe-symbol] SYMBOL    Display the given function or variable's documentation and value.
+\\[finder-by-keyword] TOPIC     Find packages matching a given topic keyword.
+\\[describe-package] PACKAGE   Describe the given Emacs Lisp package.
+\\[info-emacs-manual]           Display the Emacs manual in Info mode.
+\\[info-display-manual]           Prompt for a manual and then display it in Info mode.
+\\[describe-syntax]           Display contents of current syntax table, plus explanations.
+\\[info-lookup-symbol] SYMBOL    Show the section for the given symbol in the Info manual
               for the programming language used in this buffer.
-t           Start the Emacs learn-by-doing tutorial.
-v VARIABLE  Display the given variable's documentation and value.
-w COMMAND   Display which keystrokes invoke the given command (where-is).
-.           Display any available local help at point in the echo area.
-
-C-a         Information about Emacs.
-C-c         Emacs copying permission (GNU General Public License).
-C-d         Instructions for debugging GNU Emacs.
-C-e         External packages and information about Emacs.
-C-f         Emacs FAQ.
+\\[help-with-tutorial]           Start the Emacs learn-by-doing tutorial.
+\\[describe-variable] VARIABLE  Display the given variable's documentation and value.
+\\[where-is] COMMAND   Display which keystrokes invoke the given command (where-is).
+\\[display-local-help]           Display any available local help at point in the echo area.
+
+\\[about-emacs]         Information about Emacs.
+\\[describe-copying]         Emacs copying permission (GNU General Public License).
+\\[view-emacs-debugging]         Instructions for debugging GNU Emacs.
+\\[view-external-packages]         External packages and information about Emacs.
+\\[view-emacs-FAQ]         Emacs FAQ.
 C-m         How to order printed Emacs manuals.
 C-n         News of recent Emacs changes.
-C-o         Emacs ordering and distribution information.
-C-p         Info about known Emacs problems.
-C-s         Search forward \"help window\".
-C-t         Emacs TODO list.
-C-w         Information on absence of warranty for GNU Emacs."
+\\[describe-distribution]         Emacs ordering and distribution information.
+\\[view-emacs-problems]         Info about known Emacs problems.
+\\[search-forward-help-for-help]         Search forward \"help window\".
+\\[view-emacs-todo]         Emacs TODO list.
+\\[describe-no-warranty]         Information on absence of warranty for GNU Emacs."
   help-map)
 
 \f
@@ -492,6 +492,15 @@ view-lossage
 \f
 ;; Key bindings
 
+(defun help--key-description-fontified (keys &optional prefix)
+  "Like `key-description' but add face for \"*Help*\" buffers."
+  ;; We add both the `font-lock-face' and `face' properties here, as this
+  ;; seems to be the only way to get this to work reliably in any
+  ;; buffer.
+  (propertize (key-description keys prefix)
+              'font-lock-face 'help-key-binding
+              'face 'help-key-binding))
+
 (defun describe-bindings (&optional prefix buffer)
   "Display a buffer showing a list of all defined keys, and their definitions.
 The keys are displayed in order of precedence.
@@ -511,7 +520,6 @@ describe-bindings
     (with-current-buffer (help-buffer)
       (describe-buffer-bindings buffer prefix))))
 
-;; This function used to be in keymap.c.
 (defun describe-bindings-internal (&optional menus prefix)
   "Show a list of all defined keys, and their definitions.
 We put that list in a buffer, and display the buffer.
@@ -559,7 +567,8 @@ where-is
       (let* ((remapped (command-remapping symbol))
 	     (keys (where-is-internal
 		    symbol overriding-local-map nil nil remapped))
-	     (keys (mapconcat 'key-description keys ", "))
+             (keys (mapconcat #'help--key-description-fontified
+                              keys ", "))
 	     string)
 	(setq string
 	      (if insert
@@ -587,11 +596,11 @@ where-is
   nil)
 
 (defun help-key-description (key untranslated)
-  (let ((string (key-description key)))
+  (let ((string (help--key-description-fontified key)))
     (if (or (not untranslated)
 	    (and (eq (aref untranslated 0) ?\e) (not (eq (aref key 0) ?\e))))
 	string
-      (let ((otherstring (key-description untranslated)))
+      (let ((otherstring (help--key-description-fontified untranslated)))
 	(if (equal string otherstring)
 	    string
 	  (format "%s (translated from %s)" string otherstring))))))
@@ -1053,12 +1062,14 @@ substitute-command-keys
                                 (where-is-internal fun keymap t))))
                   (if (not key)
                       ;; Function is not on any key.
-                      (progn (insert "M-x ")
+                      (progn (insert (propertize "M-x "
+                                                 'font-lock-face 'help-key-binding
+                                                 'face 'help-key-binding))
                              (goto-char (+ end-point 3))
                              (delete-char 1))
                     ;; Function is on a key.
                     (delete-char (- end-point (point)))
-                    (insert (key-description key)))))
+                    (insert (help--key-description-fontified key)))))
                ;; 1D. \{foo} is replaced with a summary of the keymap
                ;;            (symbol-value foo).
                ;;     \<foo> just sets the keymap used for \[cmd].
@@ -1172,7 +1183,7 @@ describe-map-tree
                           (concat title
                                   (if prefix
                                       (concat " Starting With "
-                                              (key-description prefix)))
+                                              (help--key-description-fontified prefix)))
                                   ":\n"))
                       "key             binding\n"
                       "---             -------\n")))
@@ -1244,7 +1255,8 @@ help--describe-translation
   (cond ((symbolp definition)
          (insert (symbol-name definition) "\n"))
         ((or (stringp definition) (vectorp definition))
-         (insert (key-description definition nil) "\n"))
+         (insert (help--key-description-fontified definition nil)
+                 "\n"))
         ((keymapp definition)
          (insert "Prefix Command\n"))
         (t (insert "??\n"))))
@@ -1351,9 +1363,9 @@ describe-map
               (setq end (caar vect))))
           ;; Now START .. END is the range to describe next.
           ;; Insert the string to describe the event START.
-          (insert (key-description (vector start) prefix))
+          (insert (help--key-description-fontified (vector start) prefix))
           (when (not (eq start end))
-            (insert " .. " (key-description (vector end) prefix)))
+            (insert " .. " (help--key-description-fontified (vector end) prefix)))
           ;; Print a description of the definition of this character.
           ;; Called function will take care of spacing out far enough
           ;; for alignment purposes.
@@ -1420,7 +1432,7 @@ describe-map
 ;;             (setq first nil))
 ;;           (when (and prefix (> (length prefix) 0))
 ;;             (insert (format "%s" prefix)))
-;;           (insert (key-description (vector start-idx) prefix))
+;;           (insert (help--key-description-fontified (vector start-idx) prefix))
 ;;           ;; Find all consecutive characters or rows that have the
 ;;           ;; same definition.
 ;;           (while (equal (keymap--get-keyelt (aref vector (1+ idx)) nil)
@@ -1433,7 +1445,7 @@ describe-map
 ;;             (insert " .. ")
 ;;             (when (and prefix (> (length prefix) 0))
 ;;               (insert (format "%s" prefix)))
-;;             (insert (key-description (vector idx) prefix)))
+;;             (insert (help--key-description-fontified (vector idx) prefix)))
 ;;           (if transl
 ;;               (help--describe-translation definition)
 ;;             (help--describe-command definition))
diff --git a/lisp/isearch.el b/lisp/isearch.el
index 8266c4b7a0..b691673c9f 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -460,11 +460,11 @@ isearch-help-map
 (make-help-screen isearch-help-for-help-internal
   (purecopy "Type a help option: [bkm] or ?")
   "You have typed %THIS-KEY%, the help character.  Type a Help option:
-\(Type \\<help-map>\\[help-quit] to exit the Help command.)
+\(Type \\<isearch-help-map>\\[help-quit] to exit the Help command.)
 
-b           Display all Isearch key bindings.
-k KEYS      Display full documentation of Isearch key sequence.
-m           Display documentation of Isearch mode.
+\\[isearch-describe-bindings]           Display all Isearch key bindings.
+\\[isearch-describe-key] KEYS      Display full documentation of Isearch key sequence.
+\\[isearch-describe-mode]           Display documentation of Isearch mode.
 
 You can't type here other help keys available in the global help map,
 but outside of this help window when you type them in Isearch mode,
diff --git a/src/keymap.c b/src/keymap.c
index 782931fadf..8df037bffc 100644
--- a/src/keymap.c
+++ b/src/keymap.c
@@ -3021,7 +3021,10 @@ describe_vector (Lisp_Object vector, Lisp_Object prefix, Lisp_Object args,
       if (!NILP (elt_prefix))
 	insert1 (elt_prefix);
 
-      insert1 (Fkey_description (kludge, prefix));
+      Lisp_Object desc = Fkey_description (kludge, prefix);
+      if (keymap_p)
+	desc = CALLN (Fpropertize, desc, Qfont_lock_face, Qhelp_key_binding);
+      insert1 (desc);
 
       /* Find all consecutive characters or rows that have the same
 	 definition.  But, if VECTOR is a char-table, we had better
@@ -3071,7 +3074,10 @@ describe_vector (Lisp_Object vector, Lisp_Object prefix, Lisp_Object args,
 	  if (!NILP (elt_prefix))
 	    insert1 (elt_prefix);
 
-	  insert1 (Fkey_description (kludge, prefix));
+	  Lisp_Object desc = Fkey_description (kludge, prefix);
+	  if (keymap_p)
+	    desc = CALLN (Fpropertize, desc, Qfont_lock_face, Qhelp_key_binding);
+	  insert1 (desc);
 	}
 
       /* Print a description of the definition of this character.
@@ -3107,7 +3113,8 @@ syms_of_keymap (void)
 {
   DEFSYM (Qkeymap, "keymap");
   DEFSYM (Qdescribe_map_tree, "describe-map-tree");
-
+  DEFSYM (Qfont_lock_face, "font-lock-face");
+  DEFSYM (Qhelp_key_binding, "help-key-binding");
   DEFSYM (Qkeymap_canonicalize, "keymap-canonicalize");
 
   /* Now we are ready to set up this property, so we can
-- 
2.30.0


^ permalink raw reply	[flat|nested] 48+ messages in thread

* RE: [External] : Consistent face for keys in *Help* and `substitute-command-keys'
  2021-02-24  1:40   ` Consistent face for keys in *Help* and `substitute-command-keys' Stefan Kangas
@ 2021-02-24  2:24     ` Drew Adams
  2021-02-24  4:44       ` Stefan Kangas
  2021-02-24 14:00     ` Basil L. Contovounesios
                       ` (3 subsequent siblings)
  4 siblings, 1 reply; 48+ messages in thread
From: Drew Adams @ 2021-02-24  2:24 UTC (permalink / raw)
  To: Stefan Kangas, Lars Ingebrigtsen; +Cc: emacs-devel

> The first step was to insert the keybindings with the new face.  But
> rather than doing that in just `help-for-help', and perhaps improve
> other things later, I realized that it is better to decide on a
> consistent look for keybindings in _all_ help commands.
> 
> This means introducing a new face `help-key-binding' and then using
> that any time we output a keybinding for use in the help system.
> 
> But going even further than that, I realized it would be very useful if
> this face applies to any key in any message we output, by convention.
> This would be a useful improvement in consistency, in the same way it
> helps to consistently use `C-' to mean the Control modifier.  It makes
> it easier for users to see that, hey, this text is different in the
> same way that other keybindings have been, so it must also be referring to a
> keybinding.
> 
> So I have made `substitute-command-keys' add this face unconditionally
> to keys.

I think you already knew that I've been doing this
in help-fns+.el for over a decade.  I've offered
the code before, and we've discussed it.  It uses
`help-substitute-command-keys':

  Same as `substitute-command-keys', but optionally
  adds buttons for help.

  Non-nil optional arg ADD-HELP-BUTTONS does that,
  adding buttons to key descriptions, which link to
  the key's command help.

That is, keys are linked to their commands.
So their face is the face Emacs uses for links.

I think that's the right (better) approach.

My code only bothers to recognize keys within
`...'.  Emacs help buffers consistently use
that notation for keys within text, but when
keys are simply listed (e.g. `C-h b') they are
sometimes just written without `...'.

Nevertheless, in those special contexts it
should be relatively straightforward to
recognize the keys, and give them links.

(Presumably, for you to have given them a new
face, you've already got code to recognize them.)

That's maybe not so useful for `C-h b' (since
the command names have links to the same targets).
But for other cases where `...' isn't used it
could help. And even in a case like `C-h b' it
wouldn't hurt - the keys would stand out - which
is presumably your current aim.

I don't see the need for another face, one that
has no associated action.  Just put links on keys.

I couldn't change `substitute-command-keys'
itself, to add an optional arg ADD-HELP-BUTTONS,
but Emacs itself could, of course.  I just
defined a new function, adding prefix `help-'.

> Having looked over the 430 matches for s-c-k in our tree, I couldn't
> find any location where we would not benefit from this consistency.  On
> the contrary, it would be quite nice to see that the same face used in
> *Help* also shows up in messages such as these:
> 
>     "Use \\[shadow-copy-files] to update shadows."
>     "Press \\[wdired-finish-edit] when finished or \\[wdired-abort-
> changes] ..."
>     "\\[scroll-up] scrolls up, \\[scroll-down] scrolls down, ..."
> 
> Please find attached a patch.  It is a little bit rough around the
> edges still so needs polishing up and documentation.  For testing,
> try e.g. `C-h C-h', `C-h C-h', `C-h C-c', or even `C-s C-h ?'.

That's already been solved.

> Thoughts welcome.

Don't do it.  If you can recognize keys then
give them links, not just a new face.

You can start with the code in help-fns+.el.
Or you can start from scratch.  But DTRT.

^ permalink raw reply	[flat|nested] 48+ messages in thread

* RE: [External] : Consistent face for keys in *Help* and `substitute-command-keys'
  2021-02-24  2:24     ` [External] : " Drew Adams
@ 2021-02-24  4:44       ` Stefan Kangas
  2021-02-24 22:01         ` Drew Adams
  0 siblings, 1 reply; 48+ messages in thread
From: Stefan Kangas @ 2021-02-24  4:44 UTC (permalink / raw)
  To: Drew Adams, Lars Ingebrigtsen; +Cc: emacs-devel

Drew Adams <drew.adams@oracle.com> writes:

> I think you already knew that I've been doing this
> in help-fns+.el for over a decade.  I've offered
> the code before, and we've discussed it.  It uses
> `help-substitute-command-keys':

I admit that I have never used any of your libraries.

But I have seen Bug#8951 and the code you presented there.

> That is, keys are linked to their commands.
> So their face is the face Emacs uses for links.
>
> I think that's the right (better) approach.

I'm not convinced of that.  There will be many links all over the place,
sometimes many links to the same place.  It risks cluttering the help
screen too much, whereas the effect you see with my patch is more
subtle.  I also don't know of too many examples where links will be
highly useful.

Still, I'm not writing off the idea completely, I just don't currently
see how to balance the drawbacks.

>> Having looked over the 430 matches for s-c-k in our tree, I couldn't
>> find any location where we would not benefit from this consistency.  On
>> the contrary, it would be quite nice to see that the same face used in
>> *Help* also shows up in messages such as these:
>>
>>     "Use \\[shadow-copy-files] to update shadows."
>>     "Press \\[wdired-finish-edit] when finished or \\[wdired-abort-
>> changes] ..."
>>     "\\[scroll-up] scrolls up, \\[scroll-down] scrolls down, ..."
>>
>> Please find attached a patch.  It is a little bit rough around the
>> edges still so needs polishing up and documentation.  For testing,
>> try e.g. `C-h C-h', `C-h C-h', `C-h C-c', or even `C-s C-h ?'.
>
> That's already been solved.

Care to elaborate?  AFAICT, messages do not currently display
keybindings in a different face.

> You can start with the code in help-fns+.el.
> Or you can start from scratch.  But DTRT.

The work is mostly to identify the places where we output keys and
ensure the strings get the correct face properties applied to them.
Therefore, it is unfortunately not much help to start from anything that
is not the Emacs source code or a patch to the Emacs source code.



^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: Consistent face for keys in *Help* and `substitute-command-keys'
  2021-02-24  1:40   ` Consistent face for keys in *Help* and `substitute-command-keys' Stefan Kangas
  2021-02-24  2:24     ` [External] : " Drew Adams
@ 2021-02-24 14:00     ` Basil L. Contovounesios
  2021-02-24 16:35       ` Stefan Kangas
  2021-02-24 14:29     ` Lars Ingebrigtsen
                       ` (2 subsequent siblings)
  4 siblings, 1 reply; 48+ messages in thread
From: Basil L. Contovounesios @ 2021-02-24 14:00 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: Lars Ingebrigtsen, emacs-devel

Stefan Kangas <stefan@marxist.se> writes:

> Thoughts welcome.

Thanks, just some questions from me.

[...]

> +(defface help-key-binding '((t :weight semi-bold :foreground "ForestGreen"))
> +  "Face for keybindings in *Help* buffers."
> +  :group 'help)

Missing :version?

> diff --git a/lisp/help-fns.el b/lisp/help-fns.el
> index 7244695094..8277bbdad1 100644
> --- a/lisp/help-fns.el
> +++ b/lisp/help-fns.el
> @@ -466,12 +466,15 @@ help-fns--key-bindings
>                ;; If lots of ordinary text characters run this command,
>                ;; don't mention them one by one.
>                (if (< (length non-modified-keys) 10)
> -                  (princ (mapconcat #'key-description keys ", "))
> +                  (with-current-buffer standard-output
> +                    (insert (mapconcat #'help--key-description-fontified
> +                                       keys ", ")))

Why is insert needed in place of princ here, but not below?

>                  (dolist (key non-modified-keys)
>                    (setq keys (delq key keys)))
>                  (if keys
>                      (progn
> -                      (princ (mapconcat #'key-description keys ", "))
> +                      (princ (mapconcat #'help--key-description-fontified
> +                                        keys ", "))

[...]

> +                 (replace-match (propertize
> +                                 (key-description
> +                                  (substring (this-command-keys) 0 -1))
> +                                 'font-lock-face 'help-key-binding
> +                                 'face 'help-key-binding)

Why not use help--key-description-fontified here?

-- 
Basil



^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: Consistent face for keys in *Help* and `substitute-command-keys'
  2021-02-24  1:40   ` Consistent face for keys in *Help* and `substitute-command-keys' Stefan Kangas
  2021-02-24  2:24     ` [External] : " Drew Adams
  2021-02-24 14:00     ` Basil L. Contovounesios
@ 2021-02-24 14:29     ` Lars Ingebrigtsen
  2021-02-24 16:46     ` Eli Zaretskii
  2021-02-24 16:51     ` Eli Zaretskii
  4 siblings, 0 replies; 48+ messages in thread
From: Lars Ingebrigtsen @ 2021-02-24 14:29 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: emacs-devel

Stefan Kangas <stefan@marxist.se> writes:

> But going even further than that, I realized it would be very useful if
> this face applies to any key in any message we output, by convention.
> This would be a useful improvement in consistency, in the same way it
> helps to consistently use `C-' to mean the Control modifier.

I think this is a very good idea.

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



^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: Consistent face for keys in *Help* and `substitute-command-keys'
  2021-02-24 14:00     ` Basil L. Contovounesios
@ 2021-02-24 16:35       ` Stefan Kangas
  2021-02-24 19:09         ` Basil L. Contovounesios
  0 siblings, 1 reply; 48+ messages in thread
From: Stefan Kangas @ 2021-02-24 16:35 UTC (permalink / raw)
  To: Basil L. Contovounesios; +Cc: Lars Ingebrigtsen, emacs-devel

"Basil L. Contovounesios" <contovob@tcd.ie> writes:

>> +(defface help-key-binding '((t :weight semi-bold :foreground "ForestGreen"))
>> +  "Face for keybindings in *Help* buffers."
>> +  :group 'help)
>
> Missing :version?

Fixed.

>> diff --git a/lisp/help-fns.el b/lisp/help-fns.el
>> index 7244695094..8277bbdad1 100644
>> --- a/lisp/help-fns.el
>> +++ b/lisp/help-fns.el
>> @@ -466,12 +466,15 @@ help-fns--key-bindings
>>                ;; If lots of ordinary text characters run this command,
>>                ;; don't mention them one by one.
>>                (if (< (length non-modified-keys) 10)
>> -                  (princ (mapconcat #'key-description keys ", "))
>> +                  (with-current-buffer standard-output
>> +                    (insert (mapconcat #'help--key-description-fontified
>> +                                       keys ", ")))
>
> Why is insert needed in place of princ here, but not below?

We do need `insert' here.  Fixed.

>>                  (dolist (key non-modified-keys)
>>                    (setq keys (delq key keys)))
>>                  (if keys
>>                      (progn
>> -                      (princ (mapconcat #'key-description keys ", "))
>> +                      (princ (mapconcat #'help--key-description-fontified
>> +                                        keys ", "))
>
> [...]
>
>> +                 (replace-match (propertize
>> +                                 (key-description
>> +                                  (substring (this-command-keys) 0 -1))
>> +                                 'font-lock-face 'help-key-binding
>> +                                 'face 'help-key-binding)
>
> Why not use help--key-description-fontified here?

Using it would introduce a circular dependency between help-macro.el and
help.el.

Thanks for your comments.



^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: Consistent face for keys in *Help* and `substitute-command-keys'
  2021-02-24  1:40   ` Consistent face for keys in *Help* and `substitute-command-keys' Stefan Kangas
                       ` (2 preceding siblings ...)
  2021-02-24 14:29     ` Lars Ingebrigtsen
@ 2021-02-24 16:46     ` Eli Zaretskii
  2021-02-25  2:26       ` Stefan Kangas
  2021-02-24 16:51     ` Eli Zaretskii
  4 siblings, 1 reply; 48+ messages in thread
From: Eli Zaretskii @ 2021-02-24 16:46 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: larsi, emacs-devel

> From: Stefan Kangas <stefan@marxist.se>
> Date: Tue, 23 Feb 2021 19:40:07 -0600
> Cc: emacs-devel@gnu.org
> 
> So I have made `substitute-command-keys' add this face unconditionally
> to keys.
> 
> Having looked over the 430 matches for s-c-k in our tree, I couldn't
> find any location where we would not benefit from this consistency.

What about tooltips? we run their text through
substitute-command-keys, AFAIR, but the tooltip's faces need to use
smaller fonts.  So at the very least this face should be documented to
affect only some of the face attributes, not all of them (which is
generally not such a good idea, as it confuses users).



^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: Consistent face for keys in *Help* and `substitute-command-keys'
  2021-02-24  1:40   ` Consistent face for keys in *Help* and `substitute-command-keys' Stefan Kangas
                       ` (3 preceding siblings ...)
  2021-02-24 16:46     ` Eli Zaretskii
@ 2021-02-24 16:51     ` Eli Zaretskii
  2021-02-25  1:56       ` Stefan Kangas
  4 siblings, 1 reply; 48+ messages in thread
From: Eli Zaretskii @ 2021-02-24 16:51 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: larsi, emacs-devel

> From: Stefan Kangas <stefan@marxist.se>
> Date: Tue, 23 Feb 2021 19:40:07 -0600
> Cc: emacs-devel@gnu.org
> 
> --- a/src/keymap.c
> +++ b/src/keymap.c
> @@ -3021,7 +3021,10 @@ describe_vector (Lisp_Object vector, Lisp_Object prefix, Lisp_Object args,
>        if (!NILP (elt_prefix))
>  	insert1 (elt_prefix);
>  
> -      insert1 (Fkey_description (kludge, prefix));
> +      Lisp_Object desc = Fkey_description (kludge, prefix);
> +      if (keymap_p)
> +	desc = CALLN (Fpropertize, desc, Qfont_lock_face, Qhelp_key_binding);
> +      insert1 (desc);

Using Fpropertize here is suboptimal: it does several thing you don't
need when calling it from C, including consing a new string, which is
just a waste of cycles.

Please use Fadd_text_properties instead.



^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: Consistent face for keys in *Help* and `substitute-command-keys'
  2021-02-24 16:35       ` Stefan Kangas
@ 2021-02-24 19:09         ` Basil L. Contovounesios
  2021-02-25  2:11           ` Stefan Kangas
  0 siblings, 1 reply; 48+ messages in thread
From: Basil L. Contovounesios @ 2021-02-24 19:09 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: Lars Ingebrigtsen, emacs-devel

Stefan Kangas <stefan@marxist.se> writes:

> "Basil L. Contovounesios" <contovob@tcd.ie> writes:
>>
>>> +                 (replace-match (propertize
>>> +                                 (key-description
>>> +                                  (substring (this-command-keys) 0 -1))
>>> +                                 'font-lock-face 'help-key-binding
>>> +                                 'face 'help-key-binding)
>>
>> Why not use help--key-description-fontified here?
>
> Using it would introduce a circular dependency between help-macro.el and
> help.el.

How would it do that given that neither make-help-screen nor the defun
it expands to is used within help-macro.el?

-- 
Basil



^ permalink raw reply	[flat|nested] 48+ messages in thread

* RE: [External] : Consistent face for keys in *Help* and `substitute-command-keys'
  2021-02-24  4:44       ` Stefan Kangas
@ 2021-02-24 22:01         ` Drew Adams
  2021-02-25  1:25           ` Stefan Kangas
  0 siblings, 1 reply; 48+ messages in thread
From: Drew Adams @ 2021-02-24 22:01 UTC (permalink / raw)
  To: Stefan Kangas, Lars Ingebrigtsen; +Cc: emacs-devel

> > I think you already knew that I've been doing this
> > in help-fns+.el for over a decade.  I've offered
> > the code before, and we've discussed it.  It uses
> > `help-substitute-command-keys':
> 
> I admit that I have never used any of your libraries.
> 
> But I have seen Bug#8951 and the code you presented there.
> 
> > That is, keys are linked to their commands.
> > So their face is the face Emacs uses for links.
> >
> > I think that's the right (better) approach.
> 
> I'm not convinced of that.  There will be many links all
> over the place, sometimes many links to the same place.

Why do you think so?  I haven't seen that.

> It risks cluttering the help screen too much,
> whereas the effect you see with my patch is
> more subtle.

What makes you think your highlighting is more
subtle?  And more subtle than what?  You just
said that you've never even tried help-fns+.el.

> I also don't know of too many examples where
> links will be highly useful.

Specifics, please.  Show an example where a
`...' key link isn't useful.

> Still, I'm not writing off the idea completely,
> I just don't currently see how to balance the drawbacks.

What drawbacks?  So far, you've talked only
abstractly, about things that don't exist.

> > That's already been solved.
> 
> Care to elaborate?  AFAICT, messages do not currently 
> display keybindings in a different face.

I meant that `...' for keys in *Help* is solved.

And the key listings you speak of there (e.g.
C-h C-h), which do _not_ use `...', can be dealt
with separately.  That help (e.g. `C-h h') is
constructed individually - it doesn't use doc
strings or `substitute-command-keys'.

> > You can start with the code in help-fns+.el.
> > Or you can start from scratch.  But DTRT.
> 
> The work is mostly to identify the places where we output keys and
> ensure the strings get the correct face properties applied to them.
> Therefore, it is unfortunately not much help to start from anything
> that is not the Emacs source code or a patch to the Emacs source code.

And yet that's what you've done with other parts
of help-fns+.el...  Suit yourself.

My point is that it's more helpful to users to
put links on those keys, in addition to keys
wrapped with `...'.

^ permalink raw reply	[flat|nested] 48+ messages in thread

* RE: [External] : Consistent face for keys in *Help* and `substitute-command-keys'
  2021-02-24 22:01         ` Drew Adams
@ 2021-02-25  1:25           ` Stefan Kangas
  2021-02-25  6:43             ` Drew Adams
  0 siblings, 1 reply; 48+ messages in thread
From: Stefan Kangas @ 2021-02-25  1:25 UTC (permalink / raw)
  To: Drew Adams, Lars Ingebrigtsen; +Cc: emacs-devel

Drew Adams <drew.adams@oracle.com> writes:

> What makes you think your highlighting is more
> subtle?  And more subtle than what?  You just
> said that you've never even tried help-fns+.el.

OK, this is a fair point.  I will try help-fns+.el.

>> I also don't know of too many examples where
>> links will be highly useful.
>
> Specifics, please.  Show an example where a
> `...' key link isn't useful.

I actually meant what I wrote literally.

So I would rather have hoped that you have some examples of where it is
"highly useful".

>> Still, I'm not writing off the idea completely,
>> I just don't currently see how to balance the drawbacks.
>
> What drawbacks?  So far, you've talked only
> abstractly, about things that don't exist.

I see two drawbacks:

a) Too many links in the *Help* buffer that compete for attention.

b) Not being able to show a consistent face in the minibuffer and the
   help buffer.  (This is true iff we would use the `button' face also
   for linked keys in *Help*.  Of course we could just not do that, but
   then we have an inconsistent face for the links instead...)

>> The work is mostly to identify the places where we output keys and
>> ensure the strings get the correct face properties applied to them.
>> Therefore, it is unfortunately not much help to start from anything
>> that is not the Emacs source code or a patch to the Emacs source code.
>
> And yet that's what you've done with other parts
> of help-fns+.el...  Suit yourself.

I assume you refer to `describe-keymap'?  That work was very different
in nature.

My point is that it is not always as easy as just re-using some existing
code.  We still need to integrate it in Emacs.  It is of course good
that you offer your code for inclusion, but when you do so in general
terms (rather than, say, as a patch) there is still quite some work
remaining to make it happen.  This was true for a highly isolated
command like `describe-keymap', and even more so for the feature we
discuss here.



^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: Consistent face for keys in *Help* and `substitute-command-keys'
  2021-02-24 16:51     ` Eli Zaretskii
@ 2021-02-25  1:56       ` Stefan Kangas
  2021-02-25 14:24         ` Eli Zaretskii
  0 siblings, 1 reply; 48+ messages in thread
From: Stefan Kangas @ 2021-02-25  1:56 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: larsi, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

> Using Fpropertize here is suboptimal: it does several thing you don't
> need when calling it from C, including consing a new string, which is
> just a waste of cycles.
>
> Please use Fadd_text_properties instead.

How's this?

diff --git a/src/keymap.c b/src/keymap.c
index 782931fadf..e38c9b908a 100644
--- a/src/keymap.c
+++ b/src/keymap.c
@@ -2846,6 +2846,23 @@ DEFUN ("describe-vector", Fdescribe_vector,
Sdescribe_vector, 1, 2, 0,
   return unbind_to (count, Qnil);
 }

+static Lisp_Object fontify_key_properties;
+
+static Lisp_Object
+describe_key_maybe_fontify (Lisp_Object str, Lisp_Object prefix,
+				   bool keymap_p)
+{
+  Lisp_Object key_desc = Fkey_description (str, prefix);
+
+  if (keymap_p)
+    return Fadd_text_properties (make_fixnum (0),
+				 make_fixnum (SCHARS (key_desc)),
+				 fontify_key_properties,
+				 key_desc);
+  else
+    return key_desc;
+}
+
 DEFUN ("help--describe-vector", Fhelp__describe_vector,
Shelp__describe_vector, 7, 7, 0,
        doc: /* Insert in the current buffer a description of the
contents of VECTOR.
 Call DESCRIBER to insert the description of one value found in VECTOR.
@@ -3021,7 +3038,7 @@ describe_vector (Lisp_Object vector, Lisp_Object
prefix, Lisp_Object args,
       if (!NILP (elt_prefix))
 	insert1 (elt_prefix);

-      insert1 (Fkey_description (kludge, prefix));
+      insert1 (describe_key_maybe_fontify (kludge, prefix, keymap_p));

       /* Find all consecutive characters or rows that have the same
 	 definition.  But, if VECTOR is a char-table, we had better
@@ -3071,7 +3088,7 @@ describe_vector (Lisp_Object vector, Lisp_Object
prefix, Lisp_Object args,
 	  if (!NILP (elt_prefix))
 	    insert1 (elt_prefix);

-	  insert1 (Fkey_description (kludge, prefix));
+	  insert1 (describe_key_maybe_fontify (kludge, prefix, keymap_p));
 	}

       /* Print a description of the definition of this character.
@@ -3200,6 +3217,12 @@ syms_of_keymap (void)
   staticpro (&where_is_cache);
   staticpro (&where_is_cache_keymaps);

+  DEFSYM (Qfont_lock_face, "font-lock-face");
+  DEFSYM (Qhelp_key_binding, "help-key-binding");
+  staticpro (&fontify_key_properties);
+  fontify_key_properties = Fcons (Qfont_lock_face,
+				  Fcons (Qhelp_key_binding, Qnil));
+
   defsubr (&Skeymapp);
   defsubr (&Skeymap_parent);
   defsubr (&Skeymap_prompt);



^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: Consistent face for keys in *Help* and `substitute-command-keys'
  2021-02-24 19:09         ` Basil L. Contovounesios
@ 2021-02-25  2:11           ` Stefan Kangas
  0 siblings, 0 replies; 48+ messages in thread
From: Stefan Kangas @ 2021-02-25  2:11 UTC (permalink / raw)
  To: Basil L. Contovounesios; +Cc: Lars Ingebrigtsen, emacs-devel

"Basil L. Contovounesios" <contovob@tcd.ie> writes:

>> Using it would introduce a circular dependency between help-macro.el and
>> help.el.
>
> How would it do that given that neither make-help-screen nor the defun
> it expands to is used within help-macro.el?

Oops, that is true of course, it won't be cyclical.  I'll change it.



^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: Consistent face for keys in *Help* and `substitute-command-keys'
  2021-02-24 16:46     ` Eli Zaretskii
@ 2021-02-25  2:26       ` Stefan Kangas
  2021-02-25 14:28         ` Eli Zaretskii
  0 siblings, 1 reply; 48+ messages in thread
From: Stefan Kangas @ 2021-02-25  2:26 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: larsi, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

> What about tooltips? we run their text through
> substitute-command-keys, AFAIR, but the tooltip's faces need to use
> smaller fonts.  So at the very least this face should be documented to
> affect only some of the face attributes, not all of them (which is
> generally not such a good idea, as it confuses users).

This doesn't have any effect on the appearance of the tooltip, neither
in GTK or Lucid.  Perhaps because it is all overwritten with the
`tooltip' face in `tooltip'-show?  Or am I missing something?

BTW, it doesn't seem to have any effect to customize the `tooltip' face
here.  Is it supposed to change the look of the tooltip in GTK?



^ permalink raw reply	[flat|nested] 48+ messages in thread

* RE: [External] : Consistent face for keys in *Help* and `substitute-command-keys'
  2021-02-25  1:25           ` Stefan Kangas
@ 2021-02-25  6:43             ` Drew Adams
  2021-02-25 15:06               ` Eli Zaretskii
  2021-02-25 16:22               ` Stefan Kangas
  0 siblings, 2 replies; 48+ messages in thread
From: Drew Adams @ 2021-02-25  6:43 UTC (permalink / raw)
  To: Stefan Kangas, Lars Ingebrigtsen; +Cc: emacs-devel

> >> I also don't know of too many examples where
> >> links will be highly useful.
> >
> > Specifics, please.  Show an example where a
> > `...' key link isn't useful.
> 
> I actually meant what I wrote literally.
> 
> So I would rather have hoped that you have some
> examples of where it is "highly useful".

Pretty much any *Help* buffer where keys are described.

1. emacs -Q
2. Load help-fns+.el.
3. Dired a directory.
4. `C-h m'

Contrast that with the same recipe without loading
help-fns+.el.  In this case the keys aren't even
surrounded by `...', but they're nevertheless
handled by `substitute-command-keys' (which is the
real criterion), so they're given links.

Or try any other *Help* that describes keys, where
`substitute-command-keys' gets used (because of
\\[...]).

There's nothing special about Dired `C-h m'.  When
I start Emacs (even with -Q) I start in a Dired
buffer.  So it's the first thing I tried.  It's a
fine recipe, but pretty much any other help command
that shows some keys would be as fine.

> >> Still, I'm not writing off the idea completely,
> >> I just don't currently see how to balance the drawbacks.
> >
> > What drawbacks?  So far, you've talked only
> > abstractly, about things that don't exist.
> 
> I see two drawbacks:
> 
> a) Too many links in the *Help* buffer that compete for attention.

Not at all.  Try the recipe.  There's nothing
distracting; not "too many links" at all.

(Sounds like Emperor Joseph II's "Too many notes".
Not that I compare myself with Mozart. ;-))

Please show an example with "too many links".

> b) Not being able to show a consistent face in the minibuffer and the
>    help buffer.  (This is true iff we would use the `button' face also
>    for linked keys in *Help*.  Of course we could just not do that, but
>    then we have an inconsistent face for the links instead...)
> 
> >> it is unfortunately not much help to start from anything
> >> that is not the Emacs source code or a patch to the Emacs
> >> source code.
> >
> > And yet that's what you've done with other parts
> > of help-fns+.el...  Suit yourself.
> 
> I assume you refer to `describe-keymap'?  That work was
> very different in nature.

`describe-keymap', `describe-package', `describe-command',
`describe-option',... so far.

And there's nothing "very different in nature" about
any of those help-fns+.el inventions.  They're all
minor and simple, but useful.

> My point is that it is not always as easy as just re-using
> some existing code.  We still need to integrate it in Emacs.

I said, from the beginning:

  If you can recognize keys then give them links,
  not just a new face.

  You can start with the code in help-fns+.el.
  Or you can start from scratch.  But DTRT.
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

The point is to _give keys links_, not just color
them.  I have code that does that, which you can
use, but you haven't even tried yet (so you say).

If you want to do it some other way, then do that,
but do it - give keys links - color, yes, but
color with purpose.

> It is of course good that you offer your code for
> inclusion, but when you do so in general terms
> (rather than, say, as a patch) there is still quite
> some work remaining to make it happen.  This was
> true for a highly isolated command like
> `describe-keymap', and even more so for the feature
> we discuss here.

Actually no, there was nothing you needed to do, for
`describe-keymap'.  But you wanted different behavior,
so you changed it slightly.  Fine.  (That means I
still need to provide my version, which has optional
arg SEARCH-SYMBOLS-P, but so be it.)

I don't care if you use my `describe-command' or
whatever.  If the behavior you provide is equal or
better, then I can get rid of my version (except for
older Emacs versions, where it's not provided yet).

But if the behavior you provide isn't as good, then
unfortunately, I need to keep providing my version.
I'd prefer that Emacs itself offer such features - for
Emacs's benefit, not just so I can stop maintaining
them.  But if it doesn't, then it's no big deal.

IMO, keys should have links in *Help* - in general,
and pretty much all the time.  There are no doubt some
rare exceptions.  `C-h b' could be an exception, since
the keys are just listed next to their commands, which
have links (and the key and command targets would be
the same).

I can't think of any other exceptions off hand, but
there might be one or two.  Clearly, *Help* that
lists keys together with their _descriptions_ is not
an exception.  That includes `C-h C-h', just as much
as `C-h m'.

The point is a general one: link keys, just like we
link variables & functions.  Should be a no-brainer,
IMO.  I'd think that anyone comparing even just
Dired `C-h m' with & without key links would agree.
___

But what do I know?

[NIH?  I also thought (think) that Lar's "solution"
for providing command filtering for `M-x' is waaay
overengineered, obtrusive, hard to maintain, and
impossible for users to modify.  A simple `put'
of a property on the command symbol would be far
better, for the reasons I gave.

But though that's been argued several times (even
by Eli), the arguments go ignored - no counter
argument, no response at all.  Just ignore it;
it'll go away, and Emacs'll be none the wiser.
Alas.]


^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: Consistent face for keys in *Help* and `substitute-command-keys'
  2021-02-25  1:56       ` Stefan Kangas
@ 2021-02-25 14:24         ` Eli Zaretskii
  0 siblings, 0 replies; 48+ messages in thread
From: Eli Zaretskii @ 2021-02-25 14:24 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: larsi, emacs-devel

> From: Stefan Kangas <stefan@marxist.se>
> Date: Wed, 24 Feb 2021 19:56:00 -0600
> Cc: larsi@gnus.org, emacs-devel@gnu.org
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Using Fpropertize here is suboptimal: it does several thing you don't
> > need when calling it from C, including consing a new string, which is
> > just a waste of cycles.
> >
> > Please use Fadd_text_properties instead.
> 
> How's this?

LGTM, thanks.



^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: Consistent face for keys in *Help* and `substitute-command-keys'
  2021-02-25  2:26       ` Stefan Kangas
@ 2021-02-25 14:28         ` Eli Zaretskii
  2021-02-25 16:45           ` Stefan Kangas
  0 siblings, 1 reply; 48+ messages in thread
From: Eli Zaretskii @ 2021-02-25 14:28 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: larsi, emacs-devel

> From: Stefan Kangas <stefan@marxist.se>
> Date: Wed, 24 Feb 2021 20:26:46 -0600
> Cc: larsi@gnus.org, emacs-devel@gnu.org
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > What about tooltips? we run their text through
> > substitute-command-keys, AFAIR, but the tooltip's faces need to use
> > smaller fonts.  So at the very least this face should be documented to
> > affect only some of the face attributes, not all of them (which is
> > generally not such a good idea, as it confuses users).
> 
> This doesn't have any effect on the appearance of the tooltip, neither
> in GTK or Lucid.

In GTK, I'm not surprised, but I thought with Lucid Emacs itself pops
up tooltips.

What exactly did you try?  The easiest test is to put a help-echo
property on some text in Fundamental mode, then hover the mouse
pointer over that text.

> Perhaps because it is all overwritten with the
> `tooltip' face in `tooltip'-show?

No, that uses 'propertize', which _adds_ text properties.

> BTW, it doesn't seem to have any effect to customize the `tooltip' face
> here.  Is it supposed to change the look of the tooltip in GTK?

Not in GTK, I think, not unless you set x-gtk-use-system-tooltips to a
nil value.



^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [External] : Consistent face for keys in *Help* and `substitute-command-keys'
  2021-02-25  6:43             ` Drew Adams
@ 2021-02-25 15:06               ` Eli Zaretskii
  2021-02-25 16:22               ` Stefan Kangas
  1 sibling, 0 replies; 48+ messages in thread
From: Eli Zaretskii @ 2021-02-25 15:06 UTC (permalink / raw)
  To: Drew Adams; +Cc: larsi, stefan, emacs-devel

> From: Drew Adams <drew.adams@oracle.com>
> Date: Thu, 25 Feb 2021 06:43:09 +0000
> Cc: "emacs-devel@gnu.org" <emacs-devel@gnu.org>
> 
> But though that's been argued several times (even
> by Eli), the arguments go ignored - no counter
> argument, no response at all.  Just ignore it;
> it'll go away, and Emacs'll be none the wiser.

Please don't misrepresent what happens in these discussions.  Those
arguments were not ignored; expressing disagreement doesn't mean
ignoring.



^ permalink raw reply	[flat|nested] 48+ messages in thread

* RE: [External] : Consistent face for keys in *Help* and `substitute-command-keys'
  2021-02-25  6:43             ` Drew Adams
  2021-02-25 15:06               ` Eli Zaretskii
@ 2021-02-25 16:22               ` Stefan Kangas
  1 sibling, 0 replies; 48+ messages in thread
From: Stefan Kangas @ 2021-02-25 16:22 UTC (permalink / raw)
  To: Drew Adams, Lars Ingebrigtsen; +Cc: emacs-devel

Drew Adams <drew.adams@oracle.com> writes:

> Actually no, there was nothing you needed to do, for
> `describe-keymap'.  But you wanted different behavior,
> so you changed it slightly.  Fine.

There are all kinds of adaptions to make before putting things on master
besides just copying it from some library: making sure the code conforms
to our style (yours doesn't), does not have features we do not need
(support for your other "foo+" libraries), adding proper documentation
(yes, we need to update the manual as well), adding NEWS, discussing the
details, testing, etc.

I have no idea why you dismiss that work as "nothing to do".  If it is
so easy and so little work, please start sending patches instead of just
linking some library.  Thanks in advance.

> I don't care if you use my `describe-command' or
> whatever.
[...]
> But what do I know?
>
> [NIH?

Oh, please.  Here is the definition of `describe-command':

    (defun describe-command (command)
      (interactive (help-fns--describe-function-or-command-prompt 'is-command))
      (describe-function command))

Yeah, I chose not to "re-use" anything here.  Sorry for suffering from
NIH.

I would suggest you actually study my patch, and then point out any
opportunities for re-using your code.  I have now studied your code and
I see *no* such opportunities.  (This was a fully expected result, for
the reasons I have already provided.)



^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: Consistent face for keys in *Help* and `substitute-command-keys'
  2021-02-25 14:28         ` Eli Zaretskii
@ 2021-02-25 16:45           ` Stefan Kangas
  2021-02-25 18:25             ` Eli Zaretskii
  2021-02-25 19:44             ` martin rudalics
  0 siblings, 2 replies; 48+ messages in thread
From: Stefan Kangas @ 2021-02-25 16:45 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: larsi, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

> In GTK, I'm not surprised, but I thought with Lucid Emacs itself pops
> up tooltips.
>
> What exactly did you try?  The easiest test is to put a help-echo
> property on some text in Fundamental mode, then hover the mouse
> pointer over that text.

0. emacs -Q
1. M-x fundamental-mode RET
2. M-: (insert (propertize "foo" 'help-echo "foo \\[forward-line]")) RET

No visible effect on the tooltip --with-x-toolkit={gtk,lucid,athena,no}.

(With GTK, I also tried with x-gtk-use-system-tooltips set to nil.)

>> BTW, it doesn't seem to have any effect to customize the `tooltip' face
>> here.  Is it supposed to change the look of the tooltip in GTK?
>
> Not in GTK, I think, not unless you set x-gtk-use-system-tooltips to a
> nil value.

Ah, right.  I will add that to the docstring of the `tooltip' face.



^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: Consistent face for keys in *Help* and `substitute-command-keys'
  2021-02-25 16:45           ` Stefan Kangas
@ 2021-02-25 18:25             ` Eli Zaretskii
  2021-02-25 18:48               ` Stefan Kangas
  2021-02-25 19:44             ` martin rudalics
  1 sibling, 1 reply; 48+ messages in thread
From: Eli Zaretskii @ 2021-02-25 18:25 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: larsi, emacs-devel

> From: Stefan Kangas <stefan@marxist.se>
> Date: Thu, 25 Feb 2021 10:45:42 -0600
> Cc: larsi@gnus.org, emacs-devel@gnu.org
> 
> 0. emacs -Q
> 1. M-x fundamental-mode RET
> 2. M-: (insert (propertize "foo" 'help-echo "foo \\[forward-line]")) RET
> 
> No visible effect on the tooltip --with-x-toolkit={gtk,lucid,athena,no}.

And the face for keys is defined how?



^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: Consistent face for keys in *Help* and `substitute-command-keys'
  2021-02-25 18:25             ` Eli Zaretskii
@ 2021-02-25 18:48               ` Stefan Kangas
  2021-02-25 19:11                 ` Eli Zaretskii
  2021-02-25 19:14                 ` [External] : " Drew Adams
  0 siblings, 2 replies; 48+ messages in thread
From: Stefan Kangas @ 2021-02-25 18:48 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: larsi, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> 0. emacs -Q
>> 1. M-x fundamental-mode RET
>> 2. M-: (insert (propertize "foo" 'help-echo "foo \\[forward-line]")) RET
>>
>> No visible effect on the tooltip --with-x-toolkit={gtk,lucid,athena,no}.
>
> And the face for keys is defined how?

(defface help-key-binding '((t :foreground "RoyalBlue3"))
  "Face for keybindings in *Help* buffers."
  :version "28.1"
  :group 'help)



^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: Consistent face for keys in *Help* and `substitute-command-keys'
  2021-02-25 18:48               ` Stefan Kangas
@ 2021-02-25 19:11                 ` Eli Zaretskii
  2021-02-25 19:47                   ` Stefan Kangas
  2021-02-25 19:14                 ` [External] : " Drew Adams
  1 sibling, 1 reply; 48+ messages in thread
From: Eli Zaretskii @ 2021-02-25 19:11 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: larsi, emacs-devel

> From: Stefan Kangas <stefan@marxist.se>
> Date: Thu, 25 Feb 2021 12:48:37 -0600
> Cc: larsi@gnus.org, emacs-devel@gnu.org
> 
> >> 2. M-: (insert (propertize "foo" 'help-echo "foo \\[forward-line]")) RET
> >>
> >> No visible effect on the tooltip --with-x-toolkit={gtk,lucid,athena,no}.
> >
> > And the face for keys is defined how?
> 
> (defface help-key-binding '((t :foreground "RoyalBlue3"))
>   "Face for keybindings in *Help* buffers."
>   :version "28.1"
>   :group 'help)

Ah, okay: you are putting the 'face' property, but so does
'tooltip-show'.  So yes, the latter one overrides the former one.

So I guess we will need to change the design of this to avoid
overriding the whole face of a tooltip, or maybe add some special code
to help_echo_substitute_command_keys.



^ permalink raw reply	[flat|nested] 48+ messages in thread

* RE: [External] : Re: Consistent face for keys in *Help* and `substitute-command-keys'
  2021-02-25 18:48               ` Stefan Kangas
  2021-02-25 19:11                 ` Eli Zaretskii
@ 2021-02-25 19:14                 ` Drew Adams
  1 sibling, 0 replies; 48+ messages in thread
From: Drew Adams @ 2021-02-25 19:14 UTC (permalink / raw)
  To: Stefan Kangas, Eli Zaretskii; +Cc: larsi, emacs-devel

> (defface help-key-binding '((t :foreground "RoyalBlue3"))

IMO, that's too close to link color, which is "blue1".

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: Consistent face for keys in *Help* and `substitute-command-keys'
  2021-02-25 16:45           ` Stefan Kangas
  2021-02-25 18:25             ` Eli Zaretskii
@ 2021-02-25 19:44             ` martin rudalics
  1 sibling, 0 replies; 48+ messages in thread
From: martin rudalics @ 2021-02-25 19:44 UTC (permalink / raw)
  To: Stefan Kangas, Eli Zaretskii; +Cc: larsi, emacs-devel

 > 2. M-: (insert (propertize "foo" 'help-echo "foo \\[forward-line]")) RET
 >
 > No visible effect on the tooltip --with-x-toolkit={gtk,lucid,athena,no}.

It should work with 'x-show-tip'.

martin



^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: Consistent face for keys in *Help* and `substitute-command-keys'
  2021-02-25 19:11                 ` Eli Zaretskii
@ 2021-02-25 19:47                   ` Stefan Kangas
  2021-02-25 20:32                     ` Eli Zaretskii
  0 siblings, 1 reply; 48+ messages in thread
From: Stefan Kangas @ 2021-02-25 19:47 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: larsi, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

> Ah, okay: you are putting the 'face' property, but so does
> 'tooltip-show'.  So yes, the latter one overrides the former one.
>
> So I guess we will need to change the design of this to avoid
> overriding the whole face of a tooltip, or maybe add some special code
> to help_echo_substitute_command_keys.

Oh, okay.  I actually thought your concern was the opposite case here.

I think it is okay that tooltips do not use the `help-key-binding' face.
That seems in line with what other software does for tooltips, I
believe.  But I could be wrong.



^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: Consistent face for keys in *Help* and `substitute-command-keys'
  2021-02-25 19:47                   ` Stefan Kangas
@ 2021-02-25 20:32                     ` Eli Zaretskii
  2021-03-04  6:24                       ` Stefan Kangas
  0 siblings, 1 reply; 48+ messages in thread
From: Eli Zaretskii @ 2021-02-25 20:32 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: larsi, emacs-devel

> From: Stefan Kangas <stefan@marxist.se>
> Date: Thu, 25 Feb 2021 13:47:07 -0600
> Cc: larsi@gnus.org, emacs-devel@gnu.org
> 
> I think it is okay that tooltips do not use the `help-key-binding' face.

If you are willing to give up on key sequences in tooltips, then why
do it in substitute-command-keys?



^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: Consistent face for keys in *Help* and `substitute-command-keys'
  2021-02-25 20:32                     ` Eli Zaretskii
@ 2021-03-04  6:24                       ` Stefan Kangas
  2021-03-04 14:00                         ` Eli Zaretskii
  0 siblings, 1 reply; 48+ messages in thread
From: Stefan Kangas @ 2021-03-04  6:24 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: larsi, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> I think it is okay that tooltips do not use the `help-key-binding' face.
>
> If you are willing to give up on key sequences in tooltips, then why
> do it in substitute-command-keys?

I have thought a bit more about this and I can see that supporting this
in tooltips could be useful.  So I'll look into it.

> So I guess we will need to change the design of this to avoid overriding
> the whole face of a tooltip, or maybe add some special code to
> help_echo_substitute_command_keys.

Could we just use `add-face-text-property' here, perhaps?

It seems to do what we want:

    (let ((foo "x") bar)
      (add-face-text-property 0 (length foo) 'bold nil foo)
      (setq bar (concat "y" foo "y"))
      (add-face-text-property 0 (length bar) 'italic nil bar)
      bar)

    => #("yxy" 0 1 (face italic) 1 2 (face (italic bold)) 2 3 (face italic))



^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: Consistent face for keys in *Help* and `substitute-command-keys'
  2021-03-04  6:24                       ` Stefan Kangas
@ 2021-03-04 14:00                         ` Eli Zaretskii
  0 siblings, 0 replies; 48+ messages in thread
From: Eli Zaretskii @ 2021-03-04 14:00 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: larsi, emacs-devel

> From: Stefan Kangas <stefan@marxist.se>
> Date: Thu, 4 Mar 2021 00:24:14 -0600
> Cc: larsi@gnus.org, emacs-devel@gnu.org
> 
> > So I guess we will need to change the design of this to avoid overriding
> > the whole face of a tooltip, or maybe add some special code to
> > help_echo_substitute_command_keys.
> 
> Could we just use `add-face-text-property' here, perhaps?
> 
> It seems to do what we want:
> 
>     (let ((foo "x") bar)
>       (add-face-text-property 0 (length foo) 'bold nil foo)
>       (setq bar (concat "y" foo "y"))
>       (add-face-text-property 0 (length bar) 'italic nil bar)
>       bar)
> 
>     => #("yxy" 0 1 (face italic) 1 2 (face (italic bold)) 2 3 (face italic))

That's because you add a property which was unspecified by the
original face.  But in the tooltip case, the function tooltip-show
propertizes the entire text it receives with the 'tooltip' face, so
any face attributes in the text that are also specified by the
'tooltip' face will be overwritten.  So, for example, if the tooltip
text had a :background attribute, that attribute would be overwritten
by the background color of the 'tooltip' face.  Isn't that what you
see?



^ permalink raw reply	[flat|nested] 48+ messages in thread

end of thread, other threads:[~2021-03-04 14:00 UTC | newest]

Thread overview: 48+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-21 12:06 Proposal for an improved `help-for-help' Stefan Kangas
2021-02-21 16:46 ` [External] : " Drew Adams
2021-02-21 17:31   ` Stefan Kangas
2021-02-21 18:17     ` Drew Adams
2021-02-21 17:42 ` Lars Ingebrigtsen
2021-02-21 18:18   ` [External] : " Drew Adams
2021-02-21 19:49   ` Stefan Kangas
2021-02-24  1:40   ` Consistent face for keys in *Help* and `substitute-command-keys' Stefan Kangas
2021-02-24  2:24     ` [External] : " Drew Adams
2021-02-24  4:44       ` Stefan Kangas
2021-02-24 22:01         ` Drew Adams
2021-02-25  1:25           ` Stefan Kangas
2021-02-25  6:43             ` Drew Adams
2021-02-25 15:06               ` Eli Zaretskii
2021-02-25 16:22               ` Stefan Kangas
2021-02-24 14:00     ` Basil L. Contovounesios
2021-02-24 16:35       ` Stefan Kangas
2021-02-24 19:09         ` Basil L. Contovounesios
2021-02-25  2:11           ` Stefan Kangas
2021-02-24 14:29     ` Lars Ingebrigtsen
2021-02-24 16:46     ` Eli Zaretskii
2021-02-25  2:26       ` Stefan Kangas
2021-02-25 14:28         ` Eli Zaretskii
2021-02-25 16:45           ` Stefan Kangas
2021-02-25 18:25             ` Eli Zaretskii
2021-02-25 18:48               ` Stefan Kangas
2021-02-25 19:11                 ` Eli Zaretskii
2021-02-25 19:47                   ` Stefan Kangas
2021-02-25 20:32                     ` Eli Zaretskii
2021-03-04  6:24                       ` Stefan Kangas
2021-03-04 14:00                         ` Eli Zaretskii
2021-02-25 19:14                 ` [External] : " Drew Adams
2021-02-25 19:44             ` martin rudalics
2021-02-24 16:51     ` Eli Zaretskii
2021-02-25  1:56       ` Stefan Kangas
2021-02-25 14:24         ` Eli Zaretskii
2021-02-21 17:45 ` Proposal for an improved `help-for-help' Eli Zaretskii
2021-02-21 18:20   ` [External] : " Drew Adams
2021-02-21 18:48   ` Stefan Kangas
2021-02-21 19:19     ` Eli Zaretskii
2021-02-21 20:04       ` Stefan Kangas
2021-02-21 20:16         ` Eli Zaretskii
2021-02-21 23:27           ` Stefan Kangas
2021-02-22 16:12             ` Eli Zaretskii
2021-02-21 19:27 ` Howard Melman
2021-02-22 15:25   ` Stefan Kangas
2021-02-22 10:01 ` Yuri Khan
2021-02-22 15:25   ` Stefan Kangas

unofficial mirror of emacs-devel@gnu.org 

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://yhetil.org/emacs-devel/0 emacs-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 emacs-devel emacs-devel/ https://yhetil.org/emacs-devel \
		emacs-devel@gnu.org
	public-inbox-index emacs-devel

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.yhetil.org/yhetil.emacs.devel
	nntp://news.gmane.io/gmane.emacs.devel


AGPL code for this site: git clone http://ou63pmih66umazou.onion/public-inbox.git