GNU bug report logs - #77834
[PATCH] Improve help-fns-edit-variable for Lisp editing

Previous Next

Package: emacs;

Reported by: Spencer Baugh <sbaugh <at> janestreet.com>

Date: Tue, 15 Apr 2025 21:20:02 UTC

Severity: normal

Tags: patch

Done: Eli Zaretskii <eliz <at> gnu.org>

Bug is archived. No further changes may be made.

Full log


View this message in rfc822 format

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 77834 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>, larsi <at> gnus.org, azeng <at> janestreet.com
Subject: bug#77834: [PATCH] Improve help-fns-edit-variable for Lisp editing
Date: Thu, 17 Apr 2025 16:17:01 -0400
Philip Kaludercic <philipk <at> posteo.net> writes:

> Spencer Baugh <sbaugh <at> janestreet.com> writes:
>
>
> [...]
>
>> diff --git a/lisp/help-fns.el b/lisp/help-fns.el
>> index 0eb0a7a40be..7a0bc2908e9 100644
>> --- a/lisp/help-fns.el
>> +++ b/lisp/help-fns.el
>> @@ -1561,11 +1561,20 @@ help-fns-edit-variable
>>    (let ((var (get-text-property (point) 'help-fns--edit-variable)))
>>      (unless var
>>        (error "No variable under point"))
>> -    (let ((str (read-string-from-buffer
>> -                (format ";; Edit the `%s' variable." (nth 0 var))
>> -                (prin1-to-string (nth 1 var)))))
>> -      (set (nth 0 var) (read str))
>> -      (revert-buffer))))
>> +    (string-edit
>> +     (format ";; Edit the `%s' variable." (nth 0 var))
>> +     (prin1-to-string (nth 1 var))
>> +     (lambda (edited)
>> +       (set (nth 0 var) edited)
>> +       (exit-recursive-edit))
>> +     :abort-callback
>> +     (lambda ()
>> +       (exit-recursive-edit)
>> +       (error "Aborted edit, variable unchanged"))
>> +     :major-mode #'emacs-lisp-mode
>> +     :read #'read)
>> +    (recursive-edit)
>> +    (revert-buffer)))
>
> I don't remember the details here anymore, but why do you need the
> `recursive-edit' and `exit-recursive-edit'?  Shouldn't `string-edit'
> take care of this instead of the caller?
>
> [...]

No - read-string-from-buffer takes care of those, but string-edit does
not.

I stopped using read-string-from-buffer on a suggestion from Eli.  We
could add some new function "string-edit-recursive" which handles the
recursive-edit details.

However: one nice property of using string-edit with its callbacks
directly is that if the user doesn't explicitly call string-edit-done,
then SUCCESS-CALLBACK isn't called, so (as I've written the code in my
patch) the variable's value is not set.

When using read-string-from-buffer, if the user manually exits the
recursive edit, read-string-from-buffer returns the original string, so
the variable would be unnecessarily set back to its original value, even
if it was changed by something else in the meantime.  For example, this
could happen if the user forgot about the string-edit buffer, changed
the variable some other way, and then later noticed they were in a
recursive-edit and ran exit-recursive-edit manually.  My code using
string-edit won't have this problem.





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.