From e6ea00dce8c74bf0d48e769734958a741ca46ef9 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Mon, 3 Aug 2026 16:00:38 +0200 Subject: feat: persist window, splitter and column widths Resizing the window, the splitter or a thread-list column was undone by the next launch. State now round-trips through a separate settings file at ~/.local/state/qtmaildir/uistate.conf, written on close and read at startup. The state file is deliberately not the user's config: a base64 geometry blob does not belong in a hand-edited file, and rewriting that file on exit would drop its comments and key order, which QSettings does not preserve. Two details that are easy to get wrong: QStandardPaths::StateLocation appends both the organization and the application name, and both are "qtmaildir" here, so it resolves to ~/.local/state/qtmaildir/qtmaildir. The path is built from GenericStateLocation instead, matching Config::defaultPath(). Restore runs after buildMenus() rather than at the end of buildUi(): QMainWindow::restoreState() matches toolbars by object name and silently drops the position of one that does not exist yet. Every restore is conditional on a non-empty blob, so a missing or rejected state file leaves the built-in defaults instead of producing a zero-size window. Co-Authored-By: Claude Opus 5 --- src/mainwindow.cpp | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 71 insertions(+), 5 deletions(-) (limited to 'src/mainwindow.cpp') diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 48b475c..081984d 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -19,9 +19,12 @@ #include "mainwindow.h" #include +#include #include #include #include +#include +#include #include #include #include @@ -33,6 +36,7 @@ #include #include #include +#include #include #include #include @@ -61,6 +65,65 @@ QString MainWindow::cidPrefixForIndex(int index) return QStringLiteral("m%1").arg(index); } +QString MainWindow::uiStatePath() +{ + // GenericStateLocation, not StateLocation: the latter appends both the + // organization and the application name, and both are "qtmaildir" here, + // so it yields ~/.local/state/qtmaildir/qtmaildir. Built the same way + // Config::defaultPath() builds its own. + const QString base = + QStandardPaths::writableLocation(QStandardPaths::GenericStateLocation); + return base + QStringLiteral("/qtmaildir/uistate.conf"); +} + +void MainWindow::restoreUiState() +{ + QSettings state(uiStatePath(), QSettings::IniFormat); + + // Every restore is conditional: an absent or rejected blob must leave the + // buildUi() defaults alone rather than produce a zero-size window. + const QByteArray geometry = state.value(QStringLiteral("window/geometry")) + .toByteArray(); + if (!geometry.isEmpty()) { + restoreGeometry(geometry); + } + + const QByteArray windowState = state.value(QStringLiteral("window/state")) + .toByteArray(); + if (!windowState.isEmpty()) { + restoreState(windowState); + } + + const QByteArray splitter = state.value(QStringLiteral("window/splitter")) + .toByteArray(); + if (!splitter.isEmpty()) { + m_splitter->restoreState(splitter); + } + + const QByteArray header = state.value(QStringLiteral("threadlist/header")) + .toByteArray(); + if (!header.isEmpty()) { + m_threadView->horizontalHeader()->restoreState(header); + } +} + +void MainWindow::saveUiState() const +{ + QDir().mkpath(QFileInfo(uiStatePath()).absolutePath()); + QSettings state(uiStatePath(), QSettings::IniFormat); + state.setValue(QStringLiteral("window/geometry"), saveGeometry()); + state.setValue(QStringLiteral("window/state"), saveState()); + state.setValue(QStringLiteral("window/splitter"), m_splitter->saveState()); + state.setValue(QStringLiteral("threadlist/header"), + m_threadView->horizontalHeader()->saveState()); +} + +void MainWindow::closeEvent(QCloseEvent *event) +{ + saveUiState(); + QMainWindow::closeEvent(event); +} + MainWindow::MainWindow(const Config &config, QWidget *parent) : QMainWindow(parent), m_config(config) { @@ -87,6 +150,9 @@ MainWindow::MainWindow(const Config &config, QWidget *parent) buildUi(); registerActions(); buildMenus(); + // After buildMenus(): QMainWindow::restoreState() matches toolbars by + // object name, so they must already exist or their position is dropped. + restoreUiState(); wireWorker(); showWarnings(); @@ -210,11 +276,11 @@ void MainWindow::buildUi() connect(m_messageView, &MessageView::statusMessage, this, [this](const QString &text) { m_statusLabel->setText(text); }); - auto *splitter = new QSplitter(Qt::Horizontal, central); - splitter->addWidget(m_threadView); - splitter->addWidget(m_messageView); - splitter->setStretchFactor(1, 2); - layout->addWidget(splitter, 1); + m_splitter = new QSplitter(Qt::Horizontal, central); + m_splitter->addWidget(m_threadView); + m_splitter->addWidget(m_messageView); + m_splitter->setStretchFactor(1, 2); + layout->addWidget(m_splitter, 1); layout->addWidget(m_syncLog); -- cgit v1.2.3 From 212048782be680e2f99341db6c6460e59c708e7c Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Mon, 3 Aug 2026 16:23:43 +0200 Subject: feat: own the message-pane zoom and persist it Zoom was Chromium's, not the application's: the web view handled the keys natively and never told anyone, so there was no value to save. qtmaildir now owns it. Zoom in, out and reset are real actions, in the View menu and rebindable through [keys], and the factor is persisted to the UI state file. Ctrl+wheel over the body zooms and Ctrl+middle-click resets, both filtered by ancestry from an application-level filter: the events are delivered to an internal QQuickWidget the web view creates lazily, so a filter on the view itself never sees them. The factor is clamped to 0.5 - 3.0, and NaN, infinity, zero and negative values fall back to 1.0, since a corrupt state file must not be able to leave the pane unreadable with no visible way back. Both risks the plan flagged turned out not to exist, verified by probe rather than assumed. The application QAction wins over the web view's native zoom key, so the tracked factor cannot diverge from what is on screen. And the factor survives setHtml(), so the web view is the single source of truth and needs no reapply per render. A third finding is worth recording because it produced a wrong fix first. A probe using QTest::keyClick() reported Ctrl++ as a dead binding, and a test was written asserting that. Both were wrong: Ctrl++ is exactly what the '+' key emits on an Italian layout, confirmed against the real keyboard, and it is the shipped default. Whether a symbol needs Shift is a property of the layout, not of Qt, and keyClick() reproduces neither. The test now only checks that every default parses, and the comment in defaultBindings() says not to re-derive this from synthetic input. Ctrl+= is a second binding for reset, skipped when [keys] gives it to something else. Also fixes a pre-existing bug the new config key exposed. [general] entries were read as "general/", which matches nothing: QSettings' INI backend treats a section literally named [general] as its own fallback section and strips the prefix. notmuch_config had therefore never worked. Both keys are now read without it; the file format is unchanged. Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 17 +++- README.md | 4 + .../plans/2026-08-03-post-0.1.0-usability.md | 34 +++++++- src/config.cpp | 26 +++++- src/config.h | 6 ++ src/keymap.cpp | 12 +++ src/mainwindow.cpp | 38 +++++++++ src/messageview.cpp | 93 ++++++++++++++++++++++ src/messageview.h | 24 ++++++ tests/test_config.cpp | 56 +++++++++++++ tests/test_keymap.cpp | 20 +++++ tests/test_mainwindow.cpp | 3 + tests/test_messageview.cpp | 55 +++++++++++++ 13 files changed, 385 insertions(+), 3 deletions(-) (limited to 'src/mainwindow.cpp') diff --git a/CLAUDE.md b/CLAUDE.md index fd1283e..1b6a1bc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -77,9 +77,24 @@ Per-account subdirectories *are* configured, since notmuch does not model accoun **Config format gotcha:** QSettings treats `/` in a section name as a group separator, so account sections are `[account.work]`, not `[account/work]`. `childKeys` returns keys -sorted alphabetically, never in file order. Config lives at +sorted alphabetically, never in file order. **`[general]` keys are read WITHOUT the +`general/` prefix** — QSettings' INI backend treats a section literally named `[general]` +as its own fallback section and strips it, so a `general/` lookup silently matches +nothing (this is how `notmuch_config` went unnoticed as broken). Config lives at `~/.config/qtmaildir/qtmaildir.conf`. +Machine-written UI state is a **separate** file, `~/.local/state/qtmaildir/uistate.conf` +via `MainWindow::uiStatePath()`. Never write window blobs into the hand-edited config. +Build the path from `QStandardPaths::GenericStateLocation`, not `StateLocation`: the +latter appends both the organization and the application name, and both are `qtmaildir`. + +**Do not conclude a key binding is dead from `QTest::keyClick()`.** Whether a symbol needs +Shift is a layout property, not a Qt one. `Ctrl++` is the shipped `zoom_in` default and is +exactly what the `+` key emits on an Italian layout, while synthetic input never delivers +it. Verify against a real keyboard before changing a default on reachability grounds. The +separate, real trap `normalizeSequence()` handles is a **bare capital** (`N` parses to +unshifted Key_N, which no keystroke emits). + ## Web view security The most security-sensitive area: a browser engine pointed at input from strangers. Do not diff --git a/README.md b/README.md index 37ea3c9..6a0ecd7 100644 --- a/README.md +++ b/README.md @@ -79,6 +79,10 @@ identity. ; Optional. Omit to let notmuch resolve its own config, which is what keeps ; the GUI and the CLI pointed at one database. ; notmuch_config = /home/you/.notmuch-config +; Optional. Starting zoom of the message pane, 0.5 to 3.0. Only the starting +; point: once you zoom with Ctrl+wheel or Ctrl+/Ctrl-, that is remembered +; separately and this value no longer applies. +; message_zoom = 1.0 [sync] ; Optional. Omit and the Sync button disables itself with a tooltip. diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md index fc4ddbd..11863a8 100644 --- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md +++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md @@ -37,7 +37,7 @@ taking that too literally. | 1 | Splitter/column widths do not survive restart | persistence | S | **done** | | 2 | No way to see full message details (From/To/Cc/Subject) | information | M | open | | 3 | Too few clickable affordances, shortcuts are the only route | discoverability | M | **done** | -| 4 | Message-pane font size does not survive restart | persistence | S | open | +| 4 | Message-pane font size does not survive restart | persistence | S | **done** | | 5 | Thread list is cramped, poor readability | presentation | S | open | | 6 | Opened message stays unread | behavior | S | open | | 7 | HTML view should be default for HTML messages | behavior | XS | **verify first, may already be done** | @@ -261,6 +261,38 @@ already exists. - Route the actions through item 3's `QAction` conversion so they appear in the View menu, which also makes the reset discoverable. +### Outcome (done) + +Built as described, and both of the plan's stated risks turned out not to +exist. Probed rather than assumed: + +- **The application `QAction` wins over Chromium's native zoom key.** The plan + called this "the one real risk in the item". It is not one: the action fires + and the web view's own handling never runs, so the tracked factor cannot + diverge from what is on screen. +- **Zoom survives `setHtml()`.** The plan expected the view might reset it on + navigation and asked for a reapply per render. Not needed; the web view keeps + the factor, so it is the single source of truth and there is no second copy. +- **Do not test key reachability with synthetic input.** A probe using + `QTest::keyClick()` reported `Ctrl++` as a dead binding, and a test was + written asserting it. Both were wrong: `Ctrl++` is exactly what the `+` key + emits on an Italian layout, confirmed against the real keyboard, and it is + the shipped default. Whether a symbol needs Shift is a property of the + layout, not of Qt, and `keyClick()` reproduces neither. The test now only + checks that every default parses. +- `Ctrl+=` is a second binding for reset, skipped when `[keys]` gives `Ctrl+=` + to something else. Ctrl+wheel zooms and Ctrl+middle-click resets, both + filtered by ancestry from an application-level filter: the events land on an + internal `QQuickWidget` the web view creates lazily, so a filter installed on + the view itself never sees them. + +**A pre-existing bug surfaced while adding the config key.** `[general]` +entries were read as `general/`, which matches nothing: QSettings' INI +backend treats a section literally named `[general]` as its own fallback +section and strips the prefix. `notmuch_config` had therefore never worked. +Both keys are now read without the prefix; the file format the user writes is +unchanged. Regression test in `test_config`. + ## 5. Thread list is cramped **Observed:** rows are tightly packed, everything is uniform, the UI reads as diff --git a/src/config.cpp b/src/config.cpp index f21bba9..a2caaee 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -52,8 +52,32 @@ void Config::load(const QString &path) { QSettings settings(path, QSettings::IniFormat); + // Keys of [general] are read WITHOUT the "general/" prefix. QSettings' + // INI backend treats a section literally named [general] as its own + // fallback section and strips the prefix, so "general/notmuch_config" + // never matches anything, in any section arrangement (verified on + // Qt 6.11). The file still reads as [general] to the user; only the + // lookup differs. Same family of trap as the [account.work] dot and the + // childKeys() ordering already documented in CLAUDE.md. m_notmuchConfig = - settings.value(QStringLiteral("general/notmuch_config")).toString(); + settings.value(QStringLiteral("notmuch_config")).toString(); + + // Absent is fine and silent: the default is 1.0. Present but unparseable + // is a problem, since the user asked for something and is not getting it. + // The range check lives in MessageView::clampZoom(), the one place that + // knows what the web view can render. + const QVariant zoom = settings.value(QStringLiteral("message_zoom")); + if (zoom.isValid()) { + bool ok = false; + const double value = zoom.toString().toDouble(&ok); + if (ok) { + m_messageZoom = value; + } else { + addProblem(QStringLiteral("Message zoom '%1' is not a number; " + "using the default.") + .arg(zoom.toString())); + } + } m_syncCommand = settings.value(QStringLiteral("sync/command")).toString(); if (m_syncCommand.isEmpty()) { diff --git a/src/config.h b/src/config.h index 943cbd8..3e94249 100644 --- a/src/config.h +++ b/src/config.h @@ -78,6 +78,11 @@ public: /// Optional alternate notmuch config file. Empty means "let notmuch decide". QString notmuchConfig() const { return m_notmuchConfig; } + /// Starting message-pane zoom for a profile with no saved UI state. Once + /// the user zooms, the state file remembers that instead, so this is only + /// ever the default. Clamped by MessageView::clampZoom() on use. + qreal messageZoom() const { return m_messageZoom; } + /// Every non-fatal problem, both kinds below. Shown in the status bar. QStringList warnings() const { return m_warnings; } @@ -101,6 +106,7 @@ private: QList m_savedQueries; QString m_syncCommand; QString m_notmuchConfig; + qreal m_messageZoom = 1.0; QStringList m_warnings; QStringList m_problems; }; diff --git a/src/keymap.cpp b/src/keymap.cpp index 42ccd40..22c8da1 100644 --- a/src/keymap.cpp +++ b/src/keymap.cpp @@ -35,6 +35,9 @@ QStringList KeyMap::knownActions() QStringLiteral("focus_query"), QStringLiteral("toggle_html"), QStringLiteral("load_remote"), + QStringLiteral("zoom_in"), + QStringLiteral("zoom_out"), + QStringLiteral("zoom_reset"), QStringLiteral("undo"), QStringLiteral("sync"), QStringLiteral("quit"), @@ -63,6 +66,15 @@ QList> KeyMap::defaultBindings() { QStringLiteral("Ctrl+L"), QStringLiteral("focus_query") }, { QStringLiteral("Ctrl+H"), QStringLiteral("toggle_html") }, { QStringLiteral("Ctrl+M"), QStringLiteral("load_remote") }, + // Ctrl++ is what the '+' key really delivers on a layout where '+' is + // unshifted, an Italian one among them, confirmed against the actual + // keyboard. QTest::keyClick() cannot reproduce it, so a synthetic-input + // probe wrongly reports this binding as dead; do not "fix" it on that + // evidence. A US layout, where '+' is Shift+'=', wants Ctrl+Shift+= in + // [keys] instead. + { QStringLiteral("Ctrl++"), QStringLiteral("zoom_in") }, + { QStringLiteral("Ctrl+-"), QStringLiteral("zoom_out") }, + { QStringLiteral("Ctrl+0"), QStringLiteral("zoom_reset") }, { QStringLiteral("Ctrl+Z"), QStringLiteral("undo") }, { QStringLiteral("Ctrl+G"), QStringLiteral("sync") }, { QStringLiteral("Ctrl+Q"), QStringLiteral("quit") }, diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 081984d..c4955b6 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -105,6 +105,13 @@ void MainWindow::restoreUiState() if (!header.isEmpty()) { m_threadView->horizontalHeader()->restoreState(header); } + + // The config value is the starting point for a profile that has never + // zoomed; once the user does, the state file is what they last had. + // clampZoom() rejects the garbage a hand-edited file can hold. + m_messageView->setZoomFactor( + state.value(QStringLiteral("message/zoom"), m_config.messageZoom()) + .toDouble()); } void MainWindow::saveUiState() const @@ -116,6 +123,7 @@ void MainWindow::saveUiState() const state.setValue(QStringLiteral("window/splitter"), m_splitter->saveState()); state.setValue(QStringLiteral("threadlist/header"), m_threadView->horizontalHeader()->saveState()); + state.setValue(QStringLiteral("message/zoom"), m_messageView->zoomFactor()); } void MainWindow::closeEvent(QCloseEvent *event) @@ -381,6 +389,32 @@ void MainWindow::registerActions() tr("Load remote images for the current thread"), [this]() { m_messageView->loadRemoteContent(); }); + addAction(QStringLiteral("zoom_in"), tr("Zoom &in"), + tr("Enlarge the message text"), [this]() { + m_messageView->zoomIn(); + }); + addAction(QStringLiteral("zoom_out"), tr("Zoom &out"), + tr("Shrink the message text"), [this]() { + m_messageView->zoomOut(); + }); + auto *zoomReset = + addAction(QStringLiteral("zoom_reset"), tr("&Actual size"), + tr("Return the message text to its default size"), [this]() { + m_messageView->zoomReset(); + }); + + // Ctrl+= alongside the configured binding: '=' reads as "back to normal", + // and on a layout where '+' is Shift+'=' it is the unshifted key next to + // zoom in. Appended rather than assigned, so a [keys] override of + // zoom_reset keeps working and simply gains this as a second way in. + // A user who bound Ctrl+= to something else in [keys] keeps their binding. + const QKeySequence altReset(QStringLiteral("Ctrl+=")); + if (m_keyMap.actionFor(altReset).isEmpty()) { + QList shortcuts = zoomReset->shortcuts(); + shortcuts.append(altReset); + zoomReset->setShortcuts(shortcuts); + } + addAction(QStringLiteral("undo"), tr("&Undo"), tr("Undo the last tag change"), [this]() { if (m_undoStack.canUndo()) @@ -428,6 +462,10 @@ void MainWindow::buildMenus() viewMenu->addSeparator(); viewMenu->addAction(m_actions.value(QStringLiteral("toggle_html"))); viewMenu->addAction(m_actions.value(QStringLiteral("load_remote"))); + viewMenu->addSeparator(); + viewMenu->addAction(m_actions.value(QStringLiteral("zoom_in"))); + viewMenu->addAction(m_actions.value(QStringLiteral("zoom_out"))); + viewMenu->addAction(m_actions.value(QStringLiteral("zoom_reset"))); auto *helpMenu = menuBar()->addMenu(tr("&Help")); auto *shortcuts = helpMenu->addAction(tr("&Keyboard shortcuts")); diff --git a/src/messageview.cpp b/src/messageview.cpp index aebb81b..9a1d3e4 100644 --- a/src/messageview.cpp +++ b/src/messageview.cpp @@ -21,8 +21,12 @@ #include #include #include +#include +#include #include +#include #include +#include #include #include #include @@ -101,6 +105,13 @@ MessageView::MessageView(QWidget *parent) settings->setAttribute(QWebEngineSettings::PluginsEnabled, false); settings->setAttribute(QWebEngineSettings::FullScreenSupportEnabled, false); + // Ctrl+wheel zoom. The filter goes on the application rather than on + // m_view: the wheel event is delivered to an internal QQuickWidget the + // view creates lazily, so there is no child to filter at this point and a + // filter on m_view itself would never see it. eventFilter() narrows by + // ancestry, so no event outside this pane is touched. + qApp->installEventFilter(this); + m_headerLabel = new QLabel(this); m_headerLabel->setTextFormat(Qt::RichText); m_headerLabel->setWordWrap(true); @@ -281,6 +292,88 @@ void MessageView::toggleHtml() render(); } +bool MessageView::eventFilter(QObject *watched, QEvent *event) +{ + const QEvent::Type type = event->type(); + if (type != QEvent::Wheel && type != QEvent::MouseButtonPress) + return QWidget::eventFilter(watched, event); + + // Application-wide filter: only events inside this pane are ours. Anything + // else, including a Ctrl+wheel over the thread list, passes untouched. + // isAncestorOf() is false for the widget itself, so test that separately. + auto *widget = qobject_cast(watched); + if (!widget || (widget != m_view && !m_view->isAncestorOf(widget))) + return QWidget::eventFilter(watched, event); + + if (type == QEvent::Wheel) { + auto *wheel = static_cast(event); + if (!(wheel->modifiers() & Qt::ControlModifier)) + return QWidget::eventFilter(watched, event); + + // angleDelta is in eighths of a degree; one detent is 120. A high + // resolution wheel sends smaller steps, so scale rather than treating + // every event as one full step. + const int delta = wheel->angleDelta().y(); + if (delta != 0) + setZoomFactor(zoomFactor() + 0.1 * delta / 120.0); + + // Consumed, or Chromium's own Ctrl+wheel zoom would run on top of + // ours and the factor we track would no longer be what is on screen. + return true; + } + + // Ctrl+middle-click resets: the same hand that just zoomed with the wheel + // puts it back, without reaching for the keyboard. + auto *mouse = static_cast(event); + if (mouse->button() != Qt::MiddleButton + || !(mouse->modifiers() & Qt::ControlModifier)) { + return QWidget::eventFilter(watched, event); + } + + zoomReset(); + + // Consumed: a plain middle click is paste-on-X11 in some contexts, and + // this gesture must do one thing only. + return true; +} + +qreal MessageView::clampZoom(qreal factor) +{ + // qIsFinite rejects the NaN and infinity a corrupt or hand-edited state + // file can produce; qFuzzyIsNull rejects the 0.0 that a missing or + // non-numeric value converts to, which would render nothing at all. + if (!qIsFinite(factor) || factor <= 0.0) + return kDefaultZoom; + return qBound(kMinZoom, factor, kMaxZoom); +} + +qreal MessageView::zoomFactor() const +{ + // The web view is the single source of truth. It keeps the factor across + // setHtml(), verified on Qt 6.11, so there is no second copy to drift. + return m_view->zoomFactor(); +} + +void MessageView::setZoomFactor(qreal factor) +{ + m_view->setZoomFactor(clampZoom(factor)); +} + +void MessageView::zoomIn() +{ + setZoomFactor(zoomFactor() + 0.1); +} + +void MessageView::zoomOut() +{ + setZoomFactor(zoomFactor() - 0.1); +} + +void MessageView::zoomReset() +{ + setZoomFactor(kDefaultZoom); +} + void MessageView::loadRemoteContent() { // Applies to this thread only and is cleared by the next showThread(). diff --git a/src/messageview.h b/src/messageview.h index 9570db5..09e7d71 100644 --- a/src/messageview.h +++ b/src/messageview.h @@ -65,13 +65,37 @@ public: /// Tags of the thread on display, shown as chips along the bottom. void setTags(const QStringList &tags); + /// The body zoom factor. Chromium's own range is roughly 0.25 to 5.0; + /// these are tighter, since a pane at either extreme is unusable and the + /// only visible way back is a menu entry the user cannot read. + static constexpr qreal kMinZoom = 0.5; + static constexpr qreal kMaxZoom = 3.0; + static constexpr qreal kDefaultZoom = 1.0; + + /// Clamps to [kMinZoom, kMaxZoom]. A non-finite or non-positive value, + /// which is what a corrupt state file yields, falls back to kDefaultZoom. + static qreal clampZoom(qreal factor); + + qreal zoomFactor() const; + void setZoomFactor(qreal factor); + public slots: void toggleHtml(); void loadRemoteContent(); + void zoomIn(); + void zoomOut(); + void zoomReset(); signals: void statusMessage(const QString &text); +protected: + /// Turns Ctrl+wheel over the body into zoom, and Ctrl+middle-click into a + /// reset. Both events are delivered to the web view's internal QQuickWidget + /// focus proxy, not to the view itself, so this filters the whole subtree + /// rather than one widget. + bool eventFilter(QObject *watched, QEvent *event) override; + private: void render(); void updateHeader(); diff --git a/tests/test_config.cpp b/tests/test_config.cpp index e492480..367643c 100644 --- a/tests/test_config.cpp +++ b/tests/test_config.cpp @@ -34,6 +34,8 @@ private slots: void brokenSyncCommandIsAProblem(); void malformedAccountIsAProblem(); void validConfigHasNoProblems(); + void generalSectionKeysAreActuallyRead(); + void messageZoomDefaultsAndValidates(); }; static QString writeIni(const QTemporaryDir &dir, const QString &body) @@ -222,5 +224,59 @@ void TestConfig::validConfigHasNoProblems() QVERIFY(config.warnings().isEmpty()); } +void TestConfig::generalSectionKeysAreActuallyRead() +{ + // QSettings' INI backend treats a section literally named [general] as its + // own fallback section and strips the prefix, so a "general/" lookup + // matches nothing. notmuch_config was read that way and had never worked. + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral( + "[general]\n" + "notmuch_config=/somewhere/notmuch-config\n" + "\n" + "[sync]\n" + "command=/bin/true\n"))); + + QCOMPARE(config.notmuchConfig(), + QStringLiteral("/somewhere/notmuch-config")); +} + +void TestConfig::messageZoomDefaultsAndValidates() +{ + // A QTemporaryDir per case, not one shared: writeIni() always uses the + // same file name, and QSettings caches by path, so a second load of the + // same path would return the first case's contents. + + // Absent: 1.0, silently. Nothing the user asked for is being ignored. + { + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral("[general]\n"))); + QCOMPARE(config.messageZoom(), 1.0); + QVERIFY(config.problems().isEmpty()); + } + + { + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral("[general]\n" + "message_zoom=1.25\n"))); + QCOMPARE(config.messageZoom(), 1.25); + QVERIFY(config.problems().isEmpty()); + } + + // Present but unparseable is a problem: the user asked for something and + // is not getting it, which is the line addProblem() draws. + { + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral("[general]\n" + "message_zoom=huge\n"))); + QCOMPARE(config.messageZoom(), 1.0); + QCOMPARE(config.problems().size(), 1); + } +} + QTEST_MAIN(TestConfig) #include "test_config.moc" diff --git a/tests/test_keymap.cpp b/tests/test_keymap.cpp index c81eeb0..0fb4f57 100644 --- a/tests/test_keymap.cpp +++ b/tests/test_keymap.cpp @@ -36,8 +36,28 @@ private slots: void userBindingWinsOverDefaultInMenus(); void defaultsDoNotCollide(); void everyDefaultIsAKnownAction(); + void everyDefaultParses(); }; +void TestKeyMap::everyDefaultParses() +{ + // A default that does not parse is a dead binding, the failure mode + // bareCapitalMatchesShiftedPress() covers for user-written keys. + // + // This deliberately does NOT try to decide which keys a keyboard can + // deliver. Whether a symbol needs Shift is a layout property, not a Qt + // one: Ctrl++ is exactly what the '+' key emits on an Italian layout and + // is unreachable on a US one, and QTest::keyClick() cannot reproduce + // either faithfully. A test asserting reachability from synthetic input + // would encode one layout's habits as a rule for all of them. + for (const auto &binding : KeyMap::defaultBindings()) { + const QKeySequence sequence = KeyMap::normalizeSequence(binding.first); + QVERIFY2(!sequence.isEmpty(), + qPrintable(QStringLiteral("default '%1' for %2 does not parse") + .arg(binding.first, binding.second))); + } +} + void TestKeyMap::defaultsAreLoaded() { KeyMap map; diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 628472b..b4d9e4a 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -28,6 +28,7 @@ #include "config.h" #include "keymap.h" #include "mainwindow.h" +#include "messageview.h" /// MainWindow is mostly wiring, and the parts that need a real database are /// still verified manually. What is checked here is the action registry: the @@ -190,6 +191,7 @@ void TestMainWindow::uiStateSurvivesARestart() const Config config; MainWindow window(config); window.resize(resized); + window.findChild()->setZoomFactor(1.4); window.close(); // closeEvent() is what persists the state } @@ -200,6 +202,7 @@ void TestMainWindow::uiStateSurvivesARestart() const Config config; MainWindow reopened(config); QCOMPARE(reopened.size(), resized); + QCOMPARE(reopened.findChild()->zoomFactor(), 1.4); QFile::remove(MainWindow::uiStatePath()); QStandardPaths::setTestModeEnabled(false); diff --git a/tests/test_messageview.cpp b/tests/test_messageview.cpp index 0c3353d..4b6bc4c 100644 --- a/tests/test_messageview.cpp +++ b/tests/test_messageview.cpp @@ -36,6 +36,8 @@ private slots: void documentActuallyLoads(); void threadContentReachesThePage(); void dataUrlSubResourceStillBlocked(); + void zoomIsClampedToARenderableRange(); + void zoomSurvivesANewDocument(); private: QWebEngineView *webViewOf(MessageView *view) const @@ -175,5 +177,58 @@ void TestMessageView::dataUrlSubResourceStillBlocked() QVERIFY(text.contains(QStringLiteral("visible-text"))); } +void TestMessageView::zoomIsClampedToARenderableRange() +{ + // A factor outside the range leaves the pane unreadable, and the only way + // back is a menu entry the user can no longer read. A corrupt state file + // reaching setZoomFactor() must not be able to do that. + QCOMPARE(MessageView::clampZoom(100.0), MessageView::kMaxZoom); + QCOMPARE(MessageView::clampZoom(0.01), MessageView::kMinZoom); + + // A missing or non-numeric state value converts to 0.0, and a hand-edited + // one can hold NaN or an infinity. None of those may reach the web view. + QCOMPARE(MessageView::clampZoom(0.0), MessageView::kDefaultZoom); + QCOMPARE(MessageView::clampZoom(-2.0), MessageView::kDefaultZoom); + QCOMPARE(MessageView::clampZoom(qQNaN()), MessageView::kDefaultZoom); + QCOMPARE(MessageView::clampZoom(qInf()), MessageView::kDefaultZoom); + + // In-range values pass through untouched. + QCOMPARE(MessageView::clampZoom(1.4), 1.4); + + MessageView view; + view.setZoomFactor(50.0); + QCOMPARE(view.zoomFactor(), MessageView::kMaxZoom); +} + +void TestMessageView::zoomSurvivesANewDocument() +{ + // MainWindow persists whatever zoomFactor() reports and never reapplies it + // per render, which is only correct if the web view keeps the factor + // across setHtml(). Verified rather than assumed. + MessageView view; + QWebEngineView *web = webViewOf(&view); + QVERIFY(web); + + view.setZoomFactor(1.5); + + QSignalSpy loaded(web, &QWebEngineView::loadFinished); + + ParsedMessage message; + message.ok = true; + message.from = QStringLiteral("Sender "); + message.subject = QStringLiteral("Zoom"); + message.plainBody = QStringLiteral("body text"); + + ThreadRenderItem item; + item.message = message; + item.cidPrefix = QStringLiteral("m0"); + item.expanded = true; + + view.showThread({ item }); + QVERIFY(loaded.wait(15000)); + + QCOMPARE(view.zoomFactor(), 1.5); +} + QTEST_MAIN(TestMessageView) #include "test_messageview.moc" -- cgit v1.2.3 From 2c33529fb665cb54c31e54230fcb7b2491cf8565 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Mon, 3 Aug 2026 16:31:34 +0200 Subject: feat: choose the startup query by name The app opened whichever saved query sorted first alphabetically, which is not a choice anyone made: [queries] is read through childKeys(), so savedQueries().first() means "Flagged" before "Inbox" before "Unread" rather than anything the user expressed. [general] startup_query names the entry to open and defaults to Unread, so a fresh install comes up on the unified unread list. Saved-query button order is untouched and stays alphabetical. A name matching no saved query falls back to the first one rather than starting with an empty view. That is reported as a problem only when the user actually wrote the name; the built-in default naming a query they never created is not something they got wrong, and warning about it would fire on every launch of a config that has no Unread entry. Co-Authored-By: Claude Opus 5 --- README.md | 7 ++- .../plans/2026-08-03-post-0.1.0-usability.md | 15 +++++ src/config.cpp | 36 ++++++++++++ src/config.h | 18 ++++++ src/mainwindow.cpp | 8 ++- tests/test_config.cpp | 67 ++++++++++++++++++++++ 6 files changed, 148 insertions(+), 3 deletions(-) (limited to 'src/mainwindow.cpp') diff --git a/README.md b/README.md index 6a0ecd7..55b4f50 100644 --- a/README.md +++ b/README.md @@ -83,6 +83,10 @@ identity. ; point: once you zoom with Ctrl+wheel or Ctrl+/Ctrl-, that is remembered ; separately and this value no longer applies. ; message_zoom = 1.0 +; Optional. Which [queries] entry to open at startup, by name. Defaults to +; Unread. Falls back to the first saved query if no query by this name +; exists, and warns if you named one explicitly. +; startup_query = Unread [sync] ; Optional. Omit and the Sync button disables itself with a tooltip. @@ -125,7 +129,8 @@ k = prev_thread Saved-query buttons appear in alphabetical order rather than file order: QSettings returns keys sorted, and preserving file order would mean -hand-rolling an INI parser. +hand-rolling an INI parser. Which query opens at startup is therefore a +separate setting, `[general] startup_query`, rather than "the first one". ## Tags diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md index 11863a8..02bf7c4 100644 --- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md +++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md @@ -392,6 +392,21 @@ stays blocked and per-render. Do not build a full account sidebar for this. Persisting the selection may resolve the complaint entirely, and it is a fraction of the work. Reassess after. +### Partly done + +**The startup query is now chosen by name**, not by sort order. `[queries]` is +read through `childKeys()`, which sorts alphabetically, so the old +`savedQueries().first()` opened whichever entry happened to sort first, which +is why the app came up on Inbox. `[general] startup_query` names the entry, +defaults to `Unread`, and falls back to the first saved query when the name +matches nothing. Only a name the user wrote is worth a warning: the built-in +default naming a query they never created is not something they got wrong. + +Neither half of item 10 proper is done: the account selection still resets on +restart, and reaching an account's inbox is still two steps. Persisting the +selection remains the next cheap step, and the reassessment the item calls for +should happen after that rather than now. + ## 11. Icon, `.desktop` file, SlackBuild Packaging, independent of everything above, and can proceed in parallel. diff --git a/src/config.cpp b/src/config.cpp index a2caaee..e92c7f3 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -66,6 +66,14 @@ void Config::load(const QString &path) // is a problem, since the user asked for something and is not getting it. // The range check lives in MessageView::clampZoom(), the one place that // knows what the web view can render. + // Empty is treated as unset rather than as "a query named nothing". + const QString startup = + settings.value(QStringLiteral("startup_query")).toString().trimmed(); + if (!startup.isEmpty()) { + m_startupQuery = startup; + m_startupQueryWasSet = true; + } + const QVariant zoom = settings.value(QStringLiteral("message_zoom")); if (zoom.isValid()) { bool ok = false; @@ -149,6 +157,34 @@ void Config::load(const QString &path) for (const QString &name : settings.childKeys()) m_savedQueries.append({ name, settings.value(name).toString() }); settings.endGroup(); + + // Checked here rather than where startup_query is read: [queries] is not + // parsed until now. Only a name the user actually wrote is worth a + // problem; the built-in default naming a query they never created is not + // something they got wrong. + if (m_startupQueryWasSet && !m_savedQueries.isEmpty() + && startupSavedQuery().name.compare(m_startupQuery, + Qt::CaseInsensitive) != 0) { + addProblem(QStringLiteral("Startup query '%1' is not a saved query; " + "opening '%2' instead.") + .arg(m_startupQuery, startupSavedQuery().name)); + } +} + +SavedQuery Config::startupSavedQuery() const +{ + if (m_savedQueries.isEmpty()) + return {}; + + for (const SavedQuery &query : m_savedQueries) { + if (query.name.compare(m_startupQuery, Qt::CaseInsensitive) == 0) + return query; + } + + // Named a query that does not exist. Not worth a warning: the default is + // a name the user never wrote, so an install with no [queries] Unread + // entry would warn on every launch about a key it never set. + return m_savedQueries.first(); } Account Config::account(const QString &key) const diff --git a/src/config.h b/src/config.h index 3e94249..3b7fc48 100644 --- a/src/config.h +++ b/src/config.h @@ -78,6 +78,18 @@ public: /// Optional alternate notmuch config file. Empty means "let notmuch decide". QString notmuchConfig() const { return m_notmuchConfig; } + /// The saved query to open at startup, by name. Falls back to "Unread" + /// when unset, and to the first saved query when no query by that name + /// exists: [queries] is read through childKeys(), which sorts + /// alphabetically, so "first" would otherwise mean whatever happens to + /// sort first rather than anything the user chose. + QString startupQuery() const { return m_startupQuery; } + + /// The saved query startupQuery() names, or the first one when it names + /// nothing that exists. A default-constructed SavedQuery when there are + /// none at all. + SavedQuery startupSavedQuery() const; + /// Starting message-pane zoom for a profile with no saved UI state. Once /// the user zooms, the state file remembers that instead, so this is only /// ever the default. Clamped by MessageView::clampZoom() on use. @@ -107,6 +119,12 @@ private: QString m_syncCommand; QString m_notmuchConfig; qreal m_messageZoom = 1.0; + QString m_startupQuery = QStringLiteral("Unread"); + + /// Whether startup_query came from the config rather than being the + /// built-in default. Only a name the user wrote is worth reporting when + /// it matches no saved query. + bool m_startupQueryWasSet = false; QStringList m_warnings; QStringList m_problems; }; diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index c4955b6..2a79988 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -171,8 +171,12 @@ MainWindow::MainWindow(const Config &config, QWidget *parent) // modifier shortcuts such as Ctrl+Q still work there, which the old // filter blocked. - if (!m_config.savedQueries().isEmpty()) { - m_queryEdit->setText(m_config.savedQueries().first().query); + // Not savedQueries().first(): [queries] is read through childKeys(), which + // sorts alphabetically, so "first" means whatever happens to sort first + // rather than anything the user chose. Config resolves the name. + const SavedQuery startup = m_config.startupSavedQuery(); + if (!startup.query.isEmpty()) { + m_queryEdit->setText(startup.query); runCurrentQuery(); } } diff --git a/tests/test_config.cpp b/tests/test_config.cpp index 367643c..24f6afb 100644 --- a/tests/test_config.cpp +++ b/tests/test_config.cpp @@ -34,6 +34,9 @@ private slots: void brokenSyncCommandIsAProblem(); void malformedAccountIsAProblem(); void validConfigHasNoProblems(); + void startupQueryDefaultsToUnread(); + void startupQueryHonoursTheConfiguredName(); + void unknownStartupQueryFallsBackAndReports(); void generalSectionKeysAreActuallyRead(); void messageZoomDefaultsAndValidates(); }; @@ -224,6 +227,70 @@ void TestConfig::validConfigHasNoProblems() QVERIFY(config.warnings().isEmpty()); } +void TestConfig::startupQueryDefaultsToUnread() +{ + // [queries] is read through childKeys(), which sorts alphabetically, so + // savedQueries().first() is "Flagged" here. The startup query must be + // chosen by name, not by sort order. + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral( + "[queries]\n" + "Inbox=tag:inbox\n" + "Unread=tag:unread\n" + "Flagged=tag:flagged\n"))); + + QCOMPARE(config.savedQueries().first().name, QStringLiteral("Flagged")); + QCOMPARE(config.startupSavedQuery().name, QStringLiteral("Unread")); + QCOMPARE(config.startupSavedQuery().query, QStringLiteral("tag:unread")); + QVERIFY(config.problems().isEmpty()); +} + +void TestConfig::startupQueryHonoursTheConfiguredName() +{ + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral( + "[general]\n" + "startup_query=Flagged\n" + "\n" + "[queries]\n" + "Inbox=tag:inbox\n" + "Unread=tag:unread\n" + "Flagged=tag:flagged\n"))); + + QCOMPARE(config.startupSavedQuery().name, QStringLiteral("Flagged")); + QVERIFY(config.problems().isEmpty()); +} + +void TestConfig::unknownStartupQueryFallsBackAndReports() +{ + // A name the user wrote that matches nothing is a problem: they asked for + // something and are not getting it. Startup still works, on the fallback. + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral( + "[general]\n" + "startup_query=Nonexistent\n" + "\n" + "[queries]\n" + "Inbox=tag:inbox\n"))); + + QCOMPARE(config.startupSavedQuery().name, QStringLiteral("Inbox")); + QCOMPARE(config.problems().size(), 1); + + // The built-in default naming a query the user never created is NOT a + // problem: they did not get it wrong, they simply have no Unread entry. + QTemporaryDir quiet; + Config silent; + silent.load(writeIni(quiet, QStringLiteral( + "[queries]\n" + "Inbox=tag:inbox\n"))); + + QCOMPARE(silent.startupSavedQuery().name, QStringLiteral("Inbox")); + QVERIFY(silent.problems().isEmpty()); +} + void TestConfig::generalSectionKeysAreActuallyRead() { // QSettings' INI backend treats a section literally named [general] as its -- cgit v1.2.3