From acec616bee1c7a336a9dcee8338849b970c3eab1 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Mon, 17 Aug 2026 19:46:08 +0200 Subject: feat(config): read a per-account trash folder --- tests/test_config.cpp | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) (limited to 'tests/test_config.cpp') diff --git a/tests/test_config.cpp b/tests/test_config.cpp index 0dfda86..98743f5 100644 --- a/tests/test_config.cpp +++ b/tests/test_config.cpp @@ -116,6 +116,8 @@ private slots: void draftsQuerySurvivesABracketedPath(); void allDraftsQuerySkipsAccountsWithoutTheKey(); void allDraftsQueryIsIndependentOfSent(); + void anAccountCarriesItsTrashFolder(); + void aBracketedTrashFolderIsQuoted(); }; static QString writeIni(const QTemporaryDir &dir, const QString &body) @@ -953,6 +955,34 @@ void TestConfig::sentQuerySurvivesABracketedPath() "brackets as syntax and the query will match nothing"); } +void TestConfig::anAccountCarriesItsTrashFolder() +{ + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral( + "[account.work]\n" + "maildir=work\n" + "trash=Trash\n"))); + + const Account account = config.account(QStringLiteral("work")); + QCOMPARE(account.trash, QStringLiteral("Trash")); + // Quoted and globbed exactly as sentQuery() does it, so a folder with a + // space or a bracket cannot break the query. + QCOMPARE(account.trashQuery(), QStringLiteral("path:\"work/Trash/**\"")); +} + +void TestConfig::aBracketedTrashFolderIsQuoted() +{ + // The real setup nests a localised trash folder under a bracketed parent. + // The brackets are not notmuch syntax, but the quoting has to survive them. + Account account; + account.maildir = QStringLiteral("provider-a"); + account.trash = QStringLiteral("[Provider]/Cestino"); + + QCOMPARE(account.trashQuery(), + QStringLiteral("path:\"provider-a/[Provider]/Cestino/**\"")); +} + void TestConfig::sentQueryComposesWithScopedQuery() { // A Sent view under one account must not show another account's sent mail. -- cgit v1.2.3 From 9899b9af523ad2aef55300c9132d21a687a0a19d Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Mon, 17 Aug 2026 19:51:27 +0200 Subject: feat(config): warn when an account configures no trash folder The trash key is mandatory: Delete moves a file into it, so an account without one cannot delete at all. Report it as a config problem naming the account and the key, rather than degrading Delete silently, per the existing "a warning the user cannot act on teaches them to ignore warnings" rule (item 83). Several existing test fixtures loaded accounts with no trash key and asserted zero problems/warnings; added trash=Trash to those where it was incidental to what the test actually covers. --- src/config.cpp | 15 +++++++++++ tests/test_config.cpp | 56 +++++++++++++++++++++++++++++++++-------- translations/qtmaildir_it_IT.ts | 10 +++++--- 3 files changed, 67 insertions(+), 14 deletions(-) (limited to 'tests/test_config.cpp') diff --git a/src/config.cpp b/src/config.cpp index d56099a..8294e4b 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -473,6 +473,21 @@ void Config::load(const QString &path) .arg(account.key)); continue; } + + // Mandatory, unlike sent: Delete moves a file into this folder, so an + // account without one cannot delete at all. Reported rather than + // silently disabled, so the user finds out from a warning rather than + // from a Delete that quietly does nothing. The account still loads; + // only Delete is unusable, which does not warrant losing the rest of + // the account's mail. + if (account.trash.isEmpty()) { + addProblem( + tr("Account '%1' has no trash folder configured; add a " + "'trash' key to its section. Delete will not work for " + "this account until it does.") + .arg(account.key)); + } + m_accounts.append(account); } diff --git a/tests/test_config.cpp b/tests/test_config.cpp index 98743f5..10dcf68 100644 --- a/tests/test_config.cpp +++ b/tests/test_config.cpp @@ -118,6 +118,7 @@ private slots: void allDraftsQueryIsIndependentOfSent(); void anAccountCarriesItsTrashFolder(); void aBracketedTrashFolderIsQuoted(); + void anAccountWithoutATrashFolderWarns(); }; static QString writeIni(const QTemporaryDir &dir, const QString &body) @@ -430,7 +431,8 @@ void TestConfig::absentSyncCommandIsNoticeNotProblem() QTemporaryDir dir; const QString path = writeIni(dir, QStringLiteral( "[account.work]\n" - "maildir=work-mail\n")); + "maildir=work-mail\n" + "trash=Trash\n")); Config config; config.load(path); @@ -450,7 +452,8 @@ void TestConfig::brokenSyncCommandIsAProblem() "command=/nonexistent/qtmaildir-test/mailsync.sh\n" "\n" "[account.work]\n" - "maildir=work-mail\n")); + "maildir=work-mail\n" + "trash=Trash\n")); Config config; config.load(path); @@ -485,7 +488,8 @@ void TestConfig::validConfigHasNoProblems() "\n" "[account.work]\n" "maildir=work-mail\n" - "address=user@example.org\n")); + "address=user@example.org\n" + "trash=Trash\n")); Config config; config.load(path); @@ -905,7 +909,8 @@ void TestConfig::sentQueryIsEmptyWithoutTheKey() Config config; config.load(writeIni(dir, QStringLiteral( "[account.provider-c]\n" - "maildir = provider-c\n"))); + "maildir = provider-c\n" + "trash = Trash\n"))); QCOMPARE(config.accounts().size(), 1); QVERIFY(config.accounts().at(0).sentQuery().isEmpty()); @@ -983,6 +988,27 @@ void TestConfig::aBracketedTrashFolderIsQuoted() QStringLiteral("path:\"provider-a/[Provider]/Cestino/**\"")); } +void TestConfig::anAccountWithoutATrashFolderWarns() +{ + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral( + "[account.work]\n" + "maildir=work\n"))); + + // The account still loads. A missing trash folder disables Delete, it does + // not invalidate the account: the user can still read mail. + QVERIFY(config.account(QStringLiteral("work")).isValid()); + + // Names the account and the key, so the warning is actionable. A warning + // the user cannot act on teaches them to ignore warnings, which item 83 + // recorded the hard way. + QVERIFY(!config.problems().isEmpty()); + const QString joined = config.warnings().join(QLatin1Char('\n')); + QVERIFY(joined.contains(QStringLiteral("work"))); + QVERIFY(joined.contains(QStringLiteral("trash"))); +} + void TestConfig::sentQueryComposesWithScopedQuery() { // A Sent view under one account must not show another account's sent mail. @@ -1103,9 +1129,11 @@ void TestConfig::theStartupAccountIsReadAndValidated() "\n" "[account.work]\n" "maildir=work\n" + "trash=Trash\n" "\n" "[account.personal]\n" - "maildir=personal\n"))); + "maildir=personal\n" + "trash=Trash\n"))); QCOMPARE(config.startupAccount(), QStringLiteral("work")); QVERIFY(config.problems().isEmpty()); @@ -1130,7 +1158,8 @@ void TestConfig::theStartupAccountIsReadAndValidated() "startup_account=nosuchaccount\n" "\n" "[account.work]\n" - "maildir=work\n"))); + "maildir=work\n" + "trash=Trash\n"))); QVERIFY2(wrong.startupAccount().isEmpty(), "an unknown startup account was passed through rather than " "falling back to All accounts"); @@ -1155,7 +1184,8 @@ void TestConfig::theStartupAccountTakesTheKeyNotTheSyncChannel() "\n" "[account.provider-work.mailbox]\n" "maildir=provider-work.mailbox\n" - "channel=provider-workmailbox\n"))); + "channel=provider-workmailbox\n" + "trash=Trash\n"))); QCOMPARE(config.accounts().size(), 1); QCOMPARE(config.accounts().constFirst().key, @@ -1177,7 +1207,8 @@ void TestConfig::theStartupAccountTakesTheKeyNotTheSyncChannel() "\n" "[account.provider-work.mailbox]\n" "maildir=provider-work.mailbox\n" - "channel=provider-workmailbox\n"))); + "channel=provider-workmailbox\n" + "trash=Trash\n"))); QVERIFY2(byChannel.startupAccount().isEmpty(), "the sync channel was accepted as an account key"); @@ -1261,7 +1292,8 @@ void TestConfig::theStartupQuerySurvivesATranslatedFilterName() "\n" "[account.work]\n" "maildir=work\n" - "sent=Sent\n"))); + "sent=Sent\n" + "trash=Trash\n"))); QVERIFY2(!config.savedQueries().isEmpty(), "queries.json did not load, so the warning path is unreachable"); @@ -1286,7 +1318,8 @@ void TestConfig::theStartupQuerySurvivesATranslatedFilterName() "\n" "[account.work]\n" "maildir=work\n" - "sent=Sent\n"))); + "sent=Sent\n" + "trash=Trash\n"))); QVERIFY(!byLabel.savedQueries().isEmpty()); QCOMPARE(byLabel.startupSavedQuery().generated, QStringLiteral("inbox")); QVERIFY(byLabel.problems().isEmpty()); @@ -1513,7 +1546,8 @@ void TestConfig::draftsQueryIsEmptyWithoutTheKey() Config config; config.load(writeIni(dir, QStringLiteral( "[account.provider-c]\n" - "maildir = provider-c\n"))); + "maildir = provider-c\n" + "trash = Trash\n"))); QCOMPARE(config.accounts().size(), 1); QVERIFY(config.accounts().at(0).draftsQuery().isEmpty()); diff --git a/translations/qtmaildir_it_IT.ts b/translations/qtmaildir_it_IT.ts index 036ce4b..fd51547 100644 --- a/translations/qtmaildir_it_IT.ts +++ b/translations/qtmaildir_it_IT.ts @@ -43,6 +43,10 @@ [completion] extra_mimetypes: entry '%1' has no mimetype; ignoring it. [completion] extra_mimetypes: la voce '%1' non ha un mimetype; verrà ignorata. + + Account '%1' has no trash folder configured; add a 'trash' key to its section. Delete will not work for this account until it does. + L'account '%1' non ha un cestino configurato; aggiungere una chiave 'trash' alla sua sezione. L'eliminazione non funzionerà per questo account finché non verrà fatto. + Startup account '%1' is not a configured account; starting on all accounts. L'account iniziale '%1' non è un account configurato; si parte da tutti gli account. @@ -280,7 +284,7 @@ Add or remove the important tag - Aggiunge o rimuove l'etichetta importante + Aggiunge o rimuove l'etichetta importante Unmark important @@ -340,7 +344,7 @@ Add or remove the deleted tag on whole threads - Aggiunge o rimuove l'etichetta eliminato su intere conversazioni + Aggiunge o rimuove l'etichetta eliminato su intere conversazioni Undelete thread @@ -364,7 +368,7 @@ Toggle the unread tag on whole threads - Inverte l'etichetta non letto su intere conversazioni + Inverte l'etichetta non letto su intere conversazioni Mark thread read -- cgit v1.2.3 From d5e9ec157946635294001b57ce8353a109ec5051 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Mon, 17 Aug 2026 19:58:51 +0200 Subject: feat(config): add the trash query generator Adds trash as a fifth built-in query filter beside Unread, Inbox, Important and Sent, composing per-account exactly as Sent does: Config::resolvedQuery() asks each account for its own trashQuery() rather than wrapping the all-accounts union, and an account with no trash folder resolves to matchNothingQuery() rather than "match everything". Also gives the Trash button a toolbar icon (user-trash) and a trash key to the mainwindow fixture that asserts every filter button carries one; without it the button is skipped from the row entirely (no account configured a trash folder), and the existing icon test found no button to check. --- src/config.cpp | 27 +++++++++++++++++++- src/config.h | 7 ++++++ src/mainwindow.cpp | 1 + tests/test_config.cpp | 55 +++++++++++++++++++++++++++++++++++++++-- tests/test_mainwindow.cpp | 16 +++++++++--- translations/qtmaildir_it_IT.ts | 4 +++ 6 files changed, 104 insertions(+), 6 deletions(-) (limited to 'tests/test_config.cpp') diff --git a/src/config.cpp b/src/config.cpp index 8294e4b..1799ac6 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -60,7 +60,8 @@ constexpr int kQueriesFormatVersion = 1; const QStringList kQueryGenerators = { QStringLiteral("unread"), QStringLiteral("inbox"), QStringLiteral("flagged"), - QStringLiteral("sent") }; + QStringLiteral("sent"), + QStringLiteral("trash") }; /// 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 @@ -152,6 +153,11 @@ QString Config::allDraftsQuery() const return joinAccountQueries(m_accounts, &Account::draftsQuery); } +QString Config::allTrashQuery() const +{ + return joinAccountQueries(m_accounts, &Account::trashQuery); +} + QString Config::defaultPath() { const QString base = @@ -737,6 +743,8 @@ QString Config::resolvedQuery(const SavedQuery &query) const if (query.isGenerated()) { if (query.generated == QStringLiteral("sent")) return allSentQuery(); + if (query.generated == QStringLiteral("trash")) + return allTrashQuery(); // An unknown generator was reported on load. Empty rather than the // bare stored query, which for a generated entry is empty anyway and // would otherwise run as "match everything". @@ -809,6 +817,11 @@ SavedQuery Config::builtinFilter(const QString &generator) // thread would fold the user's sent message back into the conversation // it belongs to, which is item 63's finding. filter.flat = true; + } else if (generator == QStringLiteral("trash")) { + filter.name = tr("Trash"); + // NOT flat, unlike Sent. A deleted message still belongs to its + // conversation, and folding it back is what Sent had to avoid rather + // than something every folder filter wants. } return filter; @@ -833,6 +846,10 @@ QString Config::resolvedQuery(const SavedQuery &query, const QString all = allSentQuery(); return all.isEmpty() ? matchNothingQuery() : all; } + if (query.generated == QStringLiteral("trash")) { + const QString all = allTrashQuery(); + return all.isEmpty() ? matchNothingQuery() : all; + } return QStringLiteral("tag:%1").arg(generatorTag(query.generated)); } @@ -853,6 +870,14 @@ QString Config::resolvedQuery(const SavedQuery &query, return sent.isEmpty() ? matchNothingQuery() : sent; } + if (query.generated == QStringLiteral("trash")) { + // The account's OWN trash query, for the reason spelled out above the + // sent case: wrapping the all-accounts query in this account's path + // works by accident of path: being hierarchical. + const QString trash = scope.trashQuery(); + return trash.isEmpty() ? matchNothingQuery() : trash; + } + // 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`. diff --git a/src/config.h b/src/config.h index 1feeeda..f60e7cc 100644 --- a/src/config.h +++ b/src/config.h @@ -291,6 +291,13 @@ public: /// open-coded at the call site. QString allSentQuery() const; + /// Matches every configured account's trash, or empty when none has one. + /// + /// Joins only the NON-EMPTY trashQuery() results, for the same reason + /// allSentQuery() does: notmuch accepts a bare "or" without complaint and + /// silently answers a different question. + QString allTrashQuery() const; + /// Matches every configured account's drafts, or empty when none has one. /// /// Joins only the NON-EMPTY draftsQuery() results, for the same reason diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 594535b..67d3ee4 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -1893,6 +1893,7 @@ void MainWindow::buildSavedQueryRow(QWidget *parent, QVBoxLayout *layout) { QStringLiteral("inbox"), QStringLiteral("mail-inbox") }, { QStringLiteral("flagged"), QStringLiteral("starred") }, { QStringLiteral("sent"), QStringLiteral("mail-folder-sent") }, + { QStringLiteral("trash"), QStringLiteral("user-trash") }, }; button->setIcon( QIcon::fromTheme(filterIcons.value(filter.generated))); diff --git a/tests/test_config.cpp b/tests/test_config.cpp index 10dcf68..ea5c363 100644 --- a/tests/test_config.cpp +++ b/tests/test_config.cpp @@ -119,6 +119,8 @@ private slots: void anAccountCarriesItsTrashFolder(); void aBracketedTrashFolderIsQuoted(); void anAccountWithoutATrashFolderWarns(); + void theTrashFilterComposesPerAccount(); + void theTrashFilterMatchesNothingWithoutAFolder(); }; static QString writeIni(const QTemporaryDir &dir, const QString &body) @@ -1009,6 +1011,54 @@ void TestConfig::anAccountWithoutATrashFolderWarns() QVERIFY(joined.contains(QStringLiteral("trash"))); } +void TestConfig::theTrashFilterComposesPerAccount() +{ + // Two accounts, one with a plain folder and one nested under a bracketed + // parent, since the real setup has both shapes. + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral( + "[account.work]\n" + "maildir=work\n" + "trash=Trash\n" + "\n" + "[account.personal]\n" + "maildir=personal\n" + "trash=[Provider]/Cestino\n"))); + + const SavedQuery trash = Config::builtinFilter(QStringLiteral("trash")); + QVERIFY(trash.isGenerated()); + + // All accounts: the union, never a bare path that would match one account. + const QString all = config.resolvedQuery(trash, QString()); + QVERIFY(all.contains(QStringLiteral("path:\"work/Trash/**\""))); + QVERIFY(all.contains( + QStringLiteral("path:\"personal/[Provider]/Cestino/**\""))); + + // One account: that account's OWN query. Asserting on the STRING, not on a + // row count: the all-accounts query wrapped in this account's path returns + // exactly the right rows, because path: is hierarchical, so a count passes + // against the wrong thing. Config::resolvedQuery documents this trap. + const QString scoped = config.resolvedQuery(trash, QStringLiteral("work")); + QCOMPARE(scoped, QStringLiteral("path:\"work/Trash/**\"")); + QVERIFY(!scoped.contains(QStringLiteral("personal"))); +} + +void TestConfig::theTrashFilterMatchesNothingWithoutAFolder() +{ + // An empty query means "match everything" to notmuch, so a filter with + // nothing to match must say so explicitly. A button labelled Trash that + // showed the whole Maildir is the failure this prevents. + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral( + "[account.work]\n" + "maildir=work\n"))); + + const SavedQuery trash = Config::builtinFilter(QStringLiteral("trash")); + QCOMPARE(config.resolvedQuery(trash, QString()), Config::matchNothingQuery()); +} + void TestConfig::sentQueryComposesWithScopedQuery() { // A Sent view under one account must not show another account's sent mail. @@ -1432,7 +1482,7 @@ void TestConfig::everyBuiltinFilterIsAKnownGenerator() Config config; const QList filters = config.builtinFilters(); - QCOMPARE(filters.size(), 4); + QCOMPARE(filters.size(), 5); QStringList names; for (const SavedQuery &filter : filters) { @@ -1453,7 +1503,8 @@ void TestConfig::everyBuiltinFilterIsAKnownGenerator() QCOMPARE(names, (QStringList{ QStringLiteral("Unread"), QStringLiteral("Inbox"), QStringLiteral("Important"), - QStringLiteral("Sent") })); + QStringLiteral("Sent"), + QStringLiteral("Trash") })); } void TestConfig::aFilterAcrossAllAccountsIsTheUnscopedQuery() diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index b188ef4..7316689 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -7922,10 +7922,20 @@ void TestMainWindow::everyBuiltinFilterButtonCarriesAnIconAndItsText() // which is the same argument the Save button records. QTemporaryDir dir; QVERIFY(dir.isValid()); - Config config; - config.load(writeSentConfig(dir, { + const QString path = writeSentConfig(dir, { {QStringLiteral("work"), QStringLiteral("Sent")}, - })); + }); + // A trash key too, or the Trash filter finds nothing and is skipped from + // the row entirely (item 103), leaving no trashButton for this loop to + // find. + { + QSettings s(path, QSettings::IniFormat); + s.beginGroup(QStringLiteral("account.work")); + s.setValue(QStringLiteral("trash"), QStringLiteral("Trash")); + s.endGroup(); + } + Config config; + config.load(path); MainWindow window(config); diff --git a/translations/qtmaildir_it_IT.ts b/translations/qtmaildir_it_IT.ts index fd51547..ef88967 100644 --- a/translations/qtmaildir_it_IT.ts +++ b/translations/qtmaildir_it_IT.ts @@ -95,6 +95,10 @@ Sent Inviati + + Trash + Cestino + HtmlBuilder -- cgit v1.2.3