aboutsummaryrefslogtreecommitdiffstats
path: root/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md
diff options
context:
space:
mode:
Diffstat (limited to 'docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md')
-rw-r--r--docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md163
1 files changed, 163 insertions, 0 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.