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

From: Stefan Monnier <[email-masked]>
Date: Sat, 04 Feb 2023 22:54:00 -0500

> +(define-minor-mode info-button-mode
> +  "The mode creates buttons on symbols linking to their help documentation.
> +
> +"
> +  :init-value nil
> +  :lighter " B"
> +  :group 'info
> +  :version "30.1"

Since you have not specified `:global t`, this is a buffer-local minor
mode, i.e. each buffer will have its own value of the `info-button-mode` boolean.

[ BTW, whether the links look like buttons or not seems rather
  incidental, so I think the mode's name should talk about something more
  precise than "button".
  Maybe something like `info-links-to-help-mode`?  ]

> +  (if (string= major-mode "Info-mode")

`major-mode` contains a symbol, so better do (eq major-mode 'Info-mode).
This said, I don't know why you have that test.  Any reason you're
worried about this being called from another buffer?

> +      (if info-button-mode
> +          (progn
> +            (add-hook 'Info-selection-hook 'info-make-button)
> +            (info-make-button))               ;for the current buffer
> +        (remove-hook 'Info-selection-hook 'info-make-button)

Here, you install/remove the `info-make-button` function to the global
part of the `Info-selection-hook` so it will affect all Info buffers
rather than just the current one.

I don't know if this minor mode should be global or buffer-local, but
please try and make it consistent: if it's buffer-local, then it should
only affect the current buffer.

BTW, the name `info-make-button` suggests it creates a single button
(and provides no info about what kind of button this might be).

> +        (setq inhibit-read-only t)
> +        (remove-list-of-text-properties (point-min) (point-max)
> +                                        '(button category info-args))
> +        (set-buffer-modified-p nil))

I think you wanted to use `with-silent-modifications` here :-)

        Stefan