Subject: Re: bug#77834: [PATCH] Improve help-fns-edit-variable for Lisp editing
From: Eli Zaretskii <[email-masked]>
To: Spencer Baugh <[email-masked]>
Cc: [email-masked], [email-masked], [email-masked], [email-masked]
Date: Sat, 26 Apr 2025 14:55:46 +0300
List-Id: <debbugs-submit.debbugs.gnu.org>

> From: Spencer Baugh <[email-masked]>
> Cc: [email-masked],  [email-masked],  [email-masked],
>    [email-masked]
> Date: Wed, 16 Apr 2025 13:31:39 -0400
> 
> Eli Zaretskii <[email-masked]> writes:
> 
> >> From: Spencer Baugh <[email-masked]>
> >> Cc: [email-masked],  [email-masked],  [email-masked],
> >>    [email-masked]
> >> Date: Wed, 16 Apr 2025 12:29:26 -0400
> >> 
> >> >> -(defun read-string-from-buffer (prompt string)
> >> >> +(cl-defun read-string-from-buffer (prompt string
> >> >> +                                          &key major-mode
> >> >> +                                          read)
> >> >
> >> > I don't see any reasons to make this a cl-defun, just to add 2 more
> >> > optional arguments.
> >> 
> >> It's to match the existing cl-defun for string-edit.  string-edit takes
> >> keyword arguments - read-string-from-buffer probably should match it.
> >
> > But read-string-from-buffer is a public function, so any incompatible
> > changes in it should be avoided.
> >
> >> Alternatively, I could just not change read-string-from-buffer and only
> >> change string-edit.  Then help-fns-edit-variable could call string-edit
> >> directly.  That would duplicate a bit more code though.
> >
> > I think this will be better.  string-edit is already a cl-defun, so we
> > just adding new optional arguments to it.
> 
> OK, reverted the changes to read-string-from-buffer and switched
> help-fns-edit-variable to call string-edit directly.
> 
> > But we also need a default for the :read argument if it is omitted.
> 
> Good catch, done.
> 
> >> >>    "Switch to a new buffer to edit STRING in a recursive edit.
> >> >> -The user finishes editing with \\<string-edit-mode-map>\\[string-edit-done], or aborts with \\<string-edit-mode-map>\\[string-edit-abort]).
> >> >> +
> >> >> +When the user finishes editing with `string-edit-done', this function
> >> >
> >> > Why did you remove the keymap and command markup?  That loses useful
> >> > features.
> >> >
> >> >> +If the user aborts with `string-edit-abort', this function signals an
> >> >> +error.
> >> >
> >> > Same here.
> >> 
> >> I removed these because neither of these are user-facing functions; the
> >> docstrings are only read by Lisp hackers writing new packages using
> >> these functions.  And it seems to me that for a Lisp programmer it's
> >> more useful to know the name of the commands which the user uses to
> >> finish editing, rather than the bindings for those commands.
> >
> > The original text shows both the command name and its binding.
> >
> >> But this is not an important change, if you prefer it to have the keymap
> >> markup instead.
> >
> > I'd prefer not to lose the information, indeed.
> 
> OK, reverted.
> 
> Also updated the docstrings to not use passive voice.

Thanks, now installed on the master branch, and closing the bug.