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

From: "H. Dieter Wilhelm" <[email-masked]>
Date: Sun, 05 Feb 2023 14:54:32 +0100

Stefan Monnier <[email-masked]> writes:

>> +(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`?  ]

Indeed, I've changed already the non-descriptive names, thanks.

>> +  (if (string= major-mode "Info-mode")
>
> `major-mode` contains a symbol, so better do (eq major-mode 'Info-mode).

I see, I confused the symbol with its name.  (Strange then that my
version worked, anyway?)

> This said, I don't know why you have that test.  Any reason you're
> worried about this being called from another buffer?

Because this info-links-to-help-mode is useless in all buffers, except..

.. Well, I had blinkers on.  Indeed, such a mode might be helpful for
buffers except *info*! Thank you.

Is it worthwhile to generalise the functionality (maybe later), that is
the question?

>> +      (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 guess a user who doesn't like help linking won't appreciate it in all
info buffers, cloned or not.

> 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.

Please let me rethink the approach in the light of above discussion.

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

Yes, it's poor, I improved the name.

>> +        (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 :-)

I think so, thank you.  😊

  Dieter

-- 
Best wishes
H. Dieter Wilhelm
Zwingenberg, Germany