bug#49700: 27.2; [PATCH] Refactor minibuffer aborting

[Alan Mackenzie]

Hello, Miha.

Apologies for loosing track of this thread a few weeks ago.

I spent quite some time trying to find things wrong with your latest
patch, but couldn't.  ;-)  So although I haven't fully tested it, I would
be in favour of merging it to master.

-- 
Alan Mackenzie (Nuremberg, Germany).


On Fri, Sep 17, 2021 at 23:47:35 +0200, miha@kamnitnik.top wrote:
> miha--- via "Bug reports for GNU Emacs, the Swiss army knife of text
> editors" <bug-gnu-emacs@gnu.org> writes:

> > True, my code would indeed be incorrect if the number of minibuffer
> > levels to be aborted weren't to match the number of recursive edits.
> > However, in such cases, my code isn't reached because an error is
> > signaled that the current minibuffer isn't in the innermost command
> > loop.

> > Sorry for not clarifying this earlier. Would it would be enough to
> > include the following comment in the code?:

> > /* Due to the above check, the current minibuffer is in the most nested
> >    command loop, which means that we don't have to abort any extra
> >    non-minibuffer recursive edits.  Thus, the number of recursive edits
> >    we have to abort equals the number of minibuffers we have to abort.
> >    */

> > [...]


> > Perhaps the following adjustment to the doc string could be considered
> > (to be applied on top of the latest patch). It copies the beginning of
> > abort-recursive-edit's doc string and removes the link to it.

> > [...]

> I realized that my proposals are kind of scattered over multiple e-mails
> in this thread. So I'm sending them again, attached in this mail as
> complete patches that apply to current master.

> The first patch refactors abort-minibuffers
> (and improves minibuffer-quit-recursive-edit's doc string as requested).

> The second patch improves documentation of recursive editing.

> Best regards.


> From fd5be298c67157822b35cd0231c65691c48dc29e Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Miha=20Rihtar=C5=A1i=C4=8D?= <miha@kamnitnik.top>
> 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 | 20 ++++++++++++--------
>  src/eval.c         | 22 ----------------------
>  src/lisp.h         |  1 -
>  src/minibuf.c      |  9 +++++++--
>  4 files changed, 19 insertions(+), 33 deletions(-)

> diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
> index 9668e7c732..b5c0054a3c 100644
> --- a/lisp/minibuffer.el
> +++ b/lisp/minibuffer.el
> @@ -2349,14 +2349,18 @@ minibuffer-restore-windows
>  
>  (add-hook 'minibuffer-exit-hook 'minibuffer-restore-windows)
>  
> -(defun minibuffer-quit-recursive-edit ()
> -  "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))))
> +(defun minibuffer-quit-recursive-edit (&optional levels)
> +  "Quit the command that requested this recursive edit or minibuffer input.
> +Do so without terminating keyboard macro recording or 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 7bfc69b647..4e6298a101 100644
> --- a/src/lisp.h
> +++ b/src/lisp.h
> @@ -4113,7 +4113,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 0e7baf30dc..a4219d2a63 100644
> --- a/src/minibuf.c
> +++ b/src/minibuf.c
> @@ -491,8 +491,13 @@ 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);
> +       /* Due to the above check, the current minibuffer is in the
> +          most nested command loop, which means that we don't have
> +          to abort any extra non-minibuffer recursive edits.  Thus,
> +          the number of recursive edits we have to abort equals the
> +          number of minibuffers we have to abort.  */
> +       CALLN (Ffuncall, intern ("minibuffer-quit-recursive-edit"),
> +              array[1]);
>       }
>      }
>    else
> -- 
> 2.33.0


> From 387baa2b42e8220b0aac36f6f23ec3547ad0811e Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Miha=20Rihtar=C5=A1i=C4=8D?= <miha@kamnitnik.top>
> 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 ddd74d1245..3425880fec 100644
> --- a/doc/lispref/commands.texi
> +++ b/doc/lispref/commands.texi
> @@ -3585,17 +3585,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 63bf29a948..2d97429ade 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.33.0