bug#8951: 24.0.50; [PATCH] enhancement request: buttonize key names

bug#8951: 24.0.50; [PATCH] enhancement request: buttonize key names

[Drew Adams]

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

Sent to emacs-devel@gnu.org 2011/06/27:
http://lists.gnu.org/archive/html/emacs-devel/2011-06/msg01081.html
 
Submitting as enhancement request.  Patch attached.
 
Here is another patch for Emacs help functions.  It handles
`describe-mode', `describe-function', and `describe-variable',
and the same feature it provides can be applied to other help
commands where appropriate.
 
What is the feature?  Let users click a key description (i.e., a
key name, such as `C-f') in a buffer such as *Help* to see the
associated help.  This applies to key descriptions derived from
\[...] doc patterns (only).
 
`substitute-command-keys' converts a doc string that uses
patterns such as `\[COMMAND]' to COMMAND's key description.
For example, it might convert `\[mouse-yank-secondary]' to
`<M-mouse-2>'.  It interprets COMMAND's bindings relative to a
given KEYMAP if that `\[...]' pattern is preceded by a
`\<KEYMAP>' pattern.
 
Once this conversion is done, however, the COMMAND and its KEYMAP
are lost, missing from the resulting doc string.  More
importantly, this information is lost to the code that uses the
conversion result - the code for `describe-variable' etc.
 
(Yes, the original string might be available to the calling code,
but that code would need to parse it to figure out the
correspondence between COMMAND and resulting key description.
IOW, it would need to do much of what `substitute-command-keys'
already does.)
 
The user sees only the description `<M-mouse-2>' in buffer *Help*
where the substituted doc string is used.  What if s?he wants to
know more about what that key does in the documented context?
S?he can hit `C-h k' and then try the key, but that will work
only if the current mode has the same binding (e.g., with KEYMAP
active for COMMAND).
 
The aim of the feature provided by the attached patch is to
replace such simple key descriptions, which were derived from
rich info (COMMAND and KEYMAP) about a documented context, with
help buttons that use that info to link to descriptions of the
commands associated with the keys.
 
The feature has the effect of giving `substitute-command-keys' an
optional arg that, when non-nil, makes that function not only
substitute the key description for the command but also buttonize
the key-description part of the result string.
 
The effect is that if the result string is inserted in a buffer
(e.g. *Help*) then the user can click or hit `RET' on the key
description (e.g. `<M-mouse-2>') to get help on the associated
command (e.g. `mouse-yank-secondary').
 
Actually, I did not patch the `substitute-command-keys' C code to
give it an extra arg for this.  Instead, I wrote a Lisp version:
`help-substitute-command-keys'.  (For the part that handles
\{...}, this function just calls `substitute-command-keys'.)  If
someone wants to patch the C code instead, go for it.
 
Similarly, I wrote Lisp wrappers for `documentation' and
`documentation-property' that accept the same optional arg and
pass it to `help-substitute-command-keys'.
 
Then, in the body of commands `describe-variable' etc., I changed
`(documentation X)' to `(help-documentation nil t)' to get the
buttonized help string.
 
Give it a try.  If you decide to go for the idea then you might
prefer to just add the optional arg to `substitute-command-keys'
rather than creating a separate function.  (I didn't have that
choice as a Lisp user.)
 
However, even in that case you might decide to opt for a Lisp
version (why not?) - IOW, move `substitute-command-keys' to Lisp.
 
You can do that by renaming `substitute-command-keys' to
<something>, removing most of the <something> code, and renaming
`help-substitute-command-keys' to `substitute-command-keys'.  A
small amount of the <something> C code is still needed to handle
the \{..} case (no buttons).
 
If you want to try it without applying the patches, you can just
download this library and load it:
http://www.emacswiki.org/emacs/download/help-fns%2b.el.  (In that
case you will also see the automatic Info links discussed in
thread "adding manual cross-ref links to *Help*":
http://lists.gnu.org/archive/html/emacs-devel/2011-06/msg00368.html)
 
In GNU Emacs 24.0.50.1 (i386-mingw-nt5.1.2600)
 of 2011-06-27 on 3249CTO
Windowing system distributor `Microsoft Corp.', version 5.1.2600
configured using `configure --with-gcc (4.5) --no-opt --cflags
-Ic:/build/include'
 

[-- Attachment #2: help-2011-06-27.patch --]
[-- Type: application/octet-stream, Size: 9109 bytes --]

diff -c -w help.el help-patched-2011-06-27.el
*** help.el	Mon Jun 27 09:07:04 2011
--- help-patched-2011-06-27.el	Mon Jun 27 09:36:14 2011
***************
*** 251,256 ****
--- 251,390 ----
    help-map)
  
  \f
+ (defun help-documentation (function &optional raw add-help-buttons)
+   "Same as `documentation', 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."
+   (let ((raw-doc  (documentation function 'RAW)))
+     (if raw  raw-doc  (help-substitute-command-keys raw-doc add-help-buttons))))
+     
+ (defun help-documentation-property (symbol prop &optional raw add-help-buttons)
+   "Same as `documentation-property', 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."
+   (let ((raw-doc  (documentation-property symbol prop 'RAW)))
+     (if raw  raw-doc  (help-substitute-command-keys raw-doc add-help-buttons))))
+ 
+ (defun help-commands-to-key-buttons (string)
+   "Like `substitute-command-keys', but adds buttons for help on keys.
+   Key descriptions become links to help about their commands."
+   (help-substitute-command-keys string 'ADD-HELP-BUTTONS))
+ 
+ (defun help-substitute-command-keys (string &optional add-help-buttons)
+   "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."
+ 
+   ;; REPEAT:
+   ;;  Search for first occurrence of any of the patterns: \[...], \{...}, or \<...>.
+   ;;  Handle escaping via \=, if present before the pattern.
+   ;;  If pattern is a keymap (\<...>): use it from then on.
+   ;;  If pattern is a command (\[...]): (a) substitute its key description, (b) put a button on it.
+   ;;  If pattern is a bindings spec (\{...}): just substitute the usual text.
+   (with-syntax-table emacs-lisp-mode-syntax-table
+     (let* ((strg          (copy-sequence string))
+            (len-strg      (length strg))
+            (ii            0)
+            (jj            0)
+            (newstrg       "")
+            (re-command    "\\\\\\[\\(\\(\\sw\\|\\s_\\)+\\)\\]")
+            (re-keymap     "\\\\<\\(\\(\\sw\\|\\s_\\)+\\)>")
+            (re-bindings   "\\\\{\\(\\(\\sw\\|\\s_\\)+\\)}")
+            (re-any        (concat "\\(" re-command  "\\|" re-keymap "\\|" re-bindings "\\)"))
+            (keymap        (or overriding-terminal-local-map overriding-local-map))
+            (msg           nil)
+            key bindings ma mc mk mb)
+       (while (< ii len-strg)
+         (setq key       nil
+               bindings  nil
+               strg      (substring strg ii))
+         (save-match-data                ; ANY
+           (setq ma  (string-match re-any strg))
+           (if (not ma)
+               (setq newstrg  (concat newstrg strg)
+                     ii       len-strg
+                     jj       len-strg)
+             (let ((escaped  nil)
+                   (odd      nil))
+               (save-match-data
+                 (let ((ma1  ma))
+                   (setq ii  ma)
+                   (while (string-match "\\\\=$" (substring strg 0 ma1))
+                     (setq odd  (not odd)
+                           ma1  (match-beginning 0))
+                     (when odd
+                       (setq ii       (- ii 2)
+                             escaped  ma1)))))
+               (if (not escaped)
+                   (setq ii       ma
+                         jj       (match-end 0)
+                         ma       (match-string-no-properties 0 strg)
+                         newstrg  (concat newstrg (substring strg 0 ii)))
+                 (setq jj       (match-end 0) ; End of \[...], \{...}, or \<...>
+                       newstrg  (if odd
+                                    (concat newstrg
+                                            (substring strg 0 ii) ; Unescaped \='s
+                                            (substring strg ma jj)) ; \[...], \{...}, or \<...>
+                                  (concat newstrg (substring strg 0 ii)))
+                       ma       (if odd nil (match-string-no-properties 0 strg))
+                       ii       jj)))))
+         (when ma
+           (save-match-data              ; KEYMAP
+             (setq ma  (copy-sequence ma))
+             (setq mk  (string-match re-keymap ma))
+             (setq mk  (and mk (match-string-no-properties 0 ma)))
+             (when mk
+               (setq keymap  (intern (match-string-no-properties 1 ma)))
+               (if (boundp keymap)
+                   (setq keymap  (symbol-value keymap))
+                 (setq msg  (format "\nUses keymap \"%s\", which is not currently defined.\n"
+                                    keymap))
+                 (setq keymap  (or overriding-terminal-local-map overriding-local-map)))))
+           (unless mk                    ; COMMAND
+             (save-match-data
+               (setq ma  (copy-sequence ma))
+               (setq mc  (string-match re-command ma))
+               (setq mc  (and mc (match-string-no-properties 0 ma)))
+               (setq mc  (and mc (intern (substring mc 2 -1)))) ; Remove \[...] envelope
+               (when mc
+                 (let ((follow-remap  t))
+                   (while (and (setq key  (where-is-internal mc keymap 'FIRSTONLY))
+                               (vectorp key) (> (length key) 1) (eq 'remap (aref key 0))
+                               (symbolp (aref key 1)) follow-remap)
+                     (setq mc            (aref key 1)
+                           follow-remap  nil)))
+                 (setq key  (if key (key-description key) (concat "M-x " (symbol-name mc))))
+                 (when add-help-buttons (setq key  (help-key-button-string key mc))))))
+           (unless (or mk mc)            ; BINDINGS
+             (save-match-data
+               (setq ma  (copy-sequence ma))
+               (setq mb  (string-match re-bindings ma))
+               (setq mb  (and mb (match-string-no-properties 0 ma)))
+               (when mb
+                 (setq bindings  (intern (match-string-no-properties 1 ma)))
+                 (cond ((boundp bindings)
+                        (setq bindings  (substitute-command-keys mb))) ; Use original - no buttons.
+                       (t
+                        (setq msg  (format "\nUses keymap \"%s\", which is not currently defined.\n"
+                                           bindings))
+                        (setq bindings  nil))))))
+           (unless mk
+             (setq newstrg  (concat newstrg (or key bindings (substring strg ii jj)))))
+           (setq ii  (or jj len-strg))))
+       (if (string= string newstrg)
+           string  ; Return original string, not a copy, if no changes.
+         newstrg))))
+ 
+ (defun help-key-button-string (key-description command)
+   "Return a button for KEY-DESCRIPTION that links to the COMMAND description.
+ KEY-DESCRIPTION is a key-description string.
+ COMMAND is the command (a symbol) associated with the key described.
+ Return a copy of string KEY-DESCRIPTION with button properties added.
+ Clicking the button shows the help for COMMAND."
+   (let ((new-key  (copy-sequence key-description)))
+     (make-text-button new-key nil 'button (list t) ':type 'help-function 'help-args (list command))
+     new-key))
+ \f
  
  (defun function-called-at-point ()
    "Return a function around point or else called by the list containing point.
***************
*** 852,858 ****
  				     "no indicator"
  				   (format "indicator%s"
  					   indicator))))
! 		  (princ (documentation mode-function)))
  		(insert-button pretty-minor-mode
  			       'action (car help-button-cache)
  			       'follow-link t
--- 986,993 ----
  				     "no indicator"
  				   (format "indicator%s"
  					   indicator))))
!                   (with-current-buffer standard-output
!                     (insert (help-documentation mode-function nil 'ADD-HELP-BUTTONS))))
  		(insert-button pretty-minor-mode
  			       'action (car help-button-cache)
  			       'follow-link t
***************
*** 880,886 ****
  		(re-search-backward "`\\([^`']+\\)'" nil t)
  		(help-xref-button 1 'help-function-def mode file-name)))))
  	(princ ":\n")
! 	(princ (documentation major-mode)))))
    ;; For the sake of IELM and maybe others
    nil)
  
--- 1015,1022 ----
  		(re-search-backward "`\\([^`']+\\)'" nil t)
  		(help-xref-button 1 'help-function-def mode file-name)))))
  	(princ ":\n")
!         (with-current-buffer standard-output
!           (insert (help-documentation major-mode nil 'ADD-HELP-BUTTONS))))))
    ;; For the sake of IELM and maybe others
    nil)
  

Diff finished.  Mon Jun 27 09:53:41 2011

[-- Attachment #3: help-fns-2011-06-27.patch --]
[-- Type: application/octet-stream, Size: 2621 bytes --]

diff -c -w help-fns.el help-fns-patched-2011-06-27.el
*** help-fns.el	Mon Jun 27 09:08:20 2011
--- help-fns-patched-2011-06-27.el	Mon Jun 27 09:51:14 2011
***************
*** 527,533 ****
        (let* ((advertised (gethash def advertised-signature-table t))
  	     (arglist (if (listp advertised)
  			  advertised (help-function-arglist def)))
! 	     (doc (condition-case err (documentation function)
                      (error (format "No Doc! %S" err))))
  	     (usage (help-split-fundoc doc function)))
  	(with-current-buffer standard-output
--- 527,533 ----
        (let* ((advertised (gethash def advertised-signature-table t))
  	     (arglist (if (listp advertised)
  			  advertised (help-function-arglist def)))
! 	     (doc (condition-case err (help-documentation function nil t)
                      (error (format "No Doc! %S" err))))
  	     (usage (help-split-fundoc doc function)))
  	(with-current-buffer standard-output
***************
*** 545,551 ****
  			    (while (and (symbolp fun)
  					(setq fun (symbol-function fun))
  					(not (setq usage (help-split-fundoc
! 							  (documentation fun)
  							  function)))))
  			    usage)
  			  (car usage))
--- 545,551 ----
  			    (while (and (symbolp fun)
  					(setq fun (symbol-function fun))
  					(not (setq usage (help-split-fundoc
! 							  (help-documentation fun nil t)
  							  function)))))
  			    usage)
  			  (car usage))
***************
*** 786,793 ****
                     (obsolete (get variable 'byte-obsolete-variable))
  		   (use (car obsolete))
  		   (safe-var (get variable 'safe-local-variable))
!                    (doc (or (documentation-property variable 'variable-documentation)
!                             (documentation-property alias 'variable-documentation)))
                     (extra-line nil))
                ;; Add a note for variables that have been make-var-buffer-local.
                (when (and (local-variable-if-set-p variable)
--- 786,793 ----
                     (obsolete (get variable 'byte-obsolete-variable))
  		   (use (car obsolete))
  		   (safe-var (get variable 'safe-local-variable))
!                    (doc (or (help-documentation-property variable 'variable-documentation nil t)
!                             (help-documentation-property alias 'variable-documentation nil t)))
                     (extra-line nil))
                ;; Add a note for variables that have been make-var-buffer-local.
                (when (and (local-variable-if-set-p variable)

Diff finished.  Mon Jun 27 09:52:12 2011

bug#8951: 24.0.50; [PATCH] enhancement request: buttonize key names

[Stefan Monnier]

> What is the feature?  Let users click a key description (i.e., a
> key name, such as `C-f') in a buffer such as *Help* to see the
> associated help.  This applies to key descriptions derived from
> \[...] doc patterns (only).

That looks like a good idea.  Have you tried to plug it directly into
substitute-command-keys instead?  Are there places where such buttons
become annoying?

Basically, the thing I don't like about your patch is the resulting
redundancy between help-substitute-command-keys and
substitute-command-keys, which could be removed by getting rid of the
C version and only using the new Elisp version.


        Stefan

bug#8951: 24.0.50; [PATCH] enhancement request: buttonize key names

[Drew Adams]

> > Let users click a key description (i.e., a key name, such as
> > `C-f') in a buffer such as *Help* to see the associated help.
> > This applies to key descriptions derived from \[...] doc
> > patterns (only).
> 
> That looks like a good idea.  Have you tried to plug it directly into
> substitute-command-keys instead?  Are there places where such buttons
> become annoying?
> 
> Basically, the thing I don't like about your patch is the resulting
> redundancy between help-substitute-command-keys and
> substitute-command-keys, which could be removed by getting rid of the
> C version and only using the new Elisp version.

See the emacs-devel thread, where I addressed both of these things.
http://lists.gnu.org/archive/html/emacs-devel/2011-06/msg01081.html

I wrote Lisp, but if someone wants to instead patch the C code for
`substitute-command-keys' then go for it.

The Lisp version I wrote still invokes the original C code for the \\{...} case.
I did not try to rewrite that in Lisp.

IMO, the best solution would be to:

1. Keep the Lisp code I wrote (or similar), renaming it to
`substitute-command-keys'.

2. Simplify the original C code to handle just the \\{...} case, rename that
function, and use it in #1 to handle the \\{...} parts (just as now, but under
its new, {}-specific name).

Alternatively, you can write #2 in Lisp, if you like.


Wrt your question of whether "there are places where such buttons become
annoying": I would say that it does not matter whether there are currently any
such places.  There is no reason not to treat the buttonizing as optional.

`substitute-command-keys' produces a string from a string - typically doc.  But
the use of the output string need not be for a *Help* buffer (or similar context
where buttons are appropriate).  There is no reason to always buttonize these
parts of the output string.  Making buttonizing optional provides flexibility,
at no cost.  Nothing would be gained by making it mandatory.

bug#8951: 24.0.50; [PATCH] enhancement request: buttonize key names

[Stefan Monnier]

>> Basically, the thing I don't like about your patch is the resulting
>> redundancy between help-substitute-command-keys and
>> substitute-command-keys, which could be removed by getting rid of the
>> C version and only using the new Elisp version.

> See the emacs-devel thread, where I addressed both of these things.
> http://lists.gnu.org/archive/html/emacs-devel/2011-06/msg01081.html

I don't think that quite addresses it.  But, I guess it does indirectly:
you apparently haven't found problematic cases that require
distinguishing help-substitute-command-keys from
substitute-command-keys.  Instead you just personally prefer to keep the
option whether to buttonize or not.

> I wrote Lisp, but if someone wants to instead patch the C code for
> `substitute-command-keys' then go for it.

No, I'm not interested in making this C code more complicated.  I want
to go the other way around.

> The Lisp version I wrote still invokes the original C code for the
> \\{...} case.  I did not try to rewrite that in Lisp.

I see.  It should be pretty easy to do, easy exporting the needed
underlying C function to Elisp, or rewriting it in Elisp (there's no
good reason to have describe-buffer-bindings written in C, really).

> 1. Keep the Lisp code I wrote (or similar), renaming it to
> `substitute-command-keys'.

Sounds good.

> 2. Simplify the original C code to handle just the \\{...} case,
> rename that function, and use it in #1 to handle the \\{...} parts
> (just as now, but under its new, {}-specific name).

Sounds good.

> Alternatively, you can write #2 in Lisp, if you like.

Sounds good as well.

> Wrt your question of whether "there are places where such buttons
> become annoying": I would say that it does not matter whether there
> are currently any such places.  There is no reason not to treat the
> buttonizing as optional.

Of course there's the reason that providing a choice is never free, so
we should only provide the choice if there's a good reason for it.


        Stefan

bug#8951: 24.0.50; [PATCH] enhancement request: buttonize key names

[Drew Adams]

> >> getting rid of the C version and only using the new Elisp version.
> > See the emacs-devel thread, where I addressed both of these things.
> > http://lists.gnu.org/archive/html/emacs-devel/2011-06/msg01081.html
> 
> I don't think that quite addresses it.  But, I guess it does 
> indirectly: you apparently haven't found problematic cases
> that require distinguishing help-substitute-command-keys from
> substitute-command-keys.  Instead you just personally prefer 
> to keep the option whether to buttonize or not.

Yes, I suggested that `substitute-command-keys' itself be modified to accept the
optional argument.  I did not try to replace `substitute-command-keys' directly
with a version that has the optional arg to buttonize, because (a) I didn't want
to hack the C code and (b) I didn't want to bother writing the \\{...} code in
Lisp.

> > I wrote Lisp, but if someone wants to instead patch the C code for
> > `substitute-command-keys' then go for it.
> 
> No, I'm not interested in making this C code more complicated.  I want
> to go the other way around.

Glad to hear that.  That's my preference too.

> > The Lisp version I wrote still invokes the original C code for the
> > \\{...} case.  I did not try to rewrite that in Lisp.
> 
> I see.  It should be pretty easy to do, easy exporting the needed
> underlying C function to Elisp, or rewriting it in Elisp (there's no
> good reason to have describe-buffer-bindings written in C, really).

Please, go for it.  I agree.

> > 1. Keep the Lisp code I wrote (or similar), renaming it to
> > `substitute-command-keys'.
> 
> Sounds good.
> 
> > 2. Simplify the original C code to handle just the \\{...} case,
> > rename that function, and use it in #1 to handle the \\{...} parts
> > (just as now, but under its new, {}-specific name).
> 
> Sounds good.
> 
> > Alternatively, you can write #2 in Lisp, if you like.
> 
> Sounds good as well.
> 
> > Wrt your question of whether "there are places where such buttons
> > become annoying": I would say that it does not matter whether there
> > are currently any such places.  There is no reason not to treat the
> > buttonizing as optional.
> 
> Of course there's the reason that providing a choice is never free, so
> we should only provide the choice if there's a good reason for it.

I gave this reason to make it optional:

\\[], \\<>, \\{} are about mapping command names to corresponding key
descriptions - nothing more.  That in itself does not say anything about help
buffers and interactivity (clicking, RET).  The function is just a string
transducer.

The common use case for that is of course doc.  And the common use case for the
doc use case is a help buffer.

But a string of doc is more general than a help buffer, both in terms of string
vs buffer and in terms of the need for button text properties (interactivity).
There could be applications of this function to produce a readable string that
do not involve a button-clicking interactive context.

"providing a choice is never free" -

What's the cost?  And what's the cost of always doing the extra lifting of
creating buttons?

Code maintenance will be at least as easy if it is optional, possibly easier
since the additional buttonizing part is well separated.  And not creating
buttons (properties) saves a little time and memory (but that's probably not
very significant).

No, I don't feel strongly about making buttonizing optional.  But I don't see
why you wouldn't do that.

bug#8951: 24.0.50; [PATCH] enhancement request: buttonize key names

[Stefan Monnier]

>> > The Lisp version I wrote still invokes the original C code for the
>> > \\{...} case.  I did not try to rewrite that in Lisp.
>> I see.  It should be pretty easy to do, easy exporting the needed
>> underlying C function to Elisp, or rewriting it in Elisp (there's no
>> good reason to have describe-buffer-bindings written in C, really).
> Please, go for it.  I agree.

Please send an updated patch so we can install it in the `pending' branch.


        Stefan

bug#8951: 24.0.50; [PATCH] enhancement request: buttonize key names

[Drew Adams]

> >> > The Lisp version I wrote still invokes the original C 
> >> > code for the \\{...} case.  I did not try to rewrite
> >> > that in Lisp.
> >>
> >> I see.  It should be pretty easy to do, easy exporting the needed
> >> underlying C function to Elisp, or rewriting it in Elisp 
> >> (there's no good reason to have describe-buffer-bindings
> >> written in C, really).
> >
> > Please, go for it.  I agree.
> 
> Please send an updated patch so we can install it in the 
> `pending' branch.

Do you mean just redo what I did using the latest source code for the diff?  I
would be glad to do that, if that's all you meant.  Let me know.

For the rest (migration from C, changing C code, etc.), someone else will need
to do it.

bug#8951: 24.0.50; [PATCH] enhancement request: buttonize key names

[Lars Ingebrigtsen]

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

>> >> > The Lisp version I wrote still invokes the original C 
>> >> > code for the \\{...} case.  I did not try to rewrite
>> >> > that in Lisp.
>> >>
>> >> I see.  It should be pretty easy to do, easy exporting the needed
>> >> underlying C function to Elisp, or rewriting it in Elisp 
>> >> (there's no good reason to have describe-buffer-bindings
>> >> written in C, really).
>> >
>> > Please, go for it.  I agree.
>> 
>> Please send an updated patch so we can install it in the 
>> `pending' branch.
>
> Do you mean just redo what I did using the latest source code for the diff?  I
> would be glad to do that, if that's all you meant.  Let me know.
>
> For the rest (migration from C, changing C code, etc.), someone else will need
> to do it.

I'm guessing that Stefan asked you to rewrite substitute-command-keys in
Lisp.

It makes sense to do that, I think, and it seems like a nice little
project that somebody should undertake.

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

bug#44909: Hyperlinks gone for first story of two-storied *Help* buffers

[積丹尼 Dan Jacobson]

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

Look at this weird *Help* screen,


[-- Attachment #2: Ugly --]
[-- Type: image/png, Size: 25910 bytes --]

[-- Attachment #3: Type: text/plain, Size: 1200 bytes --]



Note the four non-hyperlinked items?

> How did you get that, Jacobson?
> I can't get such a two-storied *Help* buffer with either C-h f nor C-h v.
> Are you taking the wrong prescption drugs?

It's a long story:
$ emacs -Q
 C-h f			;; describe-function
 s			;; self-insert-command
 o			;; self-insert-command
 -			;; self-insert-command
 l			;; self-insert-command
 o			;; self-insert-command
 n			;; self-insert-command
 g			;; self-insert-command
 <return>		;; minibuffer-complete-and-exit
 C-x o			;; other-window
 <tab>			;; forward-button
 <tab>			;; forward-button
 <return>		;; push-button
 C-x 1			;; delete-other-windows
 C-h l			;; view-lossage

Yes, the cursor was on top of
‘global-so-long-mode’
when I did push-button.

Because global-so-long-mode is both a function and a variable, I get the
two-storied *Help* window.

But whoever invented two-storied *Help* windows didn't make sure all the
hyperlinks are hyperlinked.

Sure they are when you do
(describe-function 'global-so-long-mode)
(describe-variable 'global-so-long-mode)
individually. But not when you do them "both at the same time" via the
push-button.

emacs-version "27.1"

bug#44909: Hyperlinks gone for first story of two-storied *Help* buffers

[Eli Zaretskii]

> From: 積丹尼 Dan Jacobson
>  <jidanni@jidanni.org>
> Date: Sat, 28 Nov 2020 03:13:54 +0800
> 
> But whoever invented two-storied *Help* windows didn't make sure all the
> hyperlinks are hyperlinked.

They are not hyperlinks, they are names of commands.

Closing.

bug#44909: Hyperlinks gone for first story of two-storied *Help* buffers

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

> > But whoever invented two-storied *Help* windows didn't make sure all the
> > hyperlinks are hyperlinked.
>
> They are not hyperlinks, they are names of commands.

I think this is similar to the feature request in Bug#8951.

bug#8951: bug#44909: Hyperlinks gone for first story of two-storied *Help* buffers

[積丹尼 Dan Jacobson]

Eli Zaretskii <eliz@gnu.org> writes:
> They are not hyperlinks, they are names of commands.

I see. In
https://debbugs.gnu.org/cgi/bugreport.cgi?msg=5;filename=so-bad.png;bug=44909;att=1

The first two
Use M-x so-long-commentary for more information.
Use M-x so-long-customize to configure the behaviour.
should continue not to be hyperlinked.

Unlike the second two
Use M-x so-long-commentary for more information.
Use M-x so-long-customize to configure the behaviour.

Wonder what RMS thinks.

bug#44909: Hyperlinks gone for first story of two-storied *Help* buffers

[Lars Ingebrigtsen]

Stefan Kangas <stefan@marxist.se> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>> > But whoever invented two-storied *Help* windows didn't make sure all the
>> > hyperlinks are hyperlinked.
>>
>> They are not hyperlinks, they are names of commands.
>
> I think this is similar to the feature request in Bug#8951.

Yup.  I've now merged them.

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

bug#8951: 24.0.50; [PATCH] enhancement request: buttonize key names

[Stefan Kangas]

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

tags 8951 + patch
thanks

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> What is the feature?  Let users click a key description (i.e., a
>> key name, such as `C-f') in a buffer such as *Help* to see the
>> associated help.  This applies to key descriptions derived from
>> \[...] doc patterns (only).
>
> That looks like a good idea.  Have you tried to plug it directly into
> substitute-command-keys instead?  Are there places where such buttons
> become annoying?

(I have had a patch sitting around for a while, but it failed bootstrap.
It turns out that the fix was a simple `fboundp', heh.)

The attached patch adds a new option `help-mode--add-function-link' that
when non-nil makes `substitute-command-keys' add a link to the
`describe-function' for the bound command when inserting keys.

I have reconsidered my previously held opinion that this should be off
by default.  From using this patch, I have come to the conclusion that
this position is wrong, as this is in fact highly useful in places such
as `M-x ibuffer RET C-h m' and on many other help screen besides.  The
only adverse effect of enabling it, furthermore, is that you might have
to hit TAB a couple of times more on some help screens.  So I think
having it on by default is very much a good thing.

[-- Attachment #2: 0001-Add-new-option-help-link-key-to-documentation.patch --]
[-- Type: text/x-diff, Size: 3425 bytes --]

From b4bf34acaba98ddeae6f97355b67da5d2ac13e91 Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefan@marxist.se>
Date: Fri, 5 Mar 2021 02:49:07 +0100
Subject: [PATCH] Add new option help-link-key-to-documentation

* lisp/help.el (help-link-key-to-documentation): New user option.
(substitute-command-keys): Add link from keys to the command they
are bound to if the above new option is non-nil.  (Bug#8951)
* etc/NEWS: Announce the new option.
---
 etc/NEWS          |  8 ++++++++
 lisp/help-mode.el |  7 +++++++
 lisp/help.el      | 17 ++++++++++++++++-
 3 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/etc/NEWS b/etc/NEWS
index 36d04aa2d8..acb6c653db 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -136,6 +136,14 @@ function which returns a string.  For the first two cases, the length
 of the resulting name is controlled by 'eww-buffer-name-length'.  By
 default, no automatic renaming is performed.
 
+** help
+
+*** New user option `help-link-key-to-documentation'.
+When this option is non-nil, key bindings displayed in the *Help*
+buffer will be linked to the documentation for the command they are
+bound to.  This does not affect listings of key bindings and
+functions.
+
 ** image-dired
 
 ---
diff --git a/lisp/help-mode.el b/lisp/help-mode.el
index d61b1bdc62..53acbf97e7 100644
--- a/lisp/help-mode.el
+++ b/lisp/help-mode.el
@@ -376,6 +376,13 @@ 'help-news
     (view-buffer-other-window (find-file-noselect file))
     (goto-char pos))
   'help-echo (purecopy "mouse-2, RET: show corresponding NEWS announcement"))
+
+;;;###autoload
+(defun help-mode--add-function-link (str fun)
+  (make-text-button (copy-sequence str) nil
+                    'type 'help-function
+                    'help-args (list fun)))
+
 \f
 (defvar bookmark-make-record-function)
 (defvar help-mode--current-data nil)
diff --git a/lisp/help.el b/lisp/help.el
index a5d5037bfe..03c5b8e230 100644
--- a/lisp/help.el
+++ b/lisp/help.el
@@ -1058,6 +1058,14 @@ lookup-minor-mode-from-indicator
     result))
 
 \f
+(defcustom help-link-key-to-documentation t
+  "Non-nil means link keys to their command in *Help* buffers.
+This affects \\\\=\\[command] substitutions in documentation
+strings done by `substitute-command-keys'."
+  :type 'boolean
+  :version "29.1"
+  :group 'help)
+
 (defun substitute-command-keys (string)
   "Substitute key descriptions for command names in STRING.
 Each substring of the form \\\\=[COMMAND] is replaced by either a
@@ -1145,7 +1153,14 @@ substitute-command-keys
                         (delete-char 1))
                     ;; Function is on a key.
                     (delete-char (- end-point (point)))
-                    (insert (help--key-description-fontified key)))))
+                    (let ((key (help--key-description-fontified key)))
+                      (insert (if (and help-link-key-to-documentation
+                                       (functionp fun))
+                                  ;; The `fboundp' fixes bootstrap:
+                                  (if (fboundp 'help-mode--add-function-link)
+                                      (help-mode--add-function-link key fun)
+                                    key)
+                                key))))))
                ;; 1D. \{foo} is replaced with a summary of the keymap
                ;;            (symbol-value foo).
                ;;     \<foo> just sets the keymap used for \[cmd].
-- 
2.30.2

bug#8951: 24.0.50; [PATCH] enhancement request: buttonize key names

[Lars Ingebrigtsen]

Stefan Kangas <stefan@marxist.se> writes:

> The attached patch adds a new option `help-mode--add-function-link' that
> when non-nil makes `substitute-command-keys' add a link to the
> `describe-function' for the bound command when inserting keys.

Looks like a good idea to me.

> I have reconsidered my previously held opinion that this should be off
> by default.  From using this patch, I have come to the conclusion that
> this position is wrong, as this is in fact highly useful in places such
> as `M-x ibuffer RET C-h m' and on many other help screen besides.  The
> only adverse effect of enabling it, furthermore, is that you might have
> to hit TAB a couple of times more on some help screens.  So I think
> having it on by default is very much a good thing.

Yes, me too.  We can try it out, at least, and see how it works in
practice, but it sounds really useful to me.

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

bug#8951: 24.0.50; [PATCH] enhancement request: buttonize key names

[Stefan Kangas]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Yes, me too.  We can try it out, at least, and see how it works in
> practice, but it sounds really useful to me.

Right, and if it turns out that people hate it, it is trivial to flip
the switch to nil before Emacs 29.

bug#8951: 24.0.50; [PATCH] enhancement request: buttonize key names

[Lars Ingebrigtsen]

Stefan Kangas <stefan@marxist.se> writes:

> Right, and if it turns out that people hate it, it is trivial to flip
> the switch to nil before Emacs 29.

Yup.

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

bug#8951: 24.0.50; [PATCH] enhancement request: buttonize key names

[Stefan Kangas]

close 8951 29.1
thanks

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Stefan Kangas <stefan@marxist.se> writes:
>
>> Right, and if it turns out that people hate it, it is trivial to flip
>> the switch to nil before Emacs 29.
>
> Yup.

Now pushed to master (commit 68d11cc248).

bug#8951: [External] : Re: bug#8951: 24.0.50; [PATCH] enhancement request: buttonize key names

[Drew Adams]

> >> What is the feature?  Let users click a key description (i.e., a
> >> key name, such as `C-f') in a buffer such as *Help* to see the
> >> associated help.  This applies to key descriptions derived from
> >> \[...] doc patterns (only).
> 
> The attached patch adds a new option `help-mode--add-function-link'
> that when non-nil makes `substitute-command-keys' add a link to the
> `describe-function' for the bound command when inserting keys.
> 
> I have reconsidered my previously held opinion that this should be off
> by default.  From using this patch, I have come to the conclusion that
> this position is wrong, as this is in fact highly useful in places such
> as `M-x ibuffer RET C-h m' and on many other help screen besides.  The
> only adverse effect of enabling it, furthermore, is that you might have
> to hit TAB a couple of times more on some help screens.  So I think
> having it on by default is very much a good thing.

Glad you decided this is worthwhile, after all.

You seem to have ignored this, from https://debbugs.gnu.org/cgi/bugreport.cgi?bug=8951#17:

 I gave this reason to make it optional:

 \\[], \\<>, \\{} are about mapping command names to
 corresponding key descriptions - nothing more.  That
 in itself does not say anything about help buffers
 and interactivity (clicking, RET).  The function is
 just a string transducer.

 The common use case for that is of course doc.  And
 the common use case for the doc use case is a help buffer.

 But a string of doc is more general than a help buffer,
 both in terms of string vs buffer and in terms of the
 need for button text properties (interactivity).  There
 could be applications of this function to produce a
 readable string that do not involve a button-clicking
 interactive context.

Instead of making the choice of whether to fontify/link
key descriptions (1) depend only on a user option
(dynamically scoped variable) and (2) hard-coupling it
to use of the result in a *Help* buffer, I argued for
(1) adding an optional arg to `substitute-command-keys'
(to make the choice), and (2) separating creation of the
resulting fontified/ linkified key description from its
use in *Help* output.

I think my approach is preferable.  I suggest it still.

bug#8951: [External] : Re: bug#8951: 24.0.50; [PATCH] enhancement request: buttonize key names

[Stefan Kangas]

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

> Instead of making the choice of whether to fontify/link
> key descriptions (1) depend only on a user option
> (dynamically scoped variable) and (2) hard-coupling it
> to use of the result in a *Help* buffer, I argued for
> (1) adding an optional arg to `substitute-command-keys'
> (to make the choice),

I don't see the use-case.  I know of no places where the added linking
is not desirable, if the user has already said that she wants it in the
help buffer.

> and (2) separating creation of the resulting fontified/ linkified key
> description from its use in *Help* output.

I agree with this, but as we have discussed elsewhere this goes further
and deeper than this option.  It would amount to a major rethinking or
rewrite of help.el itself.  I hope that we will do that, eventually.

bug#8951: [External] : Re: bug#8951: 24.0.50; [PATCH] enhancement request: buttonize key names

[Drew Adams]

> > Instead of making the choice of whether to fontify/link
> > key descriptions (1) depend only on a user option
> > (dynamically scoped variable) and (2) hard-coupling it
> > to use of the result in a *Help* buffer, I argued for
> > (1) adding an optional arg to `substitute-command-keys'
> > (to make the choice),
> 
> I don't see the use-case.  I know of no places where the added linking
> is not desirable, if the user has already said that she wants it in the
> help buffer.

I'm suggesting it's not only a question of the help buffer,
and not only a question of a user option.  It's about code
that can optionally fontify/link key descriptions.  That
code can be used for *Help*, but it can be used elsewhere
as well.

`substitute-command-keys' is a general function; it's not
used only for *Help* output.  Why not give it an optional
arg to do this font/linkifying?

> > and (2) separating creation of the resulting fontified/
> > linkified key description from its use in *Help* output.
> 
> I agree with this, but as we have discussed elsewhere this goes further
> and deeper than this option.  It would amount to a major rethinking or
> rewrite of help.el itself.  I hope that we will do that, eventually.

I don't know what you mean there, or what other discussion
you're referring to.  See above; the two are related.

`substitute-command-keys' is such a function: it takes a
string and returns a string.  Give it an optional arg to
have the returned string have the properties needed.

This is what the code I use does.  See the patch at the
outset of this bug thread.  (Of course, as discussed in
this thread it will also be good to move everything to
Lisp.  But the separation I'm talking about is not
dependent on our having done that.)

bug#8951: [External] : Re: bug#8951: 24.0.50; [PATCH] enhancement request: buttonize key names

[Stefan Kangas]

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

> `substitute-command-keys' is a general function; it's not
> used only for *Help* output.  Why not give it an optional
> arg to do this font/linkifying?

I prefer not to, unless we have a use-case for it.  YAGNI.

bug#8951: [External] : Re: bug#8951: 24.0.50; [PATCH] enhancement request: buttonize key names

[Drew Adams]

> > `substitute-command-keys' is a general function; it's not
> > used only for *Help* output.  Why not give it an optional
> > arg to do this font/linkifying?
> 
> I prefer not to, unless we have a use-case for it.  YAGNI.

Grep even just the vanilla Emacs-Lisp sources for
`substitute-command-keys'.  It's not used only for
*Help*.  And then think about uses by other code,
besides vanilla Emacs.

What's the reason you prefer not to?  I don't
understand.  Maybe you have a good reason, but
I don't think you gave any (?).

bug#8951: [External] : Re: bug#8951: 24.0.50; [PATCH] enhancement request: buttonize key names

[Stefan Kangas]

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

> Grep even just the vanilla Emacs-Lisp sources for
> `substitute-command-keys'.  It's not used only for
> *Help*.  And then think about uses by other code,
> besides vanilla Emacs.

I only see a lot of places where we want whatever the user has
configured, none where we need an optional argument.

> What's the reason you prefer not to?  I don't
> understand.  Maybe you have a good reason, but
> I don't think you gave any (?).

I'm repeating myself here: I'm actually happy to add it, if we have a
use-case.  How about we try this for a start, and once users start using
this in the wild, they'll come back to us and say: hey I really want an
optional parameter here.  Then we think about if that makes sense for
their use-case, maybe it will, maybe it won't.  But at least we know
what we are trying to do.

The alternative is that we add stuff just because "maybe someone will
need it".  That leads to feature creep, bloat, and is just not a good
way to develop software.

bug#8951: [External] : Re: bug#8951: 24.0.50; [PATCH] enhancement request: buttonize key names

[Drew Adams]

> > Grep even just the vanilla Emacs-Lisp sources for
> > `substitute-command-keys'.  It's not used only for
> > *Help*.  And then think about uses by other code,
> > besides vanilla Emacs.
> 
> I only see a lot of places where we want whatever the user
> has configured, none where we need an optional argument.
>
> > What's the reason you prefer not to?  I don't
> > understand.  Maybe you have a good reason, but
> > I don't think you gave any (?).
> 
> I'm repeating myself here: 

Actually, though you may be repeating, you still
haven't given a reason for your preference.  It's
OK with me if you have no reason - just wanted to
understand, in case I was missing something.

> I'm actually happy to add it, if we have a
> use-case.  How about we try this for a start,
> and once users start using
> this in the wild, they'll come back to us and say: hey I really want an
> optional parameter here.  Then we think about if that makes sense for
> their use-case, maybe it will, maybe it won't.  But at least we know
> what we are trying to do.
> 
> The alternative is that we add stuff just because "maybe someone will
> need it".  That leads to feature creep, bloat, and is just not a good
> way to develop software.

An alternative is to not add a user option for
this just because maybe some user will need it.
You know - feature creep, bloat...

And to give users who write Lisp code the
ability to buttonize or not, wherever they use
`substitute-command-keys'.
___

That's what I did in help-fns+.el.  Never had
anyone request a user option to not have the
links.  Since 2011 - ten years in the wild.

Of course, I'd have had nothing against adding
an option for that, if users came back to me
and said: hey I really want a option for it.

In general, I'm not one who's particularly
opposed to dynamic global variables, or even to
binding them to affect behavior.  But in this
case I thought an arg was sufficient.
___

FWIW, you'll notice that there are several uses
of the original, vanilla `substitute-command-keys'
in help-fns+.el.  I don't recall the details, but
no doubt I thought those occurrences should not
buttonize key descriptions.

bug#8951: [External] : Re: bug#8951: 24.0.50; [PATCH] enhancement request: buttonize key names

[Stefan Kangas]

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

> Actually, though you may be repeating, you still
> haven't given a reason for your preference.  It's
> OK with me if you have no reason - just wanted to
> understand, in case I was missing something.

I have given my reason.  You may agree or disagree.

> That's what I did in help-fns+.el.  Never had
> anyone request a user option to not have the
> links.  Since 2011 - ten years in the wild.

Emacs has more users though.  But maybe we also don't need an option.
I have no strong opinion about that.