diff options
| -rw-r--r-- | src/config.cpp | 121 | ||||
| -rw-r--r-- | src/config.h | 46 | ||||
| -rw-r--r-- | tests/test_config.cpp | 136 |
3 files changed, 302 insertions, 1 deletions
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<SavedQuery> 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<SavedQuery> 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 <QColor> +#include <QCoreApplication> #include <QJsonObject> #include <QList> #include <QString> @@ -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<SavedQuery> 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<SavedQuery> 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 |