diff options
| -rw-r--r-- | CHANGELOG.md | 15 | ||||
| -rw-r--r-- | src/config.cpp | 27 | ||||
| -rw-r--r-- | src/mainwindow.cpp | 27 | ||||
| -rw-r--r-- | src/mainwindow.h | 8 | ||||
| -rw-r--r-- | tests/test_config.cpp | 110 | ||||
| -rw-r--r-- | tests/test_mainwindow.cpp | 62 |
6 files changed, 224 insertions, 25 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b4df18..942fe41 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,10 +14,23 @@ point at which they are stable. ### Changed - The built-in filters carry icons, like the Save button at the other end of - the query row, with their text beside them. + the query row, with their text beside them. Important is a star, and Sent + uses the folder icon rather than the envelope-in-flight some themes lack. ### Fixed +- **`startup_query` can name a built-in filter**, and looks at both those and + your saved queries. In 0.21.0 it searched saved queries only, so a + `startup_query = Inbox` stopped matching once the duplicated Inbox entry was + removed from `queries.json`, and the application opened on whichever query + happened to be first in that file. A name matching nothing now falls back to + the Unread filter rather than to an arbitrary saved query, and a saved query + still wins a name collision with a filter. + + A `startup_query` naming a filter also **runs**. A filter composes its query + from your accounts rather than storing one, and the startup path read the + stored field directly, so it would have opened on an empty query bar. + - The flagged filter is labelled **Important**, matching the action of the same name. It shipped in 0.21.0 as "Flagged", which put the same tag under two names in one window. diff --git a/src/config.cpp b/src/config.cpp index 0b7fe86..8ca7910 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -784,18 +784,31 @@ QString Config::resolvedQuery(const SavedQuery &query, SavedQuery Config::startupSavedQuery() const { - if (m_savedQueries.isEmpty()) - return {}; - + // The user's own queries first, so a saved query wins a name collision with + // a built-in filter: they named theirs deliberately, where the filter's + // name is one this application chose for them. 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(); + // Then the built-in filters. Without this, a startup_query of "Inbox" + // matched nothing once item 93 shipped Inbox as a filter and the duplicated + // saved query was removed, and the app started on whatever query happened + // to be first in the file. + for (const SavedQuery &filter : builtinFilters()) { + if (filter.name.compare(m_startupQuery, Qt::CaseInsensitive) == 0) + return filter; + } + + // Named nothing that exists. Falling back to m_savedQueries.first() is what + // this used to do and it is worse than it looks: after the duplicated + // entries were removed it could be any leftover query, so a startup view + // became a search for one sender, and an empty queries.json started nothing + // at all. A filter is always present, so the fallback can be one. + // + // Still not worth a warning: the default is a name the user never wrote. + return builtinFilter(QStringLiteral("unread")); } Account Config::account(const QString &key) const diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 4343479..3ebcbc4 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -404,9 +404,18 @@ MainWindow::MainWindow(const Config &config, QWidget *parent) // 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. + // resolvedQuery(), not startup.query: a generated entry stores no query at + // all, since its text is composed from the accounts at run time. Reading + // the field directly meant a startup_query naming a built-in filter opened + // an empty bar and ran nothing. + // + // No account scope here. The dropdown starts on "All accounts", which is + // the empty key, so this is the unscoped form either way; passing the + // selection would be reading a widget the user has not touched yet. const SavedQuery startup = m_config.startupSavedQuery(); - if (!startup.query.isEmpty()) { - m_queryEdit->setText(startup.query); + const QString startupQuery = m_config.resolvedQuery(startup, QString()); + if (!startupQuery.isEmpty()) { + m_queryEdit->setText(startupQuery); runCurrentQuery(); } } @@ -1746,13 +1755,19 @@ void MainWindow::buildSavedQueryRow(QWidget *parent, QVBoxLayout *layout) // is chrome. A name the running theme lacks degrades to text on its // own, which is why nothing here checks whether it resolved. // - // mail-mark-important matches the `flag` action's own icon, since both - // reach the same tag: the filter finds what the action marks. + // A STAR for Important, not mail-mark-important, which the `flag` + // action uses. Item 57 recorded the user asking for a star when the + // action was renamed, and on the query row the icon is read as a + // category rather than as "do this to the selection", so the two can + // differ. Chosen by the user on sight, 2026-08-15. + // + // mail-folder-sent, not mail-sent: the former is the folder shape every + // theme ships, the latter is the envelope-in-flight some do not. static const QHash<QString, QString> filterIcons = { { QStringLiteral("unread"), QStringLiteral("mail-mark-unread") }, { QStringLiteral("inbox"), QStringLiteral("mail-inbox") }, - { QStringLiteral("flagged"), QStringLiteral("mail-mark-important") }, - { QStringLiteral("sent"), QStringLiteral("mail-sent") }, + { QStringLiteral("flagged"), QStringLiteral("starred") }, + { QStringLiteral("sent"), QStringLiteral("mail-folder-sent") }, }; button->setIcon( QIcon::fromTheme(filterIcons.value(filter.generated))); diff --git a/src/mainwindow.h b/src/mainwindow.h index bb1ba75..03f21bb 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -186,6 +186,14 @@ public: /// stale, so a test standing in for the worker has to know the current one. quint64 currentGenerationForTesting() const { return m_generation; } + /// The query the visible list was actually built from. + /// + /// A test seam: a refresh re-runs THIS, never the text in the query bar, + /// and the difference is only observable through the value itself. The + /// generation counter cannot stand in for it, since a legitimate refresh + /// bumps the generation too. + QString lastRunQueryForTesting() const { return m_lastQuery; } + /// Puts the window into the state refreshCurrentQuery() leaves it in, and /// returns the generation the refresh's replies must carry. /// diff --git a/tests/test_config.cpp b/tests/test_config.cpp index e5487b8..0cacb04 100644 --- a/tests/test_config.cpp +++ b/tests/test_config.cpp @@ -60,7 +60,7 @@ private slots: void jsonWinsOnceItExists(); void malformedQueriesFileIsAProblemNotACrash(); void futureVersionIsRefusedAndReported(); - void startupQueryFallsBackToDocumentOrder(); + void startupQueryFallsBackToABuiltinFilter(); void scopedSavedQueryParenthesisesADisjunction(); void aGeneratedQueryResolvesFromTheAccounts(); void aGeneratedQueryTracksAConfigChange(); @@ -96,6 +96,9 @@ private slots: void allSentQuerySkipsAccountsWithoutTheKey(); void allSentQueryJoinsEveryConfiguredAccount(); void aStoredGeneratedQueryIsUnpinnedNotDropped(); + void theStartupQueryCanNameABuiltinFilter(); + void theStartupQueryPrefersASavedQueryOverAFilterOfTheSameName(); + void anUnmatchedStartupQueryFallsBackToAFilterNotAStrayQuery(); void theFlaggedFilterIsCalledImportant(); void everyBuiltinFilterIsAKnownGenerator(); void aFilterAcrossAllAccountsIsTheUnscopedQuery(); @@ -532,7 +535,12 @@ void TestConfig::unknownStartupQueryFallsBackAndReports() "[queries]\n" "Inbox=tag:inbox\n"))); - QCOMPARE(config.startupSavedQuery().name, QStringLiteral("Inbox")); + // The fallback is a BUILT-IN filter, not the first saved query. The old + // behaviour looked reasonable while every install carried an Inbox entry + // and became "startup opens a search for one sender" once the duplicated + // entries were removed. + QCOMPARE(config.startupSavedQuery().name, QStringLiteral("Unread")); + QVERIFY(config.startupSavedQuery().isGenerated()); QCOMPARE(config.problems().size(), 1); // The built-in default naming a query the user never created is NOT a @@ -543,7 +551,13 @@ void TestConfig::unknownStartupQueryFallsBackAndReports() "[queries]\n" "Inbox=tag:inbox\n"))); - QCOMPARE(silent.startupSavedQuery().name, QStringLiteral("Inbox")); + // And it now RESOLVES rather than falling through. The default has always + // been "Unread"; before item 93 that named nothing unless the user happened + // to have such an entry, so an install without one silently opened on + // whatever came first in the file. The built-in filter of that name is + // always there. + QCOMPARE(silent.startupSavedQuery().name, QStringLiteral("Unread")); + QVERIFY(silent.startupSavedQuery().isGenerated()); QVERIFY(silent.problems().isEmpty()); } @@ -1043,6 +1057,31 @@ static QString writeTwoAccounts(const QTemporaryDir &dir) "maildir=personal\n")); } +void TestConfig::theStartupQueryCanNameABuiltinFilter() +{ + // The defect: startup_query searched the SAVED queries only. A user whose + // startup view was "Inbox" had that name in queries.json until item 93 + // shipped Inbox as a built-in filter and the duplicate was removed; the + // name then matched nothing and the app started on whatever query happened + // to be first in the file. + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral( + "[general]\n" + "startup_query=Inbox\n" + "\n" + "[account.work]\n" + "maildir=work\n" + "sent=Sent\n"))); + + const SavedQuery startup = config.startupSavedQuery(); + QCOMPARE(startup.name, QStringLiteral("Inbox")); + QVERIFY2(startup.isGenerated(), + "the startup query matched something other than the built-in"); + QCOMPARE(config.resolvedQuery(startup, QString()), + QStringLiteral("tag:inbox")); +} + void TestConfig::theFlaggedFilterIsCalledImportant() { // Item 57 decided this and item 93 contradicted it. The `flag` ACTION has @@ -1531,6 +1570,60 @@ void TestConfig::aStoredGeneratedQueryIsUnpinnedNotDropped() } } +void TestConfig::theStartupQueryPrefersASavedQueryOverAFilterOfTheSameName() +{ + // The user's own entry wins a name collision. They named it deliberately; + // the filter's name is one this application chose for them. + QTemporaryDir dir; + const QString path = writeIni(dir, QStringLiteral( + "[general]\n" + "startup_query=Inbox\n" + "\n" + "[account.work]\n" + "maildir=work\n")); + writeQueries(dir, QStringLiteral(R"({ + "version": 1, + "queries": [ { "name": "Inbox", "query": "tag:inbox and not tag:muted" } ] + })")); + + Config config; + config.load(path); + + const SavedQuery startup = config.startupSavedQuery(); + QCOMPARE(startup.name, QStringLiteral("Inbox")); + QVERIFY2(!startup.isGenerated(), + "the built-in filter shadowed the user's own query of that name"); + QCOMPARE(startup.query, QStringLiteral("tag:inbox and not tag:muted")); +} + +void TestConfig::anUnmatchedStartupQueryFallsBackToAFilterNotAStrayQuery() +{ + // The old fallback was m_savedQueries.first(), which after item 93 removed + // the duplicated entries could be any leftover query: the user's startup + // view became a search for one sender, and an empty queries.json started + // nothing at all. A built-in filter is always present, so the fallback can + // be one. + QTemporaryDir dir; + const QString path = writeIni(dir, QStringLiteral( + "[general]\n" + "startup_query=NoSuchThing\n" + "\n" + "[account.work]\n" + "maildir=work\n")); + writeQueries(dir, QStringLiteral(R"({ + "version": 1, + "queries": [ { "name": "from bu", "query": "from:someone" } ] + })")); + + Config config; + config.load(path); + + const SavedQuery startup = config.startupSavedQuery(); + QVERIFY2(startup.isGenerated(), + "an unmatched startup query fell back to a stray saved query"); + QCOMPARE(startup.name, QStringLiteral("Unread")); +} + void TestConfig::malformedQueriesFileIsAProblemNotACrash() { QTemporaryDir dir; @@ -1567,7 +1660,11 @@ void TestConfig::futureVersionIsRefusedAndReported() /// The fallback stops meaning "alphabetically first" and starts meaning "first /// in the user's own order". "Zebra" first proves it: alphabetical would pick /// "Apple". -void TestConfig::startupQueryFallsBackToDocumentOrder() +/// Was startupQueryFallsBackToDocumentOrder, asserting the first query in the +/// file. That WAS the defect: the first entry is an arbitrary thing to open on, +/// and once item 93's duplicated entries were removed it became a leftover +/// search for one sender. +void TestConfig::startupQueryFallsBackToABuiltinFilter() { QTemporaryDir dir; const QString path = writeIni(dir, QStringLiteral( @@ -1585,7 +1682,10 @@ void TestConfig::startupQueryFallsBackToDocumentOrder() Config config; config.load(path); - QCOMPARE(config.startupSavedQuery().name, QStringLiteral("Zebra")); + const SavedQuery startup = config.startupSavedQuery(); + QVERIFY2(startup.isGenerated(), + "the fallback picked a saved query out of the file"); + QCOMPARE(startup.name, QStringLiteral("Unread")); } /// The parentheses are load-bearing. Without them `path:... and a or b` binds diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 7563365..949b59e 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -169,6 +169,7 @@ private slots: void narrowingAnEmptyQueryBarIsAPlainSearch(); void aMalformedAccountIsReportedWithoutBlockingTheConstructor(); void aWorkerBackedWindowReturnsRealThreads(); + void aGeneratedStartupQueryActuallyRuns(); void everyBuiltinFilterButtonCarriesAnIconAndItsText(); void aQueryInTheMenuCanActuallyBeRun(); void theFourBuiltinFiltersAreOnTheRowInOrder(); @@ -206,7 +207,7 @@ private slots: void aSkippedLocalSyncStillReportsTheOtherRunFinishing(); void aCronSyncRefreshesTheListWithoutAQuery(); void aCronSyncRefreshesOverASelectionWithoutClearingIt(); - void aCronSyncDoesNotRefreshBeforeAnyQueryHasRun(); + void aCronSyncRefreshesTheLastRunQueryNotTheQueryBar(); void aRefreshAddsNewMailAndDropsWhatStoppedMatching(); void theOpenThreadLeavingTheListRaisesTheStaleNotice(); void aThreadStillMatchingRaisesNoStaleNotice(); @@ -2776,25 +2777,43 @@ void TestMainWindow::aCronSyncRefreshesOverASelectionWithoutClearingIt() "one unusable on a cron timer"); } -void TestMainWindow::aCronSyncDoesNotRefreshBeforeAnyQueryHasRun() +void TestMainWindow::aCronSyncRefreshesTheLastRunQueryNotTheQueryBar() { // The query bar holds text the user has typed but not run, and a refresh // must not execute it: that is a search they never asked for. The refresh // re-runs the LAST RUN query, so with none there is nothing to do. + // + // Since item 93 a default Config DOES run a query at startup: the default + // startup name resolves to the built-in Unread filter, where before it + // named nothing and a fresh window had no last-run query at all. + // + // The property under test survives that, and is the one that matters on a + // cron timer: the refresh re-runs the LAST RUN query, never the text + // sitting in the bar. So the bar is given something the user has typed and + // not run, and the assertion is that what the refresh runs is still the + // startup query. const Config config; MainWindow window(config); auto *queryEdit = window.findChild<QLineEdit *>(); QVERIFY(queryEdit); - queryEdit->setText(QStringLiteral("tag:draft-i-was-typing")); - const quint64 before = window.currentGenerationForTesting(); + const QString ranAtStartup = window.lastRunQueryForTesting(); + QVERIFY2(!ranAtStartup.isEmpty(), + "no startup query ran, so a refresh has nothing to re-run and " + "this test cannot distinguish the two sources"); + + queryEdit->setText(QStringLiteral("tag:draft-i-was-typing")); QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged", Q_ARG(SyncMonitor::State, SyncMonitor::State::Idle)); - QCOMPARE(window.currentGenerationForTesting(), before); + QCOMPARE(window.lastRunQueryForTesting(), ranAtStartup); + QVERIFY2(!window.lastRunQueryForTesting().contains( + QStringLiteral("draft-i-was-typing")), + "the refresh executed the text in the query bar, which is a " + "search the user never asked for"); } void TestMainWindow::aRefreshAddsNewMailAndDropsWhatStoppedMatching() @@ -5070,10 +5089,17 @@ namespace { /// A config whose accounts carry the given maildir/sent pairs. An empty `sent` /// writes no key at all, which is the account-without-a-sent-folder case. QString writeSentConfig(const QTemporaryDir &dir, - const QList<QPair<QString, QString>> &accounts) + const QList<QPair<QString, QString>> &accounts, + const QString &startupQuery = QString()) { const QString path = dir.filePath(QStringLiteral("qtmaildir.conf")); QSettings s(path, QSettings::IniFormat); + // [general] keys are read WITHOUT the prefix: QSettings' INI backend treats + // a section literally named [general] as its own fallback section and + // strips it, so setValue("general/startup_query") would write a key nothing + // reads. + if (!startupQuery.isEmpty()) + s.setValue(QStringLiteral("startup_query"), startupQuery); for (const auto &account : accounts) { s.beginGroup(QStringLiteral("account.") + account.first); s.setValue(QStringLiteral("maildir"), account.first); @@ -6356,6 +6382,30 @@ void TestMainWindow::aWorkerBackedWindowReturnsRealThreads() QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); } +void TestMainWindow::aGeneratedStartupQueryActuallyRuns() +{ + // The second half of the startup defect. The constructor read + // startup.query directly, and a generated entry stores no query: its text + // is composed from the accounts at run time. So even once + // startupSavedQuery() could return a built-in filter, the window opened on + // an empty bar and ran nothing. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + Config config; + config.load(writeSentConfig(dir, { + {QStringLiteral("work"), QStringLiteral("Sent")}, + }, QStringLiteral("Sent"))); + + MainWindow window(config); + auto *queryEdit = window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + QVERIFY(queryEdit); + + // Sent, because it is the filter whose query is composed rather than + // constant: a tag filter would pass against code that only handled the + // easy half. + QCOMPARE(queryEdit->text(), config.allSentQuery()); +} + void TestMainWindow::everyBuiltinFilterButtonCarriesAnIconAndItsText() { // The filters are part of the application now, so they carry icons like the |