Subject: bug#70163: 29.3; hexl-mode incorrect docstring

From: Eli Zaretskii <[email-masked]>
Date: Thu, 04 Apr 2024 15:18:27 +0300

> From: Thierry Volpiatto <[email-masked]>
> Cc: [email-masked],  [email-masked],  [email-masked]
> Date: Thu, 04 Apr 2024 05:47:40 +0000
> 
> >> >>> Hexl-mode docstring specify "\<hexl-mode-map>" in its first line, this
> >> >>> to specify some keybinding related to this map at end, as a result the
> >> >>> documentation command returns a three line string, the first line beeing
> >> >>> a blank line until hexl.el is loaded.  I think docstrings generally
> >> >>> should not have this specification in their first line.
> >> >>> 
> >> >>>     (defun hexl-mode (&optional arg)
> >> >>>       "\\<hexl-mode-map>A mode for editing binary files in hex dump format.
> >> >>>       [...] 
> >> >>>       Most cursor movement bindings are the same: use \\[hexl-backward-char],
> >> >>>       [...]
> >> >>
> >> >> What do you suggest to do instead?
> >> >
> >> > Customary is to put it directly before the (first) keymap reference.
> >> 
> >> Exactly.
> >
> > So we don't want an empty line at the beginning of a doc string, but
> > are okay with having it farther into the doc string?
> 
> Yes, this would avoid loading the whole file just for having the first line of
> the documentation.
> 
> Actually, (documentation 'hexl-mode) returns:
> 
> "\nUses keymap `hexl-mode-map', which is not currently defined.\nA mode for editing binary files in hex dump format..."
> 
> But once it is loaded:
> 
> "A mode for editing binary files in hex dump format..."
> 
> I think it is a good practice to specify keymap just when needed as
> specified in the manual:
> 
>    • In documentation strings for a major mode, you will want to refer
>      to the key bindings of that mode’s local map, rather than global
>      ones.  Therefore, use the construct ‘\\<...>’ once in the
>      documentation string to specify which key map to use.  Do this
>      before the first use of ‘\\[...]’.  The text inside the ‘\\<...>’
>      should be the name of the variable containing the local keymap for
>      the major mode.

Does the patch below gives any better results?  It doesn't look better
to me: the "Uses keymap..." blurb gets in the way and makes the
documentation text wrong.  And it will happen regardless of where you
stuff the \\<..> thing, just in different places of the test.  It
looks like this feature was not meant to be used with libraries that
aren't loaded, or maybe the intent is to use the RAW argument in those
cases.

Stefan, why do we inject this ugly blurb into the text we return when
the library is not loaded? why not simply return the same as RAW does
in those cases?

diff --git a/lisp/hexl.el b/lisp/hexl.el
index 1288cf4..938b218 100644
--- a/lisp/hexl.el
+++ b/lisp/hexl.el
@@ -256,10 +256,10 @@ hexl-line-displen
 
 ;;;###autoload
 (defun hexl-mode (&optional arg)
-  "\\<hexl-mode-map>A mode for editing binary files in hex dump format.
+  "A mode for editing binary files in hex dump format.
 This is not an ordinary major mode; it alters some aspects
 of the current mode's behavior, but not all; also, you can exit
-Hexl mode and return to the previous mode using `hexl-mode-exit'.
+Hexl mode and return to the previous mode using \\<hexl-mode-map>\\[hexl-mode-exit].
 
 This function automatically converts a buffer into the hexl format
 using the function `hexlify-buffer'.