diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-14 11:01:58 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-14 11:01:58 +0200 |
| commit | b08923df88de7ba03135234aaf3c602f23e49e03 (patch) | |
| tree | 5cc74979efc3470e948bcdfb1b7343e880254c07 /docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | |
| parent | 27838b47bae379578f458ebaefa192b422b55e24 (diff) | |
| download | qtmaildir-b08923df88de7ba03135234aaf3c602f23e49e03.tar.gz qtmaildir-b08923df88de7ba03135234aaf3c602f23e49e03.zip | |
fix(rules): stop a rule with a spaced name from vanishing on save
A rule named "justeat orders" in the field labelled Name was written to
rules.json correctly and then dropped by every reader, because load()
required ^[a-z0-9][a-z0-9-]*$ and the save path validated nothing. The
rule stayed in the file, invisible in the dialog, never applied by the
post-new hook, and the next save from the dialog would have deleted it
outright.
The asymmetry was the defect, not the pattern. TagRules::validate() is
now the single predicate: the dialog refuses to save against it, and
load() uses it to repair rather than drop, so a rule that fails is
visible and fixable instead of silently discarded.
- The typed name is sanitised into an id when the field is committed,
so the field shows what will reach the file. uniqueId() suffixes a
collision, since sanitising is many-to-one and can manufacture the
duplicate that load() then drops.
- 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.
- A bad id loads repaired, with the warning kept: what is on disk is
not what the hook runs until the file is saved back.
Deliberately not mirrored into mailrules.py. The hook tags real mail
unattended every ten minutes, where silently renaming an id is worse
than dropping the rule; the file converges as soon as the dialog saves.
No format change, so no version bump and no two-repo commitment.
The load warning was not missing: it had been showing "1 rule could not
be read and was skipped" on every open, in the same font and colour as
the intro prose two lines above it, and read as more explanation. It is
now a red banner beside Save, with an icon and a dismiss button, and it
says the rules need attention rather than that they were skipped, which
is no longer true. Dismissal is per-appearance only; a persistent one
would re-hide the problem that went unnoticed for a session.
Both new dialog tests were confirmed to fail with the sanitiser
reverted, and the banner's styling, position and dismissal each fail
under mutation. 20 of 20 suites green, 34 tests in test_tagrules.
Closes item 83.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Diffstat (limited to 'docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md')
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 116 |
1 files changed, 8 insertions, 108 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 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 |
