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 --- tests/test_config.cpp | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) (limited to 'tests/test_config.cpp') 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" -- 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 'tests/test_config.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