GNU bug report logs -
#77834
[PATCH] Improve help-fns-edit-variable for Lisp editing
Previous Next
Full log
View this message in rfc822 format
Spencer Baugh <sbaugh <at> janestreet.com> writes:
> Eli Zaretskii <eliz <at> gnu.org> writes:
>>> Cc: azeng <at> janestreet.com, Philip Kaludercic <philipk <at> posteo.net>,
>>> Lars Ingebrigtsen <larsi <at> gnus.org>
>>> Date: Tue, 15 Apr 2025 17:18:37 -0400
>>> From: Spencer Baugh via "Bug reports for GNU Emacs,
>>> the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
>>>
>>> Before d50c82f3e98e ("Simplify
>>> 'help-enable-variable-value-editing' using 'string-edit'"),
>>> help-fns-edit-variable would open a buffer in emacs-lisp-mode
>>> and would not allow exiting that buffer with an invalid Lisp
>>> expression. Restore that functionality by enhancing string-edit
>>> to allow choosing a major mode and allow passing a function to
>>> validate the buffer contents before returning.
>>
>> Why is it better to use emacs-lisp-mode in this case? what are the
>> advantages and disadvantages?
>
> It provides syntax highlighting and completion and all the other
> features of emacs-lisp-mode. We're editing a Lisp form after all.
>
>> Philip, was this side-effect of commit d50c82f3e98e intentional or
>> just an oversight? Maybe we should simply revert that commit instead?
It was an oversight.
> I don't think we should revert the commit - I think using string-edit is
> a good change since it's almost exactly what we want for
> help-fns-edit-variable.
>
> Plus, the new functionality I'm adding to string-edit is useful in other
> places too - I plan to also use it for other later changes.
I agree, this is an improvement we should make to string-edit regardless
of `help-fns-edit-variable'.
>>> (cl-defun string-edit (prompt string success-callback
>>> - &key abort-callback)
>>> + &key abort-callback major-mode read)
>>> "Switch to a new buffer to edit STRING.
>>> -When the user finishes editing (with \\<string-edit-mode-map>\\[string-edit-done]), SUCCESS-CALLBACK
>>> -is called with the resulting string.
>>> +When the user finishes editing (with `string-edit-done'),
>>> +SUCCESS-CALLBACK is called with the resulting string. If READ is
>>> +non-nil, it is called on the string and its result is passed to
>>> +SUCCESS-CALLBACK instead. If READ signals an error, the buffer remains
>>> +open, giving the user a chance to correct a typo.
>>>
>>> -If the user aborts (with \\<string-edit-mode-map>\\[string-edit-abort]), ABORT-CALLBACK (if any) is
>>> +If the user aborts (with `string-edit-abort'), ABORT-CALLBACK (if any) is
>>> called with no parameters.
>>>
>>> PROMPT will be inserted at the start of the buffer, but won't be
>>> included in the resulting string. If PROMPT is nil, no help text
>>> will be inserted.
>>>
>>> +MAJOR-MODE is called to set up the new buffer; if not passed,
>>> +`string-edit-mode' is used.
>>
>> This doc string, both its old parts and the new additions, are
>> replete with unnecessary uses of passive tense; please fix that as
>> part of the changeset.
>
> Will do so in the next version of the patch.
>
>>> ;;;###autoload
>>> -(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.
>
> 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.
>
>>> "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.
>
> But this is not an important change, if you prefer it to have the keymap
> markup instead.
This bug report was last modified 24 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.