Alan Mackenzie <acm@muc.de> 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).