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