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

Previous Next

Full log

View this message in rfc822 format

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

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

Philip, was this side-effect of commit d50c82f3e98e intentional or
just an oversight?  Maybe we should simply revert that commit instead?

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

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

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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.