From 0ca4624195cdd8c78ff614e3912af5b914458497 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Wed, 26 Aug 2026 13:28:51 +0200 Subject: feat: flag what you answered, mark what was forwarded to you Item 68, which turned out to be three things once its premise was measured. The note asked to extend a "passed" subject rule to "Fw:"; there was no subject rule, and the correlation it rested on did not exist. What did exist was a gap nobody had reported. Reply and forward now flag their source. The Maildir R and P flags, which every other client sets and notmuch reads back as "replied" and "passed", had never been written here: measured on the developer's index, all 317 "replied" and all 6 "passed" came from other clients. ComposeWindow emits sourceMessageAnswered after a successful send and MainWindow routes it through sendMessageTagChange, message-scoped and off the undo stack, for the reason auto mark-read is: the flag records that the mail went, and the send cannot be undone. ComposeContext carries sourceMessageId rather than reusing inReplyTo, which is deliberately empty on a forward so the recipient's client does not file it under the thread it left. Keying on it made the "passed" half dead code that compiled and never fired. A resumed draft is excluded: its kind records how the file was opened, not what the user is doing, so flagging on it would set R from a guess. A received forward gets its own mark. Derived from the subject at paint time, storing nothing and reaching no server, because "passed" means "I forwarded this" and setting it from a guess would assert something false on 222 existing messages. subjectIsForwarded() shares forwardSubject()'s prefix table so the two cannot disagree, strips a Re: chain first, and takes extra locale spellings from [general] forward_prefixes, which extends the built-in table rather than replacing it. A mutation survived the first round and corrected a claim in the code: QRegularExpression::escape already makes a punctuation prefix inert, so the word guard is not about pattern validity. It stops a configured "-" matching "-: x". The comment and test say that now. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01LXCZFLXbAii5n5wtovpdhh --- tests/test_threadlistmodel.cpp | 97 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) (limited to 'tests/test_threadlistmodel.cpp') diff --git a/tests/test_threadlistmodel.cpp b/tests/test_threadlistmodel.cpp index 811b3e3..593a777 100644 --- a/tests/test_threadlistmodel.cpp +++ b/tests/test_threadlistmodel.cpp @@ -27,6 +27,9 @@ class TestThreadListModel : public QObject { Q_OBJECT private slots: + void aRowLeavesTheViewWhenItLosesTheViewsTag(); + void rowsLosingTheTagAreRemovedInOneContiguousRun(); + void theTrashViewDrawsNoDoomedFill(); void messageNodeHoldsDisplayFacts(); void rootRowsSurviveTheTreeConversion(); void repliesBecomeChildRowsUnderTheirThread(); @@ -2165,5 +2168,99 @@ void TestThreadListModel::recipientsReplaceTheSenderWhenPresent() QStringLiteral("You")); } +void TestThreadListModel::aRowLeavesTheViewWhenItLosesTheViewsTag() +{ + ThreadListModel model; + ThreadSummary a = makeThread(QStringLiteral("t1"), QStringLiteral("Keep")); + a.firstMessageId = QStringLiteral("m1"); + ThreadSummary b = makeThread(QStringLiteral("t2"), QStringLiteral("Drop")); + b.firstMessageId = QStringLiteral("m2"); + ThreadSummary c = makeThread(QStringLiteral("t3"), QStringLiteral("Keep2")); + c.firstMessageId = QStringLiteral("m3"); + model.appendBatch({ a, b, c }); + QCOMPARE(model.rowCount(), 3); + + // The middle row loses `inbox`, as Delete strips it. Middle deliberately: + // a removal at either end can be right by accident while the index + // arithmetic is wrong. + model.applyMessageTagChange(QStringLiteral("m2"), {}, + { QStringLiteral("inbox") }); + model.removeThreadsWithoutTag(QStringLiteral("inbox")); + + QCOMPARE(model.rowCount(), 2); + QCOMPARE(model.index(0, 0, QModelIndex()) + .data(ThreadListModel::SubjectRole).toString(), + QStringLiteral("Keep")); + QCOMPARE(model.index(1, 0, QModelIndex()) + .data(ThreadListModel::SubjectRole).toString(), + QStringLiteral("Keep2")); +} + +void TestThreadListModel::rowsLosingTheTagAreRemovedInOneContiguousRun() +{ + ThreadListModel model; + QList batch; + for (int i = 1; i <= 5; ++i) { + ThreadSummary t = makeThread(QStringLiteral("t%1").arg(i), + QStringLiteral("S%1").arg(i)); + t.firstMessageId = QStringLiteral("m%1").arg(i); + batch.append(t); + } + model.appendBatch(batch); + + // Three adjacent rows go at once, which is the case a backwards walk in + // runs handles and a naive forward loop gets wrong by renumbering. + for (const QString &id : { QStringLiteral("m2"), QStringLiteral("m3"), + QStringLiteral("m4") }) { + model.applyMessageTagChange(id, {}, { QStringLiteral("inbox") }); + } + model.removeThreadsWithoutTag(QStringLiteral("inbox")); + + QCOMPARE(model.rowCount(), 2); + QCOMPARE(model.index(0, 0, QModelIndex()) + .data(ThreadListModel::SubjectRole).toString(), + QStringLiteral("S1")); + QCOMPARE(model.index(1, 0, QModelIndex()) + .data(ThreadListModel::SubjectRole).toString(), + QStringLiteral("S5")); +} + +void TestThreadListModel::theTrashViewDrawsNoDoomedFill() +{ + ThreadListModel model; + ThreadSummary deleted = makeThread(QStringLiteral("t1"), + QStringLiteral("Thrown away")); + deleted.tags = QStringList{ QStringLiteral("deleted") }; + ThreadSummary spam = makeThread(QStringLiteral("t2"), + QStringLiteral("Junk")); + spam.tags = QStringList{ QStringLiteral("deleted"), QStringLiteral("spam") }; + model.appendBatch({ deleted, spam }); + + const QModelIndex first = model.index(0, 0, QModelIndex()); + const QModelIndex second = model.index(1, 0, QModelIndex()); + + // Outside the trash both are filled, which is the guard proving the + // assertion below can fail. + QVERIFY(first.data(Qt::BackgroundRole).isValid()); + QVERIFY(second.data(Qt::BackgroundRole).isValid()); + + model.setTrashView(true); + + // A plainly deleted row loses the fill AND the white text that only reads + // against it; the strike-out is what still says deleted, and is asserted + // by the font role rather than by colour. + QVERIFY(!first.data(Qt::BackgroundRole).isValid()); + QVERIFY(first.data(Qt::FontRole).value().strikeOut()); + + // A spam row keeps its tint: the trash promises "thrown away", not + // "harmless". + QVERIFY(second.data(Qt::BackgroundRole).isValid()); + + // And the flag does not stick: leaving the trash restores the fill, which + // is the leak the setter's comment warns about. + model.setTrashView(false); + QVERIFY(first.data(Qt::BackgroundRole).isValid()); +} + QTEST_MAIN(TestThreadListModel) #include "test_threadlistmodel.moc" -- cgit v1.2.3