From dc3d86a14e0727b3ea1c68f90d9278354687fadc Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Mon, 24 Aug 2026 19:49:14 +0200 Subject: docs(plans): use a placeholder name in the signature fixtures The plan's test fixtures carried the maintainer's own first name as the signature text, which reaches a committed test file. Task 1 caught and corrected it in the code; this corrects the source so tasks 2, 3, 5 and 6 do not reintroduce it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01KEcn3u19xPqv6ggD15PG4c --- docs/superpowers/plans/2026-08-24-signatures.md | 60 ++++++++++++------------- 1 file changed, 30 insertions(+), 30 deletions(-) (limited to 'docs') diff --git a/docs/superpowers/plans/2026-08-24-signatures.md b/docs/superpowers/plans/2026-08-24-signatures.md index 93026fc..0d13639 100644 --- a/docs/superpowers/plans/2026-08-24-signatures.md +++ b/docs/superpowers/plans/2026-08-24-signatures.md @@ -149,10 +149,10 @@ void TestSignatures::textIsTheFileContent() QTemporaryDir dir; QVERIFY(dir.isValid()); write(dir, { { QStringLiteral("work.md"), - QStringLiteral("Danilo\n**qtmaildir**\n") } }); + QStringLiteral("Jane Doe\n**qtmaildir**\n") } }); QCOMPARE(Signatures::text(dir.path(), QStringLiteral("work")), - QStringLiteral("Danilo\n**qtmaildir**\n")); + QStringLiteral("Jane Doe\n**qtmaildir**\n")); } void TestSignatures::textOfAnUnknownNameIsEmpty() @@ -396,9 +396,9 @@ void TestSignatures::insertingAtTheEndAppendsAfterADelimiter() const QString buffer = QStringLiteral("Hello.\n"); const QString result = Signatures::replace( - buffer, QStringLiteral("Danilo"), {}, Signatures::Position::End); + buffer, QStringLiteral("Jane Doe"), {}, Signatures::Position::End); - QCOMPARE(result, QStringLiteral("Hello.\n\n-- \nDanilo")); + QCOMPARE(result, QStringLiteral("Hello.\n\n-- \nJane Doe")); } void TestSignatures::insertingAboveTheQuotePutsItBeforeTheFirstQuotedLine() @@ -411,7 +411,7 @@ void TestSignatures::insertingAboveTheQuotePutsItBeforeTheFirstQuotedLine() "> second line\n"); const QString result = Signatures::replace( - buffer, QStringLiteral("Danilo"), {}, + buffer, QStringLiteral("Jane Doe"), {}, Signatures::Position::AboveQuote); // Before the QUOTED lines, and the attribution stays with the quote it @@ -420,7 +420,7 @@ void TestSignatures::insertingAboveTheQuotePutsItBeforeTheFirstQuotedLine() "My reply.\n" "\n" "-- \n" - "Danilo\n" + "Jane Doe\n" "\n" "On Mon, someone wrote:\n" "> the original\n" @@ -432,10 +432,10 @@ void TestSignatures::insertingAboveTheQuoteWithNoQuoteIsTheSameAsEnd() const QString buffer = QStringLiteral("A new message.\n"); const QString above = Signatures::replace( - buffer, QStringLiteral("Danilo"), {}, + buffer, QStringLiteral("Jane Doe"), {}, Signatures::Position::AboveQuote); const QString end = Signatures::replace( - buffer, QStringLiteral("Danilo"), {}, Signatures::Position::End); + buffer, QStringLiteral("Jane Doe"), {}, Signatures::Position::End); QCOMPARE(above, end); } @@ -586,27 +586,27 @@ Add the implementations: ```cpp void TestSignatures::switchingReplacesAKnownSignature() { - const QStringList known = { QStringLiteral("Danilo"), - QStringLiteral("Danilo M.\nqtmaildir") }; - const QString buffer = QStringLiteral("Hello.\n\n-- \nDanilo"); + const QStringList known = { QStringLiteral("Jane Doe"), + QStringLiteral("Jane Doe\nqtmaildir") }; + const QString buffer = QStringLiteral("Hello.\n\n-- \nJane Doe"); const QString result = Signatures::replace( - buffer, QStringLiteral("Danilo M.\nqtmaildir"), known, + buffer, QStringLiteral("Jane Doe\nqtmaildir"), known, Signatures::Position::End); QCOMPARE(result, - QStringLiteral("Hello.\n\n-- \nDanilo M.\nqtmaildir")); + QStringLiteral("Hello.\n\n-- \nJane Doe\nqtmaildir")); } void TestSignatures::switchingReplacesAKnownSignatureAboveAQuote() { - const QStringList known = { QStringLiteral("Danilo"), + const QStringList known = { QStringLiteral("Jane Doe"), QStringLiteral("Brief") }; const QString buffer = QStringLiteral( "My reply.\n" "\n" "-- \n" - "Danilo\n" + "Jane Doe\n" "\n" "On Mon, someone wrote:\n" "> the original\n"); @@ -627,8 +627,8 @@ void TestSignatures::switchingReplacesAKnownSignatureAboveAQuote() void TestSignatures::selectingNoneRemovesAKnownSignature() { - const QStringList known = { QStringLiteral("Danilo") }; - const QString buffer = QStringLiteral("Hello.\n\n-- \nDanilo"); + const QStringList known = { QStringLiteral("Jane Doe") }; + const QString buffer = QStringLiteral("Hello.\n\n-- \nJane Doe"); const QString result = Signatures::replace( buffer, QString(), known, Signatures::Position::End); @@ -642,7 +642,7 @@ void TestSignatures::aBlockMatchingNoKnownSignatureIsNotRemoved() // reaches a buffer without the user ever choosing a signature, pasted in // with quoted text from another client. Replacing from there would delete // everything after it silently. - const QStringList known = { QStringLiteral("Danilo") }; + const QStringList known = { QStringLiteral("Jane Doe") }; const QString buffer = QStringLiteral( "Hello.\n" "\n" @@ -650,13 +650,13 @@ void TestSignatures::aBlockMatchingNoKnownSignatureIsNotRemoved() "text the user pasted and wants to keep"); const QString result = Signatures::replace( - buffer, QStringLiteral("Danilo"), known, Signatures::Position::End); + buffer, QStringLiteral("Jane Doe"), known, Signatures::Position::End); // The user's text survives, and the signature is ADDED. A wrong guess // produces a visible duplicate, never a deletion. QVERIFY(result.contains( QStringLiteral("text the user pasted and wants to keep"))); - QVERIFY(result.endsWith(QStringLiteral("-- \nDanilo"))); + QVERIFY(result.endsWith(QStringLiteral("-- \nJane Doe"))); } void TestSignatures::aDelimiterInsideTheQuoteIsNotTheSignature() @@ -664,7 +664,7 @@ void TestSignatures::aDelimiterInsideTheQuoteIsNotTheSignature() // The quoted original carries the sender's own signature, quoted. A tail // rule would find it, and under End it would append after it; the block // must not be treated as this message's signature whichever way it goes. - const QStringList known = { QStringLiteral("Danilo") }; + const QStringList known = { QStringLiteral("Jane Doe") }; const QString buffer = QStringLiteral( "My reply.\n" "\n" @@ -674,10 +674,10 @@ void TestSignatures::aDelimiterInsideTheQuoteIsNotTheSignature() "> Their Name\n"); const QString result = Signatures::replace( - buffer, QStringLiteral("Danilo"), known, Signatures::Position::End); + buffer, QStringLiteral("Jane Doe"), known, Signatures::Position::End); QVERIFY(result.contains(QStringLiteral("> -- \n> Their Name"))); - QVERIFY(result.endsWith(QStringLiteral("-- \nDanilo"))); + QVERIFY(result.endsWith(QStringLiteral("-- \nJane Doe"))); } ``` @@ -1157,7 +1157,7 @@ Config TestComposeWindow::makeConfig( void TestComposeWindow::aNewMessageSeedsTheComposeSignature() { const Config config = makeConfig( - { { QStringLiteral("work.md"), QStringLiteral("Danilo") } }, + { { QStringLiteral("work.md"), QStringLiteral("Jane Doe") } }, QStringLiteral("work")); ComposeContext context; @@ -1170,7 +1170,7 @@ void TestComposeWindow::aNewMessageSeedsTheComposeSignature() auto *body = window.findChild(QStringLiteral("body")); QVERIFY(body); - QVERIFY(body->toPlainText().endsWith(QStringLiteral("-- \nDanilo"))); + QVERIFY(body->toPlainText().endsWith(QStringLiteral("-- \nJane Doe"))); } void TestComposeWindow::anAccountSignatureOverridesTheComposeOne() @@ -1196,7 +1196,7 @@ void TestComposeWindow::anAccountSignatureOverridesTheComposeOne() void TestComposeWindow::aResumedDraftSeedsNoSignature() { const Config config = makeConfig( - { { QStringLiteral("work.md"), QStringLiteral("Danilo") } }, + { { QStringLiteral("work.md"), QStringLiteral("Jane Doe") } }, QStringLiteral("work")); // The saved body already carries whatever signature it was written with. @@ -1204,7 +1204,7 @@ void TestComposeWindow::aResumedDraftSeedsNoSignature() ComposeContext context; context.kind = ComposeContext::Kind::Draft; context.accountKey = QStringLiteral("work"); - context.body = QStringLiteral("Half a thought.\n\n-- \nDanilo"); + context.body = QStringLiteral("Half a thought.\n\n-- \nJane Doe"); context.draftPath = m_dir->path() + QStringLiteral("/draft"); ComposeWindow window(context, config, m_dir->path()); @@ -1213,13 +1213,13 @@ void TestComposeWindow::aResumedDraftSeedsNoSignature() auto *body = window.findChild(QStringLiteral("body")); QVERIFY(body); - QCOMPARE(body->toPlainText().count(QStringLiteral("-- \nDanilo")), 1); + QCOMPARE(body->toPlainText().count(QStringLiteral("-- \nJane Doe")), 1); } void TestComposeWindow::anUnknownSignatureNameSeedsNothing() { const Config config = makeConfig( - { { QStringLiteral("work.md"), QStringLiteral("Danilo") } }, + { { QStringLiteral("work.md"), QStringLiteral("Jane Doe") } }, QStringLiteral("absent")); ComposeContext context; @@ -1239,7 +1239,7 @@ void TestComposeWindow::anUnknownSignatureNameSeedsNothing() void TestComposeWindow::theSwitchListsEveryFileAndNone() { const Config config = makeConfig( - { { QStringLiteral("work.md"), QStringLiteral("Danilo") }, + { { QStringLiteral("work.md"), QStringLiteral("Jane Doe") }, { QStringLiteral("brief.md"), QStringLiteral("Brief") } }, QString()); -- cgit v1.2.3 From 861628a75049002683f777baf837afe434d2f7db Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Mon, 24 Aug 2026 19:51:37 +0200 Subject: docs(backlog): item 136 fails deterministically, and names its own cause Found incidentally while building item 152, by an agent that checked rather than assumed: it ran test_mainwindow at the preceding commit in a throwaway worktree and got the identical failure, so the signatures work is ruled out. Records the assertion text, which is worth more than the flakiness history. After the undo the message file is in neither cur nor new of the account inbox, so the question narrows from "why does this race" to "where did the file go", and the trash folder and the account root are the places to look first. A move landing in the wrong folder is the mail-safety half of the fork this entry already described, and it would present exactly this way. The 70-second duration already recorded fits a QTRY_* waiting for a file that is never going to appear. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01KEcn3u19xPqv6ggD15PG4c --- .../plans/2026-08-03-post-0.1.0-usability.md | 27 +++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) (limited to 'docs') 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 15f8654..4153deb 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 @@ -205,7 +205,7 @@ taking that too literally. | 133 | The composer shows no markdown syntax highlighting | v2 | S | open, 2026-08-20, from the item 123 brainstorm. **Blocked on 123.** A `QSyntaxHighlighter` over the composer's editor, so `**bold**` reads as bold while the buffer stays plain markdown. Standard Qt, no dependency. Deliberately after 123's formatting toolbar: agreeing with the grammar about nesting and about code spans suppressing what is inside them is the expensive part, and the toolbar is what makes the feature usable | | 134 | The busy indicator is built inline and is about to be built twice | maintenance | S | done, 2026-08-20, af902e0. `BusyIndicator` (`src/busyindicator.h`) carries both modes: `MainWindow` uses the indeterminate one, and item 123's send popup takes the determinate half for its undo countdown, switching the same widget over when the command starts. Only the BAR was extracted, not the status label this row paired with it. `m_statusLabel` has 34 uses across `MainWindow` for transient messages, selection counts and sync phases, so it belongs to the window rather than to the indicator, and the send popup owns its own phase text | | 135 | The formatting toolbar's buttons stack rather than toggle | v2 | S | open, 2026-08-21, asked for by the user during item 123 task 8 and reverted the same session. **A spec change, not a defect**: it conflicts with spec:236 ("deliberately no live toggle") and spec:187-190. Both sites need amending FIRST, and the amendment must resolve what replaces bold-then-italic, which is the gesture spec:187's preserved selection exists to serve and which a toggle makes unreachable. That question is the work; the state machine is understood and written up in the section | -| 136 | `undoMovesTheMessageBack` fails when run ALONE, passes in the full suite | defect | ? | open, 2026-08-21, re-measured 2026-08-24 and it is not what the row said. Filed as an intermittent race (1 in 6); it is in fact **deterministic on the selection**: 6 failures in 6 when named on the command line, and, as of 2026-08-24, it fails in the FULL run too: measured at 58f13ad with the day's work stashed out, 274 passed and this one failed. The "passes in the suite" half of this row is therefore no longer true, and the selection-dependence it was named for may not be either. Re-measure before theorising. All three of its 15s `QTRY` timeouts expire, giving 45s against a 25s whole-suite run, so undo never moves the file rather than losing a race. A test that needs its predecessors is the likely shape (the `init()` lock-table fixture of item 61 is one candidate), which makes it a TEST defect until shown otherwise. Not caused by item 149 | +| 136 | `undoMovesTheMessageBack` fails when run ALONE, passes in the full suite | defect | ? | open, 2026-08-21, re-measured 2026-08-24 and it is not what the row said. Filed as an intermittent race (1 in 6); it is in fact **deterministic on the selection**: 6 failures in 6 when named on the command line, and, as of 2026-08-24, it fails in the FULL run too: measured at 58f13ad with the day's work stashed out, 274 passed and this one failed. The "passes in the suite" half of this row is therefore no longer true, and the selection-dependence it was named for may not be either. Re-measure before theorising. All three of its 15s `QTRY` timeouts expire, giving 45s against a 25s whole-suite run, so undo never moves the file rather than losing a race. A test that needs its predecessors is the likely shape (the `init()` lock-table fixture of item 61 is one candidate), which makes it a TEST defect until shown otherwise. Not caused by item 149, and re-confirmed 2026-08-24 as not caused by item 152 either, by running the test at the preceding commit in a throwaway worktree. The assertion that fails names the real question: the restored file is in NEITHER `cur` nor `new` of the account inbox, so establish where it went before theorising about a race | | 137 | A reply to a message that arrived at two accounts can come from the wrong one | defect | S | open, 2026-08-22, found while building item 123 task 12. `ComposeContextBuilder::accountForReply()` takes `messagePaths` PLURAL to disambiguate, and nothing upstream ever gives it more than one path, so the disambiguation is inert | | 138 | No Drafts filter beside Sent and Trash | workflow | S | **done** 2026-08-24, unreleased. Smaller than sized: `Account::draftsQuery()` and `Config::allDraftsQuery()` already existed for the placeholder pane's count, so only the `kQueryGenerators` entry, the two `resolvedQuery` branches, the label and an icon were missing, and `builtinFilters()` derives the row from that set. Follows TRASH rather than Sent: folder-matched like both, but NOT flat, since a draft reply belongs with the conversation it answers. An account with no `drafts` key shows no button at all, per item 103's rule, which the existing row test surfaced by failing until its fixture configured one | | 139 | Forward is reachable only from the Message menu | discoverability | XS | **done** 2026-08-24, unreleased, inside 140/141 as that entry said it would be. Forward is on the message pane's own bar with Compose and Reply | @@ -1305,6 +1305,31 @@ converts a real race into a slower green. If the race turns out to be in the production move rather than the test, this stops being a test-hygiene item and becomes a mail-safety one. +**Measured again 2026-08-24, and the failure is now DETERMINISTIC.** Found +incidentally while building item 152, by an agent that checked rather than +assumed: it built a throwaway worktree at the commit before its own work and +ran `test_mainwindow` there, failing identically. So the failure predates the +signatures work, and the "1 run in 6" framing in this entry's own title is +stale twice over. + +The assertion that fails, quoted exactly: + +``` +'folderHasMessageFile(root + "/acct/inbox/cur", stem) || folderHasMessageFile(root + "/acct/inbox/new", stem)' returned FALSE +``` + +That is worth more than the flakiness history, because it says WHAT is wrong +rather than how often: after the undo, the message file is in neither `cur` +nor `new` of the account's inbox. The file is not where the restore was +supposed to put it, so the question this item has to answer narrows to where +it went instead. Check the trash folder and the account root before +theorising about a race: a move landing in the wrong folder is the mail-safety +half of the fork above, and it would look exactly like this. + +The 70-second duration recorded above fits a `QTRY_*` waiting for a file that +is never going to appear, which is consistent with a wrong destination rather +than a slow one. + --- ## 152. Signatures are not managed at all -- cgit v1.2.3 From 67870d209efafd303b34ec9996c21fed9f165d1a Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Mon, 24 Aug 2026 21:32:46 +0200 Subject: docs(backlog): record item 158, a saved draft is invisible until a sync indexes it Found by hand: autosave writes the draft to the Maildir drafts folder but never indexes it, and the Drafts view is a notmuch path: query, so the draft cannot be reopened until notmuch new runs. Approach reuses the single-file index moveMessages already performs. --- .../plans/2026-08-03-post-0.1.0-usability.md | 41 ++++++++++++++++++++++ 1 file changed, 41 insertions(+) (limited to 'docs') 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 4153deb..c2c82ea 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 @@ -228,6 +228,8 @@ taking that too literally. | 156 | No delivery confirmation | v2 | ? | open, 2026-08-24, from the notes. Distinct from 154: this is a DSN (`Return-Receipt-To`, or the ESMTP NOTIFY parameter), which is the sending server's to honour rather than the reader's client. Whether it can be requested at all depends on the `send_command`, so this may not be this application's to offer | | 157 | A draft on display offers Reply and Forward, not Edit | workflow | XS | **done** 2026-08-24, unreleased, and the half item 153 did not close. `populateMessageBar()` swaps the reply pair for `edit_draft`, refilled from `updateComposeActions()` so it follows the message. **Took three hand-test rounds, each finding a defect the tests could not see.** First version shipped item 150's trap one level up: it keyed on `currentIndex()`, which a query leaves VALID on a row of the discarded result, so the bar kept the draft button after clicking Inbox and the reply pair after clicking Drafts. It answers from `m_currentMessageId`/`m_currentThreadId` now, which every blanking route clears, refilled from `showPlaceholderPane()` — the one site all five of those routes share. That exposed a THIRD defect nobody had reported and which predates the bar: enablement ran only from the two selection handlers, so Reply and Forward stayed **enabled over a blank pane**, invisible while they sat on the main toolbar among always-on actions. The bar is then HIDDEN over an empty pane (`!m_items.isEmpty()` in `MessageView::setBarActions`): the user first chose a greyed-out bar, then reversed it on sight for a better reason, that the subject and details button already vanish and a persisting bar was the only piece of header furniture that did not. **The hiding half broke the showing half**, found by hand again: `setBarActions` is called from `updateComposeActions()`, which runs BEFORE `showThread()` fills `m_items`, so the first message opened after any blanking left the bar hidden and the second showed it, reading `m_items` still holding the first — one selection behind for the life of the view. `updateHeader()` shows it, beside the details button it rides with. The test missed it by asserting before the render landed, measuring the placeholder; it waits on `showingPlaceholder()` now. Several guards and `hide()` calls were written across the three rounds and then measured dead, and removed | +| 158 | A freshly saved draft is invisible until a sync indexes it | defect | S | open, 2026-08-24, found by hand. Autosave writes the draft to the Maildir drafts folder and stops there, while the Drafts view is a notmuch `path:` query, so a draft cannot be reopened after closing the composer until `notmuch new` runs (sync or cron). Refresh only re-queries. See the section | + Sizes are rough: XS under an hour, S a sitting, M a session. --- @@ -1402,3 +1404,42 @@ edited with the user's own editor, and a text editor inside a mail client is not this project's to build. And a resumed draft seeds nothing, because the saved body already carries whatever signature it was written with, and seeding again would put a second one on a message written once. + +## 158. A freshly saved draft is invisible until a sync indexes it + +**Observed (user, 2026-08-24, by hand).** Composing a new message or resuming +a draft, then closing the composer, the draft cannot be found again in the +Drafts view until a sync runs or the cron job fires. "Refresh the search" +does not bring it back. + +**Cause (verified in the code).** Autosave writes the draft to the Maildir +drafts folder and stops there: `saveDraftNow()` → `DraftStore::write()` at +`composewindow.cpp:1021-1022`, with no indexing step. The Drafts view is a +notmuch query, `Config::allDraftsQuery()` → `Account::draftsQuery()` → +`path:".../Drafts/**"` (`config.cpp:139,166`), and "refresh" re-runs that +query against the existing index. Only `assets/mailsync.sh` runs +`notmuch new`. So the file exists on disk and is invisible to the view, which +is why the user cannot reopen it: the draft is safe but unreachable. + +**Approach.** After a successful `saveDraftNow()`, index the one file the same +way `moveMessages` already does: `notmuch_database_index_file()` at +`notmuchworker.cpp:795-798`, opening read-write, indexing the single path, +closing. This is a single-file index, not a `notmuch new`, so it does not scan +the tree or contend with the cron sync the way a full index would. The draft +then appears on the next refresh, or immediately if the worker also emits the +refresh it already emits for a move. + +**Constraints.** + +- **Thread boundary.** `ComposeWindow` lives on the UI thread and does not + talk to `NotmuchWorker`; `MainWindow` owns that connection. The request + needs a route (a `ComposeWindow` signal that `MainWindow` forwards, or a new + worker operation called from the save path). +- **Tags.** `index_file` on a brand-new message may apply notmuch's `new.tags` + (typically `unread;inbox`), which would make the draft ALSO appear in the + inbox. The drafts view is path-based and needs no tag, so the fix must strip + whatever `index_file` assigns. Verify what it assigns before assuming it is + empty. +- **Read-write burst.** This is another short read-write open on the notmuch + database, so it must follow the same close-first ordering `applyTags` uses + (`CLAUDE.md`), never holding the write lock. -- cgit v1.2.3 From f6ceeacad8e1fe15c30db66dcfde8efa9dfb4758 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Mon, 24 Aug 2026 21:37:13 +0200 Subject: docs(backlog): close item 152, signatures --- .../2026-08-03-post-0.1.0-usability-closed.md | 75 ++++++++++++++++++++++ .../plans/2026-08-03-post-0.1.0-usability.md | 73 +-------------------- 2 files changed, 76 insertions(+), 72 deletions(-) (limited to 'docs') 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 7314700..94a02d2 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 @@ -7349,6 +7349,81 @@ importing the other. --- +## 152. Signatures are not managed at all + +**Observed (user, 2026-08-24, from the notes):** listed under "some basic +functionalities not brainstormed which didn't enter the first Send +implementation", as: + +> signatures: +> - not tied to an account, with a switch in the editor bar UI. + +**Specified 2026-08-24.** The design is in +`docs/superpowers/specs/2026-08-24-signatures-design.md`. Read that rather +than this section, which records only what the brainstorm settled and why. + +**The constraint is the shape of the item, and it survived a second key.** +Not tied to an account rules out `[account.*] signature` as the whole answer. +The user then asked for that key anyway, as a convenience, and it does not +reopen the constraint: the account supplies a STARTING value, the editor-bar +switch keeps every signature reachable under any account, and changing From: +stops re-seeding the moment the user touches the switch. Seeding is not +binding. + +**One choice must serve both forms, and that costs nothing.** +`MessageBuilder` already derives `text/plain` from `markdownBody` verbatim and +`text/html` from `MarkdownRenderer::toHtml()` over the same string +(`messagebuilder.cpp:321-324`). A markdown signature in the buffer therefore +gets both, correctly, with no change to `MessageBuilder` and no second code +path. The user's "transparent to the user" requirement is a property the +pipeline already has. + +A two-file variant (`work.md` plus an optional `work.html` overriding the +rendered form) was chosen and then dropped by the user the same session: it +buys designed HTML signatures at the cost of the signature no longer being +visible in the editor, since the two parts diverge and the buffer can hold +only one of them. + +**The switch is stateless, by using the delimiter rather than tracking a +range.** `seedBody()` deliberately refuses to track "my text" and "the quote" +as separate pieces (`composewindow.cpp:640-644`), and a signature switch is a +toggle by definition, so it cannot duck that question the way the quote did. +It answers it without state: the signature is the last `-- ` block not +followed by quoted lines, found by scanning. Nothing to desync from the undo +stack, and it survives editing above it. + +**`signature_position` covers both placements over one scan.** The user's own +habit is `end`, which is the default; `above_quote` exists because other +clients offer it. The scan needed the quote-aware clause for `above_quote` +anyway, so the key is roughly ten lines rather than one, and a naive tail rule +would have eaten the quote under the other placement. + +**A delimiter alone must not authorise a deletion.** The block after `-- ` is +replaced only when its text matches one of the signatures on disk; otherwise +the new one is inserted and nothing is removed. `-- ` can reach the buffer +pasted in with quoted text from another client, and the unguarded scan would +have silently deleted everything after it. The failure is now directional: a +wrong guess adds a visible duplicate rather than losing the user's writing. +Raised by the user against the first draft of this design. + +Two markers were considered for the same problem and refused. A zero-width +character SHIPS in the sent message, fingerprinting the client in outgoing +mail, and must survive the draft round trip through GMime, quoted-printable +and `MimeParser`, which is exactly what normalises such characters away. A +doubled delimiter (`--` plus two spaces) is not the RFC 3676 separator, so no +receiving client would fold or strip the signature, and trailing whitespace is +unreliable through the same pipeline. + +**Two things the design refuses.** No signature editor: the directory is +edited with the user's own editor, and a text editor inside a mail client is +not this project's to build. And a resumed draft seeds nothing, because the +saved body already carries whatever signature it was written with, and +seeding again would put a second one on a message written once. + +**Closed 2026-08-24** (unreleased). See the status table row for the outcome. + +--- + ## 153. A draft cannot be opened for editing, so it is write-only **Observed (user, 2026-08-24).** Found the moment item 138 gave drafts a 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 c2c82ea..b86740e 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 @@ -221,7 +221,7 @@ taking that too literally. | 149 | A reply's cursor lands on the attribution line, not on blank space | defect | XS | **done** 2026-08-24, unreleased, in TWO passes. The first fixed the cursor within each branch (`End` under Above, `Start` under Below) and the user still saw the old layout, because the branches were already right and the DEFAULT was wrong: `above` shipped, and the layout asked for is what `below` produces. Default flipped, and the composer now focuses the body whenever To: is already filled, which a Reply and a Forward always are. Both halves were invisible to the existing `theQuotePositionDecidesWhereTheQuoteLands`, which asserts the quote's position and never the cursor's | | 150 | The receive-only ribbon stays up after the message that raised it is gone | defect | S | **done** 2026-08-24, unreleased. One line in `MessageView::clear()`, beside the blocked-content bar, the stale notice and the attachment bar it already reset by hand. Only `setReceiveOnlyAccount()` hid the ribbon, which every SELECTION change reaches, so a row-to-row move was never the reproducer: it survived the FOUR routes that blank the pane without one (`clear_pane`, `clear_selection`, a new query, a multi-row selection). The first test written for it passed against the defect for exactly that reason | | 151 | The message-pane bars blend into the UI and carry no severity | presentation | S | **done** 2026-08-24, unreleased. Two severities as the user asked: yellow for a warning that only explains (the receive-only ribbon), blue for one offering an action (remote content blocked, stale thread), each with its own light and dark set read off `QPalette::Base` as `HtmlBuilder` does. The blocked row had to become a WIDGET first: it was a bare `QHBoxLayout`, which has nothing to paint a ground on, and its six `hide()` sites then had to move to the wrapper or a painted empty strip would show. Both action bars put the button right of a stretch | -| 152 | Signatures are not managed at all | v2 | S | **specified** 2026-08-24, unbuilt. Design in `specs/2026-08-24-signatures-design.md`; read that, not this row. Sized `?` until the brainstorm; it is an S. One markdown file per signature under `~/.config/qtmaildir/signatures/`, spliced into the composer buffer, so `MessageBuilder` needs NO change at all: it already derives both parts from one string, which is the transparency the user asked for. The constraint holds, and the per-account key the user then asked for does not break it: an account SEEDS the choice, the editor-bar switch keeps every signature reachable | +| 152 | Signatures are not managed at all | v2 | S | **done** 2026-08-24, unreleased. One markdown file per signature under `~/.config/qtmaildir/signatures/`, spliced into the composer buffer by the `Signatures` namespace and chosen from a `QToolButton` switch on the editor bar. `[compose] signature` seeds a new message, `[account.] signature` overrides per account, `[compose] signature_position` picks end or above_quote. `MessageBuilder` is untouched: it already derives both parts from one string, the transparency the user asked for, and the per-account key does not break "not tied to an account", since an account SEEDS the choice while the switch keeps every signature reachable. A hand test caught one guard defect (a trailing newline defeated the replace guard); fixed and regression-tested | | 153 | A draft cannot be opened for editing, so it is write-only | defect | M | **done** 2026-08-24, unreleased. `ComposeContextBuilder::forDraft()` reads a draft back into a context; a new `Kind::Draft` seeds the fields verbatim, takes the body with no quote framing, and carries `draftPath` so the autosave REPLACES the file instead of leaving a second copy. `MimeParser` gained `bcc`, which nothing read before: `MessageBuilder` writes Bcc into the draft deliberately, so a resumed draft that ignored it would silently drop every blind recipient. Reachable by double-click and by an `edit_draft` action, gated on the file being in a configured drafts folder because opening ordinary mail this way would make the first autosave DELETE a received message. Found a live defect on the way, see the section | | 154 | No read confirmation | v2 | ? | open, 2026-08-24, from the notes. `Disposition-Notification-To`, which is a header `MessageBuilder` would add and a request the message pane would have to honour or ignore on the receiving side. Unspecified: whether this is send-side only, and what the reader is asked | | 155 | No urgency switch on an outgoing message | v2 | S | open, 2026-08-24, from the notes: low, regular, high. `X-Priority` and `Importance`, headers `MessageBuilder` adds; regular writes neither. A control in the composer, and the same question item 144 answered for the HTML toggle applies to where it sits | @@ -1334,77 +1334,6 @@ than a slow one. --- -## 152. Signatures are not managed at all - -**Observed (user, 2026-08-24, from the notes):** listed under "some basic -functionalities not brainstormed which didn't enter the first Send -implementation", as: - -> signatures: -> - not tied to an account, with a switch in the editor bar UI. - -**Specified 2026-08-24.** The design is in -`docs/superpowers/specs/2026-08-24-signatures-design.md`. Read that rather -than this section, which records only what the brainstorm settled and why. - -**The constraint is the shape of the item, and it survived a second key.** -Not tied to an account rules out `[account.*] signature` as the whole answer. -The user then asked for that key anyway, as a convenience, and it does not -reopen the constraint: the account supplies a STARTING value, the editor-bar -switch keeps every signature reachable under any account, and changing From: -stops re-seeding the moment the user touches the switch. Seeding is not -binding. - -**One choice must serve both forms, and that costs nothing.** -`MessageBuilder` already derives `text/plain` from `markdownBody` verbatim and -`text/html` from `MarkdownRenderer::toHtml()` over the same string -(`messagebuilder.cpp:321-324`). A markdown signature in the buffer therefore -gets both, correctly, with no change to `MessageBuilder` and no second code -path. The user's "transparent to the user" requirement is a property the -pipeline already has. - -A two-file variant (`work.md` plus an optional `work.html` overriding the -rendered form) was chosen and then dropped by the user the same session: it -buys designed HTML signatures at the cost of the signature no longer being -visible in the editor, since the two parts diverge and the buffer can hold -only one of them. - -**The switch is stateless, by using the delimiter rather than tracking a -range.** `seedBody()` deliberately refuses to track "my text" and "the quote" -as separate pieces (`composewindow.cpp:640-644`), and a signature switch is a -toggle by definition, so it cannot duck that question the way the quote did. -It answers it without state: the signature is the last `-- ` block not -followed by quoted lines, found by scanning. Nothing to desync from the undo -stack, and it survives editing above it. - -**`signature_position` covers both placements over one scan.** The user's own -habit is `end`, which is the default; `above_quote` exists because other -clients offer it. The scan needed the quote-aware clause for `above_quote` -anyway, so the key is roughly ten lines rather than one, and a naive tail rule -would have eaten the quote under the other placement. - -**A delimiter alone must not authorise a deletion.** The block after `-- ` is -replaced only when its text matches one of the signatures on disk; otherwise -the new one is inserted and nothing is removed. `-- ` can reach the buffer -pasted in with quoted text from another client, and the unguarded scan would -have silently deleted everything after it. The failure is now directional: a -wrong guess adds a visible duplicate rather than losing the user's writing. -Raised by the user against the first draft of this design. - -Two markers were considered for the same problem and refused. A zero-width -character SHIPS in the sent message, fingerprinting the client in outgoing -mail, and must survive the draft round trip through GMime, quoted-printable -and `MimeParser`, which is exactly what normalises such characters away. A -doubled delimiter (`--` plus two spaces) is not the RFC 3676 separator, so no -receiving client would fold or strip the signature, and trailing whitespace is -unreliable through the same pipeline. - -**Two things the design refuses.** No signature editor: the directory is -edited with the user's own editor, and a text editor inside a mail client is -not this project's to build. And a resumed draft seeds nothing, because the -saved body already carries whatever signature it was written with, and -seeding again would put a second one on a message written once. - ## 158. A freshly saved draft is invisible until a sync indexes it **Observed (user, 2026-08-24, by hand).** Composing a new message or resuming -- cgit v1.2.3 From 8399a2652584e348ba73f7059d9e178958855897 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Mon, 24 Aug 2026 21:58:38 +0200 Subject: docs(backlog): close item 158, drafts are indexed on save --- .../2026-08-03-post-0.1.0-usability-closed.md | 35 ++++++++++++++++++ .../plans/2026-08-03-post-0.1.0-usability.md | 43 +--------------------- 2 files changed, 36 insertions(+), 42 deletions(-) (limited to 'docs') 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 94a02d2..d6adc98 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 @@ -7513,3 +7513,38 @@ bump either way: an ignored optional field is not a breaking change. **Size: S.** Removing a field, two UI affordances and their tests. **Closed 2026-08-24** (unreleased). See the status table row for the outcome. + +--- + +## 158. A freshly saved draft is invisible until a sync indexes it + +**Observed (user, 2026-08-24, by hand).** Composing a new message or resuming +a draft, then closing the composer, the draft cannot be found again in the +Drafts view until a sync runs or the cron job fires. "Refresh the search" +does not bring it back. + +**Cause (verified in the code).** Autosave writes the draft to the Maildir +drafts folder and stops there: `saveDraftNow()` -> `DraftStore::write()` at +`composewindow.cpp`, with no indexing step. The Drafts view is a notmuch +query, `Config::allDraftsQuery()` -> `Account::draftsQuery()` -> +`path:".../Drafts/**"`, and "refresh" re-runs that query against the existing +index. Only `assets/mailsync.sh` runs `notmuch new`. So the file exists on +disk and is invisible to the view. + +**Outcome.** `saveDraftNow()` emits `draftSaved(path, previousPath)`; +`MainWindow::openComposer()` connects it to a new +`NotmuchWorker::indexDraftFile()`, which indexes the one file the way +`moveMessages()` does and removes the previous revision so a rewrite leaves no +ghost. The send path unlinks a draft it had indexed while composing, so +`draftRemoved(path)` -> `removeIndexedFile()` drops that entry. + +**Measured, and it makes the fix smaller than the item guessed.** +`notmuch_database_index_file` assigns NO tags at all, unlike `notmuch new`, +which would add `draft inbox unread` from `new.tags` and the `:2,D` flag. So +the "strip whatever index_file assigns" concern is moot: a draft indexed this +way cannot leak into a `tag:inbox` or `tag:unread` view, and no stripping is +needed. The drafts view is path-based, so zero tags is exactly enough. + +**Size: S.** One worker slot, one signal, and their tests. + +**Closed 2026-08-24** (unreleased). See the status table row for the outcome. 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 b86740e..2d99704 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 @@ -228,7 +228,7 @@ taking that too literally. | 156 | No delivery confirmation | v2 | ? | open, 2026-08-24, from the notes. Distinct from 154: this is a DSN (`Return-Receipt-To`, or the ESMTP NOTIFY parameter), which is the sending server's to honour rather than the reader's client. Whether it can be requested at all depends on the `send_command`, so this may not be this application's to offer | | 157 | A draft on display offers Reply and Forward, not Edit | workflow | XS | **done** 2026-08-24, unreleased, and the half item 153 did not close. `populateMessageBar()` swaps the reply pair for `edit_draft`, refilled from `updateComposeActions()` so it follows the message. **Took three hand-test rounds, each finding a defect the tests could not see.** First version shipped item 150's trap one level up: it keyed on `currentIndex()`, which a query leaves VALID on a row of the discarded result, so the bar kept the draft button after clicking Inbox and the reply pair after clicking Drafts. It answers from `m_currentMessageId`/`m_currentThreadId` now, which every blanking route clears, refilled from `showPlaceholderPane()` — the one site all five of those routes share. That exposed a THIRD defect nobody had reported and which predates the bar: enablement ran only from the two selection handlers, so Reply and Forward stayed **enabled over a blank pane**, invisible while they sat on the main toolbar among always-on actions. The bar is then HIDDEN over an empty pane (`!m_items.isEmpty()` in `MessageView::setBarActions`): the user first chose a greyed-out bar, then reversed it on sight for a better reason, that the subject and details button already vanish and a persisting bar was the only piece of header furniture that did not. **The hiding half broke the showing half**, found by hand again: `setBarActions` is called from `updateComposeActions()`, which runs BEFORE `showThread()` fills `m_items`, so the first message opened after any blanking left the bar hidden and the second showed it, reading `m_items` still holding the first — one selection behind for the life of the view. `updateHeader()` shows it, beside the details button it rides with. The test missed it by asserting before the render landed, measuring the placeholder; it waits on `showingPlaceholder()` now. Several guards and `hide()` calls were written across the three rounds and then measured dead, and removed | -| 158 | A freshly saved draft is invisible until a sync indexes it | defect | S | open, 2026-08-24, found by hand. Autosave writes the draft to the Maildir drafts folder and stops there, while the Drafts view is a notmuch `path:` query, so a draft cannot be reopened after closing the composer until `notmuch new` runs (sync or cron). Refresh only re-queries. See the section | +| 158 | A freshly saved draft is invisible until a sync indexes it | defect | S | **done** 2026-08-24, unreleased. `saveDraftNow()` emits `draftSaved`, which `MainWindow` connects to a new `NotmuchWorker::indexDraftFile()` that indexes the one file (previous revision removed, so a rewrite leaves no ghost), and `draftRemoved` drops the entry when a sent draft is unlinked. Measured: `index_file` assigns NO tags, so no stripping and no tag:inbox leak. See the section | Sizes are rough: XS under an hour, S a sitting, M a session. @@ -1331,44 +1331,3 @@ half of the fork above, and it would look exactly like this. The 70-second duration recorded above fits a `QTRY_*` waiting for a file that is never going to appear, which is consistent with a wrong destination rather than a slow one. - ---- - -## 158. A freshly saved draft is invisible until a sync indexes it - -**Observed (user, 2026-08-24, by hand).** Composing a new message or resuming -a draft, then closing the composer, the draft cannot be found again in the -Drafts view until a sync runs or the cron job fires. "Refresh the search" -does not bring it back. - -**Cause (verified in the code).** Autosave writes the draft to the Maildir -drafts folder and stops there: `saveDraftNow()` → `DraftStore::write()` at -`composewindow.cpp:1021-1022`, with no indexing step. The Drafts view is a -notmuch query, `Config::allDraftsQuery()` → `Account::draftsQuery()` → -`path:".../Drafts/**"` (`config.cpp:139,166`), and "refresh" re-runs that -query against the existing index. Only `assets/mailsync.sh` runs -`notmuch new`. So the file exists on disk and is invisible to the view, which -is why the user cannot reopen it: the draft is safe but unreachable. - -**Approach.** After a successful `saveDraftNow()`, index the one file the same -way `moveMessages` already does: `notmuch_database_index_file()` at -`notmuchworker.cpp:795-798`, opening read-write, indexing the single path, -closing. This is a single-file index, not a `notmuch new`, so it does not scan -the tree or contend with the cron sync the way a full index would. The draft -then appears on the next refresh, or immediately if the worker also emits the -refresh it already emits for a move. - -**Constraints.** - -- **Thread boundary.** `ComposeWindow` lives on the UI thread and does not - talk to `NotmuchWorker`; `MainWindow` owns that connection. The request - needs a route (a `ComposeWindow` signal that `MainWindow` forwards, or a new - worker operation called from the save path). -- **Tags.** `index_file` on a brand-new message may apply notmuch's `new.tags` - (typically `unread;inbox`), which would make the draft ALSO appear in the - inbox. The drafts view is path-based and needs no tag, so the fix must strip - whatever `index_file` assigns. Verify what it assigns before assuming it is - empty. -- **Read-write burst.** This is another short read-write open on the notmuch - database, so it must follow the same close-first ordering `applyTags` uses - (`CLAUDE.md`), never holding the write lock. -- cgit v1.2.3 From 0a26961f9a7ae6ab98051e182b92e64165758cf1 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Tue, 25 Aug 2026 09:12:29 +0200 Subject: fix(drafts): list drafts as messages, not threads The Drafts filter shipped threaded in item 138, reasoning that a draft reply belongs with the conversation it answers. That reasoning cost the feature: a thread row stands for its first matched message, which for a draft reply is the message being replied to, so the draft itself had no row of its own and double-clicking the conversation opened nothing. Reversed with the user. Drafts now follows Sent; Trash deliberately does not, since a deleted message still belongs to its conversation and nothing there has to be reachable for editing. The view mode was decided in three places that each compared against "sent" and had to agree: builtinFilter(), the reader that reapplies the mode, and the writer that skips storing what the generator implies. generatorIsFlat() is now the one closed set they share, and builtinFilter() sets flat from it rather than inside a branch so the set cannot drift from the labels. Setting only the branch would have looked correct. Its save/load pair survives by accident, because the writer's skip knew only "sent" and so would have stored the key for drafts. The gap is the reader's fallback, for a file carrying no flat key at all: an older build, a migration or a hand edit comes back threaded against a flat button, and the next save persists the disagreement. theDraftsFilterIsThreadedNotFlat is inverted rather than deleted, keeping its history, and now also pins Trash as threaded. The round trip is covered by extending aGeneratedEntryWritesNoRedundantKeys, which already asserted that property for Sent. Mutation-checked: reverting generatorIsFlat() to "sent" alone fails both. Suite 37 of 38; undoMovesTheMessageBack is item 136, pre-existing and on an unrelated path. Closes item 159. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UUQS6n3cmsFrsjCNmwNtf8 --- CHANGELOG.md | 4 +- .../2026-08-03-post-0.1.0-usability-closed.md | 65 ++++++++++++++++++++++ .../plans/2026-08-03-post-0.1.0-usability.md | 2 + src/config.cpp | 37 ++++++++---- tests/test_config.cpp | 36 +++++++++--- 5 files changed, 124 insertions(+), 20 deletions(-) (limited to 'docs') diff --git a/CHANGELOG.md b/CHANGELOG.md index 38d2a58..fee2199 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,7 +31,9 @@ point at which they are stable. - **A Drafts filter** in the query row, beside Sent and Trash. It matches each account's `drafts` folder, so it finds what the composer actually writes rather than trusting a flag. An account that configures no drafts folder - contributes nothing and shows no button. + contributes nothing and shows no button. Like Sent, it lists messages rather + than threads: a draft reply gets a row of its own instead of being folded + into the conversation it answers, where it could not be opened. - `Ctrl+W` closes a composer, the way it closes a window elsewhere. The draft is saved or discarded exactly as it is when the window is closed by any other route. 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 d6adc98..7a52984 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 @@ -7548,3 +7548,68 @@ needed. The drafts view is path-based, so zero tags is exactly enough. **Size: S.** One worker slot, one signal, and their tests. **Closed 2026-08-24** (unreleased). See the status table row for the outcome. + + +## 159. The Drafts view lists threads, so a draft is unreachable by double-click + +**Observed (user, 2026-08-25):** "drafts should be treated like \"Sent\", +listing only actual draft messages and not threads, otherwise I can double +click on a thread message and nothing happens." + +**Cause (verified in code):** `Config::builtinFilter()` in `src/config.cpp` +sets `filter.flat = true` for the `sent` generator only, at line 1011. The +`drafts` branch below it leaves the default `false` with a comment stating the +choice explicitly: "NOT flat, like Trash and unlike Sent: a draft reply +belongs with the conversation it answers." That was item 138's decision and it +is the thing the note contradicts. + +The consequence the user reports follows from it. A thread row stands for +`ThreadSummary::firstMessageId`, which in a Drafts view is the first MATCHED +message of the conversation, and that is not necessarily the draft. Item 153 +gated `edit_draft` on the file living in a configured drafts folder precisely +so that opening ordinary mail this way cannot make the first autosave delete a +received message, so the row is inert rather than harmful. Inert is still +"nothing happens". + +**Built 2026-08-25**, after confirming the reversal with the user. + +**Not one line, and the reason is the part worth keeping.** The obvious fix is +`filter.flat = true` in the `drafts` branch. That ships a defect: the view mode +was decided in THREE places that each hardcoded a comparison against `"sent"`, +and they have to agree. + +- `builtinFilter()` sets it for the button. +- `loadSavedQueries()` reapplies it on read, so a hand-edited or migrated file + cannot produce a threaded Sent view. +- `saveSavedQueries()` SKIPS writing it when the generator already implies it, + because a key carrying no information is one a hand-editor must read past. + +Setting only the first does not break the save/load pair, and it is worth being +exact about why: the writer's skip knew only about `sent`, so it would have +STORED `"flat": true` for drafts, and the reader would have honoured it. That +round trip survives by accident. + +What does NOT survive is a file that carries no `flat` key: one written by an +older build, migrated from elsewhere, or hand-edited, which is the case the +reader's fallback exists for. It comes back THREADED against a flat button, and +the writer then persists that disagreement on the next save. The reader is the +load-bearing site, and it is the one a per-branch fix leaves untouched. + +`generatorIsFlat()` is the fix: one closed set beside `generatorTag()`, called +from all three sites. `builtinFilter()` sets `filter.flat` once from it rather +than inside a branch, so the set cannot drift from the labels below it. + +**Trash deliberately did not follow.** A deleted message still belongs to its +conversation, and nothing in the trash has to be reachable for editing. The +test asserts this, so a future change that flattens every folder filter fails +rather than passing quietly. + +**Testing.** `theDraftsFilterIsThreadedNotFlat` asserted the old behaviour and +is inverted rather than deleted, keeping the history in its comment. The +round-trip is covered by extending `aGeneratedEntryWritesNoRedundantKeys`, +which already asserted exactly that property for `sent`, rather than by a +second test that would have restated it. Mutation-checked: reverting +`generatorIsFlat()` to `sent` alone fails both. + +Suite 37 of 38; the failure is `undoMovesTheMessageBack`, item 136, +pre-existing and on an unrelated path. 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 2d99704..4aeff26 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 @@ -230,6 +230,8 @@ taking that too literally. | 158 | A freshly saved draft is invisible until a sync indexes it | defect | S | **done** 2026-08-24, unreleased. `saveDraftNow()` emits `draftSaved`, which `MainWindow` connects to a new `NotmuchWorker::indexDraftFile()` that indexes the one file (previous revision removed, so a rewrite leaves no ghost), and `draftRemoved` drops the entry when a sent draft is unlinked. Measured: `index_file` assigns NO tags, so no stripping and no tag:inbox leak. See the section | +| 159 | The Drafts view lists threads, so a draft is unreachable by double-click | defect | S | **done** 2026-08-25, unreleased. Reverses item 138's own decision, confirmed with the user. `generatorIsFlat()` in `config.cpp` is now the single closed set of flat generators, replacing three hardcoded comparisons against `"sent"`: the built-in filter, the reader that reapplies the mode, and the writer that skips storing what the generator implies. Those three had to agree and nothing made them; a `drafts` entry saved and reloaded would otherwise have come back THREADED while the button was flat. `builtinFilter()` sets `flat` once from the helper rather than in a branch, so the set cannot drift from the labels | + Sizes are rough: XS under an hour, S a sitting, M a session. --- diff --git a/src/config.cpp b/src/config.cpp index 534ba72..d91259a 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -80,6 +80,18 @@ QString generatorTag(const QString &generator) return QString(); } +/// Whether a generator lists MESSAGES rather than threads. "sent" folds a +/// user's own message back into the conversation it answers, and "drafts" is +/// worse: a thread row stands for its first matched message, which for a draft +/// reply is the message being replied TO, so the draft itself is unreachable. +/// "trash" stays threaded, since a deleted message still belongs to its +/// conversation. Closed set, and the one place the three views are decided. +bool generatorIsFlat(const QString &generator) +{ + return generator == QStringLiteral("sent") + || generator == QStringLiteral("drafts"); +} + } // namespace QString Account::scopedQuery(const QString &query) const @@ -838,14 +850,14 @@ void Config::loadSavedQueries(const QString &configPath, QSettings &settings) query.query = object.value(QStringLiteral("query")).toString(); query.account = object.value(QStringLiteral("account")).toString(); query.generated = object.value(QStringLiteral("generated")).toString(); - // A generator carries its own view mode, so "sent" is flat whether or - // not the file says so. Storing it as a plain field would let a + // A generator carries its own view mode, so a flat one is flat whether + // or not the file says so. Storing it as a plain field would let a // hand-edited or migrated-from-elsewhere row produce a THREADED sent // view, which folds every reply back into the conversation the user // sent one message into. The file may still set it for an ordinary // query. query.flat = object.value(QStringLiteral("flat")).toBool(false) - || query.generated == QStringLiteral("sent"); + || generatorIsFlat(query.generated); if (query.isGenerated() && !kQueryGenerators.contains(query.generated)) { @@ -905,7 +917,7 @@ bool Config::saveSavedQueries() const object.insert(QStringLiteral("account"), query.account); // Skipped when the generator already implies it, which loadSavedQueries // reapplies on the way back in. - if (query.flat && query.generated != QStringLiteral("sent")) + if (query.flat && !generatorIsFlat(query.generated)) object.insert(QStringLiteral("flat"), true); for (auto it = query.unknown.begin(); it != query.unknown.end(); ++it) object.insert(it.key(), it.value()); @@ -987,6 +999,9 @@ SavedQuery Config::builtinFilter(const QString &generator) SavedQuery filter; filter.generated = generator; + // One source for the view mode, shared with the saved-query round trip, so + // a branch below cannot disagree with what loadSavedQueries reapplies. + filter.flat = generatorIsFlat(generator); // Translated, because these are the labels on the buttons. The GENERATOR // name is not: it is stored in queries.json and matched against a closed @@ -1005,16 +1020,18 @@ SavedQuery Config::builtinFilter(const QString &generator) filter.name = tr("Important"); } else if (generator == QStringLiteral("sent")) { filter.name = tr("Sent"); - // Messages rather than threads, and the only filter that sets this. A - // thread would fold the user's sent message back into the conversation - // it belongs to, which is item 63's finding. - filter.flat = true; + // Flat, per generatorIsFlat(): a thread would fold the user's sent + // message back into the conversation it belongs to, item 63's finding. } else if (generator == QStringLiteral("drafts")) { // The LABEL is translated; the generator stays `drafts`, which is what // queries.json stores and what a closed set is matched against. filter.name = tr("Drafts"); - // NOT flat, like Trash and unlike Sent: a draft reply belongs with the - // conversation it answers. + // Flat, per generatorIsFlat(). Item 138 chose threaded, reasoning that + // a draft reply belongs with the conversation it answers; item 159 + // reversed it on what that cost. A thread row stands for its first + // MATCHED message, which for a draft reply is the message being + // replied TO, so the draft itself had no row of its own and + // double-clicking the conversation opened nothing. } else if (generator == QStringLiteral("trash")) { filter.name = tr("Trash"); // NOT flat, unlike Sent. A deleted message still belongs to its diff --git a/tests/test_config.cpp b/tests/test_config.cpp index 17b8e1d..e69a073 100644 --- a/tests/test_config.cpp +++ b/tests/test_config.cpp @@ -122,7 +122,7 @@ private slots: void anAccountWithoutATrashFolderWarns(); void theDraftsFilterComposesPerAccount(); void theDraftsFilterMatchesNothingWithoutAFolder(); - void theDraftsFilterIsThreadedNotFlat(); + void theDraftsFilterIsFlatLikeSent(); void theTrashFilterComposesPerAccount(); void theTrashFilterMatchesNothingWithoutAFolder(); void anAccountWithoutASendCommandIsReceiveOnly(); @@ -1076,17 +1076,24 @@ void TestConfig::theDraftsFilterMatchesNothingWithoutAFolder() Config::matchNothingQuery()); } -void TestConfig::theDraftsFilterIsThreadedNotFlat() +void TestConfig::theDraftsFilterIsFlatLikeSent() { - // Unlike Sent, and deliberately. Sent is flat because a thread would fold - // the user's own message back into the conversation it answers, which is - // item 63's finding. A draft reply belongs with its conversation for the - // same reason a trashed message does, so drafts follow trash here. + // Item 138 shipped this THREADED, reasoning that a draft reply belongs + // with the conversation it answers. Item 159 reversed it on what that + // cost: a thread row stands for its first MATCHED message, which for a + // draft reply is the message being replied TO, so the draft had no row of + // its own and double-clicking the conversation opened nothing. const SavedQuery drafts = Config::builtinFilter(QStringLiteral("drafts")); - QVERIFY2(!drafts.flat, "the drafts filter is flat, like Sent"); + QVERIFY2(drafts.flat, "the drafts filter went back to threaded, so a draft " + "reply has no row of its own (item 159)"); const SavedQuery sent = Config::builtinFilter(QStringLiteral("sent")); QVERIFY2(sent.flat, "Sent stopped being flat, which item 63 requires"); + + // Trash deliberately did NOT follow. A deleted message still belongs to + // its conversation, and nothing has to be reachable for editing there. + const SavedQuery trash = Config::builtinFilter(QStringLiteral("trash")); + QVERIFY2(!trash.flat, "trash became flat; only sent and drafts should be"); } void TestConfig::theTrashFilterComposesPerAccount() @@ -2357,6 +2364,7 @@ void TestConfig::aGeneratedEntryWritesNoRedundantKeys() "version": 1, "queries": [ { "name": "Sent", "generated": "sent", "pinned": true }, + { "name": "Drafts", "generated": "drafts", "pinned": true }, { "name": "Inbox", "query": "tag:inbox", "pinned": true } ] })")); @@ -2381,18 +2389,28 @@ void TestConfig::aGeneratedEntryWritesNoRedundantKeys() QVERIFY2(!sent.contains(QStringLiteral("flat")), "the sent generator implies flat; storing it says nothing"); + // Drafts is the second flat generator (item 159) and must be skipped by + // the same rule, not by a second one that could disagree with it. + const QJsonObject drafts = array.at(1).toObject(); + QCOMPARE(drafts.value(QStringLiteral("generated")).toString(), + QStringLiteral("drafts")); + QVERIFY2(!drafts.contains(QStringLiteral("flat")), + "the drafts generator implies flat; storing it says nothing"); + // The ordinary entry is untouched by any of that. - const QJsonObject inbox = array.at(1).toObject(); + const QJsonObject inbox = array.at(2).toObject(); QCOMPARE(inbox.value(QStringLiteral("query")).toString(), QStringLiteral("tag:inbox")); // And it all still reads back the same. Config reloaded; reloaded.load(path); - QCOMPARE(reloaded.savedQueries().size(), 2); + QCOMPARE(reloaded.savedQueries().size(), 3); QVERIFY(reloaded.savedQueries().at(0).isGenerated()); QVERIFY2(reloaded.savedQueries().at(0).flat, "flat must come back from the generator, not from the file"); + QVERIFY2(reloaded.savedQueries().at(1).flat, + "drafts must come back flat too, from the same rule"); } void TestConfig::anAccountWithoutASendCommandIsReceiveOnly() -- cgit v1.2.3