From de0d3aaa63be2867d127304d55d34259eb6e30d6 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Sat, 15 Aug 2026 10:50:59 +0200 Subject: feat(filters): resolve built-in filters per account Item 93, the Config half. Four built-in filters, Unread, Inbox, Flagged and Sent, as generated entries in kQueryGenerators, which was already a closed set validated on load for Sent alone. resolvedQuery() gains an overload taking an account key, and that is what makes a filter compose with the account dropdown instead of fighting it. A generator is asked for the account's OWN query rather than having its all-accounts query wrapped in a scope: wrapping gives path:"a/**" and (path:"a/Sent/**" or path:"b/Sent/**") which returns the right rows only because path: is hierarchical, so a row-count test passes against it. The tests assert on the query string for that reason, and the mutation putting the wrap back fails two of them. An ordinary saved query ignores the account key and keeps resolving through its own stored account, which is the behaviour item 90 leaves alone. matchNothingQuery() exists because an empty query means "match everything" to notmuch: an account configuring no sent folder would otherwise give a button labelled Sent that shows the entire Maildir. Config gains Q_DECLARE_TR_FUNCTIONS for the filter names, which are button labels. The generator names are not translated: they are matched against the closed set and stored in queries.json, so translating them would make a file written in one locale unreadable in another. No UI yet, and no migration: the query row still builds from pinned saved queries. --- src/config.cpp | 121 +++++++++++++++++++++++++++++++++++++++++++- src/config.h | 46 +++++++++++++++++ tests/test_config.cpp | 136 ++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 302 insertions(+), 1 deletion(-) diff --git a/src/config.cpp b/src/config.cpp index ae26555..0b65053 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -57,7 +57,24 @@ constexpr int kQueriesFormatVersion = 1; /// button that silently finds nothing. Adding one here needs no format bump: /// an older build keeps the row and reports it, which is why an unknown /// generator is a problem rather than a reason to drop the entry. -const QStringList kQueryGenerators = { QStringLiteral("sent") }; +const QStringList kQueryGenerators = { QStringLiteral("unread"), + QStringLiteral("inbox"), + QStringLiteral("flagged"), + QStringLiteral("sent") }; + +/// The tag a generator matches, for the three filters that are a plain tag +/// query. Empty for "sent", which composes from each account's folder instead +/// and is handled separately. +QString generatorTag(const QString &generator) +{ + if (generator == QStringLiteral("unread")) + return QStringLiteral("unread"); + if (generator == QStringLiteral("inbox")) + return QStringLiteral("inbox"); + if (generator == QStringLiteral("flagged")) + return QStringLiteral("flagged"); + return QString(); +} } // namespace @@ -652,6 +669,108 @@ QString Config::resolvedQuery(const SavedQuery &query) const return scope.scopedQuery(query.query); } +bool Config::isKnownGenerator(const QString &generator) +{ + return kQueryGenerators.contains(generator); +} + +QString Config::matchNothingQuery() +{ + // notmuch reads an EMPTY query as "match everything", so a generator with + // nothing to match must say so explicitly. `tag:` and its negation cannot + // both hold, and the tag name is irrelevant: what matters is that this + // parses and matches nothing. A malformed string would not do, since + // notmuch accepts almost anything and matches nothing quietly, which is the + // same result reached by luck rather than by contract. + return QStringLiteral("tag:unread and not tag:unread"); +} + +QList Config::builtinFilters() +{ + // Left to right on the query row. Fixed rather than configurable: item 94 + // removes the mixed row entirely once these are confirmed, so a settings + // surface for the order would be built and deleted inside two items. + QList filters; + for (const QString &generator : kQueryGenerators) + filters.append(builtinFilter(generator)); + return filters; +} + +SavedQuery Config::builtinFilter(const QString &generator) +{ + if (!isKnownGenerator(generator)) + return {}; + + SavedQuery filter; + filter.generated = generator; + + // Translated, because these are the labels on the buttons. The GENERATOR + // name is not: it is stored in queries.json and matched against a closed + // set, so translating it would make a file written in one locale unreadable + // in another. + if (generator == QStringLiteral("unread")) { + filter.name = tr("Unread"); + } else if (generator == QStringLiteral("inbox")) { + filter.name = tr("Inbox"); + } else if (generator == QStringLiteral("flagged")) { + filter.name = tr("Flagged"); + } else if (generator == QStringLiteral("sent")) { + filter.name = tr("Sent"); + // Messages rather than threads, and the only filter that sets this. A + // thread would fold the user's sent message back into the conversation + // it belongs to, which is item 63's finding. + filter.flat = true; + } + + return filter; +} + +QString Config::resolvedQuery(const SavedQuery &query, + const QString &accountKey) const +{ + // An ordinary saved query is a DESTINATION: it states its own scope and + // ignores the dropdown, which is the behaviour item 90 leaves alone. Only a + // generated filter composes. + if (!query.isGenerated()) + return resolvedQuery(query); + + if (!isKnownGenerator(query.generated)) + return QString(); + + if (accountKey.isEmpty()) { + // Across every account, which for the tag filters is the bare query and + // for Sent is the union of the accounts' folders. + if (query.generated == QStringLiteral("sent")) { + const QString all = allSentQuery(); + return all.isEmpty() ? matchNothingQuery() : all; + } + return QStringLiteral("tag:%1").arg(generatorTag(query.generated)); + } + + const Account scope = account(accountKey); + if (!scope.isValid()) + return resolvedQuery(query, QString()); + + if (query.generated == QStringLiteral("sent")) { + // The account's OWN sent query, never the all-accounts one wrapped in + // this account's path. Wrapping gives + // path:"a/**" and (path:"a/Sent/**" or path:"b/Sent/**") + // which returns the right rows because path: is hierarchical, and is + // still wrong: it double-scopes and works by accident of the syntax. + const QString sent = scope.sentQuery(); + // Empty when the account configures no sent folder, which is a real + // case and not a misconfiguration. Returned as-is it would mean "match + // everything", so a button labelled Sent would show the whole Maildir. + return sent.isEmpty() ? matchNothingQuery() : sent; + } + + // A tag filter carries no path of its own, so scoping is exactly what + // scopedQuery() does. Its parentheses are load-bearing: `path:... and a or + // b` binds as `(path:... and a) or b`. + return scope.scopedQuery( + QStringLiteral("tag:%1").arg(generatorTag(query.generated))); +} + SavedQuery Config::startupSavedQuery() const { if (m_savedQueries.isEmpty()) diff --git a/src/config.h b/src/config.h index b9ee9d6..3946b40 100644 --- a/src/config.h +++ b/src/config.h @@ -19,6 +19,7 @@ #pragma once #include +#include #include #include #include @@ -155,6 +156,11 @@ struct SavedQuery /// allow the GUI to index a different tree than the CLI. class Config { + // Not a QObject: this class is a value holder read from every thread. The + // macro gives it tr() for the built-in filters' NAMES, which are the labels + // on the query row's buttons and therefore user-facing. + Q_DECLARE_TR_FUNCTIONS(Config) + public: /// Path used when load() is called with no argument. static QString defaultPath(); @@ -190,6 +196,46 @@ public: /// than a scope built from an empty maildir, which would be path:"/**". QString resolvedQuery(const SavedQuery &query) const; + /// The query as it should be run in one account's scope, or across all of + /// them when `accountKey` is empty. + /// + /// This is what makes a built-in filter COMPOSE with the account dropdown + /// rather than fight it (item 93). A generator is asked for the account's + /// own query, never handed its all-accounts query to wrap: wrapping gives + /// path:"a/**" and (path:"a/Sent/**" or path:"b/Sent/**") + /// which returns the right rows only because path: is hierarchical, and + /// says something other than what is meant. + /// + /// An ordinary saved query ignores `accountKey` and keeps resolving through + /// its OWN stored account, which is the behaviour item 90 leaves alone: a + /// saved query is a destination and states its own scope. + QString resolvedQuery(const SavedQuery &query, + const QString &accountKey) const; + + /// The built-in filters, in the order they appear on the query row. + /// + /// Shipped rather than stored: these are not the user's saved queries and + /// are not in queries.json at all. The row used to be whatever the user had + /// pinned, which is how it drifted (item 93). + static QList builtinFilters(); + + /// One built-in filter by generator name, or a default-constructed + /// SavedQuery when the name is not one. + static SavedQuery builtinFilter(const QString &generator); + + /// Whether `generator` is one this build knows how to resolve. + /// + /// A closed set, so a typo is reported on load rather than producing a + /// button that silently finds nothing. + static bool isKnownGenerator(const QString &generator); + + /// A query that deliberately matches no message. + /// + /// Needed because an EMPTY query means "match everything" to notmuch, so a + /// generator with nothing to match cannot simply return one: Sent under an + /// account that configures no sent folder would show the entire Maildir. + static QString matchNothingQuery(); + /// Empty when unset; the caller disables the Sync button in that case. QString syncCommand() const { return m_syncCommand; } diff --git a/tests/test_config.cpp b/tests/test_config.cpp index 3b094ba..895495d 100644 --- a/tests/test_config.cpp +++ b/tests/test_config.cpp @@ -95,6 +95,12 @@ private slots: void allSentQueryIsEmptyWhenNoAccountHasOne(); void allSentQuerySkipsAccountsWithoutTheKey(); void allSentQueryJoinsEveryConfiguredAccount(); + void everyBuiltinFilterIsAKnownGenerator(); + void aFilterAcrossAllAccountsIsTheUnscopedQuery(); + void aTagFilterScopedToAnAccountCarriesThatAccountsPath(); + void sentScopedToAnAccountIsThatAccountsSentFolderAlone(); + void sentScopedToAnAccountWithNoSentFolderMatchesNothing(); + void aFilterKeepsItsViewMode(); void draftsQueryIsEmptyWithoutTheKey(); void draftsQuerySurvivesABracketedPath(); void allDraftsQuerySkipsAccountsWithoutTheKey(); @@ -1020,6 +1026,136 @@ void TestConfig::allSentQueryJoinsEveryConfiguredAccount() QCOMPARE(all.count(QStringLiteral(" or ")), 1); } +/// Two accounts, one with a sent folder and one without. The second is the +/// case that matters most: folderQuery() returns empty for an unset folder and +/// an empty query means "match everything" to notmuch, so a filter that falls +/// back to it silently shows the whole Maildir. +static QString writeTwoAccounts(const QTemporaryDir &dir) +{ + return writeIni(dir, QStringLiteral( + "[account.work]\n" + "maildir=work\n" + "sent=Sent\n" + "\n" + "[account.personal]\n" + "maildir=personal\n")); +} + +void TestConfig::everyBuiltinFilterIsAKnownGenerator() +{ + // The guard for every case below. A filter whose generator is not in the + // closed set loads with a reported problem and resolves to an empty query, + // which means "match everything": the assertions that follow would then be + // measuring a typo rather than the design. + Config config; + const QList filters = config.builtinFilters(); + + QCOMPARE(filters.size(), 4); + + QStringList names; + for (const SavedQuery &filter : filters) { + QVERIFY2(filter.isGenerated(), + qPrintable(QStringLiteral("filter '%1' stores a query instead " + "of naming a generator") + .arg(filter.name))); + QVERIFY2(Config::isKnownGenerator(filter.generated), + qPrintable(QStringLiteral("filter '%1' names the unknown " + "generator '%2'") + .arg(filter.name, filter.generated))); + names.append(filter.name); + } + + // The order is the row's order, left to right, and is fixed rather than + // configurable: item 94 removes the mixed row entirely, so a settings + // surface for this would be built and deleted inside two items. + QCOMPARE(names, (QStringList{ QStringLiteral("Unread"), + QStringLiteral("Inbox"), + QStringLiteral("Flagged"), + QStringLiteral("Sent") })); +} + +void TestConfig::aFilterAcrossAllAccountsIsTheUnscopedQuery() +{ + QTemporaryDir dir; + Config config; + config.load(writeTwoAccounts(dir)); + + // An empty account key is "All accounts", which is what the dropdown holds + // by default. + const SavedQuery unread = config.builtinFilter(QStringLiteral("unread")); + QCOMPARE(config.resolvedQuery(unread, QString()), + QStringLiteral("tag:unread")); +} + +void TestConfig::aTagFilterScopedToAnAccountCarriesThatAccountsPath() +{ + QTemporaryDir dir; + Config config; + config.load(writeTwoAccounts(dir)); + + // A tag filter has no path of its own, so scoping it is exactly what + // Account::scopedQuery() does and nothing more is needed. + const SavedQuery unread = config.builtinFilter(QStringLiteral("unread")); + QCOMPARE(config.resolvedQuery(unread, QStringLiteral("work")), + QStringLiteral("path:\"work/**\" and (tag:unread)")); +} + +void TestConfig::sentScopedToAnAccountIsThatAccountsSentFolderAlone() +{ + QTemporaryDir dir; + Config config; + config.load(writeTwoAccounts(dir)); + + const SavedQuery sent = config.builtinFilter(QStringLiteral("sent")); + const QString scoped = config.resolvedQuery(sent, QStringLiteral("work")); + + // The whole point of a per-account generator. Wrapping the all-accounts + // query instead would give + // path:"work/**" and (path:"work/Sent/**" or path:"personal/Sent/**") + // which returns the RIGHT ROWS, because path: is hierarchical and the + // personal half cannot match inside work. It is still wrong to build: it + // double-scopes and works by accident of the path syntax rather than by + // saying what is meant. A row-count assertion passes against it, which is + // why this asserts on the string. + QCOMPARE(scoped, QStringLiteral("path:\"work/Sent/**\"")); + QVERIFY2(!scoped.contains(QStringLiteral("personal")), + "another account's sent folder leaked into a scoped Sent filter"); + QCOMPARE(scoped.count(QStringLiteral("path:")), 1); +} + +void TestConfig::sentScopedToAnAccountWithNoSentFolderMatchesNothing() +{ + QTemporaryDir dir; + Config config; + config.load(writeTwoAccounts(dir)); + + // `personal` configures no sent folder, so folderQuery() gives an empty + // string. Returned as-is that is "match everything" to notmuch, so Sent + // under this account would show the entire Maildir: the worst possible + // answer for a button labelled Sent. + const SavedQuery sent = config.builtinFilter(QStringLiteral("sent")); + const QString scoped = + config.resolvedQuery(sent, QStringLiteral("personal")); + + QVERIFY2(!scoped.isEmpty(), + "an account with no sent folder resolved to an empty query, " + "which notmuch reads as 'match everything'"); + QCOMPARE(scoped, Config::matchNothingQuery()); +} + +void TestConfig::aFilterKeepsItsViewMode() +{ + Config config; + + // Sent lists MESSAGES, the other three list threads. Not a detail to + // unify: a thread would fold the user's sent message back into the + // conversation it belongs to, which is item 63's finding. + QVERIFY(config.builtinFilter(QStringLiteral("sent")).flat); + QVERIFY(!config.builtinFilter(QStringLiteral("unread")).flat); + QVERIFY(!config.builtinFilter(QStringLiteral("inbox")).flat); + QVERIFY(!config.builtinFilter(QStringLiteral("flagged")).flat); +} + void TestConfig::draftsQueryIsEmptyWithoutTheKey() { // Optional for the same reason `sent` is, and more often absent: an -- cgit v1.2.3