From 997134302332e7641101ce68d274eb3c03e7a124 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Mon, 24 Aug 2026 19:46:14 +0200 Subject: feat(signatures): read a directory of markdown signatures One file per signature under a directory the caller names, the stem being the name shown to the user. A name containing a path separator is refused: it arrives from the config file, and it reaches a path that is read into a message about to be sent. Part of item 152. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01KEcn3u19xPqv6ggD15PG4c --- tests/CMakeLists.txt | 1 + tests/test_signatures.cpp | 104 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 105 insertions(+) create mode 100644 tests/test_signatures.cpp (limited to 'tests') diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 1af49bb..70f6537 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -65,6 +65,7 @@ add_qtmaildir_test(tagdialog) add_qtmaildir_test(tagrules) add_qtmaildir_test(rulequery) add_qtmaildir_test(searchterm) +add_qtmaildir_test(signatures) add_qtmaildir_test(busyindicator) add_qtmaildir_test(tagstrip) add_qtmaildir_test(messagedetailsdialog) diff --git a/tests/test_signatures.cpp b/tests/test_signatures.cpp new file mode 100644 index 0000000..2a75067 --- /dev/null +++ b/tests/test_signatures.cpp @@ -0,0 +1,104 @@ +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include +#include + +#include "signatures.h" + +class TestSignatures : public QObject +{ + Q_OBJECT + +private slots: + void namesAreTheFileStemsSorted(); + void namesIgnoreFilesThatAreNotMarkdown(); + void aMissingDirectoryHasNoNames(); + void textIsTheFileContent(); + void textOfAnUnknownNameIsEmpty(); + +private: + /// Writes \p files as name -> content into a fresh temporary directory. + static void write(const QTemporaryDir &dir, + const QList> &files); +}; + +void TestSignatures::write(const QTemporaryDir &dir, + const QList> &files) +{ + for (const auto &entry : files) { + QFile file(dir.path() + QStringLiteral("/") + entry.first); + QVERIFY(file.open(QIODevice::WriteOnly | QIODevice::Text)); + file.write(entry.second.toUtf8()); + file.close(); + } +} + +void TestSignatures::namesAreTheFileStemsSorted() +{ + QTemporaryDir dir; + QVERIFY(dir.isValid()); + write(dir, { { QStringLiteral("work.md"), QStringLiteral("Work") }, + { QStringLiteral("brief.md"), QStringLiteral("Brief") } }); + + QCOMPARE(Signatures::names(dir.path()), + QStringList({ QStringLiteral("brief"), QStringLiteral("work") })); +} + +void TestSignatures::namesIgnoreFilesThatAreNotMarkdown() +{ + QTemporaryDir dir; + QVERIFY(dir.isValid()); + write(dir, { { QStringLiteral("work.md"), QStringLiteral("Work") }, + { QStringLiteral("notes.txt"), QStringLiteral("Not one") }, + { QStringLiteral("README"), QStringLiteral("Nor this") } }); + + QCOMPARE(Signatures::names(dir.path()), + QStringList({ QStringLiteral("work") })); +} + +void TestSignatures::aMissingDirectoryHasNoNames() +{ + QTemporaryDir dir; + QVERIFY(dir.isValid()); + const QString missing = dir.path() + QStringLiteral("/nothing-here"); + + QVERIFY(Signatures::names(missing).isEmpty()); +} + +void TestSignatures::textIsTheFileContent() +{ + QTemporaryDir dir; + QVERIFY(dir.isValid()); + write(dir, { { QStringLiteral("work.md"), + QStringLiteral("Jane Doe\n**qtmaildir**\n") } }); + + QCOMPARE(Signatures::text(dir.path(), QStringLiteral("work")), + QStringLiteral("Jane Doe\n**qtmaildir**\n")); +} + +void TestSignatures::textOfAnUnknownNameIsEmpty() +{ + QTemporaryDir dir; + QVERIFY(dir.isValid()); + + QVERIFY(Signatures::text(dir.path(), QStringLiteral("absent")).isEmpty()); +} + +QTEST_MAIN(TestSignatures) +#include "test_signatures.moc" -- cgit v1.2.3 From be2534ab60d362b2f685130265f727190ccf867a Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Mon, 24 Aug 2026 19:54:27 +0200 Subject: feat(signatures): splice a signature into a buffer Both placements over one implementation. above_quote inserts before the attribution rather than before the first quoted line: the attribution introduces the quote and belongs with it, and a signature between the two would read as part of the quoted message. A buffer with no quote makes above_quote identical to end, so a new message needs no branch of its own. Part of item 152. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01KEcn3u19xPqv6ggD15PG4c --- src/signatures.cpp | 72 +++++++++++++++++++++++++++++++++++++++++++++-- tests/test_signatures.cpp | 62 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 131 insertions(+), 3 deletions(-) (limited to 'tests') diff --git a/src/signatures.cpp b/src/signatures.cpp index 67bf491..ebf69f5 100644 --- a/src/signatures.cpp +++ b/src/signatures.cpp @@ -55,13 +55,79 @@ QString text(const QString &dir, const QString &name) return QString::fromUtf8(file.readAll()); } +namespace { + +/// The RFC 3676 signature separator: two hyphens, a space, end of line. +/// +/// The trailing space is part of the standard and is what receiving clients +/// match on to fold or strip a signature. It is also why `--` typed by hand +/// does not collide: an editor does not add trailing whitespace on its own. +const QLatin1String kDelimiter("-- "); + +bool isQuoted(const QString &line) +{ + return line.startsWith(QLatin1Char('>')); +} + +/// The index of the first line of the quote, or -1 when the buffer has none. +/// +/// The attribution line ("On Mon, someone wrote:") is deliberately NOT +/// included: it introduces the quote and belongs with it, so a signature +/// inserted above the quote goes above the attribution too. Returning the +/// quoted line itself would strand the signature between the attribution and +/// the text it introduces. +int quoteStart(const QStringList &lines) +{ + for (int i = 0; i < lines.size(); ++i) { + if (!isQuoted(lines.at(i))) + continue; + // Walk back over the attribution and the blank line before it, so the + // signature lands above the whole block rather than inside it. + int start = i; + while (start > 0 && !lines.at(start - 1).trimmed().isEmpty() + && !isQuoted(lines.at(start - 1))) + --start; + return start; + } + return -1; +} + +} // namespace + QString replace(const QString &buffer, const QString &signature, const QStringList &known, Position position) { - Q_UNUSED(signature); Q_UNUSED(known); - Q_UNUSED(position); - return buffer; + + if (signature.isEmpty()) + return buffer; + + const QString block = QStringLiteral("\n") + kDelimiter + + QStringLiteral("\n") + signature; + + QStringList lines = buffer.split(QLatin1Char('\n')); + const int quote = + position == Position::AboveQuote ? quoteStart(lines) : -1; + + // No quote to sit above is not a special case: it is the End placement, + // which is why a New message needs no branch of its own. + if (quote < 0) + return buffer + block; + + QStringList head = lines.mid(0, quote); + const QStringList tail = lines.mid(quote); + // The head ends in however many blank lines separated the reply from the + // attribution. Drop them all and let the block supply exactly one, so the + // spacing is the same whatever the quote was seeded with. + while (!head.isEmpty() && head.last().trimmed().isEmpty()) + head.removeLast(); + + // head.join() has no trailing newline once trimmed, so the terminator for + // its last line is supplied here; `block` then opens with the blank line, + // which is the same shape as the End placement over a buffer ending in a + // newline. + return head.join(QLatin1Char('\n')) + QStringLiteral("\n") + block + + QStringLiteral("\n\n") + tail.join(QLatin1Char('\n')); } } // namespace Signatures diff --git a/tests/test_signatures.cpp b/tests/test_signatures.cpp index 2a75067..30085fb 100644 --- a/tests/test_signatures.cpp +++ b/tests/test_signatures.cpp @@ -31,6 +31,10 @@ private slots: void aMissingDirectoryHasNoNames(); void textIsTheFileContent(); void textOfAnUnknownNameIsEmpty(); + void insertingAtTheEndAppendsAfterADelimiter(); + void insertingAboveTheQuotePutsItBeforeTheFirstQuotedLine(); + void insertingAboveTheQuoteWithNoQuoteIsTheSameAsEnd(); + void insertingNothingLeavesTheBufferAlone(); private: /// Writes \p files as name -> content into a fresh temporary directory. @@ -100,5 +104,63 @@ void TestSignatures::textOfAnUnknownNameIsEmpty() QVERIFY(Signatures::text(dir.path(), QStringLiteral("absent")).isEmpty()); } +void TestSignatures::insertingAtTheEndAppendsAfterADelimiter() +{ + const QString buffer = QStringLiteral("Hello.\n"); + + const QString result = Signatures::replace( + buffer, QStringLiteral("Jane Doe"), {}, Signatures::Position::End); + + QCOMPARE(result, QStringLiteral("Hello.\n\n-- \nJane Doe")); +} + +void TestSignatures::insertingAboveTheQuotePutsItBeforeTheFirstQuotedLine() +{ + const QString buffer = QStringLiteral( + "My reply.\n" + "\n" + "On Mon, someone wrote:\n" + "> the original\n" + "> second line\n"); + + const QString result = Signatures::replace( + buffer, QStringLiteral("Jane Doe"), {}, + Signatures::Position::AboveQuote); + + // Before the QUOTED lines, and the attribution stays with the quote it + // introduces: it is the line the quote hangs from, not part of the reply. + QCOMPARE(result, QStringLiteral( + "My reply.\n" + "\n" + "-- \n" + "Jane Doe\n" + "\n" + "On Mon, someone wrote:\n" + "> the original\n" + "> second line\n")); +} + +void TestSignatures::insertingAboveTheQuoteWithNoQuoteIsTheSameAsEnd() +{ + const QString buffer = QStringLiteral("A new message.\n"); + + const QString above = Signatures::replace( + buffer, QStringLiteral("Jane Doe"), {}, + Signatures::Position::AboveQuote); + const QString end = Signatures::replace( + buffer, QStringLiteral("Jane Doe"), {}, Signatures::Position::End); + + QCOMPARE(above, end); +} + +void TestSignatures::insertingNothingLeavesTheBufferAlone() +{ + const QString buffer = QStringLiteral("Hello.\n"); + + QCOMPARE(Signatures::replace(buffer, QString(), {}, + Signatures::Position::End), + buffer); +} + QTEST_MAIN(TestSignatures) #include "test_signatures.moc" -- cgit v1.2.3 From 86af01996dd702bdc3f837e96f154088e8b45538 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Mon, 24 Aug 2026 20:00:45 +0200 Subject: feat(signatures): replace an existing signature, guarded by a match Finding a "-- " delimiter is not authority to delete what follows it. The block is replaced only when its text matches one of the signatures on disk, and otherwise the new one is inserted with nothing removed, so a wrong guess adds a visible duplicate rather than destroying the user's writing. A quoted delimiter is never the signature either: the quoted original carries the other party's, and it is not this message's to replace. The block's lower boundary is quoteStart(), not the first quoted line. The attribution introduces the quote and belongs with it, so scanning for '>' alone swallowed "On Mon, someone wrote:" into the signature block: it then matched no known signature, and had it matched, removal would have stranded the attribution above the text it introduces. The boundary the insertion uses and the boundary the removal uses have to be the same one. The guard's test was mutation-checked by making the match unconditional, which fails it. Part of item 152. --- src/signatures.cpp | 87 +++++++++++++++++++++++++++++++++++---- tests/test_signatures.cpp | 101 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 181 insertions(+), 7 deletions(-) (limited to 'tests') diff --git a/src/signatures.cpp b/src/signatures.cpp index ebf69f5..eb43048 100644 --- a/src/signatures.cpp +++ b/src/signatures.cpp @@ -76,15 +76,15 @@ bool isQuoted(const QString &line) /// inserted above the quote goes above the attribution too. Returning the /// quoted line itself would strand the signature between the attribution and /// the text it introduces. -int quoteStart(const QStringList &lines) +int quoteStart(const QStringList &lines, int from = 0) { - for (int i = 0; i < lines.size(); ++i) { + for (int i = from; i < lines.size(); ++i) { if (!isQuoted(lines.at(i))) continue; // Walk back over the attribution and the blank line before it, so the // signature lands above the whole block rather than inside it. int start = i; - while (start > 0 && !lines.at(start - 1).trimmed().isEmpty() + while (start > from && !lines.at(start - 1).trimmed().isEmpty() && !isQuoted(lines.at(start - 1))) --start; return start; @@ -92,27 +92,100 @@ int quoteStart(const QStringList &lines) return -1; } +/// Where the block introduced by the delimiter at \p delimiter ends: the start +/// of the quote below it, or the end of the buffer when there is none. +/// +/// This must use quoteStart() rather than scanning for the first quoted line, +/// because the ATTRIBUTION is part of the quote. Scanning for `>` alone puts +/// "On Mon, someone wrote:" inside the signature block, which then matches no +/// known signature and, when it did, left the attribution stranded above the +/// removed text. The two boundaries have to be the same one. +int blockEnd(const QStringList &lines, int delimiter) +{ + const int quote = quoteStart(lines, delimiter + 1); + return quote < 0 ? lines.size() : quote; +} + +/// The line index of the delimiter introducing an existing signature, or -1. +/// +/// Two conditions, and both are load-bearing. The delimiter must not be +/// QUOTED, since the quoted original carries the other party's signature and +/// it is not this message's to replace. And the block after it must MATCH one +/// of \p known: finding a delimiter is not authority to delete what follows +/// it, because "-- " reaches a buffer pasted in with quoted text. +int existingSignature(const QStringList &lines, const QStringList &known) +{ + for (int i = lines.size() - 1; i >= 0; --i) { + if (lines.at(i) != kDelimiter) + continue; + + // The block runs to the end, or to the quote when the signature sits + // above one. + const int end = blockEnd(lines, i); + // A trailing blank line belongs to the separation, not to the text. + int textEnd = end; + while (textEnd > i + 1 && lines.at(textEnd - 1).trimmed().isEmpty()) + --textEnd; + + const QString block = + lines.mid(i + 1, textEnd - (i + 1)).join(QLatin1Char('\n')); + if (known.contains(block)) + return i; + } + return -1; +} + +/// \p lines with the signature at \p delimiter removed, blank separator and +/// all. The caller has already established that the block is a known one. +QStringList withoutSignature(const QStringList &lines, int delimiter) +{ + const int end = blockEnd(lines, delimiter); + + QStringList head = lines.mid(0, delimiter); + while (!head.isEmpty() && head.last().trimmed().isEmpty()) + head.removeLast(); + + QStringList result = head; + if (end < lines.size()) { + // Something follows (the quote): restore the blank line that + // separated it from the signature now being removed. + result.append(QString()); + result.append(lines.mid(end)); + } else { + // The signature ran to the end of the buffer, and the trailing + // newline the head lost with its blank line goes back. + result.append(QString()); + } + return result; +} + } // namespace QString replace(const QString &buffer, const QString &signature, const QStringList &known, Position position) { - Q_UNUSED(known); + QStringList lines = buffer.split(QLatin1Char('\n')); + const int existing = existingSignature(lines, known); + if (existing >= 0) + lines = withoutSignature(lines, existing); + + const QString stripped = lines.join(QLatin1Char('\n')); + + // "None", or nothing to insert: the removal above is the whole operation. if (signature.isEmpty()) - return buffer; + return stripped; const QString block = QStringLiteral("\n") + kDelimiter + QStringLiteral("\n") + signature; - QStringList lines = buffer.split(QLatin1Char('\n')); const int quote = position == Position::AboveQuote ? quoteStart(lines) : -1; // No quote to sit above is not a special case: it is the End placement, // which is why a New message needs no branch of its own. if (quote < 0) - return buffer + block; + return stripped + block; QStringList head = lines.mid(0, quote); const QStringList tail = lines.mid(quote); diff --git a/tests/test_signatures.cpp b/tests/test_signatures.cpp index 30085fb..5f01cf9 100644 --- a/tests/test_signatures.cpp +++ b/tests/test_signatures.cpp @@ -35,6 +35,11 @@ private slots: void insertingAboveTheQuotePutsItBeforeTheFirstQuotedLine(); void insertingAboveTheQuoteWithNoQuoteIsTheSameAsEnd(); void insertingNothingLeavesTheBufferAlone(); + void switchingReplacesAKnownSignature(); + void switchingReplacesAKnownSignatureAboveAQuote(); + void selectingNoneRemovesAKnownSignature(); + void aBlockMatchingNoKnownSignatureIsNotRemoved(); + void aDelimiterInsideTheQuoteIsNotTheSignature(); private: /// Writes \p files as name -> content into a fresh temporary directory. @@ -162,5 +167,101 @@ void TestSignatures::insertingNothingLeavesTheBufferAlone() buffer); } +void TestSignatures::switchingReplacesAKnownSignature() +{ + 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("Jane Doe\nqtmaildir"), known, + Signatures::Position::End); + + QCOMPARE(result, + QStringLiteral("Hello.\n\n-- \nJane Doe\nqtmaildir")); +} + +void TestSignatures::switchingReplacesAKnownSignatureAboveAQuote() +{ + const QStringList known = { QStringLiteral("Jane Doe"), + QStringLiteral("Brief") }; + const QString buffer = QStringLiteral( + "My reply.\n" + "\n" + "-- \n" + "Jane Doe\n" + "\n" + "On Mon, someone wrote:\n" + "> the original\n"); + + const QString result = Signatures::replace( + buffer, QStringLiteral("Brief"), known, + Signatures::Position::AboveQuote); + + QCOMPARE(result, QStringLiteral( + "My reply.\n" + "\n" + "-- \n" + "Brief\n" + "\n" + "On Mon, someone wrote:\n" + "> the original\n")); +} + +void TestSignatures::selectingNoneRemovesAKnownSignature() +{ + 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); + + QCOMPARE(result, QStringLiteral("Hello.\n")); +} + +void TestSignatures::aBlockMatchingNoKnownSignatureIsNotRemoved() +{ + // THE test for the data-loss guard, and it must not be dropped. A "-- " + // 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("Jane Doe") }; + const QString buffer = QStringLiteral( + "Hello.\n" + "\n" + "-- \n" + "text the user pasted and wants to keep"); + + const QString result = Signatures::replace( + 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("-- \nJane Doe"))); +} + +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("Jane Doe") }; + const QString buffer = QStringLiteral( + "My reply.\n" + "\n" + "On Mon, someone wrote:\n" + "> the original\n" + "> -- \n" + "> Their Name\n"); + + const QString result = Signatures::replace( + buffer, QStringLiteral("Jane Doe"), known, Signatures::Position::End); + + QVERIFY(result.contains(QStringLiteral("> -- \n> Their Name"))); + QVERIFY(result.endsWith(QStringLiteral("-- \nJane Doe"))); +} + QTEST_MAIN(TestSignatures) #include "test_signatures.moc" -- cgit v1.2.3 From afeacd7cb99db7fcf4d677dbe8dd52b10e1284e0 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Mon, 24 Aug 2026 20:02:46 +0200 Subject: test(signatures): record what the quoted-delimiter test does not pin The test asserts that a delimiter inside the quoted original is not treated as this message's signature, and it passes whether or not the code checks for that. Two mutations were measured against it and both stayed green: trimming the delimiter comparison so a quoted "> -- " matches, and making the quoted text one of the known signatures so the match guard could not be what refuses the removal. Neither changes the output. blockEnd() stops the block at the quote, so the quoted signature survives either way, and the behaviour is correct under both. The comment says so, so the next reader does not spend the same measurements discovering that the test cannot be sharpened. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01KEcn3u19xPqv6ggD15PG4c --- tests/test_signatures.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'tests') diff --git a/tests/test_signatures.cpp b/tests/test_signatures.cpp index 5f01cf9..31f9eb4 100644 --- a/tests/test_signatures.cpp +++ b/tests/test_signatures.cpp @@ -247,6 +247,16 @@ 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. + // + // This test DOCUMENTS the case rather than pinning it, and that is worth + // knowing before trying to strengthen it. Two mutations were measured + // against it and both stayed green: trimming the delimiter comparison so + // that "> -- " matches, and making the quoted text one of the known + // signatures so the match guard could not be what refuses the removal. + // Neither changes the output, because blockEnd() stops the block at the + // quote, so the quoted signature survives whether or not the delimiter + // inside it is recognised. The behaviour is correct under both, and no + // assertion on the result can separate them. const QStringList known = { QStringLiteral("Jane Doe") }; const QString buffer = QStringLiteral( "My reply.\n" -- cgit v1.2.3 From 160c9121ec39d934330cf626978d02d0fa6d0610 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Mon, 24 Aug 2026 20:15:17 +0200 Subject: feat(config): read the three signature keys [compose] signature and signature_position, and a per-account signature that OVERRIDES the former. The account seeds the choice rather than owning it: the composer's switch keeps every signature reachable whichever account is selected, which is what keeps the note's "not tied to an account" constraint intact. The fallback is deliberately NOT resolved here. An account with no key of its own carries an empty string, so the composer can tell "says nothing" from "says none" and fall through itself. signature_position follows quote_position's shape exactly, reporting a present-but-malformed value rather than accepting it silently. Part of item 152. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01KEcn3u19xPqv6ggD15PG4c --- src/config.cpp | 36 +++++++++++++++++++++++++ src/config.h | 23 ++++++++++++++++ tests/test_config.cpp | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 132 insertions(+) (limited to 'tests') diff --git a/src/config.cpp b/src/config.cpp index 23c7364..534ba72 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -459,6 +459,17 @@ void Config::load(const QString &path) account.sent = settings.value(QStringLiteral("sent")).toString().trimmed(); + // Optional, and a STARTING value rather than a binding: the composer's + // switch keeps every signature reachable whichever account is + // selected. Left empty when absent, so the composer can tell "this + // account says nothing" from "this account says none" and fall through + // to [compose] signature itself; resolving that here would collapse + // the two. Trimmed for the same reason as sent, above: a trailing + // space would be carried into a filename lookup and match nothing, + // which is invisible in a config file. + account.signature = + settings.value(QStringLiteral("signature")).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. @@ -547,6 +558,31 @@ void Config::load(const QString &path) m_compose.sendHtml = settings.value(QStringLiteral("send_html"), true).toBool(); + // Trimmed for the same reason the account key is: it reaches a filename + // lookup, where a trailing space matches nothing invisibly. + m_compose.signature = + settings.value(QStringLiteral("signature")).toString().trimmed(); + + // The same shape as quote_position directly above: an absent key is + // silent and the struct default holds, but a PRESENT and malformed value + // is reported rather than silently accepted. value(key, default) alone + // would read "signature_position = abov" as above_quote. + const QString signaturePosition = + settings.value(QStringLiteral("signature_position"), + QStringLiteral("end")) + .toString().trimmed(); + if (signaturePosition.compare(QStringLiteral("above_quote"), + Qt::CaseInsensitive) == 0) { + m_compose.signaturePosition = Signatures::Position::AboveQuote; + } else if (signaturePosition.compare(QStringLiteral("end"), + Qt::CaseInsensitive) == 0) { + m_compose.signaturePosition = Signatures::Position::End; + } else { + addProblem(tr("[compose] signature_position '%1' is not recognised; " + "expected end or above_quote. Using end.") + .arg(signaturePosition)); + } + // Three numerics, all following the shape already established at // message_zoom, toolbar_icon_size, mark_read_delay_ms and // auto_sync_delay_ms elsewhere in this function: a QVariant, a checked diff --git a/src/config.h b/src/config.h index 4dcfbf1..02b4038 100644 --- a/src/config.h +++ b/src/config.h @@ -26,6 +26,7 @@ #include #include "completionentry.h" +#include "signatures.h" class QSettings; @@ -58,6 +59,18 @@ struct Account /// one for the account that has none. QString sent; + /// The signature seeded when composing from this account, by name. + /// + /// Optional, and it does not tie a signature to the account: the switch on + /// the composer's editor bar keeps every signature reachable whichever + /// account is selected. This is a STARTING value only, which is why the + /// user's "not tied to an account" constraint survives it (item 152). + /// + /// The fallback to [compose] signature is NOT resolved here. An account + /// with no key of its own carries an empty string and the composer falls + /// through, so the two values stay distinguishable. + QString signature; + /// The account's trash folder, relative to maildir. /// /// MANDATORY, unlike `sent` and `drafts`. Delete moves a file into this @@ -228,6 +241,16 @@ struct ComposeSettings /// accounts. Falls through when it names an account that cannot send. QString defaultAccount; + /// The signature seeded when the account carries none, by name. Empty + /// means no signature is seeded at all. + QString signature; + + /// Where a newly inserted signature goes. End by default, which is the + /// user's own habit; above_quote exists because other clients offer the + /// choice, and the splice's quote-aware scan is needed for the guard + /// either way. + Signatures::Position signaturePosition = Signatures::Position::End; + qint64 attachmentWarnBytes = 26214400; }; diff --git a/tests/test_config.cpp b/tests/test_config.cpp index a5dce9a..17b8e1d 100644 --- a/tests/test_config.cpp +++ b/tests/test_config.cpp @@ -25,6 +25,7 @@ #include #include "config.h" #include "mailsync.h" +#include "signatures.h" class TestConfig : public QObject { @@ -134,6 +135,9 @@ private slots: void garbageAttachmentWarnBytesIsRejectedNotZero(); void zeroOrNegativeAutosaveIntervalIsClamped(); void unrecognisedQuotePositionWarnsAndFallsBackToBelow(); + void theSignatureKeysAreRead(); + void anAccountSignatureOverridesTheComposeDefault(); + void aMalformedSignaturePositionIsReportedAndFallsBack(); }; static QString writeIni(const QTemporaryDir &dir, const QString &body) @@ -2577,5 +2581,74 @@ void TestConfig::unrecognisedQuotePositionWarnsAndFallsBackToBelow() "an unrecognised quote_position was accepted silently"); } +void TestConfig::theSignatureKeysAreRead() +{ + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral( + "[compose]\n" + "signature=work\n" + "signature_position=above_quote\n"))); + + QCOMPARE(config.compose().signature, QStringLiteral("work")); + QVERIFY2(config.compose().signaturePosition + == Signatures::Position::AboveQuote, + "signature_position=above_quote was not read"); +} + +void TestConfig::anAccountSignatureOverridesTheComposeDefault() +{ + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral( + "[compose]\n" + "signature=work\n" + "\n" + "[account.personal]\n" + "name=Test User\n" + "address=user@example.org\n" + "maildir=personal-mail\n" + "trash=Trash\n" + "signature=brief\n" + "\n" + "[account.other]\n" + "name=Test User\n" + "address=other@example.org\n" + "maildir=other-mail\n" + "trash=Trash\n"))); + + // The account SEEDS the choice; it does not own the signature. The key is + // a starting value and the switch keeps every signature reachable. + QCOMPARE(config.account(QStringLiteral("personal")).signature, + QStringLiteral("brief")); + // An account with no key of its own carries none, and the caller falls + // through to the [compose] default rather than this being resolved here. + QVERIFY2(config.account(QStringLiteral("other")).signature.isEmpty(), + "an account with no signature key must not inherit the " + "[compose] one: the composer resolves the fallback, not Config"); + QCOMPARE(config.compose().signature, QStringLiteral("work")); +} + +void TestConfig::aMalformedSignaturePositionIsReportedAndFallsBack() +{ + // Present and malformed is REPORTED, matching quote_position. A silent + // value(key, default) would accept "abov" as above_quote. + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral( + "[compose]\n" + "signature_position=abov\n"))); + + QVERIFY2(config.compose().signaturePosition == Signatures::Position::End, + "an unrecognised signature_position must still fall back to End"); + bool reported = false; + for (const QString &problem : config.problems()) { + if (problem.contains(QStringLiteral("signature_position"))) + reported = true; + } + QVERIFY2(reported, + "an unrecognised signature_position was accepted silently"); +} + QTEST_MAIN(TestConfig) #include "test_config.moc" -- cgit v1.2.3 From 882bb1b36fd777ec5fd5f331f2d589d48b5af5c1 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Mon, 24 Aug 2026 20:52:25 +0200 Subject: feat(compose): a signature switch on the editor bar A QToolButton with a checkable menu at the right end of the editor bar, where item 142 put the controls of the editor. Not registered in KeyMap: parented to the composer like the formatting actions, so its scope is this window. The signature is applied through a QTextCursor rather than setPlainText(), which destroys the undo stack, and the seeded one is cleared from that stack for the reason the seeded quote already is: one Ctrl+Z must not wipe content the user never typed. A resumed draft seeds nothing. Its body already carries the signature it was written with, and seeding again would put a second one on a message written once. Part of item 152. --- src/composewindow.cpp | 136 +++++++++++++++++++++++++ src/composewindow.h | 33 ++++++ tests/CMakeLists.txt | 1 + tests/test_composewindow.cpp | 220 ++++++++++++++++++++++++++++++++++++++++ translations/qtmaildir_it_IT.ts | 16 +++ 5 files changed, 406 insertions(+) create mode 100644 tests/test_composewindow.cpp (limited to 'tests') diff --git a/src/composewindow.cpp b/src/composewindow.cpp index a64736f..8236d79 100644 --- a/src/composewindow.cpp +++ b/src/composewindow.cpp @@ -25,6 +25,7 @@ #include "mimeparser.h" #include "messagesender.h" #include "senddialog.h" +#include "signatures.h" #include #include @@ -40,9 +41,11 @@ #include #include #include +#include #include #include #include +#include #include #include #include @@ -128,6 +131,7 @@ ComposeWindow::ComposeWindow(const ComposeContext &context, buildFormatToolbar(); seedFields(); seedBody(); + seedSignature(); // AFTER buildUi(), which creates m_banner, and BEFORE // refreshAttachmentList(), which renders m_attachments: extraction appends @@ -536,6 +540,29 @@ void ComposeWindow::buildFormatToolbar() m_sendHtml->setIcon(htmlIcon); m_formatToolbar->addWidget(m_sendHtml); + // The signature switch rides at the right end with Attach and the HTML + // toggle: item 142 put the controls OF THE EDITOR on this side, as against + // the formatting buttons on the left, and choosing a signature is one of + // those. + // + // A QToolButton with a menu rather than a QComboBox, matching the bar's + // other controls; a combo would read as a different class of thing. Not + // registered in KeyMap: it is parented to this window, exactly as the + // formatting actions are, so its scope is the composer and item 132's + // reachability rule does not apply. + m_signatureSwitch = new QToolButton(m_formatToolbar); + m_signatureSwitch->setObjectName(QStringLiteral("signatureSwitch")); + m_signatureSwitch->setText(tr("Signature")); + m_signatureSwitch->setToolTip( + tr("Chooses the signature added to this message.")); + m_signatureSwitch->setToolButtonStyle(Qt::ToolButtonTextBesideIcon); + m_signatureSwitch->setPopupMode(QToolButton::InstantPopup); + const QIcon signatureIcon = QIcon::fromTheme(QStringLiteral("insert-text")); + if (!signatureIcon.isNull()) + m_signatureSwitch->setIcon(signatureIcon); + m_signatureSwitch->setMenu(new QMenu(m_signatureSwitch)); + m_formatToolbar->addWidget(m_signatureSwitch); + // Send is NOT on this row: it is the terminal action, and it lives on the // button beside the headers. The QAction survives because it carries the // shortcut and is what the button triggers. @@ -664,6 +691,114 @@ void ComposeWindow::seedBody() m_body->document()->clearUndoRedoStacks(); } +void ComposeWindow::setSignatureDir(const QString &dir) +{ + m_signatureDir = dir; +} + +QStringList ComposeWindow::knownSignatures() const +{ + QStringList known; + const QStringList names = Signatures::names(m_signatureDir); + known.reserve(names.size()); + for (const QString &name : names) + known.append(Signatures::text(m_signatureDir, name)); + return known; +} + +QString ComposeWindow::seededSignatureName() const +{ + // The account SEEDS, it does not bind: this is a starting value, and the + // switch keeps every signature reachable whichever account is selected. + const Account account = m_config.account(m_context.accountKey); + if (!account.signature.isEmpty()) + return account.signature; + return m_config.compose().signature; +} + +void ComposeWindow::applySignature(const QString &name) +{ + const QString text = + name.isEmpty() ? QString() : Signatures::text(m_signatureDir, name); + + // A QTextCursor replacement rather than setPlainText(), for the reason + // recorded at applyEdit(): setPlainText() destroys the document's undo + // stack, so a switch would make everything typed before it unrecoverable. + const QString replaced = Signatures::replace( + m_body->toPlainText(), text, knownSignatures(), + m_config.compose().signaturePosition); + + QTextCursor cursor(m_body->document()); + cursor.select(QTextCursor::Document); + cursor.insertText(replaced); + + m_signatureName = name; + + for (QAction *action : m_signatureSwitch->menu()->actions()) + action->setChecked(action->data().toString() == name); +} + +void ComposeWindow::seedSignature() +{ + if (m_signatureDir.isEmpty()) { + const QString base = + QStandardPaths::writableLocation(QStandardPaths::ConfigLocation); + m_signatureDir = base + QStringLiteral("/qtmaildir/signatures"); + } + + QMenu *menu = m_signatureSwitch->menu(); + menu->clear(); + + auto *none = menu->addAction(tr("None")); + none->setCheckable(true); + none->setData(QString()); + connect(none, &QAction::triggered, this, [this]() { + m_signatureChosen = true; + applySignature(QString()); + markDirty(); + }); + + const QStringList names = Signatures::names(m_signatureDir); + for (const QString &name : names) { + auto *action = menu->addAction(name); + action->setCheckable(true); + action->setData(name); + connect(action, &QAction::triggered, this, [this, name]() { + m_signatureChosen = true; + applySignature(name); + markDirty(); + }); + } + + // A resumed draft is the message ITSELF and already carries whatever + // signature it was saved with, exactly as seedBody() takes its body + // verbatim. Seeding again would append a second one. + if (m_context.kind == ComposeContext::Kind::Draft) { + none->setChecked(true); + return; + } + + const QString seeded = seededSignatureName(); + if (seeded.isEmpty()) { + none->setChecked(true); + return; + } + if (!names.contains(seeded)) { + // Reported by Config as a problem; the composer still opens, with no + // signature, and the switch still works. + none->setChecked(true); + return; + } + + applySignature(seeded); + + // The seeded signature is not an edit the user made, so it must not + // survive as an undo step: one Ctrl+Z on a fresh composer would otherwise + // wipe content they never typed. Same reason seedBody() clears after the + // quote. + m_body->document()->clearUndoRedoStacks(); +} + void ComposeWindow::refreshAttachmentList() { m_attachmentList->clear(); @@ -897,6 +1032,7 @@ void ComposeWindow::setInputsEnabled(bool enabled) m_from->setEnabled(enabled); m_body->setReadOnly(!enabled); m_sendHtml->setEnabled(enabled); + m_signatureSwitch->setEnabled(enabled); m_attachmentList->setEnabled(enabled); m_formatToolbar->setEnabled(enabled); diff --git a/src/composewindow.h b/src/composewindow.h index c44fcda..918d4e3 100644 --- a/src/composewindow.h +++ b/src/composewindow.h @@ -96,6 +96,21 @@ public: /// and quitting therefore loses that text. bool lastSaveFailed() const { return m_saveFailed; } + /// Where the signature files live. Defaults to + /// /qtmaildir/signatures; a test points it at its own directory. + /// + /// A setter rather than a config key: nothing yet suggests the user wants + /// a second location, and the tests need to not read the real one. + void setSignatureDir(const QString &dir); + + /// Seeds the signature from config and fills the switch. + /// + /// Public and called by the constructor rather than private, so a test can + /// drive it after pointing setSignatureDir() somewhere safe. A resumed + /// draft seeds nothing: its body already carries the signature it was + /// written with. + void seedSignature(); + /// Writes the current buffer to the drafts folder now. Returns false and /// leaves the banner up on failure. /// @@ -176,6 +191,16 @@ private: /// a silently wrong send is not among the outcomes. void extractForwardedAttachments(); void seedBody(); + + /// Applies \p name to the buffer, replacing whatever is there. + void applySignature(const QString &name); + + /// The text of every signature on disk, for replace()'s guard. + QStringList knownSignatures() const; + + /// The signature name this account seeds, falling through to [compose]. + QString seededSignatureName() const; + void refreshAttachmentList(); void setInputsEnabled(bool enabled); void showSendFailure(const QString &stderrText); @@ -227,6 +252,14 @@ private: QComboBox *m_from = nullptr; QPlainTextEdit *m_body = nullptr; QToolButton *m_sendHtml = nullptr; + QToolButton *m_signatureSwitch = nullptr; + QString m_signatureDir; + QString m_signatureName; ///< The selected signature, empty for None. + + /// True once the user has used the switch. From then on a From: change + /// stops re-seeding, so a deliberate choice is never overwritten. Matches + /// how send_html seeds from context and is then left alone. + bool m_signatureChosen = false; QLabel *m_banner = nullptr; QListWidget *m_attachmentList = nullptr; QWidget *m_sendLogPane = nullptr; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 70f6537..5938aeb 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -75,6 +75,7 @@ add_qtmaildir_test(maildirname) add_qtmaildir_test(draftstore) add_qtmaildir_test(messagesender) add_qtmaildir_test(composecontext) +add_qtmaildir_test(composewindow) add_qtmaildir_test(formattoolbar) add_qtmaildir_test(senddialog) add_qtmaildir_test(translations) diff --git a/tests/test_composewindow.cpp b/tests/test_composewindow.cpp new file mode 100644 index 0000000..04d3115 --- /dev/null +++ b/tests/test_composewindow.cpp @@ -0,0 +1,220 @@ +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +#include "composecontext.h" +#include "composewindow.h" +#include "config.h" +#include "signatures.h" + +class TestComposeWindow : public QObject +{ + Q_OBJECT + +private slots: + void init(); + void cleanup(); + + void aNewMessageSeedsTheComposeSignature(); + void anAccountSignatureOverridesTheComposeOne(); + void aResumedDraftSeedsNoSignature(); + void anUnknownSignatureNameSeedsNothing(); + void theSwitchListsEveryFileAndNone(); + +private: + /// A config pointing at a signatures directory holding \p files, with one + /// account that can send. + Config makeConfig(const QList> &files, + const QString &composeSignature, + const QString &accountSignature = {}); + + /// QVERIFY cannot appear inside makeConfig(), which returns a value: the + /// macro expands to a bare `return;` on failure, which is invalid in a + /// non-void function. A void helper keeps the check and sidesteps that. + void writeFile(const QString &path, const QString &content); + + QTemporaryDir *m_dir = nullptr; + QString m_signatureDir; +}; + +void TestComposeWindow::init() +{ + m_dir = new QTemporaryDir; + QVERIFY(m_dir->isValid()); + m_signatureDir = m_dir->path() + QStringLiteral("/signatures"); + QVERIFY(QDir().mkpath(m_signatureDir)); +} + +void TestComposeWindow::cleanup() +{ + delete m_dir; + m_dir = nullptr; +} + +void TestComposeWindow::writeFile(const QString &path, const QString &content) +{ + QFile file(path); + QVERIFY(file.open(QIODevice::WriteOnly | QIODevice::Text)); + QTextStream out(&file); + out << content; +} + +Config TestComposeWindow::makeConfig( + const QList> &files, + const QString &composeSignature, const QString &accountSignature) +{ + for (const auto &entry : files) + writeFile(m_signatureDir + QStringLiteral("/") + entry.first, entry.second); + + QString conf; + { + QTextStream out(&conf); + out << "[compose]\n" + << "signature = " << composeSignature << "\n" + << "\n" + << "[account.work]\n" + << "name = Someone\n" + << "address = someone@example.org\n" + << "maildir = work\n" + << "send_command = /bin/cat\n"; + if (!accountSignature.isEmpty()) + out << "signature = " << accountSignature << "\n"; + } + const QString path = m_dir->path() + QStringLiteral("/qtmaildir.conf"); + writeFile(path, conf); + + Config config; + config.load(path); + return config; +} + +void TestComposeWindow::aNewMessageSeedsTheComposeSignature() +{ + const Config config = makeConfig( + { { QStringLiteral("work.md"), QStringLiteral("Jane Doe") } }, + QStringLiteral("work")); + + ComposeContext context; + context.kind = ComposeContext::Kind::New; + context.accountKey = QStringLiteral("work"); + + ComposeWindow window(context, config, m_dir->path()); + window.setSignatureDir(m_signatureDir); + window.seedSignature(); + + auto *body = window.findChild(QStringLiteral("body")); + QVERIFY(body); + QVERIFY(body->toPlainText().endsWith(QStringLiteral("-- \nJane Doe"))); +} + +void TestComposeWindow::anAccountSignatureOverridesTheComposeOne() +{ + const Config config = makeConfig( + { { QStringLiteral("work.md"), QStringLiteral("Long one") }, + { QStringLiteral("brief.md"), QStringLiteral("Brief") } }, + QStringLiteral("work"), QStringLiteral("brief")); + + ComposeContext context; + context.kind = ComposeContext::Kind::New; + context.accountKey = QStringLiteral("work"); + + ComposeWindow window(context, config, m_dir->path()); + window.setSignatureDir(m_signatureDir); + window.seedSignature(); + + auto *body = window.findChild(QStringLiteral("body")); + QVERIFY(body); + QVERIFY(body->toPlainText().endsWith(QStringLiteral("-- \nBrief"))); +} + +void TestComposeWindow::aResumedDraftSeedsNoSignature() +{ + const Config config = makeConfig( + { { QStringLiteral("work.md"), QStringLiteral("Jane Doe") } }, + QStringLiteral("work")); + + // The saved body already carries whatever signature it was written with. + // Seeding again would put a SECOND one on a message written once. + ComposeContext context; + context.kind = ComposeContext::Kind::Draft; + context.accountKey = QStringLiteral("work"); + context.body = QStringLiteral("Half a thought.\n\n-- \nJane Doe"); + context.draftPath = m_dir->path() + QStringLiteral("/draft"); + + ComposeWindow window(context, config, m_dir->path()); + window.setSignatureDir(m_signatureDir); + window.seedSignature(); + + auto *body = window.findChild(QStringLiteral("body")); + QVERIFY(body); + QCOMPARE(body->toPlainText().count(QStringLiteral("-- \nJane Doe")), 1); +} + +void TestComposeWindow::anUnknownSignatureNameSeedsNothing() +{ + const Config config = makeConfig( + { { QStringLiteral("work.md"), QStringLiteral("Jane Doe") } }, + QStringLiteral("absent")); + + ComposeContext context; + context.kind = ComposeContext::Kind::New; + context.accountKey = QStringLiteral("work"); + + ComposeWindow window(context, config, m_dir->path()); + window.setSignatureDir(m_signatureDir); + window.seedSignature(); + + auto *body = window.findChild(QStringLiteral("body")); + QVERIFY(body); + // No signature, and the composer still opened rather than refusing. + QVERIFY(!body->toPlainText().contains(QStringLiteral("-- "))); +} + +void TestComposeWindow::theSwitchListsEveryFileAndNone() +{ + const Config config = makeConfig( + { { QStringLiteral("work.md"), QStringLiteral("Jane Doe") }, + { QStringLiteral("brief.md"), QStringLiteral("Brief") } }, + QString()); + + ComposeContext context; + context.kind = ComposeContext::Kind::New; + context.accountKey = QStringLiteral("work"); + + ComposeWindow window(context, config, m_dir->path()); + window.setSignatureDir(m_signatureDir); + window.seedSignature(); + + auto *button = + window.findChild(QStringLiteral("signatureSwitch")); + QVERIFY(button); + QVERIFY(button->menu()); + // "None" plus one per file. + QCOMPARE(button->menu()->actions().size(), 3); +} + +QTEST_MAIN(TestComposeWindow) +#include "test_composewindow.moc" diff --git a/translations/qtmaildir_it_IT.ts b/translations/qtmaildir_it_IT.ts index 1b0e756..a9c0f88 100644 --- a/translations/qtmaildir_it_IT.ts +++ b/translations/qtmaildir_it_IT.ts @@ -98,10 +98,22 @@ Sends the message as plain text with an HTML version alongside it. The plain text is what you typed. Invia il messaggio come testo semplice con una versione HTML a fianco. Il testo semplice è quello che hai scritto. + + Signature + Firma + + + Chooses the signature added to this message. + Sceglie la firma da aggiungere a questo messaggio. + Send Invia + + None + Nessuna + Large attachment Allegato di grandi dimensioni @@ -197,6 +209,10 @@ Il messaggio È stato inviato. Non inviarlo di nuovo. [compose] quote_position '%1' is not recognised; expected above or below. Using below. [compose] quote_position '%1' non è riconosciuto; atteso above o below. Uso below. + + [compose] signature_position '%1' is not recognised; expected end or above_quote. Using end. + [compose] signature_position '%1' non è riconosciuto; atteso end o above_quote. Uso end. + [compose] autosave_interval_ms '%1' is not a number; using %2. [compose] autosave_interval_ms '%1' non è un numero; verrà usato %2. -- cgit v1.2.3 From 9b8a6da4ff39428ce22dc23e16fc48cc062bc01f Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Mon, 24 Aug 2026 21:05:45 +0200 Subject: fix(signatures): match the guard against on-disk text, newline and all The guard compared the buffer block, with trailing blank lines trimmed, against knownSignatures() values returned verbatim by text(), which carry the trailing newline every editor writes. The two never compared equal, so a signature read back from disk was always treated as unknown: switching appended a second signature instead of replacing, and None removed nothing. Normalise each known entry the same way the block scan does, once in replace(), rather than per comparison. Part of item 152. --- src/signatures.cpp | 25 ++++++++++++++++++++++++- tests/test_signatures.cpp | 17 +++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) (limited to 'tests') diff --git a/src/signatures.cpp b/src/signatures.cpp index eb43048..ef631d6 100644 --- a/src/signatures.cpp +++ b/src/signatures.cpp @@ -69,6 +69,20 @@ bool isQuoted(const QString &line) return line.startsWith(QLatin1Char('>')); } +/// \p text with trailing blank lines removed, the same normalisation the block +/// scan below applies. text() returns file content verbatim, so a signature +/// file ends with the newline every editor writes; without this the match +/// compares a block with no trailing newline against a known entry that has +/// one, and the guard silently fails, appending a second signature instead of +/// replacing the first. +QString stripTrailingBlankLines(const QString &text) +{ + QStringList lines = text.split(QLatin1Char('\n')); + while (!lines.isEmpty() && lines.last().trimmed().isEmpty()) + lines.removeLast(); + return lines.join(QLatin1Char('\n')); +} + /// The index of the first line of the quote, or -1 when the buffer has none. /// /// The attribution line ("On Mon, someone wrote:") is deliberately NOT @@ -164,9 +178,18 @@ QStringList withoutSignature(const QStringList &lines, int delimiter) QString replace(const QString &buffer, const QString &signature, const QStringList &known, Position position) { + // Normalise known to the same footing the block scan uses, once here rather + // than per comparison. knownSignatures() passes text() verbatim, trailing + // newline and all, and the match must be newline-insensitive or the guard + // treats every on-disk signature as unknown. + QStringList normalized; + normalized.reserve(known.size()); + for (const QString &entry : known) + normalized.append(stripTrailingBlankLines(entry)); + QStringList lines = buffer.split(QLatin1Char('\n')); - const int existing = existingSignature(lines, known); + const int existing = existingSignature(lines, normalized); if (existing >= 0) lines = withoutSignature(lines, existing); diff --git a/tests/test_signatures.cpp b/tests/test_signatures.cpp index 31f9eb4..47b404b 100644 --- a/tests/test_signatures.cpp +++ b/tests/test_signatures.cpp @@ -40,6 +40,7 @@ private slots: void selectingNoneRemovesAKnownSignature(); void aBlockMatchingNoKnownSignatureIsNotRemoved(); void aDelimiterInsideTheQuoteIsNotTheSignature(); + void aSignatureReadBackFromDiskIsReplaced(); private: /// Writes \p files as name -> content into a fresh temporary directory. @@ -273,5 +274,21 @@ void TestSignatures::aDelimiterInsideTheQuoteIsNotTheSignature() QVERIFY(result.endsWith(QStringLiteral("-- \nJane Doe"))); } +void TestSignatures::aSignatureReadBackFromDiskIsReplaced() +{ + // known here is what knownSignatures() produces: text() verbatim, carrying + // the trailing newline every editor writes into a file. The block scan + // treats a trailing blank line as separation rather than text, so a naive + // match compares "Jane Doe" against "Jane Doe\n" and silently fails, and + // switching then APPENDS a second signature instead of replacing the first. + const QStringList known = { QStringLiteral("Jane Doe\n") }; + const QString buffer = QStringLiteral("Hello.\n\n-- \nJane Doe\n"); + + const QString result = Signatures::replace( + buffer, QStringLiteral("Brief"), known, Signatures::Position::End); + + QCOMPARE(result, QStringLiteral("Hello.\n\n-- \nBrief")); +} + QTEST_MAIN(TestSignatures) #include "test_signatures.moc" -- cgit v1.2.3 From cf88c95aa5f16b918ebf207b3e323e44c525b440 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Mon, 24 Aug 2026 21:11:33 +0200 Subject: feat(compose): the signature follows the account until it is chosen A From: change re-seeds the signature from the newly selected account, and stops doing so the moment the user picks one from the switch. Re-seeding unconditionally is the one behaviour that can silently discard a deliberate choice made a moment earlier; this is the shape send_html already uses. seededSignatureName() reads the combo rather than the context, which records where the composer opened and does not follow a change to it. Part of item 152. --- src/composewindow.cpp | 27 ++++++++-- tests/test_composewindow.cpp | 119 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 141 insertions(+), 5 deletions(-) (limited to 'tests') diff --git a/src/composewindow.cpp b/src/composewindow.cpp index 8236d79..94a3f46 100644 --- a/src/composewindow.cpp +++ b/src/composewindow.cpp @@ -406,8 +406,22 @@ void ComposeWindow::buildUi() for (QLineEdit *field : { m_to, m_cc, m_bcc, m_subject }) connect(field, &QLineEdit::textChanged, this, &ComposeWindow::markDirty); connect(m_sendHtml, &QCheckBox::toggled, this, &ComposeWindow::markDirty); - connect(m_from, &QComboBox::currentIndexChanged, this, - &ComposeWindow::markDirty); + connect(m_from, &QComboBox::currentIndexChanged, this, [this]() { + markDirty(); + // The account SEEDS the signature, so a change to it re-seeds. It + // stops the moment the user picks one: re-seeding unconditionally is + // the one behaviour that can silently discard a deliberate choice + // made a moment earlier. Same shape as send_html, which seeds from + // context and is then left alone. + if (m_signatureChosen) + return; + const QString seeded = seededSignatureName(); + if (!Signatures::names(m_signatureDir).contains(seeded)) { + applySignature(QString()); + return; + } + applySignature(seeded); + }); } void ComposeWindow::buildFormatToolbar() @@ -708,9 +722,12 @@ QStringList ComposeWindow::knownSignatures() const QString ComposeWindow::seededSignatureName() const { - // The account SEEDS, it does not bind: this is a starting value, and the - // switch keeps every signature reachable whichever account is selected. - const Account account = m_config.account(m_context.accountKey); + // The COMBO, not m_context: the context records where the composer opened + // and does not follow a From: change, so reading it would seed the + // original account's signature for ever. + const QString key = m_from->currentData().toString(); + const Account account = + m_config.account(key.isEmpty() ? m_context.accountKey : key); if (!account.signature.isEmpty()) return account.signature; return m_config.compose().signature; diff --git a/tests/test_composewindow.cpp b/tests/test_composewindow.cpp index 04d3115..8221ce3 100644 --- a/tests/test_composewindow.cpp +++ b/tests/test_composewindow.cpp @@ -17,6 +17,7 @@ */ #include +#include #include #include #include @@ -43,6 +44,8 @@ private slots: void aResumedDraftSeedsNoSignature(); void anUnknownSignatureNameSeedsNothing(); void theSwitchListsEveryFileAndNone(); + void changingTheAccountFollowsItsSignature(); + void changingTheAccountStopsFollowingOnceTheSwitchIsUsed(); private: /// A config pointing at a signatures directory holding \p files, with one @@ -216,5 +219,121 @@ void TestComposeWindow::theSwitchListsEveryFileAndNone() QCOMPARE(button->menu()->actions().size(), 3); } +void TestComposeWindow::changingTheAccountFollowsItsSignature() +{ + for (const auto &entry : + QList>{ + { QStringLiteral("work.md"), QStringLiteral("Work sig") }, + { QStringLiteral("home.md"), QStringLiteral("Home sig") } }) { + QFile file(m_signatureDir + QStringLiteral("/") + entry.first); + QVERIFY(file.open(QIODevice::WriteOnly | QIODevice::Text)); + file.write(entry.second.toUtf8()); + file.close(); + } + + const QString path = m_dir->path() + QStringLiteral("/qtmaildir.conf"); + { + QFile file(path); + QVERIFY(file.open(QIODevice::WriteOnly | QIODevice::Text)); + QTextStream out(&file); + out << "[account.work]\n" + << "name = Someone\naddress = someone@example.org\n" + << "maildir = work\nsend_command = /bin/cat\n" + << "signature = work\n" + << "\n[account.home]\n" + << "name = Someone\naddress = other@example.org\n" + << "maildir = home\nsend_command = /bin/cat\n" + << "signature = home\n"; + } + Config config; + config.load(path); + + ComposeContext context; + context.kind = ComposeContext::Kind::New; + context.accountKey = QStringLiteral("work"); + + ComposeWindow window(context, config, m_dir->path()); + window.setSignatureDir(m_signatureDir); + window.seedSignature(); + + auto *body = window.findChild(QStringLiteral("body")); + auto *from = window.findChild(QStringLiteral("from")); + QVERIFY(body); + QVERIFY(from); + QVERIFY(body->toPlainText().contains(QStringLiteral("Work sig"))); + + // Select the other account by its key, never by index: the order of the + // combo is the config's and an index assertion would pass on the wrong one. + const int home = from->findData(QStringLiteral("home")); + QVERIFY(home >= 0); + from->setCurrentIndex(home); + + QVERIFY(body->toPlainText().contains(QStringLiteral("Home sig"))); + QVERIFY(!body->toPlainText().contains(QStringLiteral("Work sig"))); +} + +void TestComposeWindow::changingTheAccountStopsFollowingOnceTheSwitchIsUsed() +{ + for (const auto &entry : + QList>{ + { QStringLiteral("work.md"), QStringLiteral("Work sig") }, + { QStringLiteral("home.md"), QStringLiteral("Home sig") }, + { QStringLiteral("chosen.md"), QStringLiteral("Chosen sig") } }) { + QFile file(m_signatureDir + QStringLiteral("/") + entry.first); + QVERIFY(file.open(QIODevice::WriteOnly | QIODevice::Text)); + file.write(entry.second.toUtf8()); + file.close(); + } + + const QString path = m_dir->path() + QStringLiteral("/qtmaildir.conf"); + { + QFile file(path); + QVERIFY(file.open(QIODevice::WriteOnly | QIODevice::Text)); + QTextStream out(&file); + out << "[account.work]\n" + << "name = Someone\naddress = someone@example.org\n" + << "maildir = work\nsend_command = /bin/cat\n" + << "signature = work\n" + << "\n[account.home]\n" + << "name = Someone\naddress = other@example.org\n" + << "maildir = home\nsend_command = /bin/cat\n" + << "signature = home\n"; + } + Config config; + config.load(path); + + ComposeContext context; + context.kind = ComposeContext::Kind::New; + context.accountKey = QStringLiteral("work"); + + ComposeWindow window(context, config, m_dir->path()); + window.setSignatureDir(m_signatureDir); + window.seedSignature(); + + auto *body = window.findChild(QStringLiteral("body")); + auto *from = window.findChild(QStringLiteral("from")); + auto *button = + window.findChild(QStringLiteral("signatureSwitch")); + QVERIFY(body); + QVERIFY(from); + QVERIFY(button); + + // The user picks one deliberately. + for (QAction *action : button->menu()->actions()) { + if (action->data().toString() == QStringLiteral("chosen")) + action->trigger(); + } + QVERIFY(body->toPlainText().contains(QStringLiteral("Chosen sig"))); + + const int home = from->findData(QStringLiteral("home")); + QVERIFY(home >= 0); + from->setCurrentIndex(home); + + // The deliberate choice survives the account change. Overwriting it is + // the one behaviour that can silently discard something the user just did. + QVERIFY(body->toPlainText().contains(QStringLiteral("Chosen sig"))); + QVERIFY(!body->toPlainText().contains(QStringLiteral("Home sig"))); +} + QTEST_MAIN(TestComposeWindow) #include "test_composewindow.moc" -- cgit v1.2.3 From c08ca00c36d06d7b1bbd5ce73d4d4dc3ce157e1c Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Mon, 24 Aug 2026 21:19:05 +0200 Subject: fix(compose): a resumed draft does not re-seed on a From: change A resumed draft kept m_signatureChosen false, so a From: change re-seeded the signature and rewrote what the user had saved, inserting the new account's where the saved block no longer matched a known file. The draft is the user's deliberate prior state and must not follow a From: change, so the draft branch marks it chosen. --- src/composewindow.cpp | 5 ++++ tests/test_composewindow.cpp | 58 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+) (limited to 'tests') diff --git a/src/composewindow.cpp b/src/composewindow.cpp index 94a3f46..d781e52 100644 --- a/src/composewindow.cpp +++ b/src/composewindow.cpp @@ -792,6 +792,11 @@ void ComposeWindow::seedSignature() // verbatim. Seeding again would append a second one. if (m_context.kind == ComposeContext::Kind::Draft) { none->setChecked(true); + // The draft IS the user's choice: its signature is deliberate prior + // state, so a From: change must not follow the new account and + // rewrite what was saved. Marking it chosen keeps the same invariant + // the switch actions set, without ever having run the switch. + m_signatureChosen = true; return; } diff --git a/tests/test_composewindow.cpp b/tests/test_composewindow.cpp index 8221ce3..47f7d87 100644 --- a/tests/test_composewindow.cpp +++ b/tests/test_composewindow.cpp @@ -46,6 +46,7 @@ private slots: void theSwitchListsEveryFileAndNone(); void changingTheAccountFollowsItsSignature(); void changingTheAccountStopsFollowingOnceTheSwitchIsUsed(); + void aResumedDraftDoesNotReseedOnAnAccountChange(); private: /// A config pointing at a signatures directory holding \p files, with one @@ -335,5 +336,62 @@ void TestComposeWindow::changingTheAccountStopsFollowingOnceTheSwitchIsUsed() QVERIFY(!body->toPlainText().contains(QStringLiteral("Home sig"))); } +void TestComposeWindow::aResumedDraftDoesNotReseedOnAnAccountChange() +{ + for (const auto &entry : + QList>{ + { QStringLiteral("work.md"), QStringLiteral("Work sig") }, + { QStringLiteral("home.md"), QStringLiteral("Home sig") } }) { + QFile file(m_signatureDir + QStringLiteral("/") + entry.first); + QVERIFY(file.open(QIODevice::WriteOnly | QIODevice::Text)); + file.write(entry.second.toUtf8()); + file.close(); + } + + const QString path = m_dir->path() + QStringLiteral("/qtmaildir.conf"); + { + QFile file(path); + QVERIFY(file.open(QIODevice::WriteOnly | QIODevice::Text)); + QTextStream out(&file); + out << "[account.work]\n" + << "name = Someone\naddress = someone@example.org\n" + << "maildir = work\nsend_command = /bin/cat\n" + << "signature = work\n" + << "\n[account.home]\n" + << "name = Someone\naddress = other@example.org\n" + << "maildir = home\nsend_command = /bin/cat\n" + << "signature = home\n"; + } + Config config; + config.load(path); + + // The saved body already carries its own signature, which does not match + // any on-disk file. A From: change must not replace it with the new + // account's: the draft is the message the user wrote, exactly as + // seedBody() takes its body verbatim. + ComposeContext context; + context.kind = ComposeContext::Kind::Draft; + context.accountKey = QStringLiteral("work"); + context.body = QStringLiteral("Half a thought.\n\n-- \nJane Doe"); + context.draftPath = m_dir->path() + QStringLiteral("/draft"); + + ComposeWindow window(context, config, m_dir->path()); + window.setSignatureDir(m_signatureDir); + window.seedSignature(); + + auto *body = window.findChild(QStringLiteral("body")); + auto *from = window.findChild(QStringLiteral("from")); + QVERIFY(body); + QVERIFY(from); + QVERIFY(body->toPlainText().contains(QStringLiteral("Jane Doe"))); + + const int home = from->findData(QStringLiteral("home")); + QVERIFY(home >= 0); + from->setCurrentIndex(home); + + QVERIFY(body->toPlainText().contains(QStringLiteral("Jane Doe"))); + QVERIFY(!body->toPlainText().contains(QStringLiteral("Home sig"))); +} + QTEST_MAIN(TestComposeWindow) #include "test_composewindow.moc" -- cgit v1.2.3 From b7f2a4e0f8858b1aa0d86755ebab6826306f3eff Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Mon, 24 Aug 2026 21:57:28 +0200 Subject: fix(drafts): index a saved draft so it appears without a sync Autosave writes the draft to the Maildir drafts folder and stops, while the Drafts view is a notmuch path: query, so a freshly saved draft was invisible until notmuch new ran. saveDraftNow() now emits draftSaved, and MainWindow connects it to a new NotmuchWorker::indexDraftFile() that indexes the one file the way moveMessages() does, with the previous revision removed so a rewrite leaves no ghost. The send path unlinks a draft that was indexed while being composed, so draftRemoved -> removeIndexedFile() drops its entry too. Measured: notmuch_database_index_file assigns NO tags (unlike notmuch new, which adds draft inbox unread), so no tag-stripping is needed and the draft cannot leak into a tag:inbox view. Item 158. --- src/composewindow.cpp | 6 +++ src/composewindow.h | 11 +++++ src/mainwindow.cpp | 11 +++++ src/notmuchworker.cpp | 93 ++++++++++++++++++++++++++++++++++++- src/notmuchworker.h | 22 +++++++++ tests/test_composewindow.cpp | 54 ++++++++++++++++++++++ tests/test_notmuchworker.cpp | 106 +++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 302 insertions(+), 1 deletion(-) (limited to 'tests') diff --git a/src/composewindow.cpp b/src/composewindow.cpp index d781e52..c35bb5d 100644 --- a/src/composewindow.cpp +++ b/src/composewindow.cpp @@ -1018,6 +1018,7 @@ bool ComposeWindow::saveDraftNow() const QString folder = QDir(m_mailRoot).absoluteFilePath( account.maildir + QLatin1Char('/') + account.drafts); + const QString previousPath = m_draftPath; const DraftStore::Result written = DraftStore::write(folder, built.bytes, QStringLiteral("D"), m_draftPath); @@ -1038,6 +1039,10 @@ bool ComposeWindow::saveDraftNow() m_dirty = false; m_saveFailed = false; m_banner->hide(); + + // The write is done and the previous revision already unlinked; hand both + // paths up so the owner indexes the new one and drops the old (item 158). + emit draftSaved(written.path, previousPath); return true; } @@ -1200,6 +1205,7 @@ void ComposeWindow::send() dialog->setStage(SendDialog::Stage::RemovingDraft); if (!m_draftPath.isEmpty()) { QFile::remove(m_draftPath); + emit draftRemoved(m_draftPath); m_draftPath.clear(); } diff --git a/src/composewindow.h b/src/composewindow.h index 918d4e3..caff011 100644 --- a/src/composewindow.h +++ b/src/composewindow.h @@ -168,6 +168,17 @@ signals: /// pointer before WA_DeleteOnClose destroys the window. void closed(ComposeWindow *window); + /// A draft was written to disk, so the window's owner can index it and it + /// appears in the Drafts view without a full sync (item 158). + /// + /// \p path is the file just written, absolute. \p previousPath is the file + /// the write replaced, empty on the first save of a new draft. + void draftSaved(const QString &path, const QString &previousPath); + + /// A draft file was unlinked (sent), so its index entry must go too. + /// \p path is the file that was removed, absolute. + void draftRemoved(const QString &path); + protected: /// The one place the registry is told, whichever route closes the window. void closeEvent(QCloseEvent *event) override; diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 99eb2a3..af3b817 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -1138,6 +1138,17 @@ void MainWindow::openComposer(const ComposeContext &context) }); }); + // A saved draft is indexed immediately (item 158): the Drafts view is a + // path query, and without this the draft is invisible until the next sync. + // The worker lives on another thread, so this is a queued connection and + // notmuch stays on its own thread. + connect(composer, &ComposeWindow::draftSaved, m_worker, + &NotmuchWorker::indexDraftFile); + + // A draft unlinked on send must leave no ghost entry behind. + connect(composer, &ComposeWindow::draftRemoved, m_worker, + &NotmuchWorker::removeIndexedFile); + composer->show(); } diff --git a/src/notmuchworker.cpp b/src/notmuchworker.cpp index fca0a5a..16df4ed 100644 --- a/src/notmuchworker.cpp +++ b/src/notmuchworker.cpp @@ -820,8 +820,99 @@ void NotmuchWorker::moveMessages(const QStringList &messageIds, emit messagesMovedFrom(origins, destFolder); } +void NotmuchWorker::indexDraftFile(const QString &path, + const QString &previousPath) +{ + if (path.isEmpty()) + return; + + // Same ordering as applyTags() and moveMessages(): notmuch allows one open + // handle per process, so the read-only one must close before the write. + 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; + } + + notmuch_message_t *indexed = nullptr; + const notmuch_status_t added = notmuch_database_index_file( + db, path.toUtf8().constData(), nullptr, &indexed); + if (indexed) + notmuch_message_destroy(indexed); + + // DUPLICATE_MESSAGE_ID is success here, exactly as in moveMessages(): the + // file reached the database, it is only the id that was already known. + if (added != NOTMUCH_STATUS_SUCCESS + && added != NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID) { + notmuch_database_close(db); + notmuch_database_destroy(db); + emit errorOccurred( + QStringLiteral("Cannot index %1: %2") + .arg(QFileInfo(path).fileName(), + QString::fromUtf8(notmuch_status_to_string(added)))); + return; + } + + // The previous revision, if any, is already unlinked from disk; its entry + // must not linger as a ghost draft with a filename that no longer exists. + if (!previousPath.isEmpty() && previousPath != path) + notmuch_database_remove_message(db, previousPath.toUtf8().constData()); + + notmuch_database_close(db); + notmuch_database_destroy(db); +} + +void NotmuchWorker::removeIndexedFile(const QString &path) +{ + if (path.isEmpty()) + return; + + 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; + } + + notmuch_database_remove_message(db, path.toUtf8().constData()); + + notmuch_database_close(db); + notmuch_database_destroy(db); +} + void NotmuchWorker::resolveMessages(const QStringList &messageIds, - const QString &requestTag) + const QString &requestTag) { if (messageIds.isEmpty()) return; diff --git a/src/notmuchworker.h b/src/notmuchworker.h index 8ed878f..3ccf8e5 100644 --- a/src/notmuchworker.h +++ b/src/notmuchworker.h @@ -134,6 +134,28 @@ public slots: /// it, so removing before indexing loses the message's tags. void moveMessages(const QStringList &messageIds, const QString &destFolder); + /// Indexes one freshly written file, so it appears in a `path:` query + /// without a full `notmuch new` (item 158). + /// + /// The draft-save path writes the file and stops, and the Drafts view is a + /// path query, so an unindexed draft is invisible until the next sync. A + /// draft rewrite writes a NEW file (MessageBuilder generates a fresh + /// Message-ID on every build) and unlinks the old, so \p previousPath is + /// removed after the new one is indexed, mirroring moveMessages()'s + /// ordering: the old entry must not linger as a ghost draft. + /// + /// \p path is absolute, as DraftStore::write() returns it. The Maildir + /// flags on the file (the "D" flag a draft carries) drive its tags exactly + /// as they would on a later `notmuch new`. + void indexDraftFile(const QString &path, const QString &previousPath = {}); + + /// Removes one file from the index, without touching the file on disk. + /// + /// The counterpart to indexDraftFile() for the send path: a draft that was + /// indexed while being composed is unlinked when it is sent, and its entry + /// must not linger as a ghost draft until the next sync. + void removeIndexedFile(const QString &path); + /// 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 diff --git a/tests/test_composewindow.cpp b/tests/test_composewindow.cpp index 47f7d87..472c103 100644 --- a/tests/test_composewindow.cpp +++ b/tests/test_composewindow.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -47,6 +48,7 @@ private slots: void changingTheAccountFollowsItsSignature(); void changingTheAccountStopsFollowingOnceTheSwitchIsUsed(); void aResumedDraftDoesNotReseedOnAnAccountChange(); + void savingADraftEmitsItsPathAndTheReplacedOne(); private: /// A config pointing at a signatures directory holding \p files, with one @@ -393,5 +395,57 @@ void TestComposeWindow::aResumedDraftDoesNotReseedOnAnAccountChange() QVERIFY(!body->toPlainText().contains(QStringLiteral("Home sig"))); } +void TestComposeWindow::savingADraftEmitsItsPathAndTheReplacedOne() +{ + // A config whose account has a drafts folder, which makeConfig() does not + // set, so the save can actually write somewhere. + const QString confPath = m_dir->path() + QStringLiteral("/qtmaildir.conf"); + { + QString conf; + QTextStream out(&conf); + out << "[account.work]\n" + << "name = Someone\n" + << "address = someone@example.org\n" + << "maildir = work\n" + << "drafts = Drafts\n" + << "send_command = /bin/cat\n"; + writeFile(confPath, conf); + } + Config config; + config.load(confPath); + + ComposeContext context; + context.kind = ComposeContext::Kind::New; + context.accountKey = QStringLiteral("work"); + + ComposeWindow window(context, config, m_dir->path()); + auto *body = window.findChild(QStringLiteral("body")); + QVERIFY(body); + + QSignalSpy saved(&window, &ComposeWindow::draftSaved); + + body->setPlainText(QStringLiteral("First revision.")); + QVERIFY(window.saveDraftNow()); + + QCOMPARE(saved.size(), 1); + const QString first = saved.first().at(0).toString(); + const QString firstPrevious = saved.first().at(1).toString(); + QVERIFY(!first.isEmpty()); + QVERIFY(firstPrevious.isEmpty()); + QVERIFY(QFile::exists(first)); + + // A rewrite writes a fresh file and unlinks the old; the previous path + // comes back so the owner can drop the old index entry. + body->setPlainText(QStringLiteral("Second revision.")); + QVERIFY(window.saveDraftNow()); + + QCOMPARE(saved.size(), 2); + const QString second = saved.at(1).at(0).toString(); + const QString secondPrevious = saved.at(1).at(1).toString(); + QVERIFY(!second.isEmpty()); + QCOMPARE(secondPrevious, first); + QVERIFY2(second != first, "a rewrite reused the old filename"); +} + QTEST_MAIN(TestComposeWindow) #include "test_composewindow.moc" diff --git a/tests/test_notmuchworker.cpp b/tests/test_notmuchworker.cpp index 998696f..d02f8bd 100644 --- a/tests/test_notmuchworker.cpp +++ b/tests/test_notmuchworker.cpp @@ -93,6 +93,10 @@ private slots: void moveMessagesGivesTheFileAFreshMaildirName(); void moveMessagesKeepsTheMaildirFlags(); + void indexDraftFileMakesAFileFindable(); + void indexDraftFileRemovesThePreviousFile(); + void removeIndexedFileDropsTheEntry(); + void aSplitIndexStillResolvesTheMailRoot(); void aSplitIndexMovesIntoTheMaildirNotTheIndex(); void aSplitIndexListsTheMaildirsFolders(); @@ -103,6 +107,9 @@ private: /// 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); + /// Writes a draft file into /cur with the "D" flag and returns its + /// path, WITHOUT indexing it, so a test can index just that file. + QString writeDraftFile(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, @@ -215,6 +222,42 @@ bool TestNotmuchWorker::addMovableMessage(const QString &folder, return m_fixture.index(); } +QString TestNotmuchWorker::writeDraftFile(const QString &folder, + const QString &messageId) +{ + const QString dirPath = m_fixture.maildirPath() + QLatin1Char('/') + folder; + QDir dir; + if (!dir.mkpath(dirPath + QStringLiteral("/cur")) + || !dir.mkpath(dirPath + QStringLiteral("/new")) + || !dir.mkpath(dirPath + QStringLiteral("/tmp"))) { + return {}; + } + + // The same filename recipe addMessage() uses, with the draft flag instead + // of the seen flag, matching what DraftStore writes. + QString base = messageId; + base.remove(QLatin1Char('<')).remove(QLatin1Char('>')); + base.replace(QLatin1Char('@'), QLatin1Char('.')); + base.replace(QLatin1Char('/'), QLatin1Char('.')); + base += QStringLiteral(":2,D"); + + const QString path = dirPath + QStringLiteral("/cur/") + base; + QFile file(path); + if (!file.open(QIODevice::WriteOnly | QIODevice::Text)) + return {}; + QTextStream out(&file); + out << "From: You \n" + << "To: someone@example.org\n" + << "Subject: A draft\n" + << "Message-ID: <" << messageId << ">\n" + << "Date: Sun, 7 Jun 2026 10:00:00 +0000\n" + << "\n" + << "draft body\n"; + out.flush(); + file.close(); + return path; +} + QString TestNotmuchWorker::fileOf(const QString &messageId, const QString &configPath) { @@ -1383,6 +1426,69 @@ void TestNotmuchWorker::moveMessagesReportsOnlyWhatMoved() QCOMPARE(inTrash.size(), 1); } +void TestNotmuchWorker::indexDraftFileMakesAFileFindable() +{ + const QString id = QStringLiteral("draft1@example.org"); + const QString path = writeDraftFile(QStringLiteral("drafts"), id); + QVERIFY(!path.isEmpty()); + + // On disk but not indexed: no query sees it, which is item 158's defect. + QCOMPARE(runQuery(QStringLiteral("id:%1").arg(id)).size(), 0); + + NotmuchWorker worker(m_fixture.configPath()); + QSignalSpy errors(&worker, &NotmuchWorker::errorOccurred); + worker.indexDraftFile(path); + QVERIFY2(errors.isEmpty(), qPrintable(errors.value(0).value(0).toString())); + + QCOMPARE(runQuery(QStringLiteral("id:%1").arg(id)).size(), 1); +} + +void TestNotmuchWorker::indexDraftFileRemovesThePreviousFile() +{ + const QString first = QStringLiteral("draft2@example.org"); + const QString second = QStringLiteral("draft3@example.org"); + const QString firstPath = writeDraftFile(QStringLiteral("drafts"), first); + QVERIFY(!firstPath.isEmpty()); + + NotmuchWorker worker(m_fixture.configPath()); + QSignalSpy errors(&worker, &NotmuchWorker::errorOccurred); + worker.indexDraftFile(firstPath); + QVERIFY2(errors.isEmpty(), qPrintable(errors.value(0).value(0).toString())); + QCOMPARE(runQuery(QStringLiteral("id:%1").arg(first)).size(), 1); + + // A rewrite: a new file (a fresh Message-ID) and the old one unlinked, as + // DraftStore does on every autosave. The old entry must not linger. + const QString secondPath = writeDraftFile(QStringLiteral("drafts"), second); + QVERIFY(!secondPath.isEmpty()); + QVERIFY(QFile::remove(firstPath)); + + worker.indexDraftFile(secondPath, firstPath); + QVERIFY2(errors.isEmpty(), qPrintable(errors.value(0).value(0).toString())); + + QCOMPARE(runQuery(QStringLiteral("id:%1").arg(second)).size(), 1); + QCOMPARE(runQuery(QStringLiteral("id:%1").arg(first)).size(), 0); +} + +void TestNotmuchWorker::removeIndexedFileDropsTheEntry() +{ + const QString id = QStringLiteral("draft4@example.org"); + const QString path = writeDraftFile(QStringLiteral("drafts"), id); + QVERIFY(!path.isEmpty()); + + NotmuchWorker worker(m_fixture.configPath()); + QSignalSpy errors(&worker, &NotmuchWorker::errorOccurred); + worker.indexDraftFile(path); + QVERIFY2(errors.isEmpty(), qPrintable(errors.value(0).value(0).toString())); + QCOMPARE(runQuery(QStringLiteral("id:%1").arg(id)).size(), 1); + + // The send path unlinks the draft and drops its entry, so it does not + // linger as a ghost until the next sync. + QVERIFY(QFile::remove(path)); + worker.removeIndexedFile(path); + QVERIFY2(errors.isEmpty(), qPrintable(errors.value(0).value(0).toString())); + QCOMPARE(runQuery(QStringLiteral("id:%1").arg(id)).size(), 0); +} + // Item 124. notmuch can put the Xapian index outside the mail root // (`mail_root` + `path`), which is how the index moves to faster storage while -- 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 'tests') 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