From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Alan Mackenzie Newsgroups: gmane.emacs.bugs Subject: bug#49700: 27.2; [PATCH] Refactor minibuffer aborting Date: Fri, 23 Jul 2021 21:03:33 +0000 Message-ID: References: <87pmvaar0a.fsf@miha-pc> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="12856"; 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 Fri Jul 23 23:04: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 1m72L5-00037Q-E0 for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 23 Jul 2021 23:04:11 +0200 Original-Received: from localhost ([::1]:35414 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1m72L3-0002M7-Fu for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 23 Jul 2021 17:04:09 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:53772) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1m72Kw-0002Ls-WB for bug-gnu-emacs@gnu.org; Fri, 23 Jul 2021 17:04:03 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:33194) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1m72Kw-0003MP-Nx for bug-gnu-emacs@gnu.org; Fri, 23 Jul 2021 17:04:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1m72Kw-0002D5-IY for bug-gnu-emacs@gnu.org; Fri, 23 Jul 2021 17:04:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Alan Mackenzie Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 23 Jul 2021 21:04:02 +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.16270742258437 (code B ref 49700); Fri, 23 Jul 2021 21:04:02 +0000 Original-Received: (at 49700) by debbugs.gnu.org; 23 Jul 2021 21:03:45 +0000 Original-Received: from localhost ([127.0.0.1]:44740 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1m72Ke-0002Bz-TJ for submit@debbugs.gnu.org; Fri, 23 Jul 2021 17:03:45 -0400 Original-Received: from colin.muc.de ([193.149.48.1]:44209 helo=mail.muc.de) by debbugs.gnu.org with smtp (Exim 4.84_2) (envelope-from ) id 1m72Kb-0002BF-9P for 49700@debbugs.gnu.org; Fri, 23 Jul 2021 17:03:43 -0400 Original-Received: (qmail 36164 invoked by uid 3782); 23 Jul 2021 21:03:34 -0000 Original-Received: from acm.muc.de (p4fe154d9.dip0.t-ipconnect.de [79.225.84.217]) (using STARTTLS) by colin.muc.de (tmda-ofmipd) with ESMTP; Fri, 23 Jul 2021 23:03:33 +0200 Original-Received: (qmail 13838 invoked by uid 1000); 23 Jul 2021 21:03:33 -0000 Content-Disposition: inline In-Reply-To: <87pmvaar0a.fsf@miha-pc> X-Submission-Agent: TMDA/1.3.x (Ph3nix) X-Primary-Address: acm@muc.de 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:210614 Archived-At: 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. 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. I haven't yet spent enough time looking at your patch. Perhaps I'll manage it in the next week. -- Alan Mackenzie (Nuremberg, Germany).