diff options
| -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. |