From 5e30d1805656895387ba83865d9635caf2e51618 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Thu, 13 Aug 2026 19:06:41 +0200 Subject: feat(queries): store saved queries in queries.json First half of item 23. The storage moves out of the [queries] INI section into ~/.config/qtmaildir/queries.json; the UI that writes it comes next. The INI could not express order. QSettings reads a section through childKeys(), which sorts alphabetically and never follows the file, so the saved-query buttons could not be arranged and config.cpp carried a comment saying a hand-rolled parser would be needed to change that. queries.json is an ordered array and nothing sorts it on load. That also makes room for the two fields the save dialog needs: pinned, which decides whether a query is a button or a menu entry, and account, which scopes it. account stores the account KEY, not the maildir path, so it does not duplicate config that already lives in the account section and go stale when the user edits it. Config::resolvedQuery() composes through Account::scopedQuery(), whose parentheses are load-bearing: path:... and a or b binds as (path:... and a) or b, so an unparenthesised disjunction escapes its scope and matches every account. A key naming an account that no longer exists resolves to the bare query rather than a scope built from an empty maildir, which would be path:"/**" and match everything. Migration reads [queries] once, when queries.json is absent, marks every entry pinned so the query row does not empty on the first launch after an upgrade, and leaves the INI section untouched. Stripping it would mean rewriting a hand-edited file with QSettings, which drops comments and key order across the whole file. The format follows rules.json in shape only: a version and unknown fields preserved at both levels, so a file written by a later build survives a save from this one. None of its two-implementation machinery is here, because queries have exactly one reader; the version constant says so where a future reader will look. A file whose version this build does not know is refused AND blocks the save, so a newer document is never overwritten with a lossy reading of itself. Twelve tests, each checked against a mutation that puts the corresponding bug back: sorting on load fails three of them, stripping the INI section after migration fails the byte-identical assertion, concatenating the scope without parentheses fails the disjunction test, and dropping unknown-field preservation fails the round trip. The migration test compares the INI file's BYTES rather than re-reading it through QSettings, which would have passed against a rewrite that kept every value while dropping the comments. startup_query still resolves by name, but its fallback now returns the first entry in the user's own order rather than the alphabetically first one. That is user-visible for a config whose startup_query matches nothing. Co-Authored-By: Claude Opus 5 --- src/config.cpp | 185 +++++++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 174 insertions(+), 11 deletions(-) (limited to 'src/config.cpp') diff --git a/src/config.cpp b/src/config.cpp index b1f730c..8cd5e56 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -24,8 +24,14 @@ #include "messageview.h" #include +#include +#include #include +#include +#include +#include #include +#include #include #include @@ -37,6 +43,14 @@ namespace { constexpr int kMinToolbarIconSize = 16; constexpr int kMaxToolbarIconSize = 64; +/// queries.json format version. Bump only for a BREAKING change: an optional +/// field needs no bump, because an older build preserves what it does not +/// understand rather than dropping it. +/// +/// Unlike rules.json this file has ONE implementation, so a bump here is not a +/// two-repo change and no hook stops tagging if it is half-deployed. +constexpr int kQueriesFormatVersion = 1; + } // namespace QString Account::scopedQuery(const QString &query) const @@ -397,17 +411,10 @@ void Config::load(const QString &path) m_accounts.append(account); } - settings.beginGroup(QStringLiteral("queries")); - // QSettings::childKeys() returns keys in alphabetical order, not file - // order, so the saved-query button order in the UI is alphabetical too. - // A hand-rolled parser would be needed to preserve file order; not - // needed in v1. - 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 + loadSavedQueries(path, settings); + + // Checked here rather than where startup_query is read: the saved queries + // are 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() @@ -419,6 +426,162 @@ void Config::load(const QString &path) } } +QString Config::queriesPath(const QString &configPath) +{ + return QFileInfo(configPath).absolutePath() + + QStringLiteral("/queries.json"); +} + +void Config::loadSavedQueries(const QString &configPath, QSettings &settings) +{ + m_queriesPath = queriesPath(configPath); + + QFile file(m_queriesPath); + if (!file.exists()) { + // Migration. Read [queries] once, write the JSON, and leave the INI + // section alone: stripping it would mean rewriting a hand-edited file + // with QSettings, which drops comments and key order across the WHOLE + // file. A few stale lines the user can delete by hand is the cheaper + // loss, and it keeps a downgrade working. + settings.beginGroup(QStringLiteral("queries")); + const QStringList names = settings.childKeys(); + for (const QString &name : names) { + SavedQuery query; + query.name = name; + query.query = settings.value(name).toString(); + // Pinned, because these are buttons today. A migration that left + // them unpinned would empty the query row on the first launch + // after an upgrade, which reads as data loss. + query.pinned = true; + m_savedQueries.append(query); + } + settings.endGroup(); + + // Order is alphabetical here because childKeys() is genuinely all the + // INI knows. The user reorders once and it sticks from then on. + if (!names.isEmpty() && !saveSavedQueries()) { + addProblem(QStringLiteral("Could not write saved queries to %1.") + .arg(m_queriesPath)); + } + return; + } + + if (!file.open(QIODevice::ReadOnly)) { + addProblem(QStringLiteral("Could not read %1: %2.") + .arg(m_queriesPath, file.errorString())); + m_queriesRefused = true; + return; + } + + QJsonParseError error; + const QJsonDocument document = + QJsonDocument::fromJson(file.readAll(), &error); + file.close(); + + if (error.error != QJsonParseError::NoError || !document.isObject()) { + addProblem(QStringLiteral("%1 is not valid JSON: %2.") + .arg(m_queriesPath, error.errorString())); + m_queriesRefused = true; + return; + } + + const QJsonObject root = document.object(); + const int version = + root.value(QStringLiteral("version")).toInt(kQueriesFormatVersion); + if (version != kQueriesFormatVersion) { + // Refused rather than guessed at, and the refusal blocks the save: + // rewriting a newer document with this build's reading of it would + // destroy whatever the newer build stored. + addProblem(QStringLiteral("%1 has format version %2; this build " + "understands %3. Saved queries were not " + "loaded.") + .arg(m_queriesPath) + .arg(version) + .arg(kQueriesFormatVersion)); + m_queriesRefused = true; + return; + } + + for (auto it = root.begin(); it != root.end(); ++it) { + if (it.key() != QStringLiteral("version") + && it.key() != QStringLiteral("queries")) + m_queriesUnknown.insert(it.key(), it.value()); + } + + const QJsonArray array = root.value(QStringLiteral("queries")).toArray(); + for (const QJsonValue &value : array) { + const QJsonObject object = value.toObject(); + SavedQuery query; + query.name = object.value(QStringLiteral("name")).toString(); + query.query = object.value(QStringLiteral("query")).toString(); + query.pinned = object.value(QStringLiteral("pinned")).toBool(false); + query.account = object.value(QStringLiteral("account")).toString(); + + if (query.name.isEmpty()) { + addProblem(QStringLiteral("A saved query in %1 has no name and was " + "skipped.").arg(m_queriesPath)); + continue; + } + + for (auto it = object.begin(); it != object.end(); ++it) { + static const QStringList known = { + QStringLiteral("name"), QStringLiteral("query"), + QStringLiteral("pinned"), QStringLiteral("account") + }; + if (!known.contains(it.key())) + query.unknown.insert(it.key(), it.value()); + } + + m_savedQueries.append(query); + } +} + +bool Config::saveSavedQueries() const +{ + if (m_queriesPath.isEmpty() || m_queriesRefused) + return false; + + QJsonArray array; + for (const SavedQuery &query : m_savedQueries) { + QJsonObject object; + object.insert(QStringLiteral("name"), query.name); + object.insert(QStringLiteral("query"), query.query); + if (query.pinned) + object.insert(QStringLiteral("pinned"), true); + if (!query.account.isEmpty()) + object.insert(QStringLiteral("account"), query.account); + for (auto it = query.unknown.begin(); it != query.unknown.end(); ++it) + object.insert(it.key(), it.value()); + array.append(object); + } + + QJsonObject root = m_queriesUnknown; + root.insert(QStringLiteral("version"), kQueriesFormatVersion); + root.insert(QStringLiteral("queries"), array); + + QDir().mkpath(QFileInfo(m_queriesPath).absolutePath()); + + // QSaveFile writes a temporary and renames on commit, so an interrupted + // write cannot leave a half-written file where the queries used to be. + QSaveFile file(m_queriesPath); + if (!file.open(QIODevice::WriteOnly)) + return false; + file.write(QJsonDocument(root).toJson(QJsonDocument::Indented)); + return file.commit(); +} + +QString Config::resolvedQuery(const SavedQuery &query) const +{ + if (query.account.isEmpty()) + return query.query; + + const Account scope = account(query.account); + if (!scope.isValid()) + return query.query; + + return scope.scopedQuery(query.query); +} + SavedQuery Config::startupSavedQuery() const { if (m_savedQueries.isEmpty()) -- cgit v1.2.3 From 97c81f8cad571ce9ce724ddab8269e911df05a7c Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Thu, 13 Aug 2026 19:52:13 +0200 Subject: feat(queries): make Sent a saved query rather than a fixed button The user asked whether the default queries could be unified with Sent. The answer runs the other way: Sent joins the saved queries rather than the saved queries becoming hardcoded. Inbox, Unread and Important are complete strings that depend on nothing and can never go stale, so generating them would buy nothing and would cost the four things the file just gained: reordering, unpinning, renaming and deleting. Hardcoding them would also make them undeletable, which is a regression for anyone who does not want one of them. Sent is different only in that its query CANNOT be stored: it is composed from every account's `sent` key, so a stored copy goes stale the moment a folder is renamed. That is a property of Sent, not of "default queries". Storing the GENERATOR rather than its output keeps both halves: `"generated": "sent"` still resolves from the accounts at click time, and the entry is an ordinary row that can be reordered, renamed, unpinned or removed. The row now follows one rule instead of carrying one member the user did not own. Two properties had to travel with the entry. The composed query, resolved through Config::resolvedQuery() so what lands in the bar is what actually ran; and FLAT mode, since a sent view lists messages and a threaded one folds every reply back into the conversation the user sent one message into. The sent generator implies flat rather than trusting the file to say so, because a hand-edited row would otherwise produce a threaded sent view. An unknown generator is reported but the row is KEPT: a later build may know it, and dropping it here would delete it from the file on the next save, which is the same data loss the unknown-field handling exists to prevent. A generator whose accounts configure nothing is skipped entirely, exactly as the hardcoded button was hidden rather than offering one that finds nothing. Eight new tests. The four pre-existing Sent tests reach this through migration and were left alone, which is what proves the migrated path still behaves; the new ones cover a STORED file, which is the path every launch after the first takes. Mutations: a generator resolving to nothing fails three, ignoring flat fails two, and not skipping an empty generator fails one. A rename test guards the property the change exists for, since anything keyed on the literal name "Sent" would break it. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 12 +++- README.md | 28 +++++++-- src/config.cpp | 66 +++++++++++++++++++- src/config.h | 21 +++++++ src/mainwindow.cpp | 50 ++++++++------- tests/test_config.cpp | 156 ++++++++++++++++++++++++++++++++++++++++++++++ tests/test_mainwindow.cpp | 111 +++++++++++++++++++++++++++++++++ 7 files changed, 413 insertions(+), 31 deletions(-) (limited to 'src/config.cpp') diff --git a/CHANGELOG.md b/CHANGELOG.md index 428e3e5..b384eda 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,11 @@ point at which they are stable. **order**, a `pinned` flag and an optional account scope. The order in the file is the order the buttons appear in, so rearranging them is a matter of moving lines. +- **Sent is a saved query now**, carrying `"generated": "sent"` instead of a + stored query. It is still composed from your accounts' `sent` keys every time + you click it, so correcting a folder name still updates it with no edit, but + it can now be reordered, renamed, unpinned or deleted like any other entry + rather than being a fixed button you did not own. ### Changed @@ -30,13 +35,18 @@ point at which they are stable. query field is no longer squeezed by a long list of buttons. - Saved-query buttons follow the file's order instead of appearing alphabetically. +- The Sent button is no longer hardcoded beside the saved queries, so the whole + row now follows one rule instead of having one member that behaved + differently from its neighbours. ### Upgrading Saved queries move out of the `[queries]` section of `qtmaildir.conf` and into `~/.config/qtmaildir/queries.json`. **The first launch migrates them for you**: the section is read, the JSON file is written from it, and every entry is -marked pinned so your buttons stay where they were. +marked pinned so your buttons stay where they were. Sent is appended as a +`generated` entry, where its button already sat, provided an account configures +a sent folder. Your config file is left byte-for-byte alone. The old `[queries]` section stays in it, ignored from then on, and can be deleted by hand whenever you like. It is diff --git a/README.md b/README.md index 81ec7d8..749d89d 100644 --- a/README.md +++ b/README.md @@ -223,16 +223,36 @@ queries** menu, which keeps the row usable once you have more than a handful. same as choosing that account in the dropdown; leave it out for a query that spans every account. +**Sent is an entry like any other**, and the one that carries `generated` +instead of `query`: + +```json +{ "name": "Sent", "generated": "sent", "pinned": true } +``` + +A generated query is composed from your accounts every time you click it, +rather than stored. That is why Sent has no `query` of its own: it is built +from every account's `sent` key, so adding an account or correcting a folder +name updates the button with no edit here. A stored copy of the same string +would quietly go on naming the old folder. + +Being an ordinary entry, it can be reordered, renamed, unpinned or deleted like +the rest. Renaming it to `Posta inviata` changes only the label. `sent` is the +only generator today, and it is skipped entirely when no account configures a +sent folder, rather than offering a button that finds nothing. + The name is what the button says, so `Important` and `Flagged` can run the same query and differ only in the label. **Upgrading from 0.17.0 or earlier.** Saved queries used to live in a `[queries]` section of `qtmaildir.conf`. The first launch after upgrading reads that section, writes `queries.json` from it, and marks every entry pinned so -your buttons stay where they were. Your config file is not modified: the old -`[queries]` section is left exactly as it is, ignored from then on, and you can -delete it by hand whenever you like. The reason it is not removed for you is -that rewriting the file would drop your comments and reorder your keys. +your buttons stay where they were. Sent is added as a `generated` entry at the +end, where its button already sat, provided an account configures a sent +folder. Your config file is not modified: the old `[queries]` section is left +exactly as it is, ignored from then on, and you can delete it by hand whenever +you like. The reason it is not removed for you is that rewriting the file would +drop your comments and reorder your keys. One behaviour changes with the move. Buttons used to appear in alphabetical order, because the INI backend returns keys sorted and preserving file order diff --git a/src/config.cpp b/src/config.cpp index 8cd5e56..9edba50 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -51,6 +51,14 @@ constexpr int kMaxToolbarIconSize = 64; /// two-repo change and no hook stops tagging if it is half-deployed. constexpr int kQueriesFormatVersion = 1; +/// Generators a saved query may name in its `generated` field. +/// +/// A closed set, checked on load so a typo is reported rather than producing a +/// 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") }; + } // namespace QString Account::scopedQuery(const QString &query) const @@ -457,9 +465,26 @@ void Config::loadSavedQueries(const QString &configPath, QSettings &settings) } settings.endGroup(); + // Sent was a hardcoded button beside the saved queries and becomes an + // ordinary row here, so it can be reordered, renamed, unpinned or + // removed like any other. It stays GENERATED, so it still follows the + // accounts. Appended last, where the button already sat. + // + // Only when an account actually configures a sent folder: the button + // was hidden entirely otherwise, and migrating a row that always finds + // nothing would be worse than what it replaces. + if (!allSentQuery().isEmpty()) { + SavedQuery sent; + sent.name = QStringLiteral("Sent"); + sent.generated = QStringLiteral("sent"); + sent.pinned = true; + sent.flat = true; + m_savedQueries.append(sent); + } + // Order is alphabetical here because childKeys() is genuinely all the // INI knows. The user reorders once and it sticks from then on. - if (!names.isEmpty() && !saveSavedQueries()) { + if (!m_savedQueries.isEmpty() && !saveSavedQueries()) { addProblem(QStringLiteral("Could not write saved queries to %1.") .arg(m_queriesPath)); } @@ -516,6 +541,26 @@ void Config::loadSavedQueries(const QString &configPath, QSettings &settings) query.query = object.value(QStringLiteral("query")).toString(); query.pinned = object.value(QStringLiteral("pinned")).toBool(false); query.account = object.value(QStringLiteral("account")).toString(); + query.generated = object.value(QStringLiteral("generated")).toString(); + // A generator carries its own view mode, so "sent" is flat whether or + // not the file says so. Storing it as a plain field would let a + // hand-edited or migrated-from-elsewhere row produce a THREADED sent + // view, which folds every reply back into the conversation the user + // sent one message into. The file may still set it for an ordinary + // query. + query.flat = object.value(QStringLiteral("flat")).toBool(false) + || query.generated == QStringLiteral("sent"); + + if (query.isGenerated() + && !kQueryGenerators.contains(query.generated)) { + // Reported but KEPT. A later build may know this generator, and + // dropping the row here would delete it from the file on the next + // save, which is the same data loss the unknown-field handling + // exists to prevent. + addProblem(QStringLiteral("Saved query '%1' uses an unknown " + "generator '%2' and will find nothing.") + .arg(query.name, query.generated)); + } if (query.name.isEmpty()) { addProblem(QStringLiteral("A saved query in %1 has no name and was " @@ -526,7 +571,8 @@ void Config::loadSavedQueries(const QString &configPath, QSettings &settings) for (auto it = object.begin(); it != object.end(); ++it) { static const QStringList known = { QStringLiteral("name"), QStringLiteral("query"), - QStringLiteral("pinned"), QStringLiteral("account") + QStringLiteral("pinned"), QStringLiteral("account"), + QStringLiteral("generated"), QStringLiteral("flat") }; if (!known.contains(it.key())) query.unknown.insert(it.key(), it.value()); @@ -550,6 +596,10 @@ bool Config::saveSavedQueries() const object.insert(QStringLiteral("pinned"), true); if (!query.account.isEmpty()) object.insert(QStringLiteral("account"), query.account); + if (query.isGenerated()) + object.insert(QStringLiteral("generated"), query.generated); + if (query.flat) + object.insert(QStringLiteral("flat"), true); for (auto it = query.unknown.begin(); it != query.unknown.end(); ++it) object.insert(it.key(), it.value()); array.append(object); @@ -572,6 +622,18 @@ bool Config::saveSavedQueries() const QString Config::resolvedQuery(const SavedQuery &query) const { + // Composed from the accounts every time it is asked for, which is the + // point: the answer follows the config rather than a copy of it taken when + // the entry was written. + if (query.isGenerated()) { + if (query.generated == QStringLiteral("sent")) + return allSentQuery(); + // 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". + return QString(); + } + if (query.account.isEmpty()) return query.query; diff --git a/src/config.h b/src/config.h index 4211b0c..b9ee9d6 100644 --- a/src/config.h +++ b/src/config.h @@ -122,6 +122,27 @@ struct SavedQuery /// user edits it. Resolve through Config::resolvedQuery(). QString account; + /// Names a builtin that COMPOSES this query from the accounts at run time, + /// rather than storing it. Empty for an ordinary query. + /// + /// "sent" is the only one today. Its query is built from every account's + /// `sent` key, so adding an account or correcting a folder name is a config + /// edit and nothing else; a stored copy of the same string would go stale + /// silently. That property is why Sent used to be hardcoded beside the + /// saved queries instead of living with them, which left one button on the + /// row that could not be reordered, renamed, unpinned or removed. + /// + /// Storing the GENERATOR rather than its output keeps both: the query stays + /// live, and the entry is an ordinary row the user owns. + QString generated; + + /// Lists messages rather than threads. Set for the sent view, where a + /// thread would fold every reply back into the conversation the user sent + /// one message into. + bool flat = false; + + bool isGenerated() const { return !generated.isEmpty(); } + /// Keys this build does not understand, preserved verbatim so a file /// written by a later version survives a save from this one. QJsonObject unknown; diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index f06b308..62f4751 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -1673,40 +1673,34 @@ void MainWindow::buildSavedQueryRow(QWidget *parent, QVBoxLayout *layout) auto *box = new QHBoxLayout(row); box->setContentsMargins(0, 0, 0, 0); + // Sent is an ordinary row here, not a hardcoded button beside the others. + // It is still GENERATED, so its query is composed from the accounts' `sent` + // keys at click time and correcting a folder name stays a config edit and + // nothing else; what changed is that the entry can now be reordered, + // renamed, unpinned or removed like every other, instead of being the one + // control on the row the user did not own. QList unpinned; for (const SavedQuery &saved : m_config.savedQueries()) { + // A generator whose accounts configure nothing produces a button that + // always finds nothing. Skipped entirely, which is what the hardcoded + // Sent button did and is worth keeping. + if (saved.isGenerated() && m_config.resolvedQuery(saved).isEmpty()) + continue; + if (!saved.pinned) { unpinned.append(saved); continue; } auto *button = new QPushButton(saved.name, row); + // The generated entries keep a stable object name so a test can find + // the sent button without depending on what the user renamed it to. + if (saved.generated == QStringLiteral("sent")) + button->setObjectName(QStringLiteral("sentButton")); connect(button, &QPushButton::clicked, this, [this, saved]() { runSavedQuery(saved); }); box->addWidget(button); } - // Sent sits with the saved queries and is not one: its query is COMPOSED - // from the accounts' `sent` keys at click time, so adding an account or - // correcting a folder name is a config edit and nothing else. A stored - // entry holding the same string would go stale silently, and could not - // narrow to the selected account the way this does through - // runCurrentQuery()'s existing scope wrap. - // - // Hidden entirely when no account configures a sent folder, rather than - // offering a button that always finds nothing. - if (!m_config.allSentQuery().isEmpty()) { - auto *sentButton = new QPushButton(tr("Sent"), row); - sentButton->setObjectName(QStringLiteral("sentButton")); - connect(sentButton, &QPushButton::clicked, this, [this]() { - m_queryEdit->setText(m_config.allSentQuery()); - // Flat for this query only. runCurrentQuery() clears it again for - // anything else, including the same query typed by hand, so the - // flag cannot outlive the button that set it. - runQuery(FlatResult::Yes); - }); - box->addWidget(sentButton); - } - // Everything above is left-aligned; the stretch here pushes what follows // to the right edge. The buttons are the row's content and read as a set, // while the overflow menu is a control over that set, so it sits apart @@ -1751,8 +1745,16 @@ void MainWindow::runSavedQuery(const SavedQuery &saved) if (index >= 0) m_accountBox->setCurrentIndex(index); - m_queryEdit->setText(saved.query); - runCurrentQuery(); + // A generated entry has no stored query: the text is composed from the + // accounts now, so what lands in the bar is what actually ran and the user + // can see and edit it. + m_queryEdit->setText(saved.isGenerated() ? m_config.resolvedQuery(saved) + : saved.query); + + // Flat for this query only. runQuery() sets the mode on EVERY run, so the + // flag cannot outlive the entry that asked for it, including for the same + // query typed by hand afterwards. + runQuery(saved.flat ? FlatResult::Yes : FlatResult::No); } void MainWindow::saveCurrentQuery() diff --git a/tests/test_config.cpp b/tests/test_config.cpp index 8024728..dfe463b 100644 --- a/tests/test_config.cpp +++ b/tests/test_config.cpp @@ -62,6 +62,11 @@ private slots: void futureVersionIsRefusedAndReported(); void startupQueryFallsBackToDocumentOrder(); void scopedSavedQueryParenthesisesADisjunction(); + void aGeneratedQueryResolvesFromTheAccounts(); + void aGeneratedQueryTracksAConfigChange(); + void anUnknownGeneratorResolvesToNothingAndReports(); + void migrationAddsSentWhenAnAccountHasOne(); + void migrationAddsNoSentWithoutTheKey(); void generalSectionKeysAreActuallyRead(); void messageZoomDefaultsAndValidates(); void messageZoomOutOfRangeIsReported(); @@ -1416,5 +1421,156 @@ void TestConfig::scopedSavedQueryParenthesisesADisjunction() QCOMPARE(config.resolvedQuery(orphan), QStringLiteral("tag:inbox")); } +// --------------------------------------------------------------------------- +// Generated saved queries +// --------------------------------------------------------------------------- + +static QString twoAccountsWithSent() +{ + return QStringLiteral( + "[account.work]\n" + "name=Test User\n" + "address=user@example.org\n" + "maildir=work-mail\n" + "sent=Sent\n" + "\n" + "[account.personal]\n" + "name=Test User\n" + "address=me@example.net\n" + "maildir=personal\n" + "sent=[Provider]/Posta inviata\n" + ); +} + +void TestConfig::aGeneratedQueryResolvesFromTheAccounts() +{ + QTemporaryDir dir; + const QString path = writeIni(dir, twoAccountsWithSent()); + writeQueries(dir, QStringLiteral(R"({ + "version": 1, + "queries": [ + { "name": "Sent", "generated": "sent", "pinned": true } + ] + })")); + + Config config; + config.load(path); + + const SavedQuery sent = config.savedQueries().at(0); + QVERIFY(sent.isGenerated()); + // The stored query is empty; the text comes from the accounts. + QVERIFY(sent.query.isEmpty()); + QCOMPARE(config.resolvedQuery(sent), config.allSentQuery()); + QVERIFY(config.resolvedQuery(sent).contains( + QStringLiteral("path:\"work-mail/Sent/**\""))); + // The quotes matter: "[" and "]" are Xapian syntax and an unquoted term + // is parsed rather than matched. + QVERIFY(config.resolvedQuery(sent).contains( + QStringLiteral("path:\"personal/[Provider]/Posta inviata/**\""))); + + // Flat, not threaded: a sent view lists messages, and that property has to + // travel with the entry or it is lost the moment Sent is a stored row. + QVERIFY(sent.flat); +} + +/// The whole reason Sent is generated rather than stored. A stored copy would +/// keep naming an account that has been renamed or a folder that has moved. +void TestConfig::aGeneratedQueryTracksAConfigChange() +{ + QTemporaryDir dir; + const QString queries = QStringLiteral(R"({ + "version": 1, + "queries": [ { "name": "Sent", "generated": "sent" } ] + })"); + + const QString before = writeIni(dir, twoAccountsWithSent()); + writeQueries(dir, queries); + Config first; + first.load(before); + const QString firstResolved = first.resolvedQuery(first.savedQueries().at(0)); + + // The user corrects a folder name. Nothing in queries.json changes. + QTemporaryDir second; + const QString after = writeIni(second, QStringLiteral( + "[account.work]\n" + "name=Test User\n" + "address=user@example.org\n" + "maildir=work-mail\n" + "sent=Sent Items\n" + )); + writeQueries(second, queries); + Config later; + later.load(after); + const QString laterResolved = later.resolvedQuery(later.savedQueries().at(0)); + + QVERIFY(firstResolved != laterResolved); + QVERIFY(laterResolved.contains(QStringLiteral("Sent Items"))); +} + +void TestConfig::anUnknownGeneratorResolvesToNothingAndReports() +{ + QTemporaryDir dir; + const QString path = writeIni(dir, twoAccountsWithSent()); + writeQueries(dir, QStringLiteral(R"({ + "version": 1, + "queries": [ { "name": "Future", "generated": "not_a_generator" } ] + })")); + + Config config; + config.load(path); + + // Kept rather than dropped: a later build may know this generator, and + // silently deleting the row on save would lose it. + QCOMPARE(config.savedQueries().size(), 1); + QVERIFY(config.resolvedQuery(config.savedQueries().at(0)).isEmpty()); + QVERIFY2(!config.problems().isEmpty(), + "an unknown generator must be reported, not silently inert"); +} + +void TestConfig::migrationAddsSentWhenAnAccountHasOne() +{ + QTemporaryDir dir; + const QString path = writeIni(dir, twoAccountsWithSent() + + QStringLiteral( + "\n[queries]\n" + "Inbox=tag:inbox\n" + )); + + Config config; + config.load(path); + + const QList queries = config.savedQueries(); + QCOMPARE(queries.size(), 2); + // Last, where the button already sat: after the saved queries. + QCOMPARE(queries.at(1).name, QStringLiteral("Sent")); + QVERIFY(queries.at(1).isGenerated()); + QVERIFY(queries.at(1).pinned); + QVERIFY(queries.at(1).flat); +} + +/// Today the button is hidden entirely when no account configures a sent +/// folder, rather than offering one that always finds nothing. The migration +/// must not invent a row that would do exactly that. +void TestConfig::migrationAddsNoSentWithoutTheKey() +{ + QTemporaryDir dir; + const QString path = writeIni(dir, QStringLiteral( + "[account.work]\n" + "name=Test User\n" + "address=user@example.org\n" + "maildir=work-mail\n" + "\n" + "[queries]\n" + "Inbox=tag:inbox\n" + )); + + Config config; + config.load(path); + + const QList queries = config.savedQueries(); + QCOMPARE(queries.size(), 1); + QCOMPARE(queries.at(0).name, QStringLiteral("Inbox")); +} + QTEST_MAIN(TestConfig) #include "test_config.moc" diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 42d7d78..883e9a7 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -200,6 +200,9 @@ private slots: void thereIsASaveButtonBesideTheQueryBar(); void theMenuIsRightAlignedAwayFromTheButtons(); void theRowSurvivesWithNothingButUnpinnedQueries(); + void aStoredGeneratedQueryRunsFlatAndComposed(); + void aRenamedSentEntryKeepsWorking(); + void aGeneratedQueryWithNothingToShowIsSkipped(); private: /// Owns the throwaway lock table init() points every test at. A pointer @@ -5664,4 +5667,112 @@ void TestMainWindow::theRowSurvivesWithNothingButUnpinnedQueries() QCOMPARE(menuButton->menu()->actions().size(), 2); } +static QString oneAccountWithSent() +{ + return QStringLiteral( + "[account.work]\n" + "name=Test User\n" + "address=user@example.org\n" + "maildir=work-mail\n" + "sent=Sent\n" + ); +} + +/// The existing Sent tests reach the generated entry through MIGRATION, since +/// their configs have no queries.json. This one starts from a stored file, so +/// it covers the path a user is on from the second launch onwards. +void TestMainWindow::aStoredGeneratedQueryRunsFlatAndComposed() +{ + QTemporaryDir dir; + QVERIFY(dir.isValid()); + Config config; + loadWithQueries(config, dir, QStringLiteral(R"({ + "version": 1, + "queries": [ + { "name": "Sent", "generated": "sent", "pinned": true } + ] + })"), oneAccountWithSent()); + + MainWindow window(config); + auto *button = + window.findChild(QStringLiteral("sentButton")); + QVERIFY(button); + + auto *queryEdit = + window.findChild(QStringLiteral("queryEdit")); + QVERIFY(queryEdit); + auto *model = window.findChild(); + QVERIFY(model); + QVERIFY2(!model->flatMode(), "the model starts threaded"); + + button->click(); + + // Composed from the account, not read from the file: the entry stores no + // query at all. + QCOMPARE(queryEdit->text(), config.allSentQuery()); + QVERIFY(queryEdit->text().contains( + QStringLiteral("path:\"work-mail/Sent/**\""))); + QVERIFY2(model->flatMode(), + "a sent view must be flat, or replies fold back into the thread"); +} + +/// The point of the change: Sent is the user's row now. Renaming it must not +/// break it, which it would if anything keyed on the literal name "Sent". +void TestMainWindow::aRenamedSentEntryKeepsWorking() +{ + QTemporaryDir dir; + QVERIFY(dir.isValid()); + Config config; + loadWithQueries(config, dir, QStringLiteral(R"({ + "version": 1, + "queries": [ + { "name": "Posta inviata", "generated": "sent", "pinned": true } + ] + })"), oneAccountWithSent()); + + MainWindow window(config); + const QStringList labels = savedQueryButtonLabels(window); + QCOMPARE(labels, QStringList{ QStringLiteral("Posta inviata") }); + + auto *button = + window.findChild(QStringLiteral("sentButton")); + QVERIFY2(button, "the generated entry lost its identity when renamed"); + + auto *queryEdit = + window.findChild(QStringLiteral("queryEdit")); + button->click(); + QCOMPARE(queryEdit->text(), config.allSentQuery()); +} + +/// The hardcoded button was hidden entirely when no account configured a sent +/// folder, rather than offering one that always finds nothing. A stored row +/// must behave the same way. +void TestMainWindow::aGeneratedQueryWithNothingToShowIsSkipped() +{ + QTemporaryDir dir; + QVERIFY(dir.isValid()); + Config config; + loadWithQueries(config, dir, QStringLiteral(R"({ + "version": 1, + "queries": [ + { "name": "Inbox", "query": "tag:inbox", "pinned": true }, + { "name": "Sent", "generated": "sent", "pinned": true } + ] + })"), QStringLiteral( + "[account.work]\n" + "name=Test User\n" + "address=user@example.org\n" + "maildir=work-mail\n" + )); + + MainWindow window(config); + + // The guard: the row was built and the other entry did get a button, so a + // missing Sent means it was skipped rather than that nothing was built. + QCOMPARE(savedQueryButtonLabels(window), + QStringList{ QStringLiteral("Inbox") }); + QVERIFY2(!window.findChild(QStringLiteral("sentButton")), + "a generated query with nothing to show must not get a button"); +} + #include "test_mainwindow.moc" -- cgit v1.2.3 From 9be1b13b91188cf44a40c6786a83de034988cdbd Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Thu, 13 Aug 2026 20:14:25 +0200 Subject: fix(queries): stop writing keys that carry no information Saving a generated entry wrote `"query": ""` and `"flat": true` alongside its generator. Both reload correctly, so nothing was broken, but queries.json is meant to be hand-edited and each redundant key is one more thing to read past. A generated entry has no query of its own, and the sent generator already implies flat. Written now only when they say something, which is the rule `pinned` and `account` already followed: `query` is skipped for a generated entry in favour of `generated`, and `flat` is skipped when the generator implies it. Omitting `flat` is only safe because loadSavedQueries() reapplies it from the generator, so the two are coupled: the mutation that stops reapplying it fails this test and one other, in both suites. That is deliberate, since a round-trip test can otherwise pass while quietly writing less than it reads. Co-Authored-By: Claude Opus 5 --- src/config.cpp | 16 ++++++++++++---- tests/test_config.cpp | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 4 deletions(-) (limited to 'src/config.cpp') diff --git a/src/config.cpp b/src/config.cpp index 9edba50..ae26555 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -589,16 +589,24 @@ bool Config::saveSavedQueries() const QJsonArray array; for (const SavedQuery &query : m_savedQueries) { + // Only what carries information. The file is hand-editable, so a key + // that always holds the same value, or one the generator already + // implies, is just something the reader has to skip past. Same reason + // `pinned` and `account` are written only when set. QJsonObject object; object.insert(QStringLiteral("name"), query.name); - object.insert(QStringLiteral("query"), query.query); + if (query.isGenerated()) { + object.insert(QStringLiteral("generated"), query.generated); + } else { + object.insert(QStringLiteral("query"), query.query); + } if (query.pinned) object.insert(QStringLiteral("pinned"), true); if (!query.account.isEmpty()) object.insert(QStringLiteral("account"), query.account); - if (query.isGenerated()) - object.insert(QStringLiteral("generated"), query.generated); - if (query.flat) + // Skipped when the generator already implies it, which loadSavedQueries + // reapplies on the way back in. + if (query.flat && query.generated != QStringLiteral("sent")) object.insert(QStringLiteral("flat"), true); for (auto it = query.unknown.begin(); it != query.unknown.end(); ++it) object.insert(it.key(), it.value()); diff --git a/tests/test_config.cpp b/tests/test_config.cpp index dfe463b..3b094ba 100644 --- a/tests/test_config.cpp +++ b/tests/test_config.cpp @@ -67,6 +67,7 @@ private slots: void anUnknownGeneratorResolvesToNothingAndReports(); void migrationAddsSentWhenAnAccountHasOne(); void migrationAddsNoSentWithoutTheKey(); + void aGeneratedEntryWritesNoRedundantKeys(); void generalSectionKeysAreActuallyRead(); void messageZoomDefaultsAndValidates(); void messageZoomOutOfRangeIsReported(); @@ -1572,5 +1573,54 @@ void TestConfig::migrationAddsNoSentWithoutTheKey() QCOMPARE(queries.at(0).name, QStringLiteral("Inbox")); } +/// The file is meant to be hand-edited, so a key that carries no information +/// is a key the reader has to skip past. `query` says nothing on a generated +/// entry, and `flat` is implied by the sent generator. +void TestConfig::aGeneratedEntryWritesNoRedundantKeys() +{ + QTemporaryDir dir; + const QString path = writeIni(dir, twoAccountsWithSent()); + const QString queriesPath = writeQueries(dir, QStringLiteral(R"({ + "version": 1, + "queries": [ + { "name": "Sent", "generated": "sent", "pinned": true }, + { "name": "Inbox", "query": "tag:inbox", "pinned": true } + ] + })")); + + Config config; + config.load(path); + QVERIFY(config.saveSavedQueries()); + + QFile f(queriesPath); + QVERIFY(f.open(QIODevice::ReadOnly)); + const QJsonArray array = QJsonDocument::fromJson(f.readAll()) + .object() + .value(QStringLiteral("queries")) + .toArray(); + f.close(); + + const QJsonObject sent = array.at(0).toObject(); + QCOMPARE(sent.value(QStringLiteral("generated")).toString(), + QStringLiteral("sent")); + QVERIFY2(!sent.contains(QStringLiteral("query")), + "a generated entry has no query of its own to store"); + QVERIFY2(!sent.contains(QStringLiteral("flat")), + "the sent generator implies flat; storing it says nothing"); + + // The ordinary entry is untouched by any of that. + const QJsonObject inbox = array.at(1).toObject(); + QCOMPARE(inbox.value(QStringLiteral("query")).toString(), + QStringLiteral("tag:inbox")); + + // And it all still reads back the same. + Config reloaded; + reloaded.load(path); + QCOMPARE(reloaded.savedQueries().size(), 2); + QVERIFY(reloaded.savedQueries().at(0).isGenerated()); + QVERIFY2(reloaded.savedQueries().at(0).flat, + "flat must come back from the generator, not from the file"); +} + QTEST_MAIN(TestConfig) #include "test_config.moc" -- cgit v1.2.3