aboutsummaryrefslogtreecommitdiffstats
path: root/docs/superpowers
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-14 17:41:56 +0200
committerDanilo M. <danix@danix.xyz>2026-08-14 17:41:56 +0200
commit22467bf5dfdff58850f4f705b36c464c68bc56e2 (patch)
treec349a881fc414e58f9f0e21e555d2912d82c2553 /docs/superpowers
parent9fd9f06a2e9aa1e9f9a12e82cd8bf727d5f7b968 (diff)
downloadqtmaildir-22467bf5dfdff58850f4f705b36c464c68bc56e2.tar.gz
qtmaildir-22467bf5dfdff58850f4f705b36c464c68bc56e2.zip
docs(spec): design for excluding a value from a search, item 86
Two decisions the user made, both recorded with their rejected alternatives so they are not revisited. Excluding from an empty query would mean the whole Maildir minus one value. The menu entry is greyed rather than hidden when the query bar is empty, so the feature stays visible to someone exploring a fresh window, and SearchTerm::exclude returns empty for that case as a second layer against a caller that forgets the guard. The menus cannot see the query bar, so MainWindow pushes the fact down through MessageView::setHasQuery from the textChanged lambda it already runs for the Save button. A callback was rejected as an indirection with one implementation; silently doing nothing was rejected because a live menu entry that does nothing is worse than a greyed one. The backlog entry loses its inline approach and points at the spec, carrying the three constraints that decide whether it can be picked up. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Diffstat (limited to 'docs/superpowers')
-rw-r--r--docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md41
-rw-r--r--docs/superpowers/specs/2026-08-14-exclude-from-search-design.md184
2 files changed, 194 insertions, 31 deletions
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 15377ea..087ce95 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
@@ -150,7 +150,7 @@ taking that too literally.
| 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 | **done** 2026-08-14, unreleased; see `specs/2026-08-14-search-from-message-design.md`. Split from 78; rebuilt the details dialog as rows |
-| 86 | A right-click search can replace or narrow, but never exclude | workflow | S | open; follows 85. The `extend` bool has no room for a third choice, so this widens a signature rather than adding a menu entry |
+| 86 | A right-click search can replace or narrow, but never exclude | workflow | S | open, specced 2026-08-14; see `specs/2026-08-14-exclude-from-search-design.md`. Follows 85. The `extend` bool has no room for a third choice, so this widens a signature rather than adding a menu entry |
Sizes are rough: XS under an hour, S a sitting, M a session.
@@ -597,36 +597,15 @@ bool: `extend ? SearchTerm::extend(m_queryEdit->text(), query) : query`.
missing menu entry over an existing capability; there is no third state for a
menu entry to select, and the signature cannot express one.
-**Approach.** Three changes, in this order, and the first is the only design
-decision.
-
-- **Widen the signal past a bool.** A third operation makes `bool extend` wrong
- at four call sites. An enum (replace, narrow, exclude) carried through
- `MessageView`, `MessageDetailsDialog` and `MainWindow` is the honest shape.
- Note that it crosses no thread boundary, so the `Q_ENUM` metatype trap in
- `CLAUDE.md` does not apply here; these are direct connections.
-- **Add `SearchTerm::exclude(existing, addition)`**, parenthesising both sides
- exactly as `extend()` does, as `(existing) AND NOT (addition)`. The
- parenthesising is load-bearing for the same reason it is in `extend()`: the
- query bar can hold a hand-written disjunction, and an unparenthesised
- `NOT a or b` binds so that the exclusion covers only the first term. This is
- the whole of the testable surface and it needs no widget.
-- **A third menu entry per offer**, wrapped in `tr()`.
-
-**Constraints.**
-
-- **Excluding from an empty query is not a search.** `extend()` returns the
- addition alone when `existing` is empty; the same rule here would produce
- `NOT (x)`, meaning "all mail except", which is a plausible thing to ask for
- and an implausible thing to have meant by right-clicking a sender. Decide
- deliberately: either suppress the entry when the query bar is empty, or let it
- through. Do not let it fall out of the code by accident.
-- **notmuch reports nothing for a malformed query**, so correctness is asserted
- against the constructed string, never against a provoked failure or a result
- count. `SearchTerm`'s own header records this.
-- The details dialog must keep calling `accept()` BEFORE emitting, per item 85's
- trap. A new action added to that menu inherits the same requirement, and its
- mutation check hangs rather than fails.
+**Specced 2026-08-14: read
+`specs/2026-08-14-exclude-from-search-design.md` rather than planning from
+here.** Three constraints decide whether it can be picked up. The `bool extend`
+becomes a `SearchMode` enum across four signatures, which is the bulk of the
+work and touches three files. Excluding from an EMPTY query would mean the whole
+Maildir minus one value, so the entry is greyed when the query bar is empty and
+the grammar returns empty as a second layer. And correctness is asserted against
+the constructed string, never a result count, because notmuch parses `from:((((`
+cleanly and matches zero.
**Size: S.** The grammar is one function beside an existing one with existing
tests; the spread is the signature change across three files.
diff --git a/docs/superpowers/specs/2026-08-14-exclude-from-search-design.md b/docs/superpowers/specs/2026-08-14-exclude-from-search-design.md
new file mode 100644
index 0000000..1c863e8
--- /dev/null
+++ b/docs/superpowers/specs/2026-08-14-exclude-from-search-design.md
@@ -0,0 +1,184 @@
+# Excluding a value from a search
+
+Resolves backlog item **86**. Follows item 85, which shipped the two operations
+this adds a third to. Depends on nothing unshipped.
+
+## What this is
+
+Every right-click search menu gains a third entry, **Exclude from search**,
+which narrows the current query by everything that is *not* the value under the
+cursor. The two shipped entries keep their labels and their behaviour.
+
+The user's words, from the notes on 2026-08-14: "the current available search
+options are to run the query from scratch or add to the existing query. A
+missing option is to add negatively (not)."
+
+## Why it is not a one-line menu addition
+
+Item 85 shipped exactly two operations and encoded the choice between them as a
+single `bool extend`, carried end to end. `MessageView::addSearchEntries`
+(`src/messageview.cpp:566`) builds the pair per offer,
+`MessageDetailsDialog` (`src/messagedetailsdialog.cpp:88`) repeats it per row,
+and `MainWindow::runSearchFromPane` (`src/mainwindow.cpp:1654`) branches on it:
+
+```cpp
+extend ? SearchTerm::extend(m_queryEdit->text(), query) : query;
+```
+
+`SearchTerm` has no exclusion form at all. So there is no third state for a new
+menu entry to select, and the signature cannot express one. The work is a
+widening of that signature plus one function, not a menu edit.
+
+## The grammar
+
+One function in `src/searchterm.h`, beside `extend()`:
+
+```cpp
+QString exclude(const QString &existing, const QString &addition);
+```
+
+It returns `(existing) AND NOT (addition)`.
+
+**Both sides are parenthesised, and that is load-bearing rather than tidy.**
+The query bar may hold a hand-written disjunction. Unparenthesised,
+`a or b AND NOT c` binds as `a or (b AND NOT c)`: the exclusion covers only the
+second term, and every message matching `a` stays on screen including the ones
+the user asked to be rid of. Nothing reports an error. This is the same trap
+`extend()` documents and the same one the `post-new` hook handles when it scopes
+a rule with `tag:new`.
+
+### The empty cases, and where they differ from `extend()`
+
+| `existing` | `addition` | result |
+|---|---|---|
+| non-empty | non-empty | `(existing) AND NOT (addition)` |
+| non-empty | empty | `existing`, untouched |
+| empty | non-empty | **empty string** |
+| empty | empty | empty string |
+
+The third row is the decision. `extend()` returns the addition alone when
+`existing` is empty, because narrowing nothing by `x` sensibly means `x`.
+Excluding from nothing would mean `NOT (addition)`: the entire Maildir minus
+one sender. That is a legitimate query and an implausible thing to have meant by
+right-clicking a value in a fresh window, so the grammar refuses to build it.
+
+The UI never asks for it either (see the guard below). Two layers, deliberately:
+the guard is what the user sees, and the empty return is what stops a future
+caller that forgets the guard from producing a whole-Maildir scan.
+
+## The signal
+
+`bool extend` cannot carry three states. It becomes an enum in
+`src/searchterm.h`, beside the functions it selects between:
+
+```cpp
+enum class SearchMode { Replace, Narrow, Exclude };
+```
+
+It crosses **no thread boundary**, so the `Q_ENUM` metatype trap recorded in
+`CLAUDE.md` does not apply here. Every one of these connections is direct; that
+trap belongs to the queued signals into `NotmuchWorker`.
+
+Four signatures change:
+
+| where | from | to |
+|---|---|---|
+| `MessageView::searchRequested` | `(QString, bool)` | `(QString, SearchMode)` |
+| `MessageDetailsDialog::searchRequested` | `(QString, bool)` | `(QString, SearchMode)` |
+| `MessageDetailsDialog::requestSearch` | `(HeaderRow, bool)` | `(HeaderRow, SearchMode)` |
+| `MainWindow::runSearchFromPane` | `(QString, bool)` | `(QString, SearchMode)` |
+
+`runSearchFromPane` becomes a three-way switch over the mode. Its existing
+comment stays true and stays put: the result goes through the query bar and
+`runCurrentQuery()`, so the account scope, the generation counter and the
+flat-mode reset behave exactly as they do for a typed query. Nothing here builds
+a second query path.
+
+## The guard
+
+The menus cannot currently tell whether the query bar is empty. `MessageView`
+and `MessageDetailsDialog` both emit outward and never read the query, and the
+dialog is a child of the view rather than of the window.
+
+`MainWindow` pushes the fact down:
+
+```cpp
+void MessageView::setHasQuery(bool hasQuery);
+```
+
+`MainWindow` already hangs a lambda on `m_queryEdit`'s `textChanged` at
+`src/mainwindow.cpp:373`, for the Save button's enabled state. That lambda gains
+one line. `MessageView` stores the bool and passes it to
+`MessageDetailsDialog` at construction, since the dialog is built fresh per
+invocation and cannot go stale.
+
+Both menu builders then disable the Exclude action when the bool is false. The
+entry stays **visible and greyed**, not hidden: a user exploring a fresh window
+is exactly the one who should be able to see that the feature exists.
+
+Rejected alternatives, recorded so they are not revisited. A `std::function`
+callback into `MainWindow` is always fresh but adds an indirection with one
+implementation and a dangling-callback risk. Leaving the entry always enabled
+and having `MainWindow` silently do nothing is fewer lines and worse: a live
+menu entry that does nothing is a worse failure than a greyed one.
+
+## The menus
+
+Three entries, in this order, in both surfaces:
+
+```
+Search for this
+Add to search
+Exclude from search
+```
+
+The two shipped labels are untouched, so nothing the user already knows moves.
+All three are wrapped in `tr()`.
+
+Both call sites: `MessageView::addSearchEntries` (`src/messageview.cpp:566`),
+which builds a submenu per offer, and the per-row lambda in
+`MessageDetailsDialog` (`src/messagedetailsdialog.cpp:88`).
+
+## Testing
+
+**`test_searchterm`** carries the grammar, and needs no widget, no painter and
+no web engine. Cover every row of the empty-case table, and cover the
+disjunction: `exclude("tag:inbox or tag:flagged", "from:\"x\"")` must
+parenthesise the left side, which is the assertion that fails if someone
+"simplifies" the parentheses away.
+
+**Assert on the constructed string, never on a result count.** notmuch rejects
+almost nothing: `from:((((` parses cleanly and matches zero, so a malformed
+query produces an empty result rather than an error. A test asserting a failure
+or a `-1` count passes against correct code and against broken code alike. This
+is recorded in `searchterm.h` and in `test_notmuchworker.cpp`, and has been
+learned twice already.
+
+**`test_messagedetailsdialog`** covers the guard: the Exclude action is disabled
+when the dialog is constructed with no query and enabled when it is constructed
+with one.
+
+**The dialog inherits item 85's ordering trap.** A modal's signal to its parent
+is a DIRECT connection, so the emit runs the handler synchronously while
+`exec()` is still on the stack. `accept()` must run before the emit, or the
+search clears the model and blanks the pane behind a dialog the user still has
+to dismiss, while it holds the `m_items` it was built from. The new action is on
+the same path and has the same requirement.
+
+**Its mutation check hangs rather than fails.** Without the `accept()`, nothing
+ever leaves `exec()`. A hung `test_messagedetailsdialog` binary also leaves a
+stale binary that a later `ctest` will re-run, which is item 84's second trap:
+the failure appears to persist after being fixed. Kill the binary and rebuild
+before concluding anything about a hang here.
+
+## Out of scope
+
+The thread list's context menu. Item 85 left it alone deliberately and that
+holds: `ThreadSummary::authors` comes from `notmuch_thread_get_authors` and is a
+display summary reading like "Alice, Bob", so a `from:` query built from it
+matches nothing. Item 78 carries what is left of that.
+
+## Size
+
+**S.** The grammar is one function beside an existing one with an existing test
+binary. The spread is the signature change across three files.