aboutsummaryrefslogtreecommitdiffstats
path: root/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-25 19:19:21 +0200
committerDanilo M. <danix@danix.xyz>2026-08-25 19:19:21 +0200
commitdd6e7a051c22e5145c1b23da0614179aef2e1379 (patch)
tree5fd840216d5ae06715e7ca4116c6cea603a7a232 /docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md
parentdfb36213c03e678178ec1abb2327266d23015bc0 (diff)
downloadqtmaildir-dd6e7a051c22e5145c1b23da0614179aef2e1379.tar.gz
qtmaildir-dd6e7a051c22e5145c1b23da0614179aef2e1379.zip
fix: offer Delete and Restore only where they mean something
Item 168, found by the user while hand-testing 118: Delete could be triggered on a message already in the trash. Not dangerous, which is how it survived. moveMessages() finds the file already in the destination and takes its early-return branch, so the message is reported as moved, an unsynced change is counted, and nothing happened. Restore had the mirror of the same problem, added unconditionally to both menus and so offered on mail that was never deleted. Each is now hidden where it has no meaning, which is the rule item 112 established for the unread entry. The question is about the PATH, never the deleted tag: a message trashed by another client carries no such tag, which is why the trash view is path-based, and asking the tag would hide Delete on exactly the mail a trash view is full of. Delete also removes unread now, at the user's request on the same tangent. It travels inside the same sendMove() call rather than as a second write, so one undo returns the folder and the tag together. This rewrites the Maildir filename, because maildir.synchronize_flags is true, and so reaches the server: the same mechanism the post-new hook refuses to touch, and the difference is that the hook acts unattended on arriving mail while this is an explicit gesture on a message in front of the user. A mutation survived the first round and found a real hole: comparing the prefix without its trailing separator passed every test, because no fixture had a folder whose name starts with the trash folder's. Under it Delete silently vanished from mail in acct/trash-old, which is not the trash. The fixture carries that row now and all three properties are mutation-checked. The suite is 37 of 38, the failure being item 136 on an unrelated path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HFuRPtzFrSxCQjFk6tq7gD
Diffstat (limited to 'docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md')
-rw-r--r--docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md96
1 files changed, 96 insertions, 0 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.