diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-24 20:00:45 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-24 20:00:45 +0200 |
| commit | 86af01996dd702bdc3f837e96f154088e8b45538 (patch) | |
| tree | 26d236b2780ca08e31f12ad70c9f10c1f6663d29 | |
| parent | be2534ab60d362b2f685130265f727190ccf867a (diff) | |
| download | qtmaildir-86af01996dd702bdc3f837e96f154088e8b45538.tar.gz qtmaildir-86af01996dd702bdc3f837e96f154088e8b45538.zip | |
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.
| -rw-r--r-- | src/signatures.cpp | 87 | ||||
| -rw-r--r-- | tests/test_signatures.cpp | 101 |
2 files changed, 181 insertions, 7 deletions
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" |
