diff options
Diffstat (limited to 'docs/superpowers')
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md | 82 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 109 |
2 files changed, 156 insertions, 35 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 206a550..8092fb4 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 @@ -8313,3 +8313,85 @@ reporting 0 unfinished. **Size: S.** Done, at roughly twice the entry's scope because the entry's scope was wrong. + +## 118. No way to empty the trash from inside the app + +**Observed (user, 2026-08-17):** raised while reviewing item 103's spec, as +something that had been forgotten rather than newly noticed: "we could add +'Empty Trash' to the backlog as a future item. I forgot it existed, but I don't +want to squeeze it in this spec." + +**Blocked on 103**, which creates the trash folder this would empty. Until that +ships there is nothing to empty: Delete writes a tag and moves no file, so no +account has a populated trash folder except through another client. + +**Deliberately excluded from 103's spec**, at the user's request and recorded in +its "Out of scope" section. Worth keeping separate for a reason beyond scope +control: emptying the trash is the first action in this application that would +destroy mail with no undo. Every mutation so far is a tag or, after 103, a move, +and both are reversible. A purge is not. + +**Approach, unspecified.** The shape depends on decisions not yet made, and the +spec for 103 answers none of them: + +- **Local or remote.** Deleting the files locally and letting `Expunge Both` + carry it to the server is one thing; asking the provider to empty its own + trash is another, and mbsync offers no verb for the latter. The first is + probably what "Empty Trash" should mean here. +- **Whether the no-confirmation rule survives it.** It does not, on the face of + it. `CLAUDE.md` grants undo in place of confirmation dialogs, and this is the + action where undo cannot exist. That makes it the second item, after 103, that + re-examines the rule rather than assuming it, and unlike 103 it will probably + have to break it. +- **Per-account or all-accounts**, which should follow whatever the Trash filter + does once 103 ships rather than being decided independently. + +**Built 2026-08-25**, unblocked by 103. The three questions the entry left open +were put to the user and answered: + +- **Local, and let the sync carry it.** The files go, and a channel with + `Expunge Both` propagates that to the server. mbsync offers no verb for + asking a provider to empty its own trash, so the alternative was to delete + locally and not care, which brings the mail back on the next sync and reads + as the action having silently failed. +- **It confirms**, naming the count and the account, defaulting to Cancel, with + no default shortcut. CLAUDE.md now records this as the ONE exception to the + no-confirmation rule, in the same paragraph that states the rule, so the next + reader meets both together. +- **Scoped to the account selector**, like every other account-aware surface, + which is what the entry asked for. + +`NotmuchWorker::purgeMessages()` is a separate entry point from +`moveMessages()` rather than a flag on it, because the two look alike and only +one can be undone. It takes named ids only, never a folder sweep, so the blast +radius is what the dialog enumerated and the user confirmed. It deletes EVERY +file of a message: notmuch deduplicates by Message-ID, and leaving one behind +would leave the message alive in the folder the user emptied, which is the same +one-message-many-files property item 166 turned on. + +`resolveQueryMessages()` is a four-line wrapper over the existing private +`resolveQuery()`, so enumerating what is about to be destroyed needed no new +walk. The count in the dialog comes from the DATABASE rather than the model, +which holds whatever the current view is showing and is usually not the trash. + +**A defect surfaced while writing the tests**, and it is the one worth +remembering: the first version counted a message whose file was already gone as +destroyed, so the number reported for an irreversible action overstated it. An +absent file is correctly not an ERROR, since the index can name a path a sync +has removed; the mistake was treating "not an error" as "destroyed". The +mutation that restores it now fails. + +**A second defect was found by the user's own hand test**: the mail was +destroyed correctly and the LIST went on showing it until they re-ran the query +themselves. A purge is the one mutation with no optimistic update available, +because it removes rows rather than changing them, so `messagesPurged` re-runs +the current query. Nothing was connected to that signal at all, which is the +kind of gap a green suite is happy to keep. + +Verified against the live index after the user emptied one real account's +trash: zero files on disk, zero in the index. + +**Item 168 was filed from the same hand test**, on Delete being offered on mail +already in the trash. + +**Size: S.** Done. 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 ab346f5..37dc190 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 @@ -184,7 +184,7 @@ taking that too literally. | 115 | A copy from the message pane gives no confirmation | presentation | XS | **done** 2026-08-19, unreleased. Four entries report, each naming what it copied; connected to the page's own QActions, so the entry is covered wherever it is triggered from | | 116 | Copy image copies markup instead of the image | defect | XS | **dropped** 2026-08-17, same day. NOT A DEFECT: `wl-paste --list-types` run immediately after a copy reports `image/png`, `application/x-qt-image` and 30 more image flavours. The clipboard is correct and Chromium is behaving. The earlier "text only" reading was taken minutes late off a clipboard that had been overwritten, and a whole cause was theorised on it | | 117 | The message pane offers no Select all | workflow | XS | **done** 2026-08-19, unreleased. `addPaneActions()` supplies it. The call site is NOT covered by a test and cannot be: the production menu needs a real context-menu event. Stated in the test rather than faked | -| 118 | No way to empty the trash from inside the app | workflow | S | open, 2026-08-17. **Blocked on 103**, which creates the trash in the first place. Deliberately left out of 103's spec at the user's request rather than squeezed in | +| 118 | No way to empty the trash from inside the app | workflow | S | **done 2026-08-25**, unreleased. Unblocked by 103. `Message > Empty trash...`, scoped to the account selector, no shortcut. The one confirmation in this application, and CLAUDE.md now records it as the single exception rather than leaving it to be discovered. Found a defect while testing: the count claimed messages whose files were already gone | | 119 | The unsynced-changes count cannot be opened to see what it counts | information | S | open, 2026-08-19, from the notes. One of the four things it sums carries no message ids at all, so a list cannot be complete without a change to how the count is kept | | 121 | The thread list shows nothing while a query is running | feedback | S | open, 2026-08-20, from the notes. Follows item 74, which fixed the status-bar half and left the list itself blank | @@ -241,6 +241,7 @@ taking that too literally. | 165 | A draft gets a new Message-ID on every autosave | enhancement | ? | open, 2026-08-25, found while hand-testing 163 and 164. `MessageBuilder::build()` generates an id unconditionally and every autosave calls it, so each revision is a distinct MESSAGE to notmuch and to the server rather than a new version of one. Invisible while the file is replaced correctly, which item 163's fix restores; it is what turned that fork into two messages rather than one duplicated file. Needs a DECISION on what a draft's identity is before any code: a stable id reused at send, a stable id discarded at send, or the status quo. Neither `ComposeContext` nor `OutgoingMessage` has a field to carry an id, so it is not a changed call site | | 166 | Mail you send to your own other account loses `inbox` | defect | S | **done 2026-08-25**, unreleased. `sent_only()` keeps a message only when EVERY file is inside a sent folder, which is what the carve-out's docstring already claimed. No query can express it, measured; the root comes from `database.mail_root`, with a split-index fixture the ordinary layout cannot provide. Verified read-only against the live index: 780 of 807 still stripped, 27 spared, no arrival affected | | 167 | No way to tell one build of an unreleased version from another | enhancement | XS | **done 2026-08-25**, unreleased. The user chose a counter over a git description: `QTMAILDIR_BUILD_NUMBER`, a cmake option ON by default, increments a counter in the BUILD directory on every build and writes `buildnumber.h`. `QTMAILDIR_VERSION_DISPLAY` carries it; `QTMAILDIR_VERSION` stays clean and is what the window title, `applicationVersion` and the release procedure use | +| 168 | Delete is offered on mail already in the trash, and does nothing | defect | S | open, found 2026-08-25 by the user while hand-testing 118. `moveMessages()` takes its already-in-destination branch, reports the message as moved and counts an unsynced change for a move that never happened. Not dangerous, it purges nothing. Restore has the mirror of the same problem: it is offered on mail that was never deleted. The user chose to hide each where it has no meaning, the same principle as 112's hidden unread entry. **Carries a second request**: Delete should also drop `unread` | Sizes are rough: XS under an hour, S a sitting, M a session. @@ -672,40 +673,6 @@ make Save image work must not make Save link reachable again. The test fails if it does, which is the point: the handler is per-profile, so the natural implementation would light up both entries at once. -## 118. No way to empty the trash from inside the app - -**Observed (user, 2026-08-17):** raised while reviewing item 103's spec, as -something that had been forgotten rather than newly noticed: "we could add -'Empty Trash' to the backlog as a future item. I forgot it existed, but I don't -want to squeeze it in this spec." - -**Blocked on 103**, which creates the trash folder this would empty. Until that -ships there is nothing to empty: Delete writes a tag and moves no file, so no -account has a populated trash folder except through another client. - -**Deliberately excluded from 103's spec**, at the user's request and recorded in -its "Out of scope" section. Worth keeping separate for a reason beyond scope -control: emptying the trash is the first action in this application that would -destroy mail with no undo. Every mutation so far is a tag or, after 103, a move, -and both are reversible. A purge is not. - -**Approach, unspecified.** The shape depends on decisions not yet made, and the -spec for 103 answers none of them: - -- **Local or remote.** Deleting the files locally and letting `Expunge Both` - carry it to the server is one thing; asking the provider to empty its own - trash is another, and mbsync offers no verb for the latter. The first is - probably what "Empty Trash" should mean here. -- **Whether the no-confirmation rule survives it.** It does not, on the face of - it. `CLAUDE.md` grants undo in place of confirmation dialogs, and this is the - action where undo cannot exist. That makes it the second item, after 103, that - re-examines the rule rather than assuming it, and unlike 103 it will probably - have to break it. -- **Per-account or all-accounts**, which should follow whatever the Trash filter - does once 103 ships rather than being decided independently. - -**Size: S**, provisionally, and not worth sizing properly until 103 exists. - ## 119. The unsynced-changes count cannot be opened to see what it counts **Observed (user, from the notes):** "the bottom left statusbar message needs to @@ -1380,3 +1347,75 @@ id and a draft of a reply carries both. - Item 163's fix stands on its own and this does not block it: the file is replaced correctly now, so the fork this would have mitigated no longer happens by that route. + +## 168. Delete is offered on mail already in the trash, and does nothing + +**Observed (user, 2026-08-25, while hand-testing item 118):** "I noticed I can +hit delete via context menu on a message already in the trash. Seems like a +bug, unless that action doesn't do for one message what Empty trash does for +the whole view." + +It does not, and the guess in the second half is worth recording as the reason +this matters: the user's mental model was that Delete on already-trashed mail +might PURGE it. It does not, and nothing about the menu says so. + +**Cause (verified in code, 2026-08-25.)** `moveMessages()` compares the file's +directory against the destination and takes an early-return branch when they +match (`notmuchworker.cpp`, the "already where it was asked to go" branch, +added when a fresh Maildir name made a path comparison useless). That branch +appends the id to `moved` and records an origin, so the message is reported as +having moved when nothing happened. The UI counts an unsynced change for it. + +Nothing is destroyed and nothing is corrupted; the cost is a menu entry that +lies about having done something, and a pending-changes count that overstates +what a sync has to carry. + +**The mirror of the same defect is already shipped beside it.** `restore` is +added unconditionally to both the Message menu (`mainwindow.cpp:1956`) and the +thread context menu (`mainwindow.cpp:2119`), so it is offered on mail that was +never deleted, where it has as little meaning as Delete has in the trash. + +**Approach.** The user chose to hide each action where it has no meaning, +which is the principle item 112 established for the unread entry: an action +with no honest meaning for the selection is absent rather than present and +inert. + +- Delete is hidden when every selected row is already in a trash folder. +- Restore is hidden when no selected row is. +- The test for both needs a MIXED selection as well as uniform ones, for the + reason item 112 records: a selection whose rows agree answers identically + whichever way the predicate is computed. + +**Constraints.** + +- **The question is about the PATH, not the tag.** A message trashed by + another client carries no `deleted` tag at all, which is why item 103 made + the trash view path-based. Asking `tags.contains("deleted")` here would + offer Delete on exactly the mail the user is most likely to be looking at + in a trash view. +- **`selectionTagPresence()` is the wrong instrument** for the same reason, + though it is the right shape. A path predicate needs the row's path, which + `MessageNode` carries. +- Deciding this does not require deciding item 118's relationship to it: a + purge stays an explicit whole-view action, and hiding Delete does not make + Delete a purge. + +**A second request, from the same tangent (user, 2026-08-25):** "messages moved +to the trash should be automatically marked `-unread`." Deleting is a decision +about the message, so leaving it bold and unread in the trash is noise; the +count of unread mail should not include what the user threw away. + +It is one line where Delete already composes its tag change, and it carries a +constraint worth stating rather than discovering. `maildir.synchronize_flags` +is true, so removing `unread` REWRITES the Maildir filename and reaches the +server on the next mbsync. That is acceptable here and is a deliberate +exception: it is the same mechanism the `post-new` hook refuses to touch on +arriving mail, for the good reason that the hook acts unattended on mail the +user has not seen. A Delete is an explicit gesture on a message in front of +them, which is the difference. + +Undo must put it back. `TagChange::inverted()` already does, provided the +removal travels as part of the SAME change rather than as a second write, so +one undo returns both the folder and the tag. + +**Size: S** for the visibility half, XS for the `unread` half. |
