From 7845751538887d17044cdb209e967d59e5cd6966 Mon Sep 17 00:00:00 2001 From: ntomsic Date: Sat, 23 May 2026 14:42:38 -0500 Subject: [PATCH 1/5] =?UTF-8?q?qt:=20phase=204=20=E2=80=94=20extract=20Und?= =?UTF-8?q?oStack=20into=20qt/src/undo/?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pulls the static s_undoStack / s_redoStack / s_redoInProgress / kUndoCap plus pushTabUndo / pushWindowUndo / undoLastClose / redoLastClose out of MainWindow into a new `undo::` namespace under qt/src/undo/. State lives file-static in UndoStack.cpp; callers see only push / replay free functions. Replay-side hooks are two new public methods on MainWindow: closeCurrentTabForRedo (Tab redo) and closeForRedo (Window redo). Both bypass the close-confirm prompt — the user accepted the close on the original action. surfaceAt is promoted to public so undo::undoLast can title-tag the revived tab/window without going through MainWindow internals. MainWindow.cpp shrinks ~140 lines; MainWindow.h drops the UndoEntry struct and the static stack declarations. Co-Authored-By: claude-flow --- qt/CMakeLists.txt | 1 + qt/src/MainWindow.cpp | 169 ++++++-------------------------------- qt/src/MainWindow.h | 52 ++++-------- qt/src/undo/UndoStack.cpp | 167 +++++++++++++++++++++++++++++++++++++ qt/src/undo/UndoStack.h | 55 +++++++++++++ 5 files changed, 264 insertions(+), 180 deletions(-) create mode 100644 qt/src/undo/UndoStack.cpp create mode 100644 qt/src/undo/UndoStack.h diff --git a/qt/CMakeLists.txt b/qt/CMakeLists.txt index 6dc23a1cd..5371b0edf 100644 --- a/qt/CMakeLists.txt +++ b/qt/CMakeLists.txt @@ -115,6 +115,7 @@ add_executable(ghastty src/OverlayScrollbar.cpp src/SearchBar.cpp src/TabWidget.cpp + src/undo/UndoStack.cpp src/Util.cpp src/WindowBlur.cpp src/XkbTracker.cpp diff --git a/qt/src/MainWindow.cpp b/qt/src/MainWindow.cpp index 3b41230bd..13e47309e 100644 --- a/qt/src/MainWindow.cpp +++ b/qt/src/MainWindow.cpp @@ -54,6 +54,7 @@ #include "CommandPalette.h" #include "GhosttySurface.h" #include "TabWidget.h" +#include "undo/UndoStack.h" #include "Util.h" #include "WindowBlur.h" @@ -442,11 +443,11 @@ void MainWindow::removeSurface(GhosttySurface *surface) { const int index = m_tabs->indexOf(parent); // Push to undo so a shell-exited tab close is symmetric with a // user-initiated tab close (closeTab pushes too). Skip the last - // tab — its closeEvent runs pushWindowUndo and we don't want to + // tab — its closeEvent runs undo::pushWindow and we don't want to // double-stack. Also skip the quick terminal (which doesn't push // to either stack by design). if (index >= 0 && m_tabs->count() > 1 && !m_quickTerminal) - pushTabUndo(index); + undo::pushTab(m_tabs->tabText(index), m_quickTerminal); if (index >= 0) m_tabs->removeTab(index); if (parent) parent->deleteLater(); // page; destroys the surface too // The surface close was already confirmed; don't re-prompt on the @@ -461,9 +462,10 @@ void MainWindow::closeTab(int index) { QWidget *page = m_tabs->widget(index); if (!page) return; // Snapshot the tab's title for undo before we lose the reference. - // pushTabUndo is no-op for the last tab in a window — that close - // ends up triggering pushWindowUndo via closeEvent instead. - if (m_tabs->count() > 1 && !m_quickTerminal) pushTabUndo(index); + // undo::pushTab is no-op for the last tab in a window — that close + // ends up triggering undo::pushWindow via closeEvent instead. + if (m_tabs->count() > 1 && !m_quickTerminal) + undo::pushTab(m_tabs->tabText(index), m_quickTerminal); const auto inTab = page->findChildren(); for (GhosttySurface *s : inTab) m_surfaces.removeOne(s); // If the zoomed surface was in this tab, clear the stash so a later @@ -655,7 +657,11 @@ void MainWindow::closeEvent(QCloseEvent *e) { // Snapshot for undo. We push the window's full tab list so undo // restores all of them; closeTab paths skip the per-tab push when // they reach the last tab so we don't double-stack the same close. - pushWindowUndo(); + QStringList titles; + titles.reserve(m_tabs->count()); + for (int i = 0; i < m_tabs->count(); ++i) + titles << m_tabs->tabText(i); + undo::pushWindow(titles, geometry(), m_quickTerminal); e->accept(); } @@ -1444,147 +1450,22 @@ void MainWindow::setCellSize(uint32_t w, uint32_t h) { setSizeIncrement(0, 0); // back to pixel-precise } -// Process-wide undo state — see MainWindow.h. -QList MainWindow::s_undoStack; -QList MainWindow::s_redoStack; -bool MainWindow::s_redoInProgress = false; +void MainWindow::undoLastClose() { undo::undoLast(); } +void MainWindow::redoLastClose() { undo::redoLast(); } -// Snapshot the tab at `index` (its tab text — last-known title) onto -// the undo stack. Called from closeTab / closeTabsByMode / right -// before the tab is removed. No-op while a redo is replaying. -void MainWindow::pushTabUndo(int index) { - if (s_redoInProgress) return; - if (index < 0 || index >= m_tabs->count()) return; - UndoEntry e; - e.kind = UndoEntry::Kind::Tab; - e.pageTitles << m_tabs->tabText(index); - s_undoStack.append(std::move(e)); - if (s_undoStack.size() > kUndoCap) s_undoStack.removeFirst(); - // A fresh close invalidates any pending redo: the new "future" no - // longer matches what the redo stack would re-close. - s_redoStack.clear(); +// Close the active tab without prompting. Called from +// undo::redoLast for a Tab redo: the user already accepted the +// original close, so re-closing carries the same prior consent. +void MainWindow::closeCurrentTabForRedo() { + const int idx = m_tabs->currentIndex(); + if (idx >= 0) closeTab(idx); } -// Snapshot every tab in this window before it goes away. Called from -// closeAllWindows and from closeEvent for the user-driven X. No-op -// while a redo is replaying. -void MainWindow::pushWindowUndo() { - if (s_redoInProgress) return; - if (m_quickTerminal || m_tabs->count() == 0) return; - UndoEntry e; - e.kind = UndoEntry::Kind::Window; - for (int i = 0; i < m_tabs->count(); ++i) - e.pageTitles << m_tabs->tabText(i); - e.geometry = geometry(); - s_undoStack.append(std::move(e)); - if (s_undoStack.size() > kUndoCap) s_undoStack.removeFirst(); - s_redoStack.clear(); -} - -// Pop the most recent undo entry and revive it. A new tab/window is -// opened that inherits cwd from the active surface (libghostty -// supplies the cwd via inherited_config), and the saved title is -// reapplied as a manual tab-title override so it persists across -// shell prompts. -void MainWindow::undoLastClose() { - if (s_undoStack.isEmpty()) return; - const UndoEntry e = s_undoStack.takeLast(); - - // The active window picks the new tab's parent surface for cwd - // inheritance. Skip the quick terminal — it doesn't push undo - // entries and isn't a meaningful target. Fall back to the most - // recent regular window in registration order. - auto isUndoTarget = [](MainWindow *w) { - return w && !w->m_quickTerminal; - }; - MainWindow *active = qobject_cast(qApp->activeWindow()); - if (!isUndoTarget(active)) { - active = nullptr; - const QList &live = GhosttyApp::instance().windows(); - for (int i = live.size() - 1; i >= 0; --i) { - if (isUndoTarget(live.at(i))) { - active = live.at(i); - break; - } - } - } - GhosttySurface *parent = active - ? active->surfaceAt(active->m_tabs->currentIndex()) - : nullptr; - - if (e.kind == UndoEntry::Kind::Tab) { - if (!active) return; - GhosttySurface *s = active->newTab(parent ? parent->surface() : nullptr); - if (s && !e.pageTitles.isEmpty()) - active->setTabTitleOverride(s, e.pageTitles.first()); - } else { - // Window: open a fresh window, then add additional tabs to match - // the saved tab count. We don't try to recreate the split tree - // — that would require a real session save mechanism. - MainWindow *w = MainWindow::newWindow(parent ? parent->surface() : nullptr); - if (!w) return; - if (e.geometry.isValid()) w->setGeometry(e.geometry); - // Title for the (eventually created) first tab. - if (!e.pageTitles.isEmpty()) { - const QString first = e.pageTitles.first(); - QPointer wp(w); - QTimer::singleShot(0, w, [wp, first]() { - if (!wp) return; - if (auto *s = wp->surfaceAt(0)) wp->setTabTitleOverride(s, first); - }); - } - // Additional tabs for the rest of the saved set. - for (int i = 1; i < e.pageTitles.size(); ++i) { - const QString t = e.pageTitles.at(i); - QPointer wp(w); - QTimer::singleShot(0, w, [wp, t]() { - if (!wp) return; - GhosttySurface *s = - wp->newTab(wp->surfaceAt(0) ? wp->surfaceAt(0)->surface() : nullptr); - if (s) wp->setTabTitleOverride(s, t); - }); - } - } - s_redoStack.append(e); - if (s_redoStack.size() > kUndoCap) s_redoStack.removeFirst(); -} - -// Redo: re-close whatever undo just opened. We don't have a handle on -// the revived widgets so we close the active window's current tab -// (or the active window itself for a Window entry); pragmatic, -// matches what a user normally means by "redo close-tab". -// -// pushTabUndo / pushWindowUndo are no-ops while s_redoInProgress is -// true, so the close paths below don't: -// (a) clear s_redoStack — preserving the rest of the redo chain -// so a sequence of REDOs works. -// (b) push a fresh undo entry — a redo that re-closes shouldn't -// feed itself a new undo or the user can ping-pong undo/redo -// on a single past close indefinitely. -void MainWindow::redoLastClose() { - if (s_redoStack.isEmpty()) return; - UndoEntry e = s_redoStack.takeLast(); - - MainWindow *active = qobject_cast(qApp->activeWindow()); - if (!active) { - const QList &live = GhosttyApp::instance().windows(); - if (!live.isEmpty()) active = live.last(); - } - if (!active) { - // No window to act on — restore the entry so the user can retry. - s_redoStack.append(std::move(e)); - return; - } - - s_redoInProgress = true; - if (e.kind == UndoEntry::Kind::Tab) { - const int idx = active->m_tabs->currentIndex(); - if (idx >= 0) active->closeTab(idx); - } else { - active->m_skipCloseConfirm = true; - active->close(); - } - s_redoInProgress = false; +// Close the entire window without re-prompting. Called from +// undo::redoLast for a Window redo. +void MainWindow::closeForRedo() { + m_skipCloseConfirm = true; + close(); } void MainWindow::applyWindowConfig() { diff --git a/qt/src/MainWindow.h b/qt/src/MainWindow.h index 370e251e2..6f61455f7 100644 --- a/qt/src/MainWindow.h +++ b/qt/src/MainWindow.h @@ -76,17 +76,20 @@ public: // need to take a dependency on app/GhosttyApp.h. ghostty_config_t config() const; - // UNDO / REDO close-tab/window. The libghostty actions carry no - // payload — the apprt is responsible for tracking what was closed - // and reviving it. macOS uses NSUndoManager; we keep a small bounded - // stack of "snapshots" per kind. Surfaces themselves can't be - // revived (the child PTY is gone) — undo opens a fresh tab/window - // and reapplies the saved title; the new surface inherits cwd from - // the active surface (matching macOS, which also spawns a fresh - // shell rather than re-attaching). + // UNDO / REDO close-tab/window action handlers — thin wrappers that + // drive undo::undoLast / undo::redoLast. The undo state lives in + // qt/src/undo/UndoStack; the comment there explains the lifetime + // and replay model. static void undoLastClose(); static void redoLastClose(); + // Replay-side hooks used by undo::redoLast. closeCurrentTabForRedo + // closes whichever tab is currently active (Tab redo); closeForRedo + // closes the entire window (Window redo). Both bypass the + // close-confirm prompt — the user already accepted the close once. + void closeCurrentTabForRedo(); + void closeForRedo(); + // PRESENT_TERMINAL: bring this window to front and focus the surface. void presentTerminal(GhosttySurface *surface); // GOTO_WINDOW: cycle to the previous/next window in registration order. @@ -154,6 +157,9 @@ public: // First surface in the currently-visible tab, or nullptr. Used by // PROMPT_TITLE app-target promotion. GhosttySurface *currentSurface() const; + // First surface in the tab at `index`, or nullptr. Used by + // undo::undoLast to title-tag the revived tab/window. + GhosttySurface *surfaceAt(int index) const; // Default size cached on INITIAL_SIZE for RESET_WINDOW_SIZE. QSize defaultWindowSize() const { return m_defaultWindowSize; } void setDefaultWindowSize(QSize s) { m_defaultWindowSize = s; } @@ -194,7 +200,6 @@ private: void detachTab(int index); // Move `page` (a tab and its surfaces) from `src` into this window. void adoptTab(MainWindow *src, QWidget *page); - GhosttySurface *surfaceAt(int index) const; int tabIndexForSurface(GhosttySurface *surface) const; QList surfacesInTab(int index) const; @@ -254,33 +259,8 @@ private: // GhosttyApp::instance(). MainWindow's config() / needsPremultiply() // accessors forward to it. - // Snapshot of a closed tab or window for undo/redo. `pageTitles` - // holds each tab's last-known title (window snapshots have N tabs; - // tab snapshots have one). `geometry` is unused for tab snapshots. - // `kind` distinguishes the two so REDO can reclose the right thing. - struct UndoEntry { - enum class Kind { Tab, Window } kind = Kind::Tab; - QStringList pageTitles; - QRect geometry; - }; - // Bounded undo/redo stacks (tail = most recent). Each tab/window - // close pushes an entry, capped at kUndoCap; opening a new - // tab/window via undo pushes onto the redo stack. While - // `s_redoInProgress` is true, the close paths that normally - // mutate these stacks (pushTabUndo / pushWindowUndo) become - // no-ops — a redo is replaying a previous close and shouldn't - // also feed itself a fresh undo entry that the user will then - // unwind into a loop. - static QList s_undoStack; - static QList s_redoStack; - static bool s_redoInProgress; - static constexpr int kUndoCap = 16; - // Push a snapshot for the tab at `index` onto s_undoStack and - // clear the redo stack (a new close invalidates a forward redo). - void pushTabUndo(int index); - // Push a snapshot of every tab in this window onto s_undoStack as a - // single Window entry; called from closeAllWindows / closeEvent. - void pushWindowUndo(); + // The undo/redo stacks live in qt/src/undo/UndoStack — see comment + // there for the lifecycle and replay semantics. // Wakeup tick coalescing lives on GhosttyApp::m_tickPending. diff --git a/qt/src/undo/UndoStack.cpp b/qt/src/undo/UndoStack.cpp new file mode 100644 index 000000000..95674ea51 --- /dev/null +++ b/qt/src/undo/UndoStack.cpp @@ -0,0 +1,167 @@ +#include "UndoStack.h" + +#include +#include +#include +#include + +#include "../app/GhosttyApp.h" +#include "../GhosttySurface.h" +#include "../MainWindow.h" + +namespace undo { + +namespace { + +// Bounded undo / redo stacks (tail = most recent). A push past the cap +// drops the oldest entry. The redo stack is cleared by every fresh +// close — a new "future" no longer matches what redo would re-close. +constexpr int kCap = 16; + +QList &undoStack() { + static QList s; + return s; +} + +QList &redoStack() { + static QList s; + return s; +} + +// True while undo::redoLast is replaying. push* is gated on this so a +// redo that re-closes doesn't: +// (a) clear the redo stack (the rest of the redo chain stays +// playable), and +// (b) push a fresh undo entry (otherwise the user can ping-pong +// undo/redo on a single past close indefinitely). +bool g_redoInProgress = false; + +// Pick the active window for an undo target. Skips the quick terminal +// (it doesn't push undo entries, so re-opening into it isn't +// meaningful). Falls back to the most recent registered regular +// window. Returns nullptr if no regular window exists. +MainWindow *activeUndoTarget() { + auto isUndoTarget = [](MainWindow *w) { + return w && !w->isQuickTerminal(); + }; + MainWindow *active = qobject_cast(qApp->activeWindow()); + if (isUndoTarget(active)) return active; + const QList &live = GhosttyApp::instance().windows(); + for (int i = live.size() - 1; i >= 0; --i) { + if (isUndoTarget(live.at(i))) return live.at(i); + } + return nullptr; +} + +// Pick the window the user is currently looking at for a redo. Unlike +// undo, redo doesn't filter the quick terminal — REDO without an +// active regular window leaves the entry in place (caller restores). +MainWindow *activeRedoTarget() { + MainWindow *active = qobject_cast(qApp->activeWindow()); + if (active) return active; + const QList &live = GhosttyApp::instance().windows(); + return live.isEmpty() ? nullptr : live.last(); +} + +void pushUndo(Entry e) { + QList &s = undoStack(); + s.append(std::move(e)); + if (s.size() > kCap) s.removeFirst(); + // A fresh close invalidates any pending redo: the future the redo + // stack would replay no longer matches the world. + redoStack().clear(); +} + +} // namespace + +void pushTab(const QString &tabText, bool quickTerminal) { + if (g_redoInProgress) return; + if (quickTerminal) return; + Entry e; + e.kind = Entry::Kind::Tab; + e.pageTitles << tabText; + pushUndo(std::move(e)); +} + +void pushWindow(const QStringList &tabTitles, const QRect &geometry, + bool quickTerminal) { + if (g_redoInProgress) return; + if (quickTerminal || tabTitles.isEmpty()) return; + Entry e; + e.kind = Entry::Kind::Window; + e.pageTitles = tabTitles; + e.geometry = geometry; + pushUndo(std::move(e)); +} + +void undoLast() { + QList &s = undoStack(); + if (s.isEmpty()) return; + const Entry e = s.takeLast(); + + MainWindow *active = activeUndoTarget(); + GhosttySurface *parent = active ? active->currentSurface() : nullptr; + + if (e.kind == Entry::Kind::Tab) { + if (!active) return; // dropping the entry: no target to revive into + GhosttySurface *fresh = + active->newTab(parent ? parent->surface() : nullptr); + if (fresh && !e.pageTitles.isEmpty()) + active->setTabTitleOverride(fresh, e.pageTitles.first()); + } else { + // Window: spawn a fresh window, then queue extra tabs to match + // the saved tab count. We don't try to recreate the split tree + // — that would need a real session save mechanism. + MainWindow *w = + MainWindow::newWindow(parent ? parent->surface() : nullptr); + if (!w) return; + if (e.geometry.isValid()) w->setGeometry(e.geometry); + if (!e.pageTitles.isEmpty()) { + const QString first = e.pageTitles.first(); + QPointer wp(w); + QTimer::singleShot(0, w, [wp, first]() { + if (!wp) return; + if (auto *fresh = wp->surfaceAt(0)) + wp->setTabTitleOverride(fresh, first); + }); + } + for (int i = 1; i < e.pageTitles.size(); ++i) { + const QString t = e.pageTitles.at(i); + QPointer wp(w); + QTimer::singleShot(0, w, [wp, t]() { + if (!wp) return; + GhosttySurface *first = wp->surfaceAt(0); + GhosttySurface *fresh = + wp->newTab(first ? first->surface() : nullptr); + if (fresh) wp->setTabTitleOverride(fresh, t); + }); + } + } + + QList &r = redoStack(); + r.append(e); + if (r.size() > kCap) r.removeFirst(); +} + +void redoLast() { + QList &r = redoStack(); + if (r.isEmpty()) return; + Entry e = r.takeLast(); + + MainWindow *active = activeRedoTarget(); + if (!active) { + // No window to act on — restore the entry so the user can retry. + r.append(std::move(e)); + return; + } + + g_redoInProgress = true; + if (e.kind == Entry::Kind::Tab) { + active->closeCurrentTabForRedo(); + } else { + active->closeForRedo(); + } + g_redoInProgress = false; +} + +} // namespace undo diff --git a/qt/src/undo/UndoStack.h b/qt/src/undo/UndoStack.h new file mode 100644 index 000000000..939deb080 --- /dev/null +++ b/qt/src/undo/UndoStack.h @@ -0,0 +1,55 @@ +#pragma once + +#include +#include +#include +#include + +class MainWindow; + +// Process-wide undo / redo of closed tabs and windows. +// +// libghostty's UNDO / REDO actions carry no payload — the apprt +// remembers what was closed and revives it. Surfaces themselves can't +// be revived (the child PTY is gone), so undo opens a fresh tab/window +// and reapplies the saved title; the new surface inherits cwd from +// the active surface, matching macOS (which also spawns a fresh +// shell rather than re-attaching). +// +// State lives in this file's anonymous namespace; callers see only +// the four push / replay functions. push* are no-ops while a redo is +// replaying so the redo path doesn't feed itself. +namespace undo { + +// Snapshot of one closed tab or window. Window snapshots carry every +// tab's last-known title and the window's geometry; tab snapshots +// carry one title and an unused geometry. +struct Entry { + enum class Kind { Tab, Window } kind = Kind::Tab; + QStringList pageTitles; + QRect geometry; +}; + +// Snapshot the tab at `index` — single title — onto the undo stack. +// `tabText` is the tab's last-known display text; `quickTerminal` is +// the window's quick-terminal flag (quick-terminal tabs are excluded +// from the stack, mirroring the prior MainWindow behavior). +void pushTab(const QString &tabText, bool quickTerminal); + +// Snapshot every tab's title plus the window's geometry as a single +// Window entry. Excluded for the quick terminal and for empty +// windows. +void pushWindow(const QStringList &tabTitles, const QRect &geometry, + bool quickTerminal); + +// Pop the most recent entry and revive it: open a fresh tab or +// window, set the saved title(s) as a manual override, and push the +// entry onto the redo stack. No-op if the stack is empty. +void undoLast(); + +// Pop the most recent redo entry and re-close the active window's +// current tab (Tab entries) or the active window itself (Window +// entries). No-op if the stack is empty or no active window exists. +void redoLast(); + +} // namespace undo From 775905bdab89ade47cbc6fc09c71ac463ee1a865 Mon Sep 17 00:00:00 2001 From: ntomsic Date: Sat, 23 May 2026 14:46:53 -0500 Subject: [PATCH 2/5] =?UTF-8?q?qt:=20phase=205=20=E2=80=94=20extract=20Qui?= =?UTF-8?q?ckTerminal=20into=20qt/src/quickterm/?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pulls the layer-shell setup (setupLayerShell — ~100 lines reading quick-terminal-keyboard-interactivity / -screen / -size / -position plus the LayerShellQt anchor wiring) and the windowOpacity fade in/out animation out of MainWindow into a free-function `quickterm::` namespace under qt/src/quickterm/. The animation's QPropertyAnimation is parented to the QWidget via a dynamic property, so it still dies with the window — no per-window member bookkeeping needed on MainWindow anymore. MainWindow keeps the m_quickTerminal flag (too many sites read it: undo gating, refreshChrome, applyWindowConfig, etc.) and three thin forwarders (makeQuickTerminal, animateQuickTerminalIn, animateQuickTerminalOut). The autohide branch in changeEvent is one line and stays inline. MainWindow.cpp drops ~150 lines plus six now-unused includes (QEasingCurve, QPropertyAnimation, QScreen, QWindow, QCursor's sibling, LayerShellQt/window.h). MainWindow.h drops the QPropertyAnimation forward decl and the m_quickTerminalAnim member. Co-Authored-By: claude-flow --- qt/CMakeLists.txt | 1 + qt/src/MainWindow.cpp | 172 +----------------------- qt/src/MainWindow.h | 9 +- qt/src/quickterm/QuickTerminal.cpp | 202 +++++++++++++++++++++++++++++ qt/src/quickterm/QuickTerminal.h | 34 +++++ 5 files changed, 244 insertions(+), 174 deletions(-) create mode 100644 qt/src/quickterm/QuickTerminal.cpp create mode 100644 qt/src/quickterm/QuickTerminal.h diff --git a/qt/CMakeLists.txt b/qt/CMakeLists.txt index 5371b0edf..cb79d1343 100644 --- a/qt/CMakeLists.txt +++ b/qt/CMakeLists.txt @@ -113,6 +113,7 @@ add_executable(ghastty src/InspectorWindow.cpp src/MainWindow.cpp src/OverlayScrollbar.cpp + src/quickterm/QuickTerminal.cpp src/SearchBar.cpp src/TabWidget.cpp src/undo/UndoStack.cpp diff --git a/qt/src/MainWindow.cpp b/qt/src/MainWindow.cpp index 13e47309e..6925733a8 100644 --- a/qt/src/MainWindow.cpp +++ b/qt/src/MainWindow.cpp @@ -25,15 +25,12 @@ #include #include #include -#include #include #include #include -#include #include #include #include -#include #include #include #include @@ -45,14 +42,12 @@ #include #include #include -#include - -#include #include "app/GhosttyApp.h" #include "config/Config.h" #include "CommandPalette.h" #include "GhosttySurface.h" +#include "quickterm/QuickTerminal.h" #include "TabWidget.h" #include "undo/UndoStack.h" #include "Util.h" @@ -765,170 +760,13 @@ MainWindow *MainWindow::makeQuickTerminal() { delete w; return nullptr; } - w->setupLayerShell(); - w->animateQuickTerminalIn(); + quickterm::setupLayerShell(w); + quickterm::animateIn(w); return w; } -// Read quick-terminal-animation-duration (seconds) and convert to ms. -static int quickTerminalAnimationMs() { - double secs = 0.2; // matches Config.zig default - config::get(&secs, "quick-terminal-animation-duration"); - // Clamp to a sane range so a misconfigured 0 or negative value - // doesn't make the window appear/disappear instantly without an - // animation, and a very large value doesn't lock the user out. - if (secs <= 0.0) return 0; - return std::clamp(static_cast(secs * 1000.0), 1, 1000); -} - -void MainWindow::animateQuickTerminalIn() { - setWindowOpacity(0.0); - show(); - raise(); - activateWindow(); - const int ms = quickTerminalAnimationMs(); - if (ms <= 0) { - setWindowOpacity(1.0); - return; - } - // Stop any running fade so toggling rapidly doesn't stack - // animations. The animation is parented to `this` so it dies - // with the window. - if (m_quickTerminalAnim) m_quickTerminalAnim->stop(); - else m_quickTerminalAnim = new QPropertyAnimation(this, "windowOpacity", this); - m_quickTerminalAnim->setDuration(ms); - m_quickTerminalAnim->setStartValue(0.0); - m_quickTerminalAnim->setEndValue(1.0); - m_quickTerminalAnim->setEasingCurve(QEasingCurve::OutCubic); - m_quickTerminalAnim->start(); -} - -void MainWindow::animateQuickTerminalOut() { - const int ms = quickTerminalAnimationMs(); - if (ms <= 0) { - hide(); - return; - } - if (m_quickTerminalAnim) m_quickTerminalAnim->stop(); - else m_quickTerminalAnim = new QPropertyAnimation(this, "windowOpacity", this); - m_quickTerminalAnim->setDuration(ms); - m_quickTerminalAnim->setStartValue(windowOpacity()); - m_quickTerminalAnim->setEndValue(0.0); - m_quickTerminalAnim->setEasingCurve(QEasingCurve::InCubic); - // Disconnect any previous handler before reconnecting; otherwise a - // toggle-out-then-in cycle accumulates handlers that all fire on - // the next out. - disconnect(m_quickTerminalAnim, &QPropertyAnimation::finished, - this, nullptr); - connect(m_quickTerminalAnim, &QPropertyAnimation::finished, this, - [this]() { hide(); }); - m_quickTerminalAnim->start(); -} - -void MainWindow::setupLayerShell() { - // LayerShellQt attaches to the native window; force it into being. - winId(); - QWindow *handle = windowHandle(); - if (!handle) return; - LayerShellQt::Window *ls = LayerShellQt::Window::get(handle); - if (!ls) { - // The Qt frontend targets Wayland exclusively (the project - // builds against LayerShellQt for the dropdown). If we can't - // get a layer-shell handle the platform isn't supported — log - // and bail rather than silently degrading to a non-functional - // regular window. - std::fprintf(stderr, - "[ghastty] LayerShellQt::Window::get returned null; " - "the quick terminal needs a Wayland session with " - "wlr-layer-shell support (e.g. KWin, sway).\n"); - return; - } - using LSW = LayerShellQt::Window; - - ls->setLayer(LSW::LayerTop); - const QString ki = config::string("quick-terminal-keyboard-interactivity"); - ls->setKeyboardInteractivity( - ki == QLatin1String("exclusive") ? LSW::KeyboardInteractivityExclusive - : ki == QLatin1String("none") ? LSW::KeyboardInteractivityNone - : LSW::KeyboardInteractivityOnDemand); - - // quick-terminal-screen: pick which output to anchor on. - // `main` → primary screen. - // `mouse` → the screen the pointer is currently on. - // `macos-menu-bar` → macOS-only; falls through to primary on - // Linux. - // LayerShellQt 6.6+ exposes setScreen(QScreen*) on the layer-shell - // window directly; the older setScreenConfiguration is deprecated. - // Pass null to fall back to the QWindow's screen (LayerShellQt's - // documented default when neither setScreen nor - // setWantsToBeOnActiveScreen is set). - const QString screenMode = config::string("quick-terminal-screen"); - QScreen *screen = nullptr; - if (screenMode == QLatin1String("mouse")) { - screen = QGuiApplication::screenAt(QCursor::pos()); - } else if (screenMode == QLatin1String("main") || - screenMode == QLatin1String("macos-menu-bar")) { - screen = QGuiApplication::primaryScreen(); - } - ls->setScreen(screen); - if (!screen) screen = handle->screen(); - - // quick-terminal-space-behavior (`remain` / `move`) is intentionally - // not read: macOS controls whether the dropdown follows the active - // Space or pins to the one it was opened on, but Wayland's - // wlr-layer-shell has no equivalent — the compositor always renders - // the surface on the active workspace (KWin behaviour), which - // corresponds to `move`. Achieving `remain` would need a - // per-workspace pin that no mainstream compositor exposes; honour - // by no-op and document. - - const QSize scr = screen ? screen->size() : QSize(1920, 1080); - - // quick-terminal-size: primary is the edge-perpendicular extent. - ghostty_config_quick_terminal_size_s qsz = {}; - config::get(&qsz, "quick-terminal-size"); - const auto toPx = [](const ghostty_quick_terminal_size_s &s, int dim, - int fallback) -> int { - switch (s.tag) { - case GHOSTTY_QUICK_TERMINAL_SIZE_PERCENTAGE: - return static_cast(s.value.percentage / 100.0f * dim); - case GHOSTTY_QUICK_TERMINAL_SIZE_PIXELS: - return static_cast(s.value.pixels); - default: - return fallback; - } - }; - - const QString pos = config::string("quick-terminal-position"); - LSW::Anchors anchors; - QSize size; - if (pos == QLatin1String("bottom")) { - anchors = LSW::Anchors(LSW::AnchorBottom) | LSW::AnchorLeft | - LSW::AnchorRight; - size = {scr.width(), toPx(qsz.primary, scr.height(), 400)}; - } else if (pos == QLatin1String("left")) { - anchors = LSW::Anchors(LSW::AnchorLeft) | LSW::AnchorTop | - LSW::AnchorBottom; - size = {toPx(qsz.primary, scr.width(), 400), scr.height()}; - } else if (pos == QLatin1String("right")) { - anchors = LSW::Anchors(LSW::AnchorRight) | LSW::AnchorTop | - LSW::AnchorBottom; - size = {toPx(qsz.primary, scr.width(), 400), scr.height()}; - } else if (pos == QLatin1String("center")) { - anchors = LSW::Anchors(LSW::AnchorNone); - size = {toPx(qsz.primary, scr.width(), 800), - toPx(qsz.secondary, scr.height(), 400)}; - } else { // top (the default) - anchors = LSW::Anchors(LSW::AnchorTop) | LSW::AnchorLeft | - LSW::AnchorRight; - size = {scr.width(), toPx(qsz.primary, scr.height(), 400)}; - } - ls->setAnchors(anchors); - // The layer-shell protocol takes the size from the underlying - // wl_surface (i.e. the QWindow's size); LayerShellQt has no - // setDesiredSize on this Qt branch. - resize(size); -} +void MainWindow::animateQuickTerminalIn() { quickterm::animateIn(this); } +void MainWindow::animateQuickTerminalOut() { quickterm::animateOut(this); } void MainWindow::changeEvent(QEvent *e) { // quick-terminal-autohide: fade out the dropdown when it loses diff --git a/qt/src/MainWindow.h b/qt/src/MainWindow.h index 6f61455f7..dbda595d0 100644 --- a/qt/src/MainWindow.h +++ b/qt/src/MainWindow.h @@ -14,7 +14,6 @@ class QMediaPlayer; class QShowEvent; class QSplitter; class TabWidget; -class QPropertyAnimation; class QTimer; class CommandPalette; class GhosttySurface; @@ -225,10 +224,6 @@ private: // compositor (see WindowBlur). void applyBlur(); - // Turn this window into a layer-shell dropdown anchored to a screen - // edge, per the `quick-terminal-*` config. Quick-terminal only. - void setupLayerShell(); - TabWidget *m_tabs = nullptr; QList m_surfaces; // every live surface in this window bool m_firstTabPending = true; // first tab is created on show() @@ -236,8 +231,8 @@ private: bool m_skipCloseConfirm = false; // close already confirmed elsewhere bool m_quickTerminal = false; // this is the dropdown quick terminal // Per-window opacity animation for the quick terminal (fade in/out - // using quick-terminal-animation-duration). Lazily created. - QPropertyAnimation *m_quickTerminalAnim = nullptr; + // using quick-terminal-animation-duration). Owned by quickterm/'s + // dynamic-property cache on this widget; cleared on widget delete. QSize m_defaultWindowSize; // for RESET_WINDOW_SIZE; from INITIAL_SIZE // Last cell size reported by libghostty for this window's surfaces // (CELL_SIZE action). Stored so future grid-snap resizing can use diff --git a/qt/src/quickterm/QuickTerminal.cpp b/qt/src/quickterm/QuickTerminal.cpp new file mode 100644 index 000000000..35bb1b182 --- /dev/null +++ b/qt/src/quickterm/QuickTerminal.cpp @@ -0,0 +1,202 @@ +#include "QuickTerminal.h" + +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#include "../config/Config.h" +#include "ghostty.h" + +namespace quickterm { + +namespace { + +// Anim and toggle live on the QObject child tree of `window`, so +// they die with it. We keep the QPropertyAnimation as a dynamic +// property so callers don't need to thread it through. +constexpr const char *kAnimProperty = "_ghastty_qt_anim"; + +// Read quick-terminal-animation-duration (seconds) and convert to ms. +// Clamps to a sane range so a misconfigured 0/negative value doesn't +// make the window appear/disappear instantly without an animation, +// and a very large value doesn't lock the user out. +int animationMs() { + double secs = 0.2; // matches Config.zig default + config::get(&secs, "quick-terminal-animation-duration"); + if (secs <= 0.0) return 0; + return std::clamp(static_cast(secs * 1000.0), 1, 1000); +} + +// Lazily fetch (or build) the per-window opacity animation, parented +// to `window` so its lifetime tracks the widget's. +QPropertyAnimation *animFor(QWidget *window) { + auto *existing = window->property(kAnimProperty).value(); + if (existing) return existing; + auto *anim = new QPropertyAnimation(window, "windowOpacity", window); + window->setProperty(kAnimProperty, + QVariant::fromValue(anim)); + return anim; +} + +} // namespace + +void setupLayerShell(QWidget *window) { + // LayerShellQt attaches to the native window; force it into being. + window->winId(); + QWindow *handle = window->windowHandle(); + if (!handle) return; + LayerShellQt::Window *ls = LayerShellQt::Window::get(handle); + if (!ls) { + // The Qt frontend targets Wayland exclusively (the project + // builds against LayerShellQt for the dropdown). If we can't + // get a layer-shell handle the platform isn't supported — log + // and bail rather than silently degrading to a non-functional + // regular window. + std::fprintf(stderr, + "[ghastty] LayerShellQt::Window::get returned null; " + "the quick terminal needs a Wayland session with " + "wlr-layer-shell support (e.g. KWin, sway).\n"); + return; + } + using LSW = LayerShellQt::Window; + + ls->setLayer(LSW::LayerTop); + const QString ki = config::string("quick-terminal-keyboard-interactivity"); + ls->setKeyboardInteractivity( + ki == QLatin1String("exclusive") ? LSW::KeyboardInteractivityExclusive + : ki == QLatin1String("none") ? LSW::KeyboardInteractivityNone + : LSW::KeyboardInteractivityOnDemand); + + // quick-terminal-screen: pick which output to anchor on. + // `main` → primary screen. + // `mouse` → the screen the pointer is currently on. + // `macos-menu-bar` → macOS-only; falls through to primary on + // Linux. + // LayerShellQt 6.6+ exposes setScreen(QScreen*) on the layer-shell + // window directly; the older setScreenConfiguration is deprecated. + // Pass null to fall back to the QWindow's screen (LayerShellQt's + // documented default when neither setScreen nor + // setWantsToBeOnActiveScreen is set). + const QString screenMode = config::string("quick-terminal-screen"); + QScreen *screen = nullptr; + if (screenMode == QLatin1String("mouse")) { + screen = QGuiApplication::screenAt(QCursor::pos()); + } else if (screenMode == QLatin1String("main") || + screenMode == QLatin1String("macos-menu-bar")) { + screen = QGuiApplication::primaryScreen(); + } + ls->setScreen(screen); + if (!screen) screen = handle->screen(); + + // quick-terminal-space-behavior (`remain` / `move`) is intentionally + // not read: macOS controls whether the dropdown follows the active + // Space or pins to the one it was opened on, but Wayland's + // wlr-layer-shell has no equivalent — the compositor always renders + // the surface on the active workspace (KWin behaviour), which + // corresponds to `move`. Achieving `remain` would need a + // per-workspace pin that no mainstream compositor exposes; honour + // by no-op and document. + + const QSize scr = screen ? screen->size() : QSize(1920, 1080); + + // quick-terminal-size: primary is the edge-perpendicular extent. + ghostty_config_quick_terminal_size_s qsz = {}; + config::get(&qsz, "quick-terminal-size"); + const auto toPx = [](const ghostty_quick_terminal_size_s &s, int dim, + int fallback) -> int { + switch (s.tag) { + case GHOSTTY_QUICK_TERMINAL_SIZE_PERCENTAGE: + return static_cast(s.value.percentage / 100.0f * dim); + case GHOSTTY_QUICK_TERMINAL_SIZE_PIXELS: + return static_cast(s.value.pixels); + default: + return fallback; + } + }; + + const QString pos = config::string("quick-terminal-position"); + LSW::Anchors anchors; + QSize size; + if (pos == QLatin1String("bottom")) { + anchors = LSW::Anchors(LSW::AnchorBottom) | LSW::AnchorLeft | + LSW::AnchorRight; + size = {scr.width(), toPx(qsz.primary, scr.height(), 400)}; + } else if (pos == QLatin1String("left")) { + anchors = LSW::Anchors(LSW::AnchorLeft) | LSW::AnchorTop | + LSW::AnchorBottom; + size = {toPx(qsz.primary, scr.width(), 400), scr.height()}; + } else if (pos == QLatin1String("right")) { + anchors = LSW::Anchors(LSW::AnchorRight) | LSW::AnchorTop | + LSW::AnchorBottom; + size = {toPx(qsz.primary, scr.width(), 400), scr.height()}; + } else if (pos == QLatin1String("center")) { + anchors = LSW::Anchors(LSW::AnchorNone); + size = {toPx(qsz.primary, scr.width(), 800), + toPx(qsz.secondary, scr.height(), 400)}; + } else { // top (the default) + anchors = LSW::Anchors(LSW::AnchorTop) | LSW::AnchorLeft | + LSW::AnchorRight; + size = {scr.width(), toPx(qsz.primary, scr.height(), 400)}; + } + ls->setAnchors(anchors); + // The layer-shell protocol takes the size from the underlying + // wl_surface (i.e. the QWindow's size); LayerShellQt has no + // setDesiredSize on this Qt branch. + window->resize(size); +} + +void animateIn(QWidget *window) { + window->setWindowOpacity(0.0); + window->show(); + window->raise(); + window->activateWindow(); + const int ms = animationMs(); + if (ms <= 0) { + window->setWindowOpacity(1.0); + return; + } + // Stop any running fade so toggling rapidly doesn't stack + // animations. + QPropertyAnimation *anim = animFor(window); + anim->stop(); + anim->setDuration(ms); + anim->setStartValue(0.0); + anim->setEndValue(1.0); + anim->setEasingCurve(QEasingCurve::OutCubic); + anim->start(); +} + +void animateOut(QWidget *window) { + const int ms = animationMs(); + if (ms <= 0) { + window->hide(); + return; + } + QPropertyAnimation *anim = animFor(window); + anim->stop(); + anim->setDuration(ms); + anim->setStartValue(window->windowOpacity()); + anim->setEndValue(0.0); + anim->setEasingCurve(QEasingCurve::InCubic); + // Disconnect any previous handler before reconnecting; otherwise a + // toggle-out-then-in cycle accumulates handlers that all fire on + // the next out. + QObject::disconnect(anim, &QPropertyAnimation::finished, window, nullptr); + QObject::connect(anim, &QPropertyAnimation::finished, window, + [window]() { window->hide(); }); + anim->start(); +} + +} // namespace quickterm diff --git a/qt/src/quickterm/QuickTerminal.h b/qt/src/quickterm/QuickTerminal.h new file mode 100644 index 000000000..fa7f0596d --- /dev/null +++ b/qt/src/quickterm/QuickTerminal.h @@ -0,0 +1,34 @@ +#pragma once + +class QWidget; + +// Free functions that drive the dropdown quick-terminal window: a +// wlr-layer-shell anchored surface, faded in/out via windowOpacity. +// `window` is the QWidget hosting the layer-shell surface +// (MainWindow with m_quickTerminal == true). +// +// The animation is parented to `window`'s child tree, so it dies +// with the window. +namespace quickterm { + +// Configure the layer-shell anchor, screen, keyboard interactivity, +// and size from the `quick-terminal-*` config keys. Logs and bails +// when LayerShellQt isn't available — the Qt frontend is Wayland- +// only, so a missing layer-shell surface is a runtime configuration +// error, not a portability fallback. +void setupLayerShell(QWidget *window); + +// Fade the window in: opacity 0 → 1 over +// `quick-terminal-animation-duration` seconds. show()/raise()/ +// activateWindow() are called up front so the user gets the focus +// during the fade. A duration of 0 collapses to an immediate +// setWindowOpacity(1.0). +void animateIn(QWidget *window); + +// Fade the window out and hide() on completion. Disconnects any +// previous `finished` handler before reconnecting so a rapid +// in/out/in cycle doesn't pile up handlers that all fire on the +// next `out`. +void animateOut(QWidget *window); + +} // namespace quickterm From c4ab83885906d0d313b755aecb34d6d1abb30f03 Mon Sep 17 00:00:00 2001 From: ntomsic Date: Sat, 23 May 2026 14:49:48 -0500 Subject: [PATCH 3/5] =?UTF-8?q?qt:=20phase=206=20=E2=80=94=20extract=20Bel?= =?UTF-8?q?lPlayer=20into=20qt/src/bell/?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pulls the audio-bell concern (m_bellPlayer / m_bellAudio / m_bellAudioPath / m_bellAudioVolume + the playBellAudio body + the bell-audio-path / -volume cache refresh in applyWindowConfig) out of MainWindow into a per-window QObject child class. BellPlayer::refreshFromConfig replaces the inline cache update; BellPlayer::play replaces playBellAudio. Lazy QMediaPlayer + QAudioOutput allocation is preserved so a window that never rings an audio bell never instantiates them. ringBell still lives on MainWindow — the bell-features bitfield gates other features (alert, beep, border flash, title mark) that all need MainWindow internals (the tab bar, the surface list). Audio is the one feature that's purely encapsulated. MainWindow.cpp drops 4 members + the ~15-line playBellAudio body + the 5-line cache refresh; , , includes are gone too. MainWindow.h drops two forward declarations. Co-Authored-By: claude-flow --- qt/CMakeLists.txt | 1 + qt/src/MainWindow.cpp | 36 ++++++---------------------------- qt/src/MainWindow.h | 17 +++++----------- qt/src/bell/BellPlayer.cpp | 34 ++++++++++++++++++++++++++++++++ qt/src/bell/BellPlayer.h | 40 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 86 insertions(+), 42 deletions(-) create mode 100644 qt/src/bell/BellPlayer.cpp create mode 100644 qt/src/bell/BellPlayer.h diff --git a/qt/CMakeLists.txt b/qt/CMakeLists.txt index cb79d1343..bcd4ed7a7 100644 --- a/qt/CMakeLists.txt +++ b/qt/CMakeLists.txt @@ -106,6 +106,7 @@ add_executable(ghastty src/actions/TabActions.cpp src/actions/WindowActions.cpp src/app/GhosttyApp.cpp + src/bell/BellPlayer.cpp src/config/Config.cpp src/CommandPalette.cpp src/GhosttySurface.cpp diff --git a/qt/src/MainWindow.cpp b/qt/src/MainWindow.cpp index 6925733a8..af589e009 100644 --- a/qt/src/MainWindow.cpp +++ b/qt/src/MainWindow.cpp @@ -6,7 +6,6 @@ #include #include -#include #include #include #include @@ -22,7 +21,6 @@ #include #include #include -#include #include #include #include @@ -34,7 +32,6 @@ #include #include #include -#include #include #include #include @@ -44,6 +41,7 @@ #include #include "app/GhosttyApp.h" +#include "bell/BellPlayer.h" #include "config/Config.h" #include "CommandPalette.h" #include "GhosttySurface.h" @@ -1024,7 +1022,7 @@ void MainWindow::ringBell(GhosttySurface *surface) { config::bitfield("bell-features", BellAttention); if (features & BellAttention) QApplication::alert(this); if (features & BellSystem) QApplication::beep(); - if (features & BellAudio) playBellAudio(); + if (features & BellAudio && m_bellPlayer) m_bellPlayer->play(); if (!surface) return; if (features & BellBorder) surface->flashBorder(); @@ -1060,22 +1058,6 @@ void MainWindow::updateTabText(int tab) { setWindowTitle(text + QStringLiteral(" — Ghastty")); } -void MainWindow::playBellAudio() { - if (m_bellAudioPath.isEmpty()) return; - if (!m_bellPlayer) { - m_bellAudio = new QAudioOutput(this); - m_bellPlayer = new QMediaPlayer(this); - m_bellPlayer->setAudioOutput(m_bellAudio); - } - m_bellAudio->setVolume(m_bellAudioVolume); - // Stop first so a back-to-back bell restarts the clip from the - // beginning. Without this, calling play() on an already-playing - // QMediaPlayer is a no-op and rapid bells get silently swallowed. - m_bellPlayer->stop(); - m_bellPlayer->setSource(QUrl::fromLocalFile(m_bellAudioPath)); - m_bellPlayer->play(); -} - // Refresh every window's chrome from the current GhosttyApp config: tab-bar // policy, colour scheme, blur — plus window-level state that // previously only applied at startup (window-decoration, fullscreen, @@ -1323,16 +1305,10 @@ void MainWindow::applyWindowConfig() { m_tabs->tabBar()->setVisible(m_tabs->count() > 1); } - // bell-audio-path / -volume: cached on the window so playBellAudio - // doesn't re-scan the on-disk config on every bell. Refreshed on - // each applyWindowConfig (i.e. at init and on reload). - { - m_bellAudioPath = config::expandedPath("bell-audio-path"); - bool volOk = false; - const double v = - config::diskValue("bell-audio-volume").toDouble(&volOk); - m_bellAudioVolume = volOk ? v : 0.5; - } + // bell-audio: BellPlayer caches the path/volume so the bell hot + // path doesn't re-scan the on-disk config on every ring. + if (!m_bellPlayer) m_bellPlayer = new BellPlayer(this); + m_bellPlayer->refreshFromConfig(); // window-title-font-family: apply to the tab bar (and the WM // title via Qt's window-title system font is harder to override diff --git a/qt/src/MainWindow.h b/qt/src/MainWindow.h index dbda595d0..69b1a1e0a 100644 --- a/qt/src/MainWindow.h +++ b/qt/src/MainWindow.h @@ -8,9 +8,8 @@ #include "ghostty.h" -class QAudioOutput; +class BellPlayer; class QCloseEvent; -class QMediaPlayer; class QShowEvent; class QSplitter; class TabWidget; @@ -202,8 +201,6 @@ private: int tabIndexForSurface(GhosttySurface *surface) const; QList surfacesInTab(int index) const; - void playBellAudio(); - // Bell `title` feature: prefix a tab's title while any surface in it // has an unacknowledged bell. bool tabBellMarked(int tab) const; @@ -266,14 +263,10 @@ private: QSplitter *m_zoomSplitter = nullptr; int m_zoomIndex = 0; - // Bell audio playback; created lazily on the first audio bell. - // The bell-audio-path / -volume values are cached at window setup - // and refreshed on reload so the bell hot path doesn't re-scan - // the on-disk config file. - QMediaPlayer *m_bellPlayer = nullptr; - QAudioOutput *m_bellAudio = nullptr; - QString m_bellAudioPath; // expanded; empty if no clip configured - double m_bellAudioVolume = 0.5; + // Bell audio playback; lives in qt/src/bell/BellPlayer. Created + // lazily on the first applyWindowConfig pass so refreshFromConfig + // can prime its cached path/volume. + BellPlayer *m_bellPlayer = nullptr; // The command palette; created lazily on first use. CommandPalette *m_commandPalette = nullptr; diff --git a/qt/src/bell/BellPlayer.cpp b/qt/src/bell/BellPlayer.cpp new file mode 100644 index 000000000..2a667ebe9 --- /dev/null +++ b/qt/src/bell/BellPlayer.cpp @@ -0,0 +1,34 @@ +#include "BellPlayer.h" + +#include +#include +#include + +#include "../config/Config.h" + +BellPlayer::BellPlayer(QObject *parent) : QObject(parent) {} + +BellPlayer::~BellPlayer() = default; + +void BellPlayer::play() { + if (m_path.isEmpty()) return; + if (!m_player) { + m_audio = new QAudioOutput(this); + m_player = new QMediaPlayer(this); + m_player->setAudioOutput(m_audio); + } + m_audio->setVolume(m_volume); + // Stop first so a back-to-back bell restarts the clip from the + // beginning. Without this, calling play() on an already-playing + // QMediaPlayer is a no-op and rapid bells get silently swallowed. + m_player->stop(); + m_player->setSource(QUrl::fromLocalFile(m_path)); + m_player->play(); +} + +void BellPlayer::refreshFromConfig() { + m_path = config::expandedPath("bell-audio-path"); + bool volOk = false; + const double v = config::diskValue("bell-audio-volume").toDouble(&volOk); + m_volume = volOk ? v : 0.5; +} diff --git a/qt/src/bell/BellPlayer.h b/qt/src/bell/BellPlayer.h new file mode 100644 index 000000000..ca40bf2f2 --- /dev/null +++ b/qt/src/bell/BellPlayer.h @@ -0,0 +1,40 @@ +#pragma once + +#include +#include + +class QAudioOutput; +class QMediaPlayer; + +// Per-window audio bell. Owns a QMediaPlayer + QAudioOutput pair and +// caches the bell-audio-path / -volume values so the bell hot path +// doesn't re-scan the on-disk config on every ring. Built lazily on +// first play() — no QMediaPlayer is allocated until a bell actually +// fires, matching the prior MainWindow behaviour. +// +// Parented to the owning window so it dies with it. Each window +// keeps its own player so KWin can route per-window audio +// independently if the user wires that up. +class BellPlayer : public QObject { + Q_OBJECT +public: + explicit BellPlayer(QObject *parent); + ~BellPlayer() override; + + // Play the configured clip, restarting from the beginning if a + // previous play is still in flight. No-op if no clip is + // configured. The audio output volume is whatever + // refreshFromConfig last cached. + void play(); + + // Re-read bell-audio-path / bell-audio-volume from the on-disk + // config and update the cache. Called from + // applyWindowConfig (init + reload). + void refreshFromConfig(); + +private: + QString m_path; + double m_volume = 0.5; + QMediaPlayer *m_player = nullptr; + QAudioOutput *m_audio = nullptr; +}; From f070cff504e5943a1018891e01d9b7dd18e25406 Mon Sep 17 00:00:00 2001 From: ntomsic Date: Sat, 23 May 2026 14:53:14 -0500 Subject: [PATCH 4/5] =?UTF-8?q?qt:=20phase=207=20=E2=80=94=20extract=20Xkb?= =?UTF-8?q?State=20into=20qt/src/input/?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The XkbState process-singleton (libxkbcommon keymap + state used to compute the unshifted codepoint, sided modifier bits, lock state, and layout-consumed mods for libghostty's kitty encoder) was embedded ~180 lines deep in GhosttySurface.cpp. The class is fully self-contained — pure xkbcommon + XkbTracker, no QWidget — so it moves cleanly into a new qt/src/input/ directory next to its sibling XkbTracker. GhosttySurface.cpp now just #includes "input/XkbState.h" and uses XkbState::instance() at four sendKey callsites. The direct include drops out (the surface TU no longer touches xkb directly; the header pulls it in for the xkb_mod_index_t members). GhosttySurface.cpp shrinks ~180 lines. Co-Authored-By: claude-flow --- qt/CMakeLists.txt | 1 + qt/src/GhosttySurface.cpp | 200 +------------------------------------- qt/src/input/XkbState.cpp | 135 +++++++++++++++++++++++++ qt/src/input/XkbState.h | 83 ++++++++++++++++ 4 files changed, 220 insertions(+), 199 deletions(-) create mode 100644 qt/src/input/XkbState.cpp create mode 100644 qt/src/input/XkbState.h diff --git a/qt/CMakeLists.txt b/qt/CMakeLists.txt index bcd4ed7a7..a8ab37c0b 100644 --- a/qt/CMakeLists.txt +++ b/qt/CMakeLists.txt @@ -112,6 +112,7 @@ add_executable(ghastty src/GhosttySurface.cpp src/GlobalShortcuts.cpp src/InspectorWindow.cpp + src/input/XkbState.cpp src/MainWindow.cpp src/OverlayScrollbar.cpp src/quickterm/QuickTerminal.cpp diff --git a/qt/src/GhosttySurface.cpp b/qt/src/GhosttySurface.cpp index 2e7bb1213..09ad0429b 100644 --- a/qt/src/GhosttySurface.cpp +++ b/qt/src/GhosttySurface.cpp @@ -1,6 +1,7 @@ #include "GhosttySurface.h" #include "config/Config.h" +#include "input/XkbState.h" #include "InspectorWindow.h" #include "MainWindow.h" #include "OverlayScrollbar.h" @@ -49,8 +50,6 @@ #include #include -#include - GhosttySurface::GhosttySurface(ghostty_app_t app, MainWindow *owner, ghostty_surface_t parent_surface) : m_app(app), m_owner(owner), m_parentSurface(parent_surface) { @@ -697,203 +696,6 @@ void GhosttySurface::premultiplyFramebuffer() { // --- input ---------------------------------------------------------- -// Wraps a libxkbcommon keymap + state derived from the system's XKB -// defaults (XKB_DEFAULT_LAYOUT etc.). We need this for two things: -// -// 1. The unshifted codepoint a key would produce with no modifiers — -// libghostty's kitty encoder uses it to find a key entry for -// printable keys (without it, punctuation falls into a fallback -// that mis-encodes release events). -// -// 2. Which modifiers the layout "consumed" to produce the event's -// text — e.g. Shift+; → ":" consumes Shift. The encoder uses this -// to decide between plain text and a modifier-bearing CSI; without -// it Shift+punctuation gets emitted as a kitty CSI the shell can't -// decode (Shift+letter happens to work because A-Z survive that -// path). -// -// THREAD SAFETY: this is a process singleton accessed only from the Qt -// GUI thread (Qt key events are dispatched there, and so is libghostty's -// inputMethodEvent forwarding). consumedMods mutates m_query, so a -// second thread would race; do not call from worker threads. -class XkbState { -public: - static XkbState &instance() { - static XkbState self; - return self; - } - - // Level-0 (unshifted) Unicode codepoint for `keycode`, or 0 if the - // key has no associated UTF-32 (function keys, modifiers, etc.). - // - // Uses the live keymap from XkbTracker (synced via wl_keyboard) so - // the active layout group is honored. A us+ru user gets the - // correct codepoint per active group, instead of always us. - uint32_t unshiftedCodepoint(uint32_t keycode) const { - syncFromTracker(); - if (!m_unshifted) return 0; - const xkb_keysym_t sym = - xkb_state_key_get_one_sym(m_unshifted, keycode); - if (sym == XKB_KEY_NoSymbol) return 0; - return xkb_keysym_to_utf32(sym); - } - - // Side bits for the libghostty mods bitfield, derived from a - // keycode — used so that pressing Right-Shift sets BOTH the - // unsided GHOSTTY_MODS_SHIFT and the GHOSTTY_MODS_SHIFT_RIGHT bit - // (a left-side keycode sets only the unsided bit). macOS and GTK - // populate sided bits this way; Qt was leaving them empty so - // bindings that distinguish left-vs-right modifier keys couldn't - // fire. - ghostty_input_mods_e sideBitsForKeycode(uint32_t keycode) const { - syncFromTracker(); - if (!m_unshifted) return GHOSTTY_MODS_NONE; - const xkb_keysym_t sym = - xkb_state_key_get_one_sym(m_unshifted, keycode); - int r = GHOSTTY_MODS_NONE; - switch (sym) { - case XKB_KEY_Shift_R: r |= GHOSTTY_MODS_SHIFT_RIGHT; break; - case XKB_KEY_Control_R: r |= GHOSTTY_MODS_CTRL_RIGHT; break; - // Both Alt_R and ISO_Level3_Shift (AltGr) are right-Alt physically. - case XKB_KEY_Alt_R: - case XKB_KEY_ISO_Level3_Shift: r |= GHOSTTY_MODS_ALT_RIGHT; break; - case XKB_KEY_Super_R: - case XKB_KEY_Hyper_R: - case XKB_KEY_Meta_R: r |= GHOSTTY_MODS_SUPER_RIGHT; break; - default: break; - } - return static_cast(r); - } - - // Caps Lock / Num Lock state from the live wl_keyboard tracker. - ghostty_input_mods_e lockMods() const { - int r = GHOSTTY_MODS_NONE; - if (XkbTracker *t = XkbTracker::instance()) { - if (t->capsLockOn()) r |= GHOSTTY_MODS_CAPS; - if (t->numLockOn()) r |= GHOSTTY_MODS_NUM; - } - return static_cast(r); - } - - // Modifiers consumed by the layout to produce `keycode`'s text given - // `mods` are depressed. Returns the consumed subset, expressed as - // ghostty mod bits. Mutates m_query (mutable) — see thread-safety - // note on the class. - ghostty_input_mods_e consumedMods(uint32_t keycode, - ghostty_input_mods_e mods) const { - syncFromTracker(); - if (!m_query) return GHOSTTY_MODS_NONE; - xkb_mod_mask_t depressed = 0; - if ((mods & GHOSTTY_MODS_SHIFT) && m_idxShift != XKB_MOD_INVALID) - depressed |= (1u << m_idxShift); - if ((mods & GHOSTTY_MODS_CTRL) && m_idxCtrl != XKB_MOD_INVALID) - depressed |= (1u << m_idxCtrl); - if ((mods & GHOSTTY_MODS_ALT) && m_idxAlt != XKB_MOD_INVALID) - depressed |= (1u << m_idxAlt); - if ((mods & GHOSTTY_MODS_SUPER) && m_idxSuper != XKB_MOD_INVALID) - depressed |= (1u << m_idxSuper); - // Use the live group from the tracker so a layout switch (e.g. - // us↔ru) takes effect immediately. - const uint32_t group = - XkbTracker::instance() ? XkbTracker::instance()->activeGroup() : 0; - xkb_state_update_mask(m_query, depressed, 0, 0, 0, 0, group); - const xkb_mod_mask_t consumed = xkb_state_key_get_consumed_mods2( - m_query, keycode, XKB_CONSUMED_MODE_XKB); - // Reset so the next query starts from no-mods. - xkb_state_update_mask(m_query, 0, 0, 0, 0, 0, group); - int r = GHOSTTY_MODS_NONE; - if (m_idxShift != XKB_MOD_INVALID && (consumed & (1u << m_idxShift))) - r |= GHOSTTY_MODS_SHIFT; - if (m_idxCtrl != XKB_MOD_INVALID && (consumed & (1u << m_idxCtrl))) - r |= GHOSTTY_MODS_CTRL; - if (m_idxAlt != XKB_MOD_INVALID && (consumed & (1u << m_idxAlt))) - r |= GHOSTTY_MODS_ALT; - if (m_idxSuper != XKB_MOD_INVALID && (consumed & (1u << m_idxSuper))) - r |= GHOSTTY_MODS_SUPER; - return static_cast(r); - } - -private: - // Lazy: build/rebuild m_unshifted + m_query from the live keymap. - // Called from every public method; cheap when the keymap pointer - // hasn't changed (a single comparison + early-return). - void syncFromTracker() const { - XkbTracker *t = XkbTracker::instance(); - xkb_keymap *liveKm = t ? t->keymap() : nullptr; - xkb_keymap *km = liveKm ? liveKm : m_fallbackKeymap; - - if (!km && t && t->ctx()) { - // Compositor hasn't sent a keymap yet (early startup). Build a - // throwaway from XKB defaults so the first key event isn't - // dropped; it will be replaced on the next syncFromTracker - // call once the tracker has the live keymap. - m_fallbackKeymap = xkb_keymap_new_from_names( - t->ctx(), nullptr, XKB_KEYMAP_COMPILE_NO_FLAGS); - km = m_fallbackKeymap; - } - if (!km || km == m_keymap) { - // Already synced (or no keymap available at all). - // Update the live group on m_unshifted so the level-0 lookup - // honors the active layout, even when the keymap pointer - // hasn't changed. - if (m_unshifted && t) { - xkb_state_update_mask(m_unshifted, 0, 0, 0, 0, 0, t->activeGroup()); - } - return; - } - - // The live keymap was rebuilt by the tracker (or we're picking - // up the first one). Drop our derived states and rebuild. Take - // an extra ref on the keymap while it's our cached identity so - // the xkb allocator can't free it and reuse the same address - // for a different keymap (the ABA hazard the previous comment - // hand-waved away). - if (m_unshifted) xkb_state_unref(m_unshifted); - if (m_query) xkb_state_unref(m_query); - if (m_keymap) xkb_keymap_unref(m_keymap); - m_keymap = xkb_keymap_ref(km); - m_unshifted = xkb_state_new(km); - m_query = xkb_state_new(km); - m_idxShift = xkb_keymap_mod_get_index(km, XKB_MOD_NAME_SHIFT); - m_idxCtrl = xkb_keymap_mod_get_index(km, XKB_MOD_NAME_CTRL); - m_idxAlt = xkb_keymap_mod_get_index(km, XKB_MOD_NAME_ALT); - m_idxSuper = xkb_keymap_mod_get_index(km, XKB_MOD_NAME_LOGO); - if (t) - xkb_state_update_mask(m_unshifted, 0, 0, 0, 0, 0, t->activeGroup()); - } - - XkbState() = default; - - ~XkbState() { - // Run on process exit when the static is destroyed. The OS would - // reclaim regardless, but explicit teardown silences leak checkers - // and documents the ownership chain. - if (m_query) xkb_state_unref(m_query); - if (m_unshifted) xkb_state_unref(m_unshifted); - if (m_keymap) xkb_keymap_unref(m_keymap); - if (m_fallbackKeymap) xkb_keymap_unref(m_fallbackKeymap); - } - - XkbState(const XkbState &) = delete; - XkbState &operator=(const XkbState &) = delete; - - // The keymap our derived states were built from. We hold a ref - // here (taken in syncFromTracker, released on rebuild and in dtor) - // so the xkb allocator can't free + reuse the address while we - // still cache it as our identity. - mutable struct xkb_keymap *m_keymap = nullptr; - // Throwaway keymap from XKB defaults, built when the live keymap - // hasn't arrived yet. Owned. Released in dtor; never replaced. - mutable struct xkb_keymap *m_fallbackKeymap = nullptr; - mutable struct xkb_state *m_unshifted = nullptr; // no-mods state - // Reused across consumedMods calls (mutated then reset). - mutable struct xkb_state *m_query = nullptr; - mutable xkb_mod_index_t m_idxShift = XKB_MOD_INVALID; - mutable xkb_mod_index_t m_idxCtrl = XKB_MOD_INVALID; - mutable xkb_mod_index_t m_idxAlt = XKB_MOD_INVALID; - mutable xkb_mod_index_t m_idxSuper = XKB_MOD_INVALID; -}; - void GhosttySurface::sendKey(QKeyEvent *ev, ghostty_input_action_e action) { if (!m_surface) return; diff --git a/qt/src/input/XkbState.cpp b/qt/src/input/XkbState.cpp new file mode 100644 index 000000000..a30484ab8 --- /dev/null +++ b/qt/src/input/XkbState.cpp @@ -0,0 +1,135 @@ +#include "XkbState.h" + +#include "../XkbTracker.h" + +XkbState &XkbState::instance() { + static XkbState self; + return self; +} + +XkbState::~XkbState() { + // Run on process exit when the static is destroyed. The OS would + // reclaim regardless, but explicit teardown silences leak checkers + // and documents the ownership chain. + if (m_query) xkb_state_unref(m_query); + if (m_unshifted) xkb_state_unref(m_unshifted); + if (m_keymap) xkb_keymap_unref(m_keymap); + if (m_fallbackKeymap) xkb_keymap_unref(m_fallbackKeymap); +} + +uint32_t XkbState::unshiftedCodepoint(uint32_t keycode) const { + syncFromTracker(); + if (!m_unshifted) return 0; + const xkb_keysym_t sym = + xkb_state_key_get_one_sym(m_unshifted, keycode); + if (sym == XKB_KEY_NoSymbol) return 0; + return xkb_keysym_to_utf32(sym); +} + +ghostty_input_mods_e XkbState::sideBitsForKeycode(uint32_t keycode) const { + syncFromTracker(); + if (!m_unshifted) return GHOSTTY_MODS_NONE; + const xkb_keysym_t sym = + xkb_state_key_get_one_sym(m_unshifted, keycode); + int r = GHOSTTY_MODS_NONE; + switch (sym) { + case XKB_KEY_Shift_R: r |= GHOSTTY_MODS_SHIFT_RIGHT; break; + case XKB_KEY_Control_R: r |= GHOSTTY_MODS_CTRL_RIGHT; break; + // Both Alt_R and ISO_Level3_Shift (AltGr) are right-Alt physically. + case XKB_KEY_Alt_R: + case XKB_KEY_ISO_Level3_Shift: r |= GHOSTTY_MODS_ALT_RIGHT; break; + case XKB_KEY_Super_R: + case XKB_KEY_Hyper_R: + case XKB_KEY_Meta_R: r |= GHOSTTY_MODS_SUPER_RIGHT; break; + default: break; + } + return static_cast(r); +} + +ghostty_input_mods_e XkbState::lockMods() const { + int r = GHOSTTY_MODS_NONE; + if (XkbTracker *t = XkbTracker::instance()) { + if (t->capsLockOn()) r |= GHOSTTY_MODS_CAPS; + if (t->numLockOn()) r |= GHOSTTY_MODS_NUM; + } + return static_cast(r); +} + +ghostty_input_mods_e XkbState::consumedMods(uint32_t keycode, + ghostty_input_mods_e mods) const { + syncFromTracker(); + if (!m_query) return GHOSTTY_MODS_NONE; + xkb_mod_mask_t depressed = 0; + if ((mods & GHOSTTY_MODS_SHIFT) && m_idxShift != XKB_MOD_INVALID) + depressed |= (1u << m_idxShift); + if ((mods & GHOSTTY_MODS_CTRL) && m_idxCtrl != XKB_MOD_INVALID) + depressed |= (1u << m_idxCtrl); + if ((mods & GHOSTTY_MODS_ALT) && m_idxAlt != XKB_MOD_INVALID) + depressed |= (1u << m_idxAlt); + if ((mods & GHOSTTY_MODS_SUPER) && m_idxSuper != XKB_MOD_INVALID) + depressed |= (1u << m_idxSuper); + // Use the live group from the tracker so a layout switch (e.g. + // us↔ru) takes effect immediately. + const uint32_t group = + XkbTracker::instance() ? XkbTracker::instance()->activeGroup() : 0; + xkb_state_update_mask(m_query, depressed, 0, 0, 0, 0, group); + const xkb_mod_mask_t consumed = xkb_state_key_get_consumed_mods2( + m_query, keycode, XKB_CONSUMED_MODE_XKB); + // Reset so the next query starts from no-mods. + xkb_state_update_mask(m_query, 0, 0, 0, 0, 0, group); + int r = GHOSTTY_MODS_NONE; + if (m_idxShift != XKB_MOD_INVALID && (consumed & (1u << m_idxShift))) + r |= GHOSTTY_MODS_SHIFT; + if (m_idxCtrl != XKB_MOD_INVALID && (consumed & (1u << m_idxCtrl))) + r |= GHOSTTY_MODS_CTRL; + if (m_idxAlt != XKB_MOD_INVALID && (consumed & (1u << m_idxAlt))) + r |= GHOSTTY_MODS_ALT; + if (m_idxSuper != XKB_MOD_INVALID && (consumed & (1u << m_idxSuper))) + r |= GHOSTTY_MODS_SUPER; + return static_cast(r); +} + +void XkbState::syncFromTracker() const { + XkbTracker *t = XkbTracker::instance(); + xkb_keymap *liveKm = t ? t->keymap() : nullptr; + xkb_keymap *km = liveKm ? liveKm : m_fallbackKeymap; + + if (!km && t && t->ctx()) { + // Compositor hasn't sent a keymap yet (early startup). Build a + // throwaway from XKB defaults so the first key event isn't + // dropped; it will be replaced on the next syncFromTracker + // call once the tracker has the live keymap. + m_fallbackKeymap = xkb_keymap_new_from_names( + t->ctx(), nullptr, XKB_KEYMAP_COMPILE_NO_FLAGS); + km = m_fallbackKeymap; + } + if (!km || km == m_keymap) { + // Already synced (or no keymap available at all). + // Update the live group on m_unshifted so the level-0 lookup + // honors the active layout, even when the keymap pointer + // hasn't changed. + if (m_unshifted && t) { + xkb_state_update_mask(m_unshifted, 0, 0, 0, 0, 0, t->activeGroup()); + } + return; + } + + // The live keymap was rebuilt by the tracker (or we're picking + // up the first one). Drop our derived states and rebuild. Take + // an extra ref on the keymap while it's our cached identity so + // the xkb allocator can't free it and reuse the same address + // for a different keymap (the ABA hazard the previous comment + // hand-waved away). + if (m_unshifted) xkb_state_unref(m_unshifted); + if (m_query) xkb_state_unref(m_query); + if (m_keymap) xkb_keymap_unref(m_keymap); + m_keymap = xkb_keymap_ref(km); + m_unshifted = xkb_state_new(km); + m_query = xkb_state_new(km); + m_idxShift = xkb_keymap_mod_get_index(km, XKB_MOD_NAME_SHIFT); + m_idxCtrl = xkb_keymap_mod_get_index(km, XKB_MOD_NAME_CTRL); + m_idxAlt = xkb_keymap_mod_get_index(km, XKB_MOD_NAME_ALT); + m_idxSuper = xkb_keymap_mod_get_index(km, XKB_MOD_NAME_LOGO); + if (t) + xkb_state_update_mask(m_unshifted, 0, 0, 0, 0, 0, t->activeGroup()); +} diff --git a/qt/src/input/XkbState.h b/qt/src/input/XkbState.h new file mode 100644 index 000000000..a59a92c5d --- /dev/null +++ b/qt/src/input/XkbState.h @@ -0,0 +1,83 @@ +#pragma once + +#include + +#include + +#include "ghostty.h" + +// Wraps a libxkbcommon keymap + state derived from the live keymap +// XkbTracker syncs via wl_keyboard (with a fallback to the system +// XKB defaults until the compositor's keymap arrives). We need this +// for two things: +// +// 1. The unshifted codepoint a key would produce with no modifiers — +// libghostty's kitty encoder uses it to find a key entry for +// printable keys (without it, punctuation falls into a fallback +// that mis-encodes release events). +// +// 2. Which modifiers the layout "consumed" to produce the event's +// text — e.g. Shift+; → ":" consumes Shift. The encoder uses +// this to decide between plain text and a modifier-bearing CSI; +// without it Shift+punctuation gets emitted as a kitty CSI the +// shell can't decode (Shift+letter happens to work because A-Z +// survive that path). +// +// THREAD SAFETY: this is a process singleton accessed only from the +// Qt GUI thread (Qt key events are dispatched there, and so is +// libghostty's inputMethodEvent forwarding). consumedMods mutates +// internal state; do not call from worker threads. +class XkbState { +public: + static XkbState &instance(); + + // Level-0 (unshifted) Unicode codepoint for `keycode`, or 0 if the + // key has no associated UTF-32 (function keys, modifiers, etc.). + // Honors the active layout group from the live tracker so a us+ru + // user gets the correct codepoint per active group, not always us. + uint32_t unshiftedCodepoint(uint32_t keycode) const; + + // Side bits for the libghostty mods bitfield, derived from a + // keycode — pressing Right-Shift sets BOTH the unsided + // GHOSTTY_MODS_SHIFT and GHOSTTY_MODS_SHIFT_RIGHT bit (a left-side + // keycode sets only the unsided bit). macOS and GTK populate sided + // bits this way; Qt was leaving them empty so bindings that + // distinguish left-vs-right modifier keys couldn't fire. + ghostty_input_mods_e sideBitsForKeycode(uint32_t keycode) const; + + // Caps Lock / Num Lock state from the live wl_keyboard tracker. + ghostty_input_mods_e lockMods() const; + + // Modifiers consumed by the layout to produce `keycode`'s text + // given `mods` are depressed. Returns the consumed subset + // expressed as ghostty mod bits. + ghostty_input_mods_e consumedMods(uint32_t keycode, + ghostty_input_mods_e mods) const; + + XkbState(const XkbState &) = delete; + XkbState &operator=(const XkbState &) = delete; + +private: + XkbState() = default; + ~XkbState(); + + // Build / rebuild the derived states from the live keymap. Cheap + // when the keymap pointer is unchanged (one comparison + return). + void syncFromTracker() const; + + // The keymap our derived states were built from. A ref taken in + // syncFromTracker (released on rebuild and in dtor) keeps the xkb + // allocator from freeing + reusing the address while we still + // cache it as our identity. + mutable struct xkb_keymap *m_keymap = nullptr; + // Throwaway keymap from XKB defaults, built when the live keymap + // hasn't arrived yet. Owned. Released in dtor; never replaced. + mutable struct xkb_keymap *m_fallbackKeymap = nullptr; + mutable struct xkb_state *m_unshifted = nullptr; // no-mods state + // Reused across consumedMods calls (mutated then reset). + mutable struct xkb_state *m_query = nullptr; + mutable xkb_mod_index_t m_idxShift = XKB_MOD_INVALID; + mutable xkb_mod_index_t m_idxCtrl = XKB_MOD_INVALID; + mutable xkb_mod_index_t m_idxAlt = XKB_MOD_INVALID; + mutable xkb_mod_index_t m_idxSuper = XKB_MOD_INVALID; +}; From 6a90de48d9753a1107b87f4d40dca0263dda11c8 Mon Sep 17 00:00:00 2001 From: ntomsic Date: Sat, 23 May 2026 15:43:22 -0500 Subject: [PATCH 5/5] =?UTF-8?q?fix(audit):=20pass=201=20=E2=80=94=20latent?= =?UTF-8?q?=20QT-hide=20bug,=20dead=20includes,=20header=20tidy?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - QuickTerminal.cpp animateIn: clear the previous `finished -> hide()` handler that animateOut left attached to the cached QPropertyAnimation. Without this, an out-then-in cycle (autohide followed by re-toggle while the fade-out was still running) lets the leftover hide handler fire at the end of the in-fade and silently hide the just-revealed window. Latent in the original MainWindow code; fixed during the extraction. - QuickTerminal.cpp kAnimProperty: rename `_ghastty_qt_anim` → `ghastty.quickterm.anim`. Qt reserves the underscore-prefix property namespace; the dotted form is application-scoped and unambiguous. - QuickTerminal.cpp setupLayerShell: comment the `screen` reassignment after `ls->setScreen(...)` so a future reader doesn't think the anchor is being re-pointed. The reassignment is for sizing only. - MainWindow.h: drop stale `` and `` includes (UndoEntry-era leftovers; consumers that need them include them directly), and drop the orphaned three-line comment that survived the m_quickTerminalAnim member removal. - UndoStack.{h,cpp} pushTab: drop the dead `quickTerminal` parameter. Every caller in MainWindow.cpp already gates on `!m_quickTerminal`, so the function-local re-check was vacuous; the comment now documents the caller-side contract. - XkbState.cpp: include `` directly per IWYU (was reaching it transitively through XkbState.h); cache the XkbTracker singleton in `consumedMods` instead of calling `XkbTracker::instance()` twice in succession. - GhosttySurface.cpp: drop the now-dead `XkbTracker.h` include — the TU's only XkbTracker reference moved into XkbState.cpp during Phase 7. The remaining mention of "XkbTracker" in this file is a doc comment, not code. Co-Authored-By: claude-flow --- qt/src/GhosttySurface.cpp | 1 - qt/src/MainWindow.cpp | 4 ++-- qt/src/MainWindow.h | 5 ----- qt/src/input/XkbState.cpp | 6 ++++-- qt/src/quickterm/QuickTerminal.cpp | 15 +++++++++++++-- qt/src/undo/UndoStack.cpp | 3 +-- qt/src/undo/UndoStack.h | 9 ++++----- 7 files changed, 24 insertions(+), 19 deletions(-) diff --git a/qt/src/GhosttySurface.cpp b/qt/src/GhosttySurface.cpp index 09ad0429b..6176d8ae7 100644 --- a/qt/src/GhosttySurface.cpp +++ b/qt/src/GhosttySurface.cpp @@ -8,7 +8,6 @@ #include "SearchBar.h" #include "TabWidget.h" #include "Util.h" -#include "XkbTracker.h" #include #include diff --git a/qt/src/MainWindow.cpp b/qt/src/MainWindow.cpp index af589e009..f7485aa6b 100644 --- a/qt/src/MainWindow.cpp +++ b/qt/src/MainWindow.cpp @@ -440,7 +440,7 @@ void MainWindow::removeSurface(GhosttySurface *surface) { // double-stack. Also skip the quick terminal (which doesn't push // to either stack by design). if (index >= 0 && m_tabs->count() > 1 && !m_quickTerminal) - undo::pushTab(m_tabs->tabText(index), m_quickTerminal); + undo::pushTab(m_tabs->tabText(index)); if (index >= 0) m_tabs->removeTab(index); if (parent) parent->deleteLater(); // page; destroys the surface too // The surface close was already confirmed; don't re-prompt on the @@ -458,7 +458,7 @@ void MainWindow::closeTab(int index) { // undo::pushTab is no-op for the last tab in a window — that close // ends up triggering undo::pushWindow via closeEvent instead. if (m_tabs->count() > 1 && !m_quickTerminal) - undo::pushTab(m_tabs->tabText(index), m_quickTerminal); + undo::pushTab(m_tabs->tabText(index)); const auto inTab = page->findChildren(); for (GhosttySurface *s : inTab) m_surfaces.removeOne(s); // If the zoomed surface was in this tab, clear the stash so a later diff --git a/qt/src/MainWindow.h b/qt/src/MainWindow.h index 69b1a1e0a..53c99d480 100644 --- a/qt/src/MainWindow.h +++ b/qt/src/MainWindow.h @@ -1,9 +1,7 @@ #pragma once #include -#include #include -#include #include #include "ghostty.h" @@ -227,9 +225,6 @@ private: ghostty_surface_t m_firstTabParent = nullptr; // inherited by the 1st tab bool m_skipCloseConfirm = false; // close already confirmed elsewhere bool m_quickTerminal = false; // this is the dropdown quick terminal - // Per-window opacity animation for the quick terminal (fade in/out - // using quick-terminal-animation-duration). Owned by quickterm/'s - // dynamic-property cache on this widget; cleared on widget delete. QSize m_defaultWindowSize; // for RESET_WINDOW_SIZE; from INITIAL_SIZE // Last cell size reported by libghostty for this window's surfaces // (CELL_SIZE action). Stored so future grid-snap resizing can use diff --git a/qt/src/input/XkbState.cpp b/qt/src/input/XkbState.cpp index a30484ab8..c691c5f25 100644 --- a/qt/src/input/XkbState.cpp +++ b/qt/src/input/XkbState.cpp @@ -1,5 +1,7 @@ #include "XkbState.h" +#include + #include "../XkbTracker.h" XkbState &XkbState::instance() { @@ -70,8 +72,8 @@ ghostty_input_mods_e XkbState::consumedMods(uint32_t keycode, depressed |= (1u << m_idxSuper); // Use the live group from the tracker so a layout switch (e.g. // us↔ru) takes effect immediately. - const uint32_t group = - XkbTracker::instance() ? XkbTracker::instance()->activeGroup() : 0; + XkbTracker *t = XkbTracker::instance(); + const uint32_t group = t ? t->activeGroup() : 0; xkb_state_update_mask(m_query, depressed, 0, 0, 0, 0, group); const xkb_mod_mask_t consumed = xkb_state_key_get_consumed_mods2( m_query, keycode, XKB_CONSUMED_MODE_XKB); diff --git a/qt/src/quickterm/QuickTerminal.cpp b/qt/src/quickterm/QuickTerminal.cpp index 35bb1b182..b00d647c6 100644 --- a/qt/src/quickterm/QuickTerminal.cpp +++ b/qt/src/quickterm/QuickTerminal.cpp @@ -25,8 +25,10 @@ namespace { // Anim and toggle live on the QObject child tree of `window`, so // they die with it. We keep the QPropertyAnimation as a dynamic -// property so callers don't need to thread it through. -constexpr const char *kAnimProperty = "_ghastty_qt_anim"; +// property so callers don't need to thread it through. The "_q_" +// underscore-prefix space is reserved by Qt; any other prefix is +// fine and the dotted form keeps it visibly application-scoped. +constexpr const char *kAnimProperty = "ghastty.quickterm.anim"; // Read quick-terminal-animation-duration (seconds) and convert to ms. // Clamps to a sane range so a misconfigured 0/negative value doesn't @@ -98,6 +100,9 @@ void setupLayerShell(QWidget *window) { screen = QGuiApplication::primaryScreen(); } ls->setScreen(screen); + // For sizing only — LayerShellQt already has the anchor screen above + // (or fell back to the QWindow's screen via setScreen(nullptr)). We + // need a non-null QScreen below to read its pixel dimensions. if (!screen) screen = handle->screen(); // quick-terminal-space-behavior (`remain` / `move`) is intentionally @@ -171,6 +176,12 @@ void animateIn(QWidget *window) { // animations. QPropertyAnimation *anim = animFor(window); anim->stop(); + // animateOut leaves a `finished -> hide()` handler attached to the + // shared animation object. If a fade-out was interrupted by this + // fade-in (rapid out/in cycle), the leftover handler would fire at + // the end of the in-fade and silently hide the just-revealed + // window — clear it before starting. + QObject::disconnect(anim, &QPropertyAnimation::finished, window, nullptr); anim->setDuration(ms); anim->setStartValue(0.0); anim->setEndValue(1.0); diff --git a/qt/src/undo/UndoStack.cpp b/qt/src/undo/UndoStack.cpp index 95674ea51..67652197d 100644 --- a/qt/src/undo/UndoStack.cpp +++ b/qt/src/undo/UndoStack.cpp @@ -74,9 +74,8 @@ void pushUndo(Entry e) { } // namespace -void pushTab(const QString &tabText, bool quickTerminal) { +void pushTab(const QString &tabText) { if (g_redoInProgress) return; - if (quickTerminal) return; Entry e; e.kind = Entry::Kind::Tab; e.pageTitles << tabText; diff --git a/qt/src/undo/UndoStack.h b/qt/src/undo/UndoStack.h index 939deb080..3888a13b3 100644 --- a/qt/src/undo/UndoStack.h +++ b/qt/src/undo/UndoStack.h @@ -30,11 +30,10 @@ struct Entry { QRect geometry; }; -// Snapshot the tab at `index` — single title — onto the undo stack. -// `tabText` is the tab's last-known display text; `quickTerminal` is -// the window's quick-terminal flag (quick-terminal tabs are excluded -// from the stack, mirroring the prior MainWindow behavior). -void pushTab(const QString &tabText, bool quickTerminal); +// Snapshot a closed tab — its last-known display text — onto the +// undo stack. Callers MUST exclude quick-terminal and last-tab +// closes (the latter routes through pushWindow via closeEvent). +void pushTab(const QString &tabText); // Snapshot every tab's title plus the window's geometry as a single // Window entry. Excluded for the quick terminal and for empty