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

[Eli Zaretskii <eliz@gnu.org>]

> From: "H. Dieter Wilhelm" <dieter@duenenhof-wilhelm.de>
> Date: Fri, 06 Jan 2023 20:03:23 +0100
> 
> I attached a patch for the current master branch build from git
> format-patch.  And spliced the code to implement the linking of symbols
> in info manuals to the help documentation into lisp/info-xref.el.

Thanks.

I think this should be in info.el, or maybe in a separate
info-SOMETHING.el file.  info-xref.el is for a certain job, of
interest primarily to Emacs maintainers, that is different from this
one, and I'm not sure conflating them is TRT.

More specific comments below.

> +;; This library provides links of symbols (functions, variables,

The "This library" part is a remnant of the previous life of this
code, and should be reworded to refer to specific command(s).

> +;; In any case all symbol names must be known to Emacs, i.e. their
> +;; names are found in the variable `obarray'.

I think a more useful way of saying this is

  In any case, the symbol must be known to Emacs, which means it is
  either a built-in, or its Lisp package is loaded in the current
  Emacs session, or the symbol is auto-loaded.

> +;; Inform is checking if the Info documents are relevant Elisp and
      ^^^^^^
This should be adapted to the "new life" of Inform as part of Emacs.

> +;; Emacs related files to avoid false positives.  Please see the
> +;; customization variable `inform-none-emacs-or-elisp-documents'.
                              ^^^^^^
And this.

> +;;; Change Log:
> +
> +;; 1.3:
> +
> +;; Inform is checking if the Info documents are relevant Elisp and
> +;; Emacs related files to avoid false positives.
> +
> +;; 1.2:
> +
> +;; Link Elisp descriptions of symbols to their help documentation,
> +;; like the following function example: -- Function: eval form
> +
> +;; Distinguish color of texinfo links (`link' type) and Help links
> +;; (`font-lock-function-name-face')

Not sure if it makes sense to keep this change log.

> +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
> +;; Does the following belong to customize.el?
> +
> +;; Generalise linking to "customization buffers" for the "easy
> +;; customization" info documentation see also the customization
> +;; section in the elisp manual
> +
> +;; - distinguish the Customization-links from Help- and Info-links
> +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
> +
> +;;; Ideas:
> +
> +;; Link the help buffers back to higher level info manual subjects,
> +;; similar to help-fns+.el from Drew Adams.
> +
> +;; Twice clicking or RETurning removes *Help* buffer (idea: Drew
> +;; Adams)
> +
> +;; Different colors for different symbol types (idea: Drew Adams) see
> +;; package helpful and info+ / info-colors on Melpa and see
> +;; font-lock.el for common faces.
> +
> +;; - Do we need to indicate an already visited Help link with a
> +;;   different color?
> +
> +;; - Would it be be good to overtake all colors of package
> +;;   "info-colors"?
> +
> +;; - Do we need to distinguish the link FONTS? No, difficult to read!
> +
> +;; Back / Forward button in help buffer - back to info buffer or
> +;; remain in help mode?
> +
> +;; Linking of standard symbol properties?
> +
> +;; - (info "(elisp) Standard Properties")
> +
> +;;  Elisp manual examples:
> +;;       (symbol-name 'car) ... ?
> +
> +;; Shortening the verbose texinfo URLs?  But how to handle the changed
> +;; indentation?
> +
> +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

Please review this part and decide which portions should be kept,
perhaps after a suitable rewording, and which should be removed.

> +(require 'button)
> +(require 'cl-lib)
> +(require 'help-mode)                   ;redundant?

If this is added to an existing file, there should be a ^L and some
heading-style command before it.

> +;; activate inform without manually loading it. Is there a better way?
> +;; ;;;###autoload (require 'info-xref)
>  
> +;; this is spawning lisp/info-xref.el's definition to 'info! This
> +;; group is sorted now in 'docs and 'info! -FIXME-

Comments should start with a capital letter.

> +;;;###autoload
> +(defcustom info-xref-make-xref-flag t
> +  "Non-nil means create symbol links in info buffers."
> +  :type '(choice (const :tag "Create links" t)
> +                 (const :tag "Do not link" nil))
> +  :group 'info-xref)

I think we frown on autoloading defcustoms.

Also, every new defcustom should have a :version tag.

> +;; Info-director-list must be initialised
           ^^^^^^^^  
Typo.  Also, comments should be complete sentences, and end with a
period (here and elsewhere in the patch).

> +(info-initialize)

Why do you need to call this? and why on top level?

> +;; Turn into regexp list necessary? Stefan
> +;; Switch to alist with explanation of file name?
> +(defcustom info-xref-none-emacs-or-elisp-documents
> +  '("aarm2012" ; Stefan: Ada manual, Elpa archive
> +    "arm2012"  ; Stefan: Ada manual
> +    "sicp"   ; T.V: Structure and Interpretation of Computer Programs,
> +                                        ; Melpa archive
> +    )
> +  "List of all none GNU-Emacs or Elisp documentation.
> +Or other documents not to be checked for linking to their help
> +documentation.  The list must contains only the base name of the
> +files (without their file name extension \".info\")."
> +  :type '(repeat string)
> +  :group 'info-xref)

Not sure what is this about, and what do the names above signify.
There are also typos: "none GNU-Emacs", "must contains".

> +(defun Info-xref-make-xrefs (&optional buffer)
> +  "Parse and hyperlink documentation cross-references in the given BUFFER.

The doc string should tell what happens if BUFFER is omitted or nil.

> +                (while (re-search-forward Info-xref-symbol-regexp nil t)
> +                  (let* ((data (match-string 8))
> +                         (sym (intern-soft data)))
> +                    (if sym
> +                        (cond
> +                         ((match-string 3) ; `variable' &c
> +                          (and (or (boundp sym) ; `variable' doesn't ensure
> +                                        ; it's actually bound
> +                                   (get sym 'variable-documentation))
> +                               (Info-xref-button 8 'Info-xref-variable sym)))
> +                         ((match-string 4) ; `function' &c
> +                          (and (fboundp sym) ; similarly
> +                               (Info-xref-button 8 'Info-xref-function sym)))
> +                         ((match-string 5) ; `face'
> +                          (and (facep sym)
> +                               (Info-xref-button 8 'Info-xref-face sym)))
> +                         ((match-string 6)) ; nothing for `symbol'
> +                         ((match-string 7)
> +                          (Info-xref-button 8 'Info-xref-function-def sym))
> +                         ((cl-some (lambda (x) (funcall (nth 1 x) sym))
> +                                   describe-symbol-backends)
> +                          (Info-xref-button 8 'Info-xref-symbol sym)))))))

Can this be rewritten so as to avoid the need for error-prone updates
of the sub-expression numbers every time Info-xref-symbol-regexp is
modified?

Finally, this needs additions to the user manual and NEWS.