Improve help-fns-edit-variable for Lisp editing

Before d50c82f3e9 ("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.
* lisp/help-fns.el (help-fns-edit-variable): Call 'string-edit',
passing 'emacs-lisp-mode' and 'read'.
* lisp/textmodes/string-edit.el (string-edit--read): Add.
(string-edit): Add :major-mode and :read arguments and avoid
passive voice.
(read-string-from-buffer): Avoid passive voice in docs.
(string-edit-mode-map, string-edit-minor-mode-map)
(string-edit-mode, string-edit-minor-mode): Move 'string-edit'
keybindings to a minor mode.
(string-edit-done): Call 'string-edit--read' before exiting.
(Bug#77834)

lisp/help-fns.el

@@ -1570,11 +1570,20 @@ by this command."
   (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)))
 
 (autoload 'shortdoc-help-fns-examples-function "shortdoc")
 

lisp/textmodes/string-edit.el

@@ -33,20 +33,25 @@
 
 (defvar string-edit--success-callback)
 (defvar string-edit--abort-callback)
+(defvar string-edit--read)
 
 ;;;###autoload
 (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.
 
-If the user aborts (with \\<string-edit-mode-map>\\[string-edit-abort]), ABORT-CALLBACK (if any) is
-called with no parameters.
+Call MAJOR-MODE (defaulting to `string-edit-mode') to set up the new
+buffer, and insert PROMPT (defaulting to nothing) at the start of the
+buffer.
 
-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.
+When the user finishes editing (with \\<string-edit-minor-mode-map>\\[string-edit-done]), call
+READ (defaulting to `identity') on the resulting string, omitting PROMPT if any.
+
+If READ returns without an error, quit the buffer and call
+SUCCESS-CALLBACK on the result.
+
+If the user aborts (with \\<string-edit-minor-mode-map>\\[string-edit-abort]),
+call ABORT-CALLBACK (if any) with no parameters.
 
 Also see `read-string-from-buffer'."
   (with-current-buffer (generate-new-buffer "*edit string*")
@@ -74,26 +79,27 @@ Also see `read-string-from-buffer'."
 
     (set-buffer-modified-p nil)
     (setq buffer-undo-list nil)
-    (string-edit-mode)
+    (funcall (or major-mode #'string-edit-mode))
+    (string-edit-minor-mode)
     (setq-local string-edit--success-callback success-callback)
     (setq-local string-edit--abort-callback abort-callback)
+    (setq-local string-edit--read read)
     (setq-local header-line-format
                 (substitute-command-keys
-                 "Type \\<string-edit-mode-map>\\[string-edit-done] when you've finished editing or \\[string-edit-abort] to abort"))
+                 "Type \\<string-edit-minor-mode-map>\\[string-edit-done] when you've finished editing or \\[string-edit-abort] to abort"))
     (message "%s" (substitute-command-keys
-                   "Type \\<string-edit-mode-map>\\[string-edit-done] when you've finished editing"))))
+                   "Type \\<string-edit-minor-mode-map>\\[string-edit-done] when you've finished editing"))))
 
 ;;;###autoload
 (defun read-string-from-buffer (prompt string)
   "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]).
 
-PROMPT will be inserted at the start of the buffer, but won't be
-included in the resulting string.  If nil, no prompt will be
-inserted in the buffer.
+Insert PROMPT at the start of the buffer.  If nil, no prompt is
+inserted.
 
-When the user exits recursive edit, this function returns the
-edited STRING.
+When the user exits recursive edit, return the contents of the
+buffer (without including PROMPT).
 
 Also see `string-edit'."
   (string-edit
@@ -108,11 +114,16 @@ Also see `string-edit'."
   (recursive-edit)
   string)
 
-(defvar-keymap string-edit-mode-map
+(defvar-keymap string-edit-minor-mode-map
   "C-c C-c" #'string-edit-done
   "C-c C-k" #'string-edit-abort)
 
-(define-derived-mode string-edit-mode text-mode "String"
+(define-minor-mode string-edit-minor-mode
+  "Minor mode for editing strings"
+  :lighter "String"
+  :interactive nil)
+
+(define-derived-mode string-edit-mode text-mode "Text"
   "Mode for editing strings."
   :interactive nil)
 
@@ -120,13 +131,16 @@ Also see `string-edit'."
   "Finish editing the string and call the callback function.
 This will kill the current buffer."
   (interactive)
-  (goto-char (point-min))
-  ;; Skip past the help text.
-  (text-property-search-forward 'string-edit--prompt)
-  (let ((string (buffer-substring (point) (point-max)))
-        (callback string-edit--success-callback))
+  (let* ((string
+          (save-excursion
+            (goto-char (point-min))
+            ;; Skip past the help text.
+            (text-property-search-forward 'string-edit--prompt)
+            (buffer-substring (point) (point-max))))
+         (valid (funcall (or string-edit--read #'identity) string))
+         (callback string-edit--success-callback))
     (quit-window 'kill)
-    (funcall callback string)))
+    (funcall callback valid)))
 
 (defun string-edit-abort ()
   "Abort editing the current string."