diff options
Diffstat (limited to 'docs/superpowers/plans')
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md | 96 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 74 |
2 files changed, 97 insertions, 73 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 8092fb4..1208c42 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 @@ -8395,3 +8395,99 @@ trash: zero files on disk, zero in the index. already in the trash. **Size: S.** Done. + +## 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. + +**Built 2026-08-25**, both halves, to the user's own choice of "hide each +where it has no meaning". + +`everySelectedRowIsInATrashFolder()` asks each row about its own file, a reply +row's message and a thread row's displayed message, the same rule +`everySelectedRowHasTag()` follows. `refreshTrashActions()` runs beside +`refreshUnreadAction()` on both the selection change and the model's +`dataChanged`, so the entries follow a write as well as a selection. + +The `unread` removal travels inside the SAME `sendMove()` call rather than as a +second write, which is what makes one undo return the folder and the tag +together. + +**A mutation survived the first round and is worth recording**: comparing the +prefix WITHOUT its trailing separator passed every test, because no fixture had +a folder whose name starts with the trash folder's. `acct/trash-old` is a +different folder, and under that mutation Delete silently disappeared from mail +that had never been trashed, which is the quiet half of the same mistake. The +fixture carries that row now and the mutation fails. + +All three properties are mutation-checked: the separator, Restore's visibility, +and the `unread` removal. The suite is 37 of 38, the failure being item 136 on +an unrelated path, and no new user-facing strings were added. + +**Size: S** for the visibility half, XS for the `unread` half. 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 37dc190..437ceda 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 @@ -241,7 +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` | +| 168 | Delete is offered on mail already in the trash, and does nothing | defect | S | **done 2026-08-25**, unreleased. Delete is hidden when every selected row is already in its account's trash, Restore when none is, both keyed on the PATH rather than the `deleted` tag. Delete also drops `unread` now, in the same TagChange so one undo returns the folder and the tag together | Sizes are rough: XS under an hour, S a sitting, M a session. @@ -1347,75 +1347,3 @@ 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. |