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.devel Subject: Re: Stop frames stealing eachothers' minibuffers! Date: Thu, 11 Feb 2021 11:44:34 +0000 Message-ID: References: <87wnvkixrv.fsf@miha-pc> <874kinakv2.fsf@miha-pc> <87sg6690vc.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="23655"; mail-complaints-to="usenet@ciao.gmane.io" Cc: emacs-devel@gnu.org To: jakanakaevangeli Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Thu Feb 11 12:45:35 2021 Return-path: Envelope-to: ged-emacs-devel@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 1lAAPe-00061e-9r for ged-emacs-devel@m.gmane-mx.org; Thu, 11 Feb 2021 12:45:34 +0100 Original-Received: from localhost ([::1]:46964 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lAAPd-0002p0-9Q for ged-emacs-devel@m.gmane-mx.org; Thu, 11 Feb 2021 06:45:33 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:45900) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lAAOp-0002Nx-FI for emacs-devel@gnu.org; Thu, 11 Feb 2021 06:44:43 -0500 Original-Received: from colin.muc.de ([193.149.48.1]:44976 helo=mail.muc.de) by eggs.gnu.org with smtp (Exim 4.90_1) (envelope-from ) id 1lAAOk-0001rk-7b for emacs-devel@gnu.org; Thu, 11 Feb 2021 06:44:43 -0500 Original-Received: (qmail 28829 invoked by uid 3782); 11 Feb 2021 11:44:35 -0000 Original-Received: from acm.muc.de (p4fe15592.dip0.t-ipconnect.de [79.225.85.146]) (using STARTTLS) by colin.muc.de (tmda-ofmipd) with ESMTP; Thu, 11 Feb 2021 12:44:35 +0100 Original-Received: (qmail 23576 invoked by uid 1000); 11 Feb 2021 11:44:34 -0000 Content-Disposition: inline In-Reply-To: <87sg6690vc.fsf@miha-pc> X-Submission-Agent: TMDA/1.3.x (Ph3nix) X-Primary-Address: acm@muc.de Received-SPF: pass client-ip=193.149.48.1; envelope-from=acm@muc.de; helo=mail.muc.de X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.io gmane.emacs.devel:264364 Archived-At: Hello, jakanakaevangeli. On Mon, Feb 08, 2021 at 13:53:43 +0100, jakanakaevangeli wrote: > Alan Mackenzie writes: > Hello > > I spent some time boring into the doc string and the description in the > > Elisp manual. The only conclusion I could come to was that C-] is meant > > to abort EXACTLY ONE level. > > So I have tweaked the C sources once more and come up with the following > > patch. It should apply to the Emacs state after yesterday's patch. > > Please remove the patch from earlier today before you try to apply the > > new patch. Thanks! > During testing, I didn't encounter any problems related to your latest > patch so you hard work paid off. > Sadly, I did encounter 2 additional minibuffer issues which aren't > related to your latest two patches, that is, they are present regardless > if these patches are applied or not. I'm posting them here since they > are still possibly related to the new minibuffer frame-following > functionality. Thanks! > 1) With minibuffer-follows-selected-frame set to t (the default value): > - press M-x on frame A > - select frame B (the minibuffer will move to this frame) > - C-x o, to select the minibuffer > - C-g to quit it > Miniwindow stays selected and its buffer is *Minibuf-1* instead of > *Minibuf-0*. You can check this by evaluating (minibuffer-window). This one was somewhat tricky to resolve, my fixes uncovering several other inconsistencies in the minibuffer handling, which I hope I have now fixed. > 2) With minibuffer-follows-selected-frame set to nil: > - (setq set enable-recursive-minibuffers t) > - (minibuffer-depth-indicate-mode 1) > - select frame A and press M-x > - select frame B and press M-x > - select frame A and close it > - select frame B and quit its minibuffer with C-g. > This doesn't quit the outer minibuffer, as expected, but this minibuffer > isn't shown anywhere and the only reasonable way to quit it is with C-], > which, I believe, a lot of users don't know about (at least I personally > didn't until quite recently). You can check this with > (minibuffer-depth). This is more involved. What do we want to happen when a frame with open minibuffers is deleted? I would say that these minibuffers should, except in the (eq minibuffer-follows-selected-frame t) case, be aborted, together with any others in other frames whose nesting level makes this necessary. Then there're complications with recursive edits. But I haven't looked at implementing this, yet. Alternatively, maybe open minibuffers should be moved to the "previous" frame. Perhaps I should open another thread on emacs-devel for this problem. Anyhow, I have a patch for bug 1. It is a full diff from master, rather than a difference from "yesterday". I would be very grateful indeed if you could try it out, and let me know again how well it's working. I would really like to commit this to master. Thanks again! diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el index a899a943d4..aacb8ab00b 100644 --- a/lisp/minibuffer.el +++ b/lisp/minibuffer.el @@ -2116,18 +2116,19 @@ minibuffer-hide-completions (defun exit-minibuffer () "Terminate this minibuffer argument." (interactive) - (when (or - (innermost-minibuffer-p) - (not (minibufferp))) + (when (minibufferp) + (when (not (minibuffer-innermost-command-loop-p)) + (error "%s" "Not in most nested command loop")) + (when (not (innermost-minibuffer-p)) + (error "%s" "Not in most nested minibuffer"))) ;; If the command that uses this has made modifications in the minibuffer, ;; we don't want them to cause deactivation of the mark in the original ;; buffer. ;; A better solution would be to make deactivate-mark buffer-local ;; (or to turn it into a list of buffers, ...), but in the mean time, ;; this should do the trick in most cases. - (setq deactivate-mark nil) - (throw 'exit nil)) - (error "%s" "Not in most nested minibuffer")) + (setq deactivate-mark nil) + (throw 'exit nil)) (defun self-insert-and-exit () "Terminate minibuffer input." diff --git a/src/eval.c b/src/eval.c index 3aff3b56d5..91fc4e6837 100644 --- a/src/eval.c +++ b/src/eval.c @@ -1165,21 +1165,23 @@ usage: (catch TAG BODY...) */) 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) { - /* MINIBUFFER_QUIT_LEVEL is to handle quitting from nested minibuffers by - throwing t to tag `exit'. - Value -1 means there is no (throw 'exit t) in progress; - 0 means the `throw' wasn't done from an active minibuffer; - N > 0 means the `throw' was done from the minibuffer at level N. */ - static EMACS_INT minibuffer_quit_level = -1; /* This structure is made part of the chain `catchlist'. */ struct handler *c = push_handler (tag, CATCHER); if (EQ (tag, Qexit)) - minibuffer_quit_level = -1; + minibuffer_quit_level = 0; /* Call FUNC. */ if (! sys_setjmp (c->jmp)) @@ -1194,22 +1196,16 @@ internal_catch (Lisp_Object tag, Lisp_Object val = handlerlist->val; clobbered_eassert (handlerlist == c); handlerlist = handlerlist->next; - if (EQ (tag, Qexit) && EQ (val, Qt)) + 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. */ { - EMACS_INT mini_depth = this_minibuffer_depth (Qnil); - if (mini_depth && mini_depth != minibuffer_quit_level) - { - if (minibuffer_quit_level == -1) - minibuffer_quit_level = mini_depth; - if (minibuffer_quit_level - && (minibuf_level > minibuffer_quit_level)) - Fthrow (Qexit, Qt); - } + if (minibuf_level > minibuffer_quit_level + && !NILP (Fminibuffer_innermost_command_loop_p (Qnil))) + Fthrow (Qexit, Qt); else - minibuffer_quit_level = -1; + minibuffer_quit_level = 0; } return val; } diff --git a/src/lisp.h b/src/lisp.h index 409a1e7060..0847324d1f 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -4091,6 +4091,7 @@ 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; @@ -4369,6 +4370,7 @@ extern void syms_of_casetab (void); /* Defined in keyboard.c. */ +extern EMACS_INT command_loop_level; extern Lisp_Object echo_message_buffer; extern struct kboard *echo_kboard; extern void cancel_echoing (void); diff --git a/src/minibuf.c b/src/minibuf.c index 949c3d989d..4b1f4b1ff7 100644 --- a/src/minibuf.c +++ b/src/minibuf.c @@ -41,6 +41,7 @@ along with GNU Emacs. If not, see . */ minibuffer recursions are encountered. */ Lisp_Object Vminibuffer_list; +Lisp_Object Vcommand_loop_level_list; /* Data to remember during recursive minibuffer invocations. */ @@ -64,6 +65,8 @@ static Lisp_Object minibuf_prompt; static ptrdiff_t minibuf_prompt_width; static Lisp_Object nth_minibuffer (EMACS_INT depth); +static EMACS_INT minibuf_c_loop_level (EMACS_INT depth); +static void set_minibuffer_mode (Lisp_Object buf, EMACS_INT depth); /* Return TRUE when a frame switch causes a minibuffer on the old @@ -181,7 +184,12 @@ void move_minibuffer_onto_frame (void) set_window_buffer (sf->minibuffer_window, nth_minibuffer (i), 0, 0); minibuf_window = sf->minibuffer_window; if (of != sf) - set_window_buffer (of->minibuffer_window, get_minibuffer (0), 0, 0); + { + Lisp_Object temp = get_minibuffer (0); + + set_window_buffer (of->minibuffer_window, temp, 0, 0); + set_minibuffer_mode (temp, 0); + } } } @@ -389,6 +397,21 @@ No argument or nil as argument means use the current buffer as BUFFER. */) : Qnil; } +DEFUN ("minibuffer-innermost-command-loop-p", Fminibuffer_innermost_command_loop_p, + Sminibuffer_innermost_command_loop_p, 0, 1, 0, + doc: /* Return t if BUFFER is a minibuffer at the current command loop level. +No argument or nil as argument means use the current buffer as BUFFER. */) + (Lisp_Object buffer) +{ + EMACS_INT depth; + if (NILP (buffer)) + buffer = Fcurrent_buffer (); + depth = this_minibuffer_depth (buffer); + return depth && minibuf_c_loop_level (depth) == command_loop_level + ? Qt + : Qnil; +} + /* Return the nesting depth of the active minibuffer BUFFER, or 0 if BUFFER isn't such a thing. If BUFFER is nil, this means use the current buffer. */ @@ -420,12 +443,17 @@ confirm the aborting of the current minibuffer and all contained ones. */) if (!minibuf_depth) error ("Not in a minibuffer"); + if (NILP (Fminibuffer_innermost_command_loop_p (Qnil))) + error ("Not in most nested command loop"); if (minibuf_depth < minibuf_level) { array[0] = fmt; array[1] = make_fixnum (minibuf_level - minibuf_depth + 1); if (!NILP (Fyes_or_no_p (Fformat (2, array)))) - Fthrow (Qexit, Qt); + { + minibuffer_quit_level = minibuf_depth; + Fthrow (Qexit, Qt); + } } else Fthrow (Qexit, Qt); @@ -508,6 +536,7 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt, ptrdiff_t count = SPECPDL_INDEX (); Lisp_Object mini_frame, ambient_dir, minibuffer, input_method; Lisp_Object calling_frame = selected_frame; + Lisp_Object calling_window = selected_window; Lisp_Object enable_multibyte; EMACS_INT pos = 0; /* String to add to the history. */ @@ -598,7 +627,8 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt, if (minibuf_level > 1 && minibuf_moves_frame_when_opened () - && !minibuf_follows_frame ()) + && (!minibuf_follows_frame () + || (!EQ (mini_frame, selected_frame)))) { EMACS_INT i; @@ -607,8 +637,6 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt, set_window_buffer (minibuf_window, nth_minibuffer (i), 0, 0); } - record_unwind_protect_void (choose_minibuf_frame); - record_unwind_protect (restore_window_configuration, Fcons (Qt, Fcurrent_window_configuration (Qnil))); @@ -640,7 +668,9 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt, minibuf_save_list = Fcons (Voverriding_local_map, Fcons (minibuf_window, - minibuf_save_list)); + Fcons (calling_frame, + Fcons (calling_window, + minibuf_save_list)))); minibuf_save_list = Fcons (minibuf_prompt, Fcons (make_fixnum (minibuf_prompt_width), @@ -694,6 +724,7 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt, /* Switch to the minibuffer. */ minibuffer = get_minibuffer (minibuf_level); + set_minibuffer_mode (minibuffer, minibuf_level); Fset_buffer (minibuffer); /* Defeat (setq-default truncate-lines t), since truncated lines do @@ -738,6 +769,7 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt, where there is an active minibuffer. Set them to point to ` *Minibuf-0*', which is always empty. */ empty_minibuf = get_minibuffer (0); + set_minibuffer_mode (empty_minibuf, 0); FOR_EACH_FRAME (dummy, frame) { @@ -837,20 +869,6 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt, recursive_edit_1 (); - /* We've exited the recursive edit without an error, so switch the - current window away from the expired minibuffer window. */ - { - Lisp_Object prev = Fprevious_window (minibuf_window, Qnil, Qnil); - /* PREV can be on a different frame when we have a minibuffer only - frame, the other frame's minibuffer window is MINIBUF_WINDOW, - and its "focus window" is also MINIBUF_WINDOW. */ - while (!EQ (prev, minibuf_window) - && !EQ (selected_frame, WINDOW_FRAME (XWINDOW (prev)))) - prev = Fprevious_window (prev, Qnil, Qnil); - if (!EQ (prev, minibuf_window)) - Fset_frame_selected_window (selected_frame, prev, Qnil); - } - /* If cursor is on the minibuffer line, show the user we have exited by putting it in column 0. */ if (XWINDOW (minibuf_window)->cursor.vpos >= 0 @@ -959,11 +977,16 @@ Lisp_Object get_minibuffer (EMACS_INT depth) { Lisp_Object tail = Fnthcdr (make_fixnum (depth), Vminibuffer_list); + Lisp_Object cll_tail = Fnthcdr (make_fixnum (depth), + Vcommand_loop_level_list); if (NILP (tail)) { tail = list1 (Qnil); Vminibuffer_list = nconc2 (Vminibuffer_list, tail); + cll_tail = list1 (Qnil); + Vcommand_loop_level_list = nconc2 (Vcommand_loop_level_list, cll_tail); } + XSETCAR (cll_tail, make_fixnum (depth ? command_loop_level : 0)); Lisp_Object buf = Fcar (tail); if (NILP (buf) || !BUFFER_LIVE_P (XBUFFER (buf))) { @@ -973,7 +996,6 @@ get_minibuffer (EMACS_INT depth) buf = Fget_buffer_create (lname, Qnil); /* Do this before set_minibuffer_mode. */ XSETCAR (tail, buf); - set_minibuffer_mode (buf, depth); /* Although the buffer's name starts with a space, undo should be enabled in it. */ Fbuffer_enable_undo (buf); @@ -985,12 +1007,19 @@ get_minibuffer (EMACS_INT depth) while the buffer doesn't know about them any more. */ delete_all_overlays (XBUFFER (buf)); reset_buffer (XBUFFER (buf)); - set_minibuffer_mode (buf, depth); } return buf; } +static EMACS_INT minibuf_c_loop_level (EMACS_INT depth) +{ + Lisp_Object cll = Fnth (make_fixnum (depth), Vcommand_loop_level_list); + if (FIXNUMP (cll)) + return XFIXNUM (cll); + return 0; +} + static void run_exit_minibuf_hook (void) { @@ -1004,17 +1033,16 @@ static void read_minibuf_unwind (void) { Lisp_Object old_deactivate_mark; - Lisp_Object window; + Lisp_Object calling_frame; + Lisp_Object calling_window; Lisp_Object future_mini_window; - /* If this was a recursive minibuffer, - tie the minibuffer window back to the outer level minibuffer buffer. */ - minibuf_level--; - - window = minibuf_window; /* To keep things predictable, in case it matters, let's be in the - minibuffer when we reset the relevant variables. */ - Fset_buffer (XWINDOW (window)->contents); + minibuffer when we reset the relevant variables. Don't depend on + `minibuf_window' here. This could by now be the mini-window of any + frame. */ + Fset_buffer (nth_minibuffer (minibuf_level)); + minibuf_level--; /* Restore prompt, etc, from outer minibuffer level. */ Lisp_Object key_vec = Fcar (minibuf_save_list); @@ -1042,6 +1070,10 @@ read_minibuf_unwind (void) #endif future_mini_window = Fcar (minibuf_save_list); minibuf_save_list = Fcdr (minibuf_save_list); + calling_frame = Fcar (minibuf_save_list); + minibuf_save_list = Fcdr (minibuf_save_list); + calling_window = Fcar (minibuf_save_list); + minibuf_save_list = Fcdr (minibuf_save_list); /* Erase the minibuffer we were using at this level. */ { @@ -1059,7 +1091,7 @@ read_minibuf_unwind (void) mini-window back to its normal size. */ if (minibuf_level == 0 || !EQ (selected_frame, WINDOW_FRAME (XWINDOW (future_mini_window)))) - resize_mini_window (XWINDOW (window), 0); + resize_mini_window (XWINDOW (minibuf_window), 0); /* Deal with frames that should be removed when exiting the minibuffer. */ @@ -1090,6 +1122,24 @@ read_minibuf_unwind (void) to make sure we don't leave around bindings and stuff which only made sense during the read_minibuf invocation. */ call0 (intern ("minibuffer-inactive-mode")); + + /* We've exited the recursive edit, so switch the current windows + away from the expired minibuffer window, both in the current + minibuffer's frame and the original calling frame. */ + choose_minibuf_frame (); + if (!EQ (WINDOW_FRAME (XWINDOW (minibuf_window)), calling_frame)) + { + Lisp_Object prev = Fprevious_window (minibuf_window, Qnil, Qnil); + /* PREV can be on a different frame when we have a minibuffer only + frame, the other frame's minibuffer window is MINIBUF_WINDOW, + and its "focus window" is also MINIBUF_WINDOW. */ + if (!EQ (prev, minibuf_window) + && EQ (WINDOW_FRAME (XWINDOW (prev)), + WINDOW_FRAME (XWINDOW (minibuf_window)))) + Fset_frame_selected_window (selected_frame, prev, Qnil); + } + else + Fset_frame_selected_window (calling_frame, calling_window, Qnil); } @@ -2137,6 +2187,7 @@ void init_minibuf_once (void) { staticpro (&Vminibuffer_list); + staticpro (&Vcommand_loop_level_list); pdumper_do_now_and_after_load (init_minibuf_once_for_pdumper); } @@ -2150,6 +2201,7 @@ init_minibuf_once_for_pdumper (void) restore from a dump file. pdumper doesn't try to preserve frames, windows, and so on, so reset everything related here. */ Vminibuffer_list = Qnil; + Vcommand_loop_level_list = Qnil; minibuf_level = 0; minibuf_prompt = Qnil; minibuf_save_list = Qnil; @@ -2380,6 +2432,7 @@ instead. */); defsubr (&Sminibufferp); defsubr (&Sinnermost_minibuffer_p); + defsubr (&Sminibuffer_innermost_command_loop_p); defsubr (&Sabort_minibuffers); defsubr (&Sminibuffer_prompt_end); defsubr (&Sminibuffer_contents); diff --git a/src/window.h b/src/window.h index 79eb44e7a3..b6f88e8f55 100644 --- a/src/window.h +++ b/src/window.h @@ -1120,10 +1120,6 @@ void set_window_buffer (Lisp_Object window, Lisp_Object buffer, extern Lisp_Object echo_area_window; -/* Depth in recursive edits. */ - -extern EMACS_INT command_loop_level; - /* Non-zero if we should redraw the mode lines on the next redisplay. Usually set to a unique small integer so we can track the main causes of full redisplays in `redisplay--mode-lines-cause'. */ -- Alan Mackenzie (Nuremberg, Germany).