From 46c82a09903730133b06e28140d29f16b68813f4 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Wed, 5 Aug 2026 12:20:02 +0200 Subject: docs: reconcile backlog with usage notes, add items 39-45 The notes at the user's end are the upstream source and had drifted seven entries ahead of this document. Each new item's cause is verified against the code rather than restated from the note. Two are worth separating from the rest: - 41 is a defect. collectParts() files any part carrying a Content-Id into inlineParts and returns before the text/plain and text/html branches, so a message whose HTML body part has a content id parses with both body slots empty. That is one cause for both halves of the observation, a blank pane and "no HTML part". - 44 cannot be planned yet. It asks to manage filters applied at sync time, and no such filters exist here: MailSync runs one command and the script is mbsync plus notmuch new. The item needs the user to say where their rules live before it has a shape. Items 37 and 38 are deliberately left out of this commit; they document work that is built but not yet hand-tested, and belong with that code. --- .../plans/2026-08-03-post-0.1.0-usability.md | 265 ++++++++++++++++++++- 1 file changed, 263 insertions(+), 2 deletions(-) (limited to 'docs/superpowers/plans') 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 -- cgit v1.2.3