Subject: Re: bug#60587: Patch for adding links to symbols' help documentation

From: "H. Dieter Wilhelm" <[email-masked]>
Date: Fri, 27 Jan 2023 23:21:12 +0100

Thank you for your help.

Eli Zaretskii <[email-masked]> writes:

>> +  In Info buffers quoted symbol names are made into buttons which show
>> +the symbols' help documentation when typing @key{RET} or clicking
>> +@kbd{mouse-2} or @kbd{mouse-1} on it.  For example, the quoted name
>> +@code{info-other-window} is made into a button which shows the
>> +function's documentation string in another window, in the
>> +@file{*Help*} buffer.  Such quoted symbols (variables, functions and
>> +face names) are highlighted by a distinct face and can be reached, as
>> +the Info manual references, with @key{TAB} and @kbd{S-Tab}.
>
> This is fine, but please try to reword to not use the passive tense so
> much.

What do you think about the following formulation, is it already a
better "mix"?

  Info makes quoted symbol names into buttons which show the symbols'
help documentation when typing @key{RET} or clicking @kbd{mouse-2} or
@kbd{mouse-1} on it.  For example, the quoted name
@code{info-other-window} is made into a button which shows the
function's documentation string in another window, in the
@file{*Help*} buffer.  Info highlights such quoted symbols (variables,
functions and face names) by a distinct face and these can be reached,
as regular Info manual references, with @key{TAB} and @kbd{S-Tab}.

Drew reminded me that it should be better to write solely @kbd{mouse-2}
and / or possibly explaining `mouse-1-click-follows-link'.  I'm curious
about your opinion.

>> +;; The code below provides links of symbols (functions, variables, and
>> +;; faces) within Emacs' Info viewer to their builtin help
>> +;; documentation.  This linking is done when symbol names in texinfo
>> +;; documentation (like the Emacs- and Elisp manual) are:
>> +
>> +;; 1. Quoted symbol names like `quoted-symbol' or:
>
> You seem to have lost that ", or" in the rest of the items:

I wanted to express:
;; 1. Quoted symbol names like `quoted-symbol' or 2. (below).

;; 2. Function names which are prefixed by M-x, for example M-x

But you're right, since we are already in a numbered list, I dropped the
redundant phrase:

;; 1. Quoted symbol names like `quoted-symbol'.

;; 2. Function names which are prefixed by M-x, for example M-x

>> +                       (or (assoc-string (concat ifi ".info") ifiles)
>> +                           ;; the top info "dir" file
>> +                           (assoc-string (concat ifi ".info.gz") ifiles)
>> +                           ;; info files might be archived!
>> +                           (when pdir (string-match pdir ifile)))
>> +                       (not (assoc-string ifi ndocu)))))
>
> This should probably be extended to allow more extensions; see
> Info-suffix-list.

Thanks for hinting at Info-suffix-list. I'll try to accommodate the
other suffices in the code and will send another iteration of info.el
patches.

>> +(defface info-color
>> +  '((t (:inherit font-lock-doc-face
>
> Why not inherit from the 'link' face?

I wanted a different face compared to the regular Info links.  Because
they are acting differently!

But yesterday I realised that Info distinguishes (slightly) already
quoted symbols from the regular text.  I dropped now the info-color face
because the user can sufficiently distinguish between text, buttons and
Info links.

>> +(defvar-keymap info-button-map
>> +  :doc "Keymap used by buttons in Info buffers."
>> +  "RET" #'push-button
>> +  "<mouse-2>" #'push-button
>> +  "<follow-link>" 'mouse-face
>> +  ;; FIXME: You'd think that for keymaps coming from text-properties on the
>> +  ;; mode-line or header-line, the `mode-line' or `header-line' prefix
>> +  ;; shouldn't be necessary!
>> +  "<mode-line> <mouse-2>" #'push-button
>> +  "<header-line> <mouse-2>" #'push-button)
>
> Is this FXIME important enough to leave in the final version?

I really don't know, just copied the complete keymap code from
button.el.  (The comment is gone now.)

>> +(defun info-compile-emacs-info-dir-content ()
>> +  "Build a list of file names from Emacs' info directories.
>> +This function fills `info-emacs-info-dir-content' with files from
>> +`Info-directory-list'."
>> +  (setq info-emacs-info-dir-content
>> +        (mapcar 'file-name-nondirectory ;'file-name-sans-extension
>
> The comment should be removed, yes?

Yes, it's gone.

>> +(defun info-button (match-number type &rest args)
>> +  "Make a hyperlink for cross-reference text previously matched.
>> +MATCH-NUMBER is the subexpression of interest in the last matched
>> +regexp.  TYPE is the type of button to use.  Any remaining arguments are
>> +passed to the button's info-function when it is invoked.
>> +See `info-make-xrefs' Don't forget ARGS."
>                        ^^^
> A period is missing there.

Yes, a period and a whitespace ;-)

>> +  ;; Don't munge properties we've added, especially in some instances.
>> +  (unless (button-at (match-beginning match-number))
>> +    ;; (message "Creating button: %s." args)
>> +    (make-text-button (match-beginning match-number)
>> +                      (match-end match-number)
>> +                      'type type 'info-args args)))
>
> The "message" call in the comment should be removed.

Yes, I removed it now.

>> +
>> +(defvar info-symbol-context
>> +  '(( variable . "variable\\|option")
>> +    ( function . "function\\|command\\|call")
>> +    ( face . "face")
>> +    ( ignore . "symbol\\|program\\|property")
>
> Extra whitespace after an opening parenthesis.

Indeed, marked the whitespace column and C-x r t

>> +    ;; ignore symbols following this context type
>> +    ( definition . "source \\(?:code \\)?\\(?:of\\|for\\)"))
>> +  ;; function definitions in files

(changed also (definition ..) )

> These comments should be moved to _before_ the lines on which they
> comment.

Makes sense to be consistent 😅, improved that part.

>> +(defun info-make-xrefs (&optional buffer)
>> +  "Parse and hyperlink documentation cross-references in the given BUFFER.
>> +Find cross-reference information in a buffer and activate such
>> +cross references for selection with `help-follow'.  The current
>> +buffer is processed if the BUFFER argument is omitted.
>
> Our style for the last sentence is
>
>   BUFFER defaults to the current buffer if omitted or nil.
>
> I'd also move it to be the second sentence of the doc string.

I see, changed it in that way.

>> +Function names are also detected when prefixed by `M-x`, for
>> +example `M-x function-name` or are quoted and prefixed like `M-x
>> +function-name`.
>
> Please quote in doc strings `like this', not `like this`.  The latter
> is not supported by the Emacs Help features.

I'm sorry, when writing that piece I had Emacs wrongly configured
duplicating every ``' and didn't discern the difference.  ("Back" back
quotes are gone now.)

-- 
Best wishes
H. Dieter Wilhelm
Zwingenberg, Germany