diff options
Diffstat (limited to 'docs/superpowers')
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 265 |
1 files changed, 263 insertions, 2 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 c3a1f4e..99e3b64 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 @@ -16,8 +16,11 @@ notes. Cite these numbers, not the notes', and do not renumber to reconcile. **The notes are the upstream source and they keep growing.** The user adds to them while using the app, so this document goes stale on its own. Items 28 to 35 came from one such pass on 2026-08-04 and included two defects that had gone -unrecorded here for a while. Compare the two at the start of a session; the -procedure is in `CLAUDE.md`. +unrecorded here for a while. Items 39 to 45 came from the 2026-08-05 pass, which +found one more defect (41, a message body silently dropped by the MIME walk) and +one item that cannot be planned at all until the user says where the thing it +manages lives (44). Compare the two at the start of a session; the procedure is +in `CLAUDE.md`. Numbering is stable. New items append with the next free number and never renumber, so a note referring to "item 7" keeps meaning the same thing. An item @@ -81,6 +84,13 @@ taking that too literally. | 34 | No overview of the Maildir itself | information | M | open | | 35 | No refresh of the thread list after a sync | workflow | M | open | | 36 | `test_mainwindow` cannot reach the worker | testing | S | open, on demand | +| 39 | Thread list cannot be sorted by clicking a column header | workflow | S | open | +| 40 | No live filter over the current view | workflow | M | open | +| 41 | A message whose HTML body carries a `Content-Id` renders blank | correctness | S | open | +| 42 | "Syncing..." says nothing about what is being synced | feedback | S | open | +| 43 | No "Mark all read" for the current view | workflow | S | open | +| 44 | No way to manage the filters applied at sync time | workflow | ? | open, unspecified | +| 45 | Two Sync buttons on the main window | discoverability | XS | open | Sizes are rough: XS under an hour, S a sitting, M a session. @@ -1743,6 +1753,257 @@ rather than modelled. `MainWindow` and must keep working. The fixture is per-test, not a suite-wide `initTestCase`, or every case pays for a `notmuch new`. +## 39. Thread list cannot be sorted by clicking a column header + +**Observed (user, 2026-08-05):** "left pane columns order by clicking on the +column header." + +**Cause (verified in code):** nothing sorts. `setSortingEnabled` appears nowhere +in `src/`, no `QSortFilterProxyModel` exists, and `ThreadListModel` implements no +`sort()`. The order is whatever the worker emitted, and that is fixed: +`NotmuchWorker::runQuery()` calls +`notmuch_query_set_sort(..., NOTMUCH_SORT_NEWEST_FIRST)` +(`src/notmuchworker.cpp:135`). Clicking a header does nothing because the header +was never made interactive. + +**Approach.** Sort in the model, not in the query. The worker's sort is over +notmuch's own ordering and cannot express "by From" or "by Subject" at all, and +re-querying per header click would send the user's place away for a presentation +change. + +- `ThreadListView::setSortingEnabled(true)` plus a `sort()` on the model, or a + `QSortFilterProxyModel` between them. +- Persist the sort column and order into `uistate.conf`, per item 1's rule. A + sort that resets on restart is item 1 restated. + +**Constraints.** + +- **Batched appends are the real difficulty.** Threads arrive in batches of 200 + through `appendBatch()` while the query is still running, so a sorted view is + re-sorted on every batch and rows move under a selection the user is already + working in. Decide explicitly: sort only once the query completes, or accept + the movement. A proxy model makes this worse rather than better, since it + re-sorts on every insert by default. +- The date column is displayed text but must sort as a timestamp, not as a + string. `ThreadSummary` carries the real value; sort on that, not on the + rendered cell. +- Row styling (item 13) and the unread bold are per-row, so they follow the row + and need nothing here. Verify anyway after a proxy is introduced: a proxy that + forwards only `DisplayRole` drops them silently. + +## 40. No live filter over the current view + +**Observed (user, 2026-08-05):** "search in current view", spelled out as two +things: "a light filter applied live on the current view", and "a search bar +appearing as soon as we type while no entry box is focused". + +**Cause (verified in code):** the only search is the query bar, which runs a +notmuch query and replaces the result set. There is no client-side filtering +of an existing result: no `QSortFilterProxyModel` anywhere in `src/`, and +`ThreadListModel` has no filter of its own. Narrowing the current view therefore +means writing a new notmuch query and losing the view. + +**Approach.** Distinct from the query bar, and the distinction is the point: this +filters rows already fetched, without touching notmuch. + +- A filter over the model's loaded rows, matching subject and from, case + insensitively. No worker round trip. +- A filter strip that appears on the first keystroke while no entry box has + focus, and disappears on Escape, restoring the full result set. + +**Constraints.** + +- **Type-to-filter competes with the plain-letter shortcuts.** Item 3's outcome + records that a plain-letter `QAction` shortcut is suppressed only while an + editable widget has focus, which is exactly the state this feature does not + start in. Any binding that is a bare letter would be swallowed by the filter + strip or would swallow it. Check the current defaults before choosing the + trigger, and prefer appearing only for characters no action claims. +- Escape already blanks the message pane (item 32). If Escape also closes the + filter, decide the precedence explicitly rather than letting whichever handler + runs first win. +- The filter is presentation only: it must not clear the selection, the undo + stack, or the query, and the pending-edit count must not move. +- Interaction with item 39: a filter and a sort over the same rows want the same + proxy. Whichever is built first should leave room for the other. + +## 41. A message whose HTML body carries a `Content-Id` renders blank + +**Observed (user, 2026-08-05):** a specific message from a bulk sender opens +blank, and the app reports it has no HTML part. + +**Cause (verified in code): the inline-part branch runs before the body +branches, and returns.** `collectParts()` in `src/mimeparser.cpp` tests +`g_mime_part_get_content_id()` at `:133` and, whenever a part has one, files it +into `out.inlineParts` and returns at `:139`. The `text/plain` and `text/html` +assignments at `:142-146` are never reached for that part. + +Setting a `Content-Id` on the `text/html` body part is legal and common in +bulk-sender output. Such a message parses with an empty `htmlBody` and an empty +`plainBody`, so `ParsedMessage::hasHtml()` (`src/mimeparser.h:115`) is false, +`HtmlBuilder` falls through to a plain body that is also empty +(`src/htmlbuilder.cpp:208`), and the pane renders nothing. Both halves of the +user's observation follow from one wrong ordering. + +**Approach.** A `Content-Id` makes a part *referenceable*, not non-displayable. +The two are independent, and the current code treats them as exclusive. + +- Register the part in `inlineParts` as today, and then still let a + `text/plain` or `text/html` part fill the corresponding body slot when that + slot is empty. Do not return early on the presence of a content id alone. +- The existing "first one wins" rule (`out.htmlBody.isEmpty()`) already keeps a + genuinely inline image from displacing a real body, so a part that is not text + is unaffected by this change. + +**Constraints.** + +- **Do not use `Content-Disposition: inline` as the discriminator.** It is + absent far more often than it is correct, and a body part commonly carries no + disposition at all. The `attachment` check above it (`:116-117`) is already the + right test for "not a body" and should stay the only one. +- A part that is both the body and a `cid:` target must remain reachable under + its id, or a sibling referencing it breaks. Register first, then assign. +- This is `MimeParser`, which is fixture-tested: the fix needs a fixture message + whose `text/html` part carries a `Content-Id`, asserting both that the body + renders and that the id still resolves. Write the fixture by hand rather than + from real mail, per the no-personal-details rule. + +**Verification:** the user's original message renders. The `cid:` rewriting of +item 15's namespacing is unaffected, which the existing mimeparser tests already +cover. + +## 42. "Syncing..." says nothing about what is being synced + +**Observed (user, 2026-08-05):** the only feedback during a manual sync is +"Syncing" in the status bar. The user asked for the account being synced +(e.g. `Syncing provider-work`) and the operation in progress (mbsync, notmuch). + +**Cause (verified in code): the information is already arriving and is thrown +away.** `assets/mailsync.sh` streams every mbsync and `notmuch new` line, +timestamped, through `tee` (`assets/mailsync.sh:75-96`), and `MailSync` emits +each chunk as `outputReceived` (`src/mailsync.cpp:65-74`), which fills the sync +log pane. The status label is set once to `tr("Syncing...")` +(`src/mainwindow.cpp:1569`) and never updated until the run finishes. So this +needs no change to the script and no new channel; it needs the existing stream +read for state. + +**Approach.** Derive a short status from the output already being received. + +- Recognise the phase from the stream: lines before `notmuch new` starts are + mbsync's, and `notmuch new` announces itself. Show "Syncing mail (mbsync)" + then "Reindexing (notmuch)". +- mbsync prints the channel it is working on, which is the account name the user + wants to see. Take it from the output rather than from config, so what is shown + is what is actually happening. + +**Constraints.** + +- **Sync output is untrusted-ish input.** It comes from a local script, but it is + interpolated into a status label; keep it plain text and truncate it, so a long + or hostile line cannot resize the status bar or inject markup. +- The status label is shared with transient messages, which expire (item 33). + A sync phase is not transient and must not be cleared by that timer, nor + clobber a message the user is reading. +- Do not parse the output to decide success or failure. The exit status is the + authority, deliberately (`assets/mailsync.sh:101-108`), and a second opinion + derived from text would eventually disagree with it. +- Match loosely. mbsync's exact wording varies by version, and a status line that + goes blank because a string moved is worse than the current fixed one. + +## 43. No "Mark all read" for the current view + +**Observed (user, 2026-08-05):** a "Mark All Read" button next to Sync, Archive, +Delete and Undo, applying to the current view. + +**Cause (verified in code):** no such action exists. The registered actions are +the list at `src/mainwindow.cpp:590-756`; there is `toggle_unread`, which acts on +the selection, and nothing that acts on a whole result set. + +**Approach.** The machinery is already there and this is mostly a question of +scope. `applyTagsToThreads` resolves a multi-thread selection in one combined +query, per `CLAUDE.md`, so marking many threads read is one write, not N. + +- An action removing `unread` from every thread in the current view, routed + through the same funnel, with its inverse pushed onto the undo stack as a + single command. +- Item 25 already established select-all, so "select all, then toggle unread" is + the manual route today. Decide whether this item is that, or genuinely + view-wide regardless of selection. + +**Constraints.** + +- **"The current view" is not the same as "the loaded rows".** Threads arrive in + batches and a large query may still be running, so an action taken mid-load + would silently skip whatever has not arrived. Either act on the model's rows + and say so, or wait for the query to complete. Do not describe it as "all" if + it is not. +- One undo entry for the whole operation, not one per thread. A user who marks + 400 threads read and then hits Ctrl+Z expects one press to be enough. +- No confirmation dialog, per `CLAUDE.md`, even though this touches many threads. + Undo is the answer here as everywhere else. +- The pending-edit count must move by the real number of threads changed, or the + quit prompt understates the work at risk. + +## 44. No way to manage the filters applied at sync time + +**Observed (user, 2026-08-05):** "manage filters to be applied when syncing (view +existing, edit, delete, create new, copy as new, dry-run)." + +**Unspecified, and blocked on a question the user has to answer first: there are +no such filters in this application.** Nothing in `src/` applies rules at sync +time; `MailSync` runs one configured command and shows its output, and +`assets/mailsync.sh` is mbsync plus `notmuch new` under a lock, with no rule +engine anywhere in it. + +So the item is not "expose the existing filters in the UI". It is one of: + +- **A UI over rules that live somewhere else**, e.g. the companion `mailctl` + project or a hand-written notmuch tagging script the user runs after + `notmuch new`. If those exist, this item is an editor for that file and its + shape follows that file's format. +- **A rule engine in qtmaildir**, which is a materially larger piece of work and + a change to what this application is: v1 is read-and-organize over an index + someone else fills. + +**Next step: ask the user which, and where the rules live today.** The +dry-run request is the strongest hint that they have something in mind that +already runs, since a dry run only makes sense against rules that exist. + +**Constraint if it is built here:** `CLAUDE.md` records that this project does +no network protocol work at all and that fetching is external. A filter engine +that rewrites the Maildir would not violate that literally, but it would put +qtmaildir in the business of moving mail, which is a decision to take +deliberately rather than by implementing a dialog. + +## 45. Two Sync buttons on the main window + +**Observed (user, 2026-08-05):** "there's currently 2 Sync buttons on the main +interface. UX redundant." + +**Cause (verified in code): they are two separate widgets built by two separate +passes.** `m_syncButton` is a `QPushButton` created in `buildUi()` +(`src/mainwindow.cpp:441`) and placed in the query bar row. Independently, the +`sync` action registered at `:721` appears on both the File menu and the +toolbar, from item 3's menu work. Nothing removed the original button when the +toolbar gained one, so the window shows both. + +**Approach.** Drop the standalone `QPushButton` and keep the toolbar action. +The action carries its shortcut, its enabled state and its menu entry from one +place, which is the whole point of item 3's conversion; the loose button is the +last widget that predates it. + +**Constraints.** + +- **The button is not just a button today.** It is disabled while a sync runs, + including one started externally (item 27/29), and `test_mainwindow` asserts on + it by name. Whatever replaces it has to carry that state, and the tests need + pointing at the action rather than at the widget. +- Check for other loose widgets doing the same thing before touching this one, so + the fix is not repeated per widget later. +- Removing a visible control is the kind of change that looks like a regression. + Confirm with the user which of the two survives; the note says redundant, not + which one is wanted. + ## Deferred, unsized, or split out Items noted while triaging but not part of the original list. Same numbering |