From 441b93a75b9941d395edfe4b65ed0af2da9e0021 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Fri, 14 Aug 2026 12:42:38 +0200 Subject: feat(search): build notmuch terms for the right-click actions One place for the query grammar behind every search surface, with no widget involved so it is tested without a painter or a web engine. extend() parenthesises both sides. The query bar may hold a hand-written disjunction, and 'a or b AND c' binds as 'a or (b AND c)', which widens a search meant to narrow it and reports nothing. --- src/CMakeLists.txt | 1 + src/searchterm.cpp | 93 +++++++++++++++++++++++++++++ src/searchterm.h | 94 +++++++++++++++++++++++++++++ tests/CMakeLists.txt | 1 + tests/test_searchterm.cpp | 146 ++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 335 insertions(+) create mode 100644 src/searchterm.cpp create mode 100644 src/searchterm.h create mode 100644 tests/test_searchterm.cpp diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 0945f65..382f3b8 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -25,6 +25,7 @@ add_library(qtmaildir_lib STATIC mainwindow.cpp querycompleter.cpp rulequery.cpp + searchterm.cpp ) target_include_directories(qtmaildir_lib diff --git a/src/searchterm.cpp b/src/searchterm.cpp new file mode 100644 index 0000000..acc64c6 --- /dev/null +++ b/src/searchterm.cpp @@ -0,0 +1,93 @@ +/* + * 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 "searchterm.h" + +#include + +namespace SearchTerm { + +QString quote(const QString &value) +{ + // simplified() collapses every run of whitespace, newlines and tabs + // included, and trims the ends. A selection spanning paragraphs arrives + // full of newlines, which would otherwise reach the query bar verbatim. + QString cleaned = value.simplified(); + if (cleaned.isEmpty()) + return {}; + + if (cleaned.size() > kMaxValueLength) + cleaned.truncate(kMaxValueLength); + + // Backslashes first: escaping the quotes first would then escape the + // backslashes this step adds, doubling them. + cleaned.replace(QLatin1Char('\\'), QStringLiteral("\\\\")); + cleaned.replace(QLatin1Char('"'), QStringLiteral("\\\"")); + + return QLatin1Char('"') + cleaned + QLatin1Char('"'); +} + +QString field(const QString &name, const QString &value) +{ + const QString quoted = quote(value); + if (quoted.isEmpty()) + return {}; + return name + QLatin1Char(':') + quoted; +} + +QString onDate(const QDate &day) +{ + if (!day.isValid()) + return {}; + const QString text = day.toString(QStringLiteral("yyyy-MM-dd")); + return QStringLiteral("date:%1..%1").arg(text); +} + +QString tag(const QString &name) +{ + const QString trimmed = name.simplified(); + if (trimmed.isEmpty()) + return {}; + + // A tag is a token from a vocabulary the user chose, so it reads better + // unquoted in the bar they are about to edit. Quoted only when it holds + // something that would not survive. + const bool needsQuoting = + std::any_of(trimmed.cbegin(), trimmed.cend(), [](QChar ch) { + return !(ch.isLetterOrNumber() || ch == QLatin1Char('-') + || ch == QLatin1Char('_') || ch == QLatin1Char('.') + || ch == QLatin1Char('/')); + }); + + return QStringLiteral("tag:") + (needsQuoting ? quote(trimmed) : trimmed); +} + +QString extend(const QString &existing, const QString &addition) +{ + const QString left = existing.trimmed(); + const QString right = addition.trimmed(); + + if (right.isEmpty()) + return left; + if (left.isEmpty()) + return right; + + return QStringLiteral("(%1) AND (%2)").arg(left, right); +} + +} // namespace SearchTerm diff --git a/src/searchterm.h b/src/searchterm.h new file mode 100644 index 0000000..d596676 --- /dev/null +++ b/src/searchterm.h @@ -0,0 +1,94 @@ +/* + * 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. + */ + +#pragma once + +#include +#include +#include + +/// Builds the notmuch query strings behind the right-click search actions. +/// +/// Free functions and no widget, so the whole query grammar is testable +/// without a painter, a web engine or a window. Every surface that offers a +/// search goes through here, which is what stops five surfaces growing five +/// slightly different quoting rules. +/// +/// **notmuch rejects almost nothing.** `from:((((` parses cleanly and matches +/// zero, so a malformed query produces an empty result rather than an error +/// the user could act on. Correctness here cannot be checked by asking notmuch; +/// it is checked against the constructed string. +namespace SearchTerm { + +/// Longest quoted value. A selection longer than this is a mis-drag rather +/// than a search, and the query bar is an editable line the user has to be +/// able to read. +inline constexpr int kMaxValueLength = 200; + +/// Quotes an arbitrary value for use as a notmuch term. +/// +/// Whitespace and newlines collapse to single spaces, embedded quotes are +/// escaped, the value is capped at kMaxValueLength, and an empty or +/// whitespace-only value yields an EMPTY STRING rather than `""`. Callers +/// test for empty to decide whether to offer a menu entry at all. +QString quote(const QString &value); + +/// `field:"value"`, or empty when the value is empty. +/// +/// The field name is a notmuch keyword and is never translated: it is wire +/// format, not user-facing text. +QString field(const QString &name, const QString &value); + +/// `date:YYYY-MM-DD..YYYY-MM-DD` for a single day, empty for an invalid date. +/// +/// The day twice rather than the day and its successor: notmuch's range is +/// inclusive at both ends, so the naive `..next-day` form silently includes a +/// second day of mail. +QString onDate(const QDate &day); + +/// `tag:name`, quoted only when the name needs it. +QString tag(const QString &name); + +/// Narrows `existing` by `addition`, as `(existing) AND (addition)`. +/// +/// **Both sides are parenthesised and that is load-bearing.** The query bar +/// may hold a hand-written disjunction, and `a or b AND c` binds as +/// `a or (b AND c)`: the result WIDENS a search the user asked to narrow, and +/// nothing reports an error. The same trap is why the post-new hook +/// parenthesises a rule's query before scoping it with `tag:new`. +/// +/// An empty `existing` yields `addition` alone rather than `() AND (x)`, which +/// matches nothing; an empty `addition` leaves `existing` untouched. +QString extend(const QString &existing, const QString &addition); + +} // namespace SearchTerm + +/// One entry a context menu can offer: a finished query and the text naming it. +/// +/// Carried rather than rebuilt at menu-construction time, so the value a menu +/// entry searches for is the value the pane extracted, with no second parse of +/// anything already rendered. +struct SearchOffer +{ + /// Shown in the menu. Already translated, and elides a long value: the + /// query keeps the full one. + QString label; + + /// The finished notmuch query. Never empty in a constructed offer. + QString query; +}; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index e5142b6..5b0b85b 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -54,3 +54,4 @@ add_qtmaildir_test(querycompleter) add_qtmaildir_test(tagdialog) add_qtmaildir_test(tagrules) add_qtmaildir_test(rulequery) +add_qtmaildir_test(searchterm) diff --git a/tests/test_searchterm.cpp b/tests/test_searchterm.cpp new file mode 100644 index 0000000..e0bdb60 --- /dev/null +++ b/tests/test_searchterm.cpp @@ -0,0 +1,146 @@ +/* + * 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 "searchterm.h" + +/// The query grammar of the right-click search actions. +/// +/// Asserted on the CONSTRUCTED STRING throughout, never on a query notmuch +/// refuses: notmuch's parser rejects almost nothing, so `from:((((` parses +/// cleanly and matches zero. A test expecting a failure would pass against +/// correct code and against broken code alike. +class TestSearchTerm : public QObject +{ + Q_OBJECT +private slots: + void quotesAPlainValue(); + void escapesEmbeddedQuotes(); + void collapsesWhitespaceAndNewlines(); + void rejectsEmptyAndWhitespaceOnly(); + void capsAnOverlongSelection(); + void buildsAFieldTerm(); + void omitsAFieldWithNoValue(); + void buildsADateRangeForOneDay(); + void tagIsNotQuoted(); + void extendParenthesisesBothSides(); + void extendOntoAnEmptyQueryIsAReplace(); +}; + +void TestSearchTerm::quotesAPlainValue() +{ + QCOMPARE(SearchTerm::quote(QStringLiteral("Quarterly report")), + QStringLiteral("\"Quarterly report\"")); +} + +void TestSearchTerm::escapesEmbeddedQuotes() +{ + // A selection is arbitrary prose and can carry a quote. Unescaped, it ends + // the quoted string early and the rest becomes stray query syntax, which + // notmuch accepts and matches nothing on. + QCOMPARE(SearchTerm::quote(QStringLiteral("say \"hello\" now")), + QStringLiteral("\"say \\\"hello\\\" now\"")); +} + +void TestSearchTerm::collapsesWhitespaceAndNewlines() +{ + QCOMPARE(SearchTerm::quote(QStringLiteral(" two\n\nlines\there ")), + QStringLiteral("\"two lines here\"")); +} + +void TestSearchTerm::rejectsEmptyAndWhitespaceOnly() +{ + // An empty term must yield an empty string, which is what every caller + // tests to decide whether to offer a menu entry at all. + QVERIFY(SearchTerm::quote(QString()).isEmpty()); + QVERIFY(SearchTerm::quote(QStringLiteral(" \n\t ")).isEmpty()); +} + +void TestSearchTerm::capsAnOverlongSelection() +{ + // A multi-kilobyte selection is a mis-drag, not a query. + const QString huge(5000, QLatin1Char('x')); + const QString term = SearchTerm::quote(huge); + QVERIFY2(term.size() < 300, + qPrintable(QStringLiteral("term was %1 chars").arg(term.size()))); + QVERIFY(term.startsWith(QStringLiteral("\"xxx"))); + QVERIFY(term.endsWith(QLatin1Char('"'))); +} + +void TestSearchTerm::buildsAFieldTerm() +{ + QCOMPARE(SearchTerm::field(QStringLiteral("from"), + QStringLiteral("Foo ")), + QStringLiteral("from:\"Foo \"")); +} + +void TestSearchTerm::omitsAFieldWithNoValue() +{ + // A message with no Cc must not offer cc:"" , which parses cleanly and + // matches nothing, so the entry would look enabled and do nothing. + QVERIFY(SearchTerm::field(QStringLiteral("cc"), QString()).isEmpty()); +} + +void TestSearchTerm::buildsADateRangeForOneDay() +{ + // notmuch's date: range is inclusive at both ends, so one day is the day + // named twice rather than the day and its successor. + const QDate day(2026, 8, 14); + QCOMPARE(SearchTerm::onDate(day), + QStringLiteral("date:2026-08-14..2026-08-14")); + QVERIFY(SearchTerm::onDate(QDate()).isEmpty()); +} + +void TestSearchTerm::tagIsNotQuoted() +{ + // A tag name is a token from a known vocabulary, not prose. Quoting one + // is not wrong but reads badly in the bar, and the user edits that text. + QCOMPARE(SearchTerm::tag(QStringLiteral("inbox")), + QStringLiteral("tag:inbox")); + // A tag containing a space is the exception and does need quoting. + QCOMPARE(SearchTerm::tag(QStringLiteral("to do")), + QStringLiteral("tag:\"to do\"")); + QVERIFY(SearchTerm::tag(QString()).isEmpty()); +} + +void TestSearchTerm::extendParenthesisesBothSides() +{ + // THE case this exists for. The bar may hold a hand-written disjunction, + // and `a or b AND c` binds as `a or (b AND c)`: the result WIDENS a search + // the user asked to narrow. Both sides are wrapped so neither can rebind. + QCOMPARE(SearchTerm::extend(QStringLiteral("tag:inbox or tag:flagged"), + QStringLiteral("from:foo@example.org")), + QStringLiteral("(tag:inbox or tag:flagged) AND (from:foo@example.org)")); +} + +void TestSearchTerm::extendOntoAnEmptyQueryIsAReplace() +{ + // Rather than "() AND (x)", which matches nothing. + QCOMPARE(SearchTerm::extend(QString(), QStringLiteral("tag:inbox")), + QStringLiteral("tag:inbox")); + QCOMPARE(SearchTerm::extend(QStringLiteral(" "), + QStringLiteral("tag:inbox")), + QStringLiteral("tag:inbox")); + // And an empty new term leaves the existing query alone. + QCOMPARE(SearchTerm::extend(QStringLiteral("tag:inbox"), QString()), + QStringLiteral("tag:inbox")); +} + +QTEST_MAIN(TestSearchTerm) +#include "test_searchterm.moc" -- cgit v1.2.3 From 8cf71bb617c34f52357ce0597c7d07601337828b Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Fri, 14 Aug 2026 12:45:58 +0200 Subject: refactor(mime): expose the Date: header parse as MimeParser::parseDate The date search needs it and the logic already existed inside a file-local function, including the fix for Qt::RFC2822Date rejecting a string that carries a trailing timezone comment. Extracted rather than rewritten, so the second caller cannot end up without that fix. --- src/mimeparser.cpp | 15 ++++++++++----- src/mimeparser.h | 11 +++++++++++ tests/test_mimeparser.cpp | 23 +++++++++++++++++++++++ 3 files changed, 44 insertions(+), 5 deletions(-) diff --git a/src/mimeparser.cpp b/src/mimeparser.cpp index ecf56f1..2782a4a 100644 --- a/src/mimeparser.cpp +++ b/src/mimeparser.cpp @@ -272,20 +272,25 @@ QString recipientSummary(const QString &rawTo, int maxNames) return summary; } -QString attachmentFolderName(const QString &rfc822Date, const QString &subject) +QDateTime MimeParser::parseDate(const QString &rfc822Date) { - // The date prefix sorts chronologically in a file manager. A Date: header - // that does not parse is simply dropped rather than guessed at. // A trailing timezone comment, "... +0200 (CEST)", is legal per RFC 5322 // and common in the wild, but Qt::RFC2822Date rejects the whole string // when one is present (verified on Qt 6.11). Strip comments before - // parsing, or every such message silently loses its date prefix. + // parsing, or every such message silently loses its date. QString cleaned = rfc822Date; cleaned.remove(QRegularExpression(QStringLiteral("\\s*\\([^)]*\\)"))); cleaned = cleaned.trimmed(); + return QDateTime::fromString(cleaned, Qt::RFC2822Date); +} + +QString attachmentFolderName(const QString &rfc822Date, const QString &subject) +{ + // The date prefix sorts chronologically in a file manager. A Date: header + // that does not parse is simply dropped rather than guessed at. QString prefix; - const QDateTime parsed = QDateTime::fromString(cleaned, Qt::RFC2822Date); + const QDateTime parsed = MimeParser::parseDate(rfc822Date); if (parsed.isValid()) prefix = parsed.toString(QStringLiteral("yyyy-MM-dd")); diff --git a/src/mimeparser.h b/src/mimeparser.h index 9fceb2f..da54434 100644 --- a/src/mimeparser.h +++ b/src/mimeparser.h @@ -19,6 +19,7 @@ #pragma once #include +#include #include #include #include @@ -143,4 +144,14 @@ public: MimeParser(); ParsedMessage parse(const QString &filePath) const; + + /// Parses an RFC 2822 `Date:` header, returning an invalid QDateTime when + /// nothing usable is there. + /// + /// **Strips comments before parsing**, because `Qt::RFC2822Date` rejects + /// the entire string when a trailing timezone comment is present, and + /// `... +0200 (CEST)` is legal per RFC 5322 and common in the wild + /// (verified on Qt 6.11). A parser without this silently loses the date on + /// a large share of real mail. + static QDateTime parseDate(const QString &rfc822Date); }; diff --git a/tests/test_mimeparser.cpp b/tests/test_mimeparser.cpp index bac71f6..af6307e 100644 --- a/tests/test_mimeparser.cpp +++ b/tests/test_mimeparser.cpp @@ -47,6 +47,7 @@ private slots: void recipientSummarySurvivesUnusableInput(); void folderNameSurvivesATimezoneComment(); void savingABatchNeverOverwrites(); + void parsesADateWithATimezoneComment(); private: QString fixture(const QString &name) const @@ -485,5 +486,27 @@ void TestMimeParser::recipientSummarySurvivesUnusableInput() recipientSummary(QStringLiteral("\"unterminated ")); } +void TestMimeParser::parsesADateWithATimezoneComment() +{ + // Qt::RFC2822Date rejects the WHOLE string when a trailing comment is + // present (verified on Qt 6.11), and "+0200 (CEST)" is both legal and + // common. Without the comment stripped, every such message loses its date + // silently: the attachment folder loses its prefix, and a date search + // offers nothing with no indication why. + const QDateTime withComment = MimeParser::parseDate( + QStringLiteral("Fri, 14 Aug 2026 09:30:00 +0200 (CEST)")); + QVERIFY(withComment.isValid()); + QCOMPARE(withComment.date(), QDate(2026, 8, 14)); + + const QDateTime plain = MimeParser::parseDate( + QStringLiteral("Fri, 14 Aug 2026 09:30:00 +0200")); + QVERIFY(plain.isValid()); + QCOMPARE(plain.date(), QDate(2026, 8, 14)); + + // Nothing usable is an invalid QDateTime, never a guess. + QVERIFY(!MimeParser::parseDate(QStringLiteral("last Tuesday")).isValid()); + QVERIFY(!MimeParser::parseDate(QString()).isValid()); +} + QTEST_MAIN(TestMimeParser) #include "test_mimeparser.moc" -- cgit v1.2.3 From 83f42ab460820e9fd1b96653e14c08b5d51a2166 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Fri, 14 Aug 2026 12:46:56 +0200 Subject: docs: fix a weekday in the plan's date fixtures Qt::RFC2822Date validates the weekday against the date, so 'Thu, 14 Aug 2026' parses as invalid: that day is a Friday. Task 2 hit it and Task 6 carried the same wrong data. The failure is indistinguishable from the timezone-comment trap the date parse exists to handle, so the plan now names it. --- docs/superpowers/plans/2026-08-14-search-from-message.md | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/docs/superpowers/plans/2026-08-14-search-from-message.md b/docs/superpowers/plans/2026-08-14-search-from-message.md index 56cfbc6..824ccd4 100644 --- a/docs/superpowers/plans/2026-08-14-search-from-message.md +++ b/docs/superpowers/plans/2026-08-14-search-from-message.md @@ -462,12 +462,12 @@ void TestMimeParser::parsesADateWithATimezoneComment() // silently: the attachment folder loses its prefix, and a date search // offers nothing with no indication why. const QDateTime withComment = MimeParser::parseDate( - QStringLiteral("Thu, 14 Aug 2026 09:30:00 +0200 (CEST)")); + QStringLiteral("Fri, 14 Aug 2026 09:30:00 +0200 (CEST)")); QVERIFY(withComment.isValid()); QCOMPARE(withComment.date(), QDate(2026, 8, 14)); const QDateTime plain = MimeParser::parseDate( - QStringLiteral("Thu, 14 Aug 2026 09:30:00 +0200")); + QStringLiteral("Fri, 14 Aug 2026 09:30:00 +0200")); QVERIFY(plain.isValid()); QCOMPARE(plain.date(), QDate(2026, 8, 14)); @@ -1316,7 +1316,7 @@ private: item.message.from = QStringLiteral("Sender "); item.message.to = QStringLiteral("Recipient "); item.message.cc = QStringLiteral("Copied "); - item.message.date = QStringLiteral("Thu, 14 Aug 2026 09:30:00 +0200"); + item.message.date = QStringLiteral("Fri, 14 Aug 2026 09:30:00 +0200"); item.message.messageId = QStringLiteral(""); return item; } @@ -1945,6 +1945,12 @@ to a rule already exists as save the query, then build a rule from it." ## Notes for the implementer +**`Qt::RFC2822Date` also validates the weekday against the date.** Found in Task 2: +`Thu, 14 Aug 2026` parses as INVALID because 2026-08-14 is a Friday, and the +failure looks exactly like the timezone-comment bug this feature exists to avoid. +Any `Date:` header written into a test fixture must carry the right weekday. +Check with `date -d +%A` rather than guessing. + **Where this is likely to go wrong, in order of probability:** 1. **The tag strip's spacing constant.** Task 3 requires `paintEvent`, `relayout` and `chipRectAt` to agree. If the whole-suite run in Task 3 shows a message-pane test failing, the constants disagree; fix by making all three call `chipRectAt`. -- cgit v1.2.3 From 7e1cabf49c03c1e0c2926dcfcf82adedbf98940e Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Fri, 14 Aug 2026 12:49:37 +0200 Subject: feat(tags): hit-test a chip in the tag strip chipRectAt() is the single source of a chip's geometry, used by paintEvent and by the hit test, so the drawn chip and the clickable chip cannot drift. The +N chip yields nothing: it stands for a list of tags rather than one, so there is no single value a search could be built from. --- src/tagstrip.cpp | 54 ++++++++++++++++++++++-- src/tagstrip.h | 23 +++++++++++ tests/CMakeLists.txt | 1 + tests/test_tagstrip.cpp | 108 ++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 182 insertions(+), 4 deletions(-) create mode 100644 tests/test_tagstrip.cpp diff --git a/src/tagstrip.cpp b/src/tagstrip.cpp index bad116a..7e671ce 100644 --- a/src/tagstrip.cpp +++ b/src/tagstrip.cpp @@ -18,6 +18,7 @@ #include "tagstrip.h" +#include #include #include @@ -109,6 +110,50 @@ void TagStrip::resizeEvent(QResizeEvent *event) relayout(); } +QRect TagStrip::chipRectAt(int index) const +{ + if (index < 0 || index >= m_visible.size()) + return {}; + + const QFontMetrics metrics(font()); + + // Reproduces paintEvent's own vertical placement exactly, which is derived + // from the font's height rather than from the chip's, so a chip whose text + // is shorter than the line still lands on the same baseline. + const int top = (height() - (metrics.height() + TagChip::kPaddingY * 2)) / 2; + + int x = 0; + for (int i = 0; i < index; ++i) + x += TagChip::sizeFor(metrics, m_visible.at(i)).width() + TagChip::kSpacing; + + return QRect(QPoint(x, top), TagChip::sizeFor(metrics, m_visible.at(index))); +} + +QString TagStrip::chipAt(const QPoint &point) const +{ + for (int i = 0; i < m_visible.size(); ++i) { + if (chipRectAt(i).contains(point)) + return m_visible.at(i); + } + // Deliberately nothing for the overflow chip and for empty space: the +N + // chip names a list, not a tag. + return {}; +} + +void TagStrip::contextMenuEvent(QContextMenuEvent *event) +{ + const QString tag = chipAt(event->pos()); + if (tag.isEmpty()) { + // Ignored rather than accepted, so a parent that offers its own menu + // still gets the chance to show it. + event->ignore(); + return; + } + + event->accept(); + emit tagContextMenuRequested(tag, event->globalPos()); +} + void TagStrip::paintEvent(QPaintEvent *) { if (m_visible.isEmpty()) @@ -119,12 +164,13 @@ void TagStrip::paintEvent(QPaintEvent *) const int top = (height() - (metrics.height() + TagChip::kPaddingY * 2)) / 2; int x = 0; - for (const QString &tag : m_visible) { - const QSize size = TagChip::sizeFor(metrics, tag); + for (int i = 0; i < m_visible.size(); ++i) { + const QString &tag = m_visible.at(i); + const QRect rect = chipRectAt(i); const QColor colour = m_tagColors ? m_tagColors->colourFor(tag) : TagColors().colourFor(tag); - TagChip::paint(&painter, QRect(QPoint(x, top), size), tag, colour); - x += size.width() + TagChip::kSpacing; + TagChip::paint(&painter, rect, tag, colour); + x = rect.right() + 1 + TagChip::kSpacing; } if (!m_hidden.isEmpty()) { diff --git a/src/tagstrip.h b/src/tagstrip.h index 4102bed..f59233b 100644 --- a/src/tagstrip.h +++ b/src/tagstrip.h @@ -18,10 +18,12 @@ #pragma once +#include #include #include class TagColors; +class QContextMenuEvent; /// One row of tag chips under the message pane. /// @@ -48,9 +50,30 @@ public: QStringList visibleTags() const { return m_visible; } QStringList hiddenTags() const { return m_hidden; } + /// The rect of the visible chip at `index`, empty when out of range. + /// + /// The SAME function paintEvent lays out from, so what is drawn and what + /// is clickable cannot drift. `CardDelegate::expanderRectFor` exists for + /// this reason and this follows it. + QRect chipRectAt(int index) const; + + /// The tag under `point`, empty when the point is on no chip. + /// + /// The trailing "+N" chip yields an empty string: it stands for a list of + /// tags rather than for one, so there is nothing a search could name. + QString chipAt(const QPoint &point) const; + +signals: + /// A visible chip was right-clicked. `globalPos` is where to pop a menu. + /// + /// The strip does not build the menu itself: what a tag can do belongs to + /// the window, which owns the query bar and the actions. + void tagContextMenuRequested(const QString &tag, const QPoint &globalPos); + protected: void paintEvent(QPaintEvent *event) override; void resizeEvent(QResizeEvent *event) override; + void contextMenuEvent(QContextMenuEvent *event) override; private: /// Recomputes the visible/hidden split for the current width. diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 5b0b85b..68148ee 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -55,3 +55,4 @@ add_qtmaildir_test(tagdialog) add_qtmaildir_test(tagrules) add_qtmaildir_test(rulequery) add_qtmaildir_test(searchterm) +add_qtmaildir_test(tagstrip) diff --git a/tests/test_tagstrip.cpp b/tests/test_tagstrip.cpp new file mode 100644 index 0000000..636b24b --- /dev/null +++ b/tests/test_tagstrip.cpp @@ -0,0 +1,108 @@ +/* + * 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 "tagstrip.h" + +/// The chip hit test. +/// +/// Asserted on rects from chipRectAt(), which is the SAME function paintEvent +/// lays out from, so a drawn chip and a clickable chip cannot drift apart. Not +/// asserted by rendering: a pixel probe cannot tell a chip that is drawn from a +/// chip that is drawn and clickable, and both halves have been broken +/// independently in this project before. +class TestTagStrip : public QObject +{ + Q_OBJECT +private slots: + void chipAtFindsEachVisibleTag(); + void chipAtMissesTheGapAndTheEdges(); + void chipAtIgnoresTheOverflowChip(); +}; + +void TestTagStrip::chipAtFindsEachVisibleTag() +{ + TagStrip strip; + strip.resize(600, 30); + strip.setTags({ QStringLiteral("inbox"), QStringLiteral("unread") }); + + const QStringList visible = strip.visibleTags(); + QCOMPARE(visible.size(), 2); + + // The guard: the geometry this test depends on must exist before the test + // can mean anything. A zero-width chip would make every lookup below miss + // and the test would pass for the wrong reason. + for (int i = 0; i < visible.size(); ++i) { + const QRect rect = strip.chipRectAt(i); + QVERIFY2(rect.width() > 0 && rect.height() > 0, + qPrintable(QStringLiteral("chip %1 has an empty rect").arg(i))); + QCOMPARE(strip.chipAt(rect.center()), visible.at(i)); + } +} + +void TestTagStrip::chipAtMissesTheGapAndTheEdges() +{ + TagStrip strip; + strip.resize(600, 30); + strip.setTags({ QStringLiteral("inbox"), QStringLiteral("unread") }); + QCOMPARE(strip.visibleTags().size(), 2); + + const QRect first = strip.chipRectAt(0); + const QRect second = strip.chipRectAt(1); + QVERIFY2(second.left() > first.right() + 1, + "the two chips must not touch, or there is no gap to test"); + + // Between the chips: no tag, so no menu entry rather than the nearest one. + const QPoint gap((first.right() + second.left()) / 2, first.center().y()); + QVERIFY(strip.chipAt(gap).isEmpty()); + + // Past the last chip, where the strip is empty space. + QVERIFY(strip.chipAt(QPoint(strip.width() - 1, first.center().y())).isEmpty()); +} + +void TestTagStrip::chipAtIgnoresTheOverflowChip() +{ + // The +N chip stands for a LIST of tags, not for a tag, so there is no + // single value a search could be built from. + TagStrip strip; + strip.resize(90, 30); + strip.setTags({ QStringLiteral("inbox"), QStringLiteral("unread"), + QStringLiteral("flagged"), QStringLiteral("attachment"), + QStringLiteral("replied") }); + + QVERIFY2(!strip.hiddenTags().isEmpty(), + "the strip must actually overflow, or this asserts nothing"); + QVERIFY2(!strip.visibleTags().isEmpty(), + "the strip must show at least one chip to test against"); + + // Every point across the strip either finds a VISIBLE tag or nothing. The + // overflow chip sits after the visible ones and must yield nothing. + for (int x = 0; x < strip.width(); x += 3) { + const QString found = strip.chipAt(QPoint(x, strip.height() / 2)); + if (!found.isEmpty()) + QVERIFY(strip.visibleTags().contains(found)); + } + + const QRect last = strip.chipRectAt(strip.visibleTags().size() - 1); + QVERIFY(strip.chipAt(QPoint(last.right() + 5, strip.height() / 2)).isEmpty()); +} + +QTEST_MAIN(TestTagStrip) +#include "test_tagstrip.moc" -- cgit v1.2.3 From 811bea0640dbfd27ca2c47b0288716ca608e5abb Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Fri, 14 Aug 2026 12:50:42 +0200 Subject: refactor(tags): name what the overflow chip's x actually is The loop was assigning a variable it never read, overwritten on every pass and used only after, which reads as an accumulator and is not one. The overflow chip's position is derived where it is used instead. --- src/tagstrip.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/tagstrip.cpp b/src/tagstrip.cpp index 7e671ce..565a054 100644 --- a/src/tagstrip.cpp +++ b/src/tagstrip.cpp @@ -163,17 +163,20 @@ void TagStrip::paintEvent(QPaintEvent *) const QFontMetrics metrics(font()); const int top = (height() - (metrics.height() + TagChip::kPaddingY * 2)) / 2; - int x = 0; for (int i = 0; i < m_visible.size(); ++i) { const QString &tag = m_visible.at(i); - const QRect rect = chipRectAt(i); const QColor colour = m_tagColors ? m_tagColors->colourFor(tag) : TagColors().colourFor(tag); - TagChip::paint(&painter, rect, tag, colour); - x = rect.right() + 1 + TagChip::kSpacing; + TagChip::paint(&painter, chipRectAt(i), tag, colour); } if (!m_hidden.isEmpty()) { + // After the last visible chip. right() is inclusive, so +1 makes it an + // exclusive edge before the gap is added. The overflow chip is not in + // m_visible and so has no chipRectAt() of its own. + const QRect last = chipRectAt(m_visible.size() - 1); + const int x = last.right() + 1 + TagChip::kSpacing; + const QString text = overflowText(m_hidden.size()); const QSize size = TagChip::sizeFor(metrics, text); TagChip::paint(&painter, QRect(QPoint(x, top), size), text, -- cgit v1.2.3 From f7f868ce00e01e4026be19437242537967875ce5 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Fri, 14 Aug 2026 12:53:58 +0200 Subject: feat(search): offer the header's fields for searching The menu lists what is searchable rather than hit-testing which line of a rich-text label was clicked, which breaks as soon as the label wraps. The values are collected by the pass that renders the header, so nothing parses the markup back into structure. From, To and Cc appear only for a single-message thread, sharing the condition with the header's own display: a thread's recipient differs message to message, and the menu must not offer what the header is not stating. The test fixture's Date: header named the wrong weekday, which Qt::RFC2822Date rejects outright, so no date offer would have been produced from it. --- src/messageview.cpp | 72 +++++++++++++++++++++++++++++++++ src/messageview.h | 37 +++++++++++++++++ tests/test_messageview.cpp | 99 +++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 207 insertions(+), 1 deletion(-) diff --git a/src/messageview.cpp b/src/messageview.cpp index d2be380..8a1976d 100644 --- a/src/messageview.cpp +++ b/src/messageview.cpp @@ -30,6 +30,7 @@ #include #include #include +#include #include #include #include @@ -50,6 +51,7 @@ #include "cidschemehandler.h" #include "htmlbuilder.h" #include "requestinterceptor.h" +#include "searchterm.h" #include "tagstrip.h" #include "threadcidmap.h" #include "version.h" @@ -163,6 +165,12 @@ MessageView::MessageView(QWidget *parent) m_headerLabel->setWordWrap(true); m_headerLabel->setTextInteractionFlags(Qt::TextSelectableByMouse); + // Item 85: the header's values are searchable. CustomContextMenu rather + // than an action list, since the entries depend on what is displayed. + m_headerLabel->setContextMenuPolicy(Qt::CustomContextMenu); + connect(m_headerLabel, &QWidget::customContextMenuRequested, + this, &MessageView::showHeaderContextMenu); + // To the right of the header, per the user's decision: the summary answers // "who is this from", this answers "what actually happened to it". A button // and not only a shortcut, since "everything needs a memorized key" is the @@ -428,6 +436,11 @@ QString MessageView::headerMark(Marks::Mark mark) const void MessageView::updateHeader() { + // A stale offer list must not survive either exit: the early return below + // leaves nothing on screen to search, and the normal path rebuilds it from + // scratch a few lines down. + m_headerOffers.clear(); + if (m_items.isEmpty()) { m_headerLabel->clear(); m_detailsButton->hide(); @@ -440,6 +453,30 @@ void MessageView::updateHeader() // Re: prefixes that add nothing. const QString subject = m_items.first().message.subject; + // Collected by the pass that renders the label, from the same values, so + // nothing has to parse the rendered markup back into structure. + auto elided = [](const QString &value) { + constexpr int kMaxLabel = 40; + return value.size() > kMaxLabel + ? value.left(kMaxLabel) + QStringLiteral("...") + : value; + }; + + auto offer = [this](const QString &label, const QString &query) { + if (query.isEmpty()) + return; + m_headerOffers.append({ label, query }); + }; + + offer(tr("subject \"%1\"").arg(elided(subject)), + SearchTerm::field(QStringLiteral("subject"), subject)); + + const QDateTime sent = MimeParser::parseDate(m_items.first().message.date); + if (sent.isValid()) { + offer(tr("mail from %1").arg(sent.date().toString(Qt::ISODate)), + SearchTerm::onDate(sent.date())); + } + // Item 70's marks, beside the subject and OUTSIDE the message area. The // user asked for these two only: whether the thread is flagged and whether // it carries an attachment, which are the two states worth knowing before @@ -489,6 +526,16 @@ void MessageView::updateHeader() row(tr("From:"), message.from); row(tr("To:"), message.to); row(tr("Cc:"), message.cc); + + // Only here, sharing the condition with the header's own display: for + // a real thread these differ message to message, and the details + // dialog is where they are unambiguous. + offer(tr("sender %1").arg(elided(message.from)), + SearchTerm::field(QStringLiteral("from"), message.from)); + offer(tr("recipient %1").arg(elided(message.to)), + SearchTerm::field(QStringLiteral("to"), message.to)); + offer(tr("copied to %1").arg(elided(message.cc)), + SearchTerm::field(QStringLiteral("cc"), message.cc)); } else { text += QStringLiteral("
%1") .arg(tr("%n message(s) in thread", "", m_items.size())); @@ -497,6 +544,31 @@ void MessageView::updateHeader() m_headerLabel->setText(text); } +void MessageView::addSearchEntries(QMenu *menu, const QList &offers) +{ + for (const SearchOffer &entry : offers) { + auto *sub = menu->addMenu(tr("Search for %1").arg(entry.label)); + + auto *replace = sub->addAction(tr("Search for this")); + connect(replace, &QAction::triggered, this, + [this, entry]() { emit searchRequested(entry.query, false); }); + + auto *narrow = sub->addAction(tr("Add to search")); + connect(narrow, &QAction::triggered, this, + [this, entry]() { emit searchRequested(entry.query, true); }); + } +} + +void MessageView::showHeaderContextMenu(const QPoint &pos) +{ + if (m_headerOffers.isEmpty()) + return; + + QMenu menu(this); + addSearchEntries(&menu, m_headerOffers); + menu.exec(m_headerLabel->mapToGlobal(pos)); +} + void MessageView::showDetailsDialog() { if (m_items.isEmpty()) diff --git a/src/messageview.h b/src/messageview.h index 422869a..26ed216 100644 --- a/src/messageview.h +++ b/src/messageview.h @@ -25,8 +25,10 @@ #include "htmlbuilder.h" #include "marks.h" #include "mimeparser.h" +#include "searchterm.h" class QLabel; +class QMenu; class QPushButton; class QWebEngineView; class QWebEngineProfile; @@ -127,6 +129,17 @@ public: /// is rendered or the notice is hidden. QString staleMessageId() const { return m_staleMessageId; } + /// What the header can be searched for, given what it is currently showing. + /// + /// From, To and Cc appear only for a single-message thread, which is + /// exactly when the header displays them: for a real thread the recipient + /// differs message to message and the header says only the subject and the + /// count. The menu must never offer a value the header is not stating. + /// + /// The values come from the same pass that renders the label, never from + /// parsing it back: rich text does not survive a second parse. + QList headerSearchOffers() const { return m_headerOffers; } + public slots: void toggleHtml(); void loadRemoteContent(); @@ -158,6 +171,18 @@ signals: void staleThreadRecoveryRequested(const QString &threadId, const QString &messageId); + /// The user chose a search from one of the pane's context menus. + /// + /// `extend` narrows the current query rather than replacing it. The view + /// does not know what the query bar holds and must not: the window owns + /// that field and does the combining. + /// + /// Separate from queryRequested(), which carries a gate against a link in + /// a rendered document driving the thread list. These menus are chrome + /// built by our own code from values we extracted, so they need no gate, + /// and widening the existing signal would change what that gate protects. + void searchRequested(const QString &query, bool extend); + protected: /// Turns Ctrl+wheel over the body into zoom, and Ctrl+middle-click into a /// reset. Both events are delivered to the web view's internal QQuickWidget @@ -207,6 +232,15 @@ private: /// Every attachment in the thread, in the order the messages render. QList allAttachments() const; + /// Builds and pops the header's menu at `pos`, in the label's coordinates. + void showHeaderContextMenu(const QPoint &pos); + + /// Appends a "Search for ..." submenu per offer, each holding the replace + /// and the narrow operation. + /// + /// Shared with the web view's menu in a later task so the two cannot grow + /// different wording or a different pair of operations. + void addSearchEntries(QMenu *menu, const QList &offers); QList m_items; bool m_preferHtml = true; @@ -232,4 +266,7 @@ private: QPushButton *m_detailsButton = nullptr; QWidget *m_attachmentBar = nullptr; TagStrip *m_tagStrip = nullptr; + + /// Populated by updateHeader(), consumed by the header's context menu. + QList m_headerOffers; }; diff --git a/tests/test_messageview.cpp b/tests/test_messageview.cpp index e94fb86..6403308 100644 --- a/tests/test_messageview.cpp +++ b/tests/test_messageview.cpp @@ -49,6 +49,9 @@ private slots: void detailsDialogIsOfferedForEveryThread(); void placeholderRendersAndReportsItself(); void aMessageBodyCannotRunAQuery(); + void headerOffersSubjectDateAndSenderForOneMessage(); + void headerOffersNoSenderForARealThread(); + void headerOffersNothingForAnAbsentField(); private: QWebEngineView *webViewOf(MessageView *view) const @@ -372,7 +375,7 @@ static ThreadRenderItem oneMessage() message.to = QStringLiteral("Recipient "); message.cc = QStringLiteral("Copied "); message.subject = QStringLiteral("Quarterly report"); - message.date = QStringLiteral("Mon, 4 Aug 2026 09:00:00 +0200"); + message.date = QStringLiteral("Tue, 4 Aug 2026 09:00:00 +0200"); message.plainBody = QStringLiteral("body"); ThreadRenderItem item; @@ -566,5 +569,99 @@ void TestMessageView::aMessageBodyCannotRunAQuery() QVERIFY(queries.isEmpty()); } +void TestMessageView::headerOffersSubjectDateAndSenderForOneMessage() +{ + MessageView view; + view.showThread({ oneMessage() }); + + const QList offers = view.headerSearchOffers(); + + QStringList queries; + for (const SearchOffer &offer : offers) + queries << offer.query; + const QString shown = queries.join(QStringLiteral(" | ")); + + QVERIFY2(queries.contains(QStringLiteral("subject:\"Quarterly report\"")), + qPrintable(shown)); + QVERIFY2(queries.contains( + QStringLiteral("from:\"Sender \"")), + qPrintable(shown)); + QVERIFY2(queries.contains( + QStringLiteral("to:\"Recipient \"")), + qPrintable(shown)); + QVERIFY2(queries.contains( + QStringLiteral("cc:\"Copied \"")), + qPrintable(shown)); + + // The date is offered as a one-day range. Qt::RFC2822Date checks the + // weekday against the date, so a fixture with the wrong day silently + // produces no offer at all: 2026-08-04 is a Tuesday. + QVERIFY2(queries.contains(QStringLiteral("date:2026-08-04..2026-08-04")), + qPrintable(shown)); + + // Every offer carries a label the menu shows, naming the value so the user + // can see what they are about to search for. + for (const SearchOffer &offer : offers) { + QVERIFY(!offer.label.isEmpty()); + QVERIFY(!offer.query.isEmpty()); + } +} + +void TestMessageView::headerOffersNoSenderForARealThread() +{ + // The header shows From/To/Cc only for a single-message thread, because a + // thread's recipient differs message to message. The menu shares that + // condition: it must never offer a value the header is not stating. + ThreadRenderItem first = oneMessage(); + ThreadRenderItem second = oneMessage(); + second.message.from = QStringLiteral("Recipient "); + second.message.to = QStringLiteral("Sender "); + + MessageView view; + view.showThread({ first, second }); + + QStringList queries; + for (const SearchOffer &offer : view.headerSearchOffers()) + queries << offer.query; + + // THE GUARD. A test asserting only that something is absent passes against + // no implementation whatever. Subject and date must still be offered, + // which proves the list was built before the absences below mean anything. + QVERIFY2(!queries.isEmpty(), "no offers at all: the list was never built"); + QVERIFY(queries.contains(QStringLiteral("subject:\"Quarterly report\""))); + QVERIFY(queries.contains(QStringLiteral("date:2026-08-04..2026-08-04"))); + + for (const QString &query : queries) { + QVERIFY2(!query.startsWith(QStringLiteral("from:")), + qPrintable(QStringLiteral("thread offered %1").arg(query))); + QVERIFY2(!query.startsWith(QStringLiteral("to:")), + qPrintable(QStringLiteral("thread offered %1").arg(query))); + QVERIFY2(!query.startsWith(QStringLiteral("cc:")), + qPrintable(QStringLiteral("thread offered %1").arg(query))); + } +} + +void TestMessageView::headerOffersNothingForAnAbsentField() +{ + // cc:"" parses cleanly and matches nothing, so an entry built from an + // empty header would look enabled and silently do nothing. + ThreadRenderItem item = oneMessage(); + item.message.cc.clear(); + + MessageView view; + view.showThread({ item }); + + QStringList queries; + for (const SearchOffer &offer : view.headerSearchOffers()) + queries << offer.query; + + // Guard first, then the absence. + QVERIFY2(!queries.isEmpty(), "no offers at all: the list was never built"); + QVERIFY(queries.contains(QStringLiteral("subject:\"Quarterly report\""))); + + for (const QString &query : queries) + QVERIFY(!query.startsWith(QStringLiteral("cc:"))); +} + QTEST_MAIN(TestMessageView) #include "test_messageview.moc" -- cgit v1.2.3 From 8a2e02a3e1a669a738d1de4615720378d5cc2d38 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Fri, 14 Aug 2026 12:56:53 +0200 Subject: feat(search): search for the selected body text selectedText() reads the selection with no script injection; JavaScript stays disabled in the profile. The page's standard menu is kept and the entries are added to it below a separator. The quoting is tested through a function taking the text, so it needs no live web engine: a selection is arbitrary prose and can carry quotes, newlines and query syntax, none of which notmuch reports as an error. --- src/messageview.cpp | 42 ++++++++++++++++++++++++++++++++++++++++++ src/messageview.h | 12 ++++++++++++ tests/test_messageview.cpp | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+) diff --git a/src/messageview.cpp b/src/messageview.cpp index 8a1976d..5f7c9d2 100644 --- a/src/messageview.cpp +++ b/src/messageview.cpp @@ -153,6 +153,12 @@ MessageView::MessageView(QWidget *parent) settings->setAttribute(QWebEngineSettings::PluginsEnabled, false); settings->setAttribute(QWebEngineSettings::FullScreenSupportEnabled, false); + // Item 85: a selection in the body is searchable. CustomContextMenu so the + // page's standard entries survive and the search is added to them. + m_view->setContextMenuPolicy(Qt::CustomContextMenu); + connect(m_view, &QWidget::customContextMenuRequested, + this, &MessageView::showBodyContextMenu); + // Ctrl+wheel zoom. The filter goes on the application rather than on // m_view: the wheel event is delivered to an internal QQuickWidget the // view creates lazily, so there is no child to filter at this point and a @@ -569,6 +575,42 @@ void MessageView::showHeaderContextMenu(const QPoint &pos) menu.exec(m_headerLabel->mapToGlobal(pos)); } +SearchOffer MessageView::selectionSearchOffer(const QString &selectedText) const +{ + const QString query = SearchTerm::quote(selectedText); + if (query.isEmpty()) + return {}; + + constexpr int kMaxLabel = 40; + const QString shown = selectedText.simplified(); + return { shown.size() > kMaxLabel + ? shown.left(kMaxLabel) + QStringLiteral("...") + : shown, + query }; +} + +void MessageView::showBodyContextMenu(const QPoint &pos) +{ + // The page's own menu first: copy, select all and the rest stay exactly as + // they were. This adds to that menu rather than replacing it. + QMenu *menu = m_view->createStandardContextMenu(); + if (!menu) + menu = new QMenu(this); + menu->setAttribute(Qt::WA_DeleteOnClose); + + // selectedText() reads the selection out of the render process with no + // script injection. JavaScript is disabled in this profile and stays so. + const SearchOffer offer = selectionSearchOffer(m_view->page()->selectedText()); + if (!offer.query.isEmpty()) { + menu->addSeparator(); + addSearchEntries(menu, { offer }); + } + + // popup() rather than exec(): the menu owns itself via WA_DeleteOnClose and + // must not block this handler. + menu->popup(m_view->mapToGlobal(pos)); +} + void MessageView::showDetailsDialog() { if (m_items.isEmpty()) diff --git a/src/messageview.h b/src/messageview.h index 26ed216..3e600f0 100644 --- a/src/messageview.h +++ b/src/messageview.h @@ -140,6 +140,15 @@ public: /// parsing it back: rich text does not survive a second parse. QList headerSearchOffers() const { return m_headerOffers; } + /// The offer for a body selection, its query empty when there is nothing + /// usable selected. + /// + /// Takes the text rather than reading the page, so the quoting is testable + /// without a live web engine and a rendered document. A selection is + /// arbitrary prose and can carry quotes, newlines and query syntax, none + /// of which notmuch reports as an error. + SearchOffer selectionSearchOffer(const QString &selectedText) const; + public slots: void toggleHtml(); void loadRemoteContent(); @@ -235,6 +244,9 @@ private: /// Builds and pops the header's menu at `pos`, in the label's coordinates. void showHeaderContextMenu(const QPoint &pos); + /// Builds and pops the web view's menu, keeping its standard entries. + void showBodyContextMenu(const QPoint &pos); + /// Appends a "Search for ..." submenu per offer, each holding the replace /// and the narrow operation. /// diff --git a/tests/test_messageview.cpp b/tests/test_messageview.cpp index 6403308..f83a377 100644 --- a/tests/test_messageview.cpp +++ b/tests/test_messageview.cpp @@ -52,6 +52,7 @@ private slots: void headerOffersSubjectDateAndSenderForOneMessage(); void headerOffersNoSenderForARealThread(); void headerOffersNothingForAnAbsentField(); + void bodySelectionBecomesAQuotedSearch(); private: QWebEngineView *webViewOf(MessageView *view) const @@ -663,5 +664,38 @@ void TestMessageView::headerOffersNothingForAnAbsentField() QVERIFY(!query.startsWith(QStringLiteral("cc:"))); } +void TestMessageView::bodySelectionBecomesAQuotedSearch() +{ + // The selection reaches the query as ONE quoted term. Asserted on the + // constructed string: a query that lost its quoting is not an error to + // notmuch, it simply matches nothing, so nothing downstream would report + // this being wrong. + // + // Takes the text as an argument rather than reading the page, so the + // quoting is testable without a live web engine and a rendered document. + MessageView view; + + QCOMPARE(view.selectionSearchOffer(QStringLiteral("invoice 4471")).query, + QStringLiteral("\"invoice 4471\"")); + + // A selection spanning paragraphs arrives full of newlines. + QCOMPARE(view.selectionSearchOffer( + QStringLiteral("first line\n\nsecond line")).query, + QStringLiteral("\"first line second line\"")); + + // Query syntax in the selection is data, not syntax: it is quoted, not + // interpreted, so a selection reading "a or b" searches for that phrase. + QCOMPARE(view.selectionSearchOffer(QStringLiteral("tag:inbox or x")).query, + QStringLiteral("\"tag:inbox or x\"")); + + // Nothing selected means no entry, rather than an entry searching for "". + QVERIFY(view.selectionSearchOffer(QString()).query.isEmpty()); + QVERIFY(view.selectionSearchOffer(QStringLiteral(" \n ")).query.isEmpty()); + + // A usable offer always carries a label for the menu to show. + QVERIFY(!view.selectionSearchOffer(QStringLiteral("invoice 4471")) + .label.isEmpty()); +} + QTEST_MAIN(TestMessageView) #include "test_messageview.moc" -- cgit v1.2.3 From 2f124b22920fb290684206a2c905996f1c378fd7 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Fri, 14 Aug 2026 13:01:04 +0200 Subject: feat(details): rebuild the message details dialog as rows A text box could not carry a per-value context menu without parsing displayed text back into structure, and the user did not want a text box. Each row now holds its own value, its message index and its query, built from the parsed message. Every value label states Qt::PlainText. The QPlainTextEdit this replaced was plain by design rather than by style: header values come from strangers, and a QLabel guesses the format under AutoText. --- src/CMakeLists.txt | 1 + src/messagedetailsdialog.cpp | 153 ++++++++++++++++++++++++++++++ src/messagedetailsdialog.h | 87 +++++++++++++++++ src/messageview.cpp | 55 ++--------- tests/CMakeLists.txt | 1 + tests/test_messagedetailsdialog.cpp | 182 ++++++++++++++++++++++++++++++++++++ 6 files changed, 430 insertions(+), 49 deletions(-) create mode 100644 src/messagedetailsdialog.cpp create mode 100644 src/messagedetailsdialog.h create mode 100644 tests/test_messagedetailsdialog.cpp diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 382f3b8..ee1f621 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -22,6 +22,7 @@ add_library(qtmaildir_lib STATIC syncmonitor.cpp threadcidmap.cpp messageview.cpp + messagedetailsdialog.cpp mainwindow.cpp querycompleter.cpp rulequery.cpp diff --git a/src/messagedetailsdialog.cpp b/src/messagedetailsdialog.cpp new file mode 100644 index 0000000..058814b --- /dev/null +++ b/src/messagedetailsdialog.cpp @@ -0,0 +1,153 @@ +/* + * 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 "messagedetailsdialog.h" + +#include +#include +#include +#include +#include +#include +#include + +#include "mimeparser.h" +#include "searchterm.h" + +MessageDetailsDialog::MessageDetailsDialog(const QList &items, + QWidget *parent) + : QDialog(parent) +{ + setWindowTitle(tr("Message details")); + setObjectName(QStringLiteral("messageDetailsDialog")); + + buildRows(items); + + auto *layout = new QVBoxLayout(this); + + // Scrollable: a long thread has many rows, and the dialog must not grow + // past the screen to show them. + auto *scroll = new QScrollArea(this); + scroll->setWidgetResizable(true); + auto *content = new QWidget(scroll); + auto *grid = new QGridLayout(content); + + // A monospaced value keeps a long id or address readable as the record it + // is, which is what the text box did well and is worth carrying over. + const QFont fixed = QFontDatabase::systemFont(QFontDatabase::FixedFont); + + int gridRow = 0; + int lastMessage = -1; + for (const HeaderRow &row : std::as_const(m_rows)) { + if (items.size() > 1 && row.messageIndex != lastMessage) { + lastMessage = row.messageIndex; + auto *heading = new QLabel( + tr("Message %1 of %2").arg(row.messageIndex + 1) + .arg(items.size()), + content); + heading->setTextFormat(Qt::PlainText); + QFont headingFont = heading->font(); + headingFont.setBold(true); + heading->setFont(headingFont); + grid->addWidget(heading, gridRow, 0, 1, 2); + ++gridRow; + } + + auto *label = new QLabel(row.label, content); + label->setTextFormat(Qt::PlainText); + label->setAlignment(Qt::AlignTop | Qt::AlignLeft); + grid->addWidget(label, gridRow, 0); + + // PlainText stated, not inferred. A QLabel guesses under AutoText, and + // this value came from a stranger. + auto *value = new QLabel(row.value, content); + value->setTextFormat(Qt::PlainText); + value->setFont(fixed); + value->setWordWrap(true); + value->setTextInteractionFlags(Qt::TextSelectableByMouse); + + if (!row.query.isEmpty()) { + value->setContextMenuPolicy(Qt::CustomContextMenu); + connect(value, &QWidget::customContextMenuRequested, this, + [this, value, row](const QPoint &pos) { + QMenu menu(this); + auto *replace = menu.addAction(tr("Search for this")); + connect(replace, &QAction::triggered, this, + [this, row]() { requestSearch(row, false); }); + auto *narrow = menu.addAction(tr("Add to search")); + connect(narrow, &QAction::triggered, this, + [this, row]() { requestSearch(row, true); }); + menu.exec(value->mapToGlobal(pos)); + }); + } + + grid->addWidget(value, gridRow, 1); + ++gridRow; + } + + grid->setColumnStretch(1, 1); + grid->setRowStretch(gridRow, 1); + scroll->setWidget(content); + layout->addWidget(scroll); + + auto *buttons = new QDialogButtonBox(QDialogButtonBox::Close, this); + connect(buttons, &QDialogButtonBox::rejected, this, &QDialog::reject); + layout->addWidget(buttons); + + resize(700, 400); +} + +void MessageDetailsDialog::buildRows(const QList &items) +{ + for (int i = 0; i < items.size(); ++i) { + const ParsedMessage &message = items.at(i).message; + + auto add = [this, i](const QString &field, const QString &label, + const QString &value, const QString &query) { + if (value.isEmpty()) + return; // An empty row reads as a rendering fault. + m_rows.append({ field, label, value, query, i }); + }; + + add(QStringLiteral("subject"), tr("Subject:"), message.subject, + SearchTerm::field(QStringLiteral("subject"), message.subject)); + add(QStringLiteral("from"), tr("From:"), message.from, + SearchTerm::field(QStringLiteral("from"), message.from)); + add(QStringLiteral("to"), tr("To:"), message.to, + SearchTerm::field(QStringLiteral("to"), message.to)); + add(QStringLiteral("cc"), tr("Cc:"), message.cc, + SearchTerm::field(QStringLiteral("cc"), message.cc)); + + // The raw header is shown, but the query is a one-day range: a text + // match on an RFC 2822 string would match almost nothing. + const QDateTime sent = MimeParser::parseDate(message.date); + add(QStringLiteral("date"), tr("Date:"), message.date, + sent.isValid() ? SearchTerm::onDate(sent.date()) : QString()); + + // Shown but not searchable: an id names one message, and the thread + // holding it is already on screen. + add(QString(), tr("Message-Id:"), message.messageId, QString()); + } +} + +void MessageDetailsDialog::requestSearch(const HeaderRow &row, bool extend) +{ + if (row.query.isEmpty()) + return; + emit searchRequested(row.query, extend); +} diff --git a/src/messagedetailsdialog.h b/src/messagedetailsdialog.h new file mode 100644 index 0000000..f865913 --- /dev/null +++ b/src/messagedetailsdialog.h @@ -0,0 +1,87 @@ +/* + * 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. + */ + +#pragma once + +#include +#include +#include + +#include "htmlbuilder.h" + +/// One header of one message, as shown and as searched for. +/// +/// The query is built when the row is, from the parsed value, so nothing has +/// to parse displayed text back into structure. That is the whole reason this +/// dialog stopped being a text box. +struct HeaderRow +{ + /// notmuch's field name, or empty for a header with no searchable form. + /// Wire format, never translated. + QString field; + + /// Translated label shown at the start of the row, e.g. "From:". + QString label; + + /// The header's value, verbatim and untrusted. + QString value; + + /// The finished query, empty when the header has no searchable form. + QString query; + + /// Which message of the thread this row belongs to, zero-based. + int messageIndex = 0; +}; + +/// The full headers of every message in a thread, read-only. +/// +/// Rows rather than one text box, so a value can carry its own context menu +/// without anything parsing rendered text back into structure. The user also +/// asked not to be shown a text box. +/// +/// **Every value label is explicitly `Qt::PlainText`.** This replaced a +/// `QPlainTextEdit` whose plain-textness was a security property rather than a +/// style: header values come from strangers, and plain text cannot interpret +/// markup, so there is nothing to escape and nothing that can render. A QLabel +/// guesses under `Qt::AutoText`, so stating the format is what preserves that. +class MessageDetailsDialog : public QDialog +{ + Q_OBJECT +public: + explicit MessageDetailsDialog(const QList &items, + QWidget *parent = nullptr); + + /// The rows on display, in order. Exposed for testing without rendering. + QList rows() const { return m_rows; } + + /// Emits searchRequested for `row`, or nothing when the row carries no + /// searchable query. The menu entries call this; a test can too, without + /// popping a menu. + void requestSearch(const HeaderRow &row, bool extend); + +signals: + /// The user chose a search from a row's menu. `extend` narrows the current + /// query rather than replacing it. + void searchRequested(const QString &query, bool extend); + +private: + /// Builds the rows from the thread, one group per message. + void buildRows(const QList &items); + + QList m_rows; +}; diff --git a/src/messageview.cpp b/src/messageview.cpp index 5f7c9d2..fbd43d6 100644 --- a/src/messageview.cpp +++ b/src/messageview.cpp @@ -22,8 +22,6 @@ #include #include #include -#include -#include #include #include #include @@ -50,6 +48,7 @@ #include "cidschemehandler.h" #include "htmlbuilder.h" +#include "messagedetailsdialog.h" #include "requestinterceptor.h" #include "searchterm.h" #include "tagstrip.h" @@ -616,53 +615,11 @@ void MessageView::showDetailsDialog() if (m_items.isEmpty()) return; - QDialog dialog(this); - dialog.setWindowTitle(tr("Message details")); - - auto *layout = new QVBoxLayout(&dialog); - - auto *details = new QPlainTextEdit(&dialog); - details->setReadOnly(true); - // A monospaced font keeps a long Received chain readable as the wrapped - // record it is. - details->setFont(QFontDatabase::systemFont(QFontDatabase::FixedFont)); - details->setLineWrapMode(QPlainTextEdit::NoWrap); - - // setPlainText, and a QPlainTextEdit rather than a label: this dialog shows - // header values verbatim, and those come from strangers. Plain text cannot - // interpret markup, so there is nothing here to escape and nothing that - // could render. - QString text; - for (int i = 0; i < m_items.size(); ++i) { - const ParsedMessage &message = m_items.at(i).message; - - if (i > 0) - text += QLatin1Char('\n'); - if (m_items.size() > 1) - text += tr("--- Message %1 of %2 ---") - .arg(i + 1).arg(m_items.size()) + QLatin1Char('\n'); - - auto line = [&text](const QString &label, const QString &value) { - if (!value.isEmpty()) - text += label + QLatin1Char(' ') + value + QLatin1Char('\n'); - }; - - line(tr("Subject:"), message.subject); - line(tr("From:"), message.from); - line(tr("To:"), message.to); - line(tr("Cc:"), message.cc); - line(tr("Date:"), message.date); - line(tr("Message-Id:"), message.messageId); - } - details->setPlainText(text); - - layout->addWidget(details); - - auto *buttons = new QDialogButtonBox(QDialogButtonBox::Close, &dialog); - connect(buttons, &QDialogButtonBox::rejected, &dialog, &QDialog::reject); - layout->addWidget(buttons); - - dialog.resize(700, 400); + MessageDetailsDialog dialog(m_items, this); + // The dialog's searches are the pane's searches: one signal reaches the + // window whichever surface the user used. + connect(&dialog, &MessageDetailsDialog::searchRequested, + this, &MessageView::searchRequested); dialog.exec(); } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 68148ee..5f7bd48 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -56,3 +56,4 @@ add_qtmaildir_test(tagrules) add_qtmaildir_test(rulequery) add_qtmaildir_test(searchterm) add_qtmaildir_test(tagstrip) +add_qtmaildir_test(messagedetailsdialog) diff --git a/tests/test_messagedetailsdialog.cpp b/tests/test_messagedetailsdialog.cpp new file mode 100644 index 0000000..4e685ce --- /dev/null +++ b/tests/test_messagedetailsdialog.cpp @@ -0,0 +1,182 @@ +/* + * 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 "htmlbuilder.h" +#include "messagedetailsdialog.h" + +/// The details dialog, which shows every header of every message in a thread. +/// +/// Rows rather than one text box since item 85, so a value can carry its own +/// context menu without anything parsing rendered text back into structure. +class TestMessageDetailsDialog : public QObject +{ + Q_OBJECT +private slots: + void showsEveryHeaderOfEveryMessage(); + void valueLabelsCannotRenderMarkup(); + void offersASearchForEachValue(); + void omitsAnEmptyHeader(); + void messageIdIsShownButNotSearchable(); + +private: + /// One message, with every header populated. The date's weekday matches + /// the date: Qt::RFC2822Date validates the two against each other, and + /// 2026-08-14 is a Friday. + ThreadRenderItem oneMessage() const + { + ThreadRenderItem item; + item.message.ok = true; + item.message.subject = QStringLiteral("Quarterly report"); + item.message.from = QStringLiteral("Sender "); + item.message.to = QStringLiteral("Recipient "); + item.message.cc = QStringLiteral("Copied "); + item.message.date = QStringLiteral("Fri, 14 Aug 2026 09:30:00 +0200"); + item.message.messageId = QStringLiteral(""); + return item; + } +}; + +void TestMessageDetailsDialog::showsEveryHeaderOfEveryMessage() +{ + ThreadRenderItem second = oneMessage(); + second.message.subject = QStringLiteral("Re: Quarterly report"); + + MessageDetailsDialog dialog({ oneMessage(), second }); + + const QList rows = dialog.rows(); + QVERIFY2(!rows.isEmpty(), "no rows: the dialog was never populated"); + + // Both messages are represented, each row knowing which one it belongs to. + QVERIFY(std::any_of(rows.cbegin(), rows.cend(), [](const HeaderRow &row) { + return row.messageIndex == 0; + })); + QVERIFY(std::any_of(rows.cbegin(), rows.cend(), [](const HeaderRow &row) { + return row.messageIndex == 1; + })); + + QStringList values; + for (const HeaderRow &row : rows) + values << row.value; + QVERIFY(values.contains(QStringLiteral("Sender "))); + QVERIFY(values.contains(QStringLiteral("Re: Quarterly report"))); + QVERIFY(values.contains(QStringLiteral(""))); +} + +void TestMessageDetailsDialog::valueLabelsCannotRenderMarkup() +{ + // The QPlainTextEdit this replaced was plain by DESIGN, not by style: + // header values come from strangers and plain text cannot interpret + // markup. A QLabel guesses under Qt::AutoText, so every label states its + // format rather than relying on escaping, which is the same protection one + // mistake away from failing. + ThreadRenderItem hostile = oneMessage(); + hostile.message.subject = + QStringLiteral("bold"); + + MessageDetailsDialog dialog({ hostile }); + + const QList labels = dialog.findChildren(); + QVERIFY2(!labels.isEmpty(), "no labels: the dialog was never populated"); + + bool sawTheSubject = false; + for (const QLabel *label : labels) { + QCOMPARE(label->textFormat(), Qt::PlainText); + if (label->text().contains(QStringLiteral("bold"))) + sawTheSubject = true; + } + + // The markup survives AS TEXT, which is the proof it was not interpreted. + QVERIFY2(sawTheSubject, "the hostile subject never reached a label"); +} + +void TestMessageDetailsDialog::offersASearchForEachValue() +{ + MessageDetailsDialog dialog({ oneMessage() }); + + QSignalSpy spy(&dialog, &MessageDetailsDialog::searchRequested); + QVERIFY(spy.isValid()); + + const QList rows = dialog.rows(); + const auto from = std::find_if( + rows.cbegin(), rows.cend(), [](const HeaderRow &row) { + return row.field == QStringLiteral("from"); + }); + QVERIFY2(from != rows.cend(), "no From row to search from"); + QCOMPARE(from->query, QStringLiteral("from:\"Sender \"")); + + // The date becomes a one-day range rather than a text match on the header. + const auto date = std::find_if( + rows.cbegin(), rows.cend(), [](const HeaderRow &row) { + return row.field == QStringLiteral("date"); + }); + QVERIFY2(date != rows.cend(), "no Date row"); + QCOMPARE(date->query, QStringLiteral("date:2026-08-14..2026-08-14")); + + // Replacing and narrowing are both offered, and the flag distinguishes them. + dialog.requestSearch(*from, false); + dialog.requestSearch(*from, true); + QCOMPARE(spy.count(), 2); + QCOMPARE(spy.at(0).at(0).toString(), from->query); + QCOMPARE(spy.at(0).at(1).toBool(), false); + QCOMPARE(spy.at(1).at(1).toBool(), true); +} + +void TestMessageDetailsDialog::omitsAnEmptyHeader() +{ + ThreadRenderItem noCc = oneMessage(); + noCc.message.cc.clear(); + + MessageDetailsDialog dialog({ noCc }); + + const QList rows = dialog.rows(); + // Guard first: an absence assertion alone passes against no implementation. + QVERIFY2(!rows.isEmpty(), "no rows: the dialog was never populated"); + QVERIFY(std::any_of(rows.cbegin(), rows.cend(), [](const HeaderRow &row) { + return row.field == QStringLiteral("from"); + })); + + for (const HeaderRow &row : rows) + QVERIFY(row.field != QStringLiteral("cc")); +} + +void TestMessageDetailsDialog::messageIdIsShownButNotSearchable() +{ + // A message id names one message, and the thread holding it is already on + // screen, so there is nothing useful to search for. It is still shown. + MessageDetailsDialog dialog({ oneMessage() }); + + const QList rows = dialog.rows(); + const auto id = std::find_if( + rows.cbegin(), rows.cend(), [](const HeaderRow &row) { + return row.value == QStringLiteral(""); + }); + QVERIFY2(id != rows.cend(), "the message id is not shown at all"); + QVERIFY(id->query.isEmpty()); + + // And asking to search it emits nothing rather than an empty query. + QSignalSpy spy(&dialog, &MessageDetailsDialog::searchRequested); + dialog.requestSearch(*id, false); + QCOMPARE(spy.count(), 0); +} + +QTEST_MAIN(TestMessageDetailsDialog) +#include "test_messagedetailsdialog.moc" -- cgit v1.2.3 From 5b18bc2123d8bf0830ebf8ab3f735c9496342e97 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Fri, 14 Aug 2026 13:36:11 +0200 Subject: feat(search): run a search asked for from the message pane The panes carry a finished query and know nothing of the query bar; the window sets the field and calls the existing runner, so the account scope and the generation counter keep working as they do for a typed query. Narrowing combines here rather than in a pane, because only the window can see what the bar currently holds. The tag strip's chips join the header, the body selection and the details dialog as a fourth surface. Also fixes the details dialog to actually close when a search is chosen: the comment above the connection already described this requirement, but nothing called accept() or reject(), so the dialog stayed open, the query ran behind it, and the modal exec() never returned. This hung the whole test suite on QT_QPA_PLATFORM=offscreen once a covering test was added. --- src/mainwindow.cpp | 18 +++++++++++++ src/mainwindow.h | 7 ++++++ src/messageview.cpp | 30 ++++++++++++++++++++-- tests/test_mainwindow.cpp | 63 ++++++++++++++++++++++++++++++++++++++++++++++ tests/test_messageview.cpp | 53 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 169 insertions(+), 2 deletions(-) diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index bb6aa43..115646e 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -55,6 +55,7 @@ #include "querycompleter.h" #include "carddelegate.h" #include "cardlayout.h" +#include "searchterm.h" #include "tagchip.h" #include "tagdialog.h" #include "savequerydialog.h" @@ -672,6 +673,8 @@ void MainWindow::buildUi() this, &MainWindow::onPlaceholderQueryRequested); connect(m_messageView, &MessageView::staleThreadRecoveryRequested, this, &MainWindow::recoverStaleThread); + connect(m_messageView, &MessageView::searchRequested, + this, &MainWindow::runSearchFromPane); m_splitter = new QSplitter(Qt::Horizontal, central); m_splitter->addWidget(m_threadView); @@ -1648,6 +1651,21 @@ void MainWindow::onPlaceholderQueryRequested(const QString &query) runCurrentQuery(); } +void MainWindow::runSearchFromPane(const QString &query, bool extend) +{ + if (query.isEmpty()) + return; + + const QString next = + extend ? SearchTerm::extend(m_queryEdit->text(), query) : query; + + // Through the query bar and the existing runner, so the account scope, the + // generation counter and the flat-mode reset all behave exactly as they do + // for a typed query. Nothing here builds a second query path. + m_queryEdit->setText(next); + runCurrentQuery(); +} + void MainWindow::showWarnings() { const QStringList warnings = m_config.warnings() + m_keyMap.warnings(); diff --git a/src/mainwindow.h b/src/mainwindow.h index a68fcd9..f54a889 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -373,6 +373,13 @@ private slots: /// Runs a query the user clicked on the placeholder pane. void onPlaceholderQueryRequested(const QString &query); + /// Runs a search asked for from the message pane. + /// + /// `extend` narrows the current query rather than replacing it. The panes + /// carry a finished query and no knowledge of the bar; the combining + /// happens here, because only the window can see what the bar holds. + void runSearchFromPane(const QString &query, bool extend); + /// Runs one tagging rule's query in the thread list, so the user can see /// which mail it collects. The rules dialog stays open; the point is to /// compare the rule against its results. diff --git a/src/messageview.cpp b/src/messageview.cpp index fbd43d6..1e6c256 100644 --- a/src/messageview.cpp +++ b/src/messageview.cpp @@ -252,6 +252,20 @@ MessageView::MessageView(QWidget *parent) m_tagStrip = new TagStrip(this); m_tagStrip->hide(); + // Item 85: a tag chip is searchable. The strip reports which chip was hit + // and where; what a tag can do is decided here, beside the other menus, so + // all three surfaces offer the same pair of operations. + connect(m_tagStrip, &TagStrip::tagContextMenuRequested, this, + [this](const QString &tag, const QPoint &globalPos) { + const QString query = SearchTerm::tag(tag); + if (query.isEmpty()) + return; + + QMenu menu(this); + addSearchEntries(&menu, { { tr("tag %1").arg(tag), query } }); + menu.exec(globalPos); + }); + auto *layout = new QVBoxLayout(this); layout->addLayout(headerRow); layout->addLayout(blockedRow); @@ -616,10 +630,22 @@ void MessageView::showDetailsDialog() return; MessageDetailsDialog dialog(m_items, this); + // The dialog's searches are the pane's searches: one signal reaches the // window whichever surface the user used. - connect(&dialog, &MessageDetailsDialog::searchRequested, - this, &MessageView::searchRequested); + // + // It CLOSES on the way out, and that is not tidiness. The dialog is modal, + // so without this the query runs and the thread list repaints behind a + // window the user still has to dismiss, making the search look like it did + // nothing. The dialog is also built from m_items, which the new query is + // about to replace, so what it displays would describe a thread the pane + // has already stopped showing. + connect(&dialog, &MessageDetailsDialog::searchRequested, this, + [this, &dialog](const QString &query, bool extend) { + emit searchRequested(query, extend); + dialog.accept(); + }); + dialog.exec(); } diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 6e300bf..8b60e37 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -88,6 +88,9 @@ private slots: void markReadTimerIsNotArmedForAReadThread(); void aConfirmedEditArmsTheAutoSync(); void autoSyncDebouncesABurstOfEdits(); + void aSearchFromThePaneReplacesTheQuery(); + void aSearchFromThePaneCanNarrowTheQuery(); + void narrowingAnEmptyQueryBarIsAPlainSearch(); void autoSyncIsNotArmedWhenDisabledOrWithNothingPending(); void autoSyncSkipsWhileABackgroundSyncIsRunning(); void aSuccessfulSyncRefreshesRatherThanRerunningTheQuery(); @@ -3725,6 +3728,66 @@ void TestMainWindow::autoSyncDebouncesABurstOfEdits() 1); } +void TestMainWindow::aSearchFromThePaneReplacesTheQuery() +{ + const Config config; + MainWindow window(config); + + QLineEdit *queryEdit = + window.findChild(QStringLiteral("queryEdit")); + QVERIFY2(queryEdit, "no query bar: the window was never built"); + + MessageView *view = window.findChild(); + QVERIFY2(view, "no message view"); + + queryEdit->setText(QStringLiteral("tag:inbox")); + emit view->searchRequested(QStringLiteral("from:\"foo@example.org\""), false); + + QCOMPARE(queryEdit->text(), QStringLiteral("from:\"foo@example.org\"")); +} + +void TestMainWindow::aSearchFromThePaneCanNarrowTheQuery() +{ + // The case the feature exists for: a query returning a thousand threads is + // narrowed by adding a condition. BOTH sides are parenthesised, because + // 'a or b AND c' binds as 'a or (b AND c)', which WIDENS a search the user + // asked to narrow, and notmuch reports no error for it. + const Config config; + MainWindow window(config); + + QLineEdit *queryEdit = + window.findChild(QStringLiteral("queryEdit")); + QVERIFY2(queryEdit, "no query bar: the window was never built"); + + MessageView *view = window.findChild(); + QVERIFY2(view, "no message view"); + + queryEdit->setText(QStringLiteral("tag:inbox or tag:flagged")); + emit view->searchRequested(QStringLiteral("from:\"foo@example.org\""), true); + + QCOMPARE(queryEdit->text(), + QStringLiteral("(tag:inbox or tag:flagged) AND (from:\"foo@example.org\")")); +} + +void TestMainWindow::narrowingAnEmptyQueryBarIsAPlainSearch() +{ + // Rather than "() AND (x)", which matches nothing. + const Config config; + MainWindow window(config); + + QLineEdit *queryEdit = + window.findChild(QStringLiteral("queryEdit")); + QVERIFY2(queryEdit, "no query bar: the window was never built"); + + MessageView *view = window.findChild(); + QVERIFY2(view, "no message view"); + + queryEdit->clear(); + emit view->searchRequested(QStringLiteral("tag:inbox"), true); + + QCOMPARE(queryEdit->text(), QStringLiteral("tag:inbox")); +} + void TestMainWindow::autoSyncIsNotArmedWhenDisabledOrWithNothingPending() { // A negative delay is the switch that restores the pre-0.16.0 behaviour, so diff --git a/tests/test_messageview.cpp b/tests/test_messageview.cpp index f83a377..edcba03 100644 --- a/tests/test_messageview.cpp +++ b/tests/test_messageview.cpp @@ -24,6 +24,7 @@ #include #include "htmlbuilder.h" +#include "messagedetailsdialog.h" #include "messageview.h" #include "mimeparser.h" @@ -53,6 +54,7 @@ private slots: void headerOffersNoSenderForARealThread(); void headerOffersNothingForAnAbsentField(); void bodySelectionBecomesAQuotedSearch(); + void aSearchFromTheDetailsDialogClosesIt(); private: QWebEngineView *webViewOf(MessageView *view) const @@ -697,5 +699,56 @@ void TestMessageView::bodySelectionBecomesAQuotedSearch() .label.isEmpty()); } +void TestMessageView::aSearchFromTheDetailsDialogClosesIt() +{ + // The dialog is modal. Without closing it, the query runs and the thread + // list repaints BEHIND a window the user still has to dismiss, so the + // search looks like it did nothing. The dialog also describes m_items, + // which the new query is about to replace. + MessageView view; + view.showThread({ oneMessage() }); + + QSignalSpy spy(&view, &MessageView::searchRequested); + QVERIFY(spy.isValid()); + + // showDetailsDialog() blocks in exec(), so the dialog has to be driven + // from a timer once it is up. + bool foundTheDialog = false; + QTimer::singleShot(0, &view, [&view, &foundTheDialog]() { + auto *dialog = view.findChild(); + if (!dialog) { + // Never leave exec() spinning: a missing dialog must fail the test, + // not hang the suite. + QApplication::exit(1); + return; + } + foundTheDialog = true; + + const QList rows = dialog->rows(); + const auto from = std::find_if( + rows.cbegin(), rows.cend(), [](const HeaderRow &row) { + return row.field == QStringLiteral("from"); + }); + if (from == rows.cend()) { + dialog->reject(); + return; + } + + dialog->requestSearch(*from, false); + }); + + view.showDetailsDialog(); + + QVERIFY2(foundTheDialog, "the details dialog never appeared"); + QCOMPARE(spy.count(), 1); + QCOMPARE(spy.at(0).at(0).toString(), + QStringLiteral("from:\"Sender \"")); + + // exec() returned, which is the assertion: the dialog closed on its own + // rather than waiting for the user to dismiss it. + QVERIFY(!view.findChild() + || !view.findChild()->isVisible()); +} + QTEST_MAIN(TestMessageView) #include "test_messageview.moc" -- cgit v1.2.3 From fef6d7be1f70cb51431afef608b037b85029c3c2 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Fri, 14 Aug 2026 13:37:26 +0200 Subject: fix(details): close the dialog before the search runs, not after The connection is direct, so emitting first runs the query synchronously: the model clears and the pane blanks while the modal dialog is still up, holding the m_items it was built from. Closing first leaves no window in which the dialog describes a thread the pane has already dropped. --- src/messageview.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/messageview.cpp b/src/messageview.cpp index 1e6c256..2e7e694 100644 --- a/src/messageview.cpp +++ b/src/messageview.cpp @@ -637,13 +637,17 @@ void MessageView::showDetailsDialog() // It CLOSES on the way out, and that is not tidiness. The dialog is modal, // so without this the query runs and the thread list repaints behind a // window the user still has to dismiss, making the search look like it did - // nothing. The dialog is also built from m_items, which the new query is - // about to replace, so what it displays would describe a thread the pane - // has already stopped showing. + // nothing. + // + // accept() BEFORE the emit, not after. The connection is direct, so the + // emit runs the query synchronously: the model clears and this pane blanks + // while the modal dialog is still up, holding the m_items it was built + // from. Closing first leaves no window in which the dialog describes a + // thread the pane has already dropped. connect(&dialog, &MessageDetailsDialog::searchRequested, this, [this, &dialog](const QString &query, bool extend) { - emit searchRequested(query, extend); dialog.accept(); + emit searchRequested(query, extend); }); dialog.exec(); -- cgit v1.2.3 From bbf3c570215688c553fd70d8f372ae215725ca02 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Fri, 14 Aug 2026 13:40:17 +0200 Subject: docs: close item 85, searching from the message pane Five surfaces in the message pane offer a search built from what they show, replacing the query or narrowing it. The details dialog became rows along the way, which the user wanted independently of this feature. Item 78 is narrowed to the rule shortcut alone and drops to S: item 85 built the menus and item 81 the seeded dialog, so both halves already exist. Its approach text is corrected too, since it claimed the thread list holds a usable sender and notmuch_thread_get_authors returns a display summary, not an address. Three traps recorded in CLAUDE.md: a modal dialog must close before the action it asked for runs, Qt::RFC2822Date validates the weekday against the date, and every query goes through SearchTerm so five surfaces cannot grow five quoting rules. --- CHANGELOG.md | 21 +++++ CLAUDE.md | 44 ++++++++++- .../2026-08-03-post-0.1.0-usability-closed.md | 61 +++++++++++++++ .../plans/2026-08-03-post-0.1.0-usability.md | 90 +++++++++------------- 4 files changed, 159 insertions(+), 57 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3dea47f..7b22d9f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,27 @@ point at which they are stable. ## [Unreleased] +### Added + +- Anything on screen in the message pane can be searched for by right-clicking + it. The subject and the date in the header, the sender and recipients when + the thread holds one message, a tag chip, a phrase selected in the message + body, and every header of every message in the details dialog. Each offers + **Search for this**, which replaces the query, and **Add to search**, which + narrows what is already there. + + Narrowing is the half worth knowing about: a query returning a thousand + threads can be cut down by adding a sender or a date to it, without retyping + the query you started from. + + A search is only ever a search. Turning what you find into a tagging rule is + still the existing road: save the query, then create a rule from it. + +### Changed + +- The message details dialog shows labelled rows rather than one block of + text, so each value can be searched for on its own. + ## [0.19.0] - 2026-08-14 A saved query can become a tagging rule without retyping it, and a rule can no diff --git a/CLAUDE.md b/CLAUDE.md index b66c3b2..f6f9996 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -65,6 +65,7 @@ MainWindow NotmuchWorker └ MessageView (header QLabel, QWebEngineView, attachment bar, TagStrip) CardLayout (pure geometry, no painting) +SearchTerm (pure query strings, no widget) Config (INI) KeyMap MailSync (QProcess) MimeParser (GMime) SyncMonitor (/proc/locks) TagColors QueryCompleter ThreadCidMap ``` @@ -74,9 +75,19 @@ The query row and the message-pane header are **built inline in `MainWindow` and listed `QueryBar`, `SavedQueryBar`, `HeaderWidget` and `AttachmentBar`; none of those types have ever existed, and looking for them wastes a search. The widget classes that do exist are `MessageView`, `ThreadListView`, `TagStrip`, -`TagDialog`, `RowStyleDelegate` and `CardDelegate`; `TagChip` is a namespace of -painting helpers, not a widget, and `ThreadCidMap` and `CardLayout` are structs. -`SubjectDelegate` existed until item 53 and is gone. +`TagDialog`, `MessageDetailsDialog`, `RowStyleDelegate` and `CardDelegate`; +`TagChip` is a namespace of painting helpers, not a widget, `SearchTerm` is a +namespace of query builders, and `ThreadCidMap`, `CardLayout`, `SearchOffer` +and `HeaderRow` are structs. `SubjectDelegate` existed until item 53 and is +gone. + +**`MessageDetailsDialog` was a `QPlainTextEdit` inside `MessageView` until item +85.** It is rows now so each value can carry its own context menu, and its +plain-textness was a SECURITY property rather than a style: header values come +from strangers and plain text cannot interpret markup. Every value label states +`Qt::PlainText` explicitly, because a `QLabel` guesses under `Qt::AutoText`. +Escaping into a rich-text label is the same protection one mistake away from +failing, so do not "simplify" it back. **`ThreadListView` survives only for the expander hit-test.** `CardDelegate` draws the reply count, and a delegate gets no click of its own without an @@ -254,6 +265,17 @@ a failure or a `-1` count fails against correct code. This was recorded in building the rules. Assert on the positional contract, never on a provoked failure. +**Every query this application builds goes through `SearchTerm` +(`src/searchterm.h`), and that is what stops five surfaces growing five quoting +rules.** It holds no widget, so the grammar is tested without a painter or a web +engine. Two of its rules are load-bearing rather than cosmetic. `quote()` +escapes backslashes BEFORE quotes, since the other order escapes the +backslashes it just added; it truncates before escaping, so a cut cannot land +mid-escape. And `extend()` parenthesises BOTH sides, because the query bar can +hold a hand-written disjunction and `a or b AND c` binds as `a or (b AND c)`, +which widens a search the user asked to narrow, reporting nothing. This is the +same trap the `post-new` hook handles when it scopes a rule with `tag:new`. + **A writer that does not validate what its reader requires loses data silently.** `TagRules::save()` wrote any id and `load()` required `^[a-z0-9][a-z0-9-]*$`, so a rule named `justeat orders` in a field labelled @@ -341,6 +363,22 @@ one route out of three. Assert every route. Underneath sits a second trap: `done()`, so a test for the closed path has to `show()` the dialog first or it asserts nothing at all. +**A modal dialog must close BEFORE the action it asked for runs, not after.** +A signal from a dialog to its parent is a DIRECT connection, so the emit runs +the handler synchronously while `exec()` is still on the stack: the details +dialog's search ran the query, cleared the model and blanked the message pane +while the dialog was still up, holding the `m_items` it was built from. Call +`accept()` first, then emit. The mutation check for this HANGS rather than +failing, since without the `accept()` nothing ever leaves `exec()`, and a hung +test binary is item 84's second trap waiting to mislead the next run. + +**`Qt::RFC2822Date` validates the weekday against the date.** `Thu, 14 Aug +2026` parses as INVALID because that day is a Friday, and an invalid parse here +is indistinguishable from the trailing-comment trap `MimeParser::parseDate` +exists to handle. Two fixtures carried a wrong weekday, one of them +pre-existing and unnoticed until something finally parsed it. Write a date +fixture with `date -d +%A`, never from memory. + **Under a tiling compositor a window's size is not the application's to restore, and the user's desktop is Hyprland.** `saveGeometry` stores `frameGeometry` and `normalGeometry`; `restoreGeometry` restores the NORMAL diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md index 8cfa6b0..8ae8dff 100644 --- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md +++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md @@ -4741,3 +4741,64 @@ how this was found: the suite ran past its two-minute timeout with no output. The cause is not the account at all, it is `showWarnings()` raising a modal from the MainWindow constructor for any config problem, with nothing offscreen to dismiss it. Filed as item 84. + +## 85. Nothing on screen can be searched for by right-clicking it + +**Observed (2026-08-14).** The user asked, while discussing item 78, to be able +to right-click a sender, a subject, a date, a tag chip or a piece of selected +body text and be offered a search built from it, both replacing the query bar +and narrowing what is already in it. The narrowing case decided the shape: a +query returning a thousand threads is refined by adding a second condition, and +before this that meant retyping a query the user never typed. + +**Cause.** Not a defect, unbuilt. `MessageView` had no context menu on any +surface, and `TagStrip` painted chips with no hit test and no signals. + +**Done 2026-08-14**, designed in +`specs/2026-08-14-search-from-message-design.md` and built over eight tasks. +Five surfaces offer **Search for this** and **Add to search**: the header's +subject and date, its From/To/Cc for a single-message thread, a tag chip, a +body selection, and every header per message in the details dialog. + +**What is worth keeping from it.** + +`SearchTerm` (`src/searchterm.h`) owns the whole query grammar and holds no +widget, so the quoting is tested without a painter or a web engine. That +matters because **a mis-quoted notmuch query is not an error, it matches zero**: +nothing downstream would ever report the feature being broken. `extend()` +parenthesises both sides, since `a or b AND c` binds as `a or (b AND c)` and +silently WIDENS a search the user asked to narrow. + +**`Qt::RFC2822Date` validates the weekday against the date**, so `Thu, 14 Aug +2026` parses as invalid: that day is a Friday. Two test fixtures carried a wrong +weekday, one of them pre-existing, and the failure is indistinguishable from the +trailing-comment trap `MimeParser::parseDate` exists to handle. Check a date +fixture with `date -d +%A` rather than writing one from memory. + +The Date: parse was **extracted** into `MimeParser::parseDate` rather than +rewritten, because the existing copy inside a file-local function already +carried the fix for `Qt::RFC2822Date` rejecting a string with a trailing +timezone comment. A second parser without it would have lost the date on a +large share of real mail, showing up only as a menu entry that never appears. + +**The header lists its fields rather than hit-testing them.** It is one +rich-text `QLabel` of up to four lines, and mapping a point through laid-out +rich text breaks as soon as the label wraps. From/To/Cc are offered exactly when +the header displays them, which is a single-message thread, so the menu can +never name a value the header is withholding. + +**The details dialog stopped being a `QPlainTextEdit`**, which the user had +disliked independently. Its plain-textness was a security property rather than a +style: header values come from strangers and plain text cannot interpret markup. +Every value label therefore states `Qt::PlainText` explicitly, because a +`QLabel` guesses under `Qt::AutoText`, and a test enumerates every label and +asserts the format. + +**A modal dialog must close BEFORE the search it asked for runs.** The +connection is direct, so emitting first runs the query synchronously: the model +clears and the pane blanks while the dialog is still up, holding the `m_items` +it was built from. Caught by a test whose mutation check HUNG rather than +failed, which is what a missing `accept()` does to `exec()`. + +**Item 78 stays open**, carrying the rule shortcut alone. The road from a search +to a rule already exists: save the query, then create a rule from it. diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md index f562e97..980aefb 100644 --- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md +++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md @@ -142,14 +142,14 @@ taking that too literally. | 75 | The tagging rules window forgets its size and its column widths | persistence | S | **done** 2026-08-13, shipped in 0.17.0. The window-kind question is left open, see the closed-items file | | 76 | Every field in the rules dialog is free text, so a rule is easy to get wrong | workflow | M | **done** 2026-08-13, shipped in 0.17.0. See `specs/2026-08-13-rule-builder-design.md` | | 77 | No way to see what a rule would collect, in the thread list | workflow | S | **done** 2026-08-13, shipped in 0.17.0 | -| 78 | No way to build a rule from something visible in a message | workflow | M | open, narrowed 2026-08-14; the search half split out as item 85, which is the road a rule is made from. Revisit once 85 has been used | +| 78 | No way to build a rule from something visible in a message | workflow | S | open, narrowed 2026-08-14; the search half shipped as item 85, which is the road a rule is made from. Now a shortcut across that road: the menus and the seeded-dialog path both exist. Use 85 first and see which values are worth promoting | | 80 | A rule with many conditions squeezes the rule list to one visible row | defect | XS | **done** 2026-08-13, shipped in 0.17.0. Follows item 76 | | 79 | Opening the rules dialog and saving destroys the first rule | defect | XS | **fixed on `rule-builder`** 2026-08-13, unreleased. Shipped in 0.16.0; damaged one real rule, repaired by hand | | 81 | No way to turn a saved query into a tagging rule | workflow | S | **done** 2026-08-14, unreleased; see `specs/2026-08-14-query-to-rule-design.md` | | 82 | A saved query cannot be edited, unpinned or deleted from the UI | defect | S | **done** 2026-08-13, shipped in 0.18.0. Right-click offers Edit, Pin/Unpin and Delete | | 83 | A rule named with spaces is written to the file and dropped by every reader | defect | S | **done** 2026-08-14, unreleased. The name is sanitised into an id, save validates, a bad id loads for repair | | 84 | A config problem blocks `test_mainwindow` on a modal nobody can dismiss | testing | S | open; measured 2026-08-14, `showWarnings()` calls `QMessageBox::warning` from the constructor | -| 85 | Nothing on screen can be searched for by right-clicking it | workflow | M | open; designed 2026-08-14, see `specs/2026-08-14-search-from-message-design.md`. Split from 78; rebuilds the details dialog as rows | +| 85 | Nothing on screen can be searched for by right-clicking it | workflow | M | **done** 2026-08-14, unreleased; see `specs/2026-08-14-search-from-message-design.md`. Split from 78; rebuilt the details dialog as rows | Sizes are rough: XS under an hour, S a sitting, M a session. @@ -483,26 +483,45 @@ query look fast is a worse trade than the wait. **Observed.** The user would like to select an address or another piece of a message in the main window, right-click, and be offered a rule built from it. -**Cause.** Not a defect, unbuilt. The thread list has a context menu (item -24) and the message pane is a `QWebEngineView` whose selection is inside the -render process. - -**Approach.** Start from the thread list's own context menu, where the -sender is already a value the model holds, rather than from a text selection -in the web view. A "Create rule from sender" entry that opens the rules -dialog with the query prefilled covers the case the user described and needs -no new plumbing. - -**Constraints.** JavaScript is disabled in the profile and must stay -disabled, so reading a selection out of the web view means -`QWebEnginePage::selectedText()` and nothing that injects script. Do that -part only if the sender case turns out not to be enough. +**Cause.** Not a defect, unbuilt. + +**Narrowed 2026-08-14, and most of the work is already done.** The search half +shipped as item 85, which is the road a rule is made from: search for a value, +save the query, create a rule from the saved query. What remains here is a +SHORTCUT across that road, and both of its halves now exist. + +- The menus are built and every surface already extracts its value as a + finished query (`SearchOffer`, `src/searchterm.h`). A rule entry is another + action beside the two search ones, not new plumbing. +- The seeded-dialog path exists from item 81: + `MainWindow::showTagRulesDialog(const TagRule &seed)`. A rule from a message + becomes a second caller of it, with a different seed, which is what item 81's + spec anticipated when it made the seed a whole `TagRule` rather than a query + string. + +**Approach.** Decide it after using item 85 for a while. Which values are worth +promoting straight to a rule is a usage question, and the earlier answer to it +was wrong (see below), so it is worth having the evidence first. + +**Constraints.** The original approach here said to start from the thread +list's context menu "where the sender is already a value the model holds". +**That is false and item 85 verified it.** `ThreadSummary::authors` comes from +`notmuch_thread_get_authors` and is a DISPLAY SUMMARY, reading `Alice, Bob` or +`Alice| Bob`, so a `from:` built from it matches nothing. A real address comes +from `MessageNode::from` or `ParsedMessage::from`, neither of which the thread +list carries. Any thread-list entry needs an address resolved from a message +first. + +JavaScript is disabled in the profile and must stay disabled. Item 85 reads a +body selection with `QWebEnginePage::selectedText()`, which injects no script; +reuse that rather than adding anything. The rules file is shared with mailctl, so a rule created here must go through `TagRules` and preserve unknown fields; see "Changing the shared rule format" in CLAUDE.md. -**Size: M.** +**Size: S**, down from M now that item 85 has built the menus and item 81 the +seeded dialog. ## 84. A config problem blocks `test_mainwindow` on a modal nobody can dismiss @@ -558,43 +577,6 @@ here. **Size: S.** The diagnosis is the expensive part and it is already done. -## 85. Nothing on screen can be searched for by right-clicking it - -**Observed (2026-08-14).** The user asked, while discussing item 78, to be able -to right-click a sender, a subject, a date, a tag chip or a piece of selected -body text and be offered a search built from it, both replacing the query bar -and narrowing what is already in it. The narrowing case is the one that decides -the shape: a query returning a thousand threads is refined by adding a second -condition, and today that means retyping a query the user never typed. - -**Cause.** Not a defect, unbuilt. `MessageView` has no context menu on any -surface, and `TagStrip` paints chips with no hit test and no signals. - -**Approach.** Designed 2026-08-14. **Read -`specs/2026-08-14-search-from-message-design.md` rather than planning from this -entry.** - -Three constraints decide whether to open the spec. **This is search only, and -that is the point**: item 78's rule shortcut is deliberately left out, because a -saved query can already be promoted to a rule and searching is the missing step -on that road, as well as the safe one. **The details dialog is rebuilt as rows**, -which the user wanted anyway, and its value labels must stay explicitly -`Qt::PlainText`: the current `QPlainTextEdit` is a deliberate protection against -markup in header values that come from strangers. And **Add to search -parenthesises both sides**, because the bar may hold a hand-written `or` and -`a or b AND c` binds the wrong way, silently widening a search meant to narrow. - -**Constraints.** The thread list is out of scope and the reason is worth -keeping: a row's `authors` comes from `notmuch_thread_get_authors` and is a -display summary reading `Alice, Bob`, so a `from:` built from it matches -nothing. Item 78's own approach line assumed otherwise. - -A mis-quoted query is not an error to notmuch, it matches zero, so the quoting -helper is tested against the constructed string rather than against a provoked -failure. - -**Size: M.** - ## Deferred, unsized, or split out Items noted while triaging but not part of the original list. Same numbering -- cgit v1.2.3