aboutsummaryrefslogtreecommitdiffstats
path: root/docs/superpowers
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-09-14 13:46:33 +0200
committerDanilo M. <danix@danix.xyz>2026-09-14 13:46:33 +0200
commit9fdac493c9a6e35f36dc45ff0e13583ddf88605e (patch)
treeff561fba18bd543306b3166c8eb3591cb1d220e3 /docs/superpowers
parentd714483b6027425923340d1bcfa0263b8e7ac0bc (diff)
downloadqtmaildir-9fdac493c9a6e35f36dc45ff0e13583ddf88605e.tar.gz
qtmaildir-9fdac493c9a6e35f36dc45ff0e13583ddf88605e.zip
feat: a Not spam action
Backlog item 201. A message in the Spam view had no way back out: spam is one-way and Restore is hidden outside the trash. not_spam moves each message to the folder its moved-from: origin names, falling back to the account's inbox (reported) for provider-caught mail with no origin. The action is labelled "Not junk" on the free Alt+J: every letter of "Not spam" is taken in the Message menu, and Alt+P (Re&ply) and Alt+S (Mark &spam, frozen) are unavailable. restoreResolvedMessages() and the undelete_thread branch are parameterised with the cleared tag and undo description rather than copied, so Delete and Not spam cannot drift.
Diffstat (limited to 'docs/superpowers')
-rw-r--r--docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md66
-rw-r--r--docs/superpowers/plans/2026-09-13-spam-view.md36
2 files changed, 99 insertions, 3 deletions
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 6792353..9d3979f 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
@@ -274,6 +274,7 @@ taking that too literally.
| 199 | The window chrome uses the system icon theme, and the user wants a shipped set | presentation | M-L | open, 2026-09-13, from the notes ("we should ship our own icons, color themeable to be consistent in every theme a user may implement, since icons are a brand identity"). This deliberately REVERSES item 70, which drew the split as "panes are ours, chrome is the system's" and shipped `Marks` for the panes only; the note asks for the other half too, so it is a decision to revisit rather than a defect. Verified: the `themeIcons` table at `mainwindow.cpp:2211` and six `QIcon::fromTheme` sites in `composewindow.cpp` are every chrome icon, all resolved from the desktop theme. The mechanism already exists and is proven, `Marks::pixmap` compositing `fill="currentColor"` with `CompositionMode_SourceIn` so one asset serves a light and a dark palette, and `src/marks.h` records why it is compiled-in string literals rather than a `.qrc`. The size is the ARTWORK, not the code: item 70's six marks are shipped, this is roughly forty actions, each needing a drawing. Needs a decision from the user on scope before it can be sized honestly, and on whether the system theme stays as a fallback for an action with no shipped icon |
| 200 | qtmaildir cannot be launched at a given account, thread or message | workflow | M | open, **specified 2026-09-13** in `specs/2026-09-13-cli-selectors-design.md`; read that rather than this row. The user settled three things: a second launch STEERS the running window over a `QLocalServer` rather than opening a second one, the selectors are `--account`/`--thread`/`--message` (`--query` dropped as the one with no caller), and a selector matching nothing opens the window normally and says so in the status bar. The design shrank on one side and grew on the other: `recoverStaleThread()` already runs `thread:<id>` with a deferred selection and is reused as a third caller, so the selectors are the small half, while the socket (connect-first ordering, stale-socket recovery, a degrade path when no socket is possible) is the real work and adds `Qt6::Network` to the component list. Original entry: open, 2026-09-13, from the notes ("the program should accept cli parameters like `--account` or `--thread`/`--message`, so that another app can launch qtmaildir opening that account's inbox or a certain message/thread"). Verified: `main.cpp:38-66` hand-rolls a `strcmp` loop over `argv` for `--version` and `--help` only, both answering before `QApplication` exists, which is deliberate and documented. Parsing is the small half and `QCommandLineParser` covers it; the item is bigger than it looks for two reasons. There is NO single-instance mechanism (no `QLocalServer` anywhere in `src/`), so a second launch opens a second window against the same notmuch database rather than steering the running one, and notmuch permits only one open handle per process. And the selector has to reach a query the startup path does not currently take, since `--thread` names a row that may not be in the configured startup view at all. Needs a decision from the user first: whether a second launch should focus the running window (which is the useful behaviour for "another app launches qtmaildir" and is the whole cost of the item) or simply start with a different query |
| 197 | No way to say a message is not spam | workflow | S | open, 2026-09-10, split out of the 187 design at the user's decision rather than built into it. Restore already covers what qtmaildir moved: a message it marked carries `moved-from:` and goes back where it came from. The gap is mail the PROVIDER's filter caught, which was never in an inbox and carries no origin tag, so "not spam" has no recorded destination to return it to. Needs two answers before it can be planned: where such a message goes (the account's inbox is the obvious guess and is a guess), and whether anything should tell the provider its filter was wrong, which is network work this application does not do and would belong in a sidecar like item 194's. No seam is needed in the meantime: `sendMove()` already takes any destination and any tags |
+| 201 | A message in the Spam view cannot be un-spammed, even one qtmaildir put there | defect | S | open, 2026-09-14, found while testing the `spam-view` branch. Corrects item 197's claim that Restore already covers what qtmaildir moved: the CAPABILITY does (`restoreSelectedFromTrash()` reads the origin from the database), but no SURFACE offers it for spam, so only Ctrl+Z immediately after the move reverses it. The user asked for it to be built on `spam-view` before that branch merges. See the section |
Sizes are rough: XS under an hour, S a sitting, M a session.
@@ -1632,6 +1633,12 @@ destination and any tag lists, so a Not-spam action is a caller rather than a
capability. Provisioning for it now would be a hook with one hypothetical
caller, which is what YAGNI names.
+**Corrected 2026-09-14:** the interface half is item 201. Restore's visibility
+is coupled to `everySelectedRowIsInATrashFolder()`, which by design never
+answers for spam, so "Restore already covers what qtmaildir moved" was a
+capability claim that no surface offered. Read item 201 for the narrower,
+decision-free half.
+
## 198. The unsynced-changes list never says which account a message belongs to
**Observed (user, from the notes):** "when clicking on the bottom right status
@@ -1766,3 +1773,62 @@ in the notes.
`--thread` value reaches a notmuch query, so it goes through `SearchTerm`'s
quoting like every other query this application builds, rather than being
concatenated at the call site.
+
+## 201. A message in the Spam view cannot be un-spammed, even one qtmaildir put there
+
+**Observed (user, 2026-09-14, testing the `spam-view` branch).** "If a message
+is in spam, how do I unmark it spam?" The only built-in answer found was Ctrl+Z
+immediately after the move, before any other action; an edit of the `spam` tag
+alone leaves the file in the spam folder. The user asked for this to be built on
+`spam-view` before that branch merges.
+
+**Cause.** Verified in `src/`. Three facts together:
+
+1. `spam` is one-way, not a toggle. `MainWindow::spamSelected()` always resolves
+ the selection and calls `spamMessages()`/`spamThreads()`; unlike `delete`
+ (`mainwindow.cpp:1745`) and `flag` (`mainwindow.cpp:1832`) it never asks
+ `everySelectedRowHasTag("spam")`, so pressing Mark spam again moves the file
+ toward the folder it is already in rather than reverting it.
+2. Restore is hidden outside the trash. `refreshTrashActions()`
+ (`mainwindow.cpp:3936`) sets `restore`'s visibility to
+ `(!haveSelection || inTrash) && !m_replySelectionHidesDelete`, where `inTrash`
+ is `everySelectedRowIsInATrashFolder()`, which compares `account.trash` only
+ BY DESIGN so the predicate never answers for spam (the spec requires that, so
+ Delete is not hidden and Purge not offered there). Restore is coupled to the
+ same predicate, which is what hid it in the Spam view.
+3. The write side already works. `restoreSelectedFromTrash()`
+ (`mainwindow.cpp:6907`) resolves the origin from the DATABASE and calls
+ `sendMove()`, which takes any destination and any tag lists. Only the entry
+ point and the gating are missing.
+
+So item 197's "Restore already covers what qtmaildir moved" is true of the
+function and false of the interface. Its stated gap was the PROVIDER-caught
+message with no origin; this item is the narrower half, mail qtmaildir itself
+moved and can put back, and it needs no new decision.
+
+**Approach.** One of two, and the difference is what the user should decide:
+
+- **Widen Restore into the Spam view**, decoupling Restore's visibility from
+ `everySelectedRowIsInATrashFolder()` so a message carrying a `moved-from:`
+ origin can be restored from spam too. Smallest change; makes one action serve
+ both folders.
+- **Add a distinct `not_spam` action** shown on a spam-folder selection, doing
+ the existing restore and also stripping `spam`. Clearer on the bar and in the
+ menu, and costs the five registration places the rule names
+ (`KeyMap::knownActions()`, `defaultBindings()` optional, the icon table, the
+ action, a menu).
+
+Either reuses `sendMove()` and resolves the origin from the database, never the
+model (the message-scoped `restoreSelected()` reads the model, the path item 176
+and the spam-view final review warn against).
+
+**Constraints.** A move's origin is resolved by the worker, never the model
+(`CLAUDE.md`). An undo covers what the write CHANGED, not what it asked for
+(item 176). No confirmation: this is a move and it is undoable. Item 197's open
+questions, where PROVIDER-caught mail with no origin goes (the inbox guess) and
+whether to tell the provider its filter was wrong, stay out of scope here.
+
+**Verification.** A `WorkerBackedWindow` test using `QTRY_VERIFY_WITH_TIMEOUT`:
+mark a message spam, then un-spam it through the new path, asserting the file
+returns to the folder it came from, `spam` and `moved-from:` are gone, and the
+row leaves the Spam view. Because it is a move, assert the undo as well.
diff --git a/docs/superpowers/plans/2026-09-13-spam-view.md b/docs/superpowers/plans/2026-09-13-spam-view.md
index f6bec47..ebd8a6a 100644
--- a/docs/superpowers/plans/2026-09-13-spam-view.md
+++ b/docs/superpowers/plans/2026-09-13-spam-view.md
@@ -460,9 +460,39 @@ lupdate-qt6 src/ -ts translations/qtmaildir_it_IT.ts -no-obsolete -locations non
---
+### Task 9: Not spam
+
+**Files:**
+- Modify: `src/keymap.cpp` (`knownActions()`)
+- Modify: `src/mainwindow.h` (`notSpamSelected()`, `notSpamMessages()`, `notSpamThreads()`, `everySelectedRowIsInASpamFolder()`)
+- Modify: `src/mainwindow.cpp` (action, icon table, both menus, message-bar spam branch, gating, the three methods, the `not_spam_*` resolution branches)
+- Modify: `tests/test_mainwindow.cpp`
+- Modify: `CHANGELOG.md`, `README.md`, `translations/qtmaildir_it_IT.ts`
+
+**Interfaces:**
+- Consumes: `Account::spam`, `accountForMessagePath()`, `originTagFor()`, `sendMove()`, the `resolveMessages`/`resolveThreadMessages` worker calls, `kOriginTagPrefix`, `m_replySelectionHidesDelete`.
+- Produces: `MainWindow::notSpamSelected()`, `notSpamMessages(const QStringList &, const QStringList &, const QStringList &)`, `notSpamThreads(const QStringList &)`, `bool MainWindow::everySelectedRowIsInASpamFolder() const`.
+
+**Behaviour (approved 2026-09-14).** Shown when the selection is in a spam folder, hidden on reply rows and in the trash, like Delete/Restore. Moves each message back to the folder its `moved-from:` tag names, strips `spam` + that origin, adds `inbox` when the destination is the account's inbox. A message with NO origin (provider-caught) falls back to the account's inbox and is reported in the status bar, exactly as `restoreResolvedMessages()` already does for trash. A conversation row acts on the whole conversation, a message row on that message. Undoable, no confirmation, no default shortcut.
+
+- [ ] **Step 1: Register.** Add `QStringLiteral("not_spam")` to `KeyMap::knownActions()`, no `defaultBindings()` entry. Icon table: `{ QStringLiteral("not_spam"), { QStringLiteral("mail-mark-notjunk"), QString() } }` (`mail-mark-notjunk` ships in Breeze and Adwaita and is unused in the table, so no icon exception is needed).
+- [ ] **Step 2: Action + menus.** `addAction(QStringLiteral("not_spam"), tr("Not s&pam"), ...)` with tip `tr("Move the selected messages out of the spam folder")`; choose a mnemonic free in the Message menu (`&p` is `Re&ply`, `&s` is `Mark &spam`; `noMenuHasTwoEntriesSharingAMnemonic()` must stay green). Add the action to `messageMenu` and `m_threadContextMenu`.
+- [ ] **Step 3: Predicate + gating.** Add `bool MainWindow::everySelectedRowIsInASpamFolder() const` mirroring `everySelectedRowIsInATrashFolder()` but comparing against `account.spam`. In `refreshTrashActions()` compute `const bool inSpam = everySelectedRowIsInASpamFolder();` and set `not_spam` visible with `haveSelection && inSpam && !m_replySelectionHidesDelete`, so it is hidden on a reply row and everywhere outside the spam folder.
+- [ ] **Step 4: Message bar.** In `populateMessageBar()`, add a branch keyed on `everySelectedRowIsInASpamFolder() && !selection empty`, between the trash branch and the draft branch, whose list is `{ not_spam }`.
+- [ ] **Step 5: The three methods.** `notSpamSelected()` mirrors `restoreSelectedFromTrash()` but resolves through the worker with request tag `"not_spam_messages"`; `notSpamThreads()` mirrors `untrashThreads()` with `"not_spam_thread"`. Handle both in `onThreadMessagesResolved()` beside `"restore_messages"` and `"undelete_thread"`, clearing `spam` instead of `deleted`. **Prefer parameterising the existing `restoreResolvedMessages()` and the `undelete_thread` branch with the cleared tag and the undo description over copying them**, so the two scopes cannot drift; if you copy instead, say why.
+- [ ] **Step 6: Tests** in `tests/test_mainwindow.cpp`, `WorkerBackedWindow`, `QTRY_VERIFY_WITH_TIMEOUT`, assertions against the database:
+ - mark a message spam, then Not spam: the file returns to its exact original folder, `spam` and `moved-from:` are gone, and `inbox` is back when the origin is the inbox;
+ - a provider-caught message (in the spam folder, no `moved-from:`) goes to the account's inbox and the status reports that it had no origin;
+ - visibility: offered in the spam view, hidden on a reply row and in the trash view;
+ - undo restores it.
+- [ ] **Step 7: Docs + i18n.** Add a clause to the changelog's `[Unreleased]` Added entry, mention the action in `README.md`, run `lupdate-qt6`/`lrelease-qt6`, translate the new strings, and keep `ctest -R translations` green.
+- [ ] **Step 8: Run + commit.** `ctest --test-dir build -R 'keymap|mainwindow|translations'`, then `git commit -S -m "feat: a Not spam action"`.
+
+---
+
## Self-review
-- **Spec coverage:** config key + queries (Task 1), the Spam filter (Task 2), origin rename (Task 3), the move action (Task 4), message bar + icon (Task 5), Empty Spam (Task 6), stranded cleanup (Task 7), translations/docs (Task 8). The trash-predicate test is in Task 4 Step 6. Item 196 (abusectl auto-tagging) and item 197 (not-spam) are out of scope by the spec.
-- **Type consistency:** `spamQuery`/`allSpamQuery`/`isShowingSpam`/`spamSelected`/`spamMessages`/`spamThreads`/`emptySpam`/`showStrandedSpamMail`/`kOriginTagPrefix` are used with one spelling throughout.
-- **Decided:** Empty Spam removes `spam`, `unread` and `inbox` (user confirmed `unread`), and adds `deleted` + the origin placeholder. It carries no confirmation dialog and no default shortcut; it is a move and is undoable.
+- **Spec coverage:** config key + queries (Task 1), the Spam filter (Task 2), origin rename (Task 3), the move action (Task 4), message bar + icon (Task 5), Empty Spam (Task 6), stranded cleanup (Task 7), translations/docs (Task 8), Not spam (Task 9, added 2026-09-14 from backlog item 201 after the branch's final review). The trash-predicate test is in Task 4 Step 6. Item 196 (abusectl auto-tagging) and item 197's provider-notification half are out of scope by the spec; item 197's destination question is settled as the inbox fallback in Task 9.
+- **Type consistency:** `spamQuery`/`allSpamQuery`/`isShowingSpam`/`spamSelected`/`spamMessages`/`spamThreads`/`emptySpam`/`showStrandedSpamMail`/`notSpamSelected`/`notSpamMessages`/`notSpamThreads`/`everySelectedRowIsInASpamFolder`/`kOriginTagPrefix` are used with one spelling throughout.
+- **Decided:** Empty Spam removes `spam`, `unread` and `inbox` (user confirmed `unread`), and adds `deleted` + the origin placeholder. It carries no confirmation dialog and no default shortcut; it is a move and is undoable. Not spam falls back to the account's inbox for provider-caught mail, at the user's decision on 2026-09-14.
- **Known blast radius:** Task 3 renames a string literal in ~35 test sites; `grep -rn "deleted-from" src tests` must be empty afterwards.