diff options
| -rw-r--r-- | CHANGELOG.md | 54 | ||||
| -rw-r--r-- | README.md | 84 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 68 | ||||
| -rw-r--r-- | docs/superpowers/specs/2026-08-13-saved-queries-design.md | 11 | ||||
| -rw-r--r-- | src/CMakeLists.txt | 1 | ||||
| -rw-r--r-- | src/config.cpp | 255 | ||||
| -rw-r--r-- | src/config.h | 86 | ||||
| -rw-r--r-- | src/keymap.cpp | 5 | ||||
| -rw-r--r-- | src/mainwindow.cpp | 374 | ||||
| -rw-r--r-- | src/mainwindow.h | 60 | ||||
| -rw-r--r-- | src/savequerydialog.cpp | 174 | ||||
| -rw-r--r-- | src/savequerydialog.h | 79 | ||||
| -rw-r--r-- | tests/test_config.cpp | 537 | ||||
| -rw-r--r-- | tests/test_mainwindow.cpp | 710 |
14 files changed, 2431 insertions, 67 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index e867f02..5b14938 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,60 @@ point at which they are stable. ## [Unreleased] +### Added + +- A **Save query** button beside the query bar, also on the Edit menu and bound + to `Ctrl+S`, keeps the query in the bar as a saved query: naming it, + optionally scoping it to one account, and choosing whether it appears as a + button or in a menu. Saved queries no longer have to be added by + hand-editing the config file (item 23). +- Saved queries live in `~/.config/qtmaildir/queries.json`, which carries their + **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. + +- **Right-click a saved query** to edit, pin, unpin or delete it. A saved query + could previously be created and never changed: the only route to adjusting one + field was to retype the whole query under the same name, and there was no way + to delete one at all short of editing the file (item 82). Deleting asks first, + since it writes your config and is not undoable. + +### Changed + +- Saved queries have a **row of their own** beneath the query bar rather than + sharing it, and the unpinned ones sit behind a **More queries** menu, so the + 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. 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 +not removed automatically because rewriting the file would discard your comments +and reorder your keys. + +One behaviour changes with the move. Buttons used to appear in alphabetical +order and now follow the file. If `[general] startup_query` names a query that +does not exist, the fallback is likewise the first query in the file rather than +the alphabetically first one, so a config that relied on that fallback may open +on a different query than before. + ## [0.17.0] - 2026-08-13 A rule is built from rows now instead of typed into four free-text fields: @@ -106,7 +106,7 @@ identity. ; point: once you zoom with Ctrl+wheel or Ctrl+/Ctrl-, that is remembered ; separately and this value no longer applies. ; message_zoom = 1.0 -; Optional. Which [queries] entry to open at startup, by name. Defaults to +; Optional. Which saved query to open at startup, by name. Defaults to ; Unread. Falls back to the first saved query if no query by this name ; exists, and warns if you named one explicitly. ; startup_query = Unread @@ -191,11 +191,6 @@ shopping = #3366cc ; also colours shopping/amazon, shopping/nike, ... shopping/amazon = #ff9900 ; ... unless the exact tag overrides it work = #cc4444 -[queries] -Inbox = tag:inbox -Unread = tag:unread -Important = tag:flagged - [keys] Ctrl+E = archive Ctrl+D = delete @@ -203,14 +198,73 @@ j = next_thread k = prev_thread ``` -Saved-query buttons appear in alphabetical order rather than file order: -QSettings returns keys sorted, and preserving file order would mean -hand-rolling an INI parser. Which query opens at startup is therefore a -separate setting, `[general] startup_query`, rather than "the first one". +### Saved queries + +Saved queries live in `~/.config/qtmaildir/queries.json`, not in the config +file. They are written by the **Save query** action (`Ctrl+S`), which names the +query in the bar, optionally scopes it to one account, and chooses whether it +gets a button: + +```json +{ + "version": 1, + "queries": [ + { "name": "Inbox", "query": "tag:inbox", "pinned": true }, + { "name": "Unread", "query": "tag:unread", "pinned": true }, + { "name": "Billing", "query": "from:billing", "account": "work" } + ] +} +``` + +The order in the file is the order the buttons appear in, so rearranging them +is a matter of moving lines. `pinned` decides between a button and the **More +queries** menu, which keeps the row usable once you have more than a handful. +`account` names an `[account.<key>]` section and scopes the query to it, the +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 } +``` -The button text is the key you write here, so these names are yours to -choose. `Important = tag:flagged` and `Flagged = tag:flagged` run the same -query and differ only in what the button says. +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. + +**Right-click a saved query** (a button, or its entry in the menu) to edit it, +move it between the row and the menu, or delete it. Deleting asks first: it +rewrites this file and there is no undo for it. Editing a generated entry shows +its composed query read-only, since that one is built from your accounts rather +than stored. + +**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. 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 +would have meant hand-rolling a parser. They now follow the file. If +`startup_query` names a query that does not exist, the fallback is likewise the +first query in the file rather than the alphabetically first one. ### Sent mail @@ -253,7 +307,8 @@ behaves like any other query, threads and all. ## The query bar The bar at the top takes a notmuch query and shows the matching threads. -Saved queries from `[queries]` sit beside it as buttons. +Saved queries sit on their own row beneath it: the pinned ones as buttons, the +rest behind **More queries**. `Ctrl+S` keeps the current query as a new one. Completion helps with the syntax rather than replacing it. `Ctrl+Space` opens the popup, and ordinary typing keeps it up to date. Candidates carry a @@ -478,6 +533,7 @@ Defaults, all rebindable through `[keys]`: | `Ctrl+I` | `flag` | Mark important (adds `flagged`) | | `Ctrl+L` | `focus_query` | Focus and select the query bar | | `Ctrl+Space` | `complete_query` | Focus the query bar and offer completions | +| `Ctrl+S` | `save_query` | Keep the current query as a saved query | | `Ctrl+H` | `toggle_html` | Switch the thread between HTML and plain text | | `Ctrl+M` | `load_remote` | Load remote images for the current thread | | `Ctrl+T` | `edit_tags` | Add or remove any tag on the selected threads | diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md index 268dbb4..5e97337 100644 --- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md +++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md @@ -146,6 +146,7 @@ taking that too literally. | 80 | A rule with many conditions squeezes the rule list to one visible row | defect | XS | **done** 2026-08-13 on `rule-builder`, unreleased. Follows item 76 | | 79 | Opening the rules dialog and saving destroys the first rule | defect | XS | **fixed on `rule-builder`** 2026-08-13, unreleased. Shipped in 0.16.0; damaged one real rule, repaired by hand | | 81 | No way to turn a saved query into a tagging rule | workflow | S | open; depends on 23, which builds the dialog, and is excluded from its spec on purpose. Writes to the shared rules file, so it spans this repo and `mailctl` | +| 82 | A saved query cannot be edited, unpinned or deleted from the UI | defect | S | **done** 2026-08-13 on `saved-queries`, unreleased. Right-click offers Edit, Pin/Unpin and Delete | Sizes are rough: XS under an hour, S a sitting, M a session. @@ -574,6 +575,73 @@ failing silently. **Size: S** on top of 23, and not meaningful before it. +## 82. A saved query cannot be edited, unpinned or deleted from the UI + +**Observed (user, 2026-08-13):** hand-testing item 23. The user saved a query, +then asked how to unpin it, and there is no answer that does not involve either +a text editor or retyping the whole query. + +**Cause:** item 23 specified saving and nothing else, and that is exactly what +shipped. `SaveQueryDialog` opens on the contents of the query BAR, not on a +stored entry, so the only route to changing one field of an existing query is to +reconstruct the whole query, name it identically, and let +`MainWindow::saveCurrentQuery()` replace it by name. There is no delete at any +price: nothing in the UI removes an entry from `queries.json`. + +This is a defect rather than a missing enhancement. An action that creates +something the UI cannot then edit or remove is incomplete, and the user hit it +within minutes of the first hand test. + +**Approach.** A context menu on a saved-query button and on each **More +queries** entry, offering Edit, Unpin (or Pin) and Delete. + +- **Edit** opens `SaveQueryDialog` prefilled from the STORED entry rather than + from the query bar. The dialog already carries every field it needs; what it + lacks is a constructor that takes a `SavedQuery`. +- **Unpin** is a one-field write and does not need the dialog at all. +- **Delete** removes the entry and rewrites the file. + +**Constraints.** + +- `saveCurrentQuery()` already merges an existing entry's `unknown` fields over + the dialog's fresh value, and every one of these paths must do the same or a + field written by a later build is dropped by an edit here. +- Renaming through Edit is a rename, not a second entry: match on the name the + dialog was OPENED with, not the one it returns, or renaming silently creates a + duplicate and leaves the original behind. +- Delete is destructive and the file is user config, so it is one of the few + places in this application that wants a confirmation. The no-confirmation rule + in CLAUDE.md is about tag mutations, which are undoable through the undo + stack; this is not on that stack and cannot be undone. +- A test must exercise every route the way item 75's did not: the dialog's + Cancel goes through `done(int)` and never sends a `QCloseEvent`. + +**Size: S**, and it should land before the saved-query work is called done. + +**Done 2026-08-13.** A context menu on each button and each menu entry, with +Edit, Move to menu / Show as a button, and Delete. Every path goes through one +`replaceSavedQuery()`, matched on the name the dialog was OPENED with, so a +rename replaces rather than duplicating, and merging the stored entry's unknown +fields in one place rather than three. + +Two things the approach above did not anticipate. A GENERATED entry has no +query to edit, so the dialog shows its composed query read-only rather than +offering a field that changes nothing, and carries `generated` and `flat` +through an edit rather than letting it decay into a plain entry holding a +snapshot. And the overwrite notice had to learn to ignore the entry being +edited: warning that "Inbox" already exists while editing Inbox is noise. + +It also exposed a defect that predated it. `rebuildSavedQueryRow()` called +`deleteLater()` on the old row, which defers destruction to the event loop, so +the stale row went on answering `findChild()` and every lookup after a rebuild +saw the state from before the edit. It was already reachable from the save path. +Fixed by reparenting the row out immediately. + +The unknown-fields test initially passed against the merge being deleted: it +drove UNPIN, which copies the stored entry and therefore carries `unknown` +along by itself. It now goes through the edit path with a replacement that has +none, which is what the dialog actually returns. + ## Deferred, unsized, or split out Items noted while triaging but not part of the original list. Same numbering diff --git a/docs/superpowers/specs/2026-08-13-saved-queries-design.md b/docs/superpowers/specs/2026-08-13-saved-queries-design.md index 6d0019f..ba6b23e 100644 --- a/docs/superpowers/specs/2026-08-13-saved-queries-design.md +++ b/docs/superpowers/specs/2026-08-13-saved-queries-design.md @@ -160,6 +160,17 @@ Sent stays where it is. It is not a saved query, it is built from would mean generating a per-account path query into stored config, which is the duplication the `account`-key decision just rejected. +## Editing and deleting are NOT here, and that is a defect + +This document specifies creating a saved query and says nothing about changing +or removing one. That gap shipped: the first hand test produced "how do I unpin +a query?", and the honest answer was a text editor. Recorded as **item 82**, +sized S, and it should land before this work is called finished. + +Anything built there must merge the stored entry's `unknown` fields the way +`saveCurrentQuery()` does, and must match a rename on the name the dialog was +opened with rather than the one it returns. + ## What is deliberately not here **Item 81, saving a query as a tagging rule.** A saved query is a view and costs diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 6ae157b..0945f65 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -11,6 +11,7 @@ add_library(qtmaildir_lib STATIC notmuchworker.cpp tagchip.cpp tagcolors.cpp + savequerydialog.cpp tagdialog.cpp tagrules.cpp tagrulesdialog.cpp diff --git a/src/config.cpp b/src/config.cpp index b1f730c..ae26555 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -24,8 +24,14 @@ #include "messageview.h" #include <QDateTime> +#include <QDir> +#include <QFile> #include <QFileInfo> +#include <QJsonArray> +#include <QJsonDocument> +#include <QJsonObject> #include <QLocale> +#include <QSaveFile> #include <QSettings> #include <QStandardPaths> @@ -37,6 +43,22 @@ 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; + +/// 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 @@ -397,17 +419,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 +434,224 @@ 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(); + + // 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 (!m_savedQueries.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(); + 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 " + "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"), + QStringLiteral("generated"), QStringLiteral("flat") + }; + 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) { + // 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); + 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); + // 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()); + 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 +{ + // 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; + + 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()) diff --git a/src/config.h b/src/config.h index ea0b055..b9ee9d6 100644 --- a/src/config.h +++ b/src/config.h @@ -19,12 +19,15 @@ #pragma once #include <QColor> +#include <QJsonObject> #include <QList> #include <QString> #include <QStringList> #include "completionentry.h" +class QSettings; + /// One mail account. notmuch has no concept of accounts; it sees a single flat /// tree. An account is therefore a path prefix within that tree plus an /// identity. @@ -96,10 +99,53 @@ struct Account QString draftsQuery() const; }; +/// A named query, stored in queries.json. +/// +/// Stored as an ORDERED array, which is the whole reason the storage moved out +/// of `[queries]`: QSettings reads a section through childKeys(), which sorts +/// alphabetically and cannot express the order the buttons appear in. struct SavedQuery { QString name; QString query; + + /// Renders as a button in the query row; otherwise it lives in the menu. + /// The two tiers are the point of the flag: a row that shows every saved + /// query does not scale past a handful. + bool pinned = false; + + /// Account KEY, the INI group suffix ("work" from [account.work]), and + /// empty for a query that spans every account. + /// + /// A key rather than a maildir path: the path already lives in the account + /// section, and storing a second copy here would go stale the moment the + /// 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; }; /// Reads ~/.config/qtmaildir/qtmaildir.conf. @@ -117,7 +163,32 @@ public: QList<Account> accounts() const { return m_accounts; } Account account(const QString &key) const; + + /// In document order, which IS the display order. Never sort this. QList<SavedQuery> savedQueries() const { return m_savedQueries; } + void setSavedQueries(const QList<SavedQuery> &queries) + { + m_savedQueries = queries; + } + + /// Path of queries.json, derived from the config file's own directory so a + /// test can point load() at a temporary tree and get both files there. + static QString queriesPath(const QString &configPath); + + /// Writes queries.json. False when the file could not be written, or when + /// the loaded file had a version this build refuses: overwriting a + /// newer-format file with a lossy reading of it is the one outcome worth + /// preventing outright. + bool saveSavedQueries() const; + + /// The query as it should be run: scoped to its account when it names one. + /// + /// 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. An account key naming nothing returns the bare query rather + /// than a scope built from an empty maildir, which would be path:"/**". + QString resolvedQuery(const SavedQuery &query) const; /// Empty when unset; the caller disables the Sync button in that case. QString syncCommand() const { return m_syncCommand; } @@ -254,8 +325,23 @@ private: /// Records a notice: nothing is wrong, a feature is simply not configured. void addNotice(const QString &message); + /// Reads queries.json, or migrates [queries] when it is absent. Called by + /// load(), which has already parsed the INI by then. + void loadSavedQueries(const QString &configPath, QSettings &settings); + QList<Account> m_accounts; QList<SavedQuery> m_savedQueries; + + /// Where saveSavedQueries() writes, remembered from load(). + QString m_queriesPath; + + /// Top-level keys of queries.json this build does not understand. + QJsonObject m_queriesUnknown; + + /// Set when the file was refused for its version. Blocks the save, so a + /// document from a newer build is never overwritten with less than it held. + bool m_queriesRefused = false; + QString m_syncCommand; QString m_syncLog; int m_toolbarIconSize = 24; diff --git a/src/keymap.cpp b/src/keymap.cpp index 71c5355..52b015f 100644 --- a/src/keymap.cpp +++ b/src/keymap.cpp @@ -39,6 +39,7 @@ QStringList KeyMap::knownActions() QStringLiteral("flag"), QStringLiteral("focus_query"), QStringLiteral("complete_query"), + QStringLiteral("save_query"), QStringLiteral("select_all"), QStringLiteral("clear_pane"), QStringLiteral("clear_selection"), @@ -100,6 +101,10 @@ QList<QPair<QString, QString>> KeyMap::defaultBindings() // shells and editors, and it is a named key rather than a symbol, so // no layout has to shift it. { QStringLiteral("Ctrl+Space"), QStringLiteral("complete_query") }, + // The conventional save key, and free here: nothing in this window + // saves a document, so Ctrl+S is unclaimed and means what a user + // expects it to. + { QStringLiteral("Ctrl+S"), QStringLiteral("save_query") }, // The conventional select-all key, and free here: the thread list is a // read-only view, so nothing else in the window wants it. { QStringLiteral("Ctrl+A"), QStringLiteral("select_all") }, diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index e2df6de..14e4202 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -45,6 +45,7 @@ #include <QScrollBar> #include <QTimer> #include <QToolBar> +#include <QToolButton> #include <QVBoxLayout> #include "mailsync.h" @@ -56,6 +57,7 @@ #include "cardlayout.h" #include "tagchip.h" #include "tagdialog.h" +#include "savequerydialog.h" #include "tagrulesdialog.h" #include "threadlistmodel.h" #include "threadlistview.h" @@ -339,6 +341,38 @@ MainWindow::MainWindow(const Config &config, QWidget *parent) buildUi(); registerActions(); + + // After registerActions(), not inside buildUi(): the query bar exists by + // then but the action does not, so wiring this where the field is built + // silently connected nothing and left Save query enabled on an empty + // query. Hung on textChanged rather than textEdited, because the field is + // also set programmatically, by the saved-query buttons and by + // recoverStaleThread(), and the action must track those too. + if (QAction *save = m_actions.value(QStringLiteral("save_query"))) { + // setDefaultAction, not a second connect: the button then takes the + // action's text, icon, tooltip and ENABLED state, so it cannot end up + // offering to save an empty query while the menu entry refuses. + m_saveQueryButton->setDefaultAction(save); + // Icon AND text, unlike the toolbar, which follows the desktop's + // button style. This button sits in a row of text buttons, the saved + // queries, and an icon on its own next to them reads as a different + // kind of control than it is. It is also the one action whose meaning + // an icon alone does not carry: "save" is a shape everyone knows and + // the question is always "save WHAT". + m_saveQueryButton->setToolButtonStyle(Qt::ToolButtonTextBesideIcon); + // Its own text, not the action's: "&Save query..." is menu phrasing, + // and a button rendering the ampersand's accelerator and the ellipsis + // that promises a dialog reads as a menu entry that escaped. The + // action keeps both for the menu it lives in. + m_saveQueryButton->setText(tr("Save")); + + auto updateSaveState = [this, save]() { + save->setEnabled(!m_queryEdit->text().trimmed().isEmpty()); + }; + connect(m_queryEdit, &QLineEdit::textChanged, this, updateSaveState); + updateSaveState(); + } + buildMenus(); // After buildMenus(): QMainWindow::restoreState() matches toolbars by // object name, so they must already exist or their position is dropped. @@ -543,49 +577,27 @@ void MainWindow::buildUi() this, &MainWindow::onExternalSyncStateChanged); m_syncMonitor->start(); - // One row: the account dropdown, the query field, then the saved queries. - // The field is the only stretching item, so it is framed on both sides - // rather than running flush to the window edge, which is what the removed - // Sync button used to terminate. - // - // ponytail: no overflow handling. [queries] is unbounded and enough entries - // would squeeze the field, but three is the real-world case today. Item 23 - // already specifies buttons-plus-menu and is where that belongs. + // The query row proper: account, sort order, the field. The saved queries + // used to share it and now have a row of their own below, which is what + // stops an unbounded list squeezing the field (item 23; the ponytail note + // that stood here predicted exactly this). queryRow->addWidget(m_accountBox); queryRow->addWidget(m_sortOrder); queryRow->addWidget(m_queryEdit, 1); - for (const SavedQuery &saved : m_config.savedQueries()) { - auto *button = new QPushButton(saved.name, central); - connect(button, &QPushButton::clicked, this, [this, saved]() { - m_queryEdit->setText(saved.query); - runCurrentQuery(); - }); - queryRow->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 [queries] - // 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"), central); - 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); - }); - queryRow->addWidget(sentButton); - } + + // Beside the field, where a user looks for it. The menu entry and Ctrl+S + // were not enough on their own: saving is a thing you decide on while + // looking at the results, so it needs to be visible at the query bar + // rather than remembered. Created here and given its action in the + // constructor, since registerActions() has not run yet. + m_saveQueryButton = new QToolButton(central); + m_saveQueryButton->setObjectName(QStringLiteral("saveQueryButton")); + queryRow->addWidget(m_saveQueryButton); + layout->addLayout(queryRow); + buildSavedQueryRow(central, layout); + // Thread list and message pane. m_model = new ThreadListModel(this); m_model->setTagColors(&m_tagColors); @@ -836,6 +848,10 @@ void MainWindow::registerActions() tr("Edit the rules that tag mail as it arrives"), [this]() { showTagRulesDialog(); }); + addAction(QStringLiteral("save_query"), tr("&Save query..."), + tr("Keep the current query as a saved query"), [this]() { + saveCurrentQuery(); + }); addAction(QStringLiteral("toggle_html"), tr("Toggle &HTML"), tr("Switch the thread between HTML and plain text"), [this]() { m_messageView->toggleHtml(); @@ -982,6 +998,7 @@ void MainWindow::buildMenus() editMenu->addSeparator(); editMenu->addAction(m_actions.value(QStringLiteral("focus_query"))); editMenu->addAction(m_actions.value(QStringLiteral("complete_query"))); + editMenu->addAction(m_actions.value(QStringLiteral("save_query"))); editMenu->addSeparator(); editMenu->addAction(m_actions.value(QStringLiteral("select_all"))); @@ -1060,6 +1077,11 @@ void MainWindow::buildMenus() // the selection's tags. { QStringLiteral("tag_rules"), QStringLiteral("configure") }, { QStringLiteral("complete_query"), QStringLiteral("edit-find-replace") }, + // NOT "document-save": that is the floppy/disk shape, which reads as + // "write a file somewhere" and asks the user to guess what is being + // written. Saving a query is bookmarking a search, and bookmark-new is + // the icon set every desktop already uses for "keep this for later". + { QStringLiteral("save_query"), QStringLiteral("bookmark-new") }, { QStringLiteral("select_all"), QStringLiteral("edit-select-all") }, { QStringLiteral("clear_pane"), QStringLiteral("edit-clear") }, { QStringLiteral("clear_selection"), QStringLiteral("edit-clear-all") }, @@ -1644,6 +1666,282 @@ void MainWindow::showWarnings() problems.join(QLatin1Char('\n'))); } +void MainWindow::buildSavedQueryRow(QWidget *parent, QVBoxLayout *layout) +{ + auto *row = new QWidget(parent); + row->setObjectName(QStringLiteral("savedQueryRow")); + 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<SavedQuery> 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); }); + addSavedQueryActions(button, saved); + box->addWidget(button); + } + + // 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 + // from them rather than trailing the last one. + const int contentCount = box->count(); + box->addStretch(1); + + // The overflow menu, and only when something is in it: an empty menu + // button is a control that always does nothing. + if (!unpinned.isEmpty()) { + auto *menuButton = new QPushButton(tr("More queries"), row); + menuButton->setObjectName(QStringLiteral("savedQueryMenuButton")); + auto *menu = new QMenu(menuButton); + for (const SavedQuery &saved : unpinned) { + QAction *action = menu->addAction(saved.name); + connect(action, &QAction::triggered, this, + [this, saved]() { runSavedQuery(saved); }); + // A menu entry has no context menu of its own, so its own submenu + // carries the same three actions; an unpinned query would + // otherwise be the one thing that cannot be edited or deleted. + auto *entryMenu = new QMenu(menu); + addSavedQueryActions(entryMenu, saved); + action->setMenu(entryMenu); + } + menuButton->setMenu(menu); + box->addWidget(menuButton); + } + + layout->addWidget(row); + + // Nothing on either side of the stretch leaves an empty strip of padding, + // so the row goes away rather than sitting there as a gap. Counted before + // the stretch was added, since the stretch is always there: an unpinned + // query with no pinned ones still needs the row for its menu. + if (contentCount == 0 && unpinned.isEmpty()) + row->hide(); +} + +void MainWindow::addSavedQueryActions(QWidget *target, const SavedQuery &saved) +{ + target->setContextMenuPolicy(Qt::ActionsContextMenu); + + auto *edit = new QAction(tr("Edit..."), target); + edit->setObjectName(QStringLiteral("editQuery")); + connect(edit, &QAction::triggered, this, + [this, saved]() { editSavedQuery(saved); }); + target->addAction(edit); + + auto *pin = new QAction(saved.pinned ? tr("Move to menu") + : tr("Show as a button"), + target); + pin->setObjectName(QStringLiteral("pinQuery")); + connect(pin, &QAction::triggered, this, [this, saved]() { + SavedQuery toggled = saved; + toggled.pinned = !saved.pinned; + replaceSavedQuery(saved.name, toggled); + }); + target->addAction(pin); + + auto *separator = new QAction(target); + separator->setSeparator(true); + target->addAction(separator); + + auto *remove = new QAction(tr("Delete"), target); + remove->setObjectName(QStringLiteral("deleteQuery")); + connect(remove, &QAction::triggered, this, + [this, saved]() { deleteSavedQuery(saved); }); + target->addAction(remove); +} + +void MainWindow::editSavedQuery(const SavedQuery &saved) +{ + SaveQueryDialog dialog(m_config, saved, this); + if (dialog.exec() != QDialog::Accepted) + return; + + // Matched on the name the dialog OPENED with. Using the returned name would + // leave the original entry in place and add a second one under the new + // name, which is a duplicate rather than a rename. + replaceSavedQuery(saved.name, dialog.savedQuery()); +} + +void MainWindow::deleteSavedQuery(const SavedQuery &saved) +{ + // One of the few places in this application that confirms. The rule against + // confirmation dialogs covers tag mutations, which are undoable through the + // undo stack; this writes user config, is not on that stack, and cannot be + // taken back. + if (m_confirmDelete) { + const auto answer = QMessageBox::question( + this, tr("Delete saved query"), + tr("Delete the saved query '%1'?").arg(saved.name), + QMessageBox::Yes | QMessageBox::No, QMessageBox::No); + if (answer != QMessageBox::Yes) + return; + } + + replaceSavedQuery(saved.name, SavedQuery()); +} + +void MainWindow::replaceSavedQuery(const QString &originalName, + const SavedQuery &replacement) +{ + QList<SavedQuery> queries = m_config.savedQueries(); + const bool removing = replacement.name.isEmpty(); + + for (int i = 0; i < queries.size(); ++i) { + if (queries.at(i).name.compare(originalName, Qt::CaseInsensitive) != 0) + continue; + + if (removing) { + queries.removeAt(i); + } else { + // The unknown fields belong to the STORED entry: a field written by + // a later build survives an edit made here rather than being + // dropped on the next save. + SavedQuery merged = replacement; + merged.unknown = queries.at(i).unknown; + queries[i] = merged; + } + break; + } + + m_config.setSavedQueries(queries); + if (!m_config.saveSavedQueries()) { + QMessageBox::warning(this, tr("Saved queries"), + tr("Could not write the saved queries file.")); + return; + } + + rebuildSavedQueryRow(); + statusBar()->showMessage( + removing ? tr("Deleted saved query '%1'.").arg(originalName) + : tr("Updated saved query '%1'.").arg(replacement.name), + kStatusMessageMs); +} + +void MainWindow::runSavedQuery(const SavedQuery &saved) +{ + // Through the dropdown, never by pre-scoping the text: runQuery() applies + // the selected account's path itself, so a scope baked in here would be + // applied twice. An unscoped query CLEARS the selection rather than + // inheriting whatever was there, which is the defect the rules preview hit. + const int index = saved.account.isEmpty() + ? m_accountBox->findData(QString()) + : m_accountBox->findData(saved.account); + if (index >= 0) + m_accountBox->setCurrentIndex(index); + + // 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() +{ + const QString query = m_queryEdit->text().trimmed(); + if (query.isEmpty()) + return; + + SaveQueryDialog dialog(m_config, query, + m_accountBox->currentData().toString(), this); + if (dialog.exec() != QDialog::Accepted) + return; + + QList<SavedQuery> queries = m_config.savedQueries(); + const SavedQuery saved = dialog.savedQuery(); + + // Replacing by name keeps the dialog's overwrite offer honest, and keeps + // the entry where it already sat rather than moving it to the end. + bool replaced = false; + for (SavedQuery &existing : queries) { + if (existing.name.compare(saved.name, Qt::CaseInsensitive) == 0) { + // The unknown fields belong to the STORED entry, not to the + // dialog's fresh value, so a field a later build wrote survives + // being edited here. + SavedQuery merged = saved; + merged.unknown = existing.unknown; + existing = merged; + replaced = true; + break; + } + } + if (!replaced) + queries.append(saved); + + m_config.setSavedQueries(queries); + if (!m_config.saveSavedQueries()) { + QMessageBox::warning(this, tr("Save query"), + tr("Could not write the saved queries file.")); + return; + } + + rebuildSavedQueryRow(); + statusBar()->showMessage(tr("Saved query '%1'.").arg(saved.name), + kStatusMessageMs); +} + +void MainWindow::rebuildSavedQueryRow() +{ + // The row is rebuilt wholesale rather than patched: a new query can be + // pinned, unpinned, or replace an existing one, and each moves a different + // widget. Deleting and rebuilding is a handful of buttons and cannot get + // the three cases wrong. + auto *old = findChild<QWidget *>(QStringLiteral("savedQueryRow")); + if (!old) + return; + + auto *layout = qobject_cast<QVBoxLayout *>(centralWidget()->layout()); + if (!layout) + return; + + const int index = layout->indexOf(old); + layout->removeWidget(old); + // Reparented out NOW, not merely scheduled for deletion. deleteLater() + // defers destruction to the event loop, so the old row goes on answering + // findChild() until it runs, and findChild returns the FIRST match: every + // lookup after a rebuild found the stale row and reported the state from + // before the edit. Nothing visible was wrong, which is why this only + // showed up as three tests failing on a row that had in fact been rebuilt. + old->setParent(nullptr); + old->deleteLater(); + + buildSavedQueryRow(centralWidget(), layout); + + // buildSavedQueryRow appends; move it back to where the old row sat, or it + // lands under the thread list. + if (index >= 0) { + auto *item = layout->takeAt(layout->count() - 1); + layout->insertItem(index, item); + } +} + void MainWindow::runQuery(FlatResult flat) { // Set on EVERY run, not only when Yes. This is the line that stops flat diff --git a/src/mainwindow.h b/src/mainwindow.h index 18fbbb4..6e8501a 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -49,6 +49,8 @@ class QPlainTextEdit; class QSplitter; class QProgressBar; class QTimer; +class QToolButton; +class QVBoxLayout; class ThreadListModel; class MessageView; @@ -123,6 +125,25 @@ public: static void setLocksPathForTesting(const QString &path); static QString locksPath(); + /// Suppresses the delete confirmation. + /// + /// A test seam. Deleting a saved query is destructive and not on the undo + /// stack, so it asks first; a test cannot answer a modal dialog without + /// hanging, and driving one through QTest would assert the dialog rather + /// than the deletion. + void setConfirmDeleteForTesting(bool confirm) { m_confirmDelete = confirm; } + + /// Renames or replaces a stored query, as the edit dialog would on accept. + /// + /// A test seam for the rename path specifically: the dialog is modal, and + /// the property worth asserting is that a rename REPLACES rather than + /// duplicating, which is decided after the dialog returns. + void replaceSavedQueryForTesting(const QString &originalName, + const SavedQuery &replacement) + { + replaceSavedQuery(originalName, replacement); + } + /// How many commands are on the undo stack. /// /// A test seam. The undo QAction is always enabled and checks canUndo() @@ -231,6 +252,41 @@ private: /// is what widgets connect to. void runQuery(FlatResult flat); + /// Builds the row of saved-query buttons, the overflow menu and Sent. + /// + /// Its own row since item 23: an unbounded list of buttons sharing the + /// query row squeezed the field, which is the whole reason for the + /// pinned/unpinned split. + void buildSavedQueryRow(QWidget *parent, QVBoxLayout *layout); + + /// Runs a saved query, taking its account scope through the dropdown. + /// + /// Not by pre-scoping the text: runQuery() already wraps the query in the + /// selected account's path, so a scope baked in here would be applied + /// twice. Setting the dropdown also shows the user what scope they are in. + void runSavedQuery(const SavedQuery &saved); + + /// Names the current query and stores it in queries.json. + void saveCurrentQuery(); + + /// Rebuilds the saved-query row in place after the stored list changed. + void rebuildSavedQueryRow(); + + /// Hangs Edit, Pin/Unpin and Delete on a saved query's button or menu + /// entry. The only route to changing a stored query from the UI. + void addSavedQueryActions(QWidget *target, const SavedQuery &saved); + + /// Replaces the entry named `originalName`, writes the file and rebuilds + /// the row. An empty `replacement.name` deletes it instead. + /// + /// Matched on the ORIGINAL name, not the replacement's: a rename otherwise + /// leaves the old entry in place and adds a second one. + void replaceSavedQuery(const QString &originalName, + const SavedQuery &replacement); + + void editSavedQuery(const SavedQuery &saved); + void deleteSavedQuery(const SavedQuery &saved); + private slots: void runCurrentQuery() { runQuery(FlatResult::No); } @@ -604,6 +660,10 @@ private: QUndoStack m_undoStack; QLineEdit *m_queryEdit = nullptr; + /// Save query, beside the field. Driven by the save_query action. + QToolButton *m_saveQueryButton = nullptr; + /// Whether deleting a saved query asks first. Always true outside tests. + bool m_confirmDelete = true; QueryCompleter *m_queryCompleter = nullptr; /// Its own type, not the QTreeView base. The strip painting and the /// expander column are ThreadListView's, and holding the base here only diff --git a/src/savequerydialog.cpp b/src/savequerydialog.cpp new file mode 100644 index 0000000..2d09986 --- /dev/null +++ b/src/savequerydialog.cpp @@ -0,0 +1,174 @@ +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. <danix@danix.xyz> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include "savequerydialog.h" + +#include <QCheckBox> +#include <QComboBox> +#include <QDialogButtonBox> +#include <QFormLayout> +#include <QLabel> +#include <QLineEdit> +#include <QPushButton> +#include <QVBoxLayout> + +bool SaveQueryDialog::namesAnExistingQuery(const Config &config, + const QString &name) +{ + for (const SavedQuery &saved : config.savedQueries()) { + if (saved.name.compare(name.trimmed(), Qt::CaseInsensitive) == 0) + return true; + } + return false; +} + +SaveQueryDialog::SaveQueryDialog(const Config &config, const QString &query, + const QString &accountKey, QWidget *parent) + : QDialog(parent) + , m_config(config) +{ + SavedQuery initial; + initial.query = query; + initial.account = accountKey; + initial.pinned = true; + setWindowTitle(tr("Save query")); + build(initial); +} + +SaveQueryDialog::SaveQueryDialog(const Config &config, + const SavedQuery &existing, QWidget *parent) + : QDialog(parent) + , m_config(config) + , m_originalName(existing.name) + , m_generated(existing.generated) + , m_flat(existing.flat) +{ + setWindowTitle(tr("Edit saved query")); + build(existing); +} + +void SaveQueryDialog::build(const SavedQuery &initial) +{ + auto *layout = new QVBoxLayout(this); + auto *form = new QFormLayout; + + m_name = new QLineEdit(initial.name, this); + m_name->setObjectName(QStringLiteral("saveQueryName")); + m_name->setPlaceholderText(tr("A name for this query")); + form->addRow(tr("Name"), m_name); + + m_query = new QLineEdit(initial.query, this); + m_query->setObjectName(QStringLiteral("saveQueryQuery")); + if (initial.isGenerated()) { + // A generated entry has no stored query: it is composed from the + // accounts every time it runs. Shown, so the user can see what it will + // do, but read-only, since editing it would change nothing. + m_query->setText(m_config.resolvedQuery(initial)); + m_query->setReadOnly(true); + m_query->setToolTip(tr("Built from your accounts and not editable. " + "It follows the sent folder each account " + "configures.")); + form->addRow(tr("Query"), m_query); + } else { + form->addRow(tr("Query"), m_query); + } + + // The scope is stored as an account KEY, so the entries carry the key as + // data exactly as the main window's dropdown does. "All accounts" is the + // empty key, not a missing entry, so an unscoped query is a real choice + // rather than the absence of one. + m_account = new QComboBox(this); + m_account->setObjectName(QStringLiteral("saveQueryAccount")); + m_account->addItem(tr("All accounts"), QString()); + for (const Account &account : m_config.accounts()) + m_account->addItem(account.key, account.key); + const int index = m_account->findData(initial.account); + m_account->setCurrentIndex(index >= 0 ? index : 0); + form->addRow(tr("Account"), m_account); + + m_pinned = new QCheckBox(tr("Show as a button"), this); + m_pinned->setObjectName(QStringLiteral("saveQueryPinned")); + m_pinned->setChecked(initial.pinned); + form->addRow(QString(), m_pinned); + + layout->addLayout(form); + + // Says what is about to happen rather than refusing the name. Overwriting + // a saved query on purpose is a normal edit, and the only thing worth + // preventing is doing it without noticing. + m_notice = new QLabel(this); + m_notice->setObjectName(QStringLiteral("saveQueryNotice")); + m_notice->setWordWrap(true); + layout->addWidget(m_notice); + + auto *buttons = new QDialogButtonBox( + QDialogButtonBox::Save | QDialogButtonBox::Cancel, this); + connect(buttons, &QDialogButtonBox::accepted, this, &QDialog::accept); + connect(buttons, &QDialogButtonBox::rejected, this, &QDialog::reject); + layout->addWidget(buttons); + + m_ok = buttons->button(QDialogButtonBox::Save); + m_ok->setObjectName(QStringLiteral("saveQueryOk")); + + connect(m_name, &QLineEdit::textChanged, + this, &SaveQueryDialog::updateOkState); + connect(m_query, &QLineEdit::textChanged, + this, &SaveQueryDialog::updateOkState); + updateOkState(); + + m_name->setFocus(); +} + +void SaveQueryDialog::updateOkState() +{ + const QString name = m_name->text().trimmed(); + const bool usable = !name.isEmpty() && !m_query->text().trimmed().isEmpty(); + m_ok->setEnabled(usable); + + // Ignores the entry being edited: warning that "Inbox" already exists + // while editing Inbox is noise, and the real case worth catching is a + // rename onto a name something else already holds. + const bool isItsOwnName = + !m_originalName.isEmpty() + && name.compare(m_originalName, Qt::CaseInsensitive) == 0; + if (!name.isEmpty() && !isItsOwnName + && namesAnExistingQuery(m_config, name)) { + m_notice->setText( + tr("A saved query named '%1' already exists and will be " + "replaced.").arg(name)); + } else { + m_notice->clear(); + } +} + +SavedQuery SaveQueryDialog::savedQuery() const +{ + SavedQuery saved; + saved.name = m_name->text().trimmed(); + saved.query = m_query->text().trimmed(); + saved.account = m_account->currentData().toString(); + saved.pinned = m_pinned->isChecked(); + // Carried through rather than re-derived: an edit must not turn a + // generated entry into a plain one holding a snapshot of what it happened + // to resolve to today. + saved.generated = m_generated; + saved.flat = m_flat; + if (saved.isGenerated()) + saved.query.clear(); + return saved; +} diff --git a/src/savequerydialog.h b/src/savequerydialog.h new file mode 100644 index 0000000..d859254 --- /dev/null +++ b/src/savequerydialog.h @@ -0,0 +1,79 @@ +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. <danix@danix.xyz> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#pragma once + +#include <QDialog> + +#include "config.h" + +class QCheckBox; +class QComboBox; +class QLabel; +class QLineEdit; +class QPushButton; + +/// Names a query and keeps it in queries.json. +/// +/// Pure UI: it is handed the config and returns a SavedQuery. It writes +/// nothing, so it can be tested without touching a file, and the caller owns +/// the decision of what to do with the result. +class SaveQueryDialog : public QDialog +{ + Q_OBJECT +public: + /// `query` is the text to store, `accountKey` the scope to preselect, + /// which is the account the user is already looking at. + SaveQueryDialog(const Config &config, const QString &query, + const QString &accountKey, QWidget *parent = nullptr); + + /// Edits an entry that already exists, prefilled from it rather than from + /// the query bar. + SaveQueryDialog(const Config &config, const SavedQuery &existing, + QWidget *parent = nullptr); + + /// The query as edited. Only meaningful after exec() returned Accepted. + SavedQuery savedQuery() const; + + /// Whether `name` already names a stored query. Case-insensitive, matching + /// how startup_query resolves, so "inbox" and "Inbox" cannot both exist and + /// leave the user unable to tell which one a button ran. + static bool namesAnExistingQuery(const Config &config, const QString &name); + +private: + void build(const SavedQuery &initial); + void updateOkState(); + + /// The name the dialog was opened on, empty when creating. The caller + /// matches on this rather than on the returned name, so a rename replaces + /// the entry instead of adding a second one beside it. + QString m_originalName; + + /// Set for a generated entry, whose query is composed from the accounts + /// and cannot be edited here. + QString m_generated; + bool m_flat = false; + + const Config &m_config; + QLineEdit *m_name = nullptr; + QLineEdit *m_query = nullptr; + QComboBox *m_account = nullptr; + QCheckBox *m_pinned = nullptr; + QPushButton *m_ok = nullptr; + QLabel *m_notice = nullptr; +}; diff --git a/tests/test_config.cpp b/tests/test_config.cpp index 95398e6..3b094ba 100644 --- a/tests/test_config.cpp +++ b/tests/test_config.cpp @@ -19,6 +19,9 @@ #include <QtTest> #include <QTemporaryDir> #include <QSettings> +#include <QJsonArray> +#include <QJsonDocument> +#include <QJsonObject> #include "config.h" #include "mailsync.h" @@ -48,6 +51,23 @@ private slots: void startupQueryDefaultsToUnread(); void startupQueryHonoursTheConfiguredName(); void unknownStartupQueryFallsBackAndReports(); + void savedQueriesKeepTheirDocumentOrder(); + void savedQueryFieldsAreRead(); + void unknownFieldsSurviveARoundTrip(); + void savedQueriesRoundTripUnchanged(); + void migrationWritesJsonAndLeavesTheIniByteIdentical(); + void migrationPinsEveryEntry(); + void jsonWinsOnceItExists(); + void malformedQueriesFileIsAProblemNotACrash(); + void futureVersionIsRefusedAndReported(); + void startupQueryFallsBackToDocumentOrder(); + void scopedSavedQueryParenthesisesADisjunction(); + void aGeneratedQueryResolvesFromTheAccounts(); + void aGeneratedQueryTracksAConfigChange(); + void anUnknownGeneratorResolvesToNothingAndReports(); + void migrationAddsSentWhenAnAccountHasOne(); + void migrationAddsNoSentWithoutTheKey(); + void aGeneratedEntryWritesNoRedundantKeys(); void generalSectionKeysAreActuallyRead(); void messageZoomDefaultsAndValidates(); void messageZoomOutOfRangeIsReported(); @@ -139,10 +159,10 @@ void TestConfig::parsesSavedQueries() const QList<SavedQuery> queries = config.savedQueries(); QCOMPARE(queries.size(), 2); - // QSettings::childKeys() returns keys alphabetically, not in file order, - // so the UI button order is alphabetical. This assertion happens to hold - // either way since "Inbox" < "Unread", but the ordering guarantee is - // alphabetical, not "follows the file". + // Reached through migration since item 23: [queries] is read once, when + // queries.json is absent, and childKeys() is alphabetical, so the migrated + // order is too. From then on the JSON's own order wins, which is what + // savedQueriesKeepTheirDocumentOrder() covers. QCOMPARE(queries.at(0).name, QStringLiteral("Inbox")); QCOMPARE(queries.at(0).query, QStringLiteral("tag:inbox")); } @@ -1093,5 +1113,514 @@ void TestConfig::allDraftsQueryIsIndependentOfSent() QVERIFY(!sent.contains(QStringLiteral("provider-b"))); } +// --------------------------------------------------------------------------- +// queries.json (item 23) +// --------------------------------------------------------------------------- + +static QString writeQueries(const QTemporaryDir &dir, const QString &body) +{ + const QString path = dir.filePath(QStringLiteral("queries.json")); + QFile f(path); + f.open(QIODevice::WriteOnly); + f.write(body.toUtf8()); + f.close(); + return path; +} + +/// The property the INI could not provide. "Zebra" is written first and must +/// STAY first: alphabetical order would put it last, so this fails against any +/// implementation that sorts, including the one being replaced. +void TestConfig::savedQueriesKeepTheirDocumentOrder() +{ + QTemporaryDir dir; + const QString path = writeIni(dir, QString()); + writeQueries(dir, QStringLiteral(R"({ + "version": 1, + "queries": [ + { "name": "Zebra", "query": "tag:zebra" }, + { "name": "Apple", "query": "tag:apple" }, + { "name": "Middle", "query": "tag:middle" } + ] + })")); + + Config config; + config.load(path); + + const QList<SavedQuery> queries = config.savedQueries(); + QCOMPARE(queries.size(), 3); + QCOMPARE(queries.at(0).name, QStringLiteral("Zebra")); + QCOMPARE(queries.at(1).name, QStringLiteral("Apple")); + QCOMPARE(queries.at(2).name, QStringLiteral("Middle")); +} + +void TestConfig::savedQueryFieldsAreRead() +{ + QTemporaryDir dir; + const QString path = writeIni(dir, QString()); + writeQueries(dir, QStringLiteral(R"({ + "version": 1, + "queries": [ + { "name": "Inbox", "query": "tag:inbox", "pinned": true }, + { "name": "Billing", "query": "from:billing", "account": "work" } + ] + })")); + + Config config; + config.load(path); + + const QList<SavedQuery> queries = config.savedQueries(); + QCOMPARE(queries.size(), 2); + + QCOMPARE(queries.at(0).name, QStringLiteral("Inbox")); + QCOMPARE(queries.at(0).query, QStringLiteral("tag:inbox")); + QVERIFY(queries.at(0).pinned); + QVERIFY(queries.at(0).account.isEmpty()); + + // pinned defaults to false, which is what puts a query in the menu rather + // than on the row. + QVERIFY(!queries.at(1).pinned); + QCOMPARE(queries.at(1).account, QStringLiteral("work")); +} + +/// A field written by a later build must survive an older build's save, or a +/// downgrade silently strips config the user set. +void TestConfig::unknownFieldsSurviveARoundTrip() +{ + QTemporaryDir dir; + const QString path = writeIni(dir, QString()); + const QString queriesPath = writeQueries(dir, QStringLiteral(R"({ + "version": 1, + "colour_scheme": "solarized", + "queries": [ + { "name": "Inbox", "query": "tag:inbox", "icon": "mail-inbox" } + ] + })")); + + Config config; + config.load(path); + QVERIFY(config.saveSavedQueries()); + + QFile f(queriesPath); + QVERIFY(f.open(QIODevice::ReadOnly)); + const QJsonObject root = QJsonDocument::fromJson(f.readAll()).object(); + f.close(); + + QCOMPARE(root.value(QStringLiteral("colour_scheme")).toString(), + QStringLiteral("solarized")); + const QJsonObject entry = + root.value(QStringLiteral("queries")).toArray().at(0).toObject(); + QCOMPARE(entry.value(QStringLiteral("icon")).toString(), + QStringLiteral("mail-inbox")); +} + +void TestConfig::savedQueriesRoundTripUnchanged() +{ + QTemporaryDir dir; + const QString path = writeIni(dir, QString()); + writeQueries(dir, QStringLiteral(R"({ + "version": 1, + "queries": [ + { "name": "Zebra", "query": "tag:zebra", "pinned": true }, + { "name": "Apple", "query": "from:a@example.org", "account": "work" } + ] + })")); + + Config first; + first.load(path); + QVERIFY(first.saveSavedQueries()); + + Config second; + second.load(path); + + const QList<SavedQuery> a = first.savedQueries(); + const QList<SavedQuery> b = second.savedQueries(); + QCOMPARE(b.size(), a.size()); + for (int i = 0; i < a.size(); ++i) { + QCOMPARE(b.at(i).name, a.at(i).name); + QCOMPARE(b.at(i).query, a.at(i).query); + QCOMPARE(b.at(i).pinned, a.at(i).pinned); + QCOMPARE(b.at(i).account, a.at(i).account); + } +} + +/// Asserts the INI is byte-identical, NOT that it still parses. Re-reading it +/// through QSettings would pass against a rewrite that kept every value while +/// dropping the comments and key order, which is the loss this design exists +/// to avoid. +void TestConfig::migrationWritesJsonAndLeavesTheIniByteIdentical() +{ + QTemporaryDir dir; + const QString ini = QStringLiteral( + "; a comment the user wrote and expects to keep\n" + "[queries]\n" + "Unread=tag:unread\n" + "Inbox=tag:inbox\n" + "\n" + "[general]\n" + "startup_query=Unread\n" + ); + const QString path = writeIni(dir, ini); + + QFile before(path); + QVERIFY(before.open(QIODevice::ReadOnly)); + const QByteArray originalBytes = before.readAll(); + before.close(); + + Config config; + config.load(path); + + const QString queriesPath = dir.filePath(QStringLiteral("queries.json")); + QVERIFY2(QFile::exists(queriesPath), "migration did not write queries.json"); + + QFile after(path); + QVERIFY(after.open(QIODevice::ReadOnly)); + const QByteArray afterBytes = after.readAll(); + after.close(); + + QCOMPARE(afterBytes, originalBytes); + + // The [queries] section is left in place, so an older build still works. + QVERIFY(afterBytes.contains("[queries]")); + QVERIFY(afterBytes.contains("; a comment the user wrote")); +} + +/// A migrated query that was not pinned would vanish from the query row, which +/// on the first launch after an upgrade looks like data loss. +void TestConfig::migrationPinsEveryEntry() +{ + QTemporaryDir dir; + const QString path = writeIni(dir, QStringLiteral( + "[queries]\n" + "Inbox=tag:inbox\n" + "Unread=tag:unread\n" + )); + + Config config; + config.load(path); + + const QList<SavedQuery> queries = config.savedQueries(); + QCOMPARE(queries.size(), 2); + for (const SavedQuery &query : queries) + QVERIFY2(query.pinned, qPrintable( + QStringLiteral("migrated query '%1' is not pinned").arg(query.name))); +} + +/// Once the JSON exists, [queries] is dead. Two sources of truth was the +/// option this design rejected. +void TestConfig::jsonWinsOnceItExists() +{ + QTemporaryDir dir; + const QString path = writeIni(dir, QStringLiteral( + "[queries]\n" + "FromTheIni=tag:ini\n" + )); + writeQueries(dir, QStringLiteral(R"({ + "version": 1, + "queries": [ { "name": "FromTheJson", "query": "tag:json" } ] + })")); + + Config config; + config.load(path); + + const QList<SavedQuery> queries = config.savedQueries(); + QCOMPARE(queries.size(), 1); + QCOMPARE(queries.at(0).name, QStringLiteral("FromTheJson")); +} + +void TestConfig::malformedQueriesFileIsAProblemNotACrash() +{ + QTemporaryDir dir; + const QString path = writeIni(dir, QString()); + writeQueries(dir, QStringLiteral("{ this is not json at all")); + + Config config; + config.load(path); + + QVERIFY(config.savedQueries().isEmpty()); + QVERIFY2(!config.problems().isEmpty(), + "a malformed queries.json must be reported"); +} + +/// Refusing an unknown version is the same contract rules.json keeps: a file +/// from a newer build is not silently reinterpreted, and above all is not +/// overwritten with a lossy reading of itself. +void TestConfig::futureVersionIsRefusedAndReported() +{ + QTemporaryDir dir; + const QString path = writeIni(dir, QString()); + writeQueries(dir, QStringLiteral(R"({ + "version": 99, + "queries": [ { "name": "Inbox", "query": "tag:inbox" } ] + })")); + + Config config; + config.load(path); + + QVERIFY(config.savedQueries().isEmpty()); + QVERIFY(!config.problems().isEmpty()); +} + +/// The fallback stops meaning "alphabetically first" and starts meaning "first +/// in the user's own order". "Zebra" first proves it: alphabetical would pick +/// "Apple". +void TestConfig::startupQueryFallsBackToDocumentOrder() +{ + QTemporaryDir dir; + const QString path = writeIni(dir, QStringLiteral( + "[general]\n" + "startup_query=NoSuchQuery\n" + )); + writeQueries(dir, QStringLiteral(R"({ + "version": 1, + "queries": [ + { "name": "Zebra", "query": "tag:zebra" }, + { "name": "Apple", "query": "tag:apple" } + ] + })")); + + Config config; + config.load(path); + + QCOMPARE(config.startupSavedQuery().name, QStringLiteral("Zebra")); +} + +/// The parentheses are load-bearing. Without them `path:... and a or b` binds +/// as `(path:... and a) or b`, so a query saved with a disjunction escapes its +/// account scope and matches every account. +void TestConfig::scopedSavedQueryParenthesisesADisjunction() +{ + QTemporaryDir dir; + const QString path = writeIni(dir, QStringLiteral( + "[account.work]\n" + "name=Test User\n" + "address=user@example.org\n" + "maildir=work-mail\n" + )); + writeQueries(dir, QStringLiteral(R"({ + "version": 1, + "queries": [ + { "name": "Either", + "query": "from:a@example.org or from:b@example.org", + "account": "work" } + ] + })")); + + Config config; + config.load(path); + + const QString scoped = config.resolvedQuery(config.savedQueries().at(0)); + QCOMPARE(scoped, QStringLiteral( + "path:\"work-mail/**\" and " + "(from:a@example.org or from:b@example.org)")); + + // An account key naming nothing resolves to the bare query rather than a + // scope built from an empty maildir, which would be path:"/**". + SavedQuery orphan; + orphan.name = QStringLiteral("Orphan"); + orphan.query = QStringLiteral("tag:inbox"); + orphan.account = QStringLiteral("deleted-account"); + 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<SavedQuery> 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<SavedQuery> queries = config.savedQueries(); + QCOMPARE(queries.size(), 1); + 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" diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 4c1c5d9..d090016 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -52,6 +52,8 @@ #include <QImage> #include <QPainter> +#include <QToolButton> +#include <QHBoxLayout> #include <QComboBox> #include <QScrollBar> #include "tagchip.h" @@ -188,6 +190,25 @@ private slots: void flatModeDoesNotSurviveTheNextQuery(); void noTwoActionsShareAnIcon(); + void onlyPinnedQueriesBecomeButtons(); + void unpinnedQueriesReachTheMenu(); + void pinnedButtonsFollowTheDocumentOrder(); + void theSavedQueryMenuIsHiddenWhenEveryQueryIsPinned(); + void aScopedSavedQuerySelectsItsAccount(); + void anUnscopedSavedQueryClearsTheAccount(); + void theSaveQueryActionIsDisabledOnAnEmptyQuery(); + void thereIsASaveButtonBesideTheQueryBar(); + void theMenuIsRightAlignedAwayFromTheButtons(); + void theRowSurvivesWithNothingButUnpinnedQueries(); + void aStoredGeneratedQueryRunsFlatAndComposed(); + void aRenamedSentEntryKeepsWorking(); + void aGeneratedQueryWithNothingToShowIsSkipped(); + void aSavedQueryButtonOffersEditUnpinAndDelete(); + void unpinningMovesAQueryToTheMenu(); + void deletingRemovesTheQueryFromTheFile(); + void anEditedQueryKeepsItsUnknownFields(); + void renamingReplacesRatherThanDuplicating(); + private: /// Owns the throwaway lock table init() points every test at. A pointer /// rather than a value because it is rebuilt per test, and QTemporaryDir @@ -5265,4 +5286,693 @@ int main(int argc, char *argv[]) return QTest::qExec(&test, argc, argv); } +// --------------------------------------------------------------------------- +// Saved queries in the query row (item 23) +// --------------------------------------------------------------------------- + +/// Writes a config plus a queries.json beside it, and loads both. +static void loadWithQueries(Config &config, QTemporaryDir &dir, + const QString &queriesJson, + const QString &iniExtra = {}) +{ + QDir().mkpath(dir.filePath(QStringLiteral("qtmaildir"))); + const QString conf = + dir.filePath(QStringLiteral("qtmaildir/qtmaildir.conf")); + QFile ini(conf); + ini.open(QIODevice::WriteOnly | QIODevice::Text); + ini.write(iniExtra.toUtf8()); + ini.close(); + + QFile json(dir.filePath(QStringLiteral("qtmaildir/queries.json"))); + json.open(QIODevice::WriteOnly); + json.write(queriesJson.toUtf8()); + json.close(); + + config.load(conf); +} + +/// Buttons in the saved-query row, by label, in the order they are laid out. +static QStringList savedQueryButtonLabels(MainWindow &window) +{ + QStringList labels; + auto *row = window.findChild<QWidget *>(QStringLiteral("savedQueryRow")); + if (!row) + return labels; + const QList<QPushButton *> buttons = + row->findChildren<QPushButton *>(QString(), Qt::FindDirectChildrenOnly); + for (QPushButton *button : buttons) { + // The menu button is not a saved query and must not be counted as one. + if (button->objectName() != QStringLiteral("savedQueryMenuButton")) + labels.append(button->text()); + } + return labels; +} + +void TestMainWindow::onlyPinnedQueriesBecomeButtons() +{ + QTemporaryDir dir; + QVERIFY(dir.isValid()); + Config config; + loadWithQueries(config, dir, QStringLiteral(R"({ + "version": 1, + "queries": [ + { "name": "Inbox", "query": "tag:inbox", "pinned": true }, + { "name": "Buried", "query": "tag:buried" } + ] + })")); + + MainWindow window(config); + const QStringList labels = savedQueryButtonLabels(window); + + QVERIFY2(labels.contains(QStringLiteral("Inbox")), + "a pinned query must have a button"); + QVERIFY2(!labels.contains(QStringLiteral("Buried")), + "an unpinned query must NOT have a button"); +} + +void TestMainWindow::unpinnedQueriesReachTheMenu() +{ + QTemporaryDir dir; + QVERIFY(dir.isValid()); + Config config; + loadWithQueries(config, dir, QStringLiteral(R"({ + "version": 1, + "queries": [ + { "name": "Inbox", "query": "tag:inbox", "pinned": true }, + { "name": "Buried", "query": "tag:buried" } + ] + })")); + + MainWindow window(config); + auto *menuButton = + window.findChild<QPushButton *>(QStringLiteral("savedQueryMenuButton")); + QVERIFY2(menuButton, "an unpinned query needs a menu to live in"); + QVERIFY(menuButton->menu()); + + QStringList entries; + const QList<QAction *> actions = menuButton->menu()->actions(); + for (QAction *action : actions) + entries.append(action->text()); + + QVERIFY2(entries.contains(QStringLiteral("Buried")), + "the unpinned query is missing from the menu"); + // A pinned query is already a button; listing it twice is the duplicate + // this asserts against. + QVERIFY2(!entries.contains(QStringLiteral("Inbox")), + "a pinned query must not also appear in the menu"); +} + +/// The property the whole storage change was made for. "Zebra" is written +/// first and must stay first; alphabetical order would put it last. +void TestMainWindow::pinnedButtonsFollowTheDocumentOrder() +{ + QTemporaryDir dir; + QVERIFY(dir.isValid()); + Config config; + loadWithQueries(config, dir, QStringLiteral(R"({ + "version": 1, + "queries": [ + { "name": "Zebra", "query": "tag:zebra", "pinned": true }, + { "name": "Apple", "query": "tag:apple", "pinned": true } + ] + })")); + + MainWindow window(config); + const QStringList labels = savedQueryButtonLabels(window); + + QCOMPARE(labels.size(), 2); + QCOMPARE(labels.at(0), QStringLiteral("Zebra")); + QCOMPARE(labels.at(1), QStringLiteral("Apple")); +} + +void TestMainWindow::theSavedQueryMenuIsHiddenWhenEveryQueryIsPinned() +{ + QTemporaryDir dir; + QVERIFY(dir.isValid()); + Config config; + loadWithQueries(config, dir, QStringLiteral(R"({ + "version": 1, + "queries": [ + { "name": "Inbox", "query": "tag:inbox", "pinned": true } + ] + })")); + + MainWindow window(config); + + // The guard the assertion below needs. Asserting only that the menu button + // is absent passed against NO implementation at all, before any of this + // was built, so it has to prove first that the row it is looking in was + // populated and that a button was found. + const QStringList labels = savedQueryButtonLabels(window); + QCOMPARE(labels, QStringList{ QStringLiteral("Inbox") }); + + auto *menuButton = + window.findChild<QPushButton *>(QStringLiteral("savedQueryMenuButton")); + QVERIFY2(!menuButton, + "an empty menu button is a control that always does nothing"); +} + +/// The scope goes through the account dropdown rather than being baked into +/// the query text. runQuery() already scopes by that dropdown, so pre-scoping +/// the text would apply the path twice, and the selection would be invisible. +void TestMainWindow::aScopedSavedQuerySelectsItsAccount() +{ + QTemporaryDir dir; + QVERIFY(dir.isValid()); + Config config; + loadWithQueries(config, dir, QStringLiteral(R"({ + "version": 1, + "queries": [ + { "name": "Billing", "query": "from:billing", + "account": "work", "pinned": true } + ] + })"), QStringLiteral( + "[account.work]\n" + "name=Test User\n" + "address=user@example.org\n" + "maildir=work-mail\n" + "\n" + "[account.personal]\n" + "name=Test User\n" + "address=me@example.net\n" + "maildir=personal\n" + )); + + MainWindow window(config); + auto *accountBox = + window.findChild<QComboBox *>(QStringLiteral("accountBox")); + auto *queryEdit = + window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + QVERIFY(accountBox); + QVERIFY(queryEdit); + + // Start somewhere else, so a passing result cannot be the default. + accountBox->setCurrentIndex(accountBox->findData( + QStringLiteral("personal"))); + QCOMPARE(accountBox->currentData().toString(), QStringLiteral("personal")); + + auto *row = window.findChild<QWidget *>(QStringLiteral("savedQueryRow")); + QVERIFY(row); + auto *button = row->findChild<QPushButton *>(); + QVERIFY(button); + button->click(); + + QCOMPARE(accountBox->currentData().toString(), QStringLiteral("work")); + // The text is the bare query. The path scope is applied once, by + // runQuery(), from the dropdown this just set. + QCOMPARE(queryEdit->text(), QStringLiteral("from:billing")); + QVERIFY2(!queryEdit->text().contains(QStringLiteral("path:")), + "the scope must not be baked into the query text"); +} + +/// A query with no account must CLEAR the dropdown, not inherit whatever the +/// last one left there. Confirmed against the same defect in the rules +/// preview, where an already-selected account survived the click. +void TestMainWindow::anUnscopedSavedQueryClearsTheAccount() +{ + QTemporaryDir dir; + QVERIFY(dir.isValid()); + Config config; + loadWithQueries(config, dir, QStringLiteral(R"({ + "version": 1, + "queries": [ + { "name": "Everywhere", "query": "tag:inbox", "pinned": true } + ] + })"), QStringLiteral( + "[account.work]\n" + "name=Test User\n" + "address=user@example.org\n" + "maildir=work-mail\n" + )); + + MainWindow window(config); + auto *accountBox = + window.findChild<QComboBox *>(QStringLiteral("accountBox")); + QVERIFY(accountBox); + + accountBox->setCurrentIndex(accountBox->findData(QStringLiteral("work"))); + QCOMPARE(accountBox->currentData().toString(), QStringLiteral("work")); + + auto *row = window.findChild<QWidget *>(QStringLiteral("savedQueryRow")); + QVERIFY(row); + auto *button = row->findChild<QPushButton *>(); + QVERIFY(button); + button->click(); + + QVERIFY2(accountBox->currentData().toString().isEmpty(), + "an unscoped saved query must clear the account selection"); +} + +void TestMainWindow::theSaveQueryActionIsDisabledOnAnEmptyQuery() +{ + QTemporaryDir dir; + QVERIFY(dir.isValid()); + Config config; + loadWithQueries(config, dir, QStringLiteral(R"({ + "version": 1, "queries": [] + })")); + + MainWindow window(config); + auto *save = window.findChild<QAction *>(QStringLiteral("save_query")); + QVERIFY2(save, "there is no way to save a query"); + + auto *queryEdit = + window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + QVERIFY(queryEdit); + + queryEdit->clear(); + QVERIFY2(!save->isEnabled(), + "saving an empty query would store a query that matches nothing"); + + queryEdit->setText(QStringLiteral("tag:inbox")); + QVERIFY2(save->isEnabled(), "a real query must be savable"); + + // Whitespace is not a query. setText does not drive a completer, but it + // does emit textChanged, which is what the enabling is hung on. + queryEdit->setText(QStringLiteral(" ")); + QVERIFY(!save->isEnabled()); +} + +/// A menu entry and a shortcut are not a button. The spec asks for one beside +/// the query bar, and the user went looking for it there and did not find it. +void TestMainWindow::thereIsASaveButtonBesideTheQueryBar() +{ + QTemporaryDir dir; + QVERIFY(dir.isValid()); + Config config; + loadWithQueries(config, dir, QStringLiteral(R"({ + "version": 1, "queries": [] + })")); + + MainWindow window(config); + auto *button = + window.findChild<QAbstractButton *>(QStringLiteral("saveQueryButton")); + QVERIFY2(button, "no Save query button beside the query bar"); + + // Icon AND text. An icon alone was the first version and read as + // ambiguous: "save" is a familiar shape whose meaning is always "save + // what?". + auto *toolButton = qobject_cast<QToolButton *>(button); + QVERIFY(toolButton); + QCOMPARE(toolButton->toolButtonStyle(), Qt::ToolButtonTextBesideIcon); + QVERIFY2(!button->icon().isNull(), "the button has no icon"); + QVERIFY2(!button->text().isEmpty(), "the button has no text"); + + // Button phrasing, not the menu's: no accelerator ampersand, and no + // ellipsis. setDefaultAction copies the action's text, so this asserts the + // override survived it. + QVERIFY2(!button->text().contains(QLatin1Char('&')), + "the menu accelerator leaked onto the button"); + QVERIFY2(!button->text().contains(QStringLiteral("...")), + "the menu's ellipsis leaked onto the button"); + + auto *queryEdit = + window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + QVERIFY(queryEdit); + + // In the query row itself, not somewhere else in the window that a + // findChild would also reach. + QCOMPARE(button->parentWidget(), queryEdit->parentWidget()); + + // Follows the action, so it cannot offer to save an empty query while the + // menu entry correctly refuses. + queryEdit->clear(); + QVERIFY2(!button->isEnabled(), + "the button must follow the action's enabled state"); + queryEdit->setText(QStringLiteral("tag:inbox")); + QVERIFY(button->isEnabled()); +} + +/// Right-aligned, meaning a stretch sits between the buttons and the menu. +/// Asserted on the layout rather than on x coordinates: the offscreen platform +/// lays out widgets, but a geometry assertion here would also pass for a row +/// that simply ran out of width. +void TestMainWindow::theMenuIsRightAlignedAwayFromTheButtons() +{ + QTemporaryDir dir; + QVERIFY(dir.isValid()); + Config config; + loadWithQueries(config, dir, QStringLiteral(R"({ + "version": 1, + "queries": [ + { "name": "Inbox", "query": "tag:inbox", "pinned": true }, + { "name": "Buried", "query": "tag:buried" } + ] + })")); + + MainWindow window(config); + auto *row = window.findChild<QWidget *>(QStringLiteral("savedQueryRow")); + QVERIFY(row); + auto *box = qobject_cast<QHBoxLayout *>(row->layout()); + QVERIFY(box); + + auto *menuButton = + window.findChild<QPushButton *>(QStringLiteral("savedQueryMenuButton")); + QVERIFY(menuButton); + + int menuIndex = -1; + int stretchIndex = -1; + for (int i = 0; i < box->count(); ++i) { + QLayoutItem *item = box->itemAt(i); + if (item->widget() == menuButton) + menuIndex = i; + else if (!item->widget() && item->spacerItem()) + stretchIndex = i; + } + + QVERIFY2(stretchIndex >= 0, "the row has no stretch to align against"); + QVERIFY2(menuIndex > stretchIndex, + "the menu must come AFTER the stretch to sit at the right edge"); +} + +/// The row must not vanish when every saved query is unpinned: the menu is +/// then the only way to reach any of them, and hiding the row buries it. +void TestMainWindow::theRowSurvivesWithNothingButUnpinnedQueries() +{ + QTemporaryDir dir; + QVERIFY(dir.isValid()); + Config config; + loadWithQueries(config, dir, QStringLiteral(R"({ + "version": 1, + "queries": [ + { "name": "Buried", "query": "tag:buried" }, + { "name": "AlsoBuried", "query": "tag:also" } + ] + })")); + + MainWindow window(config); + auto *row = window.findChild<QWidget *>(QStringLiteral("savedQueryRow")); + QVERIFY(row); + QVERIFY2(!row->isHidden(), + "the row was hidden, so the only route to these queries is gone"); + + auto *menuButton = + window.findChild<QPushButton *>(QStringLiteral("savedQueryMenuButton")); + QVERIFY(menuButton); + 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<QPushButton *>(QStringLiteral("sentButton")); + QVERIFY(button); + + auto *queryEdit = + window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + QVERIFY(queryEdit); + auto *model = window.findChild<ThreadListModel *>(); + 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<QPushButton *>(QStringLiteral("sentButton")); + QVERIFY2(button, "the generated entry lost its identity when renamed"); + + auto *queryEdit = + window.findChild<QLineEdit *>(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<QPushButton *>(QStringLiteral("sentButton")), + "a generated query with nothing to show must not get a button"); +} + +/// Reads queries.json back from disk, which is what "it was saved" means. +static QJsonArray storedQueries(const QTemporaryDir &dir) +{ + QFile f(dir.filePath(QStringLiteral("qtmaildir/queries.json"))); + if (!f.open(QIODevice::ReadOnly)) + return {}; + const QJsonObject root = QJsonDocument::fromJson(f.readAll()).object(); + return root.value(QStringLiteral("queries")).toArray(); +} + +static QAction *contextActionNamed(MainWindow &window, QWidget *target, + const QString &objectName) +{ + const QList<QAction *> actions = target->actions(); + for (QAction *action : actions) { + if (action->objectName() == objectName) + return action; + } + Q_UNUSED(window); + return nullptr; +} + +void TestMainWindow::aSavedQueryButtonOffersEditUnpinAndDelete() +{ + QTemporaryDir dir; + QVERIFY(dir.isValid()); + Config config; + loadWithQueries(config, dir, QStringLiteral(R"({ + "version": 1, + "queries": [ + { "name": "Inbox", "query": "tag:inbox", "pinned": true } + ] + })")); + + MainWindow window(config); + auto *row = window.findChild<QWidget *>(QStringLiteral("savedQueryRow")); + QVERIFY(row); + auto *button = row->findChild<QPushButton *>(); + QVERIFY(button); + + // A context menu, so the actions live on the widget itself. + QCOMPARE(button->contextMenuPolicy(), Qt::ActionsContextMenu); + QVERIFY(contextActionNamed(window, button, QStringLiteral("editQuery"))); + QVERIFY(contextActionNamed(window, button, QStringLiteral("pinQuery"))); + QVERIFY(contextActionNamed(window, button, QStringLiteral("deleteQuery"))); +} + +void TestMainWindow::unpinningMovesAQueryToTheMenu() +{ + QTemporaryDir dir; + QVERIFY(dir.isValid()); + Config config; + loadWithQueries(config, dir, QStringLiteral(R"({ + "version": 1, + "queries": [ + { "name": "Inbox", "query": "tag:inbox", "pinned": true }, + { "name": "Other", "query": "tag:other", "pinned": true } + ] + })")); + + MainWindow window(config); + auto *row = window.findChild<QWidget *>(QStringLiteral("savedQueryRow")); + QVERIFY(row); + QCOMPARE(savedQueryButtonLabels(window).size(), 2); + QVERIFY(!window.findChild<QPushButton *>( + QStringLiteral("savedQueryMenuButton"))); + + auto *button = row->findChild<QPushButton *>(); + QVERIFY(button); + QAction *pin = contextActionNamed(window, button, QStringLiteral("pinQuery")); + QVERIFY(pin); + pin->trigger(); + + // Off the row, into the menu, and written to the file: an unpin that only + // redrew would come back pinned on the next launch. + QCOMPARE(savedQueryButtonLabels(window), QStringList{ QStringLiteral("Other") }); + auto *menuButton = + window.findChild<QPushButton *>(QStringLiteral("savedQueryMenuButton")); + QVERIFY(menuButton); + QCOMPARE(menuButton->menu()->actions().size(), 1); + + const QJsonArray stored = storedQueries(dir); + QCOMPARE(stored.size(), 2); + QCOMPARE(stored.at(0).toObject().value(QStringLiteral("name")).toString(), + QStringLiteral("Inbox")); + QVERIFY2(!stored.at(0).toObject().contains(QStringLiteral("pinned")), + "the unpin did not reach the file"); +} + +void TestMainWindow::deletingRemovesTheQueryFromTheFile() +{ + QTemporaryDir dir; + QVERIFY(dir.isValid()); + Config config; + loadWithQueries(config, dir, QStringLiteral(R"({ + "version": 1, + "queries": [ + { "name": "Doomed", "query": "tag:doomed", "pinned": true }, + { "name": "Keeper", "query": "tag:keeper", "pinned": true } + ] + })")); + + MainWindow window(config); + auto *row = window.findChild<QWidget *>(QStringLiteral("savedQueryRow")); + QVERIFY(row); + auto *button = row->findChild<QPushButton *>(); + QVERIFY(button); + QCOMPARE(button->text(), QStringLiteral("Doomed")); + + QAction *del = + contextActionNamed(window, button, QStringLiteral("deleteQuery")); + QVERIFY(del); + // Destructive and not on the undo stack, so it confirms. Suppressed here + // rather than driven through the modal dialog, which would hang the test. + window.setConfirmDeleteForTesting(false); + del->trigger(); + + QCOMPARE(savedQueryButtonLabels(window), + QStringList{ QStringLiteral("Keeper") }); + + const QJsonArray stored = storedQueries(dir); + QCOMPARE(stored.size(), 1); + QCOMPARE(stored.at(0).toObject().value(QStringLiteral("name")).toString(), + QStringLiteral("Keeper")); +} + +/// A field a later build wrote must survive an edit here, or upgrading and +/// downgrading silently strips config the user set. +void TestMainWindow::anEditedQueryKeepsItsUnknownFields() +{ + QTemporaryDir dir; + QVERIFY(dir.isValid()); + Config config; + loadWithQueries(config, dir, QStringLiteral(R"({ + "version": 1, + "queries": [ + { "name": "Inbox", "query": "tag:inbox", "pinned": true, + "icon": "mail-inbox" } + ] + })")); + + MainWindow window(config); + + // Through the EDIT path, with a replacement carrying no unknown fields of + // its own, which is exactly what SaveQueryDialog returns. Driving this + // through unpin instead proved nothing: unpin copies the stored entry, so + // it carries `unknown` along by itself and the merge is never exercised. + // That version passed with the merge deleted. + SavedQuery edited; + edited.name = QStringLiteral("Inbox"); + edited.query = QStringLiteral("tag:inbox and not tag:muted"); + edited.pinned = true; + QVERIFY(edited.unknown.isEmpty()); + window.replaceSavedQueryForTesting(QStringLiteral("Inbox"), edited); + + const QJsonArray stored = storedQueries(dir); + QCOMPARE(stored.size(), 1); + const QJsonObject entry = stored.at(0).toObject(); + // The edit landed... + QCOMPARE(entry.value(QStringLiteral("query")).toString(), + QStringLiteral("tag:inbox and not tag:muted")); + // ...and did not take the unknown field down with it. + QCOMPARE(entry.value(QStringLiteral("icon")).toString(), + QStringLiteral("mail-inbox")); +} + +/// Renaming must match on the name the dialog OPENED with. Matching on the +/// returned name leaves the original in place and adds a second entry. +void TestMainWindow::renamingReplacesRatherThanDuplicating() +{ + QTemporaryDir dir; + QVERIFY(dir.isValid()); + Config config; + loadWithQueries(config, dir, QStringLiteral(R"({ + "version": 1, + "queries": [ + { "name": "Old", "query": "tag:old", "pinned": true } + ] + })")); + + MainWindow window(config); + + SavedQuery renamed; + renamed.name = QStringLiteral("New"); + renamed.query = QStringLiteral("tag:old"); + renamed.pinned = true; + window.replaceSavedQueryForTesting(QStringLiteral("Old"), renamed); + + const QJsonArray stored = storedQueries(dir); + QCOMPARE(stored.size(), 1); + QCOMPARE(stored.at(0).toObject().value(QStringLiteral("name")).toString(), + QStringLiteral("New")); + QCOMPARE(savedQueryButtonLabels(window), QStringList{ QStringLiteral("New") }); +} + #include "test_mainwindow.moc" |