From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: miha--- via "Bug reports for GNU Emacs, the Swiss army knife of text editors" Newsgroups: gmane.emacs.bugs Subject: bug#49700: 27.2; [PATCH] Refactor minibuffer aborting Date: Sun, 01 Aug 2021 03:23:32 +0200 Message-ID: <87czqyhsa3.fsf@miha-pc> References: <87pmvaar0a.fsf@miha-pc> Reply-To: Mime-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-=" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="9451"; mail-complaints-to="usenet@ciao.gmane.io" To: 49700@debbugs.gnu.org Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sun Aug 01 03:22:12 2021 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1mA0B8-0002Ea-EK for geb-bug-gnu-emacs@m.gmane-mx.org; Sun, 01 Aug 2021 03:22:10 +0200 Original-Received: from localhost ([::1]:46306 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mA0B6-0004nU-Qs for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 31 Jul 2021 21:22:08 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:57036) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mA0B0-0004n8-7m for bug-gnu-emacs@gnu.org; Sat, 31 Jul 2021 21:22:02 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:51906) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1mA0Az-0001R4-TN for bug-gnu-emacs@gnu.org; Sat, 31 Jul 2021 21:22:01 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1mA0Az-000130-RH for bug-gnu-emacs@gnu.org; Sat, 31 Jul 2021 21:22:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sun, 01 Aug 2021 01:22:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 49700 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 49700-submit@debbugs.gnu.org id=B49700.16277808733968 (code B ref 49700); Sun, 01 Aug 2021 01:22:01 +0000 Original-Received: (at 49700) by debbugs.gnu.org; 1 Aug 2021 01:21:13 +0000 Original-Received: from localhost ([127.0.0.1]:35219 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mA0AC-00011v-RZ for submit@debbugs.gnu.org; Sat, 31 Jul 2021 21:21:13 -0400 Original-Received: from kamnitnik.top ([209.250.245.214]:52030 helo=mail.kamnitnik.top) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mA0AB-00011n-BE for 49700@debbugs.gnu.org; Sat, 31 Jul 2021 21:21:12 -0400 Original-Received: from localhost (unknown [IPv6:2a00:ee2:e04:9300:e609:6c46:d026:8c47]) by mail.kamnitnik.top (Postfix) with ESMTPSA id ADEB2BBB71 for <49700@debbugs.gnu.org>; Sun, 1 Aug 2021 01:21:09 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kamnitnik.top; s=mail; t=1627780869; bh=SxNY8po8B11bDuGAe9DMLz6ZtqT4tYRG/6AbMYPHYWY=; h=From:To:Subject:In-Reply-To:References:Date:From; b=Pwect0Ica1xV3yeX6IFv8e6uEVu47y/wFlKI8+4DYEZMLeGewK2YqR3SyaoOj0Z3N NsVf4dZvMhTj1wgPjokf8WohhxmixYM2ReT3onc5ARn92YwckMqudpZc7atRu9fChW GE4zOWF80qcQISqfdR/DQarL6zUVzGr8dv6LRL7VBaBZ4rGjv0pohhCq+AtipQwffx GUe13s7hUk0IOlq0tVW4iCAML5p6ayzFvPKM4guyypjynxcbpLyJB0SZdk6vzwa/gj pyyobB4745bEirFIMpL6R54/CRjhzzyGxULU0TssvFqG06pMHFHU3wMDN5XPl7GCAF y/5wZ350ZCMEQ== In-Reply-To: X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:211032 Archived-At: --=-=-= Content-Type: multipart/mixed; boundary="==-=-=" --==-=-= Content-Type: text/plain Content-Disposition: inline Alan Mackenzie writes: > Hello, Miha. > > On Fri, Jul 23, 2021 at 01:05:41 +0200, miha@kamnitnik.top wrote: >> The attached patch removes special handling of the 'exit tag from >> internal_catch. This special handling was introduced by Alan in commit >> Sun Jan 10 20:32:40 2021 +0000 >> (c7c154bb5756e0ae71d342c5d8aabf725877f186), hence me CC-ing him. > > Thanks, that's appreciated. > > I'm not sure I'm in favour of the change as a whole, since the proposed > code contains complexities (as does the code it would replace). I find > the use of the closures difficult to understand. But then again, I wrote > the old code, so I'm not in a position to judge whether the old or the > new is "better". > >> It also exposes Vminibuffer_list to lisp through the new function >> Fminibuffer_alist. > > Like Eli, I'm against this. Indeed, when I was modifying the minibuffer > code, I took great care to avoid Vminibuffer_list becoming visible to > Lisp. As a result, some of the current code is less elegant than it > might have been. The idea of some Lisp looping through all existing > minibuffers doing something destructive didn't help me sleep well. > > As a general point, I'm a bit worried you might not be distinguishing > between (minibuffer-depth) and (recursive-depth). They are only the same > most of the time. When (recursive-edit) gets called outside of the > minibuffer code, then these two values are different. For example, in > abort-minibuffers, you've got > >> + (when (yes-or-no-p >> + (format "Abort %s minibuffer levels? " >> + (- (recursion-depth) minibuffer-level -1))) > > .. minibuffer-level is confusingly a result of (recursion-depth), not > (minibuffer-depth), so the code isn't prompting with the number of > minibuffer levels to be aborted, but the number of recursive edits. > > As a small point, the use of cl-decf: > >> + (cl-decf minibuffer-level))) > > might be unwise. Have you checked that it works in a bootstrap build? > My fear is that in a bootstrap, minibuffer.el might be compiled before > the CL files, and then cl-decf would be wrongly compiled as a function > call rather than a macro expansion. But I haven't checked it myself. Thanks for feedback. Attached is a patch that should address all of these issues. Overall, this patch is much simpler than the original patch I proposed and the closure passing should now be hopefully easier to understand. > I've also had a look a part of your patch from Tuesday (2021-07-20), and > am unhappy about some aspects of the change to the documentation on the > Elisp manual page Recursive Editing. For example, the text no longer > says what happens on throwing a random value to 'exit (but it used to). > Also this text is generally a bit unclear; what does "a function value" > mean? I would normally understand "the value returned by a function", > but here it just means the function. But I think it would be better for > me to raise these issues in a different thread. > Please take a look at the second patch, attached to this message. I tried to improve the documentation of exiting a recursive edit in lispref. I also adjusted the doc string of the function `recursive-edit', which I forgot to do in my older patch from 2021-07-20. > > -- > Alan Mackenzie (Nuremberg, Germany). --==-=-= Content-Type: text/x-patch Content-Disposition: inline; filename=0001-Refactor-minibuffer-aborting.patch >From 91276c2485b518850b2d0d02be1823439571a3f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miha=20Rihtar=C5=A1i=C4=8D?= Date: Sat, 31 Jul 2021 13:44:21 +0200 Subject: [PATCH] Refactor minibuffer aborting * lisp/minibuffer.el (minibuffer-quit-recursive-edit): New optional argument to specify how many levels of recursion to quit. * src/minibuf.c (Fabort_minibuffers): Use minibuffer-quit-recursive-edit to quit multiple levels of minibuffer recursion. * src/eval.c (internal_catch): Remove special handling of 'exit tag. --- lisp/minibuffer.el | 16 ++++++++++------ src/eval.c | 22 ---------------------- src/lisp.h | 1 - src/minibuf.c | 4 ++-- 4 files changed, 12 insertions(+), 31 deletions(-) diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el index 3751ba80e0..912e186b06 100644 --- a/lisp/minibuffer.el +++ b/lisp/minibuffer.el @@ -2328,14 +2328,18 @@ exit-minibuffer (setq deactivate-mark nil) (throw 'exit nil)) -(defun minibuffer-quit-recursive-edit () +(defun minibuffer-quit-recursive-edit (&optional levels) "Quit the command that requested this recursive edit without error. Like `abort-recursive-edit' without aborting keyboard macro -execution." - ;; See Info node `(elisp)Recursive Editing' for an explanation of - ;; throwing a function to `exit'. - (throw 'exit (lambda () - (signal 'minibuffer-quit nil)))) +execution. LEVELS specifies the number of nested recursive edits +to quit. If nil, it defaults to 1." + (unless levels + (setq levels 1)) + (if (> levels 1) + ;; See Info node `(elisp)Recursive Editing' for an explanation + ;; of throwing a function to `exit'. + (throw 'exit (lambda () (minibuffer-quit-recursive-edit (1- levels)))) + (throw 'exit (lambda () (signal 'minibuffer-quit nil))))) (defun self-insert-and-exit () "Terminate minibuffer input." diff --git a/src/eval.c b/src/eval.c index 48104bd0f4..76fe671b6d 100644 --- a/src/eval.c +++ b/src/eval.c @@ -1174,14 +1174,6 @@ #define clobbered_eassert(E) verify (sizeof (E) != 0) FUNC should return a Lisp_Object. This is how catches are done from within C code. */ -/* MINIBUFFER_QUIT_LEVEL is to handle quitting from nested minibuffers by - throwing t to tag `exit'. - 0 means there is no (throw 'exit t) in progress, or it wasn't from - a minibuffer which isn't the most nested; - N > 0 means the `throw' was done from the minibuffer at level N which - wasn't the most nested. */ -EMACS_INT minibuffer_quit_level = 0; - Lisp_Object internal_catch (Lisp_Object tag, Lisp_Object (*func) (Lisp_Object), Lisp_Object arg) @@ -1189,9 +1181,6 @@ internal_catch (Lisp_Object tag, /* This structure is made part of the chain `catchlist'. */ struct handler *c = push_handler (tag, CATCHER); - if (EQ (tag, Qexit)) - minibuffer_quit_level = 0; - /* Call FUNC. */ if (! sys_setjmp (c->jmp)) { @@ -1205,17 +1194,6 @@ internal_catch (Lisp_Object tag, Lisp_Object val = handlerlist->val; clobbered_eassert (handlerlist == c); handlerlist = handlerlist->next; - if (EQ (tag, Qexit) && EQ (val, Qt) && minibuffer_quit_level > 0) - /* If we've thrown t to tag `exit' from within a minibuffer, we - exit all minibuffers more deeply nested than the current - one. */ - { - if (minibuf_level > minibuffer_quit_level - && !NILP (Fminibuffer_innermost_command_loop_p (Qnil))) - Fthrow (Qexit, Qt); - else - minibuffer_quit_level = 0; - } return val; } } diff --git a/src/lisp.h b/src/lisp.h index 15a42a4456..4fdee6c280 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -4112,7 +4112,6 @@ intern_c_string (const char *str) } /* Defined in eval.c. */ -extern EMACS_INT minibuffer_quit_level; extern Lisp_Object Vautoload_queue; extern Lisp_Object Vrun_hooks; extern Lisp_Object Vsignaling_function; diff --git a/src/minibuf.c b/src/minibuf.c index 0f4349e70b..f7cd2c5fcc 100644 --- a/src/minibuf.c +++ b/src/minibuf.c @@ -491,8 +491,8 @@ DEFUN ("abort-minibuffers", Fabort_minibuffers, Sabort_minibuffers, 0, 0, "", array[1] = make_fixnum (minibuf_level - minibuf_depth + 1); if (!NILP (Fyes_or_no_p (Fformat (2, array)))) { - minibuffer_quit_level = minibuf_depth; - Fthrow (Qexit, Qt); + CALLN (Ffuncall, intern ("minibuffer-quit-recursive-edit"), + array[1]); } } else -- 2.32.0 --==-=-= Content-Type: text/x-patch Content-Disposition: inline; filename=0001-Improve-documentation-of-exiting-recursive-editing.patch >From bdccb4d9399090ffe08cbdde289b5a1afd0c310d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miha=20Rihtar=C5=A1i=C4=8D?= Date: Sun, 1 Aug 2021 02:41:37 +0200 Subject: [PATCH] Improve documentation of exiting recursive editing * doc/lispref/commands.texi (Recursive Editing): Mention what happens when throwing a string or any other value to 'exit. * src/keyboard.c (Frecursive_edit): Document throwing a function to 'exit. --- doc/lispref/commands.texi | 22 ++++++++++++---------- src/keyboard.c | 18 ++++++++++++++---- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/doc/lispref/commands.texi b/doc/lispref/commands.texi index b4a8b733a0..e0982b14bb 100644 --- a/doc/lispref/commands.texi +++ b/doc/lispref/commands.texi @@ -3568,17 +3568,19 @@ Recursive Editing @cindex exit recursive editing @cindex aborting To invoke a recursive editing level, call the function -@code{recursive-edit}. This function contains the command loop; it also -contains a call to @code{catch} with tag @code{exit}, which makes it -possible to exit the recursive editing level by throwing to @code{exit} -(@pxref{Catch and Throw}). If you throw a @code{nil} value, then -@code{recursive-edit} returns normally to the function that called it. -The command @kbd{C-M-c} (@code{exit-recursive-edit}) does this. -Throwing a @code{t} value causes @code{recursive-edit} to quit, so that -control returns to the command loop one level up. This is called -@dfn{aborting}, and is done by @kbd{C-]} (@code{abort-recursive-edit}). -You can also throw a function value. In that case, +@code{recursive-edit}. This function contains the command loop; it +also contains a call to @code{catch} with tag @code{exit}, which makes +it possible to exit the recursive editing level by throwing to +@code{exit} (@pxref{Catch and Throw}). Throwing a @code{t} value +causes @code{recursive-edit} to quit, so that control returns to the +command loop one level up. This is called @dfn{aborting}, and is done +by @kbd{C-]} (@code{abort-recursive-edit}). Similarly, you can throw +a string value to make @code{recursive-edit} signal an error, printing +this string as the message. If you throw a function, @code{recursive-edit} will call it without arguments before returning. +Throwing any other value, will make @code{recursive-edit} return +normally to the function that called it. The command @kbd{C-M-c} +(@code{exit-recursive-edit}) does this. Most applications should not use recursive editing, except as part of using the minibuffer. Usually it is more convenient for the user if you diff --git a/src/keyboard.c b/src/keyboard.c index 820229cf8f..06509bcb72 100644 --- a/src/keyboard.c +++ b/src/keyboard.c @@ -753,10 +753,20 @@ DEFUN ("recursive-edit", Frecursive_edit, Srecursive_edit, 0, 0, "", doc: /* Invoke the editor command loop recursively. To get out of the recursive edit, a command can throw to `exit' -- for instance (throw \\='exit nil). -If you throw a value other than t, `recursive-edit' returns normally -to the function that called it. Throwing a t value causes -`recursive-edit' to quit, so that control returns to the command loop -one level up. + +The following values can be thrown to 'exit: + +- t causes `recursive-edit' to quit, so that control returns to the + command loop one level up. + +- A string causes `recursive-edit' to signal an error, printing this + string as the message. + +- A function causes `recursive-edit' to call this function without + arguments before returning normally. + +- Any other value causes `recursive-edit' to return normally to the + function that called it. This function is called by the editor initialization to begin editing. */) (void) -- 2.32.0 --==-=-=-- --=-=-= Content-Type: application/pgp-signature; name=signature.asc Content-Transfer-Encoding: base64 LS0tLS1CRUdJTiBQR1AgU0lHTkFUVVJFLS0tLS0KCmlRSkhCQUVCQ0FBeEZpRUVteFZuZXNvVDVy UVh2Vlhuc3drYUdwSVZtVDhGQW1FRjlEQVRIRzFwYUdGQWEyRnQKYm1sMGJtbHJMblJ2Y0FBS0NS Q3pDUm9ha2hXWlA1ZmNELzBiTm5TdE9EYThJTUZMWEFjaHpFWTgyYWtUOVFubQpuNitoYW9QalBB dWJ6MUd4V0t5VGVhNEdlYzZ0TU1mdE1iei84blB6bmFaV05tWTJhZVg0TGMyd3d2QXQyN2dOCitS UWIxV0x1YWxCcUQ0cWloOTBleHJOMEFwZ1BFRTA1N1RDMXcxYWtRc21Ob2xtcG5GSE90L0EyK1NF ME9jR2wKK0cvWERlbjZmaGo2cGZNUzZ3OGxYbU9udFhjM2NjVVNCYXU3SVpSdWtLNHdLcGM5QnlF cDkwUG5sSks1eU9QSQpvc3NVb3BYQTdQWUlaNG9jc0k3Y290TmZZWDRhc1ZqSDArdUJUakdIenVP Tk5kdjIwNXUydHMzUXU5dlVscmswCjI2ZGZmK0F4SkJlSEh0Z1JNcTBudWJZMHVNS1FhT1hCZDRz TkQwV3pVaVNDeWRMTU9ndFdCSzFITVlkNWZKVnUKNTNjQUFpM3NkdHA5aVNncklNYkNiazVrWkY3 b3lFblh0aWE0TnBRK0RWZm9hUGUwYk11WVJvR09WbnNONGVkRAplUmVJNE1CZXJwOXlDZytmY0h0 MWJ3ZVpiQ05KV0R4ZGJySmNvUmJCOGJqTTVpN2t0T251bXo3YkxqYlpwa0JqCkhudURVczJ4M2w0 ZndHVEo4UHhmRjR0VmNib1grMzA5Qi9vdmRRUW9YM085Nzg5ZzBjQnVtK2F4NTFCOFA3ZGUKaCtO aC9qeHU5a2RNalVMeUVNS1BUVUZna05lcHR1S3JUSytwRStJUzYrNVU2di8xR3hnTUQzdjcyZTky K1ZBMwpNOG16QjlNRDRZWmJRZHE4QzFkSm4rZmFLaVhCMVRNdDhQc29uSm15dC9QZzFPOUZFWCs5 anFXeHpqbm1BOWlxCmVYS09laWI1VmdOMEVRPT0KPWthWE4KLS0tLS1FTkQgUEdQIFNJR05BVFVS RS0tLS0t --=-=-=--