diff options
| -rw-r--r-- | CHANGELOG.md | 53 | ||||
| -rw-r--r-- | CLAUDE.md | 52 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md | 87 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 53 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-17-delete-to-trash.md | 94 | ||||
| -rw-r--r-- | src/config.cpp | 72 | ||||
| -rw-r--r-- | src/config.h | 44 | ||||
| -rw-r--r-- | src/keymap.cpp | 56 | ||||
| -rw-r--r-- | src/keymap.h | 8 | ||||
| -rw-r--r-- | src/mainwindow.cpp | 985 | ||||
| -rw-r--r-- | src/mainwindow.h | 265 | ||||
| -rw-r--r-- | src/notmuchworker.cpp | 247 | ||||
| -rw-r--r-- | src/notmuchworker.h | 83 | ||||
| -rw-r--r-- | src/tagdialog.cpp | 29 | ||||
| -rw-r--r-- | src/threadlistmodel.cpp | 11 | ||||
| -rw-r--r-- | src/types.h | 20 | ||||
| -rw-r--r-- | tests/test_config.cpp | 141 | ||||
| -rw-r--r-- | tests/test_mainwindow.cpp | 1950 | ||||
| -rw-r--r-- | tests/test_notmuchworker.cpp | 144 | ||||
| -rw-r--r-- | tests/test_tagdialog.cpp | 57 | ||||
| -rw-r--r-- | translations/qtmaildir_it_IT.ts | 99 |
21 files changed, 4333 insertions, 217 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 75d2de1..dbc06e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,59 @@ point at which they are stable. ## [Unreleased] +### Added + +- Delete now moves mail into the account's trash folder instead of only + tagging it. A **Trash** filter sits beside Unread, Inbox, Important and + Sent, and composes with the account selector like the others. +- **Restore from trash** (`Ctrl+R`), enabled while the trash view is showing. + A message this application deleted returns to the folder it came from; one + trashed by another client returns to the inbox. +- **Find stranded deleted mail** (`Ctrl+Alt+T`), in the Message menu. It lists + mail tagged `deleted` that never moved anywhere. Run it whenever you like; + it reports and moves nothing on its own. +- An optional per-account `inbox` key, naming the inbox folder a restore falls + back to when a message carries no record of where it came from. It defaults + to `Inbox`, so an account whose inbox is named that needs nothing. +- `Del` now deletes, alongside `Ctrl+D`. It still edits text in the query bar + and in any other text field, so nothing is lost where the key already had a + job. + +### Changed + +- Open thread, Clear message pane and Clear selection appear in the View menu. + All three existed and were reachable only by their shortcuts. +- Restoring from the trash view refreshes the list, so the restored message + leaves it straight away instead of sitting there until the Trash filter is + clicked again. Other views are unaffected: a deleted message's card + deliberately stays where it is. + +### Upgrading + +**Every account now needs a `trash` key** in `qtmaildir.conf`, naming its +trash folder relative to `maildir`: + + [account.work] + maildir = work + trash = Trash + +The folder must be one your `mbsync` configuration actually syncs, or the move +will never reach the server. Accounts without the key still load and still +read mail, but Delete cannot work on them and a warning says so at startup. + +**Name the folder exactly as it exists on the server.** A trash or inbox name +that does not match creates that folder rather than reporting an error, and +under mbsync's `Create Both` the wrongly named folder then propagates to the +mail server, where other clients will see it. + +**Mail deleted by earlier versions is not migrated.** It carries the `deleted` +tag and sits wherever it always was. Use **Find stranded deleted mail** to +review it, and Delete on what should really go. + +Note that Delete's reversibility depends on your provider: a trash folder the +provider purges on a timer will eventually remove the mail for good. + + ## [0.25.0] - 2026-08-17 Acting on a row now means the message that row displays, not the whole @@ -227,6 +227,34 @@ combined `thread:a or thread:b` query rather than one query per thread. The only escape hatch is `general/notmuch_config`, pointing at an alternate notmuch config. Per-account subdirectories *are* configured, since notmuch does not model accounts at all. +**Delete MOVES the file, and a wrong folder name reaches the mail server.** +Item 103. Every account carries a mandatory `trash` key and an optional +`inbox` one, both naming a folder relative to `maildir`. Naming a folder that +does not exist does not fail: the move CREATES it, mbsync adopts it and writes +state files for it, and under `Create Both` it then propagates to the server, +where every other client sees it. This is not theoretical. A folder name +containing a space was truncated by the origin tag, a bogus folder was created +beside the real one, and four messages of a thread were stranded in it on the +user's real mail. Treat any code that composes a folder name as reaching the +server, because it does. + +**A message records where it came from in a tag, because nothing else can.** +`deleted-from:<folder>` is written when Delete moves the file, and read back by +Restore. The file has moved, so neither the path nor anything in notmuch still +knows the original folder. A notmuch tag MAY contain a space, so tags crossing +the thread boundary are joined by a TAB rather than a space; joining on a space +truncated every folder name containing one. A message trashed by another client +carries no such tag at all, which is why the trash view is path-based and why +Restore falls back to the account's inbox rather than refusing. + +**Restore reads the DATABASE, never the model.** The model's tags come from the +query, so a row whose delete has not been re-queried still carries its pre-delete +tags: measured `[inbox,unread]` on a message already in the trash, one run in +three. The origin tag is then not found, the message falls into the no-origin +branch, and it goes to the inbox instead of where it came from, silently and +irreversibly. A restore must be right about its destination or it is worse than +doing nothing. + **The sync script lives here, in `assets/mailsync.sh`.** It moved from the companion `mailctl` project, which documents that it never calls it: the script is `mbsync` plus `notmuch new` with a lock, and qtmaildir is the only thing that @@ -577,15 +605,27 @@ a union over the conversation, so it can arm for a thread whose displayed message is already read. The write is still scoped to that message, so the cost is a no-op rather than a wrong write. -**Adding an action is four places, and three of them are enforced by tests that +**Adding an action is FIVE places, and four of them are enforced by tests that fail in confusing ways.** `KeyMap::knownActions()` (a `Q_ASSERT` in the constructor fires otherwise, and it surfaces in whichever suite happens to build a `MainWindow` first — `test_tagrules` did), `defaultBindings()` (every action -must be keyboard-reachable), and the icon table (every action must carry one). -The no-duplicate-icons rule is narrowed to actions that can reach the toolbar, -by a named exception list; the five thread actions share their twins' icons -because a submenu entry always carries text, and the test asserts none of them -is on the toolbar so the exemption cannot be abused. +must be keyboard-reachable), the icon table (every action must carry one), and +a MENU. The no-duplicate-icons rule is narrowed to actions that can reach the +toolbar, by a named exception list; the five thread actions share their twins' +icons because a submenu entry always carries text, and the test asserts none of +them is on the toolbar so the exemption cannot be abused. + +**The menu was the fifth place, and this document said four until item 103.** +Nothing enforced it, so `restore` shipped on the trash branch reachable by +`Ctrl+R` and by nothing a user could see or discover. The three existing +coverage tests each assert a different property and all three pass against an +action that appears nowhere in the interface. +`everyActionIsReachableFromAMenu()` closes it, walking every menu and submenu +from the menu bar; it found three more of the same the moment it was written +(`open_thread`, `clear_pane`, `clear_selection`). The toolbar is deliberately +NOT the test's instrument: it is a small chosen subset and always will be. An +action owning a submenu is not itself counted as reachable, since Qt emits no +`triggered` for it. **A toggle must read the state of what the row STANDS FOR, not of its thread.** `MainWindow::everySelectedRowHasTag()` is the one question `delete` and 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 f46fec5..ef51b5c 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 @@ -6612,3 +6612,90 @@ Three lessons, in the order they were paid for: `image/png` that is demonstrably on the clipboard: a Wayland clipboard-manager interaction, or GIMP's own paste path. Neither is this repository's, and neither needs an item here until it is shown to be. + +## 103. What Delete does to mail on the server is undocumented and unverified + +**Observed (user, from the notes):** "verify how 'delete' works", with two +sub-questions of their own: "trash bin (?)" and "delete from server (?)". + +**This is a question first.** The user is not reporting a defect; they are saying +they do not know what the button does to their mail, which for a destructive +action is its own problem regardless of the answer. + +**What the code does (verified).** `src/mainwindow.cpp:825` adds and removes the +`deleted` tag, and nothing else. It is a toggle, it goes through the undo stack, +and it writes a notmuch tag. + +**What that means downstream is what needs verifying, and it is NOT in this +repo.** `maildir.synchronize_flags` is true, so notmuch maps certain tags to +Maildir filename flags, and mbsync carries filename flags to the server. Whether +`deleted` is one of those, whether the user's `~/.mbsyncrc` has `Expunge Both` +(it does, on every channel), and what each provider does with a message flagged +deleted, together decide whether this button is reversible. The undo stack makes +the TAG reversible; it says nothing about what a sync did with it in between. + +**Measured 2026-08-17, and the answer is that Delete does not delete.** notmuch's +tag-to-flag table has no row for `deleted` and no `T` flag, confirmed by a probe +on a throwaway database: `+deleted` left the filename untouched while the +control `+flagged` immediately produced `:2,F`. mbsync carries filename flags, so +`Expunge Both` never sees anything to expunge, and `assets/mailsync.sh` contains +no delete path. Both sub-questions answer no: no trash bin, no deletion from the +server, and the mail stays in the Maildir and the index forever. + +**Specified in +`specs/2026-08-17-delete-to-trash-design.md`. Read that before writing code.** +Delete becomes a real move into the account's trash folder, with a `Trash` +filter beside the other built-ins and a Restore action. Three constraints decide +whether the spec is worth opening: + +- It needs a **mandatory per-account `trash` key**, so an existing config warns + until five keys are added. User-visible: minor bump and an `### Upgrading` + note. +- The worker gains its **first non-tag mutation**, a rename plus a reindex, + deliberately shaped as `moveMessages(ids, destFolder)` so Send in v2 reuses it + for Drafts and Sent. +- **848 messages carry the old tag** while sitting in an inbox, and would be + invisibly half-deleted after the change. A repeatable menu entry queries them + into the list for review; it is not a startup migration. + +The no-confirmation rule survives, with its justification amended: the mail +lands in a browsable folder, but reversibility is now bounded by the provider +where the trash is purged on a timer. + +**Size: S** for the investigation, which is done. The build is **M**. + +**Built 2026-08-19, over eight commits on `delete-to-trash`.** Delete moves the +file into the account's `trash` folder and records where it came from in a +`deleted-from:<folder>` tag, because the file has moved and nothing else still +knows. A `Trash` filter sits beside the other four, `Restore from trash` +(`Ctrl+R`) is the inverse, and `Find stranded deleted mail` (`Ctrl+Alt+T`, a +menu entry only at the user's request) lists the 848 messages the old behaviour +left tagged but unmoved. It reports and moves nothing: acting on its own would +be a bulk delete with no selection behind it. + +**Ten defects were found by hand testing, not by the suite**, each reproduced +with a probe before being fixed. One had already damaged real mail: a folder +name containing a space was truncated by the origin tag, creating a bogus folder +beside the real one and stranding four messages of a thread. mbsync had adopted +it and written state files for it. That is the fact worth carrying out of this +item: **under `Create Both` a wrongly named origin folder propagates to the mail +server**, so any code composing a folder name is reaching the server whether it +means to or not. Tags are joined by a TAB across the thread boundary for the +same reason, since a notmuch tag may contain a space. + +**Two process gaps closed alongside it.** Adding an action is FIVE places and +this repository's CLAUDE.md said four; the missing one is a menu, and nothing +enforced it, so `restore` shipped reachable by a chord and by nothing a user +could see. `everyActionIsReachableFromAMenu()` now asserts it and found three +more of the same (`open_thread`, `clear_pane`, `clear_selection`). And a guard +in `deletingTwiceLeavesNoOriginTagBehind()` queried through the query bar in the +gap between the file rename and the tag writes, where a run returns zero rows +forever because `QTRY_VERIFY` re-reads `rowCount()` and never re-runs the query: +3 failures in 12 runs, each burning a full 15s timeout, 0 in 8 after asking the +database directly. + +**Emptying the trash is item 118 and remains deferred**, at the user's request. + +**Verified:** clean build with no warnings from any changed file, 24 of 24 +suites over three consecutive runs, 211 tests in `test_mainwindow`. Every one of +the spec's six testing bullets has a test, each mutation-checked. 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 6a5e64d..1f71006 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 @@ -167,7 +167,7 @@ taking that too literally. | 100 | The message pane offers Back, Forward, Reload and Save page, none of which mean anything | defect | XS | **done** 2026-08-17, unreleased. `MessageView::removeBrowserActions()` filters the standard menu by `pageAction()` POINTER, never by text; `ViewSource` went with them, and stranded separators are swept | | 101 | Sync is account-aware for edits but not for the account the user is looking at | workflow | S | open; item 49 built the edit half deliberately. Needs a decision, see the entry | | 102 | The rules table shows no note, so the field explaining a rule is invisible until it is opened | workflow | XS | **done** 2026-08-17, unreleased. A Note column before `ColumnCount`, so the appended Matches column stays last. Found a second defect on the way: `restoreState` REFUSES a header state with a different column count, and the sized flags were being set regardless | -| 103 | What Delete does to mail on the server is undocumented and unverified | clarification | S | open; a question first, possibly no code at all | +| 103 | What Delete does to mail on the server is undocumented and unverified | clarification | S+M | done; Delete moves to the account trash, with Restore and a stranded-mail cleanup. Section in the closed file | | 104 | Mail visible in Thunderbird never reaches qtmaildir | defect | ? | open, reported 2026-08-16, cause NOT established. Most likely outside this repo; see the entry before writing code | | 109 | A root card's own message is invisible to a message-scoped write | defect | S | **done** 2026-08-16, unreleased. Found by hand-testing 108. `applyMessageTagChange` and `messageById` searched only the loaded replies, and a root's message is never among them, so the ORDINARY gesture repainted nothing and wiped the pane's chip row | | 110 | A card and the message pane show tags belonging to a message's siblings | defect | S | **done** 2026-08-16, unreleased. Found by hand-testing 109 against a real 4-message thread. `ThreadSummary::tags` is notmuch's UNION; a card standing for one message drew it. Also the reason a root card could not repaint at all | @@ -485,57 +485,6 @@ reaches it (item 42), so most of this exists. **Size: S** for the on-demand button, XS for the visibility half. Ask which. -## 103. What Delete does to mail on the server is undocumented and unverified - -**Observed (user, from the notes):** "verify how 'delete' works", with two -sub-questions of their own: "trash bin (?)" and "delete from server (?)". - -**This is a question first.** The user is not reporting a defect; they are saying -they do not know what the button does to their mail, which for a destructive -action is its own problem regardless of the answer. - -**What the code does (verified).** `src/mainwindow.cpp:825` adds and removes the -`deleted` tag, and nothing else. It is a toggle, it goes through the undo stack, -and it writes a notmuch tag. - -**What that means downstream is what needs verifying, and it is NOT in this -repo.** `maildir.synchronize_flags` is true, so notmuch maps certain tags to -Maildir filename flags, and mbsync carries filename flags to the server. Whether -`deleted` is one of those, whether the user's `~/.mbsyncrc` has `Expunge Both` -(it does, on every channel), and what each provider does with a message flagged -deleted, together decide whether this button is reversible. The undo stack makes -the TAG reversible; it says nothing about what a sync did with it in between. - -**Measured 2026-08-17, and the answer is that Delete does not delete.** notmuch's -tag-to-flag table has no row for `deleted` and no `T` flag, confirmed by a probe -on a throwaway database: `+deleted` left the filename untouched while the -control `+flagged` immediately produced `:2,F`. mbsync carries filename flags, so -`Expunge Both` never sees anything to expunge, and `assets/mailsync.sh` contains -no delete path. Both sub-questions answer no: no trash bin, no deletion from the -server, and the mail stays in the Maildir and the index forever. - -**Specified in -`specs/2026-08-17-delete-to-trash-design.md`. Read that before writing code.** -Delete becomes a real move into the account's trash folder, with a `Trash` -filter beside the other built-ins and a Restore action. Three constraints decide -whether the spec is worth opening: - -- It needs a **mandatory per-account `trash` key**, so an existing config warns - until five keys are added. User-visible: minor bump and an `### Upgrading` - note. -- The worker gains its **first non-tag mutation**, a rename plus a reindex, - deliberately shaped as `moveMessages(ids, destFolder)` so Send in v2 reuses it - for Drafts and Sent. -- **848 messages carry the old tag** while sitting in an inbox, and would be - invisibly half-deleted after the change. A repeatable menu entry queries them - into the list for review; it is not a startup migration. - -The no-confirmation rule survives, with its justification amended: the mail -lands in a browsable folder, but reversibility is now bounded by the provider -where the trash is purged on a timer. - -**Size: S** for the investigation, which is done. The build is **M**. - ## 104. Mail visible in Thunderbird never reaches qtmaildir **Observed (user, from the notes):** "sync doesn't work compared to thunderbird. diff --git a/docs/superpowers/plans/2026-08-17-delete-to-trash.md b/docs/superpowers/plans/2026-08-17-delete-to-trash.md index 4e52789..70742b5 100644 --- a/docs/superpowers/plans/2026-08-17-delete-to-trash.md +++ b/docs/superpowers/plans/2026-08-17-delete-to-trash.md @@ -52,7 +52,7 @@ No new files. Every change lands in a file that already owns that responsibility - Modify: `src/config.cpp` (`Account::trashQuery()` beside `sentQuery()` at line 130, and the account parser) - Test: `tests/test_config.cpp` -- [ ] **Step 1: Write the failing test** +- [x] **Step 1: Write the failing test** Add to `tests/test_config.cpp`, and declare both slots in the `private slots:` block: @@ -94,7 +94,7 @@ void TestConfig::aBracketedTrashFolderIsQuoted() } ``` -- [ ] **Step 2: Run test to verify it fails** +- [x] **Step 2: Run test to verify it fails** ```bash cmake --build build 2>&1 | tail -5 @@ -102,7 +102,7 @@ cmake --build build 2>&1 | tail -5 Expected: FAIL to compile, `'trash' is not a member of 'Account'`. -- [ ] **Step 3: Write minimal implementation** +- [x] **Step 3: Write minimal implementation** In `src/config.h`, add to `Account` immediately after the `sent` member: @@ -143,7 +143,7 @@ Then find where the account parser reads `sent` (search for `QStringLiteral("sen account.trash = settings.value(QStringLiteral("trash")).toString(); ``` -- [ ] **Step 4: Run test to verify it passes** +- [x] **Step 4: Run test to verify it passes** ```bash cmake --build build && QT_QPA_PLATFORM=offscreen ./build/tests/test_config @@ -151,7 +151,7 @@ cmake --build build && QT_QPA_PLATFORM=offscreen ./build/tests/test_config Expected: PASS, all tests. -- [ ] **Step 5: Commit** +- [x] **Step 5: Commit** ```bash git add src/config.h src/config.cpp tests/test_config.cpp @@ -170,7 +170,7 @@ First read how warnings are currently raised: search `src/config.cpp` for `m_warnings` and copy the surrounding form exactly. Do not invent a new mechanism. -- [ ] **Step 1: Write the failing test** +- [x] **Step 1: Write the failing test** ```cpp void TestConfig::anAccountWithoutATrashFolderWarns() @@ -204,7 +204,7 @@ void TestConfig::anAccountWithoutATrashFolderWarns() } ``` -- [ ] **Step 2: Run test to verify it fails** +- [x] **Step 2: Run test to verify it fails** ```bash cmake --build build && QT_QPA_PLATFORM=offscreen ./build/tests/test_config -functions | grep -i trash @@ -213,7 +213,7 @@ QT_QPA_PLATFORM=offscreen ./build/tests/test_config anAccountWithoutATrashFolder Expected: FAIL, `warnings` is empty. -- [ ] **Step 3: Write minimal implementation** +- [x] **Step 3: Write minimal implementation** In the account-loading loop in `src/config.cpp`, after the account is parsed and before it is appended: @@ -232,7 +232,7 @@ before it is appended: } ``` -- [ ] **Step 4: Run test to verify it passes** +- [x] **Step 4: Run test to verify it passes** ```bash cmake --build build && QT_QPA_PLATFORM=offscreen ./build/tests/test_config @@ -240,7 +240,7 @@ cmake --build build && QT_QPA_PLATFORM=offscreen ./build/tests/test_config Expected: PASS. -- [ ] **Step 5: Commit** +- [x] **Step 5: Commit** ```bash git add src/config.cpp tests/test_config.cpp @@ -256,7 +256,7 @@ git commit -m "feat(config): warn when an account configures no trash folder" - Modify: `src/config.cpp` (`kQueryGenerators` line 60, `builtinFilter()` line 757, `resolvedQuery()` line 790, `allTrashQuery()` beside line 140) - Test: `tests/test_config.cpp` -- [ ] **Step 1: Write the failing test** +- [x] **Step 1: Write the failing test** ```cpp void TestConfig::theTrashFilterComposesPerAccount() @@ -307,7 +307,7 @@ void TestConfig::theTrashFilterMatchesNothingWithoutAFolder() } ``` -- [ ] **Step 2: Run test to verify it fails** +- [x] **Step 2: Run test to verify it fails** ```bash cmake --build build && QT_QPA_PLATFORM=offscreen ./build/tests/test_config theTrashFilterComposesPerAccount @@ -315,7 +315,7 @@ cmake --build build && QT_QPA_PLATFORM=offscreen ./build/tests/test_config theTr Expected: FAIL, `trash.isGenerated()` is false because the generator is unknown. -- [ ] **Step 3: Write minimal implementation** +- [x] **Step 3: Write minimal implementation** In `src/config.cpp`, add to `kQueryGenerators` at line 60, last so it sits rightmost on the query row: @@ -377,7 +377,7 @@ And in the per-account branch, beside the `sent` case: } ``` -- [ ] **Step 4: Run test to verify it passes** +- [x] **Step 4: Run test to verify it passes** ```bash cmake --build build && QT_QPA_PLATFORM=offscreen ./build/tests/test_config @@ -385,13 +385,13 @@ cmake --build build && QT_QPA_PLATFORM=offscreen ./build/tests/test_config Expected: PASS. -- [ ] **Step 5: Verify the mutation fails** +- [x] **Step 5: Verify the mutation fails** Temporarily change the per-account branch to `return allTrashQuery();` and rebuild. `theTrashFilterComposesPerAccount` must FAIL on the `QCOMPARE`. Revert the mutation. This proves the test asserts on the string rather than on rows. -- [ ] **Step 6: Commit** +- [x] **Step 6: Commit** ```bash git add src/config.h src/config.cpp tests/test_config.cpp @@ -412,7 +412,7 @@ This is the first mutation in the project that is not a notmuch tag. Read uses is required, not stylistic, because notmuch permits one open handle per process. -- [ ] **Step 1: Write the failing test** +- [x] **Step 1: Write the failing test** Add to `tests/test_notmuchworker.cpp`, declaring each slot in `private slots:`: @@ -536,7 +536,7 @@ void TestNotmuchWorker::moveMessagesReportsOnlyWhatMoved() } ``` -- [ ] **Step 2: Run test to verify it fails** +- [x] **Step 2: Run test to verify it fails** ```bash cmake --build build 2>&1 | tail -5 @@ -544,7 +544,7 @@ cmake --build build 2>&1 | tail -5 Expected: FAIL to compile, `'moveMessages' is not a member of 'NotmuchWorker'`. -- [ ] **Step 3: Write minimal implementation** +- [x] **Step 3: Write minimal implementation** In `src/notmuchworker.h`, beside `applyTags()`: @@ -675,7 +675,7 @@ void NotmuchWorker::moveMessages(const QStringList &messageIds, } ``` -- [ ] **Step 4: Run test to verify it passes** +- [x] **Step 4: Run test to verify it passes** ```bash cmake --build build && QT_QPA_PLATFORM=offscreen ./build/tests/test_notmuchworker @@ -683,7 +683,7 @@ cmake --build build && QT_QPA_PLATFORM=offscreen ./build/tests/test_notmuchworke Expected: PASS, all tests. -- [ ] **Step 5: Verify the ordering mutation fails** +- [x] **Step 5: Verify the ordering mutation fails** Move the `notmuch_database_remove_message` call to immediately BEFORE the `notmuch_database_index_file` call and rebuild. @@ -691,7 +691,7 @@ Move the `notmuch_database_remove_message` call to immediately BEFORE the the mutation. This is the single most important check in the task: the wrong order silently destroys user tags and every other test still passes. -- [ ] **Step 6: Commit** +- [x] **Step 6: Commit** ```bash git add src/notmuchworker.h src/notmuchworker.cpp tests/test_notmuchworker.cpp @@ -716,7 +716,7 @@ selection, resolved through `everySelectedRowHasTag()`, and `CLAUDE.md` records two separate bugs that lived in those three lines. Preserve the toggle: Delete twice still means "put it back". -- [ ] **Step 1: Write the failing test** +- [x] **Step 1: Write the failing test** ```cpp void TestMainWindow::deleteMovesTheMessageToTrash() @@ -793,7 +793,7 @@ whichever helpers are missing, following the form of the ones already there. observable state with `QTRY_VERIFY_WITH_TIMEOUT` and never on worker signals or a fixed `qWait(n)`. -- [ ] **Step 2: Run test to verify it fails** +- [x] **Step 2: Run test to verify it fails** ```bash cmake --build build && QT_QPA_PLATFORM=offscreen ./build/tests/test_mainwindow deleteMovesTheMessageToTrash @@ -801,7 +801,7 @@ cmake --build build && QT_QPA_PLATFORM=offscreen ./build/tests/test_mainwindow d Expected: FAIL, the message is still in Inbox because Delete only tags. -- [ ] **Step 3: Write minimal implementation** +- [x] **Step 3: Write minimal implementation** Add a `MoveCommand` in `src/mainwindow.h`, beside `MessageTagCommand` around line 1176, following its shape exactly: @@ -888,7 +888,7 @@ the current path, and pushes one `MoveCommand` carrying `deleted` and `deleted-from:<origin>`. An account with no `trash` key contributes nothing and reports through `statusMessage`, since Task 2 already warned at load. -- [ ] **Step 4: Run test to verify it passes** +- [x] **Step 4: Run test to verify it passes** ```bash cmake --build build && QT_QPA_PLATFORM=offscreen ./build/tests/test_mainwindow @@ -896,7 +896,7 @@ cmake --build build && QT_QPA_PLATFORM=offscreen ./build/tests/test_mainwindow Expected: PASS, all tests. -- [ ] **Step 5: Commit** +- [x] **Step 5: Commit** ```bash git add src/mainwindow.h src/mainwindow.cpp tests/test_mainwindow.cpp @@ -918,7 +918,7 @@ are enforced by tests that fail confusingly: `KeyMap::knownActions()` (a `defaultBindings()` (every action must be keyboard-reachable), and the icon table (every action must carry one). -- [ ] **Step 1: Write the failing test** +- [x] **Step 1: Write the failing test** ```cpp void TestMainWindow::restoreIsOnlyEnabledInTheTrashView() @@ -985,7 +985,7 @@ void TestMainWindow::restoreFallsBackToInboxWithoutAnOriginTag() } ``` -- [ ] **Step 2: Run test to verify it fails** +- [x] **Step 2: Run test to verify it fails** ```bash cmake --build build && QT_QPA_PLATFORM=offscreen ./build/tests/test_mainwindow restoreIsOnlyEnabledInTheTrashView @@ -993,7 +993,7 @@ cmake --build build && QT_QPA_PLATFORM=offscreen ./build/tests/test_mainwindow r Expected: FAIL, there is no `restore` action. -- [ ] **Step 3: Write minimal implementation** +- [x] **Step 3: Write minimal implementation** Add `"restore"` to `KeyMap::knownActions()` and give it a binding in `defaultBindings()`. Check what is free first: @@ -1030,7 +1030,7 @@ generator's, for the current account selection. `restoreSelected()` reads each row's `deleted-from:` tag for its destination and falls back to `Inbox`, reporting which through `statusMessage`. -- [ ] **Step 4: Run test to verify it passes** +- [x] **Step 4: Run test to verify it passes** ```bash cmake --build build && QT_QPA_PLATFORM=offscreen ./build/tests/test_mainwindow @@ -1040,7 +1040,7 @@ ctest --test-dir build --output-on-failure Expected: PASS, 24 of 24. The keymap and icon-table tests fail loudly if a place was missed. -- [ ] **Step 5: Commit** +- [x] **Step 5: Commit** ```bash git add src/mainwindow.h src/mainwindow.cpp src/keymap.cpp tests/test_mainwindow.cpp @@ -1059,7 +1059,7 @@ git commit -m "feat: restore mail from the trash view" The user's constraint, verbatim: "the cleanup should be a menu entry only, not to be confused with the filter Trash". Do not add a sixth button. -- [ ] **Step 1: Write the failing test** +- [x] **Step 1: Write the failing test** ```cpp void TestMainWindow::theCleanupQueryFindsStrandedMail() @@ -1109,7 +1109,7 @@ void TestMainWindow::theCleanupQueryExcludesMailAlreadyInTrash() } ``` -- [ ] **Step 2: Run test to verify it fails** +- [x] **Step 2: Run test to verify it fails** ```bash cmake --build build && QT_QPA_PLATFORM=offscreen ./build/tests/test_mainwindow theCleanupQueryFindsStrandedMail @@ -1117,7 +1117,7 @@ cmake --build build && QT_QPA_PLATFORM=offscreen ./build/tests/test_mainwindow t Expected: FAIL, there is no `cleanup_stranded` action. -- [ ] **Step 3: Write minimal implementation** +- [x] **Step 3: Write minimal implementation** Register the action in `knownActions()`, `defaultBindings()` and the icon table as in Task 6, then: @@ -1141,7 +1141,7 @@ as in Task 6, then: Add it to a menu, not to the query row. Find where the other menu entries are built and follow that form. -- [ ] **Step 4: Run test to verify it passes** +- [x] **Step 4: Run test to verify it passes** ```bash cmake --build build && QT_QPA_PLATFORM=offscreen ./build/tests/test_mainwindow @@ -1149,7 +1149,7 @@ cmake --build build && QT_QPA_PLATFORM=offscreen ./build/tests/test_mainwindow Expected: PASS. -- [ ] **Step 5: Commit** +- [x] **Step 5: Commit** ```bash git add src/mainwindow.h src/mainwindow.cpp src/keymap.cpp tests/test_mainwindow.cpp @@ -1168,7 +1168,7 @@ Every user-facing string added above needs a translation, and `lrelease` silently DROPS an unfinished string and ships it as English inside an otherwise Italian UI. Item 108 shipped fifteen strings that way. -- [ ] **Step 1: Refresh the translation source** +- [x] **Step 1: Refresh the translation source** ```bash lupdate-qt6 src/ -ts translations/qtmaildir_it_IT.ts -no-obsolete -locations none @@ -1178,7 +1178,7 @@ Expected: a clean run reporting zero context warnings. A "tr() cannot be called without context" warning means a literal needs `QT_TRANSLATE_NOOP("TheClass", "Text")` rather than `tr()`. -- [ ] **Step 2: Translate every new string** +- [x] **Step 2: Translate every new string** Open `translations/qtmaildir_it_IT.ts` and fill in each `<translation type="unfinished">`. The new strings are the Trash filter name, the Restore and @@ -1188,7 +1188,7 @@ messages. Do NOT translate notmuch query syntax. `tag:deleted`, `path:` and `deleted-from:` are wire format. -- [ ] **Step 3: Verify nothing is unfinished** +- [x] **Step 3: Verify nothing is unfinished** ```bash lrelease-qt6 translations/qtmaildir_it_IT.ts @@ -1197,7 +1197,7 @@ lrelease-qt6 translations/qtmaildir_it_IT.ts Expected: "Generated N translation(s) (N finished, 0 unfinished)". A nonzero unfinished count means a string will ship as English. -- [ ] **Step 4: Run the translations test** +- [x] **Step 4: Run the translations test** ```bash ctest --test-dir build -R translations --output-on-failure @@ -1205,7 +1205,7 @@ ctest --test-dir build -R translations --output-on-failure Expected: PASS. -- [ ] **Step 5: Write the changelog entry** +- [x] **Step 5: Write the changelog entry** Under `## [Unreleased]` in `CHANGELOG.md`, with an `### Upgrading` section, since a working config now warns until five keys are added: @@ -1243,7 +1243,7 @@ Note that Delete's reversibility depends on your provider: a trash folder that the provider purges on a timer will eventually remove the mail for good. ``` -- [ ] **Step 6: Commit** +- [x] **Step 6: Commit** ```bash git add translations/qtmaildir_it_IT.ts CHANGELOG.md @@ -1254,7 +1254,7 @@ git commit -m "i18n: translate the trash strings, and document the trash key" ## Task 9: Full verification -- [ ] **Step 1: Clean build** +- [x] **Step 1: Clean build** ```bash rm -rf build @@ -1264,7 +1264,7 @@ cmake --build build Expected: no warnings from the changed files. -- [ ] **Step 2: Full suite** +- [x] **Step 2: Full suite** ```bash ctest --test-dir build --output-on-failure @@ -1272,13 +1272,13 @@ ctest --test-dir build --output-on-failure Expected: 24 of 24 passing. -- [ ] **Step 3: Confirm the spec's claims hold** +- [x] **Step 3: Confirm the spec's claims hold** Re-read `docs/superpowers/specs/2026-08-17-delete-to-trash-design.md` and check each testing bullet has a test. The spec lists six; all six are covered by Tasks 1, 3, 4, 5, 6 and 7. -- [ ] **Step 4: Hand the build to the user** +- [x] **Step 4: Hand the build to the user** Do NOT run `./build/src/qtmaildir`. Report what to look at: diff --git a/src/config.cpp b/src/config.cpp index 600c558..a2d1cec 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -60,7 +60,8 @@ constexpr int kQueriesFormatVersion = 1; const QStringList kQueryGenerators = { QStringLiteral("unread"), QStringLiteral("inbox"), QStringLiteral("flagged"), - QStringLiteral("sent") }; + QStringLiteral("sent"), + QStringLiteral("trash") }; /// The tag a generator matches, for the three filters that are a plain tag /// query. Empty for "sent", which composes from each account's folder instead @@ -137,6 +138,23 @@ QString Account::draftsQuery() const return folderQuery(maildir, drafts); } +QString Account::trashQuery() const +{ + return folderQuery(maildir, trash); +} + +QString Account::inboxFolder() const +{ + // Never empty: Restore needs a folder to name, and "Inbox" is both the + // Maildir convention and what mbsync's own Inbox directive defaults to. + return inbox.isEmpty() ? QStringLiteral("Inbox") : inbox; +} + +QString Account::inboxQuery() const +{ + return folderQuery(maildir, inboxFolder()); +} + QString Config::allSentQuery() const { return joinAccountQueries(m_accounts, &Account::sentQuery); @@ -147,6 +165,11 @@ QString Config::allDraftsQuery() const return joinAccountQueries(m_accounts, &Account::draftsQuery); } +QString Config::allTrashQuery() const +{ + return joinAccountQueries(m_accounts, &Account::trashQuery); +} + QString Config::defaultPath() { const QString base = @@ -433,6 +456,19 @@ void Config::load(const QString &path) account.sent = settings.value(QStringLiteral("sent")).toString().trimmed(); + // Mandatory, unlike sent: Delete moves a file into this folder, so an + // account without one cannot delete at all. Trimmed for the same + // reason as sent, above. + account.trash = + settings.value(QStringLiteral("trash")).toString().trimmed(); + + // Optional, unlike trash: inboxFolder() defaults it to "Inbox", which + // is right for any ordinary Maildir. Read so an account whose inbox is + // named otherwise can say so, rather than having Restore create a + // second folder under a name this program assumed. + account.inbox = + settings.value(QStringLiteral("inbox")).toString().trimmed(); + // Both optional, and both describe this account's chip in the thread // list. An account tag is a different taxonomy from a functional one, // saying which mailbox a thread arrived in rather than what state it @@ -462,6 +498,21 @@ void Config::load(const QString &path) .arg(account.key)); continue; } + + // Mandatory, unlike sent: Delete moves a file into this folder, so an + // account without one cannot delete at all. Reported rather than + // silently disabled, so the user finds out from a warning rather than + // from a Delete that quietly does nothing. The account still loads; + // only Delete is unusable, which does not warrant losing the rest of + // the account's mail. + if (account.trash.isEmpty()) { + addProblem( + tr("Account '%1' has no trash folder configured; add a " + "'trash' key to its section. Delete will not work for " + "this account until it does.") + .arg(account.key)); + } + m_accounts.append(account); } @@ -711,6 +762,8 @@ QString Config::resolvedQuery(const SavedQuery &query) const if (query.isGenerated()) { if (query.generated == QStringLiteral("sent")) return allSentQuery(); + if (query.generated == QStringLiteral("trash")) + return allTrashQuery(); // An unknown generator was reported on load. Empty rather than the // bare stored query, which for a generated entry is empty anyway and // would otherwise run as "match everything". @@ -783,6 +836,11 @@ SavedQuery Config::builtinFilter(const QString &generator) // thread would fold the user's sent message back into the conversation // it belongs to, which is item 63's finding. filter.flat = true; + } else if (generator == QStringLiteral("trash")) { + filter.name = tr("Trash"); + // NOT flat, unlike Sent. A deleted message still belongs to its + // conversation, and folding it back is what Sent had to avoid rather + // than something every folder filter wants. } return filter; @@ -807,6 +865,10 @@ QString Config::resolvedQuery(const SavedQuery &query, const QString all = allSentQuery(); return all.isEmpty() ? matchNothingQuery() : all; } + if (query.generated == QStringLiteral("trash")) { + const QString all = allTrashQuery(); + return all.isEmpty() ? matchNothingQuery() : all; + } return QStringLiteral("tag:%1").arg(generatorTag(query.generated)); } @@ -827,6 +889,14 @@ QString Config::resolvedQuery(const SavedQuery &query, return sent.isEmpty() ? matchNothingQuery() : sent; } + if (query.generated == QStringLiteral("trash")) { + // The account's OWN trash query, for the reason spelled out above the + // sent case: wrapping the all-accounts query in this account's path + // works by accident of path: being hierarchical. + const QString trash = scope.trashQuery(); + return trash.isEmpty() ? matchNothingQuery() : trash; + } + // A tag filter carries no path of its own, so scoping is exactly what // scopedQuery() does. Its parentheses are load-bearing: `path:... and a or // b` binds as `(path:... and a) or b`. diff --git a/src/config.h b/src/config.h index 70f7181..ede5dea 100644 --- a/src/config.h +++ b/src/config.h @@ -58,6 +58,29 @@ struct Account /// one for the account that has none. QString sent; + /// The account's trash folder, relative to maildir. + /// + /// MANDATORY, unlike `sent` and `drafts`. Delete moves a file into this + /// folder, so an account without one cannot delete at all, and the user + /// chose a config error over a per-account disabled state: "it is + /// mandatory for the program to function properly". Config::load() + /// reports a missing key through the warnings path. + QString trash; + + /// The account's inbox folder, relative to maildir. Optional. + /// + /// Only Restore reads it, as the destination for a message that carries no + /// `deleted-from:` origin, which is what mail trashed by another client + /// looks like. Defaults to "Inbox", the Maildir convention and mbsync's + /// own default. + /// + /// Configurable rather than hardcoded because the name is not ours to + /// assume: naming a folder that does not exist CREATES it, beside the real + /// one, and under mbsync's `Create Both` that folder reaches the server. + /// Unlike `trash` this is optional, since the default is right for every + /// ordinary Maildir and a wrong guess here only affects the fallback. + QString inbox; + /// Chip colour in the thread list. Invalid when unset, in which case one /// is generated from the account tag's name. QColor color; @@ -98,6 +121,20 @@ struct Account /// keys are independent, and one real account configures `drafts` with no /// `sent` at all. QString draftsQuery() const; + + /// Matches this account's trash, or empty when `trash` is unset. + /// + /// Empty is a config error rather than a legitimate state, unlike + /// sentQuery(). The query helper still returns empty so callers compose + /// uniformly; it is Config::load() that reports the problem. + QString trashQuery() const; + + /// Matches this account's inbox folder, using inboxFolder(). + QString inboxQuery() const; + + /// The inbox folder name, which is `inbox` when set and "Inbox" + /// otherwise. Never empty, so a caller always has a folder to name. + QString inboxFolder() const; }; /// A named query, stored in queries.json. @@ -275,6 +312,13 @@ public: /// open-coded at the call site. QString allSentQuery() const; + /// Matches every configured account's trash, or empty when none has one. + /// + /// Joins only the NON-EMPTY trashQuery() results, for the same reason + /// allSentQuery() does: notmuch accepts a bare "or" without complaint and + /// silently answers a different question. + QString allTrashQuery() const; + /// Matches every configured account's drafts, or empty when none has one. /// /// Joins only the NON-EMPTY draftsQuery() results, for the same reason diff --git a/src/keymap.cpp b/src/keymap.cpp index c731bbb..76c6b60 100644 --- a/src/keymap.cpp +++ b/src/keymap.cpp @@ -31,6 +31,8 @@ QStringList KeyMap::knownActions() QStringLiteral("open_thread"), QStringLiteral("archive"), QStringLiteral("delete"), + QStringLiteral("restore"), + QStringLiteral("cleanup_stranded"), QStringLiteral("spam"), QStringLiteral("toggle_unread"), QStringLiteral("mark_all_read"), @@ -97,7 +99,31 @@ QList<QPair<QString, QString>> KeyMap::defaultBindings() { QStringLiteral("Alt+Up"), QStringLiteral("prev_thread") }, { QStringLiteral("Return"), QStringLiteral("open_thread") }, { QStringLiteral("Ctrl+E"), QStringLiteral("archive") }, + // Del FIRST, and the order matters twice over. defaultSequenceFor() + // returns the first match, and sequenceFor() prefers any binding that + // is not that default, treating it as a user override; listing Del + // second therefore made it the "override" of Ctrl+D and left the two + // functions disagreeing about which key the menus should advertise. + // First also makes it the ADVERTISED one, which is the point: it is + // the key a user reaches for, and Ctrl+D is not a guess anyone makes. + // + // Bare, which is safe for a reason that does NOT generalise to other + // bare keys. Delete is not a letter, so Qt's protection for editable + // widgets does not cover it, but QLineEdit accepts the + // ShortcutOverride for Delete itself, because it is one of its own + // editing keys. Return is not, which is why that one needed an + // explicit filter in MainWindow::eventFilter() and this one does not. + // Measured both ways; see theDeleteKeyEditsTextInTheQueryBar(). + { QStringLiteral("Del"), QStringLiteral("delete") }, { QStringLiteral("Ctrl+D"), QStringLiteral("delete") }, + // Restore is only enabled in the trash view, so its key is dead + // elsewhere rather than doing something surprising. + { QStringLiteral("Ctrl+R"), QStringLiteral("restore") }, + // Item 103's cleanup. A chord rather than a plain key: it replaces the + // whole view, and it is reached from a menu far more often than from + // the keyboard. Ctrl+Shift+D is message_details and Ctrl+Alt+D is + // delete_thread, so this takes the T of "trash". + { QStringLiteral("Ctrl+Alt+T"), QStringLiteral("cleanup_stranded") }, { QStringLiteral("Ctrl+Shift+S"), QStringLiteral("spam") }, { QStringLiteral("Ctrl+U"), QStringLiteral("toggle_unread") }, // Shifted against Ctrl+U, which toggles unread on the selection: this @@ -246,7 +272,7 @@ QKeySequence KeyMap::sequenceFor(const QString &action) const if (it.value() != action) continue; - const bool isBuiltIn = !builtIn.isEmpty() && it.key() == builtIn; + const bool isBuiltIn = isDefaultBinding(it.key(), action); if (best.isEmpty()) { best = it.key(); bestIsBuiltIn = isBuiltIn; @@ -256,6 +282,13 @@ QKeySequence KeyMap::sequenceFor(const QString &action) const if (bestIsBuiltIn && !isBuiltIn) { best = it.key(); bestIsBuiltIn = false; + } else if (bestIsBuiltIn && isBuiltIn) { + // Both are defaults, so the ADVERTISED one is whichever + // defaultBindings() lists first: that order is the author's + // preference and is why Del is listed before Ctrl+D. Falling back + // to alphabetical here would advertise Ctrl+D instead. + if (it.key() == builtIn) + best = it.key(); } else if (bestIsBuiltIn == isBuiltIn && it.key().toString() < best.toString()) { best = it.key(); @@ -264,6 +297,27 @@ QKeySequence KeyMap::sequenceFor(const QString &action) const return best; } +bool KeyMap::isDefaultBinding(const QKeySequence &sequence, + const QString &action) +{ + // ANY of the action's defaults, not just the first. + // + // An action can ship with more than one binding: `delete` has Del and + // Ctrl+D. sequenceFor() compares against defaultSequenceFor(), which + // returns only the first, so the second looked like a USER binding and + // won the "a user binding always beats the default" rule. The menus then + // advertised Ctrl+D for a user who had configured nothing, and + // sequenceFor() and defaultSequenceFor() disagreed about an untouched + // action. + for (const auto &binding : defaultBindings()) { + if (binding.second == action + && normalizeSequence(binding.first) == sequence) { + return true; + } + } + return false; +} + QKeySequence KeyMap::defaultSequenceFor(const QString &action) { for (const auto &binding : defaultBindings()) { diff --git a/src/keymap.h b/src/keymap.h index 81e1813..8774209 100644 --- a/src/keymap.h +++ b/src/keymap.h @@ -73,6 +73,14 @@ public: /// The built-in sequence for an action, ignoring any user override. static QKeySequence defaultSequenceFor(const QString &action); + /// Whether `sequence` is ANY of `action`'s default bindings. + /// + /// Not the same question as `sequence == defaultSequenceFor(action)`: an + /// action can ship several, and comparing against only the first makes the + /// others look like user overrides. + static bool isDefaultBinding(const QKeySequence &sequence, + const QString &action); + /// Every action name carrying a built-in binding. static QStringList defaultActions(); diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 594535b..58c82ca 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -214,7 +214,7 @@ void MainWindow::closeEvent(QCloseEvent *event) // Degrade to a warning rather than offering a sync that cannot run. const auto answer = QMessageBox::warning( this, tr("Unsynced changes"), - tr("%n tag change(s) have not been synced, and no sync command " + tr("%n change(s) have not been synced, and no sync command " "is configured. Quit anyway?", "", pendingEditCount()), QMessageBox::Discard | QMessageBox::Cancel, QMessageBox::Cancel); @@ -228,7 +228,7 @@ void MainWindow::closeEvent(QCloseEvent *event) QMessageBox box(this); box.setIcon(QMessageBox::Question); box.setWindowTitle(tr("Unsynced changes")); - box.setText(tr("%n tag change(s) have not been synced.", "", + box.setText(tr("%n change(s) have not been synced.", "", pendingEditCount())); box.setInformativeText(tr("Sync before quitting?")); QPushButton *sync = @@ -309,6 +309,18 @@ bool MainWindow::eventFilter(QObject *watched, QEvent *event) keyEvent->accept(); return true; } + // Delete needs NO entry here, and that is worth stating because the + // reasoning that says it does is nearly right. It is bound bare to + // `delete`, and Qt's protection for editable widgets covers plain + // LETTERS only, so by the same argument that made Return a problem it + // should trigger the action while the user edits a query. + // + // It does not, because QLineEdit accepts the ShortcutOverride for + // Delete itself: Delete is one of its own editing keys, which Return + // is not. Measured both ways, with this branch present and absent: + // the action fires 0 times either way and the text is edited either + // way. Adding a guard here would be dead code carrying a test that + // cannot fail. } return QMainWindow::eventFilter(watched, event); @@ -841,10 +853,23 @@ void MainWindow::registerActions() // is about reading a message at all. const bool allDeleted = everySelectedRowHasTag(QStringLiteral("deleted")); + // Item 103. A MOVE now, not only a tag: Delete used to add `deleted` + // and leave the file exactly where it was, so deleted mail sat in the + // inbox indefinitely and only the chip said otherwise. if (allDeleted) - tagSelected({}, { QStringLiteral("deleted") }, tr("Undelete")); + restoreSelected(); else - tagSelected({ QStringLiteral("deleted") }, {}, tr("Delete")); + trashSelected(); + }); + addAction(QStringLiteral("restore"), tr("&Restore from trash"), + tr("Move the selected messages out of the trash"), [this]() { + restoreSelectedFromTrash(); + }); + addAction(QStringLiteral("cleanup_stranded"), + tr("Find &stranded deleted mail"), + tr("Show mail tagged deleted that is not in a trash folder"), + [this]() { + showStrandedDeletedMail(); }); addAction(QStringLiteral("spam"), tr("Mark &spam"), tr("Add spam and remove inbox"), [this]() { @@ -930,12 +955,19 @@ void MainWindow::registerActions() }); addAction(QStringLiteral("delete_thread"), tr("&Delete thread"), tr("Add or remove the deleted tag on whole threads"), [this]() { + // A MOVE now, like its message-scoped twin. It tagged and moved + // nothing until item 103's follow-up, so "Delete thread" left a whole + // conversation sitting in the inbox wearing a `deleted` chip: exactly + // the half-deleted state Delete stopped producing. + // + // The direction is read per MESSAGE, not from the thread's tag union. + // A thread whose root was deleted on its own carries `deleted` in the + // union while its replies do not, and asking the union there ran + // Delete a second time on messages already in the trash. if (everySelectedRowHasTag(QStringLiteral("deleted"), TagScope::Thread)) { - tagSelected({}, { QStringLiteral("deleted") }, - tr("Undelete thread"), TagScope::Thread); + restoreSelectedThreads(); } else { - tagSelected({ QStringLiteral("deleted") }, {}, tr("Delete thread"), - TagScope::Thread); + trashSelectedThreads(); } }); addAction(QStringLiteral("spam_thread"), tr("Mark thread as &spam"), @@ -1126,6 +1158,11 @@ void MainWindow::buildMenus() auto *messageMenu = menuBar()->addMenu(tr("&Message")); messageMenu->addAction(m_actions.value(QStringLiteral("archive"))); messageMenu->addAction(m_actions.value(QStringLiteral("delete"))); + // Beside Delete, whose inverse it is. Greyed outside the trash view + // rather than hidden: an action that vanishes teaches nothing, while a + // disabled entry with its shortcut beside it says both that it exists and + // where it applies. + messageMenu->addAction(m_actions.value(QStringLiteral("restore"))); messageMenu->addAction(m_actions.value(QStringLiteral("spam"))); messageMenu->addSeparator(); messageMenu->addAction(m_actions.value(QStringLiteral("toggle_unread"))); @@ -1137,11 +1174,23 @@ void MainWindow::buildMenus() // Separated from the entries above: those act on the selection, this edits // a rule store shared with mailctl and changes nothing that is on screen. messageMenu->addSeparator(); + // A MENU entry and nothing else, at the user's request: "the cleanup + // should be a menu entry only, not to be confused with the filter Trash". + // It replaces the whole view like a filter does, so a sixth button beside + // the five filters would read as one of them. + messageMenu->addAction(m_actions.value(QStringLiteral("cleanup_stranded"))); messageMenu->addAction(m_actions.value(QStringLiteral("tag_rules"))); auto *viewMenu = menuBar()->addMenu(tr("&View")); viewMenu->addAction(m_actions.value(QStringLiteral("prev_thread"))); viewMenu->addAction(m_actions.value(QStringLiteral("next_thread"))); + viewMenu->addAction(m_actions.value(QStringLiteral("open_thread"))); + viewMenu->addSeparator(); + // The two clears. Both shipped keyboard-only, which is what + // everyActionIsReachableFromAMenu() exists to stop: an action reachable + // only by a chord is an action nobody discovers. + viewMenu->addAction(m_actions.value(QStringLiteral("clear_pane"))); + viewMenu->addAction(m_actions.value(QStringLiteral("clear_selection"))); viewMenu->addSeparator(); viewMenu->addAction(m_actions.value(QStringLiteral("toggle_html"))); viewMenu->addAction(m_actions.value(QStringLiteral("load_remote"))); @@ -1183,6 +1232,13 @@ void MainWindow::buildMenus() // control: two buttons with different consequences looked identical. { QStringLiteral("archive"), QStringLiteral("mail-archive") }, { QStringLiteral("delete"), QStringLiteral("edit-delete") }, + // The inverse of delete, and the theme's own name for it: the icon + // every desktop uses for taking something back out of the wastebasket. + { QStringLiteral("restore"), QStringLiteral("edit-undelete") }, + // A SEARCH, not a delete. The action reports what it finds and moves + // nothing, so an icon from the delete family would promise the one + // thing it deliberately does not do. + { QStringLiteral("cleanup_stranded"), QStringLiteral("system-search") }, { QStringLiteral("undo"), QStringLiteral("edit-undo") }, { QStringLiteral("spam"), QStringLiteral("mail-mark-junk") }, { QStringLiteral("flag"), QStringLiteral("mail-mark-important") }, @@ -1249,6 +1305,7 @@ void MainWindow::buildMenus() m_threadContextMenu->setObjectName(QStringLiteral("threadContextMenu")); m_threadContextMenu->addAction(m_actions.value(QStringLiteral("archive"))); m_threadContextMenu->addAction(m_actions.value(QStringLiteral("delete"))); + m_threadContextMenu->addAction(m_actions.value(QStringLiteral("restore"))); m_threadContextMenu->addAction(m_actions.value(QStringLiteral("spam"))); m_threadContextMenu->addSeparator(); m_threadContextMenu->addAction(m_actions.value(QStringLiteral("toggle_unread"))); @@ -1604,6 +1661,15 @@ void MainWindow::wireWorker() connect(m_worker, &NotmuchWorker::tagsApplied, this, &MainWindow::onTagsApplied); + // messagesMovedFrom rather than messagesMoved: the tags a move carries can + // only be resolved once the origins are known, and that signal is the one + // that reports them. + connect(m_worker, &NotmuchWorker::messagesMovedFrom, + this, &MainWindow::onMessagesMoved); + + connect(m_worker, &NotmuchWorker::threadMessagesResolved, + this, &MainWindow::onThreadMessagesResolved); + m_workerThread.start(); // Queued behind the thread start, so the completer has real tags as soon @@ -1893,6 +1959,7 @@ void MainWindow::buildSavedQueryRow(QWidget *parent, QVBoxLayout *layout) { QStringLiteral("inbox"), QStringLiteral("mail-inbox") }, { QStringLiteral("flagged"), QStringLiteral("starred") }, { QStringLiteral("sent"), QStringLiteral("mail-folder-sent") }, + { QStringLiteral("trash"), QStringLiteral("user-trash") }, }; button->setIcon( QIcon::fromTheme(filterIcons.value(filter.generated))); @@ -2439,8 +2506,40 @@ void MainWindow::onQueryFinished(int total, quint64 generation) applyPendingRecovery(); } +bool MainWindow::isShowingTrash() const +{ + // Compared against the trash GENERATOR's query, not against the word + // "trash" or against a tag. The trash view is path-based so that mail + // trashed by another client shows up in it; deciding this from + // `tag:deleted` instead would disable Restore on exactly the messages + // that most need it, which is the case Restore's fallback exists for. + // + // Both scopes, because the view composes with the account dropdown like + // every other filter: one account's trash, or all of them. + const QString query = m_lastQuery.trimmed(); + if (query.isEmpty()) + return false; + + const QString all = m_config.allTrashQuery().trimmed(); + if (!all.isEmpty() && query == all) + return true; + + for (const Account &account : m_config.accounts()) { + const QString trash = account.trashQuery().trimmed(); + if (!trash.isEmpty() && query == trash) + return true; + } + return false; +} + void MainWindow::updateViewWideActions() { + // Only meaningful on mail that is actually in a trash folder. An enabled + // action that does nothing is worse than an absent one, and Restore + // outside the trash has nothing to restore from. + if (QAction *action = m_actions.value(QStringLiteral("restore"))) + action->setEnabled(isShowingTrash()); + // Threads arrive in batches of kBatchSize, so before the query reports its // total the model holds only what has landed. An action that says "all" // must not run against a partial set and silently skip the rest, and a @@ -2921,6 +3020,21 @@ bool MainWindow::aSyncHoldsTheWriteLock() const void MainWindow::flushHeldEdits() { + // Moves first, and they are flushed even when no tag edit is waiting: the + // early return below used to be the whole guard, so a held move with an + // empty edit queue would never have been sent at all. That is item 106's + // data loss with a worse shape, since a dropped move leaves the file where + // the user asked it not to be. + if (!m_heldMoves.isEmpty()) { + const QVector<HeldMove> moves = m_heldMoves; + m_heldMoves.clear(); + for (const HeldMove &move : moves) { + sendMove(move.messageIds, move.destFolder, move.add, move.remove, + move.description, move.fromUndo); + } + updatePendingIndicator(); + } + if (m_heldEdits.isEmpty()) return; @@ -3731,7 +3845,15 @@ int MainWindow::pendingEditCount() const // Each held edit counts as one whatever its size, since it carries thread // ids rather than message ids and cannot be netted against the map. const int held = int(m_heldEdits.size()); - return m_pendingTagEdits.size() + m_unnettablePendingEdits + held; + // Held MOVES count for exactly the same reason, and were missed. With no + // tag edit queued the count was 0, so the indicator stayed hidden and + // closeEvent()'s `pendingEditCount() > 0` guard never fired: a Delete + // pressed during a sync was discarded on quit with no prompt at all. That + // is item 106's data loss, and worse here, because a dropped move leaves + // the file in the folder the user asked it out of. + const int heldMoves = int(m_heldMoves.size()); + return m_pendingTagEdits.size() + m_unnettablePendingEdits + held + + heldMoves; } void MainWindow::updatePendingIndicator() @@ -3746,7 +3868,7 @@ void MainWindow::updatePendingIndicator() // they did, not the writes it became. m_pendingLabel->setText(tr("%n unsynced change(s)", "", pending)); m_pendingLabel->setToolTip( - tr("Tag changes made here that a sync has not yet carried to the mail " + tr("Changes made here that a sync has not yet carried to the mail " "store. An external notmuch run can clear them without this count " "noticing.")); m_pendingLabel->show(); @@ -3870,22 +3992,43 @@ bool MainWindow::everySelectedRowHasTag(const QString &tag, } else if (m_model->isMessageRow(index)) { tags = m_model->messageAt(index).tags; } else { - // The thread's summary, and this is a KNOWN approximation rather - // than an oversight. A thread row acts on the message its card - // displays, but that message's own tags are never in the model: - // setThreadMessages drops depth 0 because the root row stands for - // it, so there is no node to read and messageById() cannot find - // one. The summary is a union over the thread, so it answers - // "unread" while ANY message is. + // A thread row answers about the MESSAGE ITS CARD DISPLAYS, which + // is what it acts on. threadFor() already substitutes that + // message's own tags for the thread's union when they are known + // (item 110), so this reads the row's real state rather than a + // union over messages it does not stand for. // - // The consequence is bounded and only affects the DIRECTION a - // toggle picks, never what it writes: on a thread whose first - // message is read while a later one is not, Toggle unread reads - // the thread as unread and marks the first message read again, a - // no-op. Fixing it properly needs per-message state in - // ThreadSummary, which is the same thing item 87 needs; leave it - // for that item rather than guessing here. - tags = m_model->threadFor(index).tags; + // This used to read the union deliberately, with a comment + // calling the imprecision bounded because no per-message tags + // existed in the model. They do now: ThreadSummary carries + // firstMessageTags from the query, so an UNEXPANDED row already + // knows its own tags, and the comment outlived the fact. + // + // The cost of the union was not bounded once Delete became a + // MOVE. Deleting the root of a three-message thread left the two + // replies undeleted, so the union carried no `deleted`, so a + // second press read the row as not-deleted and deleted it AGAIN: + // the message was moved trash-to-trash and came out carrying + // `deleted`, `deleted-from:inbox` and `deleted-from:Trash` at + // once, with no way back. A tag toggle merely re-applied a tag it + // already had; a move re-applies the MOVE. + // + // Resolved through messageById() on the row's own message, which + // is the id messageScopeFor() will act on. Asking the same + // question the write asks is what keeps the direction and the + // write from disagreeing; the union answered a question about a + // conversation when the row stands for one message. + const ThreadSummary summary = m_model->threadFor(index); + const MessageNode own = + m_model->messageById(summary.firstMessageId); + // messageById() and NOT summary.firstMessageTags, which is the + // value the QUERY delivered and is not refreshed by an optimistic + // update: applyMessageTagChange() writes the row's node, so after + // a delete the node reads `deleted, deleted-from:inbox` while the + // summary still reads `inbox, unread`. Measured, and preferring + // the summary left this defect exactly as it was. + tags = own.messageId.isEmpty() ? summary.firstMessageTags + : own.tags; } if (!tags.contains(tag)) return false; @@ -4081,6 +4224,798 @@ void MainWindow::sendMessageTagChange(const QStringList &messageIds, Q_ARG(TagChange, m_pendingChange)); } +const QString &MainWindow::kOriginTagPlaceholder() +{ + // Not wrapped in tr(). It is never displayed: onMessagesMoved() replaces + // it with a real tag before anything reaches the worker, and a translated + // placeholder would stop matching in the one locale that translated it, + // which is the trap CLAUDE.md records for startup_query. + static const QString placeholder = + QStringLiteral("\x01qtmaildir-origin-placeholder"); + return placeholder; +} + +Account MainWindow::accountForMessagePath(const QString &path) const +{ + // From the PATH, not from the thread's account tag. The tag is optional + // config, so resolving through it would silently disable Delete for an + // account that never set one; a message's maildir prefix is what makes it + // belong to an account at all. + // + // Longest maildir wins, so nested account maildirs (`mail` and + // `mail/work`) resolve to the more specific one rather than to whichever + // happens to be listed first. + // + // BOTH path shapes are accepted, and that is not defensive coding. A + // thread row's path comes from ThreadSummary::firstMessagePath and is + // database-RELATIVE; a reply row's comes from MessageNode::filePath and is + // ABSOLUTE, because MimeParser has to open it. Matching only the relative + // form resolved every reply to no account, so Delete on a reply reported + // "no trash folder configured" and moved nothing, which is exactly the + // thread-row/reply-row asymmetry this file has been bitten by before. + // + // A `/` is required after the maildir in both cases, so `acctX` cannot + // match an account whose maildir is `acct`. + Account best; + int bestLength = -1; + for (const Account &account : m_config.accounts()) { + if (account.maildir.isEmpty()) + continue; + const QString segment = QLatin1Char('/') + account.maildir + + QLatin1Char('/'); + const bool matches = + path.startsWith(account.maildir + QLatin1Char('/')) + || path.contains(segment); + if (!matches) + continue; + if (account.maildir.length() > bestLength) { + best = account; + bestLength = account.maildir.length(); + } + } + return best; +} + +void MainWindow::trashSelected() +{ + const QModelIndexList rows = + m_threadView->selectionModel()->selectedRows(); + if (rows.isEmpty()) + return; + + // Message scope, exactly as tagSelected() uses by default: a thread row + // stands for the ONE message its card displays. Escalating to the thread + // would move a whole conversation into the trash because the user deleted + // one reply. + const ActionScope scope = m_model->messageScopeFor(rows); + if (scope.messageIds.isEmpty()) + return; + + QHash<QString, QString> pathById; + for (const QString &messageId : scope.messageIds) + pathById.insert(messageId, m_model->messageById(messageId).filePath); + + trashMessages(scope.messageIds, pathById, scope.messageCount); +} + +void MainWindow::trashMessages(const QStringList &messageIds, + const QHash<QString, QString> &pathById, + int messageCount, + const QStringList &wholeThreadIds) +{ + if (messageIds.isEmpty()) + return; + + // Grouped by destination, because moveMessages() takes one folder per call + // and a selection can span accounts with different trash folders. + // + // Paths are passed IN rather than read from the model, because the thread + // path arrives with messages the model has never seen: a thread the user + // never expanded holds no node for its replies, so a lookup there returns + // nothing and every message resolves to no account. + QHash<QString, QStringList> byTrash; + QStringList unconfigured; + for (const QString &messageId : messageIds) { + const Account account = + accountForMessagePath(pathById.value(messageId)); + if (account.trash.isEmpty()) { + unconfigured.append(messageId); + continue; + } + byTrash[account.maildir + QLatin1Char('/') + account.trash] + .append(messageId); + } + + // Task 2 warns at config load; this is the second line of defence, for a + // user who never fixed it. Reported rather than silently doing nothing, + // and NOT tagged either: a `deleted` tag on a file still in the inbox is + // precisely the half-done state this item removes. + if (!unconfigured.isEmpty()) { + m_statusLabel->setText( + tr("%n message(s) could not be deleted: no trash folder is " + "configured for their account.", "", int(unconfigured.size()))); + } + + if (byTrash.isEmpty()) + return; + + for (auto it = byTrash.cbegin(); it != byTrash.cend(); ++it) { + sendMove(it.value(), it.key(), + { QStringLiteral("deleted"), kOriginTagPlaceholder() }, {}, + tr("Delete"), false, wholeThreadIds); + } + + showTransientStatus( + tr("%1: %n message(s)", "", messageCount).arg(tr("Delete"))); +} + +QString MainWindow::originTagFor(const QString &dbRelativeFolder) const +{ + // `acct/inbox` becomes `deleted-from:inbox`. The tag stores the folder + // relative to the ACCOUNT, never to the database: the account prefix is + // recomposed from the message's own path when it is read back, so storing + // it would duplicate it and would go stale the day the user renames a + // maildir. + // + // Shared by the two sites that need the tag, rather than derived twice. + // They disagreed once already: onMessagesMoved() resolved a placeholder + // from the folder the worker reported, which on a RESTORE is the trash + // rather than the origin, so the restore stripped `deleted-from:Trash` + // and left the real tag in place. + const Account account = + accountForMessagePath(dbRelativeFolder + QLatin1Char('/')); + QString accountRelative = dbRelativeFolder; + if (!account.maildir.isEmpty() + && dbRelativeFolder.startsWith(account.maildir + QLatin1Char('/'))) { + accountRelative = dbRelativeFolder.mid(account.maildir.length() + 1); + } + if (accountRelative.isEmpty()) + return QString(); + return QStringLiteral("deleted-from:%1").arg(accountRelative); +} + +QStringList MainWindow::selectedThreadIds() const +{ + // A THREAD action on a reply row means that reply's conversation. + // + // scopeFor() reports a reply under messageIds and leaves threadIds empty, + // which is right for the mixed selections it was built for and wrong as + // the only input to a thread-scoped action: the early return on an empty + // threadIds made Delete thread do nothing at all when the selected row + // happened to be a reply. threadFor() resolves either kind of row. + const QModelIndexList rows = + m_threadView->selectionModel()->selectedRows(); + QStringList threadIds; + for (const QModelIndex &index : rows) { + const QString threadId = m_model->threadFor(index).threadId; + if (!threadId.isEmpty() && !threadIds.contains(threadId)) + threadIds.append(threadId); + } + return threadIds; +} + +void MainWindow::trashSelectedThreads() +{ + const QModelIndexList rows = + m_threadView->selectionModel()->selectedRows(); + if (rows.isEmpty()) + return; + + const QStringList threadIds = selectedThreadIds(); + if (threadIds.isEmpty()) + return; + + // Asked of the WORKER rather than resolved here. A thread the user never + // expanded has no nodes in the model for its replies, so the ids and the + // paths a move needs exist only in the database. applyTagsToThreads() + // solves the same problem the same way, for the same reason. + // + // Repainted HERE, synchronously, before the worker is asked. + // + // The move needs message ids and paths that only the database holds for an + // unexpanded thread, so the move itself is asynchronous. The DISPLAY must + // not wait for that round trip: the card is what the user watches, and + // holding it back is what made a deleted thread sit unchanged until it was + // clicked. It also keeps the toggle's direction readable immediately, so a + // second press restores rather than deleting again. + for (const QString &threadId : threadIds) + m_model->applyTagChange(threadId, { QStringLiteral("deleted") }, {}); + + m_pendingThreadScope = threadIds; + QMetaObject::invokeMethod(m_worker, "resolveThreadMessages", + Qt::QueuedConnection, + Q_ARG(QStringList, threadIds), + Q_ARG(QString, QStringLiteral("delete_thread"))); +} + +void MainWindow::onThreadMessagesResolved(const QStringList &messageIds, + const QStringList &paths, + const QStringList &tags, + const QString &requestTag) +{ + if (messageIds.size() != paths.size() || messageIds.size() != tags.size()) + return; + + QHash<QString, QString> pathById; + for (int i = 0; i < messageIds.size(); ++i) + pathById.insert(messageIds.at(i), paths.at(i)); + + const QStringList threadScope = m_pendingThreadScope; + m_pendingThreadScope.clear(); + + if (requestTag == QStringLiteral("delete_thread")) { + trashMessages(messageIds, pathById, messageIds.size(), threadScope); + return; + } + + if (requestTag == QStringLiteral("restore_messages")) { + restoreResolvedMessages(messageIds, paths, tags); + return; + } + + if (requestTag != QStringLiteral("undelete_thread")) + return; + + // Restore, resolved per message: each one goes back to the folder its own + // `deleted-from:` tag names, so a thread whose messages were deleted from + // different folders reassembles correctly rather than collapsing into one. + const QString prefix = QStringLiteral("deleted-from:"); + QHash<QString, QStringList> byOrigin; + QStringList unknown; + for (int i = 0; i < messageIds.size(); ++i) { + // Split on TAB, matching resolveThreadMessages(). A space is not a + // safe separator: a folder name containing one produces a tag + // containing one, and splitting there silently truncates the origin + // to its first word. + const QStringList messageTags = + tags.at(i).split(QLatin1Char('\t'), Qt::SkipEmptyParts); + QString origin; + for (const QString &tag : messageTags) { + if (tag.startsWith(prefix)) { + origin = tag.mid(prefix.length()); + break; + } + } + // A message with no `deleted` tag is not in the trash and has nothing + // to come back from. A thread-scoped restore reaches every message, + // including ones the user never deleted, and moving those would drag + // untouched mail out of whatever folder it legitimately sits in. + if (!messageTags.contains(QStringLiteral("deleted"))) + continue; + const Account account = + accountForMessagePath(paths.at(i)); + if (origin.isEmpty() || account.maildir.isEmpty()) { + unknown.append(messageIds.at(i)); + continue; + } + byOrigin[account.maildir + QLatin1Char('/') + origin] + .append(messageIds.at(i)); + } + + if (!unknown.isEmpty()) { + // No origin recorded: deleted by an older version or tagged by hand. + // The tag comes off so the row stops claiming to be deleted, but no + // file moves, since guessing a folder would put the message somewhere + // the user never had it. + sendMessageTagChange(unknown, {}, { QStringLiteral("deleted") }, + tr("Undelete thread")); + m_undoStack.push(new MessageTagCommand(this, unknown, {}, + { QStringLiteral("deleted") }, + tr("Undelete thread"))); + } + + for (auto it = byOrigin.cbegin(); it != byOrigin.cend(); ++it) { + // The origin tag is named here, not left as the placeholder: on a + // restore the placeholder would resolve to the folder the message is + // coming FROM, which is the trash, and strip a tag never written. + const QString origin = originTagFor(it.key()); + QStringList remove{ QStringLiteral("deleted") }; + if (!origin.isEmpty()) + remove.append(origin); + sendMove(it.value(), it.key(), {}, remove, tr("Undelete thread"), + false, threadScope); + } + + showTransientStatus(tr("%1: %n message(s)", "", messageIds.size()) + .arg(tr("Undelete thread"))); +} + +void MainWindow::restoreSelectedThreads() +{ + const QModelIndexList rows = + m_threadView->selectionModel()->selectedRows(); + if (rows.isEmpty()) + return; + + const QStringList threadIds = selectedThreadIds(); + if (threadIds.isEmpty()) + return; + + // Repainted synchronously, as the delete direction is. + for (const QString &threadId : threadIds) + m_model->applyTagChange(threadId, {}, { QStringLiteral("deleted") }); + + m_pendingThreadScope = threadIds; + QMetaObject::invokeMethod( + m_worker, "resolveThreadMessages", Qt::QueuedConnection, + Q_ARG(QStringList, threadIds), + Q_ARG(QString, QStringLiteral("undelete_thread"))); +} + +QString MainWindow::inboxFolderFor(const Account &account) const +{ + // Discovered from the account's OWN inbox query, never hardcoded. + // + // The casing is not ours to assume: the real Maildir has `Inbox` and a + // test fixture has `inbox`, and picking either would create a SECOND + // folder beside the real one on whichever side disagreed. That is exactly + // the failure a truncated origin folder caused on real mail this morning, + // and under mbsync's `Create Both` such a folder can reach the server. + // + // The inbox query is a generated `path:"<maildir>/<folder>/**"`, so the + // folder name is the part between the account prefix and the glob. + const QString query = account.inboxQuery(); + const QString prefix = + QStringLiteral("path:\"") + account.maildir + QLatin1Char('/'); + const QString suffix = QStringLiteral("/**\""); + if (query.startsWith(prefix) && query.endsWith(suffix)) { + const int from = prefix.length(); + const int length = query.length() - from - suffix.length(); + if (length > 0) + return query.mid(from, length); + } + + // No inbox configured for this account. `Inbox` is the Maildir + // convention and is what mbsync's own `Inbox` directive defaults to. + return QStringLiteral("Inbox"); +} + +void MainWindow::restoreResolvedMessages(const QStringList &messageIds, + const QStringList &paths, + const QStringList &tags) +{ + if (messageIds.size() != paths.size() || messageIds.size() != tags.size()) + return; + + const QString prefix = QStringLiteral("deleted-from:"); + QHash<QString, QStringList> byOrigin; + QHash<QString, QStringList> byInbox; + QStringList stranded; + + for (int i = 0; i < messageIds.size(); ++i) { + const QStringList messageTags = + tags.at(i).split(QLatin1Char('\t'), Qt::SkipEmptyParts); + QString origin; + for (const QString &tag : messageTags) { + if (tag.startsWith(prefix)) { + origin = tag.mid(prefix.length()); + break; + } + } + + const Account account = accountForMessagePath(paths.at(i)); + if (account.maildir.isEmpty()) { + stranded.append(messageIds.at(i)); + continue; + } + + if (origin.isEmpty()) { + // Trashed by another client, so there is no record of where it + // belongs. Inbox is the documented fallback, and it is reported: + // a guess the user is not told about is worse than the guess. + byInbox[account.maildir + QLatin1Char('/') + + account.inboxFolder()] + .append(messageIds.at(i)); + continue; + } + byOrigin[account.maildir + QLatin1Char('/') + origin] + .append(messageIds.at(i)); + } + + for (auto it = byOrigin.cbegin(); it != byOrigin.cend(); ++it) { + // The origin tag is named here rather than left as the placeholder, + // which onMessagesMoved() would resolve to the folder the message is + // coming FROM, namely the trash. + const QString origin = originTagFor(it.key()); + QStringList remove{ QStringLiteral("deleted") }; + if (!origin.isEmpty()) + remove.append(origin); + sendMove(it.value(), it.key(), {}, remove, tr("Restore")); + } + + for (auto it = byInbox.cbegin(); it != byInbox.cend(); ++it) { + sendMove(it.value(), it.key(), {}, { QStringLiteral("deleted") }, + tr("Restore")); + } + + if (!byInbox.isEmpty()) { + m_statusLabel->setText( + tr("%n message(s) had no record of where they came from and were " + "moved to the inbox.", "", int(byInbox.size()))); + } + if (!stranded.isEmpty()) { + m_statusLabel->setText( + tr("%n message(s) could not be restored: they belong to no " + "configured account.", "", int(stranded.size()))); + } +} + +void MainWindow::restoreSelectedFromTrash() +{ + const QModelIndexList rows = + m_threadView->selectionModel()->selectedRows(); + if (rows.isEmpty()) + return; + + const ActionScope scope = m_model->messageScopeFor(rows); + if (scope.messageIds.isEmpty()) + return; + + // Resolved by the WORKER, not read from the model. + // + // The model's tags come from the QUERY, and a row whose delete has not yet + // been re-queried still carries its pre-delete tags: measured + // `[inbox,unread]` on a message already in the trash, one run in three. + // The origin tag is then not found, the message falls into the + // no-origin branch, and Restore sends it to the INBOX instead of the + // folder it came from, silently and irreversibly. + // + // A restore has to be right about the destination or it is worse than + // doing nothing, so it asks the database rather than trusting a view that + // may be a moment behind. restoreSelectedThreads() already worked this + // way; this is the same reasoning applied to the message-scoped path. + m_pendingRestoreIds = scope.messageIds; + QMetaObject::invokeMethod( + m_worker, "resolveMessages", Qt::QueuedConnection, + Q_ARG(QStringList, scope.messageIds), + Q_ARG(QString, QStringLiteral("restore_messages"))); +} + +void MainWindow::showStrandedDeletedMail() +{ + // Not scoped to the selected account, deliberately. The stranded mail is + // an artefact of an old version rather than a view of anything, and the + // user wants to see all of it at once; the account dropdown is still there + // to narrow it by hand afterwards. + const QString trash = m_config.allTrashQuery(); + + // No account configures a trash folder: everything tagged `deleted` is by + // definition stranded, since there is nowhere for it to have gone. An + // empty exclusion must never be written as `not ()`, which notmuch parses + // without complaint and matches nothing, reporting a clean database. + const QString query = + trash.isEmpty() + ? QStringLiteral("tag:deleted") + : QStringLiteral("tag:deleted and not (%1)").arg(trash); + + // Into the bar, like a filter: what ran is visible and editable, and + // AlreadyScoped stops runQuery() wrapping it in the selected account's + // path, which would hide every other account's stranded mail. + m_queryEdit->setText(query); + runQuery(FlatResult::No, AccountScope::AlreadyScoped); + + // After runQuery(), which sets "Searching...": set before it, this would + // be overwritten and the user would be told nothing about what they are + // looking at. + m_statusLabel->setText(tr("Mail tagged deleted but not in a trash folder. " + "Select what should go and press Delete.")); +} + +void MainWindow::restoreSelected(bool fallbackToInbox) +{ + const QModelIndexList rows = + m_threadView->selectionModel()->selectedRows(); + if (rows.isEmpty()) + return; + + const ActionScope scope = m_model->messageScopeFor(rows); + if (scope.messageIds.isEmpty()) + return; + + // Where each message came from, read back off its own tag. This is what + // the tag exists for: the file has moved, so nothing on disk and nothing + // in notmuch still records the original folder. + const QString prefix = QStringLiteral("deleted-from:"); + QHash<QString, QStringList> byOrigin; + QStringList unknown; + for (const QString &messageId : scope.messageIds) { + const MessageNode node = m_model->messageById(messageId); + QString origin; + for (const QString &tag : node.tags) { + if (tag.startsWith(prefix)) { + origin = tag.mid(prefix.length()); + break; + } + } + // An account prefix is needed to name a folder to the worker, which + // works in database-relative paths. The origin tag stores the folder + // relative to the ACCOUNT, so the two are recomposed here. + const Account account = accountForMessagePath(node.filePath); + if (origin.isEmpty() || account.maildir.isEmpty()) { + unknown.append(messageId); + continue; + } + byOrigin[account.maildir + QLatin1Char('/') + origin].append(messageId); + } + + if (!unknown.isEmpty()) { + // No origin recorded. Two quite different situations reach here and + // they want opposite things, which is what `fallbackToInbox` selects. + // + // From the TRASH VIEW the message is demonstrably in the trash, put + // there by another client, and refusing to move it leaves the user + // looking at a message they cannot get out. Inbox is the documented + // fallback, and it is reported, because a guess the user is not told + // about is worse than the guess itself. + // + // From a second press of Delete the message is NOT in the trash: it is + // sitting wherever it always was, wearing a stale `deleted` tag from + // an older version or from a hand-written notmuch command. Moving it + // to the inbox there would relocate mail the user never asked to move. + // The tag comes off and the file stays put. + if (fallbackToInbox) { + QHash<QString, QStringList> byInbox; + QStringList stranded; + for (const QString &messageId : unknown) { + const Account account = + accountForMessagePath(m_model->messageById(messageId).filePath); + if (account.maildir.isEmpty()) { + stranded.append(messageId); + continue; + } + byInbox[account.maildir + QLatin1Char('/') + + inboxFolderFor(account)] + .append(messageId); + } + + for (auto it = byInbox.cbegin(); it != byInbox.cend(); ++it) { + sendMove(it.value(), it.key(), {}, + { QStringLiteral("deleted") }, tr("Restore")); + } + + if (!byInbox.isEmpty()) { + m_statusLabel->setText( + tr("%n message(s) had no record of where they came from " + "and were moved to the inbox.", "", + int(unknown.size() - stranded.size()))); + } + if (!stranded.isEmpty()) { + m_statusLabel->setText( + tr("%n message(s) could not be restored: they belong to no " + "configured account.", "", int(stranded.size()))); + } + } else { + sendMessageTagChange(unknown, {}, { QStringLiteral("deleted") }, + tr("Undelete")); + m_undoStack.push(new MessageTagCommand( + this, unknown, {}, { QStringLiteral("deleted") }, + tr("Undelete"))); + } + } + + for (auto it = byOrigin.cbegin(); it != byOrigin.cend(); ++it) { + // The origin tag is named HERE, not left as the placeholder. + // + // onMessagesMoved() resolves the placeholder from the origin the + // WORKER reports, which is where the message is coming FROM. On a + // delete that is the inbox and correct; on a restore it is the trash, + // so the placeholder resolved to `deleted-from:Trash` and asked to + // remove a tag that never existed, while the real `deleted-from:inbox` + // was never named. The message came home still claiming to have been + // deleted from somewhere, which then made Restore offer to move a + // message that was already back. + // + // A restore does not need the placeholder at all: the origin was just + // read off the message's own tag to decide where to send it, so the + // exact tag to strip is already known. Recomposed from the same + // account-relative form it was stored in. + const QString origin = originTagFor(it.key()); + QStringList remove{ QStringLiteral("deleted") }; + if (!origin.isEmpty()) + remove.append(origin); + sendMove(it.value(), it.key(), {}, remove, tr("Undelete")); + } + + showTransientStatus( + tr("%1: %n message(s)", "", scope.messageCount).arg(tr("Undelete"))); +} + +void MainWindow::sendMove(const QStringList &messageIds, + const QString &destFolder, const QStringList &add, + const QStringList &remove, + const QString &description, bool fromUndo, + const QStringList &wholeThreadIds) +{ + if (messageIds.isEmpty() || destFolder.isEmpty()) + return; + + // Held during a sync for the same reason every tag write is: the worker's + // read-write open BLOCKS on notmuch's exclusive lock rather than failing, + // so sending now would freeze the worker for the rest of the run. + // + // A move is held as the MOVE it is, not decomposed into a tag edit. The + // held-edit queue carries tag changes only, so a move pushed through it + // would apply the tags and never move the file, which is worse than + // waiting: the message would read as deleted and still be in the inbox. + if (aSyncHoldsTheWriteLock()) { + m_heldMoves.append(HeldMove{ messageIds, destFolder, add, remove, + description, fromUndo }); + m_statusLabel->setText( + tr("A sync is running; your change will be applied when it " + "finishes.")); + updatePendingIndicator(); + return; + } + + // Repainted NOW, before the worker is asked. + // + // The write itself waits for the move to be confirmed, and must: tagging + // the database first would leave a message marked deleted in a folder it + // never left if the rename failed. The DISPLAY has no such constraint, and + // holding it back until the round trip finished is what made a deleted row + // sit there unchanged until the user clicked it. The reply rows repainted + // and the root did not, because the replies were separately tagged while + // the root's card reads its thread's summary. + // + // Reverted by revertPendingTagChange() if the write is rejected, exactly + // as the tag path's optimistic update is. + // + // The placeholder is dropped rather than displayed: the real origin is not + // known until the worker answers, and a chip reading the placeholder's + // literal name would be worse than one chip arriving a moment late. + QStringList displayAdd; + for (const QString &tag : add) { + if (tag != kOriginTagPlaceholder()) + displayAdd.append(tag); + } + QStringList displayRemove; + for (const QString &tag : remove) { + if (tag != kOriginTagPlaceholder()) + displayRemove.append(tag); + } + // A thread-scoped move already repainted its rows in + // trashSelectedThreads() / restoreSelectedThreads(), synchronously, before + // the worker was asked to resolve the threads at all. Repeating it here + // would be harmless but redundant; more importantly the caller there needs + // the repaint to happen WITHOUT a worker round trip, which is the whole + // reason it is not done from this function. + // + // applyTagChange() is what those callers use, and applyMessageTagChange() + // is what this one uses, and the difference is not a style choice: the + // former moves the thread's SUMMARY, which a thread row's card draws from, + // while the latter deliberately leaves a multi-message thread's summary + // alone because one message's edit does not describe the conversation. + if (wholeThreadIds.isEmpty()) { + for (const QString &messageId : messageIds) + m_model->applyMessageTagChange(messageId, displayAdd, displayRemove); + } + + // What to tag once the move is CONFIRMED. Tagging now would leave a + // message marked deleted in a folder it never left if the rename failed. + // + // A QUEUE, not a map keyed on the destination: two Deletes in the same + // account before the first confirmation arrives both name `acct/Trash`, + // so the second insert overwrote the first and the second confirmation + // took an empty PendingMove. That file landed in the trash carrying + // neither `deleted` nor `deleted-from:`, which makes it unrestorable and + // invisible to a `tag:deleted` query. The worker handles one move at a + // time on its own thread and emits in the order it was asked, so a plain + // FIFO matches confirmations to requests without needing a key at all. + m_pendingMoves.enqueue(PendingMove{ add, remove, description, fromUndo }); + + QMetaObject::invokeMethod(m_worker, "moveMessages", Qt::QueuedConnection, + Q_ARG(QStringList, messageIds), + Q_ARG(QString, destFolder)); +} + +void MainWindow::onMessagesMoved(const QMap<QString, QString> &originByMessageId, + const QString &destFolder) +{ + if (m_pendingMoves.isEmpty()) + return; + const PendingMove pending = m_pendingMoves.dequeue(); + if (originByMessageId.isEmpty()) + return; + + // The origin differs per message, so the tags do too: two messages deleted + // from different folders get different `deleted-from:` tags out of one + // gesture. Grouped by the resolved tag list so identical ones still travel + // as a single write. + QHash<QString, QStringList> byOrigin; + for (auto it = originByMessageId.cbegin(); it != originByMessageId.cend(); + ++it) { + byOrigin[it.value()].append(it.key()); + } + + for (auto it = byOrigin.cbegin(); it != byOrigin.cend(); ++it) { + // The origin tag names the folder relative to the ACCOUNT, not to the + // database: `inbox`, never `acct/inbox`. Restore recomposes the + // account prefix from the message's own path, so storing it here would + // duplicate it, and a stored account prefix would go stale the day the + // user renames a maildir. + // + // The worker reports `acct/inbox`; the account's own maildir is + // `acct`, so the stored tag is `inbox`. Resolved through the first + // message's path, which is still the account's whichever folder it + // sits in now. + const QString originTag = originTagFor(it.key()); + + auto resolve = [&](const QStringList &tags) { + QStringList out; + for (const QString &tag : tags) { + if (tag != kOriginTagPlaceholder()) { + out.append(tag); + continue; + } + if (!originTag.isEmpty()) + out.append(originTag); + } + return out; + }; + + const QStringList resolvedAdd = resolve(pending.add); + const QStringList resolvedRemove = resolve(pending.remove); + sendMessageTagChange(it.value(), resolvedAdd, resolvedRemove, + pending.description); + + // The undo entry carries the RESOLVED tags, and is pushed per origin + // group rather than once for the batch. + // + // It used to be handed pending.add straight, which still holds the + // unresolved placeholder: undo then asked to remove a tag by that + // literal name, which no message carries, so the removal was a silent + // no-op and `deleted-from:inbox` survived the undo. The file came home + // still claiming to have been deleted from somewhere. Same defect as + // the one the second-Delete path had, reached through Ctrl+Z instead. + // + // Per group because the placeholder resolves to a DIFFERENT tag per + // origin: one command for a batch spanning two folders could only + // carry one of them, so the other would be the wrong tag rather than + // merely an unresolved one. + if (!pending.fromUndo) { + QMap<QString, QString> groupOrigins; + for (const QString &messageId : it.value()) + groupOrigins.insert(messageId, originByMessageId.value(messageId)); + m_undoStack.push(new MoveCommand(this, groupOrigins, destFolder, + resolvedAdd, resolvedRemove, + pending.description)); + } + } + + // A restore out of the TRASH VIEW leaves the row it came from showing a + // message that is no longer there, and only a refresh can say so. + // + // Reported from a hand test: the move was correct and the row sat in the + // list until the Trash filter was clicked again. The trash view is PATH + // based, so a restored message stops matching the query the list was built + // from, which is a state no tag change can express. Nothing else here + // removes a row, deliberately: in an ordinary view a deleted message's + // card should stay put, since one deleted reply does not doom the + // conversation. + // + // refreshCurrentQuery() rather than runCurrentQuery(): it clears nothing, + // so the selection, the expanded threads, the undo stack and the message + // being read all survive. Re-running the query outright would destroy the + // undo entry this function just pushed, which is the one thing a restore + // must leave intact. + // + // Gated on isShowingTrash() and not on the destination: a Delete is a move + // too and reaches this same slot, and refreshing after every delete would + // make a row vanish from under the user in every other view. + if (isShowingTrash()) + refreshCurrentQuery(); + + // The undo entries are pushed inside the loop above, one per origin + // group, because the placeholder resolves per origin. Nothing is pushed + // for a move the undo stack itself started: a MoveCommand is confirmed + // through this same slot, so pushing unconditionally left the undo of a + // Delete putting a fresh command on the stack instead of consuming the + // one it undid, and a second press of undo re-deleted the message. The + // flag rides on PendingMove because the answer has to survive the queued + // round trip; a window-wide "am I undoing" flag would long since have + // been cleared by the time the worker replies. +} + void MainWindow::sendThreadTagChange(const QStringList &threadIds, const QStringList &add, const QStringList &remove, diff --git a/src/mainwindow.h b/src/mainwindow.h index e741416..5b1621f 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -21,6 +21,7 @@ #include <QHash> #include <QSet> #include <QMainWindow> +#include <QQueue> #include <QPointer> #include <QThread> #include <QUndoCommand> @@ -739,6 +740,172 @@ private: const QStringList &remove, const QString &description); + /// Moves messages into `destFolder` and applies the tags that go with it. + /// + /// The counterpart to sendMessageTagChange() for the one action that is + /// not purely a tag change. Both trashSelected() and MoveCommand route + /// through this. + /// + /// The tags are NOT applied here: they are applied when the worker + /// confirms the move, in onMessagesMoved(). Tagging first would leave a + /// message marked `deleted` in a folder it never left if the rename + /// failed, which is the half-done state item 103 exists to remove. + /// + /// `add` may contain the placeholder kOriginTagPlaceholder, which + /// onMessagesMoved() replaces with `deleted-from:<origin>` per message. + /// The origin is not known until the worker reports it, and it differs per + /// message in a multi-row selection. + /// `fromUndo` marks a move the undo stack itself started, which must NOT + /// push a command of its own when it is confirmed. See onMessagesMoved(). + /// `wholeThreadIds`, when non-empty, says this move covers every message + /// of those threads, so the optimistic repaint updates each thread's + /// SUMMARY rather than each message's node. A thread row's card reads the + /// summary, so a thread-scoped move that updated only nodes repainted the + /// replies and left the root card stale until the next query. + void sendMove(const QStringList &messageIds, const QString &destFolder, + const QStringList &add, const QStringList &remove, + const QString &description, bool fromUndo = false, + const QStringList &wholeThreadIds = {}); + + /// Moves each selected row's message to its account's trash, tagging it + /// `deleted` and recording where it came from. + void trashSelected(); + + /// The half of trashSelected() that does the work, given the messages and + /// their paths. + /// + /// Paths are passed in rather than looked up, because the thread-scoped + /// caller has messages the MODEL has never seen: an unexpanded thread + /// holds no node for its replies, so a model lookup resolves them to no + /// account and the move is silently dropped. The worker supplies them. + void trashMessages(const QStringList &messageIds, + const QHash<QString, QString> &pathById, + int messageCount, + const QStringList &wholeThreadIds = {}); + + /// Moves every message of each selected THREAD to its account's trash. + /// + /// Asynchronous, unlike its message-scoped twin: the ids and paths of an + /// unexpanded thread's messages live only in the database, so this asks + /// the worker and finishes in onThreadMessagesResolved(). + void trashSelectedThreads(); + + /// The thread ids the selection covers, resolving a reply row to its own + /// thread. scopeFor() reports a reply under messageIds instead, which left + /// a thread action on a reply row doing nothing at all. + QStringList selectedThreadIds() const; + + /// The inverse of trashSelectedThreads(): moves every message of each + /// selected thread back where it came from. + void restoreSelectedThreads(); + + /// Runs the thread-scoped delete once the worker has resolved the + /// threads to messages. + void onThreadMessagesResolved(const QStringList &messageIds, + const QStringList &paths, + const QStringList &tags, + const QString &requestTag); + + /// The inverse: moves each selected row's message back to the folder its + /// `deleted-from:` tag names, stripping both tags. + /// + /// `fallbackToInbox` decides what happens to a message with NO origin tag, + /// and the two callers want opposite things. From the trash view the + /// message is demonstrably in the trash, trashed by another client, and + /// must still come out: it goes to the inbox, reported. From a second + /// press of Delete it is not in the trash at all and merely wears a stale + /// tag, so the tag comes off and the file stays where it is. + void restoreSelected(bool fallbackToInbox = false); + + /// Restore as reached from the TRASH VIEW: resolves each selected + /// message against the database first, then moves it. + /// + /// Asynchronous, unlike restoreSelected(), and that is the point. The + /// model's tags come from the query, so a row whose delete has not been + /// re-queried still carries its pre-delete tags; reading the origin from + /// there found none and sent the message to the INBOX instead of the + /// folder it came from, one run in three. + void restoreSelectedFromTrash(); + + /// Runs the query that finds mail tagged `deleted` whose file never left + /// its original folder, which is what every version before item 103 left + /// behind. It REPORTS and moves nothing: acting on its own would be a bulk + /// delete with no selection behind it, and the user asked for something + /// they could come back to and review. + /// + /// Repeatable rather than a one-time startup migration, for the same + /// reason: mail reaches this state again whenever another client tags + /// without moving. + void showStrandedDeletedMail(); + + /// Moves each resolved message home, using the tags and paths the WORKER + /// reported rather than anything the model holds. + void restoreResolvedMessages(const QStringList &messageIds, + const QStringList &paths, + const QStringList &tags); + + /// The messages a resolveMessages() request was made for. + QStringList m_pendingRestoreIds; + + /// The account's inbox FOLDER name, discovered from its inbox query. + /// + /// Never hardcoded: the real Maildir has `Inbox` and a fixture has + /// `inbox`, and assuming either would create a second folder beside the + /// real one on the side that disagreed. + QString inboxFolderFor(const Account &account) const; + + /// Whether the current query IS a trash view, for either scope. + /// + /// Compared against the trash generator's own query rather than against a + /// tag: the view is path-based so mail trashed by another client appears + /// in it, and such a message carries no tag of ours. + bool isShowingTrash() const; + + /// The `deleted-from:` tag naming `dbRelativeFolder`, or empty when no + /// account owns it. + /// + /// One rule for both sites that need the tag: the delete that writes it + /// and the restore that strips it. Deriving it twice let them disagree, + /// and a restore stripped a tag that had never been written. + QString originTagFor(const QString &dbRelativeFolder) const; + + /// The account whose maildir contains `path`, or an invalid account when + /// no configured maildir does. + /// + /// Resolved from the PATH rather than from the thread's account tag. The + /// tag is optional config, so an account without one would resolve to + /// nothing and silently disable Delete; the maildir prefix is what makes + /// a message belong to an account in the first place. + Account accountForMessagePath(const QString &path) const; + + /// Confirms a move: applies the tags the move was asked to carry, with the + /// origin placeholder resolved per message. + void onMessagesMoved(const QMap<QString, QString> &originByMessageId, + const QString &destFolder); + + /// What a move asked to be tagged, held until the worker confirms it. + /// + /// A FIFO and not a map keyed on the destination: two Deletes in one + /// account before the first confirmation arrives name the same folder, so + /// a keyed map dropped the first entry and left the second confirmation + /// with nothing to apply. That file reached the trash carrying neither + /// `deleted` nor `deleted-from:`, unrestorable and invisible to a + /// `tag:deleted` query. The worker moves one batch at a time and emits in + /// request order, so position alone matches a confirmation to its request. + struct PendingMove { + QStringList add; + QStringList remove; + QString description; + /// Set for a move the undo stack started, which must not push again. + bool fromUndo = false; + }; + QQueue<PendingMove> m_pendingMoves; + + /// The threads a resolveThreadMessages() request was made for, held until + /// the answer arrives so the optimistic repaint knows the move is + /// thread-scoped. + QStringList m_pendingThreadScope; + /// Undoes the optimistic model update for a write the worker rejected. void revertPendingTagChange(); @@ -791,10 +958,38 @@ private: /// it. Order matters: two edits touching one thread must reach the database /// in the order they were made, or the later one does not win. QVector<HeldEdit> m_heldEdits; + + /// A MOVE not yet sent, for the same reason a tag edit is held. + /// + /// A separate queue rather than an entry in m_heldEdits, because a move is + /// not a tag change and cannot be replayed as one: pushing it through the + /// edit queue would apply `deleted` and never move the file, leaving the + /// message reading as deleted while still sitting in the inbox. Item 106 + /// recorded what a dropped held edit costs, and a move dropped the same + /// way is worse: the tag lands and the file does not. + struct HeldMove { + QStringList messageIds; + QString destFolder; + QStringList add; + QStringList remove; + QString description; + /// Carried through the hold, or a move undone during a sync would + /// push a command when it is finally flushed. + bool fromUndo = false; + }; + QVector<HeldMove> m_heldMoves; + quint64 m_flushGeneration = 0; friend class ThreadTagCommand; friend class MessageTagCommand; + friend class MoveCommand; + + /// Stands in for `deleted-from:<origin>` between asking for a move and + /// learning where each message actually came from. Not a tag anyone ever + /// sees: onMessagesMoved() substitutes the real one per message before + /// anything is written. + static const QString &kOriginTagPlaceholder(); Config m_config; KeyMap m_keyMap; @@ -1192,3 +1387,73 @@ private: QString m_description; bool m_firstRedo = true; }; + +/// Undo entry for a message MOVE, which is a file rename plus a tag change. +/// +/// The destination is CARRIED rather than derived, and that is the whole +/// reason `deleted-from:` exists at all. A Maildir filename does not record +/// where a message came from, and once the file has moved notmuch cannot +/// answer either, so an undo that recomputed the origin would have nothing to +/// recompute it from. Each message carries its own, since one selection can +/// span folders and accounts. +/// +/// Grouped by destination: undoing a delete of five messages from three +/// folders is three moves, not five, because moveMessages() takes one folder +/// per call. +class MoveCommand : public QUndoCommand +{ +public: + /// `originByMessageId` names where each message came FROM, and + /// `destFolder` where they all went. + MoveCommand(MainWindow *window, + const QMap<QString, QString> &originByMessageId, + const QString &destFolder, const QStringList &add, + const QStringList &remove, const QString &description) + : QUndoCommand(description), m_window(window), + m_origins(originByMessageId), m_dest(destFolder), m_add(add), + m_remove(remove), m_description(description) {} + + /// The stack calls redo() when the command is pushed, by which point the + /// move has already been sent, so the first call is skipped. Same shape as + /// the two tag commands above. + void redo() override + { + if (m_firstRedo) { + m_firstRedo = false; + return; + } + // Also fromUndo: a redo replays a command that is ALREADY on the + // stack, so confirming it must not push a duplicate either. + m_window->sendMove(m_origins.keys(), m_dest, m_add, m_remove, + m_description, true); + } + + void undo() override + { + // Back to each message's OWN folder, one call per distinct + // destination. The tags invert with the direction: what the delete + // added, the undo removes. + QHash<QString, QStringList> byOrigin; + for (auto it = m_origins.cbegin(); it != m_origins.cend(); ++it) { + if (!it.value().isEmpty()) + byOrigin[it.value()].append(it.key()); + } + for (auto it = byOrigin.cbegin(); it != byOrigin.cend(); ++it) { + // fromUndo: this move is the undo, so its confirmation must not + // push a command of its own. Without it the stack grew on every + // press and a second undo re-deleted the message. + m_window->sendMove(it.value(), it.key(), m_remove, m_add, + QStringLiteral("Undo %1").arg(m_description), + true); + } + } + +private: + MainWindow *m_window; + QMap<QString, QString> m_origins; + QString m_dest; + QStringList m_add; + QStringList m_remove; + QString m_description; + bool m_firstRedo = true; +}; diff --git a/src/notmuchworker.cpp b/src/notmuchworker.cpp index b152830..2c03226 100644 --- a/src/notmuchworker.cpp +++ b/src/notmuchworker.cpp @@ -160,6 +160,39 @@ void walkReplies(notmuch_messages_t *messages, int depth, } } +/// The Maildir FOLDER a message file sits in, relative to the database root. +/// +/// `<root>/acct/inbox/cur/12345` becomes `acct/inbox`: the `cur`/`new` segment +/// is stripped because it is Maildir's read-state bookkeeping rather than part +/// of the folder's name, and moveMessages() takes a folder without one. That +/// makes the value round-trip: what comes out here can be handed straight back +/// to move a message home. +/// +/// Empty when the file is not under the root at all, which the caller treats as +/// "origin unknown" rather than guessing. A wrong folder here would send a +/// restored message somewhere the user never had it. +QString folderOfMessageFile(const QString &root, const QString &filePath) +{ + const QString rootPath = QDir(root).absolutePath(); + const QString dir = QFileInfo(filePath).absolutePath(); + + const QString relative = QDir(rootPath).relativeFilePath(dir); + // relativeFilePath happily walks upwards, so a path outside the root comes + // back as `../something` rather than as a failure. + if (relative.isEmpty() || relative == QStringLiteral(".") + || relative.startsWith(QStringLiteral("../"))) { + return QString(); + } + + QStringList parts = relative.split(QLatin1Char('/'), Qt::SkipEmptyParts); + if (!parts.isEmpty() + && (parts.last() == QStringLiteral("cur") + || parts.last() == QStringLiteral("new"))) { + parts.removeLast(); + } + return parts.join(QLatin1Char('/')); +} + } // namespace /// Registers SortOrder for queued calls, once, before main() runs. @@ -254,6 +287,13 @@ void NotmuchWorker::runQuery(const QString &query, quint64 generation, } NmThreads threads(rawThreads); + // Message paths are reported RELATIVE to this. An absolute path would be + // useless to the UI, which knows accounts only by their maildir, a + // database-relative prefix: comparing the two never matched and left every + // row resolving to no account at all. + const QString dbRoot = + QDir(QString::fromUtf8(notmuch_database_get_path(m_db))).absolutePath(); + QVector<ThreadSummary> batch; batch.reserve(kBatchSize); int total = 0; @@ -317,6 +357,10 @@ void NotmuchWorker::runQuery(const QString &query, quint64 generation, // The card's own tags, beside the thread's union above. // Same walk, same index read, no extra query. summary.firstMessageTags = tagsOf(message); + // Which account this belongs to, for Delete's destination. + summary.firstMessagePath = QDir(dbRoot).relativeFilePath( + QString::fromUtf8( + notmuch_message_get_filename(message))); break; } } @@ -329,6 +373,10 @@ void NotmuchWorker::runQuery(const QString &query, quint64 generation, // The card's own tags, beside the thread's union above. // Same walk, same index read, no extra query. summary.firstMessageTags = tagsOf(first); + // Which account this belongs to, for Delete's destination. + summary.firstMessagePath = QDir(dbRoot).relativeFilePath( + QString::fromUtf8( + notmuch_message_get_filename(first))); } } } @@ -614,6 +662,205 @@ void NotmuchWorker::applyTags(const TagChange &change) emit tagsApplied(change); } +void NotmuchWorker::moveMessages(const QStringList &messageIds, + const QString &destFolder) +{ + if (messageIds.isEmpty() || destFolder.isEmpty()) + return; + + // The read-only handle must be closed first: notmuch allows only one open + // handle per process. Same ordering as applyTags, for the same reason. + close(); + + const QByteArray configPath = configPathArg(); + notmuch_database_t *db = nullptr; + char *error = nullptr; + const notmuch_status_t status = notmuch_database_open_with_config( + nullptr, + NOTMUCH_DATABASE_MODE_READ_WRITE, + configPath.isEmpty() ? nullptr : configPath.constData(), + nullptr, + &db, + &error); + + if (status != NOTMUCH_STATUS_SUCCESS) { + emit errorOccurred( + QStringLiteral("Cannot open database for writing: %1") + .arg(QString::fromUtf8(error ? error + : notmuch_status_to_string(status)))); + free(error); + return; + } + + const QString root = QString::fromUtf8(notmuch_database_get_path(db)); + const QString destDir = + root + QLatin1Char('/') + destFolder + QStringLiteral("/cur"); + + QStringList moved; + QMap<QString, QString> origins; + for (const QString &id : messageIds) { + notmuch_message_t *raw = nullptr; + // find_message reports SUCCESS with a null message when the id is not + // in the database, so both have to be checked. A stale id must not + // abort the batch: the live ids alongside it still need moving. + if (notmuch_database_find_message(db, id.toUtf8().constData(), &raw) + != NOTMUCH_STATUS_SUCCESS || !raw) { + continue; + } + NmMessage message(raw); + + const char *rawName = notmuch_message_get_filename(message.get()); + if (!rawName) + continue; + const QString from = QString::fromUtf8(rawName); + // The handle is released before the file moves under it. + message.reset(); + + // Where it is coming FROM, captured here because this is the only + // moment the old filename exists. See messagesMovedFrom(). + const QString origin = folderOfMessageFile(root, from); + + // cur/, never new/. A file dropped in new/ is re-announced as fresh + // mail by every reader of the Maildir. + if (!QDir().mkpath(destDir)) { + emit errorOccurred(QStringLiteral("Cannot create folder %1") + .arg(destDir)); + continue; + } + + const QString to = destDir + QLatin1Char('/') + QFileInfo(from).fileName(); + if (from == to) { + // Already where it was asked to go. Reported as moved, since the + // caller's request is satisfied. + moved.append(id); + origins.insert(id, origin); + continue; + } + + if (!QFile::rename(from, to)) { + emit errorOccurred(QStringLiteral("Cannot move %1 to %2") + .arg(QFileInfo(from).fileName(), destFolder)); + continue; + } + + // Index the NEW path BEFORE dropping the old one. The reverse order + // removes the last filename for this message id, which deletes the + // database entry and every tag on it; the file then reindexes as a + // brand new message with default tags, silently. + notmuch_message_t *indexed = nullptr; + const notmuch_status_t added = notmuch_database_index_file( + db, to.toUtf8().constData(), nullptr, &indexed); + if (indexed) + notmuch_message_destroy(indexed); + + // DUPLICATE_MESSAGE_ID is success here: it means the id was already + // known, which is exactly the case for a file this just moved. + if (added != NOTMUCH_STATUS_SUCCESS + && added != NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID) { + QFile::rename(to, from); + emit errorOccurred(QStringLiteral("Cannot index %1 at its new path: %2") + .arg(id, QString::fromUtf8( + notmuch_status_to_string(added)))); + continue; + } + + notmuch_database_remove_message(db, from.toUtf8().constData()); + moved.append(id); + origins.insert(id, origin); + } + + notmuch_database_close(db); + notmuch_database_destroy(db); + + emit messagesMoved(moved, destFolder); + emit messagesMovedFrom(origins, destFolder); +} + +void NotmuchWorker::resolveMessages(const QStringList &messageIds, + const QString &requestTag) +{ + if (messageIds.isEmpty()) + return; + + QStringList terms; + terms.reserve(messageIds.size()); + for (const QString &id : messageIds) + terms.append(QStringLiteral("id:%1").arg(id)); + + resolveQuery(terms.join(QStringLiteral(" or ")), requestTag); +} + +void NotmuchWorker::resolveThreadMessages(const QStringList &threadIds, + const QString &requestTag) +{ + if (threadIds.isEmpty()) + return; + + // One combined query, for the reason applyTagsToThreads() gives: a query + // per thread reopens the same Xapian cursor once per selected row. + QStringList terms; + terms.reserve(threadIds.size()); + for (const QString &id : threadIds) + terms.append(QStringLiteral("thread:%1").arg(id)); + + resolveQuery(terms.join(QStringLiteral(" or ")), requestTag); +} + +void NotmuchWorker::resolveQuery(const QString &query, + const QString &requestTag) +{ + if (!openReadOnly()) + return; + + NmQuery nmQuery(notmuch_query_create(m_db, query.toUtf8().constData())); + if (!nmQuery) { + emit errorOccurred(QStringLiteral("Cannot resolve selected threads")); + return; + } + + notmuch_messages_t *raw = nullptr; + if (notmuch_query_search_messages(nmQuery.get(), &raw) + != NOTMUCH_STATUS_SUCCESS) { + emit errorOccurred(QStringLiteral("Cannot resolve selected threads")); + return; + } + + // Paths are reported RELATIVE to the database root, matching + // ThreadSummary::firstMessagePath: the UI knows accounts only by their + // maildir, itself a database-relative prefix. + const QString dbRoot = + QDir(QString::fromUtf8(notmuch_database_get_path(m_db))).absolutePath(); + + QStringList messageIds; + QStringList paths; + QStringList tags; + NmMessages messages(raw); + for (; notmuch_messages_valid(messages.get()); + notmuch_messages_move_to_next(messages.get())) { + NmMessage message(notmuch_messages_get(messages.get())); + if (!message) + continue; + const char *rawName = notmuch_message_get_filename(message.get()); + if (!rawName) + continue; + messageIds.append( + QString::fromUtf8(notmuch_message_get_message_id(message.get()))); + paths.append( + QDir(dbRoot).relativeFilePath(QString::fromUtf8(rawName))); + // Joined by a TAB, not a space. A notmuch tag may absolutely contain + // a space: a Maildir folder named "Inbox/SlackBuilds users" produces + // `deleted-from:Inbox/SlackBuilds users`, and splitting that on spaces + // truncated the folder to "Inbox/SlackBuilds". Restore then moved the + // messages into a folder of that name, CREATING it, so four real + // messages ended up in a directory mbsync does not sync and the user + // could not find them. A tab cannot appear in a tag, because notmuch's + // own dump/restore format is whitespace-delimited by line. + tags.append(tagsOf(message.get()).join(QLatin1Char('\t'))); + } + + emit threadMessagesResolved(messageIds, paths, tags, requestTag); +} + void NotmuchWorker::requestAllTags(quint64 generation) { if (!openReadOnly()) diff --git a/src/notmuchworker.h b/src/notmuchworker.h index f07e563..9932e59 100644 --- a/src/notmuchworker.h +++ b/src/notmuchworker.h @@ -18,6 +18,7 @@ #pragma once +#include <QMap> #include <QObject> #include <QStringList> #include <QVector> @@ -120,6 +121,19 @@ public slots: /// it would block the user's cron `notmuch new`. void applyTags(const TagChange &change); + /// Moves messages into `destFolder`, relative to the database path. + /// + /// A folder NAME rather than a "move to trash" call, because v2's Send + /// needs exactly this operation for Drafts and Sent. Nothing + /// trash-specific belongs here. + /// + /// The first mutation in this class that is not a notmuch tag: a rename on + /// disk plus a reindex. Ordering is rename, index the new path, drop the + /// old one. Indexing first is required, not stylistic: removing the last + /// filename for a message id deletes the database entry and every tag on + /// it, so removing before indexing loses the message's tags. + void moveMessages(const QStringList &messageIds, const QString &destFolder); + /// Batch tagging over whole threads. The UI holds thread ids, not message /// ids, for rows it has not opened, so the resolution happens here where /// the database handle lives. This is the path the archive/flag/delete @@ -129,6 +143,39 @@ public slots: const QStringList &remove, const QString &description); + /// Resolves whole threads to the message ids and file paths they contain. + /// + /// Delete thread MOVES every message, and a move needs message ids, which + /// the UI does not hold for a thread it never expanded. The resolution + /// happens here for the same reason applyTagsToThreads() does it here: + /// the database handle lives on this thread, and one combined query beats + /// reopening the cursor per thread. + /// + /// Paths come back beside the ids because the destination is per ACCOUNT + /// and the UI resolves an account from a message's path. Without them the + /// caller would know which messages to move and not where any of them + /// belongs. + void resolveThreadMessages(const QStringList &threadIds, + const QString &requestTag); + + /// The same walk for a set of MESSAGE ids rather than thread ids. + /// + /// Restore needs each message's tags and path to decide where to send it, + /// and must not read them from the model: the model's tags come from the + /// query, so a row whose delete has not been re-queried still carries its + /// pre-delete tags and the origin tag is missing. A restore that guesses + /// the destination is worse than one that does nothing. + void resolveMessages(const QStringList &messageIds, + const QString &requestTag); + +private: + /// The shared walk behind resolveMessages() and resolveThreadMessages(): + /// runs `query` and emits threadMessagesResolved() with each match's id, + /// database-relative path and tab-joined tags. + void resolveQuery(const QString &query, const QString &requestTag); + +public slots: + /// Every tag in the database, sorted. Feeds query bar completion, which /// cannot offer tag names it has no way to enumerate. Called at startup, /// after a sync, and after a tag mutation introduces an unknown tag. @@ -184,6 +231,42 @@ signals: quint64 generation); void messageLoaded(const QVector<MessageRef> &messages, quint64 generation); void tagsApplied(const TagChange &change); + + /// Carries the ids that ACTUALLY moved, which may be fewer than requested. + /// A stale id, a missing folder or a failed rename drops out here rather + /// than aborting the batch. + void messagesMoved(const QStringList &messageIds, const QString &destFolder); + + /// The same move, reported per message with the folder it came FROM. + /// + /// Emitted alongside messagesMoved rather than replacing it: that signal's + /// shape is what test_notmuchworker asserts on, and a caller wanting only + /// "did it move" should not have to unpack a map. + /// + /// The origin has to be reported from HERE because nowhere else knows it. + /// A Maildir filename does not record the folder a message came from, and + /// once the file has moved notmuch cannot answer either; the UI holds no + /// path at all for a thread row it has not expanded. This is the one + /// moment the old filename exists, so it is the only place the origin can + /// be derived. + /// + /// Folders are relative to the database path and carry no `cur`/`new` + /// segment, matching the `destFolder` moveMessages() takes, so a value + /// from here can be passed straight back to move a message home. + void messagesMovedFrom(const QMap<QString, QString> &originByMessageId, + const QString &destFolder); + /// The answer to resolveThreadMessages(), as parallel lists: `messageIds` + /// and the database-relative `paths` of the same messages, in the same + /// order. `requestTag` is echoed back so a caller can tell which request + /// this answers. + /// `tags` carries each message's tags joined by a space, in the same + /// order. Needed because Restore reads a message's `deleted-from:` tag to + /// decide where to send it, and an unexpanded thread's messages have no + /// node in the model to read tags from. + void threadMessagesResolved(const QStringList &messageIds, + const QStringList &paths, + const QStringList &tags, + const QString &requestTag); void allTagsReady(const QStringList &tags, quint64 generation); /// One entry per requested query, in the order they were asked for. A query diff --git a/src/tagdialog.cpp b/src/tagdialog.cpp index 1fb3f18..fb2c629 100644 --- a/src/tagdialog.cpp +++ b/src/tagdialog.cpp @@ -268,16 +268,25 @@ void TagDialog::accept() QStringList add = splitTags(m_addEdit->text()); QStringList remove = splitTags(m_removeEdit->text()); - // Validate before applying anything: a partial change is worse than none, - // since the user cannot tell which half landed. - for (const QStringList &list : { add, remove }) { - for (const QString &tag : list) { - const TagNameProblem problem = validateTagName(tag); - if (problem != TagNameProblem::Ok) { - QMessageBox::warning(this, tr("Invalid tag"), - tagNameProblemText(problem, tag)); - return; // Stay open, with the text still there to fix. - } + // Validate what is being ADDED. A partial change is worse than none, since + // the user cannot tell which half landed, so this runs before anything is + // applied. + // + // REMOVAL is deliberately not validated. The rules here exist to stop a + // troublesome tag being CREATED; a tag that already exists is a fact, and + // refusing to remove it because it breaks a rule leaves the user with a + // tag they can see, cannot type, and cannot get rid of. That happened with + // `deleted-from:Inbox/SlackBuilds users`: an origin tag naming a Maildir + // folder whose name contains a space, rejected by the space rule, so the + // one dialog that could have cleared it refused the only text that names + // it. Whether such a tag SHOULD exist is a separate question from whether + // the user may delete it, and the answer to the second is always yes. + for (const QString &tag : add) { + const TagNameProblem problem = validateTagName(tag); + if (problem != TagNameProblem::Ok) { + QMessageBox::warning(this, tr("Invalid tag"), + tagNameProblemText(problem, tag)); + return; // Stay open, with the text still there to fix. } } diff --git a/src/threadlistmodel.cpp b/src/threadlistmodel.cpp index 6ddd85c..6162a5f 100644 --- a/src/threadlistmodel.cpp +++ b/src/threadlistmodel.cpp @@ -696,6 +696,10 @@ ThreadListModel::nodeFor(const ThreadSummary &summary) node.first.messageId = summary.firstMessageId; node.first.threadId = summary.threadId; node.first.tags = summary.firstMessageTags; + // Carried alongside the tags, for the same reason messageById() + // carries it onto a synthesised root: an unexpanded row has to know + // which account it belongs to before Delete can name a folder. + node.first.filePath = summary.firstMessagePath; } return node; } @@ -984,6 +988,12 @@ MessageNode ThreadListModel::messageById(const QString &messageId) const root.subject = node.summary.subject; root.date = node.summary.date; root.tags = node.summary.tags; + // Carried from the query, so an UNEXPANDED row still knows which + // account it belongs to. Delete needs that to name a trash folder, + // and an unexpanded row is the ordinary case rather than an edge + // one: without this every thread row resolved to no account and + // Delete reported "no trash folder configured" for all of them. + root.filePath = node.summary.firstMessagePath; return root; } @@ -1198,6 +1208,7 @@ void ThreadListModel::applyMessageTagChange(const QString &messageId, node.first.messageId = node.summary.firstMessageId; node.first.threadId = node.summary.threadId; node.first.tags = node.summary.tags; + node.first.filePath = node.summary.firstMessagePath; } retag(node.first.tags); diff --git a/src/types.h b/src/types.h index 409ce79..f4d387a 100644 --- a/src/types.h +++ b/src/types.h @@ -62,6 +62,26 @@ struct ThreadSummary /// file. Do not move it behind a flag by analogy with `recipients`. QStringList firstMessageTags; + /// That message's file, RELATIVE to the database path, which is what says + /// which ACCOUNT it belongs to. + /// + /// Relative and not absolute, deliberately. The UI knows an account only + /// by its `maildir`, itself a database-relative prefix, so an absolute + /// path here matches no account and silently resolves every row to none. + /// + /// Needed because Delete moves the file (item 103) and the destination is + /// per account, so the action has to resolve an account before it can name + /// a trash folder. Resolving through the thread's account TAG instead is + /// not equivalent: that tag is optional config, so an account without one + /// would silently be undeletable, while a maildir prefix is what makes a + /// message belong to an account in the first place. + /// + /// Free for the same reason firstMessageId and firstMessageTags are: the + /// walk that finds that message is already happening, and this reads the + /// INDEX rather than the message file. Do not move it behind a flag by + /// analogy with `recipients`. + QString firstMessagePath; + /// Who the thread's messages were sent TO, summarised for one line. /// /// Empty unless the query asked for it, and that is a performance diff --git a/tests/test_config.cpp b/tests/test_config.cpp index 0dfda86..ea5c363 100644 --- a/tests/test_config.cpp +++ b/tests/test_config.cpp @@ -116,6 +116,11 @@ private slots: void draftsQuerySurvivesABracketedPath(); void allDraftsQuerySkipsAccountsWithoutTheKey(); void allDraftsQueryIsIndependentOfSent(); + void anAccountCarriesItsTrashFolder(); + void aBracketedTrashFolderIsQuoted(); + void anAccountWithoutATrashFolderWarns(); + void theTrashFilterComposesPerAccount(); + void theTrashFilterMatchesNothingWithoutAFolder(); }; static QString writeIni(const QTemporaryDir &dir, const QString &body) @@ -428,7 +433,8 @@ void TestConfig::absentSyncCommandIsNoticeNotProblem() QTemporaryDir dir; const QString path = writeIni(dir, QStringLiteral( "[account.work]\n" - "maildir=work-mail\n")); + "maildir=work-mail\n" + "trash=Trash\n")); Config config; config.load(path); @@ -448,7 +454,8 @@ void TestConfig::brokenSyncCommandIsAProblem() "command=/nonexistent/qtmaildir-test/mailsync.sh\n" "\n" "[account.work]\n" - "maildir=work-mail\n")); + "maildir=work-mail\n" + "trash=Trash\n")); Config config; config.load(path); @@ -483,7 +490,8 @@ void TestConfig::validConfigHasNoProblems() "\n" "[account.work]\n" "maildir=work-mail\n" - "address=user@example.org\n")); + "address=user@example.org\n" + "trash=Trash\n")); Config config; config.load(path); @@ -903,7 +911,8 @@ void TestConfig::sentQueryIsEmptyWithoutTheKey() Config config; config.load(writeIni(dir, QStringLiteral( "[account.provider-c]\n" - "maildir = provider-c\n"))); + "maildir = provider-c\n" + "trash = Trash\n"))); QCOMPARE(config.accounts().size(), 1); QVERIFY(config.accounts().at(0).sentQuery().isEmpty()); @@ -953,6 +962,103 @@ void TestConfig::sentQuerySurvivesABracketedPath() "brackets as syntax and the query will match nothing"); } +void TestConfig::anAccountCarriesItsTrashFolder() +{ + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral( + "[account.work]\n" + "maildir=work\n" + "trash=Trash\n"))); + + const Account account = config.account(QStringLiteral("work")); + QCOMPARE(account.trash, QStringLiteral("Trash")); + // Quoted and globbed exactly as sentQuery() does it, so a folder with a + // space or a bracket cannot break the query. + QCOMPARE(account.trashQuery(), QStringLiteral("path:\"work/Trash/**\"")); +} + +void TestConfig::aBracketedTrashFolderIsQuoted() +{ + // The real setup nests a localised trash folder under a bracketed parent. + // The brackets are not notmuch syntax, but the quoting has to survive them. + Account account; + account.maildir = QStringLiteral("provider-a"); + account.trash = QStringLiteral("[Provider]/Cestino"); + + QCOMPARE(account.trashQuery(), + QStringLiteral("path:\"provider-a/[Provider]/Cestino/**\"")); +} + +void TestConfig::anAccountWithoutATrashFolderWarns() +{ + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral( + "[account.work]\n" + "maildir=work\n"))); + + // The account still loads. A missing trash folder disables Delete, it does + // not invalidate the account: the user can still read mail. + QVERIFY(config.account(QStringLiteral("work")).isValid()); + + // Names the account and the key, so the warning is actionable. A warning + // the user cannot act on teaches them to ignore warnings, which item 83 + // recorded the hard way. + QVERIFY(!config.problems().isEmpty()); + const QString joined = config.warnings().join(QLatin1Char('\n')); + QVERIFY(joined.contains(QStringLiteral("work"))); + QVERIFY(joined.contains(QStringLiteral("trash"))); +} + +void TestConfig::theTrashFilterComposesPerAccount() +{ + // Two accounts, one with a plain folder and one nested under a bracketed + // parent, since the real setup has both shapes. + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral( + "[account.work]\n" + "maildir=work\n" + "trash=Trash\n" + "\n" + "[account.personal]\n" + "maildir=personal\n" + "trash=[Provider]/Cestino\n"))); + + const SavedQuery trash = Config::builtinFilter(QStringLiteral("trash")); + QVERIFY(trash.isGenerated()); + + // All accounts: the union, never a bare path that would match one account. + const QString all = config.resolvedQuery(trash, QString()); + QVERIFY(all.contains(QStringLiteral("path:\"work/Trash/**\""))); + QVERIFY(all.contains( + QStringLiteral("path:\"personal/[Provider]/Cestino/**\""))); + + // One account: that account's OWN query. Asserting on the STRING, not on a + // row count: the all-accounts query wrapped in this account's path returns + // exactly the right rows, because path: is hierarchical, so a count passes + // against the wrong thing. Config::resolvedQuery documents this trap. + const QString scoped = config.resolvedQuery(trash, QStringLiteral("work")); + QCOMPARE(scoped, QStringLiteral("path:\"work/Trash/**\"")); + QVERIFY(!scoped.contains(QStringLiteral("personal"))); +} + +void TestConfig::theTrashFilterMatchesNothingWithoutAFolder() +{ + // An empty query means "match everything" to notmuch, so a filter with + // nothing to match must say so explicitly. A button labelled Trash that + // showed the whole Maildir is the failure this prevents. + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral( + "[account.work]\n" + "maildir=work\n"))); + + const SavedQuery trash = Config::builtinFilter(QStringLiteral("trash")); + QCOMPARE(config.resolvedQuery(trash, QString()), Config::matchNothingQuery()); +} + void TestConfig::sentQueryComposesWithScopedQuery() { // A Sent view under one account must not show another account's sent mail. @@ -1073,9 +1179,11 @@ void TestConfig::theStartupAccountIsReadAndValidated() "\n" "[account.work]\n" "maildir=work\n" + "trash=Trash\n" "\n" "[account.personal]\n" - "maildir=personal\n"))); + "maildir=personal\n" + "trash=Trash\n"))); QCOMPARE(config.startupAccount(), QStringLiteral("work")); QVERIFY(config.problems().isEmpty()); @@ -1100,7 +1208,8 @@ void TestConfig::theStartupAccountIsReadAndValidated() "startup_account=nosuchaccount\n" "\n" "[account.work]\n" - "maildir=work\n"))); + "maildir=work\n" + "trash=Trash\n"))); QVERIFY2(wrong.startupAccount().isEmpty(), "an unknown startup account was passed through rather than " "falling back to All accounts"); @@ -1125,7 +1234,8 @@ void TestConfig::theStartupAccountTakesTheKeyNotTheSyncChannel() "\n" "[account.provider-work.mailbox]\n" "maildir=provider-work.mailbox\n" - "channel=provider-workmailbox\n"))); + "channel=provider-workmailbox\n" + "trash=Trash\n"))); QCOMPARE(config.accounts().size(), 1); QCOMPARE(config.accounts().constFirst().key, @@ -1147,7 +1257,8 @@ void TestConfig::theStartupAccountTakesTheKeyNotTheSyncChannel() "\n" "[account.provider-work.mailbox]\n" "maildir=provider-work.mailbox\n" - "channel=provider-workmailbox\n"))); + "channel=provider-workmailbox\n" + "trash=Trash\n"))); QVERIFY2(byChannel.startupAccount().isEmpty(), "the sync channel was accepted as an account key"); @@ -1231,7 +1342,8 @@ void TestConfig::theStartupQuerySurvivesATranslatedFilterName() "\n" "[account.work]\n" "maildir=work\n" - "sent=Sent\n"))); + "sent=Sent\n" + "trash=Trash\n"))); QVERIFY2(!config.savedQueries().isEmpty(), "queries.json did not load, so the warning path is unreachable"); @@ -1256,7 +1368,8 @@ void TestConfig::theStartupQuerySurvivesATranslatedFilterName() "\n" "[account.work]\n" "maildir=work\n" - "sent=Sent\n"))); + "sent=Sent\n" + "trash=Trash\n"))); QVERIFY(!byLabel.savedQueries().isEmpty()); QCOMPARE(byLabel.startupSavedQuery().generated, QStringLiteral("inbox")); QVERIFY(byLabel.problems().isEmpty()); @@ -1369,7 +1482,7 @@ void TestConfig::everyBuiltinFilterIsAKnownGenerator() Config config; const QList<SavedQuery> filters = config.builtinFilters(); - QCOMPARE(filters.size(), 4); + QCOMPARE(filters.size(), 5); QStringList names; for (const SavedQuery &filter : filters) { @@ -1390,7 +1503,8 @@ void TestConfig::everyBuiltinFilterIsAKnownGenerator() QCOMPARE(names, (QStringList{ QStringLiteral("Unread"), QStringLiteral("Inbox"), QStringLiteral("Important"), - QStringLiteral("Sent") })); + QStringLiteral("Sent"), + QStringLiteral("Trash") })); } void TestConfig::aFilterAcrossAllAccountsIsTheUnscopedQuery() @@ -1483,7 +1597,8 @@ void TestConfig::draftsQueryIsEmptyWithoutTheKey() Config config; config.load(writeIni(dir, QStringLiteral( "[account.provider-c]\n" - "maildir = provider-c\n"))); + "maildir = provider-c\n" + "trash = Trash\n"))); QCOMPARE(config.accounts().size(), 1); QVERIFY(config.accounts().at(0).draftsQuery().isEmpty()); diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index b188ef4..79e137a 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -16,6 +16,7 @@ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ +#include <QProcess> #include <QtTest> #include <QAction> @@ -26,6 +27,7 @@ #include <QKeyEvent> #include <QLabel> #include <QLineEdit> +#include <QMenuBar> #include <QMenu> #include <QPushButton> #include <QProgressBar> @@ -86,8 +88,17 @@ public: /// `accountKey` and `accountMaildir` add one [account.<key>] section, which /// is what makes runQuery() scope the bar's text with scopedQuery(). A test /// that never selects an account can leave them empty. + /// + /// `accountTrash` writes that section's `trash` key, which Delete needs to + /// know where to move a file to. A DEFAULTED parameter rather than an + /// overload: an overload would have to repeat the whole body, and every + /// existing caller passes no account at all and so writes no section and + /// no trash key either. A caller that names an account and wants Delete to + /// work has to say where its trash is, which is the same requirement the + /// real config imposes. bool build(const QString &accountKey = QString(), - const QString &accountMaildir = QString()) + const QString &accountMaildir = QString(), + const QString &accountTrash = QString()) { if (!m_fixture.isValid()) { m_error = QStringLiteral("fixture directory invalid"); @@ -123,6 +134,13 @@ public: // so the section is [account.key], never [account/key]. out << "\n[account." << accountKey << "]\n" << "maildir=" << accountMaildir << "\n"; + if (!accountTrash.isEmpty()) + out << "trash=" << accountTrash << "\n"; + // The fixture's folders are lowercase, unlike the Maildir + // convention Account::inboxFolder() defaults to. Stated rather + // than assumed, which is the whole point of the key: naming a + // folder that does not exist would CREATE it. + out << "inbox=inbox\n"; } } file.close(); @@ -313,6 +331,7 @@ private slots: void aCronSyncDoesNotClearAnEditMadeWhileItRan(); void everyActionCarriesAnIcon(); + void everyActionIsReachableFromAMenu(); void theToolbarDoesNotOverrideTheDesktopButtonStyle(); void theImportantActionIsLabelledImportant(); void theImportantActionStillWritesTheFlaggedTag(); @@ -347,6 +366,32 @@ private slots: void anEditedQueryKeepsItsUnknownFields(); void renamingReplacesRatherThanDuplicating(); + void deleteMovesTheMessageToTrash(); + void deleteRecordsWhereTheMessageCameFrom(); + void undoMovesTheMessageBack(); + void deleteOnAReplyMovesThatReplyOnly(); + void deleteWithoutATrashFolderSaysSoRatherThanDoingNothing(); + void undoingADeleteConsumesItsCommandRatherThanPushingAnother(); + void aDeleteHeldDuringASyncCountsAsUnsyncedWork(); + void twoDeletesToOneTrashBothGetTheirTags(); + void deletingTwiceLeavesNoOriginTagBehind(); + void undoOfADeleteRemovesTheOriginTagToo(); + void deletingAThreadRootTwiceRestoresItRatherThanRedeleting(); + void deleteThreadMovesEveryMessageAndRepaintsTheRootCard(); + void aFolderNameWithASpaceSurvivesTheRoundTrip(); + void deleteIsBoundToTheDeleteKey(); + void theDeleteKeyEditsTextInTheQueryBar(); + void restoreIsReachableWithoutTheKeyboard(); + void restoreIsOnlyEnabledInTheTrashView(); + void restoreReturnsAMessageToItsOriginFolder(); + void restoreFallsBackToInboxWithoutAnOriginTag(); + void theCleanupQueryFindsStrandedMail(); + void theCleanupQueryExcludesMailAlreadyInTrash(); + void aMoveThatRelocatesNothingWritesNoTag(); + void restoringFromTheTrashViewRefreshesTheList(); + void theRefreshAfterARestoreLeavesUndoIntact(); + void deletingOutsideTheTrashViewLeavesTheRowInPlace(); + private: /// Owns the throwaway lock table init() points every test at. A pointer /// rather than a value because it is rebuilt per test, and QTemporaryDir @@ -4737,6 +4782,16 @@ static QModelIndex expandSecondThreadAndSelectItsReply( void TestMainWindow::deleteOnAReplyReadsItsOwnThreadNotTheFirstInTheList() { + // Item 88's trap, still live: a toggle must read the state of the row it + // is on, not of whichever thread sits at that row NUMBER in the list. + // + // Through `delete_thread` rather than `delete`. Since item 103 Delete + // MOVES the file, so it is no longer a pure toggle over a tag and needs a + // configured trash folder and a worker; `delete_thread` is the variant + // that stayed tag-only, and it is a toggle over `deleted` exactly as + // Delete used to be. The message-scoped Delete's own direction choice is + // covered by the worker-backed cases at the bottom of this file, which is + // where a move can actually be observed. const Config config; MainWindow window(config); @@ -4744,7 +4799,7 @@ void TestMainWindow::deleteOnAReplyReadsItsOwnThreadNotTheFirstInTheList() QVERIFY(model); auto *view = window.findChild<QTreeView *>(); QVERIFY(view); - auto *action = window.findChild<QAction *>(QStringLiteral("delete")); + auto *action = window.findChild<QAction *>(QStringLiteral("delete_thread")); QVERIFY(action); // t1 deleted, t2 not. Reading t1's state for a reply of t2 makes the @@ -4755,21 +4810,27 @@ void TestMainWindow::deleteOnAReplyReadsItsOwnThreadNotTheFirstInTheList() "the fixture did not produce a reply row at row 0, so this test " "would assert nothing about item 88's trap"); + // t2 is the reply's thread and is NOT deleted, so the correct direction + // is Delete. Reading t1's state instead would choose Undelete. + QVERIFY2(!model->threadAt(1).isDeleted(), + "the fixture's second thread is already deleted, so both " + "directions would look alike and this test would assert nothing"); + action->trigger(); - // Delete, because the message's own thread is not deleted. The write goes - // through scopeFor() and lands on the message either way; what is under - // test is the DIRECTION, which is chosen from the state that was read. - QVERIFY2(window.pendingMessageIdsForTesting().contains( - QStringLiteral("m1@example.org")), - "Delete on a reply did not act on that reply"); - QCOMPARE(window.undoDepthForTesting(), 1); - QVERIFY2(window.undoTextForTesting().contains(QStringLiteral("Delete")), - qPrintable(QStringLiteral( - "Delete on a reply of an undeleted thread chose " - "the wrong direction: %1. It read the FIRST " - "thread's state, which is deleted.") - .arg(window.undoTextForTesting()))); + // Asserted on the MODEL, not on the undo stack. Delete thread MOVES since + // item 103's follow-up, and the undo entry is pushed once the worker + // confirms the move, which this bare window has no database to perform. + // The DIRECTION is chosen synchronously and is what item 88's trap was + // about: the repaint below happens only on the delete direction. + QVERIFY2(model->threadAt(1).isDeleted(), + "Delete on a reply of an undeleted thread chose the wrong " + "direction: it read the FIRST thread's state, which is deleted"); + // And the OTHER thread is untouched: the action must act on the reply's + // own conversation, not on both. + QVERIFY2(model->threadAt(0).isDeleted(), + "the fixture's first thread stopped being deleted, which means " + "the action reached a thread it was never pointed at"); } void TestMainWindow::toggleUnreadOnAReplyReadsItsOwnThreadNotTheFirstInTheList() @@ -4985,6 +5046,11 @@ void TestMainWindow::markCurrentThreadReadResolvesTheThreadThroughTheIndex() void TestMainWindow::deletingAReplyRepaintsThatReplyRow() { + // `spam`, not `delete`. Since item 103 Delete MOVES the file, so it needs + // an account with a configured trash folder and a worker to do the move; + // this bare window has neither, and Delete correctly refuses. What is + // under test here is unchanged by that: `spam` is the other message-scoped + // tag-only action, and it paints the same doomed state. // The user's report, at the gesture level: "I'm hitting delete on a reply // to a thread, I see the edits counter increasing but I have no feedback // if that message is being deleted." The model-level test proves @@ -4997,7 +5063,7 @@ void TestMainWindow::deletingAReplyRepaintsThatReplyRow() QVERIFY(model); auto *view = window.findChild<QTreeView *>(); QVERIFY(view); - auto *action = window.findChild<QAction *>(QStringLiteral("delete")); + auto *action = window.findChild<QAction *>(QStringLiteral("spam")); QVERIFY(action); const QModelIndex reply = @@ -5006,13 +5072,13 @@ void TestMainWindow::deletingAReplyRepaintsThatReplyRow() // Nothing to see before the gesture, so the assertion after it means // something. - QVERIFY(!model->messageAt(reply).isDeleted()); + QVERIFY(!model->messageAt(reply).isSpam()); const QVariant before = model->data(reply, Qt::BackgroundRole); QSignalSpy spy(model, &QAbstractItemModel::dataChanged); action->trigger(); - QVERIFY2(model->messageAt(reply).isDeleted(), + QVERIFY2(model->messageAt(reply).isSpam(), "Delete on a reply left the reply's own row unchanged, so the " "pending count moved and the user saw nothing"); QVERIFY2(spy.count() >= 1, "no repaint was requested for the reply's row"); @@ -5022,7 +5088,7 @@ void TestMainWindow::deletingAReplyRepaintsThatReplyRow() // The THREAD row must not follow: it stands for the whole conversation, // and one deleted reply does not doom it. const QModelIndex threadRow = reply.parent(); - QVERIFY2(!model->threadFor(threadRow).isDeleted(), + QVERIFY2(!model->threadFor(threadRow).isSpam(), "deleting one reply marked its whole thread deleted"); } @@ -5113,6 +5179,11 @@ void TestMainWindow::toggleUnreadOnAReplyRepaintsItInBothDirections() void TestMainWindow::taggingTheOpenReplyUpdatesTheMessagePaneStrip() { + // `spam`, not `delete`. Since item 103 Delete MOVES the file, so it needs + // an account with a configured trash folder and a worker to do the move; + // this bare window has neither, and Delete correctly refuses. What is + // under test here is unchanged by that: `spam` is the other message-scoped + // tag-only action, and it paints the same doomed state. // The user's report: "the right pane chips are not [repainted], for it to // sync I have to change message and go back to the edited one". // @@ -5136,7 +5207,7 @@ void TestMainWindow::taggingTheOpenReplyUpdatesTheMessagePaneStrip() const auto stripTags = [strip]() { return strip->visibleTags() + strip->hiddenTags(); }; - auto *action = window.findChild<QAction *>(QStringLiteral("delete")); + auto *action = window.findChild<QAction *>(QStringLiteral("spam")); QVERIFY(action); // A tag the strip will actually draw. Account tags are filtered out by the @@ -5151,11 +5222,11 @@ void TestMainWindow::taggingTheOpenReplyUpdatesTheMessagePaneStrip() QVERIFY2(stripTags().contains(QStringLiteral("todo")), "the strip does not show the selected reply's tags, so this test " "cannot tell a missing refresh from a strip that never had them"); - QVERIFY(!stripTags().contains(QStringLiteral("deleted"))); + QVERIFY(!stripTags().contains(QStringLiteral("spam"))); action->trigger(); - QVERIFY2(stripTags().contains(QStringLiteral("deleted")), + QVERIFY2(stripTags().contains(QStringLiteral("spam")), "the message pane's chips still describe the reply as it was " "before the edit; the user has to select away and back to see it"); } @@ -5231,6 +5302,11 @@ void TestMainWindow::taggingAnUnrelatedReplyLeavesTheStripAlone() void TestMainWindow::aHeldMessageEditIsSentWhenTheSyncEnds() { + // `spam`, not `delete`. Since item 103 Delete MOVES the file, so it needs + // an account with a configured trash folder and a worker to do the move; + // this bare window has neither, and Delete correctly refuses. What is + // under test here is unchanged by that: `spam` is the other message-scoped + // tag-only action, and it paints the same doomed state. // Found by reading while fixing the strip refresh, not reported. // // flushHeldEdits() looped over edit.threadIds and called @@ -5246,7 +5322,7 @@ void TestMainWindow::aHeldMessageEditIsSentWhenTheSyncEnds() QVERIFY(model); auto *view = window.findChild<QTreeView *>(); QVERIFY(view); - auto *action = window.findChild<QAction *>(QStringLiteral("delete")); + auto *action = window.findChild<QAction *>(QStringLiteral("spam")); QVERIFY(action); const QModelIndex reply = @@ -5281,12 +5357,17 @@ void TestMainWindow::aHeldMessageEditIsSentWhenTheSyncEnds() // And the row still shows it: the flush takes the optimistic update back // before re-sending, so a bug there leaves the row wrong in the other // direction. - QVERIFY2(model->messageAt(reply).isDeleted(), + QVERIFY2(model->messageAt(reply).isSpam(), "sending the held edit lost the tag from the reply's row"); } void TestMainWindow::anActionOnAThreadRowActsOnTheMessageItDisplays() { + // `spam`, not `delete`. Since item 103 Delete MOVES the file, so it needs + // an account with a configured trash folder and a worker to do the move; + // this bare window has neither, and Delete correctly refuses. What is + // under test here is unchanged by that: `spam` is the other message-scoped + // tag-only action, and it paints the same doomed state. // Item 108, the whole point of it. A root card renders ONE message since // item 66, so acting on it acts on that message; the conversation is // reached through the explicit thread actions. @@ -5303,7 +5384,7 @@ void TestMainWindow::anActionOnAThreadRowActsOnTheMessageItDisplays() model->appendBatch({ t }); selectThreadRow(view, 0); - auto *deleteAction = window.findChild<QAction *>(QStringLiteral("delete")); + auto *deleteAction = window.findChild<QAction *>(QStringLiteral("spam")); QVERIFY(deleteAction); deleteAction->trigger(); @@ -5315,18 +5396,24 @@ void TestMainWindow::anActionOnAThreadRowActsOnTheMessageItDisplays() // The thread action is how the conversation is reached, and it must still // work from the same selection. + // + // Asserted on the MODEL rather than on a pending write. Delete thread + // MOVES every message since item 103's follow-up, and a move needs ids and + // paths that only the database holds for a thread this bare window never + // expanded, so the write is issued after a worker round trip that never + // completes here. What is synchronous, and what this test is about, is the + // scope: the whole thread is marked, not the one message its card shows. auto *deleteThread = window.findChild<QAction *>(QStringLiteral("delete_thread")); QVERIFY(deleteThread); + QVERIFY2(!model->threadAt(0).isDeleted(), + "the thread already read as deleted, so the check below would " + "pass without the action doing anything"); deleteThread->trigger(); - QCOMPARE(window.pendingThreadIdsForTesting(), - QStringList{ QStringLiteral("t1") }); - - // Two commands, one per gesture, each recording the scope it used: a thread - // action that pushed the message command would undo a fraction of what it - // did. - QCOMPARE(window.undoDepthForTesting(), 2); + QVERIFY2(model->threadAt(0).isDeleted(), + "Delete thread did not mark the whole thread, so the card paints " + "undeleted until the row is clicked"); } void TestMainWindow::theThreadSubmenuIsReachableFromBothMenus() @@ -5501,6 +5588,11 @@ void TestMainWindow::autoMarkReadArmsForAReplyToo() void TestMainWindow::taggingTheOpenRootMessageKeepsTheStripPopulated() { + // `spam`, not `delete`. Since item 103 Delete MOVES the file, so it needs + // an account with a configured trash folder and a worker to do the move; + // this bare window has neither, and Delete correctly refuses. What is + // under test here is unchanged by that: `spam` is the other message-scoped + // tag-only action, and it paints the same doomed state. // The user, 2026-08-16: "right pane loses the chip row when repainting, it // simply disappears". // @@ -5543,7 +5635,7 @@ void TestMainWindow::taggingTheOpenRootMessageKeepsTheStripPopulated() QVERIFY2(stripTags().contains(QStringLiteral("todo")), "the strip never showed the selected thread's tags"); - auto *action = window.findChild<QAction *>(QStringLiteral("delete")); + auto *action = window.findChild<QAction *>(QStringLiteral("spam")); QVERIFY(action); action->trigger(); @@ -5553,7 +5645,7 @@ void TestMainWindow::taggingTheOpenRootMessageKeepsTheStripPopulated() "and set the strip to the resulting empty tag list"); QVERIFY2(stripTags().contains(QStringLiteral("todo")), "the strip lost the tag the message still carries"); - QVERIFY2(stripTags().contains(QStringLiteral("deleted")), + QVERIFY2(stripTags().contains(QStringLiteral("spam")), "the strip did not pick up the tag just written"); } @@ -6300,6 +6392,75 @@ void TestMainWindow::aCronSyncDoesNotClearAnEditMadeWhileItRan() // Items 56 and 57. +void TestMainWindow::everyActionIsReachableFromAMenu() +{ + // The fourth registration site nothing enforced. CLAUDE.md says adding an + // action is four places: knownActions(), defaultBindings(), the icon table + // and the action itself. It is FIVE, and the fifth is a menu. + // + // Found the hard way on the trash branch: `restore` shipped keyboard-only, + // reachable by a chord and by nothing a user could see or discover, and no + // test noticed. The three existing coverage tests each assert a different + // property and all three pass against an action that appears nowhere in + // the interface. + // + // The MENU rather than the toolbar, since the toolbar is a small + // deliberate subset and always will be. Every menu is walked, submenus + // included, because the five whole-thread actions live only in the "Whole + // thread" submenu. + const Config config; + MainWindow window(config); + + auto *bar = window.menuBar(); + QVERIFY(bar); + + QSet<QAction *> reachable; + QList<QMenu *> pending; + const auto topLevel = bar->actions(); + for (QAction *action : topLevel) { + if (action->menu()) + pending.append(action->menu()); + } + QVERIFY2(!pending.isEmpty(), "the menu bar holds no menus"); + + while (!pending.isEmpty()) { + QMenu *menu = pending.takeFirst(); + const auto entries = menu->actions(); + for (QAction *entry : entries) { + if (QMenu *sub = entry->menu()) { + pending.append(sub); + // An action owning a menu emits no `triggered`, so it is the + // submenu that makes its children reachable and never the + // parent entry itself. Not counted as reachable. + continue; + } + reachable.insert(entry); + } + } + + // The guard, before anything is asserted about what is missing: a walk + // that found nothing would report every action as unreachable and read as + // a catastrophic regression rather than as a broken probe. + QVERIFY2(reachable.size() > 10, + qPrintable(QStringLiteral("the menu walk found only %1 entries") + .arg(reachable.size()))); + + QStringList unreachable; + for (const QString &name : KeyMap::knownActions()) { + auto *action = window.findChild<QAction *>(name); + QVERIFY2(action, qPrintable(QStringLiteral("no action named %1").arg(name))); + if (!reachable.contains(action)) + unreachable.append(name); + } + + QVERIFY2(unreachable.isEmpty(), + qPrintable(QStringLiteral("%1 action(s) reach no menu, so they " + "exist only for whoever already knows " + "the chord: %2") + .arg(unreachable.size()) + .arg(unreachable.join(QStringLiteral(", "))))); +} + void TestMainWindow::everyActionCarriesAnIcon() { // Item 56. The complaint was inconsistency, not absence: eight actions had @@ -7922,10 +8083,20 @@ void TestMainWindow::everyBuiltinFilterButtonCarriesAnIconAndItsText() // which is the same argument the Save button records. QTemporaryDir dir; QVERIFY(dir.isValid()); - Config config; - config.load(writeSentConfig(dir, { + const QString path = writeSentConfig(dir, { {QStringLiteral("work"), QStringLiteral("Sent")}, - })); + }); + // A trash key too, or the Trash filter finds nothing and is skipped from + // the row entirely (item 103), leaving no trashButton for this loop to + // find. + { + QSettings s(path, QSettings::IniFormat); + s.beginGroup(QStringLiteral("account.work")); + s.setValue(QStringLiteral("trash"), QStringLiteral("Trash")); + s.endGroup(); + } + Config config; + config.load(path); MainWindow window(config); @@ -8688,4 +8859,1709 @@ void TestMainWindow::aSingleMessageIdQuerysCardOpensInTheMessagePane() QTRY_VERIFY_WITH_TIMEOUT(!pane->showingPlaceholder(), 15000); } +/// Whether any file in `dir` belongs to the message whose filename starts with +/// `stem`. +/// +/// A Maildir filename is NOT stable across a move, which is the trap this +/// exists to avoid. `maildir.synchronize_flags` is on, so notmuch rewrites the +/// name to carry the read/seen flags: a message that leaves `new/del1.x` lands +/// as `cur/del1.x:2,S`. Asserting on the exact basename therefore fails +/// against a move that worked perfectly, which is how three of these tests +/// first "failed". +/// Counts messages matching `query` in the fixture's database, by running +/// notmuch itself. +/// +/// Asked directly rather than through the query bar because the UI's +/// rowCount() reads 0 for the whole interval before the worker answers, so an +/// assertion that a tag is ABSENT is satisfied by the gap before any answer +/// arrives and passes against a database that still carries the tag. +static int notmuchCount(const QString &configPath, const QString &query) +{ + QProcess process; + QProcessEnvironment env = QProcessEnvironment::systemEnvironment(); + env.insert(QStringLiteral("NOTMUCH_CONFIG"), configPath); + process.setProcessEnvironment(env); + process.start(QStringLiteral("notmuch"), + { QStringLiteral("count"), query }); + if (!process.waitForFinished(15000)) + return -1; + bool ok = false; + const int count = + QString::fromUtf8(process.readAllStandardOutput()).trimmed().toInt(&ok); + return ok ? count : -1; +} + +/// Applies a tag change with the notmuch binary, for the one thing the UI +/// cannot produce any more: a message tagged `deleted` while its file is still +/// in the inbox. That is the state the OLD Delete left mail in, and the state +/// the cleanup action exists to find, so a test for it has to write it +/// directly rather than through an action that now moves the file too. +static bool notmuchTag(const QString &configPath, const QStringList &args) +{ + QProcess process; + QProcessEnvironment env = QProcessEnvironment::systemEnvironment(); + env.insert(QStringLiteral("NOTMUCH_CONFIG"), configPath); + process.setProcessEnvironment(env); + process.start(QStringLiteral("notmuch"), + QStringList{ QStringLiteral("tag") } + args); + return process.waitForFinished(15000) && process.exitCode() == 0; +} + +static bool folderHasMessageFile(const QString &dir, const QString &stem) +{ + QDir directory(dir); + if (!directory.exists()) + return false; + const QStringList entries = directory.entryList(QDir::Files); + for (const QString &entry : entries) { + if (entry == stem || entry.startsWith(stem + QLatin1Char(':'))) + return true; + } + return false; +} + +void TestMainWindow::deleteMovesTheMessageToTrash() +{ + // The whole point of item 103. Before it, Delete added a tag and moved no + // file, so deleted mail sat in the inbox for good. + WorkerBackedWindow backed; + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/inbox"), QStringLiteral("del1@example.org"), + QStringLiteral("Delete me"), QStringLiteral("sender@example.org"), + // Friday, verified with `date -d 2026-08-14 +%A`. + QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"), + QStringLiteral("Body text."))); + QVERIFY2(backed.build(QStringLiteral("acct"), QStringLiteral("acct"), + QStringLiteral("Trash")), + qPrintable(backed.error())); + + MainWindow window(backed.config()); + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<ThreadListView *>(); + QVERIFY(view); + auto *queryEdit = + window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + QVERIFY(queryEdit); + + queryEdit->setText(QStringLiteral("tag:inbox")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); + + const QString root = backed.fixture().maildirPath(); + const QString inbox = root + QStringLiteral("/acct/inbox/new"); + const QString stem = QStringLiteral("del1.example.org"); + QVERIFY(folderHasMessageFile(inbox, stem)); + + view->setCurrentIndex(model->index(0, 0, QModelIndex())); + + auto *del = window.findChild<QAction *>(QStringLiteral("delete")); + QVERIFY(del); + del->trigger(); + + // The filesystem half. cur/, never new/: a file in new/ is re-announced as + // fresh mail by every reader of the Maildir. + const QString trash = root + QStringLiteral("/acct/Trash/cur"); + QTRY_VERIFY_WITH_TIMEOUT(folderHasMessageFile(trash, stem), 15000); + QVERIFY2(!folderHasMessageFile(inbox, stem), + "the file is in the trash and still in the inbox"); + QVERIFY2(!folderHasMessageFile(root + QStringLiteral("/acct/inbox/cur"), + stem), + "the file is in the trash and still in the inbox"); + + // The index half, which the filesystem cannot see. A moved file with a + // stale index entry sits correctly on disk and is invisible to every query. + queryEdit->setText(QStringLiteral("path:\"acct/Trash/**\"")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); +} + +void TestMainWindow::deleteRecordsWhereTheMessageCameFrom() +{ + // A Maildir filename does not record where a message came from, and once + // the file has moved notmuch cannot know either. The tag is the only + // record, and Restore needs it days later. + WorkerBackedWindow backed; + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/inbox"), QStringLiteral("del2@example.org"), + QStringLiteral("Delete me too"), QStringLiteral("sender@example.org"), + QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"), + QStringLiteral("Body text."))); + QVERIFY2(backed.build(QStringLiteral("acct"), QStringLiteral("acct"), + QStringLiteral("Trash")), + qPrintable(backed.error())); + + MainWindow window(backed.config()); + auto *model = window.findChild<ThreadListModel *>(); + auto *view = window.findChild<ThreadListView *>(); + auto *queryEdit = + window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + QVERIFY(model && view && queryEdit); + + queryEdit->setText(QStringLiteral("tag:inbox")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); + + view->setCurrentIndex(model->index(0, 0, QModelIndex())); + window.findChild<QAction *>(QStringLiteral("delete"))->trigger(); + + // Asked of the database, not of the model: the model's optimistic update + // would report the tag whether or not the write ever landed. + // Re-queried by id and asserted on the TAG LIST the database returns. + // + // Not with `tag:"deleted-from:inbox"` in the query: notmuch's parser does + // not match a quoted tag containing a colon that way, so such a query + // returns nothing against a perfectly tagged message and reads as the + // feature being broken. Asking for the message and inspecting its tags + // cannot fail that way. + // Re-run per attempt, not once. The tag write is QUEUED behind the move, + // so a single query can land before the tags do; QTRY_VERIFY on the + // model's contents would then re-test a result that can never change, + // because nothing re-asks the database. Asking again each time is what + // makes this wait for the write rather than for the clock. + bool tagged = false; + for (int attempt = 0; attempt < 30 && !tagged; ++attempt) { + queryEdit->setText(QStringLiteral("id:del2@example.org")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); + const QStringList tags = model->threadAt(0).tags; + tagged = tags.contains(QStringLiteral("deleted")) + && tags.contains(QStringLiteral("deleted-from:inbox")); + if (!tagged) + QTest::qWait(200); + } + QVERIFY2(tagged, + qPrintable(QStringLiteral("tags after the delete: %1") + .arg(model->threadAt(0).tags.join( + QLatin1Char(' '))))); +} + +void TestMainWindow::deletingTwiceLeavesNoOriginTagBehind() +{ + // Delete twice is the ordinary way back: the action toggles, so a second + // press on a deleted message restores it. That path is NOT the undo path + // and had its own defect. + // + // onMessagesMoved() resolved the origin placeholder from the folder the + // WORKER reported, which is where the message came FROM. On a delete that + // is the inbox and correct. On a restore it is the TRASH, so the restore + // asked to remove `deleted-from:Trash`, a tag that had never been written, + // while the real `deleted-from:inbox` was never named and stayed on the + // message. It came home still claiming to have been deleted from + // somewhere, which makes Restore offer to move a message already at home. + // + // Reported from a hand test. The undo test passed throughout, because undo + // carries its tags on the command and never resolves a placeholder. + WorkerBackedWindow backed; + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/inbox"), QStringLiteral("twice@example.org"), + QStringLiteral("Delete me twice"), QStringLiteral("sender@example.org"), + QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"), + QStringLiteral("Body text."))); + QVERIFY2(backed.build(QStringLiteral("acct"), QStringLiteral("acct"), + QStringLiteral("Trash")), + qPrintable(backed.error())); + + MainWindow window(backed.config()); + auto *model = window.findChild<ThreadListModel *>(); + auto *view = window.findChild<ThreadListView *>(); + auto *queryEdit = + window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + QVERIFY(model && view && queryEdit); + + queryEdit->setText(QStringLiteral("tag:inbox")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); + + const QString root = backed.fixture().maildirPath(); + const QString stem = QStringLiteral("twice.example.org"); + const QString trash = root + QStringLiteral("/acct/Trash/cur"); + + view->setCurrentIndex(model->index(0, 0, QModelIndex())); + window.findChild<QAction *>(QStringLiteral("delete"))->trigger(); + QTRY_VERIFY_WITH_TIMEOUT(folderHasMessageFile(trash, stem), 15000); + + // The origin tag really was written, so the assertion after the second + // delete is about it being REMOVED rather than never having existed. + // + // Asked of the DATABASE, not through the query bar. The file arriving in + // the trash is not the end of the delete: the tag writes land after the + // rename this test waits for, and a query bar run inside that gap returns + // zero rows FOREVER, because QTRY_VERIFY re-reads rowCount() and never + // re-runs the query. Measured 1 failure in 3 runs, each burning the full + // 15s timeout on a guard that was correct about a database it had asked + // too early. + QTRY_VERIFY_WITH_TIMEOUT( + notmuchCount(backed.fixture().configPath(), + QStringLiteral("id:twice@example.org and " + "tag:\"deleted-from:inbox\"")) == 1, + 15000); + + // Second press on the same message, which restores it. + queryEdit->setText(QStringLiteral("id:twice@example.org")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); + view->setCurrentIndex(model->index(0, 0, QModelIndex())); + window.findChild<QAction *>(QStringLiteral("delete"))->trigger(); + + QTRY_VERIFY_WITH_TIMEOUT( + folderHasMessageFile(root + QStringLiteral("/acct/inbox/cur"), stem) + || folderHasMessageFile(root + QStringLiteral("/acct/inbox/new"), + stem), + 15000); + + // The file arriving is NOT the end of the restore. The tags are written + // only once the worker confirms the move, so the writes land after the + // rename the assertion above waits for. Querying in that gap reads the + // state before the restore finished tagging, which is how an earlier + // version of this test passed against the bug it exists to catch. + // + // Waited on the `deleted` tag, which the restore removes on every code + // path, rather than on a fixed sleep. + QTRY_VERIFY_WITH_TIMEOUT( + notmuchCount(backed.fixture().configPath(), + QStringLiteral("id:twice@example.org and tag:deleted")) + == 0, + 15000); + + // BOTH tags gone, asked of the database. `deleted-from:` left behind is + // the defect this covers, and it survived a green suite before. + // The origin tag specifically, asserted on its OWN query. + // + // A combined `tag:deleted or tag:"deleted-from:inbox"` query is NOT + // equivalent and passed against the bug: `deleted` is removed correctly + // and promptly, so the disjunction went to zero on that term alone while + // the origin tag was still on the message. Split, so the assertion can + // only be satisfied by the tag it names. + // Asked of notmuch DIRECTLY, not through the query bar. + // + // A UI query cannot answer this reliably: rowCount() is 0 for the whole + // interval before the worker replies, so QTRY_VERIFY(rowCount() == 0) is + // satisfied instantly by the empty pre-result and passes against any + // state of the database. Measured while building this test: 0 right after + // returnPressed(), 1 once the answer actually landed. The database is the + // thing under test here, so it is asked directly. + const QString cfg = backed.fixture().configPath(); + + // The message still exists: an assertion that a tag is absent would be + // satisfied just as well by the message having vanished. + QCOMPARE(notmuchCount(cfg, QStringLiteral("id:twice@example.org")), 1); + + // The origin tag is gone. This is the defect: it used to survive the + // restore, because the placeholder resolved to `deleted-from:Trash`, the + // folder the message was coming FROM, and stripped a tag that had never + // been written. + QCOMPARE(notmuchCount(cfg, + QStringLiteral("id:twice@example.org and " + "tag:\"deleted-from:inbox\"")), + 0); + + // And no tag naming the trash was invented in its place. + QCOMPARE(notmuchCount(cfg, + QStringLiteral("id:twice@example.org and " + "tag:\"deleted-from:Trash\"")), + 0); + + // `deleted` itself, so a fix that dropped this one instead cannot hide. + QCOMPARE(notmuchCount( + cfg, QStringLiteral("id:twice@example.org and tag:deleted")), + 0); + +} + +void TestMainWindow::undoOfADeleteRemovesTheOriginTagToo() +{ + // Ctrl+Z is a THIRD way back, beside the second Delete, and it had the + // same defect for a different reason. + // + // MoveCommand was constructed with pending.add, which still holds the + // unresolved origin PLACEHOLDER: onMessagesMoved() resolved the + // placeholder for the tags it wrote to the database, but handed the undo + // command the raw list. Undo then asked to remove a tag by the + // placeholder's literal name, which no message carries, so the removal + // was a silent no-op and `deleted-from:inbox` survived. The message came + // home still claiming to have been deleted from somewhere, which makes + // Restore offer to move a message that is already at home. + // + // Reported from a hand test after the second-Delete path was fixed: that + // fix did not touch this one, and the existing undo test asserted on the + // file's location rather than on its tags. + WorkerBackedWindow backed; + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/inbox"), QStringLiteral("undotag@example.org"), + QStringLiteral("Undo my tags"), QStringLiteral("sender@example.org"), + QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"), + QStringLiteral("Body text."))); + QVERIFY2(backed.build(QStringLiteral("acct"), QStringLiteral("acct"), + QStringLiteral("Trash")), + qPrintable(backed.error())); + + MainWindow window(backed.config()); + auto *model = window.findChild<ThreadListModel *>(); + auto *view = window.findChild<ThreadListView *>(); + auto *queryEdit = + window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + QVERIFY(model && view && queryEdit); + + queryEdit->setText(QStringLiteral("tag:inbox")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); + + const QString root = backed.fixture().maildirPath(); + const QString stem = QStringLiteral("undotag.example.org"); + const QString cfg = backed.fixture().configPath(); + + view->setCurrentIndex(model->index(0, 0, QModelIndex())); + window.findChild<QAction *>(QStringLiteral("delete"))->trigger(); + QTRY_VERIFY_WITH_TIMEOUT( + folderHasMessageFile(root + QStringLiteral("/acct/Trash/cur"), stem), + 15000); + + // The origin tag really was written, so the assertion after the undo is + // about it being REMOVED rather than never having existed. + QTRY_VERIFY_WITH_TIMEOUT( + notmuchCount(cfg, QStringLiteral("id:undotag@example.org and " + "tag:\"deleted-from:inbox\"")) == 1, + 15000); + + window.findChild<QAction *>(QStringLiteral("undo"))->trigger(); + QTRY_VERIFY_WITH_TIMEOUT( + folderHasMessageFile(root + QStringLiteral("/acct/inbox/cur"), stem) + || folderHasMessageFile(root + QStringLiteral("/acct/inbox/new"), + stem), + 15000); + + // The file arriving is not the end of the undo: the tags are written only + // once the worker confirms the move, so they land after the rename. + QTRY_VERIFY_WITH_TIMEOUT( + notmuchCount(cfg, + QStringLiteral("id:undotag@example.org and tag:deleted")) + == 0, + 15000); + + // Asked of notmuch directly. A UI query cannot answer this: rowCount() is + // 0 for the whole interval before the worker replies, so an assertion + // that a tag is absent is satisfied by the gap before any answer arrives. + QCOMPARE(notmuchCount(cfg, QStringLiteral("id:undotag@example.org")), 1); + QCOMPARE(notmuchCount(cfg, + QStringLiteral("id:undotag@example.org and " + "tag:\"deleted-from:inbox\"")), + 0); + QCOMPARE(notmuchCount(cfg, + QStringLiteral("id:undotag@example.org and " + "tag:\"deleted-from:Trash\"")), + 0); +} + +void TestMainWindow::deletingAThreadRootTwiceRestoresItRatherThanRedeleting() +{ + // The toggle asked a THREAD ROW about its thread's tags, which notmuch + // gives as a UNION over the conversation. Delete the root of a + // three-message thread and the two replies are untouched, so the union + // carries no `deleted`, so a second press read the row as not-deleted and + // ran Delete AGAIN: the message was moved trash-to-trash and came out + // carrying `deleted`, `deleted-from:inbox` AND `deleted-from:Trash`, with + // no way back, since a later restore would send it to the trash it now + // claims to have come from. + // + // The union was a documented approximation, called bounded because the + // worst case for a TAG toggle was re-applying a tag the message already + // had, which is a no-op. A MOVE re-applies the move. The comment outlived + // the code it described. + // + // The row must be left ALONE between the two presses: a re-query rebuilds + // it from the database and hides the defect, which is why an earlier + // version of this probe passed. The user's gesture is two presses on the + // list as it stands. + WorkerBackedWindow backed; + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/inbox"), QStringLiteral("troot@example.org"), + QStringLiteral("Thread root"), QStringLiteral("sender@example.org"), + QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"), + QStringLiteral("Root body."))); + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/inbox"), QStringLiteral("trep1@example.org"), + QStringLiteral("Re: Thread root"), QStringLiteral("other@example.org"), + QStringLiteral("Fri, 14 Aug 2026 11:00:00 +0200"), + QStringLiteral("Reply one."), true, + QStringLiteral("troot@example.org"))); + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/inbox"), QStringLiteral("trep2@example.org"), + QStringLiteral("Re: Thread root"), QStringLiteral("third@example.org"), + QStringLiteral("Fri, 14 Aug 2026 12:00:00 +0200"), + QStringLiteral("Reply two."), true, + QStringLiteral("troot@example.org"))); + QVERIFY2(backed.build(QStringLiteral("acct"), QStringLiteral("acct"), + QStringLiteral("Trash")), + qPrintable(backed.error())); + + MainWindow window(backed.config()); + auto *model = window.findChild<ThreadListModel *>(); + auto *view = window.findChild<ThreadListView *>(); + auto *queryEdit = + window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + QVERIFY(model && view && queryEdit); + + queryEdit->setText(QStringLiteral("tag:inbox")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); + + const QString root = backed.fixture().maildirPath(); + const QString cfg = backed.fixture().configPath(); + const QString stem = QStringLiteral("troot.example.org"); + const QString trash = root + QStringLiteral("/acct/Trash/cur"); + + // Three messages, so the union genuinely differs from the root's own + // tags. With one message the two are identical and the defect cannot + // appear at all. + QCOMPARE(notmuchCount(cfg, QStringLiteral("thread:{id:troot@example.org}")), + 3); + + view->setCurrentIndex(model->index(0, 0, QModelIndex())); + window.findChild<QAction *>(QStringLiteral("delete"))->trigger(); + QTRY_VERIFY_WITH_TIMEOUT(folderHasMessageFile(trash, stem), 15000); + QTRY_VERIFY_WITH_TIMEOUT( + notmuchCount(cfg, QStringLiteral("id:troot@example.org and " + "tag:\"deleted-from:inbox\"")) == 1, + 15000); + + // Only the root moved. The replies are what make the union disagree, so + // this is also the guard the rest of the test depends on. + QCOMPARE(notmuchCount(cfg, QStringLiteral("id:trep1@example.org and " + "tag:deleted")), + 0); + QCOMPARE(notmuchCount(cfg, QStringLiteral("id:trep2@example.org and " + "tag:deleted")), + 0); + + // Second press on the row as it stands, no re-query. + view->setCurrentIndex(model->index(0, 0, QModelIndex())); + window.findChild<QAction *>(QStringLiteral("delete"))->trigger(); + + QTRY_VERIFY_WITH_TIMEOUT( + folderHasMessageFile(root + QStringLiteral("/acct/inbox/cur"), stem) + || folderHasMessageFile(root + QStringLiteral("/acct/inbox/new"), + stem), + 15000); + QTRY_VERIFY_WITH_TIMEOUT( + notmuchCount(cfg, + QStringLiteral("id:troot@example.org and tag:deleted")) + == 0, + 15000); + + // Asked of notmuch directly: a UI query reads 0 rows for the whole + // interval before the worker answers, so an absence assertion through the + // query bar passes against any state of the database. + QCOMPARE(notmuchCount(cfg, QStringLiteral("id:troot@example.org")), 1); + QCOMPARE(notmuchCount(cfg, QStringLiteral("id:troot@example.org and " + "tag:\"deleted-from:inbox\"")), + 0); + // The tag the re-delete invented. Its presence is the signature of this + // defect rather than a variation on the origin-tag ones. + QCOMPARE(notmuchCount(cfg, QStringLiteral("id:troot@example.org and " + "tag:\"deleted-from:Trash\"")), + 0); + QVERIFY2(!folderHasMessageFile(trash, stem), + "the second press left the message in the trash"); +} + +void TestMainWindow::deleteThreadMovesEveryMessageAndRepaintsTheRootCard() +{ + // Two defects in one gesture, both reported from a hand test. + // + // Delete thread never moved anything: it was left calling tagSelected() + // when Delete became a move, so a whole conversation stayed in the inbox + // wearing a `deleted` chip, which is the half-deleted state item 103 + // existed to remove. It moves every message now, each carrying its own + // `deleted-from:` origin so a thread spanning folders reassembles. + // + // And the ROOT card did not repaint until it was clicked, while its + // replies did. A thread-scoped move updated each message's node; + // applyMessageTagChange() deliberately leaves a multi-message thread's + // SUMMARY alone, because one message's edit does not describe the + // conversation. The replies have nodes and repainted; the root card reads + // the summary and did not. A thread-scoped move DID change every message, + // so the summary genuinely moves and applyTagChange() is the right update. + // + // The stale summary was also why a second press did nothing: the toggle + // asks the summary for its direction and kept reading "not deleted". + WorkerBackedWindow backed; + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/inbox"), QStringLiteral("dt0@example.org"), + QStringLiteral("DT root"), QStringLiteral("sender@example.org"), + QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"), + QStringLiteral("Root body."))); + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/inbox"), QStringLiteral("dt1@example.org"), + QStringLiteral("Re: DT root"), QStringLiteral("other@example.org"), + QStringLiteral("Fri, 14 Aug 2026 11:00:00 +0200"), + QStringLiteral("Reply one."), true, QStringLiteral("dt0@example.org"))); + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/inbox"), QStringLiteral("dt2@example.org"), + QStringLiteral("Re: DT root"), QStringLiteral("third@example.org"), + QStringLiteral("Fri, 14 Aug 2026 12:00:00 +0200"), + QStringLiteral("Reply two."), true, QStringLiteral("dt0@example.org"))); + QVERIFY2(backed.build(QStringLiteral("acct"), QStringLiteral("acct"), + QStringLiteral("Trash")), + qPrintable(backed.error())); + + MainWindow window(backed.config()); + auto *model = window.findChild<ThreadListModel *>(); + auto *view = window.findChild<ThreadListView *>(); + auto *queryEdit = + window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + QVERIFY(model && view && queryEdit); + + queryEdit->setText(QStringLiteral("tag:inbox")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); + + const QString root = backed.fixture().maildirPath(); + const QString cfg = backed.fixture().configPath(); + const QString trash = root + QStringLiteral("/acct/Trash/cur"); + const QString thread = QStringLiteral("thread:{id:dt0@example.org}"); + + // Three messages, so a thread-scoped action is distinguishable from a + // message-scoped one at all. + QCOMPARE(notmuchCount(cfg, thread), 3); + + view->setCurrentIndex(model->index(0, 0, QModelIndex())); + view->expand(model->index(0, 0, QModelIndex())); + window.findChild<QAction *>(QStringLiteral("delete_thread"))->trigger(); + + // Every message MOVED, not merely tagged. This is the half that was + // missing entirely: the action tagged and moved nothing. + QTRY_VERIFY_WITH_TIMEOUT( + folderHasMessageFile(trash, QStringLiteral("dt0.example.org")) + && folderHasMessageFile(trash, QStringLiteral("dt1.example.org")) + && folderHasMessageFile(trash, QStringLiteral("dt2.example.org")), + 15000); + QTRY_VERIFY_WITH_TIMEOUT( + notmuchCount(cfg, thread + QStringLiteral(" and tag:deleted")) == 3, + 15000); + // Each with its own origin, which is what makes the move reversible. + QCOMPARE(notmuchCount(cfg, thread + + QStringLiteral(" and " + "tag:\"deleted-from:inbox\"")), + 3); + + // The ROOT CARD's own state, which is what the user watches. Read from the + // summary because that is what a thread row draws, and it is the value + // that stayed stale: the replies repainted and the root did not. + QVERIFY2(model->threadAt(0).tags.contains(QStringLiteral("deleted")), + "the root card still reads as not deleted, so it paints " + "undeleted until the row is clicked"); + + // Second press restores the whole thread, which only works if the toggle + // can see the state the first press produced. + view->setCurrentIndex(model->index(0, 0, QModelIndex())); + window.findChild<QAction *>(QStringLiteral("delete_thread"))->trigger(); + + QTRY_VERIFY_WITH_TIMEOUT( + notmuchCount(cfg, thread + QStringLiteral(" and tag:deleted")) == 0, + 15000); + + // Home, and nothing left behind in the trash. + QCOMPARE(notmuchCount(cfg, thread), 3); + QCOMPARE(notmuchCount(cfg, thread + + QStringLiteral(" and " + "tag:\"deleted-from:inbox\"")), + 0); + QVERIFY(!folderHasMessageFile(trash, QStringLiteral("dt0.example.org"))); + QVERIFY(!folderHasMessageFile(trash, QStringLiteral("dt1.example.org"))); + QVERIFY(!folderHasMessageFile(trash, QStringLiteral("dt2.example.org"))); +} + +void TestMainWindow::aFolderNameWithASpaceSurvivesTheRoundTrip() +{ + // A notmuch tag MAY contain a space, and a Maildir folder name may too. + // The worker reported each message's tags as one space-joined string, so + // `deleted-from:Inbox/SlackBuilds users` was split back into + // "deleted-from:Inbox/SlackBuilds" and "users", and Restore moved the + // messages to the truncated folder, CREATING it. On the user's real + // Maildir that put four messages into a directory mbsync does not sync, + // beside the real folder of 808, and they read as missing. + // + // The leftover origin tag was the visible half: the restore stripped the + // truncated name, which no message carried, so the real tag stayed on. + // + // Separator is a TAB now. A tag cannot contain one, since notmuch's own + // dump format is line-based and whitespace-delimited. + WorkerBackedWindow backed; + const QString folder = QStringLiteral("acct/Inbox/SlackBuilds users"); + QVERIFY(backed.fixture().addMessage( + folder, QStringLiteral("sp0@example.org"), QStringLiteral("SP root"), + QStringLiteral("sender@example.org"), + QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"), + QStringLiteral("Root body."))); + QVERIFY(backed.fixture().addMessage( + folder, QStringLiteral("sp1@example.org"), + QStringLiteral("Re: SP root"), QStringLiteral("other@example.org"), + QStringLiteral("Fri, 14 Aug 2026 11:00:00 +0200"), + QStringLiteral("Reply."), true, QStringLiteral("sp0@example.org"))); + QVERIFY2(backed.build(QStringLiteral("acct"), QStringLiteral("acct"), + QStringLiteral("Trash")), + qPrintable(backed.error())); + + MainWindow window(backed.config()); + auto *model = window.findChild<ThreadListModel *>(); + auto *view = window.findChild<ThreadListView *>(); + auto *queryEdit = + window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + QVERIFY(model && view && queryEdit); + + queryEdit->setText(QStringLiteral("tag:inbox")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); + + const QString root = backed.fixture().maildirPath(); + const QString cfg = backed.fixture().configPath(); + const QString thread = QStringLiteral("thread:{id:sp0@example.org}"); + const QString home = root + QLatin1Char('/') + folder; + + view->setCurrentIndex(model->index(0, 0, QModelIndex())); + window.findChild<QAction *>(QStringLiteral("delete_thread"))->trigger(); + + QTRY_VERIFY_WITH_TIMEOUT( + notmuchCount(cfg, thread + QStringLiteral(" and tag:deleted")) == 2, + 15000); + + // The origin tag carries the WHOLE folder name, space included. + QCOMPARE(notmuchCount(cfg, + thread + + QStringLiteral(" and tag:\"deleted-from:" + "Inbox/SlackBuilds users\"")), + 2); + + // Back again. + view->setCurrentIndex(model->index(0, 0, QModelIndex())); + window.findChild<QAction *>(QStringLiteral("delete_thread"))->trigger(); + + QTRY_VERIFY_WITH_TIMEOUT( + notmuchCount(cfg, thread + QStringLiteral(" and tag:deleted")) == 0, + 15000); + + // No origin tag left behind. This is the half the user saw: a tag they + // could see, could not type, and could not remove. + QCOMPARE(notmuchCount(cfg, + thread + + QStringLiteral(" and tag:\"deleted-from:" + "Inbox/SlackBuilds users\"")), + 0); + // Nor a truncated one, which is what a space-split would have written. + QCOMPARE(notmuchCount(cfg, + thread + + QStringLiteral(" and tag:\"deleted-from:" + "Inbox/SlackBuilds\"")), + 0); + + // Home, in the folder with the space in its name. + QVERIFY2(folderHasMessageFile(home + QStringLiteral("/cur"), + QStringLiteral("sp0.example.org")), + "the root did not come back to the folder it was deleted from"); + QVERIFY2(folderHasMessageFile(home + QStringLiteral("/cur"), + QStringLiteral("sp1.example.org")), + "the reply did not come back to the folder it was deleted from"); + + // And the truncated folder was never created. Its existence is the defect + // that hid four real messages from the user and from mbsync. + QVERIFY2(!QDir(root + QStringLiteral("/acct/Inbox/SlackBuilds")).exists(), + "a folder named after the truncated origin was created, so the " + "messages are somewhere mbsync will never sync"); +} + +void TestMainWindow::deleteIsBoundToTheDeleteKey() +{ + // Del is the key a user reaches for, and Ctrl+D is not a guess anyone + // makes. Both are bound; this asserts the bare one is really there, + // since setShortcut() keeps only the LAST of several and silently drops + // the rest, which would leave the documented binding absent. + const Config config; + MainWindow window(config); + + auto *action = window.findChild<QAction *>(QStringLiteral("delete")); + QVERIFY(action); + + QVERIFY2(action->shortcuts().contains(QKeySequence(Qt::Key_Delete)), + qPrintable(QStringLiteral("delete is not on the Del key; it has: %1") + .arg(QKeySequence::listToString(action->shortcuts())))); +} + +void TestMainWindow::theDeleteKeyEditsTextInTheQueryBar() +{ + // `delete` is bound to bare Del, and a QAction shortcut is dispatched + // BEFORE the focused widget sees the key. Qt withholds only plain LETTERS + // from editable widgets, so by the argument that made bare Return break + // the query bar, Delete should move mail to the trash while the user is + // editing a query. + // + // It does not: QLineEdit accepts the ShortcutOverride for Delete itself, + // because Delete is one of its own editing keys, which Return is not. That + // is a property of Qt rather than of this code, which is exactly why it is + // pinned here: it is the assumption the bare binding rests on, and if a + // future Qt or a future focus proxy changes it, mail gets deleted while + // someone types. + // + // Asserted on the ACTION not firing, not on the ShortcutOverride phase. A + // probe on the override reports notify=1 accepted=1 whether or not this + // window filters the key, since QLineEdit accepts it either way, so it + // cannot distinguish the two and passes against any implementation. + // Measured, while trying to write this test the obvious way. + const Config config; + MainWindow window(config); + window.show(); + QVERIFY(QTest::qWaitForWindowExposed(&window)); + + auto *queryEdit = + window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + auto *deleteAction = window.findChild<QAction *>(QStringLiteral("delete")); + QVERIFY(queryEdit && deleteAction); + + int fired = 0; + QObject::connect(deleteAction, &QAction::triggered, + [&fired]() { ++fired; }); + + queryEdit->setFocus(); + QTRY_VERIFY(queryEdit->hasFocus()); + queryEdit->setText(QStringLiteral("tag:inbox")); + queryEdit->setCursorPosition(0); + + QTest::keyClick(queryEdit, Qt::Key_Delete); + + QCOMPARE(fired, 0); + QCOMPARE(queryEdit->text(), QStringLiteral("ag:inbox")); +} + +void TestMainWindow::restoreIsReachableWithoutTheKeyboard() +{ + // Restore shipped as a keyboard shortcut and nothing else: registered, + // iconned, enabled correctly, and present in no menu at all. A user who + // does not read the changelog would never learn it exists, and Ctrl+R is + // not a guess anyone makes. + // + // The four places an action must touch are enforced by tests + // (knownActions, defaultBindings, the icon table); being REACHABLE is a + // fifth that nothing checked, which is why the gap survived a green suite. + const Config config; + MainWindow window(config); + + auto *restore = window.findChild<QAction *>(QStringLiteral("restore")); + QVERIFY(restore); + + const auto menuContains = [](const QMenu *menu, const QAction *action) { + return menu && menu->actions().contains(action); + }; + + // A menu on the MENU BAR, beside Delete whose inverse it is. The context + // menu is excluded here so this assertion cannot be satisfied by the one + // the next assertion checks: findChildren finds both. + auto *context = + window.findChild<QMenu *>(QStringLiteral("threadContextMenu")); + QVERIFY(context); + + bool inAMenuBarMenu = false; + for (const QMenu *menu : window.findChildren<QMenu *>()) { + if (menu != context && menuContains(menu, restore)) { + inAMenuBarMenu = true; + break; + } + } + QVERIFY2(inAMenuBarMenu, + "Restore is in no menu-bar menu, so a user browsing the menus " + "would never learn it exists"); + + // And the thread list's context menu, which is where the other + // message-scoped actions are reached by mouse. + QVERIFY2(menuContains(context, restore), + "Restore is missing from the thread context menu"); +} + +void TestMainWindow::restoreIsOnlyEnabledInTheTrashView() +{ + // Restore has no meaning outside the trash, and an enabled action that + // does nothing is worse than an absent one. + // + // Enabled from the QUERY rather than from the selection's tags: a message + // trashed by another client carries no tag of ours and must still be + // restorable, which is the whole reason the trash view is path-based. + WorkerBackedWindow backed; + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/inbox"), QStringLiteral("re1@example.org"), + QStringLiteral("In the inbox"), QStringLiteral("sender@example.org"), + QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"), + QStringLiteral("Body text."))); + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/Trash"), QStringLiteral("re2@example.org"), + QStringLiteral("In the trash"), QStringLiteral("other@example.org"), + QStringLiteral("Fri, 14 Aug 2026 11:00:00 +0200"), + QStringLiteral("Body text."))); + QVERIFY2(backed.build(QStringLiteral("acct"), QStringLiteral("acct"), + QStringLiteral("Trash")), + qPrintable(backed.error())); + + MainWindow window(backed.config()); + auto *model = window.findChild<ThreadListModel *>(); + auto *queryEdit = + window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + auto *restore = window.findChild<QAction *>(QStringLiteral("restore")); + QVERIFY(model && queryEdit); + QVERIFY2(restore, "there is no restore action"); + + // An ordinary view. Both fixture messages carry `inbox`, since the + // fixture tags all new mail that way regardless of folder, so this is two + // rows rather than one; the count is not what is under test. + queryEdit->setText(QStringLiteral("tag:inbox")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 2, 15000); + QVERIFY2(!restore->isEnabled(), + "Restore is enabled in an ordinary view, where it means nothing"); + + // The trash view, which is the account's own generated trash query. + queryEdit->setText(QStringLiteral("path:\"acct/Trash/**\"")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); + QVERIFY2(restore->isEnabled(), + "Restore is disabled in the trash view, where it is the point"); +} + +void TestMainWindow::restoreReturnsAMessageToItsOriginFolder() +{ + WorkerBackedWindow backed; + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/inbox"), QStringLiteral("ro1@example.org"), + QStringLiteral("Send me back"), QStringLiteral("sender@example.org"), + QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"), + QStringLiteral("Body text."))); + QVERIFY2(backed.build(QStringLiteral("acct"), QStringLiteral("acct"), + QStringLiteral("Trash")), + qPrintable(backed.error())); + + MainWindow window(backed.config()); + auto *model = window.findChild<ThreadListModel *>(); + auto *view = window.findChild<ThreadListView *>(); + auto *queryEdit = + window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + QVERIFY(model && view && queryEdit); + + const QString root = backed.fixture().maildirPath(); + const QString cfg = backed.fixture().configPath(); + const QString stem = QStringLiteral("ro1.example.org"); + + queryEdit->setText(QStringLiteral("tag:inbox")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); + view->setCurrentIndex(model->index(0, 0, QModelIndex())); + window.findChild<QAction *>(QStringLiteral("delete"))->trigger(); + QTRY_VERIFY_WITH_TIMEOUT( + folderHasMessageFile(root + QStringLiteral("/acct/Trash/cur"), stem), + 15000); + + // Now from the trash view, through Restore rather than through a second + // Delete: this is the action the user reaches for when browsing trash. + queryEdit->setText(QStringLiteral("path:\"acct/Trash/**\"")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); + view->setCurrentIndex(model->index(0, 0, QModelIndex())); + window.findChild<QAction *>(QStringLiteral("restore"))->trigger(); + + QTRY_VERIFY_WITH_TIMEOUT( + folderHasMessageFile(root + QStringLiteral("/acct/inbox/cur"), stem) + || folderHasMessageFile(root + QStringLiteral("/acct/inbox/new"), + stem), + 15000); + // Waited on the ORIGIN tag, not on `deleted`. + // + // Both come off in one write, but the file rename and the tag write are + // separate operations and the assertions below raced the second one: + // measured 1 failure in 3 runs waiting on `deleted` alone, reporting the + // origin tag still present. Waiting on the tag this test is actually about + // removes the race rather than papering over it with a longer timeout. + QTRY_VERIFY_WITH_TIMEOUT( + notmuchCount(cfg, QStringLiteral("id:ro1@example.org and " + "tag:\"deleted-from:inbox\"")) == 0, + 15000); + QTRY_VERIFY_WITH_TIMEOUT( + notmuchCount(cfg, + QStringLiteral("id:ro1@example.org and tag:deleted")) == 0, + 15000); + + QCOMPARE(notmuchCount(cfg, QStringLiteral("id:ro1@example.org")), 1); + QVERIFY(!folderHasMessageFile(root + QStringLiteral("/acct/Trash/cur"), + stem)); +} + +void TestMainWindow::restoreFallsBackToInboxWithoutAnOriginTag() +{ + // A message trashed by ANOTHER client: it sits in the trash folder and + // carries no `deleted-from:` tag, because nothing here put it there. The + // real Maildir has such messages, which is why the trash view is path + // based rather than tag based. + // + // Inbox is the documented fallback. Refusing to move it would leave the + // user with a message they can see in the trash and cannot get out. + WorkerBackedWindow backed; + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/Trash"), QStringLiteral("foreign@example.org"), + QStringLiteral("Trashed elsewhere"), QStringLiteral("sender@example.org"), + QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"), + QStringLiteral("Body text."))); + QVERIFY2(backed.build(QStringLiteral("acct"), QStringLiteral("acct"), + QStringLiteral("Trash")), + qPrintable(backed.error())); + + MainWindow window(backed.config()); + auto *model = window.findChild<ThreadListModel *>(); + auto *view = window.findChild<ThreadListView *>(); + auto *queryEdit = + window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + QVERIFY(model && view && queryEdit); + + const QString root = backed.fixture().maildirPath(); + const QString cfg = backed.fixture().configPath(); + const QString stem = QStringLiteral("foreign.example.org"); + + // The guard this test needs: no origin tag, so the fallback is what is + // under test rather than an ordinary restore. + QCOMPARE(notmuchCount(cfg, QStringLiteral("id:foreign@example.org and " + "tag:\"deleted-from:inbox\"")), + 0); + + queryEdit->setText(QStringLiteral("path:\"acct/Trash/**\"")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); + view->setCurrentIndex(model->index(0, 0, QModelIndex())); + window.findChild<QAction *>(QStringLiteral("restore"))->trigger(); + + QTRY_VERIFY_WITH_TIMEOUT( + folderHasMessageFile(root + QStringLiteral("/acct/inbox/cur"), stem) + || folderHasMessageFile(root + QStringLiteral("/acct/inbox/new"), + stem), + 15000); + QVERIFY2(!folderHasMessageFile(root + QStringLiteral("/acct/Trash/cur"), + stem), + "the message was copied out of the trash rather than moved"); +} + +void TestMainWindow::undoMovesTheMessageBack() +{ + // Undo is this project's answer to the confirmation dialog it rules out, + // so a delete that cannot be undone is a delete with no safety net at all. + WorkerBackedWindow backed; + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/inbox"), QStringLiteral("del3@example.org"), + QStringLiteral("Put me back"), QStringLiteral("sender@example.org"), + QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"), + QStringLiteral("Body text."))); + QVERIFY2(backed.build(QStringLiteral("acct"), QStringLiteral("acct"), + QStringLiteral("Trash")), + qPrintable(backed.error())); + + MainWindow window(backed.config()); + auto *model = window.findChild<ThreadListModel *>(); + auto *view = window.findChild<ThreadListView *>(); + auto *queryEdit = + window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + QVERIFY(model && view && queryEdit); + + queryEdit->setText(QStringLiteral("tag:inbox")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); + + const QString root = backed.fixture().maildirPath(); + const QString stem = QStringLiteral("del3.example.org"); + const QString trash = root + QStringLiteral("/acct/Trash/cur"); + + view->setCurrentIndex(model->index(0, 0, QModelIndex())); + window.findChild<QAction *>(QStringLiteral("delete"))->trigger(); + QTRY_VERIFY_WITH_TIMEOUT(folderHasMessageFile(trash, stem), 15000); + + window.findChild<QAction *>(QStringLiteral("undo"))->trigger(); + + // Back in the EXACT folder it came from. A move-back that guessed "inbox" + // for every account would pass a laxer assertion than this one. + // + // cur/, not the new/ it started in: a file coming back from the trash has + // been read, and re-announcing it as fresh mail is worse than the flag + // change. + QTRY_VERIFY_WITH_TIMEOUT( + folderHasMessageFile(root + QStringLiteral("/acct/inbox/cur"), stem) + || folderHasMessageFile(root + QStringLiteral("/acct/inbox/new"), + stem), + 15000); + QVERIFY2(!folderHasMessageFile(trash, stem), + "undo restored the file and left a copy in the trash"); + + // Both tags gone, asked of the database. `deleted-from:` left behind would + // make Restore offer to move a message that is already home. + queryEdit->setText(QStringLiteral( + "id:del3@example.org and (tag:deleted or tag:\"deleted-from:inbox\")")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 0, 15000); + // The guard the assertion above needs: a query that matches nothing + // because the message vanished would pass it too. + queryEdit->setText(QStringLiteral("id:del3@example.org")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); +} + +void TestMainWindow::deleteOnAReplyMovesThatReplyOnly() +{ + // The reply case. A test asserting on a root selection is the one case + // where the wrong resolution is accidentally right, so a mutation on this + // path stays green without it. + // + // Put under the SECOND thread, so the wrong answer is plausible rather + // than accidentally correct. + WorkerBackedWindow backed; + NotmuchFixture &fx = backed.fixture(); + QVERIFY(fx.addMessage( + QStringLiteral("acct/inbox"), QStringLiteral("other@example.org"), + QStringLiteral("An unrelated thread"), + QStringLiteral("sender@example.org"), + QStringLiteral("Fri, 14 Aug 2026 09:00:00 +0200"), + QStringLiteral("Body text."))); + QVERIFY(fx.addMessage( + QStringLiteral("acct/inbox"), QStringLiteral("rootof@example.org"), + QStringLiteral("A conversation"), QStringLiteral("sender@example.org"), + QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"), + QStringLiteral("Body text."))); + QVERIFY(fx.addMessage( + QStringLiteral("acct/inbox"), QStringLiteral("reply@example.org"), + QStringLiteral("Re: A conversation"), + QStringLiteral("other@example.org"), + QStringLiteral("Fri, 14 Aug 2026 11:00:00 +0200"), + QStringLiteral("Reply body."), true, + QStringLiteral("rootof@example.org"))); + QVERIFY2(backed.build(QStringLiteral("acct"), QStringLiteral("acct"), + QStringLiteral("Trash")), + qPrintable(backed.error())); + + MainWindow window(backed.config()); + auto *model = window.findChild<ThreadListModel *>(); + auto *view = window.findChild<ThreadListView *>(); + auto *queryEdit = + window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + QVERIFY(model && view && queryEdit); + + queryEdit->setText(QStringLiteral("tag:inbox")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 2, 15000); + + // Whichever row holds the conversation. The sort is the model's business, + // so this asks rather than assuming. + QModelIndex conversation; + for (int row = 0; row < model->rowCount(QModelIndex()); ++row) { + const QModelIndex index = model->index(row, 0, QModelIndex()); + if (model->threadAt(row).totalCount > 1) { + conversation = index; + break; + } + } + QVERIFY2(conversation.isValid(), "no multi-message thread in the list"); + + view->expand(conversation); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(conversation) == 1, 15000); + + const QModelIndex replyIndex = model->index(0, 0, conversation); + QVERIFY(model->isMessageRow(replyIndex)); + QCOMPARE(model->messageAt(replyIndex).messageId, + QStringLiteral("reply@example.org")); + + view->setCurrentIndex(replyIndex); + window.findChild<QAction *>(QStringLiteral("delete"))->trigger(); + + const QString root = fx.maildirPath(); + QTRY_VERIFY_WITH_TIMEOUT( + folderHasMessageFile(root + QStringLiteral("/acct/Trash/cur"), + QStringLiteral("reply.example.org")), + 15000); + + // Only that reply. Escalating a message-scoped delete to its thread would + // move the root as well, which is the failure worth naming: the user + // deleted one reply and lost the conversation. + QVERIFY2(!folderHasMessageFile(root + QStringLiteral("/acct/Trash/cur"), + QStringLiteral("rootof.example.org")), + "deleting a reply moved its thread's root as well"); +} + +void TestMainWindow::undoingADeleteConsumesItsCommandRatherThanPushingAnother() +{ + // A move is confirmed through onMessagesMoved(), and so is the move an + // UNDO makes. Pushing a command there unconditionally meant undo left a + // fresh command on the stack instead of consuming the one it undid, so + // the stack grew on every press: "Delete", "Undo Delete", "Undo Undo + // Delete". A user pressing undo twice to be sure re-deleted the mail they + // had just rescued, which is the opposite of what undo is for here, undo + // being this project's stand-in for a confirmation dialog. + WorkerBackedWindow backed; + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/inbox"), QStringLiteral("undo2@example.org"), + QStringLiteral("Undo twice"), QStringLiteral("sender@example.org"), + QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"), + QStringLiteral("Body text."))); + QVERIFY2(backed.build(QStringLiteral("acct"), QStringLiteral("acct"), + QStringLiteral("Trash")), + qPrintable(backed.error())); + + MainWindow window(backed.config()); + auto *model = window.findChild<ThreadListModel *>(); + auto *view = window.findChild<ThreadListView *>(); + auto *queryEdit = + window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + QVERIFY(model && view && queryEdit); + + queryEdit->setText(QStringLiteral("tag:inbox")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); + + const QString root = backed.fixture().maildirPath(); + const QString stem = QStringLiteral("undo2.example.org"); + const QString trash = root + QStringLiteral("/acct/Trash/cur"); + const auto inInbox = [&] { + return folderHasMessageFile(root + QStringLiteral("/acct/inbox/cur"), + stem) + || folderHasMessageFile( + root + QStringLiteral("/acct/inbox/new"), stem); + }; + + view->setCurrentIndex(model->index(0, 0, QModelIndex())); + window.findChild<QAction *>(QStringLiteral("delete"))->trigger(); + QTRY_VERIFY_WITH_TIMEOUT(folderHasMessageFile(trash, stem), 15000); + + // The guard the assertions below need: one command, from the delete. + QTRY_VERIFY_WITH_TIMEOUT(window.undoDepthForTesting() == 1, 15000); + + window.findChild<QAction *>(QStringLiteral("undo"))->trigger(); + QTRY_VERIFY_WITH_TIMEOUT(inInbox(), 15000); + + // The stack is spent. Asserted on undoText rather than depth alone + // because a command that is merely marked done still reports its text, + // and it is the text the user reads off the Edit menu. + QTRY_VERIFY_WITH_TIMEOUT(window.undoTextForTesting().isEmpty(), 15000); + + // And the real point: pressing undo again must not move the message + // anywhere. Before the fix this put it straight back in the trash. + window.findChild<QAction *>(QStringLiteral("undo"))->trigger(); + QTest::qWait(1500); + QVERIFY2(!folderHasMessageFile(trash, stem), + "a second undo re-deleted the message the first one restored"); + QVERIFY2(inInbox(), "a second undo moved the message out of the inbox"); +} + +void TestMainWindow::aDeleteHeldDuringASyncCountsAsUnsyncedWork() +{ + // pendingEditCount() summed the held TAG edits and not the held MOVES, so + // a Delete pressed during a sync left the count at zero: the indicator + // stayed hidden and closeEvent()'s `pendingEditCount() > 0` guard never + // fired, discarding the move on quit with no prompt. That is item 106's + // data loss with a worse shape, since a dropped move leaves the file in + // the folder the user asked it out of. + WorkerBackedWindow backed; + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/inbox"), QStringLiteral("held1@example.org"), + QStringLiteral("Held by a sync"), QStringLiteral("sender@example.org"), + QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"), + QStringLiteral("Body text."))); + QVERIFY2(backed.build(QStringLiteral("acct"), QStringLiteral("acct"), + QStringLiteral("Trash")), + qPrintable(backed.error())); + + MainWindow window(backed.config()); + auto *model = window.findChild<ThreadListModel *>(); + auto *view = window.findChild<ThreadListView *>(); + auto *queryEdit = + window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + auto *label = window.findChild<QLabel *>(QStringLiteral("pendingEdits")); + QVERIFY(model && view && queryEdit && label); + + queryEdit->setText(QStringLiteral("tag:inbox")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); + + // Hidden before the gesture, so the assertion after it means something. + QVERIFY2(label->isHidden(), "the pending indicator was already showing"); + + // A sync now holds the write lock, which is what makes the move held + // rather than sent. + QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged", + Q_ARG(SyncMonitor::State, + SyncMonitor::State::Running)); + + view->setCurrentIndex(model->index(0, 0, QModelIndex())); + window.findChild<QAction *>(QStringLiteral("delete"))->trigger(); + + QVERIFY2(!label->isHidden(), + "a Delete held by a sync did not count as unsynced work, so " + "quitting would have discarded it with no prompt"); + + // The file really is still where it was: this is a HELD move, not a + // failed one, and the indicator would be meaningless otherwise. + const QString root = backed.fixture().maildirPath(); + QVERIFY(!folderHasMessageFile(root + QStringLiteral("/acct/Trash/cur"), + QStringLiteral("held1.example.org"))); +} + +void TestMainWindow::twoDeletesToOneTrashBothGetTheirTags() +{ + // The pending-move table was keyed on the destination folder, so two + // Deletes in one account before the first confirmation arrived both named + // `acct/Trash`: the second insert overwrote the first and the second + // confirmation took an empty entry. That file reached the trash carrying + // neither `deleted` nor `deleted-from:`, which makes it unrestorable by + // Restore and invisible to a `tag:deleted` query. + WorkerBackedWindow backed; + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/inbox"), QStringLiteral("two1@example.org"), + QStringLiteral("First"), QStringLiteral("sender@example.org"), + QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"), + QStringLiteral("Body text."))); + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/inbox"), QStringLiteral("two2@example.org"), + QStringLiteral("Second"), QStringLiteral("other@example.org"), + QStringLiteral("Fri, 14 Aug 2026 11:00:00 +0200"), + QStringLiteral("Body text."))); + QVERIFY2(backed.build(QStringLiteral("acct"), QStringLiteral("acct"), + QStringLiteral("Trash")), + qPrintable(backed.error())); + + MainWindow window(backed.config()); + auto *model = window.findChild<ThreadListModel *>(); + auto *view = window.findChild<ThreadListView *>(); + auto *queryEdit = + window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + QVERIFY(model && view && queryEdit); + + queryEdit->setText(QStringLiteral("tag:inbox")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 2, 15000); + + // Both Deletes issued back to back, WITHOUT waiting for the first to be + // confirmed. That is the whole point: waiting would serialise them and + // the keyed table would have coped. + view->setCurrentIndex(model->index(0, 0, QModelIndex())); + window.findChild<QAction *>(QStringLiteral("delete"))->trigger(); + view->setCurrentIndex(model->index(1, 0, QModelIndex())); + window.findChild<QAction *>(QStringLiteral("delete"))->trigger(); + + const QString root = backed.fixture().maildirPath(); + const QString trash = root + QStringLiteral("/acct/Trash/cur"); + QTRY_VERIFY_WITH_TIMEOUT( + folderHasMessageFile(trash, QStringLiteral("two1.example.org")) + && folderHasMessageFile(trash, QStringLiteral("two2.example.org")), + 15000); + + // Both carry BOTH tags, asked of the database rather than of the model: + // the defect was a write that never happened, and the model would have + // shown the optimistic state either way. + queryEdit->setText(QStringLiteral( + "tag:deleted and tag:\"deleted-from:inbox\" and " + "(id:two1@example.org or id:two2@example.org)")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 2, 15000); +} + +void TestMainWindow::deleteWithoutATrashFolderSaysSoRatherThanDoingNothing() +{ + // Task 2 warns at config load. This is the second line of defence: a key + // the user never fixed must not leave Delete silently inert. + WorkerBackedWindow backed; + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/inbox"), QStringLiteral("notrash@example.org"), + QStringLiteral("Nowhere to go"), QStringLiteral("sender@example.org"), + QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"), + QStringLiteral("Body text."))); + // No trash key, which is what this is about. + QVERIFY2(backed.build(QStringLiteral("acct"), QStringLiteral("acct")), + qPrintable(backed.error())); + + MainWindow window(backed.config()); + auto *model = window.findChild<ThreadListModel *>(); + auto *view = window.findChild<ThreadListView *>(); + auto *queryEdit = + window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + auto *status = window.findChild<QLabel *>(QStringLiteral("statusMessage")); + QVERIFY(model && view && queryEdit && status); + + queryEdit->setText(QStringLiteral("tag:inbox")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); + + view->setCurrentIndex(model->index(0, 0, QModelIndex())); + + // Cleared FIRST, so the assertion below cannot be satisfied by whatever + // the query left behind. Without this the test passes against a Delete + // that says nothing at all, which is exactly what it exists to catch: it + // did, before the implementation landed. + status->clear(); + window.findChild<QAction *>(QStringLiteral("delete"))->trigger(); + + QVERIFY2(status->text().contains(QStringLiteral("trash")), + qPrintable(QStringLiteral( + "Delete with no trash folder configured said: '%1'") + .arg(status->text()))); + + // And it did not tag the message either. A `deleted` tag with the file + // still in the inbox is exactly the half-done state item 103 removes. + const QString mail = backed.fixture().maildirPath(); + QVERIFY(folderHasMessageFile(mail + QStringLiteral("/acct/inbox/new"), + QStringLiteral("notrash.example.org")) + || folderHasMessageFile(mail + QStringLiteral("/acct/inbox/cur"), + QStringLiteral("notrash.example.org"))); +} + +void TestMainWindow::theCleanupQueryFindsStrandedMail() +{ + // The state 848 real messages are in today: tagged `deleted` by a version + // of Delete that only ever tagged, with the file still sitting in the + // inbox. Nothing moves them on their own, so the action reports them and + // the user decides. + WorkerBackedWindow backed; + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/inbox"), QStringLiteral("strand@example.org"), + QStringLiteral("Tagged but never moved"), + QStringLiteral("sender@example.org"), + QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"), + QStringLiteral("Body text."))); + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/inbox"), QStringLiteral("keep@example.org"), + QStringLiteral("Perfectly ordinary mail"), + QStringLiteral("other@example.org"), + QStringLiteral("Fri, 14 Aug 2026 11:00:00 +0200"), + QStringLiteral("Body text."))); + QVERIFY2(backed.build(QStringLiteral("acct"), QStringLiteral("acct"), + QStringLiteral("Trash")), + qPrintable(backed.error())); + + const QString cfg = backed.fixture().configPath(); + QVERIFY(notmuchTag(cfg, { QStringLiteral("+deleted"), + QStringLiteral("--"), + QStringLiteral("id:strand@example.org") })); + // The guard, before anything is asserted about what the action finds: one + // message is stranded and one is not, so a query that simply returns + // everything cannot pass. + QCOMPARE(notmuchCount(cfg, QStringLiteral("tag:deleted")), 1); + + MainWindow window(backed.config()); + auto *model = window.findChild<ThreadListModel *>(); + auto *queryEdit = + window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + auto *cleanup = + window.findChild<QAction *>(QStringLiteral("cleanup_stranded")); + QVERIFY(model && queryEdit); + QVERIFY2(cleanup, "there is no cleanup_stranded action"); + + cleanup->trigger(); + + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); + // The query lands in the bar, like every other generated query, so what + // ran is visible and the user can edit it. + QVERIFY2(queryEdit->text().contains(QStringLiteral("tag:deleted")), + qPrintable(QStringLiteral("the bar holds '%1'") + .arg(queryEdit->text()))); + QVERIFY2(queryEdit->text().contains(QStringLiteral("not ")), + qPrintable(QStringLiteral("the bar holds '%1'") + .arg(queryEdit->text()))); + + // It reports and moves NOTHING. A cleanup that acted on its own would be a + // bulk delete with no selection behind it, which is the opposite of what + // the user asked for. + const QString mail = backed.fixture().maildirPath(); + QVERIFY(folderHasMessageFile(mail + QStringLiteral("/acct/inbox/new"), + QStringLiteral("strand.example.org")) + || folderHasMessageFile(mail + QStringLiteral("/acct/inbox/cur"), + QStringLiteral("strand.example.org"))); + QCOMPARE(notmuchCount(cfg, QStringLiteral("path:\"acct/Trash/**\"")), 0); +} + +void TestMainWindow::theCleanupQueryExcludesMailAlreadyInTrash() +{ + // Properly trashed mail carries the tag AND sits in the folder. Without + // the exclusion this reports every deleted message ever, which makes the + // action useless the moment Delete starts working. + WorkerBackedWindow backed; + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/inbox"), QStringLiteral("cln1@example.org"), + QStringLiteral("Going to the trash"), + QStringLiteral("sender@example.org"), + QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"), + QStringLiteral("Body text."))); + QVERIFY2(backed.build(QStringLiteral("acct"), QStringLiteral("acct"), + QStringLiteral("Trash")), + qPrintable(backed.error())); + + MainWindow window(backed.config()); + auto *model = window.findChild<ThreadListModel *>(); + auto *view = window.findChild<ThreadListView *>(); + auto *queryEdit = + window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + QVERIFY(model && view && queryEdit); + + const QString cfg = backed.fixture().configPath(); + const QString mail = backed.fixture().maildirPath(); + + queryEdit->setText(QStringLiteral("tag:inbox")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); + view->setCurrentIndex(model->index(0, 0, QModelIndex())); + window.findChild<QAction *>(QStringLiteral("delete"))->trigger(); + + // Asked of the database, never of the list: rowCount() reads 0 for the + // whole interval before the worker answers, so "the cleanup found + // nothing" would pass against a delete that never happened. + QTRY_VERIFY_WITH_TIMEOUT( + folderHasMessageFile(mail + QStringLiteral("/acct/Trash/cur"), + QStringLiteral("cln1.example.org")), + 15000); + QTRY_VERIFY_WITH_TIMEOUT( + notmuchCount(cfg, QStringLiteral("tag:deleted")) == 1, 15000); + + auto *cleanup = + window.findChild<QAction *>(QStringLiteral("cleanup_stranded")); + QVERIFY2(cleanup, "there is no cleanup_stranded action"); + cleanup->trigger(); + + // The query the action ran, asked of notmuch directly. The list is the + // wrong instrument for an emptiness claim, for the reason above. + QTRY_VERIFY_WITH_TIMEOUT(!queryEdit->text().isEmpty(), 15000); + QCOMPARE(notmuchCount(cfg, queryEdit->text()), 0); +} + +void TestMainWindow::aMoveThatRelocatesNothingWritesNoTag() +{ + // The spec's ordering bullet, at the UI level: a failed rename must leave + // no tag. The worker half is moveMessagesReportsOnlyWhatMoved(); this is + // the other half, that the window writes tags only for what the worker + // reported as actually moved. + // + // The failure is provoked by making the destination unwritable, which is + // the closest thing to a failed rename that a test can arrange without + // stubbing the worker. A tag written anyway would be the exact half-done + // state item 103 exists to remove: a message marked deleted, with its file + // still in the inbox and its origin tag lying about where it went. + WorkerBackedWindow backed; + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/inbox"), QStringLiteral("nomove@example.org"), + QStringLiteral("Cannot be moved"), QStringLiteral("sender@example.org"), + QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"), + QStringLiteral("Body text."))); + QVERIFY2(backed.build(QStringLiteral("acct"), QStringLiteral("acct"), + QStringLiteral("Trash")), + qPrintable(backed.error())); + + const QString root = backed.fixture().maildirPath(); + const QString cfg = backed.fixture().configPath(); + + // The trash as a FILE where the folder must be, so creating the Maildir + // subdirectories under it cannot succeed. A read-only directory would be + // ignored by a test running as root, which this one must not depend on. + QFile blocker(root + QStringLiteral("/acct/Trash")); + QVERIFY2(blocker.open(QIODevice::WriteOnly), + "could not put a file where the trash folder would go"); + blocker.close(); + + MainWindow window(backed.config()); + auto *model = window.findChild<ThreadListModel *>(); + auto *view = window.findChild<ThreadListView *>(); + auto *queryEdit = + window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + QVERIFY(model && view && queryEdit); + + queryEdit->setText(QStringLiteral("tag:inbox")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); + view->setCurrentIndex(model->index(0, 0, QModelIndex())); + window.findChild<QAction *>(QStringLiteral("delete"))->trigger(); + + // The guard, so this cannot pass by the delete never having been + // attempted: the message is still there and still findable afterwards. + QTRY_VERIFY_WITH_TIMEOUT( + notmuchCount(cfg, QStringLiteral("id:nomove@example.org")) == 1, 15000); + + // A tag write is a round trip, so an immediate read would pass against a + // write still in flight. Given time to arrive, then asserted absent. + QTest::qWait(1500); + QCOMPARE(notmuchCount(cfg, QStringLiteral("id:nomove@example.org and " + "tag:deleted")), 0); + QCOMPARE(notmuchCount(cfg, QStringLiteral("id:nomove@example.org and " + "tag:\"deleted-from:inbox\"")), 0); + + // And the file never left. + QVERIFY(folderHasMessageFile(root + QStringLiteral("/acct/inbox/new"), + QStringLiteral("nomove.example.org")) + || folderHasMessageFile(root + QStringLiteral("/acct/inbox/cur"), + QStringLiteral("nomove.example.org"))); +} + +void TestMainWindow::restoringFromTheTrashViewRefreshesTheList() +{ + // Reported from a hand test: Restore moved the message correctly and the + // row it came from sat in the trash list until the Trash filter was + // clicked again. + // + // The trash view is PATH based, so a restored message no longer matches + // the query the list was built from. That is a state no tag change can + // express: onMessagesMoved() updates tags and the undo stack and never + // removes a row, which is right in an ordinary view (a deleted message's + // card should stay put) and wrong here, where the row is the one thing + // that is now false. + WorkerBackedWindow backed; + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/Trash"), QStringLiteral("refr1@example.org"), + QStringLiteral("Restore me"), QStringLiteral("sender@example.org"), + QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"), + QStringLiteral("Body text."))); + QVERIFY2(backed.build(QStringLiteral("acct"), QStringLiteral("acct"), + QStringLiteral("Trash")), + qPrintable(backed.error())); + + MainWindow window(backed.config()); + auto *model = window.findChild<ThreadListModel *>(); + auto *view = window.findChild<ThreadListView *>(); + auto *queryEdit = + window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + QVERIFY(model && view && queryEdit); + + const QString root = backed.fixture().maildirPath(); + const QString stem = QStringLiteral("refr1.example.org"); + + // The account's own generated trash query, which is what the Trash filter + // puts in the bar. + queryEdit->setText(QStringLiteral("path:\"acct/Trash/**\"")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); + + view->setCurrentIndex(model->index(0, 0, QModelIndex())); + window.findChild<QAction *>(QStringLiteral("restore"))->trigger(); + + // The move really happened, waited on the FILE. Without this the row + // assertion below could pass against a restore that never ran. + QTRY_VERIFY_WITH_TIMEOUT( + folderHasMessageFile(root + QStringLiteral("/acct/inbox/cur"), stem) + || folderHasMessageFile(root + QStringLiteral("/acct/inbox/new"), + stem), + 15000); + + // And the list no longer shows it, without the user touching anything. + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 0, 15000); +} + +void TestMainWindow::theRefreshAfterARestoreLeavesUndoIntact() +{ + // The refresh that fixes the stale trash row runs immediately after the + // undo entry is pushed, so it must not be the thing that destroys it. + // runCurrentQuery() clears the undo stack outright, which would make + // Restore the one mutation in the window with no way back; this asserts + // the non-destructive refresh was used and stayed non-destructive. + WorkerBackedWindow backed; + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/Trash"), QStringLiteral("undoref@example.org"), + QStringLiteral("Restore then undo"), + QStringLiteral("sender@example.org"), + QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"), + QStringLiteral("Body text."))); + QVERIFY2(backed.build(QStringLiteral("acct"), QStringLiteral("acct"), + QStringLiteral("Trash")), + qPrintable(backed.error())); + + MainWindow window(backed.config()); + auto *model = window.findChild<ThreadListModel *>(); + auto *view = window.findChild<ThreadListView *>(); + auto *queryEdit = + window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + QVERIFY(model && view && queryEdit); + + const QString root = backed.fixture().maildirPath(); + const QString stem = QStringLiteral("undoref.example.org"); + + queryEdit->setText(QStringLiteral("path:\"acct/Trash/**\"")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); + + view->setCurrentIndex(model->index(0, 0, QModelIndex())); + window.findChild<QAction *>(QStringLiteral("restore"))->trigger(); + + QTRY_VERIFY_WITH_TIMEOUT( + folderHasMessageFile(root + QStringLiteral("/acct/inbox/cur"), stem) + || folderHasMessageFile(root + QStringLiteral("/acct/inbox/new"), + stem), + 15000); + // The refresh has run by now, which is what the row count proves. + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 0, 15000); + + // And the undo entry is still there afterwards. + auto *undo = window.findChild<QAction *>(QStringLiteral("undo")); + QVERIFY(undo); + QVERIFY2(undo->isEnabled(), + "the refresh after a restore cleared the undo stack"); + + undo->trigger(); + + // Back in the trash, asserted on the FILE: the undo has to move it, not + // merely re-tag it. + QTRY_VERIFY_WITH_TIMEOUT( + folderHasMessageFile(root + QStringLiteral("/acct/Trash/cur"), stem), + 15000); +} + +void TestMainWindow::deletingOutsideTheTrashViewLeavesTheRowInPlace() +{ + // The other half of the trash-view refresh, and the reason it is gated. + // + // A Delete is a move too and reaches the same confirmation slot. Refreshing + // on every move would make the row vanish from under the user in every + // ordinary view, which this project has decided against twice: the card + // deliberately stays put, because one deleted message does not doom the + // conversation and a row disappearing mid-gesture loses the user's place. + // + // Nothing asserted this, so a mutation dropping the isShowingTrash() gate + // passed the whole suite. + WorkerBackedWindow backed; + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/inbox"), QStringLiteral("stay1@example.org"), + QStringLiteral("Stay on screen"), QStringLiteral("sender@example.org"), + QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"), + QStringLiteral("Body text."))); + QVERIFY2(backed.build(QStringLiteral("acct"), QStringLiteral("acct"), + QStringLiteral("Trash")), + qPrintable(backed.error())); + + MainWindow window(backed.config()); + auto *model = window.findChild<ThreadListModel *>(); + auto *view = window.findChild<ThreadListView *>(); + auto *queryEdit = + window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + QVERIFY(model && view && queryEdit); + + const QString root = backed.fixture().maildirPath(); + const QString stem = QStringLiteral("stay1.example.org"); + + // An ORDINARY view that the message STOPS MATCHING once the delete lands. + // Both halves matter and the first draft of this test had only one: a + // `tag:inbox` view looks ordinary but Delete adds `deleted` and the origin + // tag and removes nothing, so the message keeps `inbox` and keeps matching. + // A refresh there is a no-op, and the mutation dropping the + // isShowingTrash() gate passed against it. + // + // A path query on the inbox folder is the honest instrument: the file + // really leaves that folder, so the row survives only because nothing + // refreshed. + queryEdit->setText(QStringLiteral("path:\"acct/inbox/**\"")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); + + view->setCurrentIndex(model->index(0, 0, QModelIndex())); + window.findChild<QAction *>(QStringLiteral("delete"))->trigger(); + + // The delete really happened, waited on the file rather than on the list. + QTRY_VERIFY_WITH_TIMEOUT( + folderHasMessageFile(root + QStringLiteral("/acct/Trash/cur"), stem), + 15000); + + // A refresh is a queued round trip, so an immediate read would pass against + // one still in flight. Given time to arrive, then asserted not to have + // taken the row away. + QTest::qWait(1500); + QCOMPARE(model->rowCount(QModelIndex()), 1); +} + #include "test_mainwindow.moc" diff --git a/tests/test_notmuchworker.cpp b/tests/test_notmuchworker.cpp index 899fd11..bf1c23c 100644 --- a/tests/test_notmuchworker.cpp +++ b/tests/test_notmuchworker.cpp @@ -86,7 +86,20 @@ private slots: void requestFoldersListsEveryMaildirFolder(); void requestFoldersOnUnreadableConfigEmitsError(); + void moveMessagesRelocatesTheFile(); + void moveMessagesReindexesAtTheNewPath(); + void moveMessagesKeepsTheMessagesTags(); + void moveMessagesReportsOnlyWhatMoved(); + private: + /// Adds one read message in `folder` and reindexes, for the move tests. + /// Each of those takes its own message, because a move is destructive and + /// the fixture database is shared by every test in this class. + bool addMovableMessage(const QString &folder, const QString &messageId); + /// The single file backing `messageId`, or an empty string when the + /// database does not know the id. + QString fileOf(const QString &messageId); + /// Tags of one message, read back through a fresh worker query. QStringList tagsOf(const QString &messageId); QVector<MessageRef> messagesOfThread(const QString &threadId, @@ -181,6 +194,34 @@ QString TestNotmuchWorker::threadIdOf(const QString &subject) return QString(); } +bool TestNotmuchWorker::addMovableMessage(const QString &folder, + const QString &messageId) +{ + if (!m_fixture.addMessage(folder, messageId, + QStringLiteral("Movable %1").arg(messageId), + QStringLiteral("Erin <erin@example.org>"), + QStringLiteral("Sun, 7 Jun 2026 10:00:00 +0000"), + QStringLiteral("movable body"), false)) { + return false; + } + return m_fixture.index(); +} + +QString TestNotmuchWorker::fileOf(const QString &messageId) +{ + NotmuchWorker worker(m_fixture.configPath()); + QSignalSpy loaded(&worker, &NotmuchWorker::threadLoaded); + worker.loadThread(QStringLiteral("{id:%1}").arg(messageId), QString(), 1); + if (loaded.isEmpty()) + return {}; + const auto messages = loaded.first().at(0).value<QVector<MessageRef>>(); + for (const MessageRef &m : messages) { + if (m.messageId == messageId) + return m.filePath; + } + return {}; +} + QVector<MessageRef> TestNotmuchWorker::messagesOfThread(const QString &threadId, const QString &matchQuery, bool matchedOnly) @@ -1096,5 +1137,108 @@ void TestNotmuchWorker::requestFoldersOnUnreadableConfigEmitsError() QVERIFY(ready.isEmpty()); } +void TestNotmuchWorker::moveMessagesRelocatesTheFile() +{ + const QString id = QStringLiteral("move1@example.org"); + QVERIFY2(addMovableMessage(QStringLiteral("inbox"), id), + qPrintable(m_fixture.error())); + + const QString before = fileOf(id); + QVERIFY(!before.isEmpty()); + QVERIFY(QFile::exists(before)); + + NotmuchWorker worker(m_fixture.configPath()); + QSignalSpy moved(&worker, &NotmuchWorker::messagesMoved); + QSignalSpy errors(&worker, &NotmuchWorker::errorOccurred); + + worker.moveMessages({ id }, QStringLiteral("trash")); + QVERIFY2(errors.isEmpty(), qPrintable(errors.value(0).value(0).toString())); + + QCOMPARE(moved.size(), 1); + QCOMPARE(moved.first().at(0).toStringList(), QStringList{ id }); + QCOMPARE(moved.first().at(1).toString(), QStringLiteral("trash")); + + // cur/, never new/: a file in new/ is re-announced as fresh mail by every + // reader of the Maildir. + const QString expected = m_fixture.maildirPath() + QStringLiteral("/trash/cur/") + + QFileInfo(before).fileName(); + QVERIFY2(QFile::exists(expected), qPrintable(expected)); + QVERIFY(!QFile::exists(before)); +} + +void TestNotmuchWorker::moveMessagesReindexesAtTheNewPath() +{ + // The half a filesystem check cannot see. A moved file with a stale index + // entry sits correctly on disk and is invisible to every query. + const QString id = QStringLiteral("move2@example.org"); + QVERIFY2(addMovableMessage(QStringLiteral("inbox"), id), + qPrintable(m_fixture.error())); + + NotmuchWorker worker(m_fixture.configPath()); + QSignalSpy errors(&worker, &NotmuchWorker::errorOccurred); + worker.moveMessages({ id }, QStringLiteral("trash")); + QVERIFY2(errors.isEmpty(), qPrintable(errors.value(0).value(0).toString())); + + const QVector<ThreadSummary> inTrash = + runQuery(QStringLiteral("path:\"trash/**\" and id:%1").arg(id)); + QCOMPARE(inTrash.size(), 1); + + const QVector<ThreadSummary> inInbox = + runQuery(QStringLiteral("path:\"inbox/**\" and id:%1").arg(id)); + QCOMPARE(inInbox.size(), 0); +} + +void TestNotmuchWorker::moveMessagesKeepsTheMessagesTags() +{ + // The ordering test. notmuch_database_remove_message() removes the LAST + // filename for a message id by deleting the whole database entry, tags + // included, so the new path must be indexed before the old one is dropped. + // The reverse order leaves the file correctly placed, findable by query, + // and stripped of every tag the user ever put on it. + const QString id = QStringLiteral("move3@example.org"); + QVERIFY2(addMovableMessage(QStringLiteral("inbox"), id), + qPrintable(m_fixture.error())); + + NotmuchWorker tagger(m_fixture.configPath()); + tagger.applyTags(TagChange{ { id }, + { QStringLiteral("keepme") }, + {}, + QStringLiteral("Tag before moving") }); + QVERIFY(tagsOf(id).contains(QStringLiteral("keepme"))); + + NotmuchWorker worker(m_fixture.configPath()); + QSignalSpy errors(&worker, &NotmuchWorker::errorOccurred); + worker.moveMessages({ id }, QStringLiteral("trash")); + QVERIFY2(errors.isEmpty(), qPrintable(errors.value(0).value(0).toString())); + + const QStringList after = tagsOf(id); + QVERIFY2(after.contains(QStringLiteral("keepme")), + qPrintable(QStringLiteral("tags after the move: %1") + .arg(after.join(QLatin1Char(' '))))); +} + +void TestNotmuchWorker::moveMessagesReportsOnlyWhatMoved() +{ + // A stale id must not abort the batch, and must not be reported as moved + // either: a caller that assumed the request succeeded would show a delete + // that never happened. + const QString id = QStringLiteral("move4@example.org"); + QVERIFY2(addMovableMessage(QStringLiteral("inbox"), id), + qPrintable(m_fixture.error())); + + NotmuchWorker worker(m_fixture.configPath()); + QSignalSpy moved(&worker, &NotmuchWorker::messagesMoved); + + worker.moveMessages({ QStringLiteral("nosuchmessage@example.org"), id }, + QStringLiteral("trash")); + + QCOMPARE(moved.size(), 1); + QCOMPARE(moved.first().at(0).toStringList(), QStringList{ id }); + + const QVector<ThreadSummary> inTrash = + runQuery(QStringLiteral("path:\"trash/**\" and id:%1").arg(id)); + QCOMPARE(inTrash.size(), 1); +} + QTEST_MAIN(TestNotmuchWorker) #include "test_notmuchworker.moc" diff --git a/tests/test_tagdialog.cpp b/tests/test_tagdialog.cpp index 5289599..9b12109 100644 --- a/tests/test_tagdialog.cpp +++ b/tests/test_tagdialog.cpp @@ -47,6 +47,7 @@ private slots: void acceptingACandidateKeepsTheOtherTags(); void removeCompletesOnlyTheSelectionsOwnTags(); void removeStillAcceptsATagItDoesNotSuggest(); + void aTagWithASpaceCanStillBeRemoved(); }; void TestTagDialog::validNamesAreAccepted() @@ -166,6 +167,62 @@ void TestTagDialog::multipleTagsSeparateOnComma() QStringLiteral("three") })); } +void TestTagDialog::aTagWithASpaceCanStillBeRemoved() +{ + // validateTagName() rejects a space, and that rule is right: it stops a + // troublesome tag being CREATED. It ran on the removal list too, which is + // not the same question. A tag that already exists is a fact, and refusing + // to remove it because it breaks a naming rule leaves the user with a tag + // they can see and cannot get rid of. + // + // Reached by a real Maildir: a folder named "Inbox/SlackBuilds users" + // produced `deleted-from:Inbox/SlackBuilds users`, and the one dialog that + // could have cleared it refused the only text that names it. + // + // Only the TYPED route was blocked. Unchecking appends to the removal list + // after validation has run, so it worked throughout; that asymmetry is why + // both routes are asserted here rather than just the one that failed. + const QString spaced = + QStringLiteral("deleted-from:Inbox/SlackBuilds users"); + QHash<QString, int> current; + current.insert(spaced, 1); + + // Typed into the remove field, which is what a user does for a tag they + // can see on the message. Before the fix this raised a modal warning and + // returned without accepting, so the dialog simply would not close. + TagDialog typed({ spaced }, current, 1); + const QList<QLineEdit *> edits = typed.findChildren<QLineEdit *>(); + QCOMPARE(edits.size(), 2); + edits.at(1)->setText(spaced); + typed.accept(); + + QCOMPARE(typed.tagsToRemove(), QStringList{ spaced }); + QVERIFY(typed.tagsToAdd().isEmpty()); + + // And unchecking it in the list, the other way to the same place. + TagDialog unchecked({ spaced }, current, 1); + auto *list = unchecked.findChild<QListWidget *>(); + QVERIFY(list); + QCOMPARE(list->count(), 1); + QCOMPARE(list->item(0)->data(Qt::UserRole).toString(), spaced); + list->item(0)->setCheckState(Qt::Unchecked); + unchecked.accept(); + + QCOMPARE(unchecked.tagsToRemove(), QStringList{ spaced }); + + // ADDING one is still refused, which is the rule this must not have + // weakened. accept() returns without setting the lists, so the dialog + // stays open with the text there to fix. + TagDialog added({}, {}, 1); + const QList<QLineEdit *> addEdits = added.findChildren<QLineEdit *>(); + QCOMPARE(addEdits.size(), 2); + addEdits.at(0)->setText(QStringLiteral("two words")); + // Not calling accept(): it would raise a modal warning and block. The + // validator is the thing under test and is asked directly. + QVERIFY(validateTagName(QStringLiteral("two words")) + != TagNameProblem::Ok); +} + void TestTagDialog::uncheckingACurrentTagRemovesIt() { // Every selected thread carries "inbox", so its box starts checked. diff --git a/translations/qtmaildir_it_IT.ts b/translations/qtmaildir_it_IT.ts index 036ce4b..b9515b4 100644 --- a/translations/qtmaildir_it_IT.ts +++ b/translations/qtmaildir_it_IT.ts @@ -44,6 +44,10 @@ <translation>[completion] extra_mimetypes: la voce '%1' non ha un mimetype; verrà ignorata.</translation> </message> <message> + <source>Account '%1' has no trash folder configured; add a 'trash' key to its section. Delete will not work for this account until it does.</source> + <translation>L'account '%1' non ha un cestino configurato; aggiungere una chiave 'trash' alla sua sezione. L'eliminazione non funzionerà per questo account finché non verrà fatto.</translation> + </message> + <message> <source>Startup account '%1' is not a configured account; starting on all accounts.</source> <translation>L'account iniziale '%1' non è un account configurato; si parte da tutti gli account.</translation> </message> @@ -91,6 +95,10 @@ <source>Sent</source> <translation>Inviati</translation> </message> + <message> + <source>Trash</source> + <translation>Cestino</translation> + </message> </context> <context> <name>HtmlBuilder</name> @@ -124,20 +132,6 @@ <source>Unsynced changes</source> <translation>Modifiche non sincronizzate</translation> </message> - <message numerus="yes"> - <source>%n tag change(s) have not been synced, and no sync command is configured. Quit anyway?</source> - <translation> - <numerusform>%n modifica alle etichette non è stata sincronizzata e non è configurato alcun comando di sincronizzazione. Uscire comunque?</numerusform> - <numerusform>%n modifiche alle etichette non sono state sincronizzate e non è configurato alcun comando di sincronizzazione. Uscire comunque?</numerusform> - </translation> - </message> - <message numerus="yes"> - <source>%n tag change(s) have not been synced.</source> - <translation> - <numerusform>%n modifica alle etichette non è stata sincronizzata.</numerusform> - <numerusform>%n modifiche alle etichette non sono state sincronizzate.</numerusform> - </translation> - </message> <message> <source>Sync before quitting?</source> <translation>Sincronizzare prima di uscire?</translation> @@ -255,6 +249,39 @@ <translation>Aggiunge o rimuove l'etichetta deleted</translation> </message> <message> + <source>Changes made here that a sync has not yet carried to the mail store. An external notmuch run can clear them without this count noticing.</source> + <translation>Modifiche fatte qui che nessuna sincronizzazione ha ancora trasferito all'archivio di posta. Un'esecuzione esterna di notmuch può azzerarle senza che questo conteggio se ne accorga.</translation> + </message> + <message numerus="yes"> + <source>%n message(s) could not be deleted: no trash folder is configured for their account.</source> + <translation> + <numerusform>%n messaggio non è stato eliminato: nessuna cartella cestino è configurata per il suo account.</numerusform> + <numerusform>%n messaggi non sono stati eliminati: nessuna cartella cestino è configurata per il loro account.</numerusform> + </translation> + </message> + <message> + <source>Restore</source> + <translation>Ripristina</translation> + </message> + <message numerus="yes"> + <source>%n message(s) had no record of where they came from and were moved to the inbox.</source> + <translation> + <numerusform>%n messaggio non aveva traccia della sua provenienza ed è stato spostato in arrivo.</numerusform> + <numerusform>%n messaggi non avevano traccia della loro provenienza e sono stati spostati in arrivo.</numerusform> + </translation> + </message> + <message numerus="yes"> + <source>%n message(s) could not be restored: they belong to no configured account.</source> + <translation> + <numerusform>%n messaggio non è stato ripristinato: non appartiene ad alcun account configurato.</numerusform> + <numerusform>%n messaggi non sono stati ripristinati: non appartengono ad alcun account configurato.</numerusform> + </translation> + </message> + <message> + <source>Mail tagged deleted but not in a trash folder. Select what should go and press Delete.</source> + <translation>Posta etichettata come eliminata ma non in un cestino. Seleziona cosa deve essere rimosso e premi Elimina.</translation> + </message> + <message> <source>Undelete</source> <translation>Ripristina</translation> </message> @@ -262,6 +289,20 @@ <source>Delete</source> <translation>Elimina</translation> </message> + <message numerus="yes"> + <source>%n change(s) have not been synced, and no sync command is configured. Quit anyway?</source> + <translation> + <numerusform>%n modifica non è stata sincronizzata e non è configurato alcun comando di sincronizzazione. Uscire comunque?</numerusform> + <numerusform>%n modifiche non sono state sincronizzate e non è configurato alcun comando di sincronizzazione. Uscire comunque?</numerusform> + </translation> + </message> + <message numerus="yes"> + <source>%n change(s) have not been synced.</source> + <translation> + <numerusform>%n modifica non è stata sincronizzata.</numerusform> + <numerusform>%n modifiche non sono state sincronizzate.</numerusform> + </translation> + </message> <message> <source>Mark &spam</source> <translation>Segna come &spam</translation> @@ -280,7 +321,7 @@ </message> <message> <source>Add or remove the important tag</source> - <translation>Aggiunge o rimuove l'etichetta importante</translation> + <translation>Aggiunge o rimuove l'etichetta importante</translation> </message> <message> <source>Unmark important</source> @@ -340,21 +381,33 @@ </message> <message> <source>Add or remove the deleted tag on whole threads</source> - <translation>Aggiunge o rimuove l'etichetta eliminato su intere conversazioni</translation> + <translation>Aggiunge o rimuove l'etichetta eliminato su intere conversazioni</translation> </message> <message> <source>Undelete thread</source> <translation>Ripristina conversazione</translation> </message> <message> - <source>Delete thread</source> - <translation>Elimina conversazione</translation> - </message> - <message> <source>Mark thread as &spam</source> <translation>Segna conversazione come &spam</translation> </message> <message> + <source>&Restore from trash</source> + <translation>&Ripristina dal cestino</translation> + </message> + <message> + <source>Move the selected messages out of the trash</source> + <translation>Sposta i messaggi selezionati fuori dal cestino</translation> + </message> + <message> + <source>Find &stranded deleted mail</source> + <translation>&Cerca posta eliminata non spostata</translation> + </message> + <message> + <source>Show mail tagged deleted that is not in a trash folder</source> + <translation>Mostra la posta etichettata come eliminata che non si trova in un cestino</translation> + </message> + <message> <source>Add spam and remove inbox on whole threads</source> <translation>Aggiunge spam e rimuove inbox su intere conversazioni</translation> </message> @@ -364,7 +417,7 @@ </message> <message> <source>Toggle the unread tag on whole threads</source> - <translation>Inverte l'etichetta non letto su intere conversazioni</translation> + <translation>Inverte l'etichetta non letto su intere conversazioni</translation> </message> <message> <source>Mark thread read</source> @@ -836,10 +889,6 @@ </translation> </message> <message> - <source>Tag changes made here that a sync has not yet carried to the mail store. An external notmuch run can clear them without this count noticing.</source> - <translation>Modifiche alle etichette fatte qui che nessuna sincronizzazione ha ancora trasferito all'archivio di posta. Un'esecuzione esterna di notmuch può azzerarle senza che questo conteggio se ne accorga.</translation> - </message> - <message> <source>&Whole thread</source> <translation>&Intera conversazione</translation> </message> |