diff options
Diffstat (limited to 'docs/superpowers')
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md | 163 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 116 |
2 files changed, 171 insertions, 108 deletions
diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md index 4ea175a..c8ffb53 100644 --- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md +++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md @@ -1171,6 +1171,40 @@ delivered per-message inspection at size S, touching only `htmlbuilder.cpp` and neither fragile area, but gives no message rows in the list and no reply-tree indentation. The user chose the full model deliberately. +## 23. No way to save a search query from the UI + +**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. 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 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. + ## 24. No right-click actions on the thread list **Observed (user, 2026-08-04):** "right click actions on the list (left pane)". @@ -4532,3 +4566,132 @@ the event loop has run, so the test needs `processEvents` after selecting a rule or it measures one row's height twice. **Size: XS.** +## 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. + + +## 83. A rule named with spaces is written to the file and dropped by every reader + +**Observed (user, 2026-08-14):** a rule filled in from the dialog, with the +count checked against the preview, was gone on reopening the window. Repeated +attempts under different names lost it every time. Closing and restarting the +application did not bring it back. + +**Cause (verified against the live file).** The rule was on disk, exactly once, +with its query, tags and note intact. Its `id` was `justeat orders`, and +`TagRules::load()` required `^[a-z0-9][a-z0-9-]*$`, so it was discarded on every +read while continuing to occupy the file. + +The fault was the asymmetry, not the pattern. The save path took +`m_id->text().trimmed()` verbatim and wrote it; the load path validated and +dropped. A rule could therefore be written correctly and never come back, and +nothing in between reported it. `mailrules.py` enforces the same pattern, so +the rule was not tagging mail either: it had been inert since the day it was +written. + +Two aggravating properties, both worse than the drop itself. The dropped rule +was invisible in the dialog while still present in the file, so the next save +from the dialog would have deleted it permanently, and the field is labelled +**Name**, which invites prose. + +**The warning was not missing.** `showWarnings()` already surfaced "1 rule could +not be read and was skipped" on every open. It was correct, it was ignored, and +it was a dead end: the rule it named could not be reached, so there was nothing +to do about it. A warning the user cannot act on trains them to stop reading +warnings. + +**Fixed 2026-08-14, three parts.** + +- **The name is sanitised into an id when the field is committed**, not on save, + so what the field shows is what reaches the file. `TagRules::sanitiseId()` + lowercases, collapses every run of anything else to one dash and trims the + ends; `uniqueId()` adds a numeric suffix, because sanitising is many-to-one + and manufactures the duplicate that `load()` then drops. +- **`TagRules::validate()` is the one predicate**, run before the write and + reported through the warning label rather than a modal, since `saveForTest()` + drives this path directly and a `QMessageBox` there would hang the suite. +- **A bad id now loads repaired rather than dropped**, so the rule is visible + and fixable. The warning still fires, because what is on disk is not what the + hook runs until the file is saved back. + +**Deliberately not mirrored into `mailrules.py`.** The hook applies rules to +real mail every ten minutes with nobody watching its output; silently renaming +an id there would tag mail under a rule the file does not contain. Dropping is +the correct failure for the hook and repairing is the correct one for the +editor, and the file converges as soon as the dialog saves. No format change and +no version bump, so this stayed a single-repo fix. + +**An already-legal id is never rewritten**, including one like `a---b` that +sanitising would otherwise collapse. Rewriting valid ids would churn a file +mailctl also reads and show a diff the user never made. A test asserting the +collapse was written first and was wrong; the code was right. + +**Verification.** Both new dialog tests were confirmed to fail with the +sanitiser reverted, one on the field contents and one on the save being refused. +32 tests in `test_tagrules`, 20 of 20 suites green. The user's own rule was +repaired in place to `justeat-orders` and verified to load through +`mailrules.py`, 18 rules and no warnings. 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 5e97337..e60b88c 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; see `specs/2026-08-13-saved-queries-design.md`. Saved queries move to `queries.json`, carrying order, `pinned` and account scope | +| 23 | No way to save a search query from the UI | workflow | M | **done** 2026-08-13, shipped in 0.18.0; see `specs/2026-08-13-saved-queries-design.md` | | 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** | @@ -139,14 +139,15 @@ taking that too literally. | 72 | No khard/khal integration | workflow | ? | open, unspecified; the user places it after send, so v2 at the earliest | | 73 | This backlog is past four thousand lines | maintenance | S | **done** 2026-08-13; 5056 lines to 578, closed sections moved to `2026-08-03-post-0.1.0-usability-closed.md` | | 74 | "Searching..." keeps claiming a query is running while rows are already arriving | feedback | XS | open; cause measured 2026-08-11, the delay itself is the cold page cache and is not fixable here | -| 75 | The tagging rules window forgets its size and its column widths | persistence | S | **done** 2026-08-13 on `rule-builder`, unreleased. The window-kind question is left open, see the closed-items file | -| 76 | Every field in the rules dialog is free text, so a rule is easy to get wrong | workflow | M | **done** 2026-08-13 on `rule-builder`, unreleased. See `specs/2026-08-13-rule-builder-design.md` | -| 77 | No way to see what a rule would collect, in the thread list | workflow | S | **done** 2026-08-13 on `rule-builder`, unreleased | +| 75 | The tagging rules window forgets its size and its column widths | persistence | S | **done** 2026-08-13, shipped in 0.17.0. The window-kind question is left open, see the closed-items file | +| 76 | Every field in the rules dialog is free text, so a rule is easy to get wrong | workflow | M | **done** 2026-08-13, shipped in 0.17.0. See `specs/2026-08-13-rule-builder-design.md` | +| 77 | No way to see what a rule would collect, in the thread list | workflow | S | **done** 2026-08-13, shipped in 0.17.0 | | 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 | +| 80 | A rule with many conditions squeezes the rule list to one visible row | defect | XS | **done** 2026-08-13, shipped in 0.17.0. 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 | +| 81 | No way to turn a saved query into a tagging rule | workflow | S | open; 23 has shipped, so the dialog it depends on exists. 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, shipped in 0.18.0. Right-click offers Edit, Pin/Unpin and Delete | +| 83 | A rule named with spaces is written to the file and dropped by every reader | defect | S | **done** 2026-08-14, unreleased. The name is sanitised into an id, save validates, a bad id loads for repair | Sizes are rough: XS under an hour, S a sitting, M a session. @@ -210,40 +211,6 @@ literal, the descriptions are prose. **Verification:** run `lupdate` and read the generated `.ts`. A string that does not appear there is not translatable, whatever the source looks like. -## 23. No way to save a search query from the UI - -**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. 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 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 **Observed:** twice in one session (0.8.0), a defect could not be given a @@ -575,73 +542,6 @@ 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 |