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 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 136 insertions(+) (limited to 'src/composewindow.cpp') 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); -- 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 'src/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 'src/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 'src/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