bug#62427: tab-bar-new-tab-to now handles cases with multiple side-windows

[Benson Chu <bensonchu457@fastmail.com>]

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

I've done some digging into whether we can mimic OR use display-buffer
when creating a new tab, and while we could do this, we also advertise
two other behaviors for tab-bar-new-tab-choice that would make the
function needlessly complex, as use/mimicry of display-buffer would only
help in the case of when tab-bar-new-tab-choice is t.

On a side-note (side-window? haha), I've got a patch that side-steps
(hehe) the issue of modifying the 'window-side parameter completely, by
binding to window--sides-inhibit-check to t.

Having bound this, the issues wrt split-window and delete-other-windows
is no longer present, and thus we no longer have to touch the
'window-side parameter at all.

The same can't be said for the 'window-atom parameter (there's no
variable to bind to ignore checks), so it still needs to be removed in
order to ensure a safe call to delete-other-windows and split-window.

Would this be a better patch? It's now less destructive on existing
window parameters.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-tab-bar-new-tab-inhibit-side-window-checks.patch --]
[-- Type: text/x-patch, Size: 1947 bytes --]

From 2eaeea1f815b5e6c75947e9373c1b2f8fc539344 Mon Sep 17 00:00:00 2001
From: Benson Chu <bensonchu457@gmail.com>
Date: Fri, 24 Mar 2023 15:38:03 -0500
Subject: [PATCH] ; tab-bar-new-tab inhibit side-window checks

; Previously, calling tab-bar-new-tab-to only removes the 'window-side
; property on the currently selected window, and then a call to
; delete-other-windows was made to ensure that the selected window was
; the only window. We can skip this check by shadowing
; window--sides-inhibit-check to t.

* lisp/tab-bar.el (tab-bar-new-tab-to): inhibit side-window checks

Copyright-paperwork-exempt: yes
---
 lisp/tab-bar.el | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/lisp/tab-bar.el b/lisp/tab-bar.el
index dce6fa735fc..c7983146bf9 100644
--- a/lisp/tab-bar.el
+++ b/lisp/tab-bar.el
@@ -1552,15 +1552,14 @@ tab-bar-new-tab-to
       ;; Handle the case when it's called in the active minibuffer.
       (when (minibuffer-selected-window)
         (select-window (minibuffer-selected-window)))
-      ;; Remove window parameters that can cause problems
-      ;; with `delete-other-windows' and `split-window'.
-      (unless (eq tab-bar-new-tab-choice 'clone)
-        (set-window-parameter nil 'window-atom nil)
-        (set-window-parameter nil 'window-side nil))
-      (let ((ignore-window-parameters t))
+      (let ((ignore-window-parameters t)
+            (window--sides-inhibit-check t))
         (if (eq tab-bar-new-tab-choice 'clone)
             ;; Create new unique windows with the same layout
             (window-state-put (window-state-get))
+          ;; Remove window parameters that can cause problems
+          ;; with `delete-other-windows' and `split-window'.
+          (set-window-parameter nil 'window-atom nil)
           (delete-other-windows)
           (if (eq tab-bar-new-tab-choice 'window)
               ;; Create new unique window from remaining window
-- 
2.40.0


[-- Attachment #3: Type: text/plain, Size: 1319 bytes --]


Juri Linkov <juri@linkov.net> writes:

>> 1. (setq debug-on-error t enable-recursive-minibuffers t)
>> 2. M-:   (or any other prompt like 'M-x')
>> 3. Resize the minibuffer window e.g. with the mouse to at least 4 lines high
>> 4. M-x windmove-swap-states-up RET
>> 5. Click on the bottom window
>> 6. C-x t 2
>> 7. Click on the bottom window
>> 8. C-x t 2
>>
>> Debugger entered--Lisp error: (error "Specified root is not an ancestor of specified win...")
>>   delete-other-windows-internal(#<window 4 on *scratch*> #<window 7 on  *Minibuf-1*>)
>>   delete-other-windows()
>>   tab-bar-new-tab-to()
>>   tab-new(nil)
>
> Maybe the simplest fix is just not to create such a broken
> window configuration:
>
> diff --git a/lisp/windmove.el b/lisp/windmove.el
> index 06ce16c0d42..57511291588 100644
> --- a/lisp/windmove.el
> +++ b/lisp/windmove.el
> @@ -724,6 +724,8 @@ windmove-swap-states-in-direction
>                                             nil windmove-wrap-around 'nomini)))
>      (cond ((or (null other-window) (window-minibuffer-p other-window))
>             (user-error "No window %s from selected window" dir))
> +          ((window-minibuffer-p (selected-window))
> +           (user-error "Selected window is the minibuffer"))
>            (t
>             (window-swap-states nil other-window)))))