From unknown Fri Jun 20 07:13:03 2025 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-Mailer: MIME-tools 5.509 (Entity 5.509) Content-Type: text/plain; charset=utf-8 From: bug#77834 <77834@debbugs.gnu.org> To: bug#77834 <77834@debbugs.gnu.org> Subject: Status: [PATCH] Improve help-fns-edit-variable for Lisp editing Reply-To: bug#77834 <77834@debbugs.gnu.org> Date: Fri, 20 Jun 2025 14:13:03 +0000 retitle 77834 [PATCH] Improve help-fns-edit-variable for Lisp editing reassign 77834 emacs submitter 77834 Spencer Baugh severity 77834 normal tag 77834 patch thanks From debbugs-submit-bounces@debbugs.gnu.org Tue Apr 15 17:19:12 2025 Received: (at submit) by debbugs.gnu.org; 15 Apr 2025 21:19:12 +0000 Received: from localhost ([127.0.0.1]:56732 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1u4ngQ-0004CC-LP for submit@debbugs.gnu.org; Tue, 15 Apr 2025 17:19:11 -0400 Received: from lists.gnu.org ([2001:470:142::17]:60666) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1u4ngK-00048n-Cg for submit@debbugs.gnu.org; Tue, 15 Apr 2025 17:19:03 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1u4ng4-0003IM-Kd for bug-gnu-emacs@gnu.org; Tue, 15 Apr 2025 17:18:44 -0400 Received: from mxout5.mail.janestreet.com ([64.215.233.18]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1u4ng0-0003cM-Lc for bug-gnu-emacs@gnu.org; Tue, 15 Apr 2025 17:18:44 -0400 From: Spencer Baugh To: bug-gnu-emacs@gnu.org Subject: [PATCH] Improve help-fns-edit-variable for Lisp editing X-Debbugs-Cc: azeng@janestreet.com, Philip Kaludercic , Lars Ingebrigtsen Date: Tue, 15 Apr 2025 17:18:37 -0400 Message-ID: MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=janestreet.com; s=waixah; t=1744751917; bh=Mma7W9yiacEHGhTJiAOMmopFQ7OiS0163RWzo9S1eaw=; h=From:To:Subject:Date; b=2/pxBvvs66fOm6OLyAbzL9FKINnoTiAKGPoQFIoX0wtbWhI6sigXegU+BYf3qGEwW q5/5s5H6NgFcKbnUM0fU2yKsd/i4LpnXNcJUtM732psRg4VSLqAyPZaz1vE6NWA86x DYvAXYb5r3rncD8ebroUgi1fMu3SmCOrDVc/5C8t+rL3z0VxZb6YnAZyCCUp1WUrOr /aTYs1N4hWNTFV16pLglCVYRgH05X47dyi8UJIh1LRnQnf0UjCer9AxbWuNnZAGcJY CLi3pBrdBQ8imBDBLVQByuoWwxZaIjaRR8T8gIdnKaRj3y75ypZVwfmGvHD+5106XT UlpzgRFfx0PFA== Received-SPF: pass client-ip=64.215.233.18; envelope-from=sbaugh@janestreet.com; helo=mxout5.mail.janestreet.com X-Spam_score_int: -43 X-Spam_score: -4.4 X-Spam_bar: ---- X-Spam_report: (-4.4 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_MED=-2.3, RCVD_IN_MSPIKE_H5=0.001, RCVD_IN_MSPIKE_WL=0.001, RCVD_IN_VALIDITY_RPBL_BLOCKED=0.001, RCVD_IN_VALIDITY_SAFE_BLOCKED=0.001, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-Spam-Score: 0.9 (/) X-Debbugs-Envelope-To: submit X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -0.1 (/) --=-=-= Content-Type: text/plain Tags: patch 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. In GNU Emacs 30.1.50 (build 8, x86_64-pc-linux-gnu, X toolkit, cairo version 1.15.12, Xaw scroll bars) of 2025-04-10 built on igm-qws-u22796a Repository revision: 128bc06bfcc56a35d6b5555cc546faaf0d964df0 Repository branch: emacs-30 Windowing system distributor 'The X.Org Foundation', version 11.0.12011000 System Description: Rocky Linux 8.10 (Green Obsidian) Configured using: 'configure --config-cache --with-x-toolkit=lucid --without-gpm --without-gconf --without-selinux --without-imagemagick --with-modules --with-gif=no --with-cairo --with-rsvg --without-compress-install --with-tree-sitter --with-native-compilation=aot' --=-=-= Content-Type: text/patch Content-Disposition: attachment; filename=0001-Improve-help-fns-edit-variable-for-Lisp-editing.patch >From 26ea5e0cbbe85950d4362489ffe054cd570395c8 Mon Sep 17 00:00:00 2001 From: Spencer Baugh Date: Tue, 15 Apr 2025 17:17:27 -0400 Subject: [PATCH] Improve help-fns-edit-variable for Lisp editing 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. * lisp/help-fns.el (help-fns-edit-variable): Pass emacs-lisp-mode and read to read-string-from-buffer. * lisp/textmodes/string-edit.el (string-edit--read): Add. (string-edit, read-string-from-buffer): Add :major-mode and :read arguments. (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. --- lisp/help-fns.el | 10 +++-- lisp/textmodes/string-edit.el | 70 +++++++++++++++++++++++++---------- 2 files changed, 56 insertions(+), 24 deletions(-) diff --git a/lisp/help-fns.el b/lisp/help-fns.el index 0eb0a7a40be..57aab4d82e1 100644 --- a/lisp/help-fns.el +++ b/lisp/help-fns.el @@ -1561,10 +1561,12 @@ 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)) + (let ((expr (read-string-from-buffer + (format ";; Edit the `%s' variable." (nth 0 var)) + (prin1-to-string (nth 1 var)) + :major-mode #'emacs-lisp-mode + :read #'read))) + (set (nth 0 var) expr) (revert-buffer)))) (autoload 'shortdoc-help-fns-examples-function "shortdoc") diff --git a/lisp/textmodes/string-edit.el b/lisp/textmodes/string-edit.el index 3c76db202c7..47b5d4c337d 100644 --- a/lisp/textmodes/string-edit.el +++ b/lisp/textmodes/string-edit.el @@ -33,21 +33,28 @@ string-edit-prompt (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-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-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. + Also see `read-string-from-buffer'." (with-current-buffer (generate-new-buffer "*edit string*") (when prompt @@ -74,26 +81,39 @@ string-edit (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-done] when you've finished editing or \\[string-edit-abort] to abort")) + "Type \\\\[string-edit-done] when you've finished editing or \\[string-edit-abort] to abort")) (message "%s" (substitute-command-keys - "Type \\\\[string-edit-done] when you've finished editing")))) + "Type \\\\[string-edit-done] when you've finished editing")))) ;;;###autoload -(defun read-string-from-buffer (prompt string) +(cl-defun read-string-from-buffer (prompt string + &key major-mode + read) "Switch to a new buffer to edit STRING in a recursive edit. -The user finishes editing with \\\\[string-edit-done], or aborts with \\\\[string-edit-abort]). + +When the user finishes editing with `string-edit-done', this function +returns that string. If READ is non-nil, it is called on the string and +its return value is returned 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-abort', this function signals an +error. 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. -When the user exits recursive edit, this function returns the -edited STRING. +MAJOR-MODE is called to set up the new buffer; if not passed, +`string-edit-mode' is used. + +When the user finished editing Also see `string-edit'." (string-edit @@ -104,15 +124,22 @@ read-string-from-buffer (exit-recursive-edit)) :abort-callback (lambda () (exit-recursive-edit) - (error "Aborted edit"))) + (error "Aborted edit")) + :major-mode major-mode + :read read) (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 +147,16 @@ string-edit-done "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 string-edit--read string)) + (callback string-edit--success-callback)) (quit-window 'kill) - (funcall callback string))) + (funcall callback valid))) (defun string-edit-abort () "Abort editing the current string." -- 2.39.3 --=-=-=-- From debbugs-submit-bounces@debbugs.gnu.org Wed Apr 16 04:01:55 2025 Received: (at 77834) by debbugs.gnu.org; 16 Apr 2025 08:01:55 +0000 Received: from localhost ([127.0.0.1]:34409 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1u4xiU-0006XJ-Ow for submit@debbugs.gnu.org; Wed, 16 Apr 2025 04:01:55 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:37076) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1u4xiN-0006W9-J1 for 77834@debbugs.gnu.org; Wed, 16 Apr 2025 04:01:51 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1u4xiE-0003LP-1m; Wed, 16 Apr 2025 04:01:38 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date: mime-version; bh=b+ChpNRwt1WJGCGIRSkgqZp0WlcLGQ258PJ7NtS2x0E=; b=T3E+u0sN8jBO 2GVzMfqelLfes3VMqHpq10dvjZVTzO11ZjVKKTPuLiqAWdG+8ZA3JYkR92vjM1YUNmy2csBOxy226 mJ5mBG0KxrHsYw0DxwfzkgDYo8/714gzBqGNH/uQytk3pRnHoIABgWriwJ9XXR0K84zfw7L3kncUL KGKacjt1aoZwrDCDXF2e6omMKnmul8Wd6AzsVapBIoMxVxf9Hgh/azfr85z7OtV1c0PM7haIv255D tc1CpsRjkcky3QeyGC8wfv0m8CLU56HQQuFIo1JumSwLYi/tXwBPXbVgRJ6Sik8psr8lm8YyYCOY5 eVNRV4zr4wnVzlV9DIqu8Q==; Date: Wed, 16 Apr 2025 11:01:12 +0300 Message-Id: <86plhcfsc7.fsf@gnu.org> From: Eli Zaretskii To: Spencer Baugh In-Reply-To: (bug-gnu-emacs@gnu.org) Subject: Re: bug#77834: [PATCH] Improve help-fns-edit-variable for Lisp editing References: X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 77834 Cc: 77834@debbugs.gnu.org, larsi@gnus.org, philipk@posteo.net, azeng@janestreet.com X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) > Cc: azeng@janestreet.com, Philip Kaludercic , > Lars Ingebrigtsen > 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" > > 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-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-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-done], or aborts with \\\\[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. From debbugs-submit-bounces@debbugs.gnu.org Wed Apr 16 12:29:46 2025 Received: (at 77834) by debbugs.gnu.org; 16 Apr 2025 16:29:47 +0000 Received: from localhost ([127.0.0.1]:40884 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1u55du-0007pi-3M for submit@debbugs.gnu.org; Wed, 16 Apr 2025 12:29:46 -0400 Received: from mxout5.mail.janestreet.com ([64.215.233.18]:38657) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1u55dk-0007o2-Bz for 77834@debbugs.gnu.org; Wed, 16 Apr 2025 12:29:38 -0400 From: Spencer Baugh To: Eli Zaretskii Subject: Re: bug#77834: [PATCH] Improve help-fns-edit-variable for Lisp editing In-Reply-To: <86plhcfsc7.fsf@gnu.org> (Eli Zaretskii's message of "Wed, 16 Apr 2025 11:01:12 +0300") References: <86plhcfsc7.fsf@gnu.org> Date: Wed, 16 Apr 2025 12:29:26 -0400 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=janestreet.com; s=waixah; t=1744820966; bh=rg0YEKfsXvmSisZ7BIBDcJzuxk2VZ7HRb9k04DUjxeo=; h=From:To:Cc:Subject:In-Reply-To:References:Date; b=Dk8LfZ+Taqw8/tBuTKjHrnJ8DrwEQ39qQyGpmkSULFHg/PPceZQ86Xo5/ZFbGDu7j y/KFSv0kTfXIuAf+6zWBg8hyuuflvCbGdxR4LbL4TdtjsnOUtXBSARVNA8nBaUS6jp ZIZxsUgp9hOgnKdCH5baqiVsgOhuOkJ1H3elzegG6RPIRDiib7J4PP2gemcfJhxqxb DRgjJR40gwDJBA5rGTA8xNtNjYKRoUu+nOB3EwPgrY68FL3mrwL1RObi70kUWXnfYe 7In//Hv2r7ET7xPyeGUbT81+dcyOxPsa1OpZEKXrClIV67xBJ89C8e79cx0ph9Xfxi mraxi9vJrzTug== X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 77834 Cc: 77834@debbugs.gnu.org, larsi@gnus.org, philipk@posteo.net, azeng@janestreet.com X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) Eli Zaretskii writes: >> Cc: azeng@janestreet.com, Philip Kaludercic , >> Lars Ingebrigtsen >> 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" >> >> 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? 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. >> (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-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-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-done], or aborts with \\\\[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. From debbugs-submit-bounces@debbugs.gnu.org Wed Apr 16 13:02:59 2025 Received: (at 77834) by debbugs.gnu.org; 16 Apr 2025 17:02:59 +0000 Received: from localhost ([127.0.0.1]:41198 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1u56A5-0006oR-UD for submit@debbugs.gnu.org; Wed, 16 Apr 2025 13:02:59 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:52964) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1u568o-0006es-1O for 77834@debbugs.gnu.org; Wed, 16 Apr 2025 13:01:41 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1u568h-0003T1-IC; Wed, 16 Apr 2025 13:01:31 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date: mime-version; bh=Y5+vL94RGu5EooopbaHcWfACHOuaqvFkxdztnkDcgiU=; b=fUSe+qNU23kP icJilmdCEZDgGVYe7TuGaC01wSO3WCN6Z/Z60riU5YOPgGaHnmIeRqsRYN1KgbZuVHTmHgB+xhPnN FU0VWX6yRjuug3unTs4wlHoX34rXS1OKT/ntTFMW51leuZdfRtUsSM+ysshvbvEOA5nyNDU7qNrpl FEIzCsEeiFlvWu0ozgyBDuzdeFy7zYfTXVWgVUS2Z6LvJN7nfkkVKKvTfQ/YsmBft0Y/WMCCnfUou JjC6YHAxNvoUEzVd/0llyJU8MDj+qkETic0IxPU/qDH0bHj++StDFDQm02fBN1yL525bodCr28Ed1 o8IV2ZRhvbnpjZf9vLx5Cg==; Date: Wed, 16 Apr 2025 20:00:54 +0300 Message-Id: <867c3kf3cp.fsf@gnu.org> From: Eli Zaretskii To: Spencer Baugh In-Reply-To: (message from Spencer Baugh on Wed, 16 Apr 2025 12:29:26 -0400) Subject: Re: bug#77834: [PATCH] Improve help-fns-edit-variable for Lisp editing References: <86plhcfsc7.fsf@gnu.org> X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 77834 Cc: 77834@debbugs.gnu.org, larsi@gnus.org, philipk@posteo.net, azeng@janestreet.com X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) > From: Spencer Baugh > Cc: 77834@debbugs.gnu.org, larsi@gnus.org, philipk@posteo.net, > azeng@janestreet.com > 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. But we also need a default for the :read argument if it is omitted. > >> "Switch to a new buffer to edit STRING in a recursive edit. > >> -The user finishes editing with \\\\[string-edit-done], or aborts with \\\\[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. Thanks. From debbugs-submit-bounces@debbugs.gnu.org Wed Apr 16 13:32:06 2025 Received: (at 77834) by debbugs.gnu.org; 16 Apr 2025 17:32:07 +0000 Received: from localhost ([127.0.0.1]:41306 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1u56c9-0001JI-O2 for submit@debbugs.gnu.org; Wed, 16 Apr 2025 13:32:06 -0400 Received: from mxout5.mail.janestreet.com ([64.215.233.18]:35519) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1u56bx-0001HO-C0 for 77834@debbugs.gnu.org; Wed, 16 Apr 2025 13:31:53 -0400 From: Spencer Baugh To: Eli Zaretskii Subject: Re: bug#77834: [PATCH] Improve help-fns-edit-variable for Lisp editing In-Reply-To: <867c3kf3cp.fsf@gnu.org> (Eli Zaretskii's message of "Wed, 16 Apr 2025 20:00:54 +0300") References: <86plhcfsc7.fsf@gnu.org> <867c3kf3cp.fsf@gnu.org> Date: Wed, 16 Apr 2025 13:31:39 -0400 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=janestreet.com; s=waixah; t=1744824699; bh=vpGrpB8szvcN2Mk/l4pmPCaHVU8YoYSYTU117GdsIJE=; h=From:To:Cc:Subject:In-Reply-To:References:Date; b=zycCytk/ebH6cYIWvHCjQU5ERL3Gz8jjIzvbwZbvtfG60y/ky4AJA3JzqVLieMCvT T+5QkVpJRNik12wRmW5VrW5RC+OrgHhx/csL+8cuj8UTiyCbEdqmaKgWCoSR5mqYe6 z6hoKzQa6Fi/cvOXWx7TvuJ96M34fuKZSoD57bd7t/+Q9FJYcOLfhjRUFN213XqhqO vjgSS45B7jXOHiUDx45AHNFJMvYbfsYgbkUID4gOHb5xqxIZcv11B2XTO93vTrW6c2 12ifA5CRSJDqmZV/abwO+dd8w7AT147BirXtpxqRvkRd2by3fc+YB0n++rgTp6ygSs IYzri3RhAFxAw== X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 77834 Cc: 77834@debbugs.gnu.org, larsi@gnus.org, philipk@posteo.net, azeng@janestreet.com X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) --=-=-= Content-Type: text/plain Eli Zaretskii writes: >> From: Spencer Baugh >> Cc: 77834@debbugs.gnu.org, larsi@gnus.org, philipk@posteo.net, >> azeng@janestreet.com >> 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-done], or aborts with \\\\[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. --=-=-= Content-Type: text/x-patch Content-Disposition: inline; filename=0001-Improve-help-fns-edit-variable-for-Lisp-editing.patch >From 6ea71c8f516c6cc9ae3ec38dc8a331fd17afc3d3 Mon Sep 17 00:00:00 2001 From: Spencer Baugh Date: Tue, 15 Apr 2025 17:17:27 -0400 Subject: [PATCH] Improve help-fns-edit-variable for Lisp editing 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. * 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. --- lisp/help-fns.el | 19 ++++++++--- lisp/textmodes/string-edit.el | 62 +++++++++++++++++++++-------------- 2 files changed, 52 insertions(+), 29 deletions(-) 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))) (autoload 'shortdoc-help-fns-examples-function "shortdoc") diff --git a/lisp/textmodes/string-edit.el b/lisp/textmodes/string-edit.el index 3c76db202c7..c8fdffe5f4c 100644 --- a/lisp/textmodes/string-edit.el +++ b/lisp/textmodes/string-edit.el @@ -33,20 +33,25 @@ string-edit-prompt (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-done]), SUCCESS-CALLBACK -is called with the resulting string. -If the user aborts (with \\\\[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-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-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 @@ string-edit (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-done] when you've finished editing or \\[string-edit-abort] to abort")) + "Type \\\\[string-edit-done] when you've finished editing or \\[string-edit-abort] to abort")) (message "%s" (substitute-command-keys - "Type \\\\[string-edit-done] when you've finished editing")))) + "Type \\\\[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-done], or aborts with \\\\[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 @@ read-string-from-buffer (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 @@ string-edit-done "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." -- 2.39.3 --=-=-=-- From debbugs-submit-bounces@debbugs.gnu.org Thu Apr 17 04:05:06 2025 Received: (at 77834) by debbugs.gnu.org; 17 Apr 2025 08:05:07 +0000 Received: from localhost ([127.0.0.1]:45672 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1u5KF5-0002Rm-Uu for submit@debbugs.gnu.org; Thu, 17 Apr 2025 04:05:06 -0400 Received: from mout01.posteo.de ([185.67.36.65]:36981) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1u5KEw-0002Pu-RW for 77834@debbugs.gnu.org; Thu, 17 Apr 2025 04:04:57 -0400 Received: from submission (posteo.de [185.67.36.169]) by mout01.posteo.de (Postfix) with ESMTPS id 9F391240027 for <77834@debbugs.gnu.org>; Thu, 17 Apr 2025 10:04:47 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.net; s=2017; t=1744877087; bh=oXLNEC1smjt/YtUWz5ZWlvRC9bUt6qZyOhK12wTT/0k=; h=From:To:Cc:Subject:Autocrypt:OpenPGP:Date:Message-ID:MIME-Version: Content-Type:From; b=CyXWq5ubfr/zGNC96HzjQd0dXW/JwuqI99/Cj+DkQXiD4Fy3JzVC7ORmjQ7vU+MJy apuujpiVlwKxxa24Yjjfnvlnb5wYFEH70cGwuSzlzPwsHzPOBW5D5uAE/49rwd/tdK L8npVq9gDO6I9yyjshSIgCcPLfkdKY3PKVh+zNV6l5h/J3Qygz/V4sfWR5hZ7HiIOM zKzF/ads5Q2w4QcEIVC1yTnZje9wU/VRdljglF6xjV5McGdEFXHN458R+yuvCRwQKY JhQB2WR8QRuMQ9aIxtN5Qlss1HwCR2LkoNmbugmJ7xqvvplz4TueAogFzD95/kPrZ7 s5wZFiyJWykZw== Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4ZdVmZ3kw3z6tm8; Thu, 17 Apr 2025 10:04:46 +0200 (CEST) From: Philip Kaludercic To: Spencer Baugh Subject: Re: bug#77834: [PATCH] Improve help-fns-edit-variable for Lisp editing In-Reply-To: References: <86plhcfsc7.fsf@gnu.org> Autocrypt: addr=philipk@posteo.net; keydata= mDMEZBBQQhYJKwYBBAHaRw8BAQdAHJuofBrfqFh12uQu0Yi7mrl525F28eTmwUDflFNmdui0QlBo aWxpcCBLYWx1ZGVyY2ljIChnZW5lcmF0ZWQgYnkgYXV0b2NyeXB0LmVsKSA8cGhpbGlwa0Bwb3N0 ZW8ubmV0PoiWBBMWCAA+FiEEDg7HY17ghYlni8XN8xYDWXahwukFAmQQUEICGwMFCQHhM4AFCwkI BwIGFQoJCAsCBBYCAwECHgECF4AACgkQ8xYDWXahwulikAEA77hloUiSrXgFkUVJhlKBpLCHUjA0 mWZ9j9w5d08+jVwBAK6c4iGP7j+/PhbkxaEKa4V3MzIl7zJkcNNjHCXmvFcEuDgEZBBQQhIKKwYB BAGXVQEFAQEHQI5NLiLRjZy3OfSt1dhCmFyn+fN/QKELUYQetiaoe+MMAwEIB4h+BBgWCAAmFiEE Dg7HY17ghYlni8XN8xYDWXahwukFAmQQUEICGwwFCQHhM4AACgkQ8xYDWXahwukm+wEA8cml4JpK NeAu65rg+auKrPOP6TP/4YWRCTIvuYDm0joBALw98AMz7/qMHvSCeU/hw9PL6u6R2EScxtpKnWof z4oM OpenPGP: id=7126E1DE2F0CE35C770BED01F2C3CC513DB89F66; url="https://keys.openpgp.org/vks/v1/by-fingerprint/7126E1DE2F0CE35C770BED01F2C3CC513DB89F66"; preference=signencrypt Date: Thu, 17 Apr 2025 08:04:46 +0000 Message-ID: <87cydb5i3l.fsf@posteo.net> MIME-Version: 1.0 Content-Type: text/plain X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 77834 Cc: 77834@debbugs.gnu.org, Eli Zaretskii , larsi@gnus.org, azeng@janestreet.com X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) Spencer Baugh writes: > Eli Zaretskii writes: >>> Cc: azeng@janestreet.com, Philip Kaludercic , >>> Lars Ingebrigtsen >>> 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" >>> >>> 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-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-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-done], or aborts with \\\\[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. From debbugs-submit-bounces@debbugs.gnu.org Thu Apr 17 04:06:07 2025 Received: (at 77834) by debbugs.gnu.org; 17 Apr 2025 08:06:08 +0000 Received: from localhost ([127.0.0.1]:45680 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1u5KG5-0002dS-W2 for submit@debbugs.gnu.org; Thu, 17 Apr 2025 04:06:07 -0400 Received: from mout02.posteo.de ([185.67.36.66]:50665) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1u5KG3-0002cF-Cp for 77834@debbugs.gnu.org; Thu, 17 Apr 2025 04:06:04 -0400 Received: from submission (posteo.de [185.67.36.169]) by mout02.posteo.de (Postfix) with ESMTPS id 1AC0A240101 for <77834@debbugs.gnu.org>; Thu, 17 Apr 2025 10:05:55 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.net; s=2017; t=1744877157; bh=wq6LL+HimAuNZWnn+nF5YAaHRtL/8FQdfNAGBKURFaw=; h=From:To:Cc:Subject:Autocrypt:OpenPGP:Date:Message-ID:MIME-Version: Content-Type:From; b=T19zk2Kgb7ZX/3qDLXEnKVqGdsDXzTUT9iuQNWZ4xjUk6POVx8yYlcoiMyW3HVjJE fr1OnqoUuR1eHiUXVyBLPkSlzxF+6WaMsatQRCzZozHcX6wQPExQxdcUOYVqT6iUf2 0Lt/3Aoy4u/Khi9dkXVMY0uyFKvmBKTlIHTGUczdJWJnbtRcjbaKmuC80facOFy9ke rde1/BZGhs11+oXtkZF1VyEWWGbTbA05ONx+XTZs/DeCrStcPXCuTJvOBGDYOpPJ5Y CkDA33KA5+DMDLt5JYfdDj9YDQpcdUlOq0qN2itB0cteQJ2gT5Oo717EQMmU3Z+wQO rUry+B+K83jJg== Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4ZdVnt3dQJz9rxK; Thu, 17 Apr 2025 10:05:54 +0200 (CEST) From: Philip Kaludercic To: Spencer Baugh Subject: Re: bug#77834: [PATCH] Improve help-fns-edit-variable for Lisp editing In-Reply-To: References: <86plhcfsc7.fsf@gnu.org> <867c3kf3cp.fsf@gnu.org> Autocrypt: addr=philipk@posteo.net; keydata= mDMEZBBQQhYJKwYBBAHaRw8BAQdAHJuofBrfqFh12uQu0Yi7mrl525F28eTmwUDflFNmdui0QlBo aWxpcCBLYWx1ZGVyY2ljIChnZW5lcmF0ZWQgYnkgYXV0b2NyeXB0LmVsKSA8cGhpbGlwa0Bwb3N0 ZW8ubmV0PoiWBBMWCAA+FiEEDg7HY17ghYlni8XN8xYDWXahwukFAmQQUEICGwMFCQHhM4AFCwkI BwIGFQoJCAsCBBYCAwECHgECF4AACgkQ8xYDWXahwulikAEA77hloUiSrXgFkUVJhlKBpLCHUjA0 mWZ9j9w5d08+jVwBAK6c4iGP7j+/PhbkxaEKa4V3MzIl7zJkcNNjHCXmvFcEuDgEZBBQQhIKKwYB BAGXVQEFAQEHQI5NLiLRjZy3OfSt1dhCmFyn+fN/QKELUYQetiaoe+MMAwEIB4h+BBgWCAAmFiEE Dg7HY17ghYlni8XN8xYDWXahwukFAmQQUEICGwwFCQHhM4AACgkQ8xYDWXahwukm+wEA8cml4JpK NeAu65rg+auKrPOP6TP/4YWRCTIvuYDm0joBALw98AMz7/qMHvSCeU/hw9PL6u6R2EScxtpKnWof z4oM OpenPGP: id=7126E1DE2F0CE35C770BED01F2C3CC513DB89F66; url="https://keys.openpgp.org/vks/v1/by-fingerprint/7126E1DE2F0CE35C770BED01F2C3CC513DB89F66"; preference=signencrypt Date: Thu, 17 Apr 2025 08:05:54 +0000 Message-ID: <878qnz5i1p.fsf@posteo.net> MIME-Version: 1.0 Content-Type: text/plain X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 77834 Cc: 77834@debbugs.gnu.org, Eli Zaretskii , larsi@gnus.org, azeng@janestreet.com X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) Spencer Baugh 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? [...] From debbugs-submit-bounces@debbugs.gnu.org Thu Apr 17 16:17:13 2025 Received: (at 77834) by debbugs.gnu.org; 17 Apr 2025 20:17:14 +0000 Received: from localhost ([127.0.0.1]:48557 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1u5Vfc-0005fp-24 for submit@debbugs.gnu.org; Thu, 17 Apr 2025 16:17:13 -0400 Received: from mxout5.mail.janestreet.com ([64.215.233.18]:43487) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1u5VfW-0005dX-I5 for 77834@debbugs.gnu.org; Thu, 17 Apr 2025 16:17:09 -0400 From: Spencer Baugh To: Philip Kaludercic Subject: Re: bug#77834: [PATCH] Improve help-fns-edit-variable for Lisp editing In-Reply-To: <878qnz5i1p.fsf@posteo.net> (Philip Kaludercic's message of "Thu, 17 Apr 2025 08:05:54 +0000") References: <86plhcfsc7.fsf@gnu.org> <867c3kf3cp.fsf@gnu.org> <878qnz5i1p.fsf@posteo.net> Date: Thu, 17 Apr 2025 16:17:01 -0400 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=janestreet.com; s=waixah; t=1744921021; bh=rPEKL/QAHli5XKOJy+MFDH/j9Ta+xdF6zVzUHOGygVY=; h=From:To:Cc:Subject:In-Reply-To:References:Date; b=YCWnDcJr7/ru3dPsDChBbvyODFK150zmOaxbpmf7bMT4gA+e8c4Y07GsM6yKAc0w/ qYWIi2ev1F9Zu9p/yRTharqsh0fUfJzABRmeQvIa2S9xQCWRQIkGFlsgzJ1gBhHV4U OrqqDcFZruCredkujJ0gn8rfW877sTzctdX9sEmpvTX6kFz9ancUOH0z3GtoSTTgSB oIsfD+jSGf8JQhnYBsHqUeLxxVncF+hvgb2jS4IEypo9MsPrAsHzn65E3H351/3D1e wdnGpRBDE2Zqr7ynYxa1wv6BXqqXyLy810N10iaNLXP7rUcqiSaqcimezz3HD1MDZr aa4BykZPS+UFQ== X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 77834 Cc: 77834@debbugs.gnu.org, Eli Zaretskii , larsi@gnus.org, azeng@janestreet.com X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) Philip Kaludercic writes: > Spencer Baugh 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. From debbugs-submit-bounces@debbugs.gnu.org Sat Apr 26 07:56:04 2025 Received: (at 77834-done) by debbugs.gnu.org; 26 Apr 2025 11:56:04 +0000 Received: from localhost ([127.0.0.1]:58973 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1u8e8Z-00062r-Ia for submit@debbugs.gnu.org; Sat, 26 Apr 2025 07:56:04 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:34800) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1u8e8V-00062F-Nc for 77834-done@debbugs.gnu.org; Sat, 26 Apr 2025 07:56:00 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1u8e8O-00036d-TO; Sat, 26 Apr 2025 07:55:52 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date: mime-version; bh=0frzqvoe4Y1z9x04hMIBq2SiB8VnzKte7r1OBrz17XM=; b=ajdDkIThfQXY sofYGNWgQSsthwSodkzDLE524amKQ/epxWKkfwmXSwmUTFIGHlr6DkGx6CxfKZOaZAfSKzofdBZDx 4nZQBAMEB6f91m6Db1sPwU/EpCo+fWsBXPvnHUpg2qIiroS5IeESRtUy1o/WpzsFg6v58aWx+QbtH 8bQHG9/x2GMedaMLE7fSenKRApQEAjIh65WA9V19HOHA7x5WEzGvNq8y3pTZk01YIbx2BXPOlfGlX vHU5AcbCZewL9P+g9p4XpdU8e9GscmGVxIztIRRupEJBAH4GilAL9qGGDjcxFtAdC1ISSIhhBrNqe GZn9sgEO497tjygYlf4f4A==; Date: Sat, 26 Apr 2025 14:55:46 +0300 Message-Id: <8634dvuofx.fsf@gnu.org> From: Eli Zaretskii To: Spencer Baugh In-Reply-To: (message from Spencer Baugh on Wed, 16 Apr 2025 13:31:39 -0400) Subject: Re: bug#77834: [PATCH] Improve help-fns-edit-variable for Lisp editing References: <86plhcfsc7.fsf@gnu.org> <867c3kf3cp.fsf@gnu.org> X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 77834-done Cc: 77834-done@debbugs.gnu.org, larsi@gnus.org, philipk@posteo.net, azeng@janestreet.com X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) > From: Spencer Baugh > Cc: 77834@debbugs.gnu.org, larsi@gnus.org, philipk@posteo.net, > azeng@janestreet.com > Date: Wed, 16 Apr 2025 13:31:39 -0400 > > Eli Zaretskii writes: > > >> From: Spencer Baugh > >> Cc: 77834@debbugs.gnu.org, larsi@gnus.org, philipk@posteo.net, > >> azeng@janestreet.com > >> 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-done], or aborts with \\\\[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. From unknown Fri Jun 20 07:13:03 2025 Received: (at fakecontrol) by fakecontrolmessage; To: internal_control@debbugs.gnu.org From: Debbugs Internal Request Subject: Internal Control Message-Id: bug archived. Date: Sun, 25 May 2025 11:24:17 +0000 User-Agent: Fakemail v42.6.9 # This is a fake control message. # # The action: # bug archived. thanks # This fakemail brought to you by your local debbugs # administrator