diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-13 19:00:04 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-13 19:00:04 +0200 |
| commit | f389db3aad498d46c95c2a95b4280ffb541043b2 (patch) | |
| tree | a4c228184210aab5ba8380b8345dde297aa2b0e2 /docs/superpowers | |
| parent | a198c116e14622be6ba76b78fca5d92b4b4f62bd (diff) | |
| download | qtmaildir-f389db3aad498d46c95c2a95b4280ffb541043b2.tar.gz qtmaildir-f389db3aad498d46c95c2a95b4280ffb541043b2.zip | |
docs: spec the saved-query file, and reduce item 23 to a pointer
Item 23 had grown past what a backlog entry should hold: a storage format, a
migration, a dialog and a layout change. This document's own rule says a fully
specified item moves to specs/ and leaves behind the two or three things that
decide whether it can be picked up, the way items 53, 63 and 76 went. The entry
now carries the observation, the three deciding constraints and the relation to
item 10, and points at the spec for the rest.
The spec pins what was still loose. The JSON is an ordered array, since the
ordering is the whole reason for moving off [queries], and nothing may sort it
on load. A query's account scope stores the account KEY, the INI group suffix,
rather than the maildir path, so it does not duplicate config that already lives
in one place and go stale when the user edits it; the scope then composes
through Account::scopedQuery(), whose parenthesisation is load-bearing for the
same reason it is in the rules hook, an unparenthesised disjunction escapes its
scope and matches every account.
Migrated entries are pinned, so the query row does not silently empty on the
first run after upgrade, and migration order is alphabetical because that is
genuinely all the INI knows. Sent stays out of the file: it is generated from
allSentQuery() rather than stored, and folding it in would mean writing a
per-account path query into stored config, which is the duplication the
account-key decision just rejected.
The testing section is written against the traps already recorded in CLAUDE.md.
The migration test asserts the INI file is byte-identical rather than re-reading
it through QSettings, which would pass against a rewrite that preserved values
while dropping comments; the round-trip test asserts order, which is the
property the INI could not provide; and the dialog is left to a hand test,
because the offscreen platform cannot assert sizing at all and a Cancel goes
through done(int) rather than closeEvent.
Every code reference in the spec was checked against the files rather than
copied from the entry.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Diffstat (limited to 'docs/superpowers')
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 133 | ||||
| -rw-r--r-- | docs/superpowers/specs/2026-08-13-saved-queries-design.md | 200 |
2 files changed, 228 insertions, 105 deletions
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 5faf859..268dbb4 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 @@ -87,7 +87,7 @@ taking that too literally. | 20 | Thread view does not match the user's mental model | presentation | L | **done** 2026-08-10, as the card list; see 53 | | 21 | Default shortcuts are not sensible enough | discoverability | S | open | | 22 | Translatability audit and i18n wiring | correctness | M | open | -| 23 | No way to save a search query from the UI | workflow | M | open, specified 2026-08-13. Saved queries move to `queries.json`, which carries order, `pinned` and account scope; wants a spec before building | +| 23 | No way to save a search query from the UI | workflow | M | open, specified 2026-08-13; see `specs/2026-08-13-saved-queries-design.md`. Saved queries move to `queries.json`, carrying order, `pinned` and account scope | | 24 | No right-click actions on the thread list | discoverability | S | **done** | | 25 | No select-all, and bulk actions are undiscoverable | workflow | S | **done** | | 26 | No way to add or remove an arbitrary tag from the UI | workflow | S | **done** | @@ -145,7 +145,7 @@ taking that too literally. | 78 | No way to build a rule from something visible in a message | workflow | M | open; wants 76 first, so the created rule lands in a form that can hold it | | 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; split out of 23 on 2026-08-13 and depends on it. Writes to the shared rules file, so it spans this repo and `mailctl` | +| 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` | Sizes are rough: XS under an hour, S a sitting, M a session. @@ -213,112 +213,35 @@ does not appear there is not translatable, whatever the source looks like. **Observed (user, 2026-08-04):** saved queries live in the config file only. There is no way to keep a query you have just written without editing -`qtmaildir.conf` by hand. - -**Design (user, 2026-08-13).** The user described the shape they want, which is -more specific than the original sketch and settles the file question below. - -- A **Save query** button beside the search bar. It opens a dialog on the - current query bar contents, where the query gets a **name**, an **account - scope**, and whatever else is worth editing before it is kept. -- **Two tiers, presented differently.** Queries like Inbox and Unread stay - visible as buttons; user-made ones like "all messages from one correspondent" - go behind a menu. Today every entry in `[queries]` becomes a button - (`src/mainwindow.cpp:557`), which is the thing that does not scale. -- If the tiers split, **the button row moves to a row of its own**, rather than - sharing the query row with the account box, the sort dropdown and the query - bar. - -**There is no built-in default set.** Nothing in the code distinguishes a -default query from a user-made one: every entry in `[queries]` is user-written -config and is rendered identically. Sent is the lone exception and is not in -`[queries]` at all, it is built from `allSentQuery()` -(`src/mainwindow.cpp:579`). So the two tiers need a mechanism that does not -exist yet. - -**Decision (user, 2026-08-13): saved queries move out of the INI into -`~/.config/qtmaildir/queries.json`.** The user proposed it by analogy with the -rules file, and it is a better answer than the two the entry had considered (a -per-query `pinned` flag inside `[queries]`, or a `[general] pinned_queries` -list). It solves three things at once where those solved one each. - -- **Order.** `[queries]` is read through `childKeys()`, which returns keys - alphabetically and not in file order, so button order is alphabetical today - and cannot be chosen. `src/config.cpp:401` already records that a hand-rolled - parser would be needed to change it. A JSON array is ordered intrinsically, - so the row of buttons can be arranged. -- **The `pinned` flag**, which decides the two tiers. -- **Per-query account scope**, which the save dialog wants and which - `SavedQuery` (`src/config.h:99`) has no room for: it is `{name, query}` and - nothing else. - -**Take the shape of `rules.json`, not its machinery.** That file is JSON because -it has TWO independent implementations that must agree, this repo and mailctl's -`mailrules.py`; the unknown-field preservation and the `kFormatVersion` -handshake exist to stop two programs destroying each other's writes. Queries -have exactly one reader, so copying that wholesale would import the ceremony -without the reason for it. What is worth carrying across is a versioned document -read and written by one class, preserving unknown fields on the way through so a -field added later is not stripped by an older build. - -**Migration: read `[queries]` once, write the JSON, and leave the INI section -alone.** `[queries]` exists in the user's live config and in the README, so it -cannot simply stop being read. On a run where `queries.json` is absent, the -section is read, converted and written out; from then on `[queries]` is ignored -entirely. **Do not strip the section:** removing it means rewriting the -hand-edited file with QSettings, which drops comments and key order across the -WHOLE file and is exactly the loss this decision avoids. Leaving it costs a few -stale lines the user can delete by hand, and keeps a downgrade working. The -README should mark it legacy. Reject the third option, reading both forever: -two sources of truth for one thing, with no answer for which wins on a name -collision or where the dialog writes. - -**This retires the entry's original open question.** It asked whether the write -went to `qtmaildir.conf` or `uistate.conf`, and both answers were poor: the -first machine-writes a hand-edited file, the second files user intent as window -state. A machine-written JSON document beside the hand-written INI is the -cleaner split, and the README caveat about losing comments goes away with it. -The file belongs in `~/.config/`, not `~/.local/state/`: it is the user's -intent, hand-editable, and belongs in a config backup, which a state directory -is commonly excluded from. - -**"Save as a filter" is deliberately not part of this item.** The user raised it -as an option: the same dialog could write the query out as a tagging rule, the -reverse of item 77's Preview in list. It is left out because a rule is not a -view. Rules live in `~/.config/mailrules/rules.json`, which has two independent -implementations that agree by test rather than by shared code, and a `post-new` -hook applies them to real mail every ten minutes. Writing one is a two-repo -commitment with a live blast radius, and folding it in here would make a -presentation change carry that risk. It is item 81. +`qtmaildir.conf` by hand. Underneath it, every saved query becomes a button +(`src/mainwindow.cpp:557`), so the query row grows without bound and cannot tell +Inbox from a one-off search. + +**Specified 2026-08-13. Read +`specs/2026-08-13-saved-queries-design.md` instead of planning from here.** + +The three things that decide whether this can be picked up: + +- **Saved queries move out of `[queries]` into + `~/.config/qtmaildir/queries.json`**, gaining an order, a `pinned` flag and a + per-query account scope. The INI cannot express order at all: `childKeys()` + returns keys alphabetically and `src/config.cpp:401` already records that a + hand-rolled parser would be needed to change it. +- **It stays a single-repo change.** The format takes the shape of `rules.json`, + a versioned document preserving unknown fields, but none of its + two-implementation machinery, because queries have one reader. +- **`startup_query`'s fallback changes meaning**, from alphabetically-first to + first-in-the-user's-order. User-visible, so this is a minor bump and wants a + changelog line. The README documents `[queries]` in three places, one of which + explains the alphabetical ordering this removes. **Relation to item 10.** Item 10 is postponed, but its second half proposed exactly this: "saved queries that carry their own account scope, so one action -gets there". The 2026-08-13 design puts account scope in the save dialog -outright, so item 10's remaining complaint is answered as a side effect rather -than needing its own work. `Account::scopedQuery()` (`src/config.cpp:42`) -already composes the scope, so this is a UI question, not a query question. Do -not reopen item 10 to do it: the user postponed it and asked that the remaining -work not be proposed unprompted. - -**Constraint:** the button row is built inline in `MainWindow` -(`src/mainwindow.cpp:554-587`). There is no `SavedQueryBar` class and there -never has been, whatever an older revision of this entry said. Moving the -buttons to their own row is a change to that layout code. - -**Constraint: `startup_query` names a saved query by name**, and resolves -against the list with a case-insensitive match plus a fallback to the first -entry (`Config::startupSavedQuery()`, `src/config.cpp:422`). It must keep -resolving across the migration, which also means the "first entry" fallback -stops meaning "alphabetically first" and starts meaning "first in the user's own -order". That is an improvement rather than a regression, but it changes which -query a config with an unmatched `startup_query` opens on, so it is a -user-visible change and belongs in the changelog. - -**Constraint: the README documents `[queries]` in three places** (the -`startup_query` note at :109, the sample config at :194, and the query-row -description at :256), including a line explaining that buttons appear in -alphabetical order rather than file order. That explanation is exactly what this -change removes, so all three need revising, not just the sample. +gets there". The account scope in the save dialog answers it as a side effect. +Do not reopen item 10 to do it: the user postponed it and asked that the +remaining work not be proposed unprompted. + +**Item 81 depends on this** and is deliberately not part of it. ## 36. `test_mainwindow` cannot reach the worker diff --git a/docs/superpowers/specs/2026-08-13-saved-queries-design.md b/docs/superpowers/specs/2026-08-13-saved-queries-design.md new file mode 100644 index 0000000..6d0019f --- /dev/null +++ b/docs/superpowers/specs/2026-08-13-saved-queries-design.md @@ -0,0 +1,200 @@ +# Saved queries in a file of their own: design + +Backlog item 23, "No way to save a search query from the UI", and item 81, "No +way to turn a saved query into a tagging rule", which depends on it. + +**Status:** design approved 2026-08-13, not implemented. + +## The problem this solves + +A saved query can only be created by hand-editing `qtmaildir.conf`. There is no +way to keep a query you have just written and are looking at the results of, +which is exactly the moment you know it is worth keeping. + +Underneath that sits a second problem the user named separately: every saved +query becomes a button in the query row (`src/mainwindow.cpp:557`), so the row +grows without bound and cannot distinguish "Inbox", which belongs on screen +permanently, from "all messages from one correspondent", which belongs in a +menu. + +## Why the storage moves, and what that decides + +The obvious implementation writes back into `[queries]`. It was rejected, and +the reason shapes the rest of this document. + +**`[queries]` cannot express order.** It is read through +`QSettings::childKeys()`, which returns keys alphabetically and never in file +order. `src/config.cpp:401` already carries a comment saying a hand-rolled +parser would be needed to change that, and the README documents the alphabetical +button order as though it were a feature. A row of pinned buttons that cannot be +arranged is not worth building. + +**QSettings would also destroy the file it wrote to.** It preserves neither +comments nor key order, so the first save from the dialog reformats a +hand-edited config wholesale. Item 1 established that machine-written state stays +out of `qtmaildir.conf` for precisely this reason. + +So saved queries move to **`~/.config/qtmaildir/queries.json`**, and gain three +things the INI could not hold: an order, a `pinned` flag, and a per-query account +scope. + +**In `~/.config/`, not `~/.local/state/`.** A named, scoped query is user intent, +hand-editable, and belongs in a config backup; a state directory is commonly +excluded from one. `uistate.conf` keeps window blobs and column widths, which is +a different kind of thing. + +### The rules.json analogy, and where it stops + +The user proposed this by analogy with `~/.config/mailrules/rules.json`. The +analogy is right about the shape and wrong about the machinery, and the +difference is worth stating because copying too much of it would be a real cost. + +`rules.json` carries unknown-field preservation and a `kFormatVersion` handshake +because it has **two independent implementations** that agree by test rather than +by shared code: `src/tagrules.cpp` here and `mailrules.py` in `../mailctl`. All +of that exists to stop two programs destroying each other's writes. + +`queries.json` has exactly one reader. What carries across: + +- a versioned document, so a future breaking change has a way to announce itself +- unknown fields preserved through a load/save round trip, so a field added by a + later build is not stripped by an older one + +What does not carry across: any notion of a second implementation, a shared +contract, or a two-repo change. This is a single-repo change and must stay one. + +## The format + +```json +{ + "version": 1, + "queries": [ + { "name": "Inbox", "query": "tag:inbox", "pinned": true }, + { "name": "Unread", "query": "tag:unread", "pinned": true }, + { "name": "Work invoices", "query": "from:billing", "account": "work" } + ] +} +``` + +`queries` is an **array**, and its order is the display order. That is the whole +point of the change; nothing may sort it on load. + +| field | required | meaning | +|---|---|---| +| `name` | yes | Display name, and what `startup_query` matches against | +| `query` | yes | notmuch query, wire format, never translated | +| `pinned` | no, default `false` | Renders as a button; otherwise it lives in the menu | +| `account` | no | An account **key**, the INI group suffix (`work` from `[account.work]`), not a path or a display name | + +**`account` stores the key, not the maildir path.** `Account::scopedQuery()` +(`src/config.cpp:42`) composes `path:"<maildir>/**"` from the account, so storing +the path would duplicate config that already exists in one place and go stale +when the user edits it. A key naming an account that no longer exists resolves to +no scope, with a config problem reported the way an unmatched `startup_query` +already is. + +**Scope composes, it does not replace.** A query with an `account` runs as +`Account::scopedQuery(query)`, which yields `path:"<maildir>/**" and (<query>)`. +The parenthesisation is load-bearing for the same reason it is in the rules hook: +`path:... and a or b` binds as `(path:... and a) or b`, so an unparenthesised +disjunction escapes its scope and matches every account. `scopedQuery` already +does this correctly; the requirement is to use it rather than concatenating. + +## Migration + +On a run where `queries.json` does not exist and `[queries]` does: read the INI +section, write the JSON, and **leave `[queries]` in place, untouched**. From then +on the JSON is the only source and `[queries]` is ignored entirely. + +**Do not strip the section.** Removing it means rewriting the hand-edited file +with QSettings, which drops comments and key order across the whole file, not +just the part being removed. That is the loss this design exists to avoid. +Leaving it costs a few stale lines the user can delete by hand whenever they +like, and keeps a downgrade to an older build working. + +Migrated entries get `pinned: true`, since they are buttons today and the +migration must not silently empty the query row. Order is alphabetical on +migration because that is genuinely all the INI knows; the user reorders once, by +hand or in the dialog, and it sticks from then on. + +**Rejected: reading both forever.** Two sources of truth for one thing, with no +answer for a name colliding across them and no obvious place for the dialog to +write. + +## Consequences for `startup_query` + +`[general] startup_query` names a saved query and resolves case-insensitively, +falling back to the first entry (`Config::startupSavedQuery()`, +`src/config.cpp:422`). It keeps working unchanged, but **its fallback quietly +changes meaning**: "the first entry" stops being "alphabetically first" and +becomes "first in the user's own order". + +That is an improvement, and it is still a user-visible behaviour change for a +config whose `startup_query` matches nothing. It belongs in the changelog, and +it makes this a minor version bump rather than a patch. + +## The UI + +### A Save query button beside the search bar + +Opens a dialog on the current query bar contents, holding: + +- **name**, required, and unique. A name that already exists offers to overwrite + rather than silently creating a duplicate the menu would render twice. +- **query**, editable, prefilled from the query bar. +- **account scope**, a dropdown of configured accounts plus "All accounts". +- **pinned**, a checkbox: button or menu. + +### The button row moves to a row of its own + +Today the query row holds the account box, the sort dropdown, the query bar and +every saved-query button (`src/mainwindow.cpp:554-587`). Pinned queries move to +their own row beneath it, and the unpinned ones go into a menu. + +**There is no `SavedQueryBar` class**, whatever older revisions of the backlog +said. These buttons are built inline in `MainWindow` and this is a change to that +layout code. + +Sent stays where it is. It is not a saved query, it is built from +`allSentQuery()` (`src/mainwindow.cpp:579`), and folding it into `queries.json` +would mean generating a per-account path query into stored config, which is the +duplication the `account`-key decision just rejected. + +## What is deliberately not here + +**Item 81, saving a query as a tagging rule.** A saved query is a view and costs +nothing if it is wrong. A rule is applied to real mail by the `post-new` hook +every ten minutes and lives in the file shared with mailctl. Keeping it out means +item 23 ships as a single-repo change; folding it in would give a presentation +change a two-repo commitment and a live blast radius. + +**Item 10's account persistence.** Per-query account scope answers item 10's +remaining complaint as a side effect, which is noted in both entries. Item 10 +itself stays postponed and is not reopened here: the user asked that the rest of +it not be proposed unprompted. + +## Testing + +`Config` is already unit-tested, and this is mostly a format with an ordering +guarantee, so most of it is testable without a window. + +- Round trip: load, save, load, and assert the document is unchanged **including + order**, which is the property the INI could not provide. +- Unknown fields survive a round trip. Write a field no build knows, load, save, + assert it is still there. +- Migration: an `[queries]` section and no JSON produces a JSON file with every + entry `pinned`, and leaves the INI file **byte-identical**. Assert on the file + bytes, not on a re-read through QSettings, which would pass against a rewrite + that happened to preserve values while dropping comments. +- A malformed or unreadable `queries.json` reports a config problem and does not + take the query row down with it. +- `startup_query` still resolves by name, and its fallback now returns the first + entry in document order. +- Scope composition yields `path:"<maildir>/**" and (<query>)` with the + parentheses present. Assert on the composed string for a query containing + `or`, which is the case that breaks without them. + +**The dialog itself needs a hand test**, per the offscreen-platform constraints +already recorded in CLAUDE.md: sizing cannot be asserted there at all, and a +dialog's Cancel path goes through `done(int)` rather than `closeEvent`, so a test +that only exercises `close()` proves less than it appears to. |
