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/test_signatures.cpp | 104 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 104 insertions(+) create mode 100644 tests/test_signatures.cpp (limited to 'tests/test_signatures.cpp') 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/test_signatures.cpp') 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/test_signatures.cpp') 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/test_signatures.cpp') 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 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/test_signatures.cpp') 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