summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-09-07 10:12:10 +0200
committerDanilo M. <danix@danix.xyz>2026-09-07 10:12:10 +0200
commita00d35452fcef8c8bc723ba7045fcae38fe8f315 (patch)
tree510d0bb2d9265367648f38bbf43186f471379716
parent6326030b7179580b934ba852b0fd98577bfb464a (diff)
downloadqtmaildir-a00d35452fcef8c8bc723ba7045fcae38fe8f315.tar.gz
qtmaildir-a00d35452fcef8c8bc723ba7045fcae38fe8f315.zip
fix: refresh the current query when the index changes
Item 192's second half, answered by the user: indexing is not repainting. The sent copy became findable the moment it was sent and a Sent view already on screen still did not show it, because nothing re-ran the query. The model cannot insert the row optimistically either, since item 170's constraint applies: the query never returned that thread. NotmuchWorker::indexChanged() is emitted at the end of both indexDraftFile() and removeIndexedFile(), the only two entry points that change what a path query would return without any query having run. MainWindow connects it to refreshCurrentQuery(), which covers all three gestures a path view can miss: a sent copy indexed, a draft saved, a draft's entry dropped on send. Wiring only the indexing half would have left a ghost draft row visible in a Drafts view after a send. The signal carries nothing, so it cannot invite an optimistic insert. It is emitted after the database closes, so a refresh reaching notmuch on the next turn of the event loop cannot race the write handle. refreshCurrentQuery() rather than runCurrentQuery(): a send must not clear the selection, the expansions or the undo stack of the window behind the composer. aSentMessageAppearsInASentViewAlreadyOnScreen drives a real send through a worker-backed window, asserts the Sent view is empty first, and asserts the row arrives with no second returnPressed() and no sync. Mutation-checked by disabling the connection. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FWeBw3UqxpBktSc1AkZ6ir
-rw-r--r--CHANGELOG.md4
-rw-r--r--docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md51
-rw-r--r--docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md2
-rw-r--r--src/mainwindow.cpp13
-rw-r--r--src/notmuchworker.cpp8
-rw-r--r--src/notmuchworker.h15
-rw-r--r--tests/test_mainwindow.cpp82
7 files changed, 166 insertions, 9 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5f118b3..e1378fc 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -49,7 +49,9 @@ point at which they are stable.
that view is a search over the index rather than a listing of the folder, so
the message stayed invisible until the next sync, up to ten minutes later.
Saved drafts have been indexed immediately since 0.22.0; the sent copy now
- is too.
+ is too, and the view you are looking at refreshes to show it rather than
+ waiting for a sync. The same refresh reaches the Drafts view, so a saved
+ draft appears and a sent one's entry leaves without a sync either.
- **A draft no longer becomes a new message every time it is saved.** Each
autosave built the draft under a fresh Message-ID, and because mbsync
uploads each revision to the drafts folder before the next save removes the
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 c37f250..6613059 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
@@ -10187,11 +10187,48 @@ was that anything ever CALLS it for a sent copy. Asserting the path exists is
not decoration: announcing a path that was never written is the ghost entry
`removeIndexedFile()` exists to undo. Mutation-checked by removing the emit.
-**Left open deliberately, and it is a real question rather than an
-omission.** Indexing is not repainting. A Sent view already on screen does not
-gain the row from this, because neither this nor the draft path re-runs the
-query, and the model cannot insert a row optimistically for a thread its query
-never returned (item 170's constraint). The message is now findable the moment
-it is sent, which is what the user reported; whether the view should also
-refresh itself is a separate decision.
+**The open question was answered by the user on 2026-09-07: the Sent view
+should refresh and show the message.** Indexing is not repainting, and the
+first build left it there: the message became findable the moment it was sent
+and a Sent view on screen still did not show it, because nothing re-ran the
+query. The model cannot insert the row optimistically either, since item 170's
+constraint applies, the query never returned that thread.
+
+**`NotmuchWorker::indexChanged()`, connected to
+`MainWindow::refreshCurrentQuery()`.** Emitted at the end of BOTH
+`indexDraftFile()` and `removeIndexedFile()`, which are the only two entry
+points that change what a path query would return without any query having
+run. One connection covers all three gestures a path view can miss: a sent
+copy indexed, a draft saved, a draft's entry dropped on send. Wiring only the
+indexing half would have left a ghost draft row visible in a Drafts view after
+a send, which is the same defect one signal over.
+
+Four decisions in it that are not incidental:
+
+- **The signal carries nothing.** A payload would invite an optimistic insert,
+ which item 170 rules out; the refresh re-runs the whole query, so which file
+ moved is not the UI's question.
+- **`refreshCurrentQuery()`, never `runCurrentQuery()`.** A send must not clear
+ the selection, the expansions, the undo stack or the message being read in
+ the window behind the composer. The refresh reconciles instead of replacing,
+ returns early when no query has run, and leaves a view of another folder
+ showing no change at all.
+- **Emitted AFTER the database is closed**, so a refresh reaching the database
+ on the next turn of the event loop cannot race the write handle. notmuch
+ permits one open handle per process, the same ordering `applyTags()` and
+ `moveMessages()` obey.
+- **A send fires it twice**, once for the sent copy and once for the draft
+ removal. Both are queued and serialize, and the generation counter makes the
+ second supersede the first, so the cost is one redundant query and the final
+ state is correct. Not worth a coalescing timer.
+
+**One test, end to end through a real send.**
+`aSentMessageAppearsInASentViewAlreadyOnScreen` builds a worker-backed window
+with a sending account whose `sent` is configured, runs the Sent view's own
+`path:` query, asserts it is EMPTY (a test starting from a non-empty view
+could not tell a refresh from a row that was already there), composes and
+sends, and asserts the row arrives with no further gesture: no second
+`returnPressed()`, no sync. A test that re-ran the query by hand would pass
+against the defect. Mutation-checked by disabling the connection, which fails
+on the row count.
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 da568c9..681200e 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
@@ -265,7 +265,7 @@ taking that too literally.
| 189 | The message bar carries only Reply, Forward and Delete | presentation | S | **done 2026-08-29**, unreleased. Star and Archive joined the bar's ordinary branch, Archive leaving the main toolbar as Delete did. `mark_all_read` deliberately did NOT move, at the user's decision: it is the one action that ignores the selection. Item 140's toolbar test listed `archive` as a list-wide action and had to be corrected, which is the classification this item changed. Section in the closed file. Original entry: Asks for Star (`flag`) and Archive on the bar, and raises Mark all read as a question. Two of the three are selection-scoped and fit the bar's rule as it stands; **`mark_all_read` does not**, since it deliberately ignores the selection and acts on every row in the view, which is the one action in the window that does. Needs a decision from the user on that one and on whether Archive LEAVES the main toolbar the way Delete did |
| 190 | Mark spam is not on the message bar, and its icon was never chosen for one | presentation | XS | open, 2026-09-06, from the notes. The bar's ordinary branch carries Reply, Forward, Star, Archive, Delete after item 189 and `spam` is not among them, though it meets the bar's rule (selection-scoped, undoable). Two halves: put it on the bar, and settle the icon, which the note asks to be "a bug, or a skull, or something that signifies bad/evil" and which is `mail-mark-junk` today, chosen for a menu where the label carries the meaning. **Paired with 187**, which changes what the action DOES (moves the file); ordering is the user's call |
| 191 | The Sent view collapses two messages you sent in one conversation into one row | defect | S | **done 2026-09-06**, unreleased, from a hand test. The Sent and Drafts views are flat, but the worker emitted one summary per THREAD and picked a single matched message to stand for it, oldest-first. A conversation replied to twice showed one row, dated by the thread and opening the OLDER message, and the newer one was reachable nowhere. Also a data-safety defect: `firstMessagePath` named the wrong file, so Delete would have moved it. A second half, found by hand once the rows appeared: the sort notmuch applies is a THREAD sort, so both rows took their thread's position and an older reply drew above a newer one. Flat rows are now sorted as one list. Section in the closed file |
-| 192 | A sent message does not appear in the Sent view until the next sync | defect | XS | **done 2026-09-06**, unreleased. The sent copy was filed correctly and never announced, so the index did not know it and the Sent view, a path query, could not show it. Measured as 65 files against 64 indexed. One signal to the worker, mirroring what drafts have had since item 158. Section in the closed file |
+| 192 | A sent message does not appear in the Sent view until the next sync | defect | XS | **done 2026-09-06**, unreleased. The sent copy was filed correctly and never announced, so the index did not know it and the Sent view, a path query, could not show it. Measured as 65 files against 64 indexed. One signal to the worker, mirroring what drafts have had since item 158. The open question, whether the view should also refresh, was answered yes by the user on 2026-09-07 and built: `indexChanged()` to `refreshCurrentQuery()`. Section in the closed file |
Sizes are rough: XS under an hour, S a sitting, M a session.
diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index abc7875..41dbe9c 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -2756,6 +2756,19 @@ void MainWindow::wireWorker()
connect(m_worker, &NotmuchWorker::threadMessagesResolved,
this, &MainWindow::onThreadMessagesResolved);
+ // Item 192's second half. Indexing a file is not repainting a view: the
+ // sent copy became findable the instant it was indexed, and a Sent view
+ // already on screen still did not show it, because nothing re-ran the
+ // query. Both index entry points report here, so a saved draft, a sent
+ // copy and a removed draft each reach the list that queries their folder.
+ //
+ // refreshCurrentQuery(), NOT runCurrentQuery(): a send must not clear the
+ // selection, the expansions or the undo stack of the window behind the
+ // composer. It returns early when no query has run and reconciles rather
+ // than replacing, so a view of another folder simply sees no change.
+ connect(m_worker, &NotmuchWorker::indexChanged,
+ this, &MainWindow::refreshCurrentQuery);
+
m_workerThread.start();
// Queued behind the thread start, so the completer has real tags as soon
diff --git a/src/notmuchworker.cpp b/src/notmuchworker.cpp
index c8a2485..bc3ae65 100644
--- a/src/notmuchworker.cpp
+++ b/src/notmuchworker.cpp
@@ -1476,6 +1476,12 @@ void NotmuchWorker::indexDraftFile(const QString &path,
notmuch_database_close(db);
notmuch_database_destroy(db);
+
+ // AFTER the close, so a refresh that reaches the database on the next turn
+ // of the event loop cannot race the write handle this function still held.
+ // notmuch permits one open handle per process, which is the same ordering
+ // constraint applyTags() and moveMessages() obey.
+ emit indexChanged();
}
void NotmuchWorker::removeIndexedFile(const QString &path)
@@ -1509,6 +1515,8 @@ void NotmuchWorker::removeIndexedFile(const QString &path)
notmuch_database_close(db);
notmuch_database_destroy(db);
+
+ emit indexChanged();
}
void NotmuchWorker::resolveMessages(const QStringList &messageIds,
diff --git a/src/notmuchworker.h b/src/notmuchworker.h
index a0c8feb..32eab58 100644
--- a/src/notmuchworker.h
+++ b/src/notmuchworker.h
@@ -351,6 +351,21 @@ signals:
void threadDigestLoaded(const ThreadDigest &digest, quint64 generation);
void tagsApplied(const TagChange &change);
+ /// One file entered or left the index outside a query (item 192).
+ ///
+ /// Emitted by indexDraftFile() and removeIndexedFile(), the two entry
+ /// points that change what a path query would return without any query
+ /// having run. A view built on such a query, Sent and Drafts both, is
+ /// stale the moment either fires: indexing a sent copy is what makes the
+ /// message findable, and MainWindow refreshes on this so the view the user
+ /// is looking at gains the row rather than waiting for the next sync.
+ ///
+ /// Carries nothing. The refresh re-runs the whole query, so which file
+ /// moved is not the UI's question; a payload would only invite an
+ /// optimistic insert, which item 170 rules out for a thread the query
+ /// never returned.
+ void indexChanged();
+
/// 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.
diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp
index f97b9bd..1c54ebb 100644
--- a/tests/test_mainwindow.cpp
+++ b/tests/test_mainwindow.cpp
@@ -545,6 +545,7 @@ private slots:
void aNewMessageSeedsTheHtmlToggleFromConfig();
void disablingInputsCoversEveryFieldAndTheToolbar();
void aFiledSentCopyIsAnnouncedForIndexing();
+ void aSentMessageAppearsInASentViewAlreadyOnScreen();
void aFailedSendCanBeRetriedWithoutFilingTheWrongCopy();
void anUnchangedMessageIsNotWrittenAgain();
void closingInsideTheDebounceStillSavesTheDraft();
@@ -15136,6 +15137,87 @@ void TestMainWindow::aFiledSentCopyIsAnnouncedForIndexing()
.arg(announced, sentCur)));
}
+/// Item 192's second half, from the user: indexing is not repainting.
+///
+/// The sent copy became findable the instant it was indexed and the Sent view
+/// on screen still did not show it, because nothing re-ran the query. A
+/// `path:` view is built once and holds whatever the index said then; the
+/// model cannot insert the row optimistically either, since item 170's
+/// constraint applies, the query never returned that thread.
+///
+/// So the worker reports indexChanged() and the window refreshes. Asserted
+/// end to end through a real send rather than by emitting the signal: what is
+/// unproven is that the send path reaches the refresh, and a test that emits
+/// the signal itself proves only that the connection compiles.
+///
+/// refreshCurrentQuery(), which is why the query bar is never touched again
+/// after the first run: a test that re-ran the query by hand would pass
+/// against the defect.
+void TestMainWindow::aSentMessageAppearsInASentViewAlreadyOnScreen()
+{
+ WorkerBackedWindow backed;
+ // One message, in a folder the Sent view does NOT cover. It exists so the
+ // fixture indexes at all (notmuch refuses an empty tree) and so a Sent
+ // view showing zero rows is a real answer rather than an empty database.
+ QVERIFY(backed.fixture().addMessage(
+ QStringLiteral("work/inbox"), QStringLiteral("sv0@example.org"),
+ QStringLiteral("Unrelated"), QStringLiteral("other@example.org"),
+ // Friday, verified with `date -d 2026-08-14 +%A`. Qt::RFC2822Date
+ // validates the weekday against the date.
+ QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"),
+ QStringLiteral("Body text."), false));
+ QVERIFY2(backed.buildWithAccounts(
+ { { QStringLiteral("work"), QStringLiteral("work"),
+ QString(), QStringLiteral("/bin/true"),
+ QStringLiteral("you@example.org"), QString(),
+ QStringLiteral("Sent") } },
+ QStringLiteral("send_delay_ms=0")),
+ qPrintable(backed.error()));
+
+ MainWindow window(backed.config());
+ auto *model = window.findChild<ThreadListModel *>();
+ auto *queryEdit =
+ window.findChild<QLineEdit *>(QStringLiteral("queryEdit"));
+ QVERIFY(model && queryEdit);
+
+ // The Sent view as the application builds it: a path query over the
+ // account's sent folder. Run BEFORE the send, which is the whole point,
+ // and never run again by this test.
+ queryEdit->setText(QStringLiteral("path:\"work/Sent/**\""));
+ queryEdit->returnPressed();
+
+ // The mail root is what the composer needs and it arrives asynchronously,
+ // so this waits on it as well: openComposer() refuses without one and the
+ // send would never happen, which reads exactly like the defect.
+ QTRY_VERIFY_WITH_TIMEOUT(!window.mailRootForTesting().isEmpty(), 15000);
+ QApplication::processEvents();
+
+ // The guard. An empty view here is the state the refresh must change, and
+ // a test that started from a non-empty one could not tell a refresh from
+ // the row that was already there.
+ QCOMPARE(model->rowCount(QModelIndex()), 0);
+
+ auto *compose = window.findChild<QAction *>(QStringLiteral("compose"));
+ QVERIFY(compose);
+ compose->trigger();
+ QTRY_VERIFY_WITH_TIMEOUT(window.openComposerCount() == 1, 15000);
+ ComposeWindow *composer = window.openComposersForTest().value(0);
+ QVERIFY(composer);
+
+ auto *to = composer->findChild<QLineEdit *>(QStringLiteral("to"));
+ auto *body = composer->findChild<QPlainTextEdit *>(QStringLiteral("body"));
+ auto *sendAction =
+ composer->findChild<QAction *>(QStringLiteral("compose_send"));
+ QVERIFY(to && body && sendAction);
+ to->setText(QStringLiteral("someone@example.org"));
+ body->setPlainText(QStringLiteral("Text."));
+ sendAction->trigger();
+
+ // No further gesture: no second returnPressed(), no sync. The row must
+ // arrive because the index changed and the window noticed.
+ QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000);
+}
+
void TestMainWindow::disablingInputsCoversEveryFieldAndTheToolbar()
{
ComposeFixture fixture;