diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-04 19:07:07 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-04 19:07:07 +0200 |
| commit | b34b18c3a6288031c4eed1687f8e53487c3af56f (patch) | |
| tree | 7d6e8087ac3bbc733b313fb1910d51d3114c8402 | |
| parent | a2af17fe440fa7bf725b83c548396e0b47238f10 (diff) | |
| download | qtmaildir-b34b18c3a6288031c4eed1687f8e53487c3af56f.tar.gz qtmaildir-b34b18c3a6288031c4eed1687f8e53487c3af56f.zip | |
docs: record items 28-35 from the user's notes, and how to keep them in sync
The backlog is downstream of the user's own notes, which they add to while
using the app. Comparing the two turned up nine entries with no item here,
two of them defects rather than enhancements:
- Re-adding `unread` after the automatic mark-read counts 2 unsynced
changes when the mail store is back where it started. The counter
counts writes and never decrements, so any add-then-remove of the
same tag inflates it (item 28).
- The Sync button stays enabled while a background sync holds the lock.
That was a written constraint of item 27 and shipped unbuilt, which
is the strongest argument for doing this comparison at all (item 29).
The rest are enhancements: a useful blank right pane, for which the user
has produced the two logo images added here; a default button on the quit
prompt, recorded as needing a repro because the code does set one; Esc to
blank the pane; expiring status messages; a Maildir overview; and an
automatic refresh after a sync.
Item 35 is the one 0.8.0 deliberately did not build. A background sync
reports rather than refreshes, because runCurrentQuery() clears the undo
stack, the selection and the message pane. A non-destructive refresh is
real work, so it is an item rather than a flag.
Item 27 gains its outcome, including that its own proposed approach was
the wrong one: flock -n acquires in order to test, so polling with it
would have caused the exact skips the sync script reports.
CLAUDE.md now carries the reconciliation step, since a note saying "X is
broken" is a bug report that will sit in a personal file indefinitely
unless someone goes looking.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
| -rw-r--r-- | CLAUDE.md | 20 | ||||
| -rw-r--r-- | assets/images/qtmaildir-dark.png | bin | 0 -> 61700 bytes | |||
| -rw-r--r-- | assets/images/qtmaildir-light.png | bin | 0 -> 87063 bytes | |||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 332 |
4 files changed, 351 insertions, 1 deletions
@@ -169,6 +169,26 @@ loosen any of these without an explicit decision. against the chosen directory, and refuse anything escaping it. Compare resolved paths as paths, not with `startsWith` — `/tmp/safe-evil` passes a `startsWith("/tmp/safe")` check. +## At the start of a session: reconcile the backlog with the user's notes + +The backlog at `docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md` is +**downstream** of the user's own notes at +`~/Documents/Obsidian/note/notes on qtmaildir.md`. The user writes to those notes +whenever they use the application and hit something, so the backlog goes stale on +its own between sessions. + +**Read both and diff them before picking up work.** Anything in the notes with no +item in the backlog gets appended with the next free number, in the backlog's +own format (Observed / Cause / Approach / Constraints), with the cause **verified +in the code, not copied from the note**. The two documents are numbered +independently and drifted long ago; never renumber to reconcile them. + +This is not busywork. The 2026-08-04 pass found nine unrecorded entries, two of +them defects rather than enhancements, and one of those was a constraint this +backlog had already specified and that shipped unbuilt (item 29). A note saying +"X does not work" is a bug report, and it will sit in a personal notes file +indefinitely unless someone goes looking. + ## Working on this repo Implementation follows `docs/superpowers/plans/2026-08-02-qtmaildir-v1.md` (14 tasks) diff --git a/assets/images/qtmaildir-dark.png b/assets/images/qtmaildir-dark.png Binary files differnew file mode 100644 index 0000000..d380a75 --- /dev/null +++ b/assets/images/qtmaildir-dark.png diff --git a/assets/images/qtmaildir-light.png b/assets/images/qtmaildir-light.png Binary files differnew file mode 100644 index 0000000..43af85d --- /dev/null +++ b/assets/images/qtmaildir-light.png 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 277283e..4dab8a9 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 @@ -13,6 +13,12 @@ those notes called 12 is item 13 here, and item 14 here (the tag column) was never in them at all. Items 15 to 17 come from a later pass over the same 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`. + 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 that is dropped stays in the table marked `dropped` with a one-line reason. @@ -65,7 +71,15 @@ taking that too literally. | 24 | No right-click actions on the thread list | discoverability | S | open | | 25 | No select-all, and bulk actions are undiscoverable | workflow | S | open | | 26 | No way to add or remove an arbitrary tag from the UI | workflow | S | **done** | -| 27 | The UI cannot see a sync it did not start | feedback | S | open | +| 27 | The UI cannot see a sync it did not start | feedback | S | **done** | +| 28 | Re-adding `unread` counts 2 unsynced changes, not 0 | correctness | S | open | +| 29 | Sync button stays enabled during a background sync | feedback | XS | open | +| 30 | The blank right pane is wasted space | presentation | M | open | +| 31 | The quit prompt has no highlighted default button | discoverability | XS | open, needs repro | +| 32 | Esc does not blank the right pane | workflow | XS | open | +| 33 | Status bar messages never expire | feedback | S | open | +| 34 | No overview of the Maildir itself | information | M | open | +| 35 | No refresh of the thread list after a sync | workflow | M | open | Sizes are rough: XS under an hour, S a sitting, M a session. @@ -1260,8 +1274,324 @@ project has been bitten by twice:** Sync button should be disabled while the lock is held, since starting one would only produce the `EX_TEMPFAIL` skip. +### Outcome (done, 0.8.0) + +Both probes the item demanded were run before any code, and one of them changed +the design. **The lock is observable, but only through `/proc/locks`**, and two +of the three plausible ways to read it are wrong: + +- **`flock -n`, which this item proposed, is the one that must not be used.** + It acquires in order to test, so polling every two seconds opens a window + every two seconds in which a starting `mailsync.sh` is refused the lock and + exits 75. It would cause the very skips the script reports. The item's own + approach section specified this; it was wrong. +- `fcntl(F_OFD_GETLK)` never acquires and looks ideal, but reports UNLOCKED + against a lock held by `flock(2)`: separate lock namespaces in the kernel, + which cannot see each other. A silent false negative. +- `/proc/locks` is a pure read, matched by inode. It observes `flock(2)` + correctly and takes no lock, which also answers the second probe: 200 reads + left the lock table unchanged and the polling process holding nothing, so it + cannot contend with the Xapian write lock `notmuch new` holds in the same run. + +`SyncMonitor` keeps the parsing separate from the polling so the parsing is +testable, and reports **Unknown** rather than Idle where `/proc/locks` cannot be +read. "No sync is running" is the claim that would let the window quit, so it is +never guessed. + +**It reports rather than refreshes**, which is narrower than the item implies. +`runCurrentQuery()` clears the undo stack, the selection and the message pane: +right for a query the user typed, hostile for one a cron timer fired six times +an hour. The status bar says a background sync finished and suggests pressing +Enter. See item 35 for the non-destructive refresh this defers. + +**One constraint above was not built: the Sync button is still enabled while an +external sync holds the lock.** That is item 29, split out rather than left +buried here. + +**A defect found by hand testing, after the feature was believed done.** A sync +this window started reported itself as a background one, a moment after it +finished. Ownership of a lock period was decided when the lock was RELEASED, by +asking `MailSync::isRunning()`, but the process exits before the next poll sees +the lock gone, so the answer was always "not ours". Ownership is now latched +when the lock appears, and handed back on exit 75, since a skip means the lock +was never ours. + +**That fix has no regression test, deliberately recorded.** Reproducing it needs +`isRunning()` true at one transition and false at the next, which needs a +configured sync command and a live child process; that attempt left a process +running for the length of the suite and popped a dialog on the user's screen. It +was verified against a standalone model of both code paths instead. A real test +needs the notmuch fixture. + --- +## 28. Re-adding `unread` counts 2 unsynced changes, not 0 + +**Observed (user, 2026-08-04):** open a thread, wait the two seconds for the +automatic mark-read, then press **Ctrl+U** to put `unread` back. The indicator +reads **2 unsynced changes**. The mail store is in exactly the state it started +in, so the honest answer is 0. + +**Cause: the counter counts writes, not net state.** `onTagsApplied()` does a +bare `++m_pendingEdits` (`src/mainwindow.cpp:1293`) for every confirmed +mutation, and nothing ever decrements. The automatic mark-read is one confirmed +write, the manual toggle back is a second, so a round trip that cancels itself +out reads as two outstanding changes. + +This is not specific to mark-read. Any add-then-remove of the same tag inflates +it the same way: archive then undo, flag then unflag. Mark-read is simply the +one path that fires without the user asking, which is why it surfaced here. + +**The design question this item has to answer first.** "Unsynced changes" can +mean two different things and the fix depends on which: + +- *Writes the index has taken that a sync has not carried over.* Two writes did + happen, notmuch's database did change twice, and `notmuch new` will still + visit those messages. On this reading 2 is correct and the wording is what is + wrong. +- *Net difference between the index and the mail store.* On this reading the + answer is 0 and the counter needs to track state, not events. + +The user's expectation is clearly the second. Note that item 18 chose the first +deliberately: it counts at the point a write is **confirmed**, precisely so an +optimistic update the worker rejects is not counted. That reasoning stays valid; +what it did not consider is a write that undoes an earlier one. + +**Approach, if net state is the answer.** Track the set of (thread, tag) +changes rather than a count, and collapse a pair that cancels. `TagChange` +already carries add and remove lists and already knows how to invert itself +(`TagChange::inverted()`, used by the undo stack), so the machinery for +recognising an inverse exists. + +**Constraints.** + +- **A sync must still reset it to zero**, and a failed sync must still not, per + item 18. Whatever replaces the counter keeps both properties. +- The indicator is a lower bound on confidence, not a guarantee: an external + `notmuch new` can carry changes across without this window knowing. Do not let + a more precise counter imply more certainty than it has. +- Do not fix this by not counting the automatic mark-read. It is a real write to + the index, and hiding it would make the count wrong in the other direction. + +## 29. Sync button stays enabled during a background sync + +**Observed (user, 2026-08-04):** while a cron sync runs, the Sync button is +still clickable. Pressing it starts a run that can only be refused. + +**Cause: a constraint of item 27 that was specified and then not built.** That +item states plainly that "the Sync button should be disabled while the lock is +held, since starting one would only produce the `EX_TEMPFAIL` skip". +`setSyncBusy()` (`src/mainwindow.cpp:1357`) is the only thing that touches +`m_syncButton->setEnabled()`, and it is called only from this window's own sync +path. `onExternalSyncStateChanged()` shows the progress bar and writes the +status text but never touches the button. + +**Approach.** Have the external handler drive the same enable/disable that +`setSyncBusy()` does, rather than duplicating the rule. The two paths already +share the progress bar; the button is the piece that was missed. + +**Constraints.** + +- **Re-enable on `Unknown`, not only on `Idle`.** Where `/proc/locks` cannot be + read the monitor claims nothing, and a button left permanently disabled on a + platform that cannot observe the lock is worse than one that occasionally + offers a run that gets skipped. +- The button must not end up enabled during this window's own sync because the + external handler ran last. Both paths write the same widget, so whichever + fires second wins; make the rule a single function of both states rather than + two independent assignments. +- Exit 75 handling stays. Disabling the button makes the skip rarer, not + impossible: cron can take the lock between the poll and the click. + +## 30. The blank right pane is wasted space + +**Observed (user, 2026-08-04):** with no thread selected the message pane is +empty. The user wants it to carry something useful, and named three things: the +application logo, tips and tricks, and messages about the current action, giving +"N messages selected" as the example. + +**Assets exist.** The user has produced `assets/images/qtmaildir-light.png` and +`qtmaildir-dark.png` (924x540 each) for this item. + +**Cause:** `MessageView::clear()` leaves the web view showing nothing. Since +0.8.0 the pane is also blanked deliberately whenever more than one thread is +selected, which makes this item more visible than it was: multi-select is now a +routine gesture that produces an empty pane every time. + +**Approach.** A placeholder document rendered into the existing web view when +there is nothing to show, rather than a second widget stacked behind it. +`HtmlBuilder` already produces the pane's HTML and `MessageView` already knows +how to hand it a document, so the placeholder is a third document shape +alongside "one message" and "a thread". + +**The selection-count half is the one with real value**, and it pairs with the +status-bar count added in 0.8.0. The pane already blanks on a multi-row +selection; saying "3 threads selected" there answers the question the blank +raises. + +**Constraints.** + +- **Two images, because the pane must follow the desktop theme.** Item 5 already + records the rule against hardcoded colours, and a light-theme logo on a dark + desktop is exactly that fault in image form. Pick by palette, not by guessing. +- **Do not load the logo over `file:`.** The web view runs with + `LocalContentCanAccessFileUrls` false and an interceptor that blocks every + request by default, per `CLAUDE.md`. The image has to arrive as a `data:` URI + or through the existing `qtmaildir:` scheme handler, and the interceptor's + document-URL exemption must still match exactly. +- Tips and tricks is the weakest of the three and should be built last, if at + all. A tip nobody can dismiss becomes noise on the hundredth launch. +- The placeholder must not appear between a selection and its render, or every + thread open flashes a logo first. + +## 31. The quit prompt has no highlighted default button + +**Observed (user, 2026-08-04):** "quit popup has no Predefined answer (there's +no highlighted button)." + +**Needs a repro before any change: the code says otherwise.** The three-button +prompt sets one explicitly, `box.setDefaultButton(sync)` +(`src/mainwindow.cpp:191`), and the no-sync-command variant passes +`QMessageBox::Cancel` as its default argument (`:167`). Both should render a +highlighted button. + +Possible explanations, in the order worth checking: + +- The dialog the user saw was neither of those. There are five other + `QMessageBox` calls in `MainWindow` (`:205`, `:224`, `:991`, `:1260`, + `:1277`), and the two sync-failure ones appear on the exit path, so a failed + exit-sync shows a second dialog immediately after the first. +- The default is set but the style draws no visible focus ring, which is a + platform theme question rather than a code one. +- `setDefaultButton()` is being overridden by the button roles: a + `DestructiveRole` button can take precedence in some styles. + +**Approach: reproduce first, and record which dialog.** If it is one of the +sync-failure boxes, the fix is to give those an explicit default. If it is a +theme issue the item becomes a documentation note rather than a change. + +**Constraint:** whatever default is chosen must be the safe one. On a prompt +about losing unsynced work, Enter must not fall on "Quit anyway". + +## 32. Esc does not blank the right pane + +**Observed (user, 2026-08-04):** "Esc in the main window should blank the right +pane." + +**Cause:** nothing binds Escape at window level. `Key_Escape` appears once in +`src/`, in `QueryCompleter` (`src/querycompleter.cpp:474`), where it dismisses +the completion popup. The main window has no handler. + +**Approach.** A registered action like any other, so it reaches the menus, the +shortcut reference and `[keys]`, calling the same `MessageView::clear()` the +multi-select path already uses. `m_currentThreadId` must be cleared with it, or +a late-arriving `threadLoaded` will paint the thread straight back, which is the +race documented in `CLAUDE.md` and fixed in 0.8.0. + +**Constraints.** + +- **Escape must not be stolen from the completer.** A window-level shortcut + outranks the focused widget, which is exactly how `Return` broke for the query + bar and needed an event filter to claim back (item 21 records this). Verify + the popup still dismisses before shipping. +- Blanking is a view change, not a mail change: it must not clear the selection, + the query, or the undo stack. +- Decide what Escape does when the pane is already blank. Doing nothing is fine; + clearing the selection as a second step would be surprising. + +## 33. Status bar messages never expire + +**Observed (user, 2026-08-04):** "the status bar should return to default status +after showing a message for N seconds." + +**Cause:** every message is written with a bare `m_statusLabel->setText()`, +eighteen call sites in `MainWindow`, and nothing ever clears one. Whatever was +written last stays until something else overwrites it, so a transient message +like "Sync complete" persists as though it described the current state. + +**Approach.** Route transient messages through one helper that sets the text and +arms a single-shot timer to restore a default, rather than adding a timer per +call site. `QStatusBar::showMessage()` already implements exactly this with a +timeout argument, and the label is a custom widget added with `addWidget()` +rather than the status bar's own message area, so switching to it is worth +considering before writing a bespoke timer. + +**The item's real question is what "default" means.** Candidates: the thread +count from the last query (`onQueryFinished` already writes this), or empty. +The count is more useful and is what the user already sees after a query +completes. + +**Constraints.** + +- **Not every message is transient.** The selection count added in 0.8.0 + describes current state and must persist while the selection does; expiring it + would be a regression. Distinguish state from events rather than putting a + timeout on everything. +- The 0.8.0 selection-count code already takes back only a message it wrote + itself, comparing against `m_selectionMessage`. Any general mechanism should + generalise that rather than defeat it. +- An error must not vanish before it is read. Sync failures already open the log + pane, which does persist, but the status text should outlast a two-second + timeout. + +## 34. No overview of the Maildir itself + +**Observed (user, 2026-08-04):** wants "info on the maildir": total messages, +number of accounts, and possibly more. + +**Cause:** nothing in the UI reports database-level facts. Every query returns a +thread count for that query (`onQueryFinished`), but there is no path that asks +notmuch about the database as a whole. + +**Approach.** A dialog, reached from Help or File, showing what notmuch can +answer cheaply. `notmuch_database_get_all_tags()` is already wired for the +completer (item 17), so the tag count is free. A total message count needs a new +worker call. + +**Constraints.** + +- **This is a worker query like any other.** No `notmuch_*` pointer crosses the + thread boundary; the result comes back as plain values, per `CLAUDE.md`. +- **The account count comes from config, not from notmuch.** notmuch does not + model accounts at all, which is why per-account subdirectories are configured + in the first place. Do not try to derive it from the database. +- Counting every message in a large database is not free. Measure before putting + it somewhere that opens on every launch; a dialog the user asks for is the + right shape, a status-bar field refreshed continuously is not. + +## 35. No refresh of the thread list after a sync + +**Observed (user, 2026-08-04):** "auto refresh list after sync." + +**Cause: half of this works and the other half was deliberately not built.** A +sync this window starts already refreshes: `onSyncFinished()` calls +`runCurrentQuery()` on success. A background sync does not, and 0.8.0 chose that +on purpose, because `runCurrentQuery()` clears the undo stack +(`src/mainwindow.cpp`, the query-start path), the selection and the message +pane. Firing it when a cron timer finishes would discard undo history and close +the thread being read, up to six times an hour, with no action from the user. +The status bar suggests pressing Enter instead. + +**So the work is a non-destructive refresh**, not a call to the existing one. +That is a real piece of work and is why this is its own item rather than a flag. + +**Approach.** Re-run the query and reconcile the result against the current +model instead of clearing it: keep rows that are still present, add new ones, +remove the gone. The generation counter already distinguishes a stale result +from a current one, so the plumbing for a second concurrent query exists. + +**Constraints.** + +- **The undo stack is cleared on query for a real reason.** Its entries refer to + rows the new result set discards, and a stale entry inverts into a model + update that does nothing while the database change still happens, leaving undo + half-applied. A refresh that keeps the stack has to keep those references + valid, which is the hard part of this item and the reason it is sized M. +- The selected thread must stay selected and stay open if it survives the + refresh. Losing your place is the failure this item exists to avoid. +- Scroll position likewise. +- A refresh must not re-trigger mark-read for the thread already on screen. + ## Deferred, unsized, or split out Items noted while triaging but not part of the original list. Same numbering |
