From 6a90de48d9753a1107b87f4d40dca0263dda11c8 Mon Sep 17 00:00:00 2001 From: ntomsic Date: Sat, 23 May 2026 15:43:22 -0500 Subject: [PATCH] =?UTF-8?q?fix(audit):=20pass=201=20=E2=80=94=20latent=20Q?= =?UTF-8?q?T-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