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