bug#47300: delete-window to select window with same position

[martin rudalics <rudalics@gmx.at>]

[-- Attachment #1: Type: text/plain, Size: 1385 bytes --]

 > I changed <= to < to fix only Y coordinates for top/bottom windows,
 > but maybe X coordinates should still use <= ?

The function has to treat the case where an edge is shared between two
windows separately.  A non-shared edge always belongs to the
corresponding window.

 > Please note that the crucial difference is that now it uses
 > frame relative positions rather than window relative ones
 > with '(window-edges window nil nil t)' of the window to be deleted,
 > and window coordinates added to X and Y of posn-at-point.

Right.  But the X and Y values are needed separately only at the time
`window-at-pos' is called.

 > 1. `get-mru-window' could be one possible choice of the new option.
 > 2. `use-posn-at-point' could be another choice.

I called them just 'mru' and 'pos' now.  The option is called
`selected-window-after-deletion', there might be better names.

 > 3. Selecting the first window could remain in `delete-window-internal'.

It has to stay there.

 > 4. But what to do with this code block with '(other-window -1 frame)'?
 >     Should this be simply deleted?

That `other-window' was a bug: A frame that is not selected should not
get selected because we delete a window on it.  In addition, the old
code did not record a newly selected window unless that window was
produced by `get-mru-window'.  I tried to fix those too.  Please have a
look.

martin

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: selected-window-after-deletion.diff --]
[-- Type: text/x-patch; name="selected-window-after-deletion.diff", Size: 10809 bytes --]

diff --git a/lisp/window.el b/lisp/window.el
index fd1c617d6b..20f664c6f5 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -2538,12 +2538,14 @@ get-lru-window
 	    (setq best-window window)))))
     (or best-window second-best-window)))

-(defun get-mru-window (&optional all-frames dedicated not-selected)
+(defun get-mru-window (&optional all-frames dedicated not-selected no-other)
    "Return the most recently used window on frames specified by ALL-FRAMES.
 A minibuffer window is never a candidate.  A dedicated window is
 never a candidate unless DEDICATED is non-nil, so if all windows
 are dedicated, the value is nil.  Optional argument NOT-SELECTED
-non-nil means never return the selected window.
+non-nil means never return the selected window.  Optional
+argument NO_OTHER non-nil means to never return a window whose
+'no-other-window' parameter is non-nil.

 The following non-nil values of the optional argument ALL-FRAMES
 have special meanings:
@@ -2565,7 +2567,9 @@ get-mru-window
       (setq time (window-use-time window))
       (when (and (or dedicated (not (window-dedicated-p window)))
 		 (or (not not-selected) (not (eq window (selected-window))))
-		 (or (not best-time) (> time best-time)))
+                 (or (not no-other)
+                     (not (window-parameter window 'no-other-window)))
+                 (or (not best-time) (> time best-time)))
 	(setq best-time time)
 	(setq best-window window)))
     best-window))
@@ -4130,18 +4134,53 @@ window-deletable-p
       ;; of its frame.
       t))))

-(defun window--in-subtree-p (window root)
-  "Return t if WINDOW is either ROOT or a member of ROOT's subtree."
-  (or (eq window root)
-      (let ((parent (window-parent window)))
-	(catch 'done
-	  (while parent
-	    (if (eq parent root)
-		(throw 'done t)
-	      (setq parent (window-parent parent))))))))
+(defun window-at-pos (x y &optional frame no-other)
+  "Return live window at coordinates X, Y on specified FRAME.
+X and Y are counted in pixels from an origin at 0, 0 of FRAME's
+native frame.  A coordinate on an edge shared by two windows is
+attributed to the window on the right (or below).  Return nil if
+no such window can be found.
+
+Optional argument FRAME must specify a live frame and defaults to
+the selected one.  Optional argument NO-OTHER non-nil means to
+not return a window with a non-nil 'no-other-window' parameter."
+  (setq frame (window-normalize-frame frame))
+  (let* ((root-edges (window-edges (frame-root-window frame) nil nil t))
+         (root-left (nth 2 root-edges))
+         (root-bottom (nth 3 root-edges)))
+    (catch 'window
+      (walk-window-tree
+       (lambda (window)
+         (let ((edges (window-edges window nil nil t)))
+	   (when (and (>= x (nth 0 edges))
+                      (or (< x (nth 2 edges)) (= x root-left))
+		      (>= y (nth 1 edges))
+                      (or (< y (nth 3 edges)) (= y root-bottom)))
+             (if (and no-other (window-parameter window 'no-other-window))
+                 (throw 'window nil)
+	       (throw 'window window)))))
+       frame))))
+
+(defcustom selected-window-after-deletion 'mru
+  "How to choose a frame's selected window after window deletion.
+When a frame's selected window gets deleted, Emacs has to choose
+another live window on that frame to serve as its selected
+window.  This option allows to control which window gets selected
+instead.
+
+The possible choices are 'mru' (the default) to select the most
+recently used window on that frame and 'pos' to choose the window
+at the position of point of the previously selected window.  If
+this is nil, choose the frame's first window instead.  A window
+with a non-nil 'no-other-window' parameter is never chosen."
+  :type '(choice (const :tag "Most recently used" mru)
+                 (const :tag "At position" pos)
+                 nil)
+  :group 'windows
+  :version "28.1")

 (defun delete-window (&optional window)
-  "Delete WINDOW.
+  "Delete specified WINDOW.
 WINDOW must be a valid window and defaults to the selected one.
 Return nil.

@@ -4156,7 +4195,11 @@ delete-window
 `delete-window' with the root of the atomic window as its
 argument.  Signal an error if WINDOW is either the only window on
 its frame, the last non-side window, or part of an atomic window
-that is its frame's root window."
+that is its frame's root window.
+
+If WINDOW is the selected window on its frame, choose some other
+window as that frame's selected window according to the value of
+the option `selected-window-after-deletion'."
   (interactive)
   (setq window (window-normalize-window window))
   (let* ((frame (window-frame window))
@@ -4191,11 +4234,11 @@ delete-window
              (window-combination-resize
               (or window-combination-resize
                   (window-parameter parent 'window-side)))
-	     (frame-selected
-	      (window--in-subtree-p (frame-selected-window frame) window))
+             (frame-selected-window (frame-selected-window frame))
 	     ;; Emacs 23 preferably gives WINDOW's space to its left
 	     ;; sibling.
-	     (sibling (or (window-left window) (window-right window))))
+	     (sibling (or (window-left window) (window-right window)))
+             frame-selected-window-edges frame-selected-window-pos)
 	(window--resize-reset frame horizontal)
 	(cond
 	 ((and (not (eq window-combination-resize t))
@@ -4211,15 +4254,63 @@ delete-window
 	 (t
 	  ;; Can't do without resizing fixed-size windows.
 	  (window--resize-siblings window (- size) horizontal t)))
+
+        (when (eq selected-window-after-deletion 'pos)
+          ;; Remember edges and position of point of the selected window
+          ;; of WINDOW'S frame.
+          (setq frame-selected-window-edges
+                (window-edges frame-selected-window nil nil t))
+          (setq frame-selected-window-pos
+                (nth 2 (posn-at-point nil frame-selected-window))))
+
 	;; Actually delete WINDOW.
 	(delete-window-internal window)
 	(window--pixel-to-total frame horizontal)
-	(when (and frame-selected
-		   (window-parameter
-		    (frame-selected-window frame) 'no-other-window))
-	  ;; `delete-window-internal' has selected a window that should
-	  ;; not be selected, fix this here.
-	  (other-window -1 frame))
+
+        ;; If we deleted the selected window of WINDOW's frame, choose
+        ;; another one based on `selected-window-after-deletion'.  Note
+        ;; that both `window-at-pos' and `get-mru-window' mail fail to
+        ;; produce a suitable window in which case we will fall back on
+        ;; its frame's first window, chosen by `delete-window-internal'.
+        (cond
+         ((window-live-p frame-selected-window))
+         ((and frame-selected-window-pos
+               ;; We have a recorded position of point of the previously
+               ;; selected window.  Try to find the window that is now
+               ;; at that position.
+               (let ((new-frame-selected-window
+		      (window-at-pos
+                       (+ (nth 0 frame-selected-window-edges)
+                          (car frame-selected-window-pos))
+                       (+ (nth 1 frame-selected-window-edges)
+                          (cdr frame-selected-window-pos))
+                       frame t)))
+                 (and new-frame-selected-window
+                      ;; Select window at WINDOW's position at point.
+	              (set-frame-selected-window
+                       frame new-frame-selected-window)))))
+         ((and (eq selected-window-after-deletion 'mru)
+               ;; Try to use the most recently used window.
+               (let ((mru-window (get-mru-window frame nil nil t)))
+                 (and mru-window
+	              (set-frame-selected-window frame mru-window)))))
+         ((and (window-parameter
+                (frame-selected-window frame) 'no-other-window)
+               ;; If `delete-window-internal' selected a window with a
+               ;; non-nil 'no-other-window' parameter as its frame's
+               ;; selected window, try to choose another one.
+               (catch 'found
+                 (walk-window-tree
+                  (lambda (other)
+                    (unless (window-parameter other 'no-other-window)
+                      (set-frame-selected-window frame other)
+                      (throw 'found t)))
+                  frame))))
+         (t
+          ;; Record the window chosen by `delete-window-internal'.
+          (set-frame-selected-window
+           frame (frame-selected-window frame))))
+
 	(window--check frame)
 	;; Always return nil.
 	nil))))
diff --git a/src/window.c b/src/window.c
index 2d98ae5f15..db324effcc 100644
--- a/src/window.c
+++ b/src/window.c
@@ -5148,15 +5148,13 @@ DEFUN ("delete-window-internal", Fdelete_window_internal, Sdelete_window_interna
       adjust_frame_glyphs (f);

       if (!WINDOW_LIVE_P (FRAME_SELECTED_WINDOW (f)))
-	/* We deleted the frame's selected window.  */
+	/* We apparently deleted the frame's selected window; use the
+	   frame's first window as substitute but don't record it yet.
+	   `delete-window' may have something better up its sleeves.  */
 	{
 	  /* Use the frame's first window as fallback ...  */
 	  Lisp_Object new_selected_window = Fframe_first_window (frame);
-	  /* ... but preferably use its most recently used window.  */
-	  Lisp_Object mru_window;

-	  /* `get-mru-window' might fail for some reason so play it safe
-	  - promote the first window _without recording it_ first.  */
 	  if (EQ (FRAME_SELECTED_WINDOW (f), selected_window))
 	    Fselect_window (new_selected_window, Qt);
 	  else
@@ -5164,24 +5162,9 @@ DEFUN ("delete-window-internal", Fdelete_window_internal, Sdelete_window_interna
 	       last selected window on F was an active minibuffer, we
 	       want to return to it on a later Fselect_frame.  */
 	    fset_selected_window (f, new_selected_window);
-
-	  unblock_input ();
-
-	  /* Now look whether `get-mru-window' gets us something.  */
-	  mru_window = call1 (Qget_mru_window, frame);
-	  if (WINDOW_LIVE_P (mru_window)
-	      && EQ (XWINDOW (mru_window)->frame, frame))
-	    new_selected_window = mru_window;
-
-	  /* If all ended up well, we now promote the mru window.  */
-	  if (EQ (FRAME_SELECTED_WINDOW (f), selected_window))
-	    Fselect_window (new_selected_window, Qnil);
-	  else
-	    fset_selected_window (f, new_selected_window);
 	}
-      else
-	unblock_input ();

+      unblock_input ();
       FRAME_WINDOW_CHANGE (f) = true;
     }
   else