From 630c7ceae736fe96310079d6ea518e82a1815660 Mon Sep 17 00:00:00 2001 From: ntomsic Date: Thu, 21 May 2026 09:48:44 -0500 Subject: [PATCH] =?UTF-8?q?qt:=20parity=20tier=203=20batch=2011=20?= =?UTF-8?q?=E2=80=94=20split=20focus=20+=20cross-window=20tab=20DnD=20(B34?= =?UTF-8?q?,=20B35,=20I10)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three fixes plus PARITY housekeeping for batch 10. B35 — gotoSplit(PREVIOUS|NEXT) now walks the QSplitter tree depth-first instead of sorting panes by widget center. macOS + GTK both walk the surface tree; the flat-by-center sort got 3-pane symmetric splits right by accident but landed at the wrong neighbour for nested unbalanced trees (a 3-pane vertical split next to a 1-pane sibling cycled in pixel order, not tree order). The directional UP/DOWN/LEFT/RIGHT cases still use the screen-center heuristic — that's what feels right to the user when the binding says "move to the pane to my left." B34 / I10 — Cross-window tab adoption. TabBar::dropEvent now emits TabWidget::tabAdoptRequested when a tear-off from a different window's bar is dropped on ours; MainWindow listens and calls the existing adoptTab path. Resolves the source window via TabBar::parentWidget()->parent() (the source TabWidget's owner). The tear-off-back-onto-own-bar cancel path is unchanged. Plus tick PARITY.md for batch 10: B2, B3, B5, B41, B45, B46 — `4c903802a` B20 — confirmed honored (Qt delivers bare-modifier QKeyEvents) B38 — duplicate of C19, already fixed in `8bd64d0fa` Co-Authored-By: claude-flow --- qt/PARITY.md | 16 ++++++------- qt/src/MainWindow.cpp | 54 ++++++++++++++++++++++++++++++++++++------- qt/src/TabWidget.cpp | 28 +++++++++++++++------- qt/src/TabWidget.h | 8 +++++++ 4 files changed, 82 insertions(+), 24 deletions(-) diff --git a/qt/PARITY.md b/qt/PARITY.md index 530ffc917..d73eb7c03 100644 --- a/qt/PARITY.md +++ b/qt/PARITY.md @@ -29,10 +29,10 @@ checkbox and link the commit hash. ### Lifecycle / quit - [x] **B1.** `quit-after-last-window-closed-delay` does nothing on natural close (`MainWindow.cpp:255`). Delay timer only fires when libghostty issues `QUIT_TIMER`, but closing the last window via the title-bar X keeps the process alive forever (since Qt's `quitOnLastWindowClosed` was set false to allow the delay path). macOS handles via `applicationShouldTerminateAfterLastWindowClosed`; GTK wires last-window-close → `startQuitTimer` (`application.zig:820-862`). — fixed in `7c3868b5b` -- [ ] **B2.** `CLOSE_ALL_WINDOWS` always force-terminates (`MainWindow.cpp:1367-1370`, `:602-621`). Qt collapses `QUIT` and `CLOSE_ALL_WINDOWS` into the same path, both calling `qApp->quit()`. macOS keeps them distinct (close-all doesn't terminate). User binds close-all-windows → app quits unexpectedly. -- [ ] **B3.** `m_skipCloseConfirm` never cleared (`MainWindow.cpp:577-585`, `:454`, `:474`, `:509`). After one skip-confirmed close, if the window is re-shown via `toggleVisibility`, the next close also skips confirmation. macOS resets per-action. +- [x] **B2.** `CLOSE_ALL_WINDOWS` always force-terminates. — fixed in `4c903802a` (split QUIT vs CLOSE_ALL_WINDOWS via thenQuit param). +- [x] **B3.** `m_skipCloseConfirm` never cleared. — fixed in `4c903802a` (closeEvent consumes the flag for THIS attempt only). - [x] **B4.** `confirm-close-surface` config option ignored (`MainWindow.cpp:587-599`). Qt always uses libghostty's `needs_confirm_quit`. User setting `false` / `always` / `always-cwd` has no effect. — fixed in `33b5dee46` -- [ ] **B5.** `closeAllWindows` ignores `quit-after-last-window-closed=false` — windowless keep-alive impossible after close-all. +- [x] **B5.** `closeAllWindows` ignores `quit-after-last-window-closed=false` — fixed in `4c903802a` (CLOSE_ALL_WINDOWS path now reads quit-after-last-window-closed; `false` keeps the process alive after close-all). ### Action coverage @@ -69,7 +69,7 @@ checkbox and link the commit hash. ### Input / keyboard / mouse - [x] **B19.** Mouse buttons 4-11 not delivered (`GhosttySurface.cpp:710-715`). Only Left/Right/Middle mapped; back/forward buttons silently dropped. macOS + GTK both handle 4-11. — fixed in `a48ff0fb8` -- [ ] **B20.** Modifier release doesn't synthesize event (`sendKey`). Bare Shift/Ctrl/Alt presses don't produce kitty progressive-enhancement events. macOS uses `flagsChanged`; GTK derives from physical_key. +- [x] **B20.** Modifier release doesn't synthesize event (`sendKey`). Bare Shift/Ctrl/Alt presses don't produce kitty progressive-enhancement events. macOS uses `flagsChanged`; GTK derives from physical_key. — confirmed honored: Qt's xcb/wayland plugins do deliver QKeyEvent with `Qt::Key_Shift`/`Key_Control`/etc. and `nativeScanCode` populated for bare modifier transitions; sendKey forwards them. libghostty's kitty encoder uses the XKB keycode to identify the modifier. No Ghastty-side change needed. - [x] **B21.** `consumed_mods` only computed for printable events (`GhosttySurface.cpp:699-701`). Keypad/function/Backspace/arrows lose consumed-mods info. macOS + GTK compute unconditionally. — fixed in `13d4353b1` - [x] **B22.** Caps Lock + Num Lock state never set in mods (`translateMods`). Kitty CSI-u relies on these bits. — fixed in `913f192d8` - [x] **B23.** Sided modifiers (left vs right) not reported. `left_shift` vs `right_shift` keybinds can't fire. macOS + GTK both populate `mods.sides.*`. — fixed in `8e8725274` @@ -90,18 +90,18 @@ checkbox and link the commit hash. - [ ] **B35.** Split focus order sorts by widget center, not split tree (`MainWindow.cpp:809-858`). Nested unbalanced trees cycle in a different order than macOS+GTK use. - [ ] **B36.** QSplitter handle drag bypasses libghostty (`MainWindow.cpp:381`). Mouse-drag updates Qt's splitter ratios but never tells libghostty; "split equalize" later won't restore correctly. - [x] **B37.** Split equalize is per-splitter, not tree-aware (`MainWindow.cpp:886-896`). 3-pane vertical next to 1-pane gets 1:1 instead of 3:1. macOS + GTK use `surfaceTree.equalized()` which weights by leaf count. — fixed in `cd38f4bd5` -- [ ] **B38.** No `split-preserve-zoom` config. macOS persists zoom across focus moves with `navigation` setting. +- [x] **B38.** No `split-preserve-zoom` config. macOS persists zoom across focus moves with `navigation` setting. — fixed in `8bd64d0fa` (same site as C19). - [x] **B39.** Tab right-click context menu absent. macOS + GTK have full menu (Close/Close-Others/Close-Right/Rename/Pin). — fixed in `cd38f4bd5` - [x] **B40.** `window-decoration` only handles `none` (`MainWindow.cpp:268`). `auto`/`client`/`server` all collapse. Wayland has no portable way to force CSD vs SSD; the platform decides. — confirmed in `8e8725274` -- [ ] **B41.** `window-theme` partial (`MainWindow.cpp:1040`). `ghostty` mode (luminance-detected from background color) and full OS-scheme follow not implemented; pre-Qt 6.8 has zero theming. +- [x] **B41.** `window-theme` partial (`MainWindow.cpp:1040`). `ghostty` mode (luminance-detected from background color) and full OS-scheme follow not implemented; pre-Qt 6.8 has zero theming. — fixed in `4c903802a` (`ghostty` mode was already implemented; pre-6.8 fallback now synthesizes a QApplication palette for forced light/dark/ghostty). ### Quick terminal - [x] **B42.** No animation (slide-in/out). macOS uses `NSAnimationContext`. — fixed in `cd38f4bd5` (fade via QPropertyAnimation; slide infeasible under LayerShellQt) - [x] **B43.** `quick-terminal-screen` not honored. macOS resolves which monitor. — fixed in `6d700c36b` (handle->setScreen() before LayerShellQt anchoring; honors `main` / `mouse`; `macos-menu-bar` falls through to primary) - [x] ~~**B44.** `quick-terminal-position = center` not handled (`MainWindow.cpp:700`).~~ Audit was wrong; already handled at `MainWindow.cpp:766`. -- [ ] **B45.** `quick-terminal-space-behavior` not honored. -- [ ] **B46.** No fallback for non-Wayland — `LayerShellQt::Window::get()` returning null leaves a regular window without telling libghostty. +- [x] **B45.** `quick-terminal-space-behavior` not honored. — confirmed in `4c903802a` as a no-op. Wayland's wlr-layer-shell has no per-workspace pin; KWin always renders layer surfaces on the active workspace (= `move`). `remain` semantics are not achievable on Linux/Wayland. +- [x] **B46.** No fallback for non-Wayland — `LayerShellQt::Window::get()` returning null leaves a regular window without telling libghostty. — fixed in `4c903802a` (XWayland / X11 fall back to FramelessWindowHint + StaysOnTop + Tool with a 60%/40% top-centered placement). ### Misc diff --git a/qt/src/MainWindow.cpp b/qt/src/MainWindow.cpp index 4d9f19480..9ea592d30 100644 --- a/qt/src/MainWindow.cpp +++ b/qt/src/MainWindow.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include #include @@ -130,6 +131,28 @@ MainWindow::MainWindow() { connect(m_tabs, &TabWidget::tabTornOff, this, &MainWindow::detachTab); connect(m_tabs, &TabWidget::tabContextMenuRequested, this, &MainWindow::showTabContextMenu); + // Cross-window tab adoption: a TabBar dropEvent emits this when a + // tear-off from a different window's bar lands on ours. Resolve + // the source window via TabBar::parentWidget()->parent() and + // call adoptTab. + connect(m_tabs, &TabWidget::tabAdoptRequested, this, + [this](TabBar *origin) { + if (!origin) return; + // The TabBar's grandparent is the source MainWindow + // (TabBar -> TabWidget -> MainWindow). + auto *srcTabs = qobject_cast(origin->parentWidget()); + if (!srcTabs) return; + auto *srcWin = qobject_cast(srcTabs->parentWidget()); + if (!srcWin || srcWin == this) return; + // Adopt the source's currently-dragged tab. The current + // index is the tab being dragged at the time the drop + // landed on our bar (startTearOff settled in-bar + // reorder before exec, so currentIndex is stable). + const int idx = srcTabs->currentIndex(); + if (idx < 0) return; + QWidget *page = srcTabs->widget(idx); + if (page) adoptTab(srcWin, page); + }); } MainWindow::~MainWindow() { @@ -1219,15 +1242,30 @@ void MainWindow::gotoSplit(GhosttySurface *from, GhosttySurface *target = nullptr; if (dir == GHOSTTY_GOTO_SPLIT_PREVIOUS || dir == GHOSTTY_GOTO_SPLIT_NEXT) { - // Cycle through panes in reading order. - std::sort(panes.begin(), panes.end(), - [&](GhosttySurface *a, GhosttySurface *b) { - const QPoint pa = centerOf(a), pb = centerOf(b); - return pa.y() != pb.y() ? pa.y() < pb.y() : pa.x() < pb.x(); - }); - const int i = panes.indexOf(from); + // Cycle in split-tree order, not screen-position order. macOS + // and GTK both walk the surface tree depth-first; sorting by + // widget center put nested unbalanced trees in a different + // order than the user's mental model of "the next pane in the + // tree." A flat sort got 3/4 right by accident — fixing it for + // the asymmetric case. + QList order; + std::function walk = [&](QWidget *w) { + if (auto *s = qobject_cast(w)) { + order.append(s); + } else if (auto *sp = qobject_cast(w)) { + for (int i = 0; i < sp->count(); ++i) walk(sp->widget(i)); + } else if (w) { + // The tab page itself: descend into its child layout. + for (QObject *c : w->children()) + if (auto *cw = qobject_cast(c)) walk(cw); + } + }; + walk(m_tabs->widget(tab)); + if (order.isEmpty()) return; + const int i = order.indexOf(from); + if (i < 0) return; const int step = dir == GHOSTTY_GOTO_SPLIT_NEXT ? 1 : -1; - target = panes[(i + step + panes.size()) % panes.size()]; + target = order[(i + step + order.size()) % order.size()]; } else { // Directional: the nearest pane whose center lies that way. const QPoint fc = centerOf(from); diff --git a/qt/src/TabWidget.cpp b/qt/src/TabWidget.cpp index 4fcf612b3..5b6b774d2 100644 --- a/qt/src/TabWidget.cpp +++ b/qt/src/TabWidget.cpp @@ -179,15 +179,27 @@ void TabBar::dragEnterEvent(QDragEnterEvent *e) { } void TabBar::dropEvent(QDropEvent *e) { - // Dropping a tear-off back on a tab bar cancels it. Mark the flag on - // the *originating* bar (carried in the MIME payload), not this one - // — a tear-off can be dropped onto a different window's bar. + // Dropping a tear-off on a tab bar cancels the tear-off. Two cases: + // - dropped on the originating bar (or anywhere we can't decode): + // mark m_dropHandled so the source's startTearOff loop drops + // the tab back into place. + // - dropped on a *different* window's bar: cancel the tear-off + // AND ask the parent TabWidget to adopt the source tab into + // this window. macOS + GTK both support cross-window tab + // adoption this way. if (e->mimeData()->hasFormat(QString::fromLatin1(kGhosttyTabMime))) { - if (TabBar *origin = decodeOrigin( - e->mimeData()->data(QString::fromLatin1(kTearOffOriginRole)))) - origin->m_dropHandled = true; - else - m_dropHandled = true; // fallback: mark ourselves + TabBar *origin = decodeOrigin( + e->mimeData()->data(QString::fromLatin1(kTearOffOriginRole))); + if (origin) origin->m_dropHandled = true; + else m_dropHandled = true; + if (origin && origin != this) { + // Cross-window adoption. The parent QTabWidget signal carries + // the origin bar; the MainWindow on the receiving side resolves + // it to a source window + page and calls adoptTab. We emit + // through the TabWidget so MainWindow only listens at one site. + if (auto *tw = qobject_cast(parentWidget())) + emit tw->tabAdoptRequested(origin); + } e->acceptProposedAction(); } } diff --git a/qt/src/TabWidget.h b/qt/src/TabWidget.h index f5c7c4803..d69b458e9 100644 --- a/qt/src/TabWidget.h +++ b/qt/src/TabWidget.h @@ -39,6 +39,13 @@ public: signals: // The tab was dragged off and released clear of its window. void tabTornOff(int index); + // A tear-off from a *different* window's bar was dropped onto this + // one. `originBar` is the source TabBar; the receiving MainWindow + // looks up the originating window/page and adopts it. Both + // pointers stay valid for the duration of the signal handler — + // the drag's nested event loop has just exited and the source + // window can't have been deleted mid-emit. + void tabAdoptRequested(TabBar *originBar); // The user right-clicked a tab; the parent should show a context // menu (Close / Close Others / Close Tabs to the Right / Rename). // index is the tab index under the click; globalPos is screen- @@ -82,5 +89,6 @@ public: signals: void tabTornOff(int index); + void tabAdoptRequested(TabBar *originBar); void tabContextMenuRequested(int index, const QPoint &globalPos); };