aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-10 09:33:44 +0200
committerDanilo M. <danix@danix.xyz>2026-08-10 09:33:44 +0200
commit01419de209c2b5e2ae7b996e6b5ff1baa2efb3da (patch)
tree25718fd36e87eac721f41b02cca46b6fb07e94d9
parentf72dba9f6c463c6823d85701e51d8be38dd22a62 (diff)
parente1dba2987a9a1e87b92801959df9c9d4f1375d2f (diff)
downloadqtmaildir-01419de209c2b5e2ae7b996e6b5ff1baa2efb3da.tar.gz
qtmaildir-01419de209c2b5e2ae7b996e6b5ff1baa2efb3da.zip
Merge branch 'card-list': the thread pane as a list of cards
Replaces the five-column grid with a single column of three-line cards. Item 53 recorded that the columns, not the cues drawn inside them, were what made the list read as a table of records; item 20 had already shipped finished, tested and green and been rejected on sight for exactly that reason. A card is sender and date, subject with the flag, attachment and reply-count marks, and tags, at one uniform height. Replies indent under a continuous spine and show only the tags their thread does not carry. The account colour runs down the card's left edge, replacing the chip that used to eat a third of every subject line, with matching swatches in the account dropdown. Sorting newest or oldest first is new and remembered. Closes items 20, 51, 53 and 60. The four defects that mattered were all found by rendering cards to an image and looking at them, with the suite green through every one: a date clipped on unread cards because bold is wider than the font the layout measured, an accent bar painted in a colour identical to the background, an expander pill in a palette role a theme had made equal to Base, and three separate faults from trusting notmuch's reply depth to mean structure when it only means how notmuch happened to thread the mail.
-rw-r--r--CHANGELOG.md53
-rw-r--r--CLAUDE.md119
-rw-r--r--docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md30
-rw-r--r--docs/superpowers/plans/2026-08-08-item-20-message-rows.md9
-rw-r--r--src/CMakeLists.txt2
-rw-r--r--src/carddelegate.cpp261
-rw-r--r--src/carddelegate.h72
-rw-r--r--src/cardlayout.cpp186
-rw-r--r--src/cardlayout.h150
-rw-r--r--src/keymap.cpp36
-rw-r--r--src/keymap.h9
-rw-r--r--src/mainwindow.cpp507
-rw-r--r--src/mainwindow.h98
-rw-r--r--src/notmuchworker.cpp164
-rw-r--r--src/notmuchworker.h41
-rw-r--r--src/tagchip.cpp176
-rw-r--r--src/tagchip.h42
-rw-r--r--src/threadlistmodel.cpp499
-rw-r--r--src/threadlistmodel.h185
-rw-r--r--src/threadlistview.cpp141
-rw-r--r--src/threadlistview.h39
-rw-r--r--src/types.h58
-rw-r--r--tests/CMakeLists.txt1
-rw-r--r--tests/test_cardlayout.cpp383
-rw-r--r--tests/test_mainwindow.cpp1059
-rw-r--r--tests/test_notmuchworker.cpp140
-rw-r--r--tests/test_threadlistmodel.cpp681
27 files changed, 4294 insertions, 847 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index fdbb08f..5bf9256 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,6 +11,29 @@ point at which they are stable.
## [Unreleased]
+### Changed
+
+- **The thread list is now a list of cards rather than a table of columns.**
+ Each thread shows its sender and date, its subject with the flag, attachment
+ and reply-count marks, and its tags, on three lines at one uniform height.
+ Expanding a thread shows its replies indented under a continuous spine,
+ carrying only the tags the thread itself does not have, and without the `Re:`
+ prefix every reply used to repeat.
+- **Threads can be listed newest or oldest first**, from a new control beside
+ the query bar. The choice is remembered between sessions.
+- **An account's colour now runs down the left edge of its threads**, and down
+ the spine of their replies, replacing the account chip that used to sit in
+ front of every subject. The account dropdown shows the same colours, so which
+ colour means which account is readable in one place.
+- **Alt+Up and Alt+Down step between threads**, alongside the existing Ctrl+J
+ and Ctrl+K. Plain Up and Down now step message by message through an expanded
+ thread, which is the view's own behaviour rather than a binding.
+- **An out-of-range `message_zoom` now says so.** The documented 0.5 to 3.0
+ range was already enforced on the way to the web view, so a `message_zoom` of
+ 500 rendered at 3.0 rather than unusably, but nothing reported that the value
+ in the file was not the value on screen. It is now listed with the other
+ configuration problems at startup.
+
### Fixed
- **The message pane no longer comes back as a sliver.** A splitter position is
@@ -19,13 +42,18 @@ point at which they are stable.
left, in one real case 29px. The pane now has a minimum width and cannot be
collapsed, which covers the restore and the equivalent drag.
-### Changed
+- **Clicking a thread no longer scrolls the list sideways.** A card is exactly
+ the width of the pane, so there is nowhere to scroll to.
+- **Next and previous thread no longer step onto a reply** when a thread is
+ expanded. They skip message rows, so they keep meaning thread-to-thread.
-- **An out-of-range `message_zoom` now says so.** The documented 0.5 to 3.0
- range was already enforced on the way to the web view, so a `message_zoom` of
- 500 rendered at 3.0 rather than unusably, but nothing reported that the value
- in the file was not the value on screen. It is now listed with the other
- configuration problems at startup.
+### Upgrading
+
+- **Saved thread-list column widths are ignored.** There is one column now, so
+ the `threadlist/header` and `threadlist/columns` entries in
+ `~/.local/state/qtmaildir/uistate.conf` no longer do anything. Nothing needs
+ to be done: they are read past and can be left in place or deleted. Window
+ geometry, the splitter position and the message zoom are unaffected.
## [0.12.1] - 2026-08-09
@@ -62,6 +90,19 @@ if you leave it alone:
### Added
+- **Threads expand in the list to show their replies.** A thread with more than
+ one message carries an expander; opening it lists the replies as indented rows
+ beneath it, marked with a thread line, a tinted background and smaller text.
+ The replies are fetched when you expand, not with the query, so a large result
+ still paints immediately.
+- **Selecting a reply opens that message on its own**, rather than the whole
+ conversation, which is the point of having message rows at all.
+- **Actions follow what you selected, and the status bar says what they will
+ touch.** A thread row acts on the whole thread and reports "1 thread selected
+ (7 messages)" before and "(whole thread)" after; a reply row acts on that one
+ message. Both are undoable. There is no confirmation dialog, deliberately:
+ undo is this application's answer to a mistaken action, and naming the scope
+ is what makes it usable.
- **A sync now fetches only the accounts you have edited.** Tagging mail in one
account and syncing no longer pulls every other account as well. A sync with
nothing outstanding is a plain fetch and still covers everything, since
diff --git a/CLAUDE.md b/CLAUDE.md
index b7b8c07..ded87e2 100644
--- a/CLAUDE.md
+++ b/CLAUDE.md
@@ -42,10 +42,11 @@ UI thread Worker thread
MainWindow NotmuchWorker
├ query row: QComboBox, QLineEdit, └ owns the only notmuch_database_t*
│ saved-query QPushButtons
- ├ ThreadListView ── ThreadListModel
- │ (RowStyleDelegate every column, SubjectDelegate on Subject)
+ ├ ThreadListView (QTreeView) ── ThreadListModel (QAbstractItemModel)
+ │ ONE column of cards; CardDelegate paints each whole, from CardLayout
└ MessageView (header QLabel, QWebEngineView, attachment bar, TagStrip)
+CardLayout (pure geometry, no painting)
Config (INI) KeyMap MailSync (QProcess) MimeParser (GMime)
SyncMonitor (/proc/locks) TagColors QueryCompleter ThreadCidMap
```
@@ -55,27 +56,111 @@ 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 `SubjectDelegate`; `TagChip` is a namespace
-of painting helpers, not a widget, and `ThreadCidMap` is a struct.
-
-**`ThreadListView` exists because a delegate cannot paint outside its column.**
-The tag chips under each row are one strip spanning the whole width, so they
-are drawn in the view's `paintEvent` after the cells. Consequences that are
-easy to undo by accident: `SubjectDelegate` reads `AccountLabelRole`, which
-belongs to the ROW, so installing it view-wide draws the account chip into
-every column (a `Q_ASSERT` catches this); row height must be set on the
-vertical header, since a table takes one height per row and a column's
-`sizeHint` only applies if the view happens to ask that column; and because
-alternating colours, the selection and the model's `BackgroundRole` are all
-painted per cell, the view has to fill the strip's band itself, honouring all
-three or a deleted row is cut in half and every other row shows a bare stripe.
+`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.
+
+**`ThreadListView` survives only for the expander hit-test.** `CardDelegate`
+draws the reply count, and a delegate gets no click of its own without an
+editor, so the view owns the click and asks the delegate for the rect rather
+than recomputing it.
+
+Until item 53 it also painted a row-wide strip of tag chips after the cells,
+because a delegate cannot paint outside its column and the strip spanned all
+five. That is why the class exists at all, and the history is worth keeping:
+the arithmetic it needed produced a deleted row cut in half and every other row
+showing a bare stripe, both because the view had to re-honour alternating
+colours, the selection and `BackgroundRole` across cells it did not own. With
+one column there is nothing to span, so the `paintEvent` and its band
+arithmetic are deleted and none of that applies any more.
+
+**A card layout must be testable without a painter.** `CardLayout` computes
+every rect on a card and touches no `QPainter` and no widget, so the geometry
+has tests that a blank render cannot defeat. When changing what a card shows,
+change `CardLayout` and assert there; a test that renders the delegate and
+counts pixels proves nothing, for the reasons under "Rendering probes lie".
+Two traps it already handles: `QRect::right()` is inclusive, so the right edge
+is carried as an exclusive one, and `QFont::pointSizeF()` returns -1 for a font
+set in pixels, which qt6ct does.
+
+**`QTreeView`'s Up/Down already walk into an expanded thread's replies**, and
+that is where message-to-message navigation comes from. Do not bind arrow keys
+as `QAction` shortcuts to get it: a shortcut is dispatched before the focused
+widget sees the key and Qt withholds only plain LETTERS from editable widgets,
+so a bare `Up` would break the query bar, the tag dialog and the web view at
+once. `Alt+Up`/`Alt+Down` are chords and therefore safe; `Shift+Up`/`Down` is
+the built-in extend-selection and must be left alone. Binding two sequences to
+one action needs `setShortcuts`, not `setShortcut`, which keeps only the last.
+
+**`Q_ENUM` is not enough to send an enum across a queued connection.** It gives
+the type a meta-object entry, not a metatype registered under the name
+`invokeMethod` resolves, so a `Q_ARG` carrying it is dropped at runtime with a
+warning and the slot runs with a default. `NotmuchWorker::SortOrder` is
+registered beside the type for this reason, not in `MainWindow`, so a caller
+that never constructs one still gets it.
+
+**It is a `QTreeView` over a `QAbstractItemModel` since item 20**, because a
+thread's replies are child rows and a table can neither indent nor expand. What
+did NOT survive that port is anything keyed on a row NUMBER: a tree numbers rows
+per parent, so `row 0` exists once per expanded thread and `current.row() + 1`
+names a sibling rather than the next thread. Navigation walks with
+`indexBelow`/`indexAbove`; `QTableView::isRowSelected(int)` has no equivalent —
+use `selectionModel()->isSelected(index)`. Row height comes from
+`setUniformRowHeights` plus `CardDelegate::sizeHint`, since a tree has no
+vertical header to carry a default section size. Indentation is
+`setIndentation(0)`: `CardLayout` draws the indent inside the card's own rect,
+so `visualRect` reports the SAME left edge for a thread and its reply and a
+geometry probe sees no nesting in a correctly nested list.
+
+**Three traps in the expander, all of which shipped a plausible-looking broken
+build before being caught.** `QTreeView::drawBranches` is the documented hook and
+does not work when the expander sits on a content column: it runs BEFORE the
+row's cells, so the delegate's background paints over it (a 60-pixel triangle
+survived as 8). `CardDelegate` draws it instead, as the reply count on the
+card's second line. `setRootIsDecorated(false)`, needed to stop the style
+drawing its own indicator underneath, also removes the style's HIT AREA, so the
+glyph renders perfectly and is inert; `ThreadListView::mousePressEvent` handles
+the click, asking `CardDelegate::expanderRectFor` for the target so the drawn
+and clickable rects cannot drift. A fourth trap died with the grid: `isExpanded`
+is keyed on column 0, which used to disagree with the subject-column index.
+
+**Visible, clickable and toggling are three separate properties.** A test for
+one passes against the other two being broken, which happened twice in one
+session: a pixel test proved the triangle was drawn while nothing could click
+it, and a click test proved it opened while it could never close.
+
+**Assert a reply's indent on where the TEXT lands, never on `visualRect`.** The
+reason has inverted twice and the rule has not. Under item 20 the geometry was
+indented while the text was not, because the delegate laid text out from its own
+left edge; now `setIndentation(0)` means `visualRect` reports no indent at all
+while the text is indented, because `CardLayout` draws it inside the card's rect.
+A probe on `visualRect` therefore endorsed a broken layout then and would fail a
+correct one now. Assert on `CardLayout::contentLeft`.
+
+**`paintEvent` ran AFTER the cells**, which is why anything the view filled
+across a row covered the text the delegate had just drawn: the reply tint filled
+the full row height in one version and erased every sender and subject, measured
+at zero surviving text pixels. Recorded because it is the class of bug a view
+that paints invites. `ThreadListView` no longer paints at all.
**No `notmuch_*` pointer ever crosses the thread boundary.** Data crosses as the plain
-value structs in `src/types.h` (`ThreadSummary`, `MessageRef`, `TagChange`), over queued
+value structs in `src/types.h` (`ThreadSummary`, `MessageRef`, `MessageNode`,
+`ActionScope`, `TagChange`), over queued
signals in both directions. `notmuchworker.cpp` is the only file that includes `notmuch.h`
outside `src/nmraii.h`; C handles are owned by the `NmQuery`/`NmMessages`/`NmThread`/…
RAII aliases there so they cannot leak.
+**The one exception, and it is a double-free if undone.** Messages reached
+through `notmuch_thread_get_toplevel_messages` / `notmuch_message_get_replies`
+are owned by the THREAD and freed with it (`notmuch.h:1637`), so `walkReplies`
+in `notmuchworker.cpp` holds them as raw `notmuch_message_t*`: an `NmMessage`
+wrapper would call `notmuch_message_destroy` on memory the thread frees again.
+The whole walk must finish while the `NmThread` is alive. Related: replies are
+unreachable from a query walk at all — `notmuch_message_get_replies` returns
+NULL for a message from `notmuch_query_search_messages` (`notmuch.h:1617-1628`),
+which is why `loadThreadTree` exists beside `loadThread` rather than replacing
+it.
+
**Generation counters, not cancellation.** Each query bumps a `quint64` generation passed
through to the worker and back on every result signal. The UI discards results whose
generation is stale. The worker never needs to know a query was superseded. Threads are
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 17cf434..ad6dbc0 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
@@ -77,7 +77,7 @@ taking that too literally.
| 17 | No completion for tags in the query bar | workflow | M | **done** |
| 18 | No visual cue that there are unsynced edits | feedback | S | **done** |
| 19 | No prompt to sync on exit when edits are pending | behavior | S | **done** |
-| 20 | Thread view does not match the user's mental model | presentation | L | built on branch `item-20-message-rows`, **not merged**; see 53 |
+| 20 | Thread view does not match the user's mental model | presentation | L | rebuilt as cards on branch `card-list`, **not merged**; awaiting the user's verdict |
| 21 | Default shortcuts are not sensible enough | discoverability | S | open |
| 22 | Translatability audit and i18n wiring | correctness | M | open |
| 23 | No way to save a search query from the UI | workflow | M | open |
@@ -108,16 +108,16 @@ taking that too literally.
| 48 | Removing a tag suggests every tag, not the thread's own | workflow | XS | **done** |
| 49 | Sync runs every account regardless of what changed | workflow | M | **done** |
| 50 | Esc blanks the pane but leaves the row selected | workflow | XS | **done** |
-| 51 | Clicking a subject scrolls the list sideways | presentation | XS | open; resolved as a side effect of 53, do not work separately |
+| 51 | Clicking a subject scrolls the list sideways | presentation | XS | built on branch `card-list`, **not merged**; a card is viewport width |
| 52 | `test_querycompleter` fails under Wayland, passes offscreen | testing | XS | **done** |
-| 53 | Message rows still read as a table, not as a conversation | presentation | M | open, specified 2026-08-09; see the card-list spec |
+| 53 | Message rows still read as a table, not as a conversation | presentation | M | built 2026-08-10 on branch `card-list`, **not merged**; awaiting the user's verdict |
| 54 | A cron sync carries the edits but the count still says pending | correctness | S | **done** |
| 55 | In a narrow window the message pane is invisible | presentation | XS | **done** |
| 56 | No action carries an icon, so the toolbar reserves space for nothing | presentation | S | **done** |
| 57 | "Flag" would read better as "Important" or "Starred" | presentation | XS | **done** |
| 58 | `message_zoom` documents a 0.5 to 3.0 range and enforces none of it | correctness | XS | **done** |
| 59 | Archive and Mark all read shipped with the same icon | presentation | XS | **done** |
-| 60 | Next thread dead-ends on the last reply of an expanded thread | defect | XS | open; branch only, fix as part of 53 |
+| 60 | Next thread dead-ends on the last reply of an expanded thread | defect | XS | **done** on the branch; already fixed by 5487d58, see below |
Sizes are rough: XS under an hour, S a sitting, M a session.
@@ -3683,11 +3683,23 @@ navigation actions against the same rule.
current index, which follow visible rows across parent boundaries. For
thread-to-thread jumping, skip any index whose `IsMessageRole` is true.
-**Specified as part of the card-list spec**
-(`docs/superpowers/specs/2026-08-09-card-list-design.md`), which also adds
-Alt+Up/Down for these actions and relies on `QTreeView`'s built-in Up/Down for
-stepping through replies. Fix it there rather than separately, unless the card
-list is dropped.
+**The cause above is wrong, and was corrected on 2026-08-10.** It was read off
+`master`, where the arithmetic really is `current.row() + 1`. The branch does
+not do that: `5487d58` added `MainWindow::threadRowOf()`, which walks up to the
+containing thread BEFORE the arithmetic, so from the last reply of an expanded
+thread `next_thread` already reached the next thread. The defect was fixed in
+the same commit that could have introduced it, one commit before this entry was
+written. Verified by writing both failing tests first, on the branch, and
+watching them pass against unchanged code.
+
+The entry is kept rather than deleted, because the reasoning it records is
+sound and the tests it demanded now exist. It is a reminder that a cause
+"verified in code" is only verified against the branch it was read on.
+
+**Superseded by the card-list work all the same.** Both actions now walk with
+`indexBelow`/`indexAbove` (`card-list`, 2026-08-10), so nothing in that path is
+keyed on a row number, which is the rule a deeper tree would break next.
+Alt+Up/Down were added alongside Ctrl+J/K there.
**Constraint.** The test that would catch this must start from the **last reply
of an expanded thread**. A test that arrows down a collapsed list passes against
diff --git a/docs/superpowers/plans/2026-08-08-item-20-message-rows.md b/docs/superpowers/plans/2026-08-08-item-20-message-rows.md
index 5935777..f4882c7 100644
--- a/docs/superpowers/plans/2026-08-08-item-20-message-rows.md
+++ b/docs/superpowers/plans/2026-08-08-item-20-message-rows.md
@@ -1,14 +1,5 @@
# Item 20: Message Rows in the Thread List — Implementation Plan
-> **EXECUTED AND PARKED, 2026-08-08. Do not run this plan again.**
-> Every task here was implemented on the branch `item-20-message-rows`, which is
-> pushed to both remotes and **not merged**. master has these documents and none
-> of that code. The user's verdict on the finished result was that the table
-> layout does not fit the use, and item 53 in
-> `2026-08-03-post-0.1.0-usability.md` carries the diagnosis. This document is
-> kept for the API facts it verified and the reasoning it records, not as work
-> to pick up. Read item 53 first.
-
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
**Goal:** Turn the flat thread list into a tree where a thread's root row is its first message, expanding reveals the replies indented by reply depth, and selecting a reply opens that single message in the reading pane.
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index cae3bd4..fdac2c1 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -5,6 +5,8 @@ add_library(qtmaildir_lib STATIC
requestinterceptor.cpp
htmlbuilder.cpp
cidschemehandler.cpp
+ cardlayout.cpp
+ carddelegate.cpp
notmuchworker.cpp
tagchip.cpp
tagcolors.cpp
diff --git a/src/carddelegate.cpp b/src/carddelegate.cpp
new file mode 100644
index 0000000..f13205e
--- /dev/null
+++ b/src/carddelegate.cpp
@@ -0,0 +1,261 @@
+/*
+ * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs
+ * Copyright (C) 2026 Danilo M. <danix@danix.xyz>
+ *
+ * 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 "carddelegate.h"
+
+#include "cardlayout.h"
+#include "threadlistmodel.h"
+
+#include <QApplication>
+#include <QDateTime>
+#include <QGuiApplication>
+#include <QPainter>
+#include <QRegularExpression>
+#include <QStyle>
+
+namespace {
+
+CardLayout::Input inputFor(const QModelIndex &index)
+{
+ CardLayout::Input in;
+ in.isMessage = index.data(ThreadListModel::IsMessageRole).toBool();
+ in.depth = index.data(ThreadListModel::MessageDepthRole).toInt();
+ in.replyCount = index.data(ThreadListModel::ReplyCountRole).toInt();
+ return in;
+}
+
+} // namespace
+
+QRect CardDelegate::expanderRectFor(const QStyleOptionViewItem &option,
+ const QModelIndex &index)
+{
+ return CardLayout::compute(inputFor(index), option.rect, option.font)
+ .expanderRect;
+}
+
+QColor CardDelegate::accentLineColour(const QColor &accountColour)
+{
+ if (!accountColour.isValid())
+ return ThreadListModel::threadLineColour();
+
+ // 0.35 toward Base was the first attempt and produced an INVISIBLE bar on
+ // a dark theme: rendered against a Base of (0.169, 0.169, 0.169) it landed
+ // at (0.18, 0.22, 0.26), which is the background. The weight is a fraction
+ // OF THE ACCOUNT COLOUR, so a low one keeps the background, not the hue.
+ //
+ // The bar is the account's colour, undiluted. Blending it toward Base at
+ // all was the mistake: a chip's colour is chosen to carry text on top and
+ // is therefore already muted, and three pixels of a muted colour on a dark
+ // background is nothing at all. There is no text on this bar, so nothing
+ // needs the contrast a chip's fill was picked for.
+ //
+ // What DOES step back is the spine, below: a line running the height of a
+ // whole expansion has to be followable without competing with the senders
+ // beside it, which is a different problem from a 3px edge marker.
+ return accountColour;
+}
+
+QSize CardDelegate::sizeHint(const QStyleOptionViewItem &option,
+ const QModelIndex &index) const
+{
+ Q_UNUSED(index);
+ // One height for every row, thread and reply alike. Asserted directly in
+ // test_cardlayout rather than left to two cards happening to agree.
+ return QSize(option.rect.width(), CardLayout::heightFor(option.font));
+}
+
+void CardDelegate::paint(QPainter *painter, const QStyleOptionViewItem &option,
+ const QModelIndex &index) const
+{
+ // Background, selection and any model fill first, through the style, so a
+ // selected or doomed card looks right before anything is drawn on top.
+ QStyleOptionViewItem chrome = option;
+ initStyleOption(&chrome, index);
+ chrome.text.clear();
+ const QWidget *widget = option.widget;
+ QStyle *style = widget ? widget->style() : QApplication::style();
+ style->drawControl(QStyle::CE_ItemViewItem, &chrome, painter, widget);
+
+ const CardLayout card =
+ CardLayout::compute(inputFor(index), option.rect, option.font);
+
+ painter->save();
+
+ // The account's colour, for both the accent bar and the spines.
+ //
+ // A reply must resolve its THREAD's colour, not its own: AccountColourRole
+ // is empty on a message row, and a spine that fell back to the neutral
+ // line under an accented root would break the one continuous edge this
+ // design is built on. index.parent() is the thread for a depth-1 reply and
+ // the containing subtree for a deeper one, so walk to the root.
+ QModelIndex root = index;
+ while (root.parent().isValid())
+ root = root.parent();
+ const QColor accountColour =
+ root.data(ThreadListModel::AccountColourRole).value<QColor>();
+ const QColor lineColour = accentLineColour(accountColour);
+
+ // The accent bar, thread cards only. Drawn after the chrome so the
+ // selection highlight cannot cover it: which account a card belongs to
+ // must stay readable on the row the user is looking at.
+ if (!card.accentRect.isEmpty())
+ painter->fillRect(card.accentRect, lineColour);
+
+ // Spines, under everything else, in the account's hue so an expanded thread
+ // is bounded by one colour from its root to its last reply. Muted against
+ // the pane's own background, unlike the accent bar: this line runs the full
+ // height of every reply and at full strength it shouts.
+ if (!card.spines.isEmpty()) {
+ const QColor base =
+ QGuiApplication::palette().color(QPalette::Base);
+ constexpr qreal kSpineWeight = 0.55;
+ const qreal inverse = 1.0 - kSpineWeight;
+ const QColor spineColour = QColor::fromRgbF(
+ lineColour.redF() * kSpineWeight + base.redF() * inverse,
+ lineColour.greenF() * kSpineWeight + base.greenF() * inverse,
+ lineColour.blueF() * kSpineWeight + base.blueF() * inverse);
+ for (const QRect &spine : card.spines)
+ painter->fillRect(spine, spineColour);
+ }
+
+ // Selection outranks the model's foreground, and the order matters: a read
+ // card carries a dimmed colour blended against the UNSELECTED background,
+ // so over the highlight it lands grey-on-highlight and close to unreadable.
+ const QVariant foreground = index.data(Qt::ForegroundRole);
+ if (option.state & QStyle::State_Selected)
+ painter->setPen(option.palette.highlightedText().color());
+ else if (foreground.isValid())
+ painter->setPen(foreground.value<QBrush>().color());
+ else
+ painter->setPen(option.palette.text().color());
+
+ // The model's font carries bold for unread and strike-out for deleted;
+ // initStyleOption resolved it into chrome.font.
+ painter->setFont(chrome.font);
+ const QFontMetrics metrics(chrome.font);
+
+ // Line 1: sender, then the date flush right.
+ painter->drawText(card.senderRect, Qt::AlignVCenter | Qt::AlignLeft,
+ metrics.elidedText(
+ index.data(ThreadListModel::SendersRole).toString(),
+ Qt::ElideRight, card.senderRect.width()));
+ const QDateTime date =
+ index.data(ThreadListModel::DateRole).toDateTime();
+ painter->drawText(card.dateRect, Qt::AlignVCenter | Qt::AlignRight,
+ CardLayout::formatDate(date));
+
+ // Line 2: the flag mark, the subject, the attachment mark.
+ QString subject = index.data(ThreadListModel::SubjectRole).toString();
+ if (index.data(ThreadListModel::IsMessageRole).toBool()) {
+ // Every reply repeating "Re: <the thread's subject>" is the visual
+ // signature of a table of records, which is what item 53 is about.
+ static const QRegularExpression re(
+ QStringLiteral("^\\s*(?:[Rr][Ee]\\s*:\\s*)+"));
+ subject.remove(re);
+ }
+ QString line2;
+ if (index.data(ThreadListModel::IsFlaggedRole).toBool())
+ line2 += ThreadListModel::flagGlyph() + QLatin1Char(' ');
+ line2 += subject;
+ if (index.data(ThreadListModel::HasAttachmentRole).toBool())
+ line2 += QLatin1Char(' ') + ThreadListModel::attachmentGlyph();
+ painter->drawText(card.subjectRect, Qt::AlignVCenter | Qt::AlignLeft,
+ metrics.elidedText(line2, Qt::ElideRight,
+ card.subjectRect.width()));
+
+ // The reply count, which is also the expander, drawn as a PILL.
+ //
+ // A bare "3" on the card's own background read as an unexplained number
+ // beside the subject and gave no hint that it could be clicked. The chip
+ // shape says "this is a control", matching the tag chips on line 3, and the
+ // word says what the number counts.
+ if (!card.expanderRect.isEmpty()) {
+ const int count = index.data(ThreadListModel::ReplyCountRole).toInt();
+ const QString label = CardLayout::expanderLabel(
+ count, option.state & QStyle::State_Open);
+
+ painter->save();
+ painter->setFont(CardLayout::smallFont(chrome.font));
+
+ // Blended from Text toward Base rather than taken from a palette ROLE.
+ // QPalette::Button is the role this obviously wants and it is
+ // #2b2b2b against a Base of #2b2b2b on the user's theme: byte
+ // identical, so the pill was invisible. A theme is free to make any two
+ // roles equal, and several do; a blend cannot collide with the surface
+ // it sits on because it is defined relative to it.
+ //
+ // Toward Text, so it darkens on a light theme and lightens on a dark
+ // one, the same trick replyBackground() and threadLineColour() use.
+ const QColor base = option.palette.color(QPalette::Base);
+ const QColor text = option.palette.color(QPalette::Text);
+ constexpr qreal kFillWeight = 0.18;
+ const QColor fill = QColor::fromRgbF(
+ text.redF() * kFillWeight + base.redF() * (1.0 - kFillWeight),
+ text.greenF() * kFillWeight + base.greenF() * (1.0 - kFillWeight),
+ text.blueF() * kFillWeight + base.blueF() * (1.0 - kFillWeight));
+ painter->setRenderHint(QPainter::Antialiasing, true);
+ painter->setPen(Qt::NoPen);
+ painter->setBrush(fill);
+ // Fully rounded ends, the same shape TagChip paints: the radius is half
+ // the height, so the pill cannot look like a rectangle with soft corners.
+ const qreal radius = card.expanderRect.height() / 2.0;
+ painter->drawRoundedRect(card.expanderRect, radius, radius);
+
+ // The pen is restored from the card's own text colour rather than
+ // ButtonText, which belongs to the role that just proved unreliable.
+ painter->setPen(option.state & QStyle::State_Selected
+ ? option.palette.highlightedText().color()
+ : text);
+ painter->drawText(card.expanderRect, Qt::AlignCenter, label);
+ painter->restore();
+ }
+
+ painter->restore();
+
+ // Line 3: the chips. A thread card draws its own tags; a reply draws only
+ // the tags its thread does not already carry, so the thread's chips are
+ // not repeated down the whole expansion.
+ const bool isMessage =
+ index.data(ThreadListModel::IsMessageRole).toBool();
+ const QStringList tags =
+ index.data(isMessage ? ThreadListModel::MessageOwnTagsRole
+ : ThreadListModel::PillTagsRole)
+ .toStringList();
+ const QVariantList colours =
+ index.data(isMessage ? ThreadListModel::MessageOwnColoursRole
+ : ThreadListModel::PillColoursRole)
+ .toList();
+
+ const QFont chipFont = CardLayout::smallFont(chrome.font);
+ const QFontMetrics chipMetrics(chipFont);
+ painter->save();
+ painter->setFont(chipFont);
+ int x = card.tagRect.left();
+ for (int i = 0; i < tags.size(); ++i) {
+ const QSize size = TagChip::sizeFor(chipMetrics, tags.at(i));
+ if (x + size.width() > card.tagRect.right())
+ break; // Out of room; a clipped chip reads as a rendering fault.
+ const QColor colour = i < colours.size()
+ ? colours.at(i).value<QColor>()
+ : QColor(0x55, 0x55, 0x5f);
+ TagChip::paint(painter, QRect(QPoint(x, card.tagRect.top()), size),
+ tags.at(i), colour);
+ x += size.width() + TagChip::kSpacing;
+ }
+ painter->restore();
+}
diff --git a/src/carddelegate.h b/src/carddelegate.h
new file mode 100644
index 0000000..7012eaa
--- /dev/null
+++ b/src/carddelegate.h
@@ -0,0 +1,72 @@
+/*
+ * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs
+ * Copyright (C) 2026 Danilo M. <danix@danix.xyz>
+ *
+ * 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 "tagchip.h"
+
+/// Paints a whole card: three lines, all of it, including the tag chips.
+///
+/// It replaces both SubjectDelegate and ThreadListView::paintEvent. The view
+/// used to paint the tag strip because a delegate cannot paint outside its
+/// column and the strip spanned all five; with one column there is nothing to
+/// span, so the strip comes home to the delegate and the view stops painting
+/// entirely. That removes the two failure modes CLAUDE.md records for the
+/// strip, a deleted row cut in half and every other row showing a bare stripe,
+/// both of which existed because the view had to re-honour alternating
+/// colours, the selection and BackgroundRole across cells it did not own.
+///
+/// Inherits RowStyleDelegate for its one job, which still matters: Qt resolves
+/// Qt::ForegroundRole into the palette's Text roles and then prefers those over
+/// HighlightedText, so the read/unread dimming would otherwise win on a
+/// selected row and land as grey on the highlight colour.
+class CardDelegate : public RowStyleDelegate
+{
+ Q_OBJECT
+public:
+ using RowStyleDelegate::RowStyleDelegate;
+
+ void paint(QPainter *painter, const QStyleOptionViewItem &option,
+ const QModelIndex &index) const override;
+ QSize sizeHint(const QStyleOptionViewItem &option,
+ const QModelIndex &index) const override;
+
+ /// The expander's rect for a row, so the VIEW can hit-test a click without
+ /// duplicating the layout. The delegate draws it and the view owns the
+ /// click, because a delegate gets no click of its own without an editor.
+ static QRect expanderRectFor(const QStyleOptionViewItem &option,
+ const QModelIndex &index);
+
+ /// The colour the accent bar is painted in: the account's own, undiluted.
+ ///
+ /// Blending it toward the palette's Base was tried first, at the 0.35
+ /// weight threadLineColour() uses, and produced an INVISIBLE bar on a dark
+ /// theme: against a Base of (0.169, 0.169, 0.169) it landed at (0.18, 0.22,
+ /// 0.26), which is the background. The weight is a fraction OF THE ACCOUNT
+ /// COLOUR, so a low one keeps the background rather than the hue.
+ ///
+ /// An account colour is already chosen to be a chip's fill with legible
+ /// text on top, so it is muted to begin with; three pixels of a muted
+ /// colour is nothing. Nothing is drawn on this bar, so it needs none of the
+ /// contrast that choice was made for. The SPINE is where the muting belongs
+ /// and is blended in paint(): it runs the full height of every reply in an
+ /// expansion and has to be followable without competing with the senders.
+ ///
+ /// Falls back to threadLineColour() for a thread with no account tag.
+ static QColor accentLineColour(const QColor &accountColour);
+};
diff --git a/src/cardlayout.cpp b/src/cardlayout.cpp
new file mode 100644
index 0000000..f542df0
--- /dev/null
+++ b/src/cardlayout.cpp
@@ -0,0 +1,186 @@
+/*
+ * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs
+ * Copyright (C) 2026 Danilo M. <danix@danix.xyz>
+ *
+ * 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 "cardlayout.h"
+
+#include <QFontMetrics>
+#include <QLocale>
+
+QString CardLayout::formatDate(const QDateTime &date)
+{
+ // The system locale's own short format, not a hardcoded pattern: an
+ // Italian desktop writes 10/08/2025, not 2025-08-10, and a mail client
+ // that disagrees with every other application on screen is simply wrong.
+ return QLocale::system().toString(date, QLocale::ShortFormat);
+}
+
+QString CardLayout::expanderLabel(int replyCount, bool expanded)
+{
+ // "3 replies", not a bare "3". The count alone reads as an unexplained
+ // number beside the subject, and the word is what says the card opens.
+ //
+ // Not translated through tr() here because CardLayout is a plain struct
+ // rather than a QObject; the delegate is where a translated build would
+ // wrap this, and the string is deliberately kept in one place so there is
+ // exactly one thing to change.
+ const QString glyph = expanded ? QStringLiteral("\u25be")
+ : QStringLiteral("\u25b8");
+ const QString word = replyCount == 1 ? QStringLiteral("reply")
+ : QStringLiteral("replies");
+ return QStringLiteral("%1 %2 %3").arg(glyph).arg(replyCount).arg(word);
+}
+
+QString CardLayout::widestDateSample()
+{
+ // A real date run through the same formatter, with the wide digits and a
+ // two-digit day and month, so the reserved width matches what is drawn
+ // whatever the locale's pattern turns out to be. Guessing a pattern here
+ // would reintroduce the clipping this exists to prevent.
+ static const QString sample = [] {
+ const QDateTime wide(QDate(2028, 12, 28), QTime(22, 58));
+ return formatDate(wide);
+ }();
+ return sample;
+}
+
+QFont CardLayout::smallFont(const QFont &cardFont)
+{
+ QFont small = cardFont;
+ // Derived from the card's font rather than fixed, so it follows the
+ // desktop's font size instead of shrinking to nothing on a large one.
+ //
+ // pointSizeF() returns -1 for a font set in PIXELS, which qt6ct and some
+ // styles do. Subtracting from -1 would ask for an invalid size and Qt
+ // would silently keep the original, making the small font the same size as
+ // the card's; the pixel branch avoids that.
+ if (small.pointSizeF() > 0.0)
+ small.setPointSizeF(qMax(6.0, cardFont.pointSizeF() - 1.0));
+ else if (small.pixelSize() > 0)
+ small.setPixelSize(qMax(8, cardFont.pixelSize() - 1));
+ return small;
+}
+
+int CardLayout::heightFor(const QFont &font)
+{
+ const QFontMetrics metrics(font);
+ const QFontMetrics smallMetrics(smallFont(font));
+ // Two lines at the card's font, one at the small one, plus the padding
+ // above the first and below the last.
+ return kPaddingY * 2 + metrics.height() * 2 + smallMetrics.height();
+}
+
+CardLayout CardLayout::compute(const Input &input, const QRect &rect,
+ const QFont &font)
+{
+ CardLayout out;
+ const QFontMetrics metrics(font);
+ const QFontMetrics smallMetrics(smallFont(font));
+
+ out.totalHeight = rect.height();
+
+ // The accent bar sits flush against the card's left edge, on thread cards
+ // only, and everything else starts after it so no text sits on the colour.
+ if (!input.isMessage) {
+ out.accentRect =
+ QRect(rect.left(), rect.top(), kAccentWidth, rect.height());
+ }
+ const int textLeft = rect.left() + kAccentWidth;
+
+ // Indent, capped. qMin rather than a branch so depth 5 and depth 50 land
+ // in exactly the same place.
+ //
+ // A MESSAGE row is nested at least one level whatever depth it reports.
+ // notmuch numbers every message of a thread with no usable In-Reply-To as
+ // depth 0, so a flat thread's replies arrived here claiming no nesting and
+ // drew flush against their own thread with no spine, while a nested
+ // thread's replies indented normally: two different shapes on screen for
+ // the same relationship. Being a child row IS the nesting; the depth only
+ // says how much further to go.
+ const int effectiveDepth =
+ input.isMessage ? qMax(1, input.depth) : input.depth;
+ const int depth = qMin(effectiveDepth, kMaxDepth);
+ const int indent = depth * kIndentStep;
+ out.contentLeft = textLeft + kPaddingX + indent;
+
+ // One spine per level actually indented, each running the card's full
+ // height so an expansion reads as one continuous block.
+ for (int level = 0; level < depth; ++level) {
+ const int x = textLeft + kPaddingX + level * kIndentStep
+ + kIndentStep / 2;
+ out.spines.append(QRect(x, rect.top(), 2, rect.height()));
+ }
+
+ // The EXCLUSIVE right edge: one past the last pixel a card may draw on.
+ // QRect::right() is inclusive (left + width - 1), so building widths from
+ // it directly lands everything one pixel short of the intended padding.
+ const int right = rect.right() + 1 - kPaddingX;
+ const int lineOneTop = rect.top() + kPaddingY;
+ const int lineTwoTop = lineOneTop + metrics.height();
+ const int lineThreeTop = lineTwoTop + metrics.height();
+
+ // The date is measured first and the sender gets what is left, so a long
+ // sender is elided rather than painting over the date.
+ //
+ // Measured BOLD whatever font this is handed. An unread card draws bold and
+ // the delegate computes its layout from the view's regular font, so a rect
+ // sized regular clips a bold date: 154px reserved against 170px needed, one
+ // digit of the year gone from every unread card. Reserving the wider of the
+ // two costs a few pixels on a read card and cannot disagree with what is
+ // painted.
+ QFont dateFont = font;
+ dateFont.setBold(true);
+ const int dateWidth =
+ QFontMetrics(dateFont).horizontalAdvance(widestDateSample());
+ out.dateRect = QRect(right - dateWidth, lineOneTop, dateWidth,
+ metrics.height());
+ out.senderRect = QRect(out.contentLeft, lineOneTop,
+ qMax(0, out.dateRect.left() - out.contentLeft
+ - kPaddingX),
+ metrics.height());
+
+ // The expander is the reply count as a PILL, on line two and on the right.
+ //
+ // Sized from the label actually drawn rather than from a fixed sample, so
+ // the background and the text inside it cannot disagree. Both states of the
+ // glyph are measured because the rect must not change width when the card
+ // is expanded: a pill that resized on click would shift the subject's
+ // elision under the pointer.
+ if (input.replyCount > 0) {
+ const int collapsed = smallMetrics.horizontalAdvance(
+ expanderLabel(input.replyCount, false));
+ const int expanded = smallMetrics.horizontalAdvance(
+ expanderLabel(input.replyCount, true));
+ const int countWidth =
+ qMax(collapsed, expanded) + kPillPaddingX * 2;
+ out.expanderRect = QRect(right - countWidth, lineTwoTop, countWidth,
+ metrics.height());
+ }
+
+ const int subjectRight = out.expanderRect.isEmpty()
+ ? right
+ : out.expanderRect.left() - kPaddingX;
+ out.subjectRect = QRect(out.contentLeft, lineTwoTop,
+ qMax(0, subjectRight - out.contentLeft),
+ metrics.height());
+
+ out.tagRect = QRect(out.contentLeft, lineThreeTop,
+ qMax(0, right - out.contentLeft),
+ smallMetrics.height());
+
+ return out;
+}
diff --git a/src/cardlayout.h b/src/cardlayout.h
new file mode 100644
index 0000000..ef6a563
--- /dev/null
+++ b/src/cardlayout.h
@@ -0,0 +1,150 @@
+/*
+ * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs
+ * Copyright (C) 2026 Danilo M. <danix@danix.xyz>
+ *
+ * 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 <QDateTime>
+#include <QFont>
+#include <QRect>
+#include <QVector>
+
+/// Where everything on a card goes, with no painting and no widget.
+///
+/// Split out from CardDelegate on purpose. A delegate needs a live QPainter and
+/// an exposed view before it draws anything, which is what makes delegate tests
+/// fragile: CLAUDE.md records that viewport()->render() returns a blank image in
+/// several ordinary situations, and that a probe reporting "no ink anywhere" is
+/// far more likely broken than the code it is testing. Every geometric claim
+/// about a card is therefore made here, where a test is a function call.
+///
+/// The card is three lines, always:
+///
+/// sender ................................ date <- senderRect/dateRect
+/// * subject @ v 3 replies <- subjectRect/expanderRect
+/// [tag] [tag] <- tagRect
+struct CardLayout
+{
+ /// What the model says about the row. Deliberately plain data: the layout
+ /// must be computable in a test without a model or a view.
+ struct Input
+ {
+ bool isMessage = false;
+ int depth = 0; ///< 0 for a thread root, 1 for a direct reply.
+ int replyCount = 0; ///< 0 means no expander.
+ };
+
+ /// Width of the account accent bar down a thread card's left edge.
+ ///
+ /// A starting value, not a settled one. Five accounts is enough that two
+ /// colours distinct as chips can read alike as thin stripes, and that can
+ /// only be judged against real cards on the user's own screen and theme
+ /// (Task 10). Widen it there if the accounts are not tellable apart.
+ static constexpr int kAccentWidth = 3;
+
+ /// Horizontal breathing room at the card's edges, measured from the accent
+ /// bar rather than from the card, so text does not sit on the colour.
+ static constexpr int kPaddingX = 8;
+
+ /// Vertical breathing room above the first line and below the last.
+ static constexpr int kPaddingY = 4;
+
+ /// How far one level of reply nesting indents.
+ static constexpr int kIndentStep = 18;
+
+ /// The depth past which nothing indents further.
+ ///
+ /// A mailing-list chain can nest a dozen deep, and without a cap the
+ /// sender is eventually pushed off the right edge. Item 20 accepted that
+ /// deep chains must be capped in the VIEW rather than flattened in the
+ /// model, and this is that cap. Rows past it draw at this depth's indent
+ /// with no marker saying so.
+ static constexpr int kMaxDepth = 4;
+
+ QRect senderRect;
+ QRect dateRect;
+ QRect subjectRect;
+ QRect tagRect;
+
+ /// The reply count's rect, and the click target that toggles the thread.
+ /// Empty when the row has no replies.
+ QRect expanderRect;
+
+ /// The account accent bar down the card's left edge.
+ ///
+ /// Thread cards only. A reply's account is its thread's, stated once at the
+ /// head of the conversation, and a second vertical line in a reply's gutter
+ /// would sit a few pixels from the spine and compete with it. The spine
+ /// carries the accent instead, so an expansion is bounded by one colour
+ /// without ever drawing two lines. Empty on a reply.
+ QRect accentRect;
+
+ /// One full-height vertical line per depth level, outermost first.
+ QVector<QRect> spines;
+
+ /// Where the card's text starts, after any indent.
+ int contentLeft = 0;
+
+ int totalHeight = 0;
+
+ /// The height EVERY row gets, thread and reply alike.
+ ///
+ /// Uniform by design: it keeps setUniformRowHeights(true), which is the
+ /// single cheapest property of this layout, since no scrolling or
+ /// hit-testing arithmetic has to account for rows of differing size. The
+ /// cost is a blank third line on a card with no tags, which was accepted
+ /// explicitly.
+ static int heightFor(const QFont &font);
+
+ /// The font the tag chips and the reply count are drawn in: a size down
+ /// from the card's own, so they read as annotation rather than as a third
+ /// column of content.
+ static QFont smallFont(const QFont &cardFont);
+
+ static CardLayout compute(const Input &input, const QRect &rect,
+ const QFont &font);
+
+ /// How a card writes a date, in the user's own locale.
+ ///
+ /// Never a hardcoded pattern. "yyyy-MM-dd hh:mm" is a US-looking format
+ /// that an Italian desktop does not use, and the whole point of asking the
+ /// system locale is that the user reads dates the way their desktop writes
+ /// them everywhere else.
+ ///
+ /// Shared with the layout so the width reserved for the date and the text
+ /// drawn into it come from one place: a locale whose short format is
+ /// longer than the reserved rect would clip, which is exactly the fault
+ /// bold text produced.
+ static QString formatDate(const QDateTime &date);
+
+ /// The widest string formatDate() can return, for reserving space.
+ static QString widestDateSample();
+
+ /// The expander's label: the reply count with its glyph, as drawn.
+ ///
+ /// Shared with the layout for the same reason as formatDate: the rect
+ /// reserved for the pill and the text put inside it must come from one
+ /// place, or a count wider than the sample the layout guessed at spills
+ /// out of its own background.
+ ///
+ /// `expanded` chooses which way the triangle points.
+ static QString expanderLabel(int replyCount, bool expanded);
+
+ /// Padding inside the expander pill, matching a tag chip's, so the two read
+ /// as the same kind of object on the card.
+ static constexpr int kPillPaddingX = 8;
+};
diff --git a/src/keymap.cpp b/src/keymap.cpp
index 29a5f71..cdd26a1 100644
--- a/src/keymap.cpp
+++ b/src/keymap.cpp
@@ -18,6 +18,8 @@
#include "keymap.h"
+#include <algorithm>
+
#include <QSettings>
QStringList KeyMap::knownActions()
@@ -64,6 +66,18 @@ QList<QPair<QString, QString>> KeyMap::defaultBindings()
return {
{ QStringLiteral("Ctrl+J"), QStringLiteral("next_thread") },
{ QStringLiteral("Ctrl+K"), QStringLiteral("prev_thread") },
+ // Alt, because Shift+Up/Down is QTreeView's built-in extend-selection,
+ // which multi-row tagging depends on, and plain Up/Down is the view's
+ // own navigation, which already steps INTO an expanded thread's
+ // replies and is what gives message-to-message movement for free.
+ //
+ // These must stay chords. Every action is a QAction with
+ // WindowShortcut, dispatched before the focused widget sees the key,
+ // and Qt withholds only plain LETTERS from editable widgets: a bare
+ // Up bound here would break the arrow keys in the query bar, the tag
+ // dialog and the web view at once, exactly as Return did.
+ { QStringLiteral("Alt+Down"), QStringLiteral("next_thread") },
+ { QStringLiteral("Alt+Up"), QStringLiteral("prev_thread") },
{ QStringLiteral("Return"), QStringLiteral("open_thread") },
{ QStringLiteral("Ctrl+E"), QStringLiteral("archive") },
{ QStringLiteral("Ctrl+D"), QStringLiteral("delete") },
@@ -153,6 +167,28 @@ void KeyMap::loadDefaults()
m_bindings.insert(normalizeSequence(binding.first), binding.second);
}
+QList<QKeySequence> KeyMap::sequencesFor(const QString &action) const
+{
+ const QKeySequence primary = sequenceFor(action);
+ if (primary.isEmpty())
+ return {};
+
+ QList<QKeySequence> all{ primary };
+ QList<QKeySequence> rest;
+ for (auto it = m_bindings.cbegin(); it != m_bindings.cend(); ++it) {
+ if (it.value() == action && it.key() != primary)
+ rest.append(it.key());
+ }
+ // QHash iteration order is unspecified, so the tail is sorted rather than
+ // left to chance: an action's shortcut list must not reorder between runs.
+ std::sort(rest.begin(), rest.end(),
+ [](const QKeySequence &a, const QKeySequence &b) {
+ return a.toString() < b.toString();
+ });
+ all += rest;
+ return all;
+}
+
QKeySequence KeyMap::sequenceFor(const QString &action) const
{
// Several sequences can reach one action: the built-in default, which
diff --git a/src/keymap.h b/src/keymap.h
index 1c7df5f..f0addf5 100644
--- a/src/keymap.h
+++ b/src/keymap.h
@@ -55,6 +55,15 @@ public:
/// text, so the menu shows a stable choice rather than a hash-order one.
QKeySequence sequenceFor(const QString &action) const;
+ /// EVERY sequence bound to an action, with sequenceFor()'s choice first.
+ ///
+ /// An action can have more than one binding, and setShortcut() keeps only
+ /// the last: next_thread ships with both Ctrl+J and Alt+Down, and with the
+ /// singular setter whichever arrived second was silently unreachable.
+ /// Ordered rather than hash-ordered, so the menu still advertises the same
+ /// binding sequenceFor() chose.
+ QList<QKeySequence> sequencesFor(const QString &action) const;
+
/// The built-in sequence for an action, ignoring any user override.
static QKeySequence defaultSequenceFor(const QString &action);
diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index 11de1d4..b7efec5 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -42,7 +42,7 @@
#include <QSplitter>
#include <QStandardPaths>
#include <QStatusBar>
-#include <QTableView>
+#include <QScrollBar>
#include <QTimer>
#include <QToolBar>
#include <QVBoxLayout>
@@ -52,6 +52,8 @@
#include "mimeparser.h"
#include "notmuchworker.h"
#include "querycompleter.h"
+#include "carddelegate.h"
+#include "cardlayout.h"
#include "tagchip.h"
#include "tagdialog.h"
#include "threadlistmodel.h"
@@ -99,6 +101,40 @@ QString MainWindow::locksPath()
return g_locksPath;
}
+/// The thread row containing an index: the index itself when it is already a
+/// thread row, its parent when it is a message row.
+///
+/// Replaces the arithmetic on row numbers that a table permitted. In a tree a
+/// row number only identifies a row within one parent, so "current.row() + 1"
+/// means the next SIBLING, which under an expanded thread is the next reply.
+QModelIndex MainWindow::threadRowOf(const QModelIndex &index) const
+{
+ if (!index.isValid())
+ return {};
+ return index.parent().isValid() ? index.parent() : index;
+}
+
+/// Selects a whole row, the way QTableView::selectRow did.
+///
+/// QTreeView has no selectRow, and SelectRows on the selection model is not a
+/// substitute: it governs what a click extends to, not what a programmatic
+/// select() covers.
+void MainWindow::selectRowAt(const QModelIndex &index)
+{
+ if (!index.isValid())
+ return;
+
+ m_threadView->selectionModel()->select(
+ index, QItemSelectionModel::ClearAndSelect | QItemSelectionModel::Rows);
+ m_threadView->setCurrentIndex(index);
+}
+
+/// Selects the top-level thread row at `row`.
+void MainWindow::selectThreadRow(int row)
+{
+ selectRowAt(m_model->index(row, 0, QModelIndex()));
+}
+
void MainWindow::restoreUiState()
{
QSettings state(uiStatePath(), QSettings::IniFormat);
@@ -123,20 +159,15 @@ void MainWindow::restoreUiState()
m_splitter->restoreState(splitter);
}
- // A header blob saved against a different set of columns must be
- // discarded, not restored. QHeaderView::restoreState() returns TRUE for a
- // blob with fewer sections than the model and applies the old widths to
- // the wrong columns: adding the attachment column in front shifted every
- // saved width one place right, silently mangling the layout with no error
- // to detect it by (verified on Qt 6.11). The column count is stored
- // alongside and the blob is only used when it still matches.
- const QByteArray header = state.value(QStringLiteral("threadlist/header"))
- .toByteArray();
- const int savedColumns =
- state.value(QStringLiteral("threadlist/columns")).toInt();
- if (!header.isEmpty() && savedColumns == ThreadListModel::ColumnCount) {
- m_threadView->horizontalHeader()->restoreState(header);
- }
+ // No thread-list header state is read. The pane is one column drawn whole
+ // by CardDelegate, so there are no widths to restore; a blob saved by an
+ // older version is simply ignored (item 53's Upgrading note).
+
+ // Range-guarded on read: a stale or hand-edited file can hold anything,
+ // and setCurrentIndex() on a value with no row silently selects nothing.
+ const int sort =
+ state.value(QStringLiteral("threadlist/sortOrder"), 0).toInt();
+ m_sortOrder->setCurrentIndex(sort == 1 ? 1 : 0);
// The config value is the starting point for a profile that has never
// zoomed; once the user does, the state file is what they last had.
@@ -153,11 +184,8 @@ void MainWindow::saveUiState() const
state.setValue(QStringLiteral("window/geometry"), saveGeometry());
state.setValue(QStringLiteral("window/state"), saveState());
state.setValue(QStringLiteral("window/splitter"), m_splitter->saveState());
- state.setValue(QStringLiteral("threadlist/header"),
- m_threadView->horizontalHeader()->saveState());
- // Guards the blob above: see restoreUiState().
- state.setValue(QStringLiteral("threadlist/columns"),
- int(ThreadListModel::ColumnCount));
+ state.setValue(QStringLiteral("threadlist/sortOrder"),
+ m_sortOrder->currentIndex());
state.setValue(QStringLiteral("message/zoom"), m_messageView->zoomFactor());
}
@@ -289,8 +317,10 @@ MainWindow::MainWindow(const Config &config, QWidget *parent)
qRegisterMetaType<MessageRef>();
qRegisterMetaType<TagChange>();
qRegisterMetaType<DatabaseStats>();
+ qRegisterMetaType<MessageNode>();
qRegisterMetaType<QVector<ThreadSummary>>();
qRegisterMetaType<QVector<MessageRef>>();
+ qRegisterMetaType<QVector<MessageNode>>();
m_keyMap.loadDefaults();
{
@@ -397,9 +427,34 @@ void MainWindow::buildUi()
// Query row.
auto *queryRow = new QHBoxLayout;
m_accountBox = new QComboBox(central);
+ m_accountBox->setObjectName(QStringLiteral("accountBox"));
m_accountBox->addItem(tr("All accounts"), QString());
- for (const Account &account : m_config.accounts())
+ for (const Account &account : m_config.accounts()) {
m_accountBox->addItem(account.key, account.key);
+ // The RAW account colour here, not CardDelegate's blended line colour:
+ // a swatch is a filled patch like a chip, not a thin line, so it wants
+ // the colour the account was actually given. Qt renders a
+ // DecorationRole colour as a swatch itself, with no delegate.
+ //
+ // This is what makes the accent bar on a card mean anything: a colour
+ // down a card's edge says nothing until something maps it to a name.
+ m_accountBox->setItemData(
+ m_accountBox->count() - 1,
+ m_tagColors.colourFor(TagColors::tagForAccountKey(account.key)),
+ Qt::DecorationRole);
+ }
+
+ // Sort order. Two entries, straight to notmuch: this ADDS a feature rather
+ // than replacing one, since the old column header was decorative and
+ // nothing implemented click-to-sort.
+ m_sortOrder = new QComboBox(central);
+ m_sortOrder->setObjectName(QStringLiteral("sortOrder"));
+ // Order matters: the index is what uistate.conf stores.
+ m_sortOrder->addItem(tr("Newest first"));
+ m_sortOrder->addItem(tr("Oldest first"));
+ m_sortOrder->setToolTip(tr("The order threads are listed in"));
+ connect(m_sortOrder, &QComboBox::currentIndexChanged,
+ this, &MainWindow::runCurrentQuery);
m_queryEdit = new QLineEdit(central);
m_queryEdit->setPlaceholderText(tr("notmuch query, e.g. tag:inbox"));
@@ -488,6 +543,7 @@ void MainWindow::buildUi()
// would squeeze the field, but three is the real-world case today. Item 23
// already specifies buttons-plus-menu and is where that belongs.
queryRow->addWidget(m_accountBox);
+ queryRow->addWidget(m_sortOrder);
queryRow->addWidget(m_queryEdit, 1);
for (const SavedQuery &saved : m_config.savedQueries()) {
auto *button = new QPushButton(saved.name, central);
@@ -507,58 +563,48 @@ void MainWindow::buildUi()
// delegate is confined to one column's rectangle.
m_threadView = new ThreadListView(central);
m_threadView->setModel(m_model);
+ m_threadView->setItemDelegate(new CardDelegate(this));
+ m_threadView->setHeaderHidden(true);
m_threadView->setSelectionBehavior(QAbstractItemView::SelectRows);
m_threadView->setSelectionMode(QAbstractItemView::ExtendedSelection);
- m_threadView->verticalHeader()->hide();
- m_threadView->horizontalHeader()->setStretchLastSection(false);
- // Every column Interactive, Subject included: Stretch and ResizeToContents
- // both compute a width and discard the user's drag. Nothing absorbs spare
- // width as a result, so the columns end where they end.
- for (int column = 0; column < ThreadListModel::ColumnCount; ++column) {
- m_threadView->horizontalHeader()->setSectionResizeMode(
- column, QHeaderView::Interactive);
- }
- // Two delegates, and the split is not cosmetic. RowStyleDelegate carries
- // only the selection fix every column needs: the read/unread dimming
- // arrives as a Qt::ForegroundRole, which Qt's painting prefers over the
- // highlight, leaving a selected read row grey on the selection colour.
- //
- // SubjectDelegate adds the account chip and the tag pills, and must go on
- // the subject column ALONE. It reads AccountLabelRole, a property of the
- // row rather than of a cell, so installed view-wide it draws the chip into
- // every column: tried once, and the list came out with a chip repeated
- // four times per row.
- m_threadView->setItemDelegate(new RowStyleDelegate(this));
- m_threadView->setItemDelegateForColumn(ThreadListModel::SubjectColumn,
- new SubjectDelegate(this));
-
- // One height for every row, set here rather than left to a column's
- // sizeHint: a QTableView takes a single height per row, so a hint from the
- // subject column alone would only apply if the view happened to ask it.
- m_threadView->verticalHeader()->setDefaultSectionSize(
- SubjectDelegate::rowHeightFor(m_threadView->font()));
- // Widening a column past the viewport scrolls rather than squeezing the
- // others. Per-pixel so the scroll does not jump a whole column at a time.
- // Banding, so the eye can follow a row across four columns and a pill
- // strip without losing it. The colour comes from the palette's
- // AlternateBase, so it follows the desktop theme.
+ // No style-drawn branch decoration. CardDelegate draws the expander itself,
+ // because drawBranches runs BEFORE the row's cells and the delegate's own
+ // background paints straight over anything put there: a 60-pixel triangle
+ // once survived as 8. Leaving both enabled would draw the theme's dot
+ // underneath the delegate's glyph.
+ m_threadView->setRootIsDecorated(false);
+
+ // Zero, because CardLayout draws the indent itself. Qt's own indentation
+ // would shift the card's rect, and every rect on the card is measured from
+ // that rect's left edge, so the two would compound.
+ m_threadView->setIndentation(0);
+
+ // One height for every row. A QTreeView has no vertical header to carry a
+ // default section size, so the height comes from uniformRowHeights plus
+ // CardDelegate::sizeHint.
+ m_threadView->setUniformRowHeights(true);
+
+ // Banding, so the eye can follow a card across the pane. The colour comes
+ // from the palette's AlternateBase, so it follows the desktop theme.
m_threadView->setAlternatingRowColors(true);
- m_threadView->setHorizontalScrollBarPolicy(Qt::ScrollBarAsNeeded);
- m_threadView->setHorizontalScrollMode(QAbstractItemView::ScrollPerPixel);
-
- // Starting widths only; a drag overrides them, and they are what the
- // saved-widths item will persist.
- // Without this the attachment column cannot be narrow at all: the default
- // minimum section size is 58px on this platform, and setColumnWidth()
- // clamps to it silently rather than reporting the smaller value back.
- m_threadView->horizontalHeader()->setMinimumSectionSize(24);
- m_threadView->setColumnWidth(ThreadListModel::AttachmentColumn, 28);
- m_threadView->setColumnWidth(ThreadListModel::FlagColumn, 28);
- m_threadView->setColumnWidth(ThreadListModel::DateColumn, 130);
- m_threadView->setColumnWidth(ThreadListModel::AuthorsColumn, 180);
- m_threadView->setColumnWidth(ThreadListModel::SubjectColumn, 520);
+ // A card is exactly viewport width, so there is nothing to scroll to
+ // sideways. Turning the bar off is what closes item 51: a click used to
+ // scroll the list horizontally, because the subject column was wider than
+ // the viewport and auto-scroll brought the clicked index fully into view.
+ m_threadView->setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOff);
+
+ // Scrolling a whole card at a time rather than a fraction of one, so a
+ // card is never left half above the top edge.
+ m_threadView->verticalScrollBar()->setSingleStep(
+ CardLayout::heightFor(m_threadView->font()));
+
+ // Replies are loaded when a thread is expanded, not with the query.
+ // Walking the reply tree of every thread in a 10k-thread result would cost
+ // far more than the query itself and almost none of it would be looked at.
+ connect(m_threadView, &QTreeView::expanded,
+ this, &MainWindow::onThreadExpanded);
connect(m_threadView->selectionModel(),
&QItemSelectionModel::currentRowChanged,
@@ -617,9 +663,11 @@ QAction *MainWindow::addAction(const QString &name, const QString &text,
// The binding comes from KeyMap, so a [keys] override reaches the menus
// and the shortcut reference as well as the keyboard.
- const QKeySequence sequence = m_keyMap.sequenceFor(name);
- if (!sequence.isEmpty())
- action->setShortcut(sequence);
+ // Plural: an action can carry more than one binding, and setShortcut()
+ // keeps only the last one given. next_thread has both Ctrl+J and Alt+Down.
+ const QList<QKeySequence> sequences = m_keyMap.sequencesFor(name);
+ if (!sequences.isEmpty())
+ action->setShortcuts(sequences);
// Shortcuts must work while focus is in the thread list or the message
// view, not only on the window itself.
@@ -643,16 +691,32 @@ void MainWindow::registerActions()
});
addAction(QStringLiteral("next_thread"), tr("&Next thread"),
tr("Select the next thread"), [this]() {
- const QModelIndex current = m_threadView->currentIndex();
- const int row = current.isValid() ? current.row() + 1 : 0;
- if (row < m_model->rowCount())
- m_threadView->selectRow(row);
+ // Walked by INDEX, never by row number. A tree numbers rows per
+ // parent, so current.row() + 1 names a SIBLING: from the last reply of
+ // an expanded thread it asks for a row that does not exist, and from a
+ // thread row it counts top-level threads only by accident (item 60).
+ //
+ // The skip loop is what keeps this meaning thread-to-thread while the
+ // view's own Up/Down still steps message-to-message.
+ QModelIndex index = m_threadView->indexBelow(
+ m_threadView->currentIndex());
+ while (index.isValid()
+ && index.data(ThreadListModel::IsMessageRole).toBool()) {
+ index = m_threadView->indexBelow(index);
+ }
+ if (index.isValid())
+ selectRowAt(index);
});
addAction(QStringLiteral("prev_thread"), tr("&Previous thread"),
tr("Select the previous thread"), [this]() {
- const QModelIndex current = m_threadView->currentIndex();
- if (current.isValid() && current.row() > 0)
- m_threadView->selectRow(current.row() - 1);
+ QModelIndex index = m_threadView->indexAbove(
+ m_threadView->currentIndex());
+ while (index.isValid()
+ && index.data(ThreadListModel::IsMessageRole).toBool()) {
+ index = m_threadView->indexAbove(index);
+ }
+ if (index.isValid())
+ selectRowAt(index);
});
addAction(QStringLiteral("open_thread"), tr("&Open thread"),
tr("Focus the thread list"), [this]() {
@@ -802,6 +866,7 @@ void MainWindow::registerActions()
// thread straight back, which is the queued-reply race documented in
// CLAUDE.md.
m_currentThreadId.clear();
+ m_currentMessageId.clear();
m_messageView->clear();
showPlaceholderPane();
m_markReadTimer->stop();
@@ -835,6 +900,7 @@ void MainWindow::registerActions()
m_threadView->setCurrentIndex(QModelIndex());
m_currentThreadId.clear();
+ m_currentMessageId.clear();
m_messageView->clear();
showPlaceholderPane();
m_markReadTimer->stop();
@@ -985,7 +1051,7 @@ void MainWindow::buildMenus()
m_threadContextMenu->addAction(m_actions.value(QStringLiteral("select_all")));
m_threadView->setContextMenuPolicy(Qt::CustomContextMenu);
- connect(m_threadView, &QTableView::customContextMenuRequested,
+ connect(m_threadView, &QWidget::customContextMenuRequested,
this, &MainWindow::showThreadContextMenu);
// The frequent subset only. A toolbar holding every action is as
@@ -1245,6 +1311,10 @@ void MainWindow::wireWorker()
this, &MainWindow::onThreadsReady);
connect(m_worker, &NotmuchWorker::queryFinished,
this, &MainWindow::onQueryFinished);
+ connect(m_worker, &NotmuchWorker::threadTreeLoaded,
+ this, &MainWindow::onThreadTreeLoaded);
+ connect(m_worker, &NotmuchWorker::messageLoaded,
+ this, &MainWindow::onMessageLoaded);
connect(m_worker, &NotmuchWorker::threadLoaded,
this, &MainWindow::onThreadLoaded);
connect(m_worker, &NotmuchWorker::errorOccurred,
@@ -1429,9 +1499,13 @@ void MainWindow::runCurrentQuery()
m_queryComplete = false;
updateViewWideActions();
+ const auto sort = m_sortOrder->currentIndex() == 1
+ ? NotmuchWorker::OldestFirst
+ : NotmuchWorker::NewestFirst;
QMetaObject::invokeMethod(m_worker, "runQuery", Qt::QueuedConnection,
Q_ARG(QString, query),
- Q_ARG(quint64, m_generation));
+ Q_ARG(quint64, m_generation),
+ Q_ARG(NotmuchWorker::SortOrder, sort));
}
void MainWindow::onThreadsReady(const QVector<ThreadSummary> &threads,
@@ -1519,41 +1593,90 @@ void MainWindow::showThreadContextMenu(const QPoint &pos)
// collapsing to the clicked row here would silently narrow a deliberate
// multi-row selection to one. Right-clicking outside it selects that row
// instead, which is what every other list does.
- if (!m_threadView->selectionModel()->isRowSelected(index.row()))
- m_threadView->selectRow(index.row());
+ if (!m_threadView->selectionModel()->isSelected(index))
+ selectRowAt(index);
m_threadContextMenu->popup(m_threadView->viewport()->mapToGlobal(pos));
}
void MainWindow::onSelectionChanged()
{
- const int selected = m_threadView->selectionModel()->selectedRows().size();
- if (selected <= 1) {
- // Clearing the count here would wipe whatever the last action reported
- // ("Archive: 3 threads"), which is the more useful message once the
- // selection is gone. Only a count this function wrote is taken back.
- if (m_statusLabel->text() == m_selectionMessage)
- m_statusLabel->clear();
- m_selectionMessage.clear();
+ const QModelIndexList rows = m_threadView->selectionModel()->selectedRows();
+ const int selected = rows.size();
+ if (selected == 1) {
+ // One row selected. With two kinds of row this is exactly where the
+ // scope became ambiguous: a thread root stands for every message in it,
+ // a message row for one, and the keypress looks identical. Naming it
+ // here is what this project does instead of a confirmation dialog,
+ // which CLAUDE.md rules out for tag mutations.
+ const ActionScope scope = m_model->scopeFor(rows);
+
+ if (scope.wholeThread) {
+ m_selectionMessage =
+ tr("1 thread selected (%n message(s))", "", scope.messageCount);
+ m_statusLabel->setText(m_selectionMessage);
+ m_statusTimer->stop();
+ m_transientMessage.clear();
+ } else {
+ // Reading one message is not a bulk action and gets no count.
+ if (m_statusLabel->text() == m_selectionMessage)
+ m_statusLabel->clear();
+ m_selectionMessage.clear();
+ }
- // Collapsing a multi-row selection back to one row has to load that
- // row here, and cannot be left to onThreadSelected. currentRowChanged
- // is emitted BEFORE the selection model is updated (verified against
- // Qt 6.11), so when a click collapses three rows to one, that handler
- // still sees three selected, takes the multi-select branch and returns
- // without loading anything. Only this signal sees the real count.
+ // Collapsing a multi-row selection back to one row has to load that row
+ // here, and cannot be left to onThreadSelected: currentRowChanged is
+ // emitted BEFORE the selection model is updated (verified against
+ // Qt 6.11), so that handler still sees the old count and returns
+ // without loading anything.
+ //
+ // Compared per row kind. A message row's row number indexes its
+ // siblings, so threadAt() on one answers about an unrelated thread and
+ // the comparison below would be against the wrong id.
const QModelIndex current = m_threadView->currentIndex();
- if (current.isValid()
- && m_model->threadAt(current.row()).threadId != m_currentThreadId) {
- onThreadSelected(current, QModelIndex());
+ if (current.isValid()) {
+ const bool changed =
+ m_model->isMessageRow(current)
+ ? m_model->messageAt(current).messageId != m_currentMessageId
+ : m_model->threadAt(current.row()).threadId
+ != m_currentThreadId;
+ if (changed)
+ onThreadSelected(current, QModelIndex());
}
return;
}
+ if (selected < 1) {
+ // Nothing selected. Clearing unconditionally would wipe whatever the
+ // last action reported ("Archive: 3 threads"), which is the more useful
+ // message once the selection is gone, so only a count this function
+ // wrote is taken back.
+ if (m_statusLabel->text() == m_selectionMessage)
+ m_statusLabel->clear();
+ m_selectionMessage.clear();
+ return;
+ }
+
// The count is the part that actually teaches multi-select: it acknowledges
// the selection while it is being built, rather than only after an action
// has already been applied to it.
- m_selectionMessage = tr("%n thread(s) selected", "", selected);
+ //
+ // Reported per row kind rather than as a bare row count, so a mixed
+ // selection says what it will really touch instead of calling three replies
+ // "3 threads".
+ const ActionScope scope = m_model->scopeFor(rows);
+ if (!scope.threadIds.isEmpty() && scope.messageIds.isEmpty()) {
+ m_selectionMessage =
+ tr("%n thread(s) selected (%1 messages)", "", scope.threadIds.size())
+ .arg(scope.messageCount);
+ } else if (scope.threadIds.isEmpty()) {
+ m_selectionMessage =
+ tr("%n message(s) selected", "", scope.messageIds.size());
+ } else {
+ m_selectionMessage =
+ tr("%n thread(s) and %1 message(s) selected", "",
+ scope.threadIds.size()).arg(scope.messageIds.size());
+ }
m_statusLabel->setText(m_selectionMessage);
// State, not an event: it must persist while the selection does. Cancel any
@@ -1568,6 +1691,7 @@ void MainWindow::onSelectionChanged()
m_markReadTimer->stop();
m_markReadThreadId.clear();
m_currentThreadId.clear();
+ m_currentMessageId.clear();
m_messageView->clear();
showPlaceholderPane();
}
@@ -1599,21 +1723,121 @@ void MainWindow::onThreadSelected(const QModelIndex &current,
m_markReadTimer->stop();
m_markReadThreadId.clear();
m_currentThreadId.clear();
+ m_currentMessageId.clear();
m_messageView->clear();
showPlaceholderPane();
return;
}
+ // A message row renders that message ALONE. Checked before threadAt(),
+ // which takes a top-level row number: a child's row number indexes its
+ // siblings, so passing it here would silently load whichever thread happens
+ // to sit at that position in the list.
+ if (m_model->isMessageRow(current)) {
+ const MessageNode node = m_model->messageAt(current);
+ if (node.messageId.isEmpty())
+ return;
+
+ // No mark-read timer for a message row in this pass. Marking one
+ // message of a thread read is a per-message tag write, and the
+ // pending-edit map is keyed by thread; item 28 is the record of what
+ // happens when that count goes wrong.
+ m_markReadTimer->stop();
+ m_markReadThreadId.clear();
+
+ m_currentThreadId.clear();
+ m_currentMessageId = node.messageId;
+ m_messageView->setTags(node.tags);
+ QMetaObject::invokeMethod(m_worker, "loadMessage", Qt::QueuedConnection,
+ Q_ARG(QString, node.messageId),
+ Q_ARG(quint64, m_generation));
+ return;
+ }
+
const ThreadSummary thread = m_model->threadAt(current.row());
m_currentThreadId = thread.threadId;
m_messageView->setTags(thread.tags);
scheduleMarkRead(thread);
+
+ // The root card IS the thread's first message, so selecting it renders
+ // that message rather than the whole conversation. Loading the thread here
+ // made the first message unreachable: the pane showed every message with
+ // only the last expanded, and no row in the list offered the first one,
+ // since the reply rows are messages two onward.
+ //
+ // Known only once the replies have been loaded, which happens when the
+ // thread is expanded. Until then the thread is the honest answer: it
+ // contains the first message, where a guess might not.
+ const QString firstId =
+ m_model->data(current, ThreadListModel::MessageIdRole).toString();
+ if (!firstId.isEmpty()) {
+ m_currentMessageId = firstId;
+ QMetaObject::invokeMethod(m_worker, "loadMessage",
+ Qt::QueuedConnection,
+ Q_ARG(QString, firstId),
+ Q_ARG(quint64, m_generation));
+ return;
+ }
+
+ m_currentMessageId.clear();
QMetaObject::invokeMethod(m_worker, "loadThread", Qt::QueuedConnection,
Q_ARG(QString, m_currentThreadId),
Q_ARG(QString, m_lastQuery),
Q_ARG(quint64, m_generation));
}
+void MainWindow::onMessageLoaded(const QVector<MessageRef> &messages,
+ quint64 generation)
+{
+ // The same two guards onThreadLoaded carries. A stale generation means the
+ // query moved on, and a reply landing after the selection grew past one row
+ // would paint a message back over a deliberately blanked pane.
+ if (generation != m_generation || messages.isEmpty())
+ return;
+ if (m_threadView->selectionModel()->selectedRows().size() > 1)
+ return;
+
+ // A third guard this one needs and onThreadLoaded does not: a reply that
+ // lands after the selection moved to a THREAD row would render one message
+ // where the whole conversation belongs.
+ if (m_currentMessageId.isEmpty())
+ return;
+
+ onThreadLoaded(messages, generation);
+}
+
+void MainWindow::onThreadExpanded(const QModelIndex &index)
+{
+ if (!index.isValid() || m_model->isMessageRow(index))
+ return;
+
+ const QString threadId =
+ m_model->data(index, ThreadListModel::ThreadIdRole).toString();
+ if (threadId.isEmpty())
+ return;
+
+ QMetaObject::invokeMethod(m_worker, "loadThreadTree", Qt::QueuedConnection,
+ Q_ARG(QString, threadId),
+ Q_ARG(QString, m_lastQuery),
+ Q_ARG(quint64, m_generation));
+}
+
+void MainWindow::onThreadTreeLoaded(const QVector<MessageNode> &nodes,
+ quint64 generation)
+{
+ // The same generation guard every other worker reply carries: an expansion
+ // whose query has since been replaced must not insert rows into the new
+ // result, where that thread may not even appear.
+ if (generation != m_generation || nodes.isEmpty())
+ return;
+
+ // Every node in one reply belongs to one thread, so the first one names it.
+ // Read from the node rather than remembered from the request: two
+ // expansions can be in flight at once, and pairing them by order would
+ // attach one thread's replies to the other.
+ m_model->setThreadMessages(nodes.first().threadId, nodes);
+}
+
void MainWindow::onThreadLoaded(const QVector<MessageRef> &messages,
quint64 generation)
{
@@ -2297,20 +2521,83 @@ void MainWindow::tagSelected(const QStringList &add, const QStringList &remove,
if (rows.isEmpty())
return;
- QStringList threadIds;
- threadIds.reserve(rows.size());
- for (const QModelIndex &index : rows)
- threadIds.append(m_model->threadAt(index.row()).threadId);
+ // Resolved through the model rather than by mapping rows to threads here.
+ // A message row's row number indexes its siblings, so the old
+ // threadAt(index.row()) mapping silently acted on whichever thread sat at
+ // that position in the list.
+ const ActionScope scope = m_model->scopeFor(rows);
+ if (scope.isEmpty())
+ return;
- sendThreadTagChange(threadIds, add, remove, description);
+ if (!scope.threadIds.isEmpty()) {
+ sendThreadTagChange(scope.threadIds, add, remove, description);
- // Pushed for undo. The inverse re-resolves the same threads, so it works
- // whether or not those rows are still selected.
- m_undoStack.push(new ThreadTagCommand(this, threadIds, add, remove,
- description));
+ // Pushed for undo. The inverse re-resolves the same threads, so it
+ // works whether or not those rows are still selected.
+ m_undoStack.push(new ThreadTagCommand(this, scope.threadIds, add,
+ remove, description));
+ }
+
+ if (!scope.messageIds.isEmpty()) {
+ sendMessageTagChange(scope.messageIds, add, remove, description);
+ m_undoStack.push(new MessageTagCommand(this, scope.messageIds, add,
+ remove, description));
+ }
+ // The scope named after the fact, since the selection may well be gone by
+ // the time the user reads it. This is what stands in for the confirmation
+ // dialog CLAUDE.md rules out: undo is the safety net, and undo is only
+ // usable if the user can tell that something larger than they meant has
+ // just happened.
showTransientStatus(
- tr("%1: %n thread(s)", "", threadIds.size()).arg(description));
+ scope.wholeThread
+ ? tr("%1: %n message(s) (whole thread)", "", scope.messageCount)
+ .arg(description)
+ : tr("%1: %n message(s)", "", scope.messageCount).arg(description));
+}
+
+void MainWindow::sendMessageTagChange(const QStringList &messageIds,
+ const QStringList &add,
+ const QStringList &remove,
+ const QString &description)
+{
+ if (messageIds.isEmpty())
+ return;
+
+ // No optimistic model update. applyTagChange is keyed by THREAD and would
+ // repaint the whole row as though every message in it had changed, which
+ // for a one-message edit is a lie the user would see and then watch
+ // silently correct itself on the next query.
+
+ // The accounts this touches, resolved through the containing threads: the
+ // account is a property of the thread, and the sync needs the channel
+ // whether one message moved or seven.
+ for (const QString &messageId : messageIds) {
+ const QString threadId = m_model->threadIdForMessage(messageId);
+ if (threadId.isEmpty())
+ continue;
+ for (const QString &key : m_model->accountKeysForThread(threadId))
+ m_editedAccounts.insert(key);
+ }
+
+ // Held during a sync for exactly the reason the thread path is: the
+ // worker's read-write open BLOCKS on notmuch's exclusive lock rather than
+ // failing, so sending now would freeze the worker for the rest of the run.
+ if (aSyncHoldsTheWriteLock()) {
+ m_heldEdits.append(HeldEdit{
+ {}, TagChange{ messageIds, add, remove, description } });
+ m_statusLabel->setText(
+ tr("A sync is running; your change will be applied when it "
+ "finishes."));
+ updatePendingIndicator();
+ return;
+ }
+
+ m_pendingThreadIds.clear();
+ m_pendingChange = TagChange{ messageIds, add, remove, description };
+
+ QMetaObject::invokeMethod(m_worker, "applyTags", Qt::QueuedConnection,
+ Q_ARG(TagChange, m_pendingChange));
}
void MainWindow::sendThreadTagChange(const QStringList &threadIds,
diff --git a/src/mainwindow.h b/src/mainwindow.h
index d8401a3..ccac5ad 100644
--- a/src/mainwindow.h
+++ b/src/mainwindow.h
@@ -41,7 +41,7 @@
class QAction;
class QLineEdit;
class QMenu;
-class QTableView;
+class ThreadListView;
class QLabel;
class QPushButton;
class QComboBox;
@@ -129,6 +129,19 @@ public:
/// command was pushed, which is what "this did nothing" has to assert.
int undoDepthForTesting() const { return m_undoStack.count(); }
+ /// The ids the last tag change was sent for, and whether they were thread
+ /// ids or message ids.
+ ///
+ /// Exposed because the difference is invisible from outside otherwise: a
+ /// message row routed down the thread path produces the same undo depth and
+ /// the same status text while tagging every sibling in the thread. A
+ /// mutation that made exactly that change passed the whole suite.
+ QStringList pendingThreadIdsForTesting() const { return m_pendingThreadIds; }
+ QStringList pendingMessageIdsForTesting() const
+ {
+ return m_pendingChange.messageIds;
+ }
+
/// The generation a worker reply must carry to be accepted.
///
/// A test seam: onQueryFinished() discards a reply whose generation is
@@ -164,6 +177,17 @@ private slots:
/// the click lands inside.
void showThreadContextMenu(const QPoint &pos);
void onThreadLoaded(const QVector<MessageRef> &messages, quint64 generation);
+
+ /// Asks the worker for a thread's reply tree when its row is expanded.
+ void onThreadExpanded(const QModelIndex &index);
+
+ /// Fills in the expanded thread's message rows.
+ void onThreadTreeLoaded(const QVector<MessageNode> &nodes,
+ quint64 generation);
+
+ /// Renders the single message a message row asked for.
+ void onMessageLoaded(const QVector<MessageRef> &messages,
+ quint64 generation);
void onWorkerError(const QString &message);
void onSyncFinished(bool success, int exitCode);
@@ -212,6 +236,15 @@ private:
/// A missing or rejected blob leaves the buildUi() defaults in place.
void restoreUiState();
+ /// The thread row containing an index: itself for a thread row, its parent
+ /// for a message row.
+ QModelIndex threadRowOf(const QModelIndex &index) const;
+
+ /// Selects a whole row. QTreeView has no selectRow of its own.
+ void selectRowAt(const QModelIndex &index);
+
+ /// Selects the top-level thread row at `row`.
+ void selectThreadRow(int row);
void saveUiState() const;
void registerActions();
@@ -335,6 +368,13 @@ private:
const QStringList &remove,
const QString &description);
+ /// The same for individual MESSAGES, without touching the undo stack.
+ /// Both tagSelected() and MessageTagCommand route through this.
+ void sendMessageTagChange(const QStringList &messageIds,
+ const QStringList &add,
+ const QStringList &remove,
+ const QString &description);
+
/// Undoes the optimistic model update for a write the worker rejected.
void revertPendingTagChange();
@@ -368,6 +408,7 @@ private:
QVector<HeldEdit> m_heldEdits;
friend class ThreadTagCommand;
+ friend class MessageTagCommand;
Config m_config;
KeyMap m_keyMap;
@@ -410,7 +451,10 @@ private:
QLineEdit *m_queryEdit = nullptr;
QueryCompleter *m_queryCompleter = nullptr;
- QTableView *m_threadView = nullptr;
+ /// Its own type, not the QTreeView base. The strip painting and the
+ /// expander column are ThreadListView's, and holding the base here only
+ /// hid that from every reader.
+ ThreadListView *m_threadView = nullptr;
/// Right-click menu for the thread list, holding the same QActions the
/// menu bar does.
@@ -424,6 +468,7 @@ private:
/// placeholder's own text wraps every couple of words.
static constexpr int kMinMessagePaneWidth = 300;
QComboBox *m_accountBox = nullptr;
+ QComboBox *m_sortOrder = nullptr;
QLabel *m_statusLabel = nullptr;
/// Expires a transient status message. See showTransientStatus().
@@ -495,6 +540,11 @@ private:
QString m_lastQuery;
QString m_currentThreadId;
+ /// The message a MESSAGE row is showing, empty whenever the pane holds a
+ /// whole thread. The two are mutually exclusive and each clears the other,
+ /// so a late reply can tell which kind of selection it belongs to.
+ QString m_currentMessageId;
+
/// The selection count last written to the status bar, so it can be taken
/// back without clobbering a message some other action put there.
QString m_selectionMessage;
@@ -601,3 +651,47 @@ private:
QString m_description;
bool m_firstRedo = true;
};
+
+/// Undo entry for a tag change over individual MESSAGES.
+///
+/// Stores message ids, unlike ThreadTagCommand, and that difference is the
+/// point rather than an inconsistency: a message row acts on one message, so
+/// re-resolving its thread on undo would restore tags across every sibling the
+/// action never touched.
+class MessageTagCommand : public QUndoCommand
+{
+public:
+ MessageTagCommand(MainWindow *window, const QStringList &messageIds,
+ const QStringList &add, const QStringList &remove,
+ const QString &description)
+ : QUndoCommand(description), m_window(window),
+ m_messageIds(messageIds), m_add(add), m_remove(remove),
+ m_description(description) {}
+
+ /// The stack calls redo() when the command is pushed, by which point the
+ /// change has already been sent, so the first call is skipped.
+ void redo() override
+ {
+ if (m_firstRedo) {
+ m_firstRedo = false;
+ return;
+ }
+ m_window->sendMessageTagChange(m_messageIds, m_add, m_remove,
+ m_description);
+ }
+
+ void undo() override
+ {
+ m_window->sendMessageTagChange(
+ m_messageIds, m_remove, m_add,
+ QStringLiteral("Undo %1").arg(m_description));
+ }
+
+private:
+ MainWindow *m_window;
+ QStringList m_messageIds;
+ QStringList m_add;
+ QStringList m_remove;
+ QString m_description;
+ bool m_firstRedo = true;
+};
diff --git a/src/notmuchworker.cpp b/src/notmuchworker.cpp
index 752a52c..d34c032 100644
--- a/src/notmuchworker.cpp
+++ b/src/notmuchworker.cpp
@@ -69,11 +69,75 @@ bool collectMessageIds(notmuch_database_t *db, const QString &query,
return true;
}
+/// Walks a thread's reply structure depth-first, appending each message with
+/// its depth.
+///
+/// Takes RAW notmuch_message_t*, deliberately, against the rule that every
+/// handle in this file is RAII-owned. Messages reached through a thread belong
+/// to that thread and are freed with it (notmuch.h:1637), so wrapping one in
+/// NmMessage would call notmuch_message_destroy on memory the thread frees
+/// again. The NmThread in the caller is what keeps every pointer here alive,
+/// and this must not outlive it.
+///
+/// No match-set argument, unlike loadThread. A row is drawn for every message
+/// in the thread regardless of the query: the list is where the user goes to
+/// SEE the thread's shape, and hiding replies that did not match would make the
+/// reply count disagree with the rows beneath it.
+void walkReplies(notmuch_messages_t *messages, int depth,
+ QVector<MessageNode> *out)
+{
+ for (; notmuch_messages_valid(messages);
+ notmuch_messages_move_to_next(messages)) {
+
+ notmuch_message_t *message = notmuch_messages_get(messages);
+ if (!message)
+ continue;
+
+ MessageNode node;
+ node.messageId =
+ QString::fromUtf8(notmuch_message_get_message_id(message));
+ node.threadId =
+ QString::fromUtf8(notmuch_message_get_thread_id(message));
+ node.filePath =
+ QString::fromUtf8(notmuch_message_get_filename(message));
+ node.from =
+ QString::fromUtf8(notmuch_message_get_header(message, "from"));
+ node.subject =
+ QString::fromUtf8(notmuch_message_get_header(message, "subject"));
+ node.date =
+ QDateTime::fromSecsSinceEpoch(notmuch_message_get_date(message));
+ node.tags = tagsOf(message);
+ node.depth = depth;
+ out->append(node);
+
+ // NULL is a legitimate "no replies" here: notmuch_messages_valid
+ // accepts it and returns FALSE (notmuch.h:1630), so a leaf needs no
+ // guard of its own.
+ walkReplies(notmuch_message_get_replies(message), depth + 1, out);
+ }
+}
+
} // namespace
+/// Registers SortOrder for queued calls, once, before main() runs.
+///
+/// Q_ENUM alone is NOT enough for a queued Q_ARG: it gives the enum a
+/// meta-object entry, not a metatype registered under the name invokeMethod
+/// resolves, so MainWindow's queued runQuery would drop its sort argument at
+/// runtime with a warning and every query would silently run newest-first.
+///
+/// Here rather than in MainWindow's constructor, because the registration
+/// belongs to the type rather than to one consumer: a caller that never
+/// constructs a MainWindow (a test, or a future headless mode) needs it too,
+/// and that is exactly how the first attempt at this passed by accident and
+/// failed under test.
+static const int kSortOrderMetaType =
+ qRegisterMetaType<NotmuchWorker::SortOrder>("NotmuchWorker::SortOrder");
+
NotmuchWorker::NotmuchWorker(const QString &notmuchConfigPath, QObject *parent)
: QObject(parent), m_configPath(notmuchConfigPath)
{
+ Q_UNUSED(kSortOrderMetaType);
}
NotmuchWorker::~NotmuchWorker()
@@ -122,7 +186,8 @@ void NotmuchWorker::close()
}
}
-void NotmuchWorker::runQuery(const QString &query, quint64 generation)
+void NotmuchWorker::runQuery(const QString &query, quint64 generation,
+ SortOrder sort)
{
if (!openReadOnly())
return;
@@ -132,7 +197,9 @@ void NotmuchWorker::runQuery(const QString &query, quint64 generation)
emit errorOccurred(QStringLiteral("Invalid query: %1").arg(query));
return;
}
- notmuch_query_set_sort(nmQuery.get(), NOTMUCH_SORT_NEWEST_FIRST);
+ notmuch_query_set_sort(nmQuery.get(),
+ sort == OldestFirst ? NOTMUCH_SORT_OLDEST_FIRST
+ : NOTMUCH_SORT_NEWEST_FIRST);
notmuch_threads_t *rawThreads = nullptr;
const notmuch_status_t status =
@@ -242,6 +309,99 @@ void NotmuchWorker::loadThread(const QString &threadId,
emit threadLoaded(result, generation);
}
+void NotmuchWorker::loadThreadTree(const QString &threadId,
+ const QString &matchQuery,
+ quint64 generation)
+{
+ // Accepted for signature symmetry with loadThread, and unused on purpose:
+ // see walkReplies on why every message in the thread gets a row.
+ Q_UNUSED(matchQuery);
+
+ if (!openReadOnly())
+ return;
+
+ const QString query = QStringLiteral("thread:%1").arg(threadId);
+ NmQuery nmQuery(notmuch_query_create(m_db, query.toUtf8().constData()));
+ if (!nmQuery) {
+ emit errorOccurred(
+ QStringLiteral("Cannot load thread %1").arg(threadId));
+ return;
+ }
+
+ // search_threads, not search_messages. The messages have to come from a
+ // notmuch_thread_t or notmuch_message_get_replies returns NULL for every
+ // one of them and the walk below produces a flat list at depth 0.
+ notmuch_threads_t *rawThreads = nullptr;
+ if (notmuch_query_search_threads(nmQuery.get(), &rawThreads)
+ != NOTMUCH_STATUS_SUCCESS) {
+ emit errorOccurred(
+ QStringLiteral("Cannot search thread %1").arg(threadId));
+ return;
+ }
+ NmThreads threads(rawThreads);
+
+ QVector<MessageNode> nodes;
+ if (notmuch_threads_valid(threads.get())) {
+ // Held for the whole walk: every message pointer inside belongs to this
+ // thread and dies with it.
+ NmThread thread(notmuch_threads_get(threads.get()));
+ if (thread) {
+ walkReplies(notmuch_thread_get_toplevel_messages(thread.get()), 0,
+ &nodes);
+ }
+ }
+
+ emit threadTreeLoaded(nodes, generation);
+}
+
+void NotmuchWorker::loadMessage(const QString &messageId, quint64 generation)
+{
+ if (!openReadOnly())
+ return;
+
+ // id: is an exact-match prefix, and the id is quoted because a message id
+ // can legitimately contain characters notmuch's parser would otherwise read
+ // as query syntax.
+ const QString query = QStringLiteral("id:\"%1\"").arg(messageId);
+ NmQuery nmQuery(notmuch_query_create(m_db, query.toUtf8().constData()));
+ if (!nmQuery) {
+ emit errorOccurred(
+ QStringLiteral("Cannot load message %1").arg(messageId));
+ return;
+ }
+
+ notmuch_messages_t *rawMessages = nullptr;
+ if (notmuch_query_search_messages(nmQuery.get(), &rawMessages)
+ != NOTMUCH_STATUS_SUCCESS) {
+ emit errorOccurred(
+ QStringLiteral("Cannot search message %1").arg(messageId));
+ return;
+ }
+ NmMessages messages(rawMessages);
+
+ QVector<MessageRef> result;
+ if (notmuch_messages_valid(messages.get())) {
+ NmMessage message(notmuch_messages_get(messages.get()));
+ if (message) {
+ MessageRef ref;
+ ref.messageId = QString::fromUtf8(
+ notmuch_message_get_message_id(message.get()));
+ ref.filePath = QString::fromUtf8(
+ notmuch_message_get_filename(message.get()));
+ ref.tags = tagsOf(message.get());
+
+ // Always matched: the user asked for this message by clicking its
+ // row, so rendering it as a stub would answer the wrong question.
+ ref.matched = true;
+ result.append(ref);
+ }
+ }
+
+ // Emitted even when empty, so the UI's handler runs and can decide what to
+ // do rather than waiting for a reply that never comes.
+ emit messageLoaded(result, generation);
+}
+
void NotmuchWorker::applyTagsToThreads(const QStringList &threadIds,
const QStringList &add,
const QStringList &remove,
diff --git a/src/notmuchworker.h b/src/notmuchworker.h
index 7d6a587..1d8c8c0 100644
--- a/src/notmuchworker.h
+++ b/src/notmuchworker.h
@@ -44,10 +44,24 @@ public:
/// Threads emitted per threadsReady() signal.
static constexpr int kBatchSize = 200;
+ /// The sort orders offered to the user.
+ ///
+ /// Two, not four. notmuch also has NOTMUCH_SORT_MESSAGE_ID and
+ /// NOTMUCH_SORT_UNSORTED, and neither is an order a human wants. Sorting
+ /// by sender or subject is deliberately absent: notmuch cannot do it, so
+ /// the model would have to sort after results arrive, which fights the
+ /// batching that makes a 10k-thread query paint immediately.
+ enum SortOrder {
+ NewestFirst,
+ OldestFirst,
+ };
+ Q_ENUM(SortOrder)
+
public slots:
/// Runs a query. generation lets the UI discard results from a superseded
/// query without the worker needing to know about cancellation.
- void runQuery(const QString &query, quint64 generation);
+ void runQuery(const QString &query, quint64 generation,
+ SortOrder sort = NewestFirst);
/// Loads the messages of one thread, oldest first. matchQuery is the
/// user's current query; messages matching it render expanded, the rest
@@ -55,6 +69,28 @@ public slots:
void loadThread(const QString &threadId, const QString &matchQuery,
quint64 generation);
+ /// Loads a thread as a reply TREE, for the message rows in the list.
+ ///
+ /// Separate from loadThread rather than replacing it, for a reason that is
+ /// not stylistic: loadThread walks notmuch_query_search_messages, and a
+ /// message obtained that way returns NULL from
+ /// notmuch_message_get_replies (notmuch.h:1617-1628), so that walk cannot
+ /// produce reply depth at all. The tree has to come from
+ /// notmuch_thread_get_toplevel_messages instead. The message pane still
+ /// wants the flat list; only the list wants the tree.
+ ///
+ /// matchQuery is accepted for signature symmetry with loadThread and is
+ /// deliberately unused: see the comment on the walk in the .cpp.
+ void loadThreadTree(const QString &threadId, const QString &matchQuery,
+ quint64 generation);
+
+ /// Loads ONE message, for a message row selected in the list.
+ ///
+ /// Emits messageLoaded with an empty vector when the id is unknown, which
+ /// is an ordinary race after a reindex rather than an error worth
+ /// reporting.
+ void loadMessage(const QString &messageId, quint64 generation);
+
/// Applies tag changes. Opens the database read-write, applies, and closes
/// immediately: notmuch's write lock is exclusive process-wide, so holding
/// it would block the user's cron `notmuch new`.
@@ -100,6 +136,9 @@ signals:
void threadsReady(const QVector<ThreadSummary> &threads, quint64 generation);
void queryFinished(int totalThreads, quint64 generation);
void threadLoaded(const QVector<MessageRef> &messages, quint64 generation);
+ void threadTreeLoaded(const QVector<MessageNode> &nodes,
+ quint64 generation);
+ void messageLoaded(const QVector<MessageRef> &messages, quint64 generation);
void tagsApplied(const TagChange &change);
void allTagsReady(const QStringList &tags, quint64 generation);
diff --git a/src/tagchip.cpp b/src/tagchip.cpp
index 1f4ad79..d770a56 100644
--- a/src/tagchip.cpp
+++ b/src/tagchip.cpp
@@ -55,42 +55,6 @@ void paint(QPainter *painter, const QRect &rect, const QString &text,
} // namespace TagChip
-int SubjectDelegate::subjectBandHeight(const QStyleOptionViewItem &option)
-{
- return QFontMetrics(option.font).height();
-}
-
-QFont SubjectDelegate::pillFont(const QFont &rowFont)
-{
- QFont font = rowFont;
-
- // Two points down, floored. One point was measured to change nothing at a
- // 12pt desktop font: 12 and 11 both render 17px tall, so the pills came
- // out the same size as the subject and read as competing content rather
- // than as annotation.
- //
- // pointSize() is -1 when the font was specified in pixels, which
- // subtracting from would be nonsense, hence the two branches.
- if (rowFont.pointSize() > 0)
- font.setPointSize(qMax(6, rowFont.pointSize() - 2));
- else if (rowFont.pixelSize() > 0)
- font.setPixelSize(qMax(8, rowFont.pixelSize() - 3));
-
- return font;
-}
-
-int SubjectDelegate::rowHeightFor(const QFont &rowFont)
-{
- // The text band uses the ROW's font and the strip its own smaller one.
- // Measuring both with one font is what put the pills over the date text.
- const QFontMetrics rowMetrics(rowFont);
- const QFontMetrics pillMetrics(pillFont(rowFont));
-
- return rowMetrics.height()
- + TagChip::sizeFor(pillMetrics, QStringLiteral("x")).height()
- + kRowPadding * 2 + TagChip::kSpacing;
-}
-
void RowStyleDelegate::initStyleOption(QStyleOptionViewItem *option,
const QModelIndex &index) const
{
@@ -115,143 +79,9 @@ void RowStyleDelegate::initStyleOption(QStyleOptionViewItem *option,
option->palette.setColor(QPalette::WindowText, highlighted);
}
- // Top-aligned and on one line, matching the subject beside them.
- //
- // The row is tall enough for a pill strip under the text, and Qt centres a
- // cell's text in the whole rectangle by default: date and sender floated
- // into the middle while the subject sat at the top, so the three did not
- // share a baseline. Confining the rectangle to the text band puts them all
- // on one.
- //
- // Wrapping matters more than it looks. A long sender ran to a second line,
- // which reached down into the strip's band and collided with the pills; a
- // cell cannot know they are there, since the view paints them afterwards.
- // Eliding keeps every row's text inside its own band whatever it holds.
- // Top of the row rather than centre of it, so the alignment is expressed
- // without shrinking the rectangle: the rect is also what the background
- // and selection fill are drawn into, and clipping it to the text band
- // would leave the highlight covering only the upper part of the row.
+ // One line, elided. A card draws its own text through CardDelegate, but
+ // this still governs whatever Qt draws for the item itself, and a wrapped
+ // string would run past the card's own three lines.
option->features &= ~QStyleOptionViewItem::WrapText;
option->textElideMode = Qt::ElideRight;
-
- // The marker columns keep their centring. Their glyphs are the row's
- // symbols rather than its text, so aligning them with the subject's
- // baseline would strand them at the top of a tall row with the pill strip
- // empty beneath; centred, they read as marking the whole row.
- const bool marker = index.column() == ThreadListModel::AttachmentColumn
- || index.column() == ThreadListModel::FlagColumn;
- option->displayAlignment = marker
- ? Qt::AlignCenter
- : (Qt::AlignLeft | Qt::AlignTop);
-}
-
-void SubjectDelegate::paint(QPainter *painter, const QStyleOptionViewItem &option,
- const QModelIndex &index) const
-{
- // AccountLabelRole is a property of the ROW, not of a cell, so this
- // delegate must only ever be installed on the subject column. Installed
- // view-wide it draws the account chip into every column, which is exactly
- // what happened when that was tried.
- Q_ASSERT(index.column() == ThreadListModel::SubjectColumn);
-
- const QString account =
- index.data(ThreadListModel::AccountLabelRole).toString();
- if (account.isEmpty()) {
- // No chip to draw, so the base class renders the text, confined to the
- // upper band: the lower one belongs to the row-wide pill strip that
- // ThreadListView paints after every cell.
- QStyleOptionViewItem chrome = option;
- initStyleOption(&chrome, index);
- chrome.rect.setHeight(subjectBandHeight(option));
- QStyledItemDelegate::paint(painter, chrome, index);
-
- return;
- }
-
- // Draw the row's own background and selection first, then the chip and the
- // subject on top, so a selected or struck-through row still looks right.
- QStyleOptionViewItem chrome = option;
- initStyleOption(&chrome, index);
- chrome.text.clear();
- const QWidget *widget = option.widget;
- QStyle *style = widget ? widget->style() : QApplication::style();
- style->drawControl(QStyle::CE_ItemViewItem, &chrome, painter, widget);
-
- const QFontMetrics metrics(option.font);
- const QSize chipSize = TagChip::sizeFor(metrics, account);
-
- // The subject and its chip occupy the upper band; ThreadListView paints
- // the pill strip across the lower one. Centring the chip in the whole row
- // would leave it floating beside that gap rather than beside its text.
- const int textBandHeight = subjectBandHeight(option);
- const int textTop = option.rect.top() + kRowPadding;
-
- const QRect chipRect(option.rect.left() + TagChip::kSpacing,
- textTop + (textBandHeight - chipSize.height()) / 2,
- chipSize.width(), chipSize.height());
-
- const QColor colour =
- index.data(ThreadListModel::AccountColourRole).value<QColor>();
- TagChip::paint(painter, chipRect, account,
- colour.isValid() ? colour : QColor(0x55, 0x55, 0x5f));
-
- // The subject follows the chip, elided so a long one cannot overflow.
- QRect textRect = option.rect;
- textRect.setLeft(chipRect.right() + TagChip::kSpacing * 2);
- textRect.setTop(textTop);
- textRect.setHeight(textBandHeight);
- if (textRect.width() <= 0)
- return;
-
- painter->save();
- // Selection outranks the model's colour, and that order matters. A read
- // thread carries a dimmed foreground blended against the UNSELECTED
- // background, so painting it over the highlight leaves grey-on-purple,
- // which is close to unreadable. The highlight already carries the "this
- // row" signal, so the read/unread distinction can yield to it for as long
- // as the row is selected.
- //
- // A doomed thread is the exception that proves the rule: its white is not
- // a dimming but a contrast requirement against its own fill, and the fill
- // is drawn under the selection too.
- const QVariant foreground = index.data(Qt::ForegroundRole);
- if (option.state & QStyle::State_Selected)
- painter->setPen(option.palette.highlightedText().color());
- else if (foreground.isValid())
- painter->setPen(foreground.value<QBrush>().color());
- else
- painter->setPen(option.palette.text().color());
-
- // The model's font carries bold for unread and strike-out for deleted.
- // initStyleOption() already resolved it into chrome.font; using it rather
- // than option.font is what keeps those cues on a delegate-drawn subject.
- const QVariant fontData = index.data(Qt::FontRole);
- const QFont rowFont = fontData.isValid() ? fontData.value<QFont>()
- : chrome.font;
- painter->setFont(rowFont);
- const QFontMetrics rowMetrics(rowFont);
- painter->drawText(textRect, Qt::AlignVCenter | Qt::AlignLeft,
- rowMetrics.elidedText(index.data(Qt::DisplayRole).toString(),
- Qt::ElideRight, textRect.width()));
- painter->restore();
-}
-
-QSize SubjectDelegate::sizeHint(const QStyleOptionViewItem &option,
- const QModelIndex &index) const
-{
- QSize size = QStyledItemDelegate::sizeHint(option, index);
- const QString account =
- index.data(ThreadListModel::AccountLabelRole).toString();
- if (!account.isEmpty()) {
- const QFontMetrics metrics(option.font);
- size.setWidth(size.width() + TagChip::sizeFor(metrics, account).width()
- + TagChip::kSpacing * 3);
- }
-
- // Height comes from rowHeightFor(), applied by the view to every row at
- // once. A QTableView takes ONE height per row, so a hint returned here
- // would only win if the view happened to ask this column, and this
- // delegate is on the subject column alone.
- size.setHeight(rowHeightFor(option.font));
- return size;
}
diff --git a/src/tagchip.h b/src/tagchip.h
index cc3b5de..5514cae 100644
--- a/src/tagchip.h
+++ b/src/tagchip.h
@@ -59,8 +59,8 @@ void paint(QPainter *painter, const QRect &rect, const QString &text,
/// the selection highlight it lands as grey on the highlight colour, close to
/// unreadable.
///
-/// Applied to the columns that have no delegate of their own; SubjectDelegate
-/// inherits it for the subject column.
+/// Inherited by CardDelegate, which is the only delegate the thread list
+/// installs.
class RowStyleDelegate : public QStyledItemDelegate
{
Q_OBJECT
@@ -72,41 +72,3 @@ protected:
const QModelIndex &index) const override;
};
-/// Item delegate for the subject column: draws the account chip in front of
-/// the subject text, so which mailbox a thread came from reads at a glance
-/// without a tags column spelling it out.
-/// **Install on the subject column only.** It reads AccountLabelRole, which is
-/// a property of the row rather than of a cell, so as a view-wide delegate it
-/// draws the account chip into every column.
-class SubjectDelegate : public RowStyleDelegate
-{
- Q_OBJECT
-public:
- using RowStyleDelegate::RowStyleDelegate;
-
- void paint(QPainter *painter, const QStyleOptionViewItem &option,
- const QModelIndex &index) const override;
- QSize sizeHint(const QStyleOptionViewItem &option,
- const QModelIndex &index) const override;
-
- /// Vertical breathing room above the subject and below the pill row.
- static constexpr int kRowPadding = 4;
-
- /// The font the pill strip is drawn in: a size down from the row's own.
- ///
- /// At the same size the pills read as a second row of content competing
- /// with the subject, rather than as annotation beneath it. Derived from
- /// the row font rather than fixed, so it follows the desktop's font size.
- static QFont pillFont(const QFont &rowFont);
-
- /// The height every row gets, tall enough for the subject and a pill strip
- /// beneath it. The view applies this itself: a QTableView takes one height
- /// for the whole row, so leaving it to a single column's sizeHint would
- /// let whichever column the view happens to ask decide.
- static int rowHeightFor(const QFont &rowFont);
-
-protected:
- /// The height of the band the subject text occupies. Everything below it
- /// belongs to ThreadListView's row-wide pill strip.
- static int subjectBandHeight(const QStyleOptionViewItem &option);
-};
diff --git a/src/threadlistmodel.cpp b/src/threadlistmodel.cpp
index 4bafca8..bdc7e96 100644
--- a/src/threadlistmodel.cpp
+++ b/src/threadlistmodel.cpp
@@ -64,7 +64,7 @@ QString ThreadListModel::flagGlyph()
// U+2605 BLACK STAR, with the same fallback reasoning as the paperclip: an
// unrenderable codepoint shows as tofu, which reads as breakage rather
// than as "flagged". The solid star, not the outlined U+2606, since it has
- // to register at column width beside a paperclip.
+ // to register at small size beside a paperclip.
static const QString glyph = [] {
const char32_t star = 0x2605;
const QString preferred = QString::fromUcs4(&star, 1);
@@ -75,6 +75,46 @@ QString ThreadListModel::flagGlyph()
return glyph;
}
+QColor ThreadListModel::replyBackground()
+{
+ // Mixed from the palette rather than fixed, for the same reason as
+ // readColour: a tint that reads as "grouped" on a light theme is either
+ // invisible or muddy on a dark one.
+ //
+ // Toward Text rather than toward a hue, so it darkens on a light theme and
+ // lightens on a dark one without picking a colour that means something
+ // else. 0.07 is deliberately near the threshold of noticing: it is a
+ // grouping cue sitting beside the deleted and spam fills, which carry
+ // actual meaning and must stay the loudest thing in the list.
+ const QPalette palette = QGuiApplication::palette();
+ const QColor base = palette.color(QPalette::Base);
+ const QColor text = palette.color(QPalette::Text);
+
+ constexpr qreal kWeight = 0.07;
+ const qreal inverse = 1.0 - kWeight;
+ return QColor::fromRgbF(
+ text.redF() * kWeight + base.redF() * inverse,
+ text.greenF() * kWeight + base.greenF() * inverse,
+ text.blueF() * kWeight + base.blueF() * inverse);
+}
+
+QColor ThreadListModel::threadLineColour()
+{
+ // Stronger than the tint, weaker than the text: the line is structure, so
+ // it has to be followable down a long expansion without competing with the
+ // senders beside it.
+ const QPalette palette = QGuiApplication::palette();
+ const QColor base = palette.color(QPalette::Base);
+ const QColor text = palette.color(QPalette::Text);
+
+ constexpr qreal kWeight = 0.35;
+ const qreal inverse = 1.0 - kWeight;
+ return QColor::fromRgbF(
+ text.redF() * kWeight + base.redF() * inverse,
+ text.greenF() * kWeight + base.greenF() * inverse,
+ text.blueF() * kWeight + base.blueF() * inverse);
+}
+
QColor ThreadListModel::readColour()
{
// Derived from the palette, never hardcoded: a fixed grey that reads as
@@ -99,37 +139,255 @@ QColor ThreadListModel::readColour()
}
ThreadListModel::ThreadListModel(QObject *parent)
- : QAbstractTableModel(parent)
+ : QAbstractItemModel(parent)
+{
+}
+
+QModelIndex ThreadListModel::index(int row, int column,
+ const QModelIndex &parent) const
{
+ if (!hasIndex(row, column, parent))
+ return {};
+
+ // A root row. -1 as the internal id marks it, so parent() can tell the two
+ // kinds apart without storing a node pointer per index.
+ if (!parent.isValid())
+ return createIndex(row, column, static_cast<quintptr>(-1));
+
+ // A child row: the internal id is its parent's row, which is all parent()
+ // needs to rebuild the thread index.
+ return createIndex(row, column, static_cast<quintptr>(parent.row()));
+}
+
+QModelIndex ThreadListModel::parent(const QModelIndex &child) const
+{
+ if (!child.isValid())
+ return {};
+
+ const quintptr id = child.internalId();
+ if (id == static_cast<quintptr>(-1))
+ return {};
+
+ // Column 0, always. Qt requires a parent index in the first column, and
+ // returning the child's own column instead breaks selection and the
+ // expander, silently and only for the other columns.
+ return createIndex(static_cast<int>(id), 0, static_cast<quintptr>(-1));
}
int ThreadListModel::rowCount(const QModelIndex &parent) const
{
- return parent.isValid() ? 0 : m_threads.size();
+ if (!parent.isValid())
+ return m_threads.size();
+
+ // Only a thread row has children, and only in its first column. A tree
+ // takes one set of children per row; offering them under every column makes
+ // the view draw an expander in each one.
+ if (parent.parent().isValid() || parent.column() != 0)
+ return 0;
+
+ if (parent.row() < 0 || parent.row() >= m_threads.size())
+ return 0;
+
+ return m_threads.at(parent.row()).children.size();
+}
+
+bool ThreadListModel::hasChildren(const QModelIndex &parent) const
+{
+ if (!parent.isValid())
+ return !m_threads.isEmpty();
+
+ // A message row is always a leaf. Reply depth is drawn from the node's own
+ // depth, not from further nesting, so nothing hangs under a reply.
+ if (parent.parent().isValid())
+ return false;
+
+ if (parent.column() != 0)
+ return false;
+
+ if (parent.row() < 0 || parent.row() >= m_threads.size())
+ return false;
+
+ const ThreadNode &node = m_threads.at(parent.row());
+
+ // Once loaded the children are the truth, including "there are none", which
+ // is how a thread whose totalCount counted duplicates stops offering an
+ // expander that opens onto nothing.
+ if (node.loaded)
+ return !node.children.isEmpty();
+
+ // Before loading, the summary's count is all there is. A thread of one
+ // message has no replies and must not offer an expander.
+ return node.summary.totalCount > 1;
}
int ThreadListModel::columnCount(const QModelIndex &parent) const
{
- return parent.isValid() ? 0 : ColumnCount;
+ // One column: the card is drawn whole by CardDelegate. The five-column
+ // grid is what item 53 removed.
+ //
+ // Answered for a valid parent too. Returning 0 there, as the table version
+ // did, would give message rows no columns at all and render them blank.
+ Q_UNUSED(parent);
+ return 1;
}
QVariant ThreadListModel::data(const QModelIndex &index, int role) const
{
// A stale index from a view that has not caught up with a clear() can carry
// any row or column, so both bounds are checked rather than trusted.
- if (!index.isValid() || index.row() < 0 || index.row() >= m_threads.size()
- || index.column() < 0 || index.column() >= ColumnCount) {
+ if (!index.isValid() || index.row() < 0
+ || index.column() != 0) {
return {};
}
- const ThreadSummary &thread = m_threads.at(index.row());
+ // A message row. Handled before the bounds check below, since a child row's
+ // number indexes its siblings, not m_threads.
+ if (isMessageRow(index)) {
+ const MessageNode node = messageAt(index);
+ if (node.messageId.isEmpty())
+ return {};
+
+ switch (role) {
+ case IsMessageRole:
+ return true;
+ case MessageIdRole:
+ return node.messageId;
+ case MessageDepthRole:
+ return node.depth;
+ case HasRepliesRole:
+ // A reply never has its own expander: nesting past the first level
+ // is drawn from depth, not from further parent-child structure.
+ return false;
+ case ThreadIdRole:
+ // A message row still belongs to a thread, and a caller that only
+ // needs the containing thread must not have to walk up itself.
+ return node.threadId;
+ case TagsRole:
+ case PillTagsRole:
+ // No strip under a child row: the strip is a ROW-wide band carrying
+ // the thread's tags, and one under every reply would stripe the
+ // list and repeat the same tags down the whole expansion.
+ return QStringList();
+ case PillColoursRole:
+ return QVariantList();
+ case MessageOwnTagsRole: {
+ // Set difference against the parent THREAD, not against a global
+ // list: "own" means "not already said by the card above this one".
+ // The parent row indexes m_threads directly, which is the same
+ // mapping messageAt() uses to reach this node.
+ const int threadRow = index.parent().row();
+ const QStringList threadTags =
+ (threadRow >= 0 && threadRow < m_threads.size())
+ ? m_threads.at(threadRow).summary.tags
+ : QStringList();
+ QStringList own;
+ for (const QString &tag : node.tags) {
+ if (!threadTags.contains(tag))
+ own.append(tag);
+ }
+ // Sorted, so a reply does not reshuffle its own chips between
+ // repaints, matching what PillTagsRole already guarantees.
+ own.sort();
+ return own;
+ }
+ case MessageOwnColoursRole: {
+ const QStringList own =
+ data(index, MessageOwnTagsRole).toStringList();
+ QVariantList colours;
+ colours.reserve(own.size());
+ for (const QString &tag : own) {
+ colours.append(m_tagColors ? m_tagColors->colourFor(tag)
+ : TagColors().colourFor(tag));
+ }
+ return colours;
+ }
+ case AccountLabelRole:
+ return QString();
+ case Qt::DisplayRole:
+ case SubjectRole:
+ return node.subject;
+ case SendersRole:
+ // The REPLY's sender, not the thread's author summary. Reading the
+ // thread's fields here would look almost right, since the first
+ // sender usually appears in both.
+ return node.from;
+ case DateRole:
+ return node.date;
+ case HasAttachmentRole:
+ return node.hasAttachment();
+ case IsFlaggedRole:
+ return node.isFlagged();
+ case ReplyCountRole:
+ // A reply never offers an expander: nesting past the first level is
+ // drawn from depth, not from further parent-child structure.
+ return 0;
+ case Qt::BackgroundRole:
+ // Tinted, so an expanded thread reads as one block rather than as
+ // more table rows. Applied per cell here; ThreadListView fills the
+ // same colour across the strip's band so the row does not end up
+ // half tinted.
+ return replyBackground();
+ case Qt::FontRole: {
+ // A size down from the thread rows, so a thread reads as the
+ // heading and its replies as the contents. Never bold: an unread
+ // reply is still subordinate to the thread it belongs to, and the
+ // thread row above already carries the unread cue for the whole
+ // conversation.
+ QFont font = QGuiApplication::font();
+ if (font.pointSize() > 0)
+ font.setPointSize(qMax(6, font.pointSize() - 1));
+ else if (font.pixelSize() > 0)
+ font.setPixelSize(qMax(8, font.pixelSize() - 2));
+ return font;
+ }
+ case Qt::ForegroundRole:
+ // Dimmed whether read or not, for the same reason as the font: a
+ // reply is subordinate content. An unread one is left undimmed so
+ // it can still be found.
+ return node.isUnread() ? QVariant() : QVariant(readColour());
+ default:
+ return {};
+ }
+ }
+
+ if (index.row() >= m_threads.size())
+ return {};
+
+ const ThreadSummary &thread = m_threads.at(index.row()).summary;
if (role == ThreadIdRole)
return thread.threadId;
+ // Answered rather than left to fall through as an invalid QVariant. An
+ // invalid one converts to false and an empty string anyway, so the
+ // behaviour is the same, but a role the model never mentions is a latent
+ // bug the next reader has to prove is harmless.
+ if (role == IsMessageRole)
+ return false;
+
+ if (role == MessageIdRole) {
+ // The thread's FIRST message, once known, because the root card is
+ // that message: selecting it renders one message rather than the whole
+ // conversation. Empty before the replies are loaded, which is the
+ // caller's signal to load the thread instead of guessing at a message.
+ return m_threads.at(index.row()).first.messageId;
+ }
+
+ if (role == MessageDepthRole)
+ return 0;
+
+ if (role == HasRepliesRole)
+ return hasChildren(index.siblingAtColumn(0));
+
if (role == TagsRole)
return thread.tags;
+ if (role == MessageOwnTagsRole)
+ return QStringList();
+
+ if (role == MessageOwnColoursRole)
+ return QVariantList();
+
if (role == PillTagsRole || role == PillColoursRole) {
// Everything the row already says another way is dropped: the account
// is the chip in the subject cell, flagged is the star column,
@@ -186,46 +444,42 @@ QVariant ThreadListModel::data(const QModelIndex &index, int role) const
return {};
}
- if (role == Qt::ToolTipRole && index.column() == AttachmentColumn)
- return thread.hasAttachment() ? tr("Has an attachment") : QVariant();
-
- // "Important", matching the action's own wording (item 57). The underlying
- // tag is still `flagged` and isFlagged() still tests for it; only what the
- // user reads changed.
- if (role == Qt::ToolTipRole && index.column() == FlagColumn)
- return thread.isFlagged() ? tr("Important") : QVariant();
-
- // Both marker columns: a glyph reads as a marker only when it sits in the
- // middle of its column rather than against the text beside it.
- if (role == Qt::TextAlignmentRole
- && (index.column() == AttachmentColumn || index.column() == FlagColumn)) {
- return QVariant::fromValue(Qt::AlignCenter);
+ if (role == Qt::ToolTipRole) {
+ // One tooltip for the whole card, since the marks no longer have
+ // columns of their own to be hovered separately. "Important" matches
+ // the action's own wording (item 57); the underlying tag is still
+ // `flagged` and isFlagged() still tests for it.
+ QStringList marks;
+ if (thread.isFlagged())
+ marks.append(tr("Important"));
+ if (thread.hasAttachment())
+ marks.append(tr("Has an attachment"));
+ return marks.isEmpty() ? QVariant() : marks.join(QStringLiteral(", "));
}
- if (role == Qt::DisplayRole) {
- switch (index.column()) {
- case AttachmentColumn:
- // A glyph rather than an icon resource: no new asset to ship, and
- // it inherits the row's font, so it strikes through with a doomed
- // thread like every other cell.
- return thread.hasAttachment() ? attachmentGlyph() : QString();
- case FlagColumn:
- // A glyph rather than an icon, for the same reasons as the
- // paperclip: no asset to ship, and it inherits the row's font so
- // it strikes through with a doomed thread.
- return thread.isFlagged() ? flagGlyph() : QString();
- case DateColumn:
- return thread.date.toString(QStringLiteral("yyyy-MM-dd hh:mm"));
- case AuthorsColumn:
- return thread.authors;
- case SubjectColumn:
- return thread.totalCount > 1
- ? QStringLiteral("%1 (%2)").arg(thread.subject)
- .arg(thread.totalCount)
- : thread.subject;
- default:
- return {};
- }
+ switch (role) {
+ case Qt::DisplayRole:
+ case SubjectRole:
+ // Bare, with no "(3)" message-count suffix. The count is drawn on the
+ // card's second line as the expander, so a suffix here would state it
+ // twice on the same card.
+ return thread.subject;
+ case SendersRole:
+ return thread.authors;
+ case DateRole:
+ // The QDateTime itself. Formatting belongs to the delegate now: the
+ // card decides how much of a date it has room for, and a pre-formatted
+ // string takes that decision away from it.
+ return thread.date;
+ case HasAttachmentRole:
+ return thread.hasAttachment();
+ case IsFlaggedRole:
+ return thread.isFlagged();
+ case ReplyCountRole:
+ // totalCount includes the root message, which is the card itself.
+ return qMax(0, thread.totalCount - 1);
+ default:
+ break;
}
// A thread tagged deleted or spam is on its way out, and the user needs to
@@ -283,24 +537,6 @@ QVariant ThreadListModel::data(const QModelIndex &index, int role) const
return {};
}
-QVariant ThreadListModel::headerData(int section, Qt::Orientation orientation,
- int role) const
-{
- if (orientation != Qt::Horizontal || role != Qt::DisplayRole)
- return {};
-
- switch (section) {
- // No label: any text would set a minimum width far wider than the icon,
- // which defeats the point of a narrow column.
- case AttachmentColumn: return QString();
- case FlagColumn: return QString();
- case DateColumn: return tr("Date");
- case AuthorsColumn: return tr("From");
- case SubjectColumn: return tr("Subject");
- default: return {};
- }
-}
-
void ThreadListModel::appendBatch(const QVector<ThreadSummary> &batch)
{
// beginInsertRows with an empty range violates Qt's contract, so the guard
@@ -310,7 +546,8 @@ void ThreadListModel::appendBatch(const QVector<ThreadSummary> &batch)
const int first = m_threads.size();
beginInsertRows({}, first, first + batch.size() - 1);
- m_threads.append(batch);
+ for (const ThreadSummary &summary : batch)
+ m_threads.append(ThreadNode{ summary, {}, {}, false });
endInsertRows();
}
@@ -321,17 +558,137 @@ void ThreadListModel::clear()
endResetModel();
}
+void ThreadListModel::setThreadMessages(const QString &threadId,
+ const QVector<MessageNode> &nodes)
+{
+ for (int row = 0; row < m_threads.size(); ++row) {
+ if (m_threads.at(row).summary.threadId != threadId)
+ continue;
+
+ const QModelIndex parent = index(row, 0, QModelIndex());
+
+ // Replace, not append. A thread reloaded after a sync would otherwise
+ // list every reply twice.
+ if (!m_threads.at(row).children.isEmpty()) {
+ beginRemoveRows(parent, 0, m_threads.at(row).children.size() - 1);
+ m_threads[row].children.clear();
+ endRemoveRows();
+ }
+
+ // Every message EXCEPT the first, which is the root card itself.
+ //
+ // Selecting on depth > 0 instead was wrong, and wrong in a way that
+ // only showed on real mail: notmuch_thread_get_toplevel_messages
+ // returns every message at depth 0 when a thread carries no usable
+ // In-Reply-To, so a flat thread contributed no children at all. The
+ // card advertised "3 replies" and expanded onto nothing. Measured in
+ // the user's database: of 396 inbox threads, three are flat, one of
+ // them nine messages long, and every two-message thread of this kind
+ // was affected, which is why the fault looked like "the expander only
+ // works with more than one reply".
+ //
+ // Position also happens to be the right rule rather than a workaround.
+ // The root card IS the thread's first message, so the row under it is
+ // the second message whatever depth notmuch assigns it.
+ QVector<MessageNode> children = nodes.mid(1);
+
+ // Kept so the root card can render its own message. It is the card the
+ // user clicks to read the thread's opening message.
+ m_threads[row].first = nodes.isEmpty() ? MessageNode() : nodes.first();
+
+ if (!children.isEmpty()) {
+ beginInsertRows(parent, 0, children.size() - 1);
+ m_threads[row].children = children;
+ endInsertRows();
+ }
+
+ // Set even when there are no replies: that is the difference between a
+ // single-message thread and one whose replies were never fetched.
+ m_threads[row].loaded = true;
+ return;
+ }
+}
+
+bool ThreadListModel::isMessageRow(const QModelIndex &index) const
+{
+ return index.isValid() && index.parent().isValid();
+}
+
+MessageNode ThreadListModel::messageAt(const QModelIndex &index) const
+{
+ if (!isMessageRow(index))
+ return {};
+
+ const int threadRow = index.parent().row();
+ if (threadRow < 0 || threadRow >= m_threads.size())
+ return {};
+
+ const QVector<MessageNode> &children = m_threads.at(threadRow).children;
+ if (index.row() < 0 || index.row() >= children.size())
+ return {};
+
+ return children.at(index.row());
+}
+
+QString ThreadListModel::threadIdForMessage(const QString &messageId) const
+{
+ for (const ThreadNode &node : m_threads) {
+ for (const MessageNode &child : node.children) {
+ if (child.messageId == messageId)
+ return node.summary.threadId;
+ }
+ }
+ return {};
+}
+
+ActionScope ThreadListModel::scopeFor(const QModelIndexList &selection) const
+{
+ ActionScope scope;
+
+ for (const QModelIndex &index : selection) {
+ if (isMessageRow(index)) {
+ const MessageNode node = messageAt(index);
+ if (node.messageId.isEmpty()
+ || scope.messageIds.contains(node.messageId))
+ continue;
+ scope.messageIds.append(node.messageId);
+ scope.messageCount += 1;
+ continue;
+ }
+
+ if (index.row() < 0 || index.row() >= m_threads.size())
+ continue;
+
+ const ThreadSummary &summary = m_threads.at(index.row()).summary;
+ if (scope.threadIds.contains(summary.threadId))
+ continue;
+
+ scope.threadIds.append(summary.threadId);
+
+ // totalCount, not the loaded children: a thread that was never expanded
+ // still has all of its messages, and counting only what happens to be
+ // on screen would understate what the action does. Floored at 1, since
+ // a summary with no count still stands for at least the message that
+ // produced it.
+ scope.messageCount += qMax(1, summary.totalCount);
+ scope.wholeThread = true;
+ }
+
+ return scope;
+}
+
ThreadSummary ThreadListModel::threadAt(int row) const
{
if (row < 0 || row >= m_threads.size())
return {};
- return m_threads.at(row);
+ return m_threads.at(row).summary;
}
QStringList ThreadListModel::accountKeysForThread(const QString &threadId) const
{
QStringList keys;
- for (const ThreadSummary &thread : m_threads) {
+ for (const ThreadNode &node : m_threads) {
+ const ThreadSummary &thread = node.summary;
if (thread.threadId != threadId)
continue;
for (const QString &tag : thread.tags) {
@@ -351,10 +708,10 @@ void ThreadListModel::applyTagChange(const QString &threadId,
const QStringList &removed)
{
for (int row = 0; row < m_threads.size(); ++row) {
- if (m_threads.at(row).threadId != threadId)
+ if (m_threads.at(row).summary.threadId != threadId)
continue;
- QStringList &tags = m_threads[row].tags;
+ QStringList &tags = m_threads[row].summary.tags;
for (const QString &tag : removed)
tags.removeAll(tag);
for (const QString &tag : added) {
@@ -362,9 +719,9 @@ void ThreadListModel::applyTagChange(const QString &threadId,
tags.append(tag);
}
- // The whole row repaints: unread state drives the font of every column,
- // not just the tags one.
- emit dataChanged(index(row, 0), index(row, ColumnCount - 1));
+ // The whole card repaints: unread state drives its font, and the tags
+ // it draws on line 3 have just changed.
+ emit dataChanged(index(row, 0), index(row, 0));
return;
}
}
diff --git a/src/threadlistmodel.h b/src/threadlistmodel.h
index 2eaa88e..f381ffa 100644
--- a/src/threadlistmodel.h
+++ b/src/threadlistmodel.h
@@ -18,39 +18,26 @@
#pragma once
-#include <QAbstractTableModel>
+#include <QAbstractItemModel>
#include <QColor>
#include <QVector>
#include "tagcolors.h"
#include "types.h"
-/// Table model over query results, filled in batches so a large query paints
+/// Tree model over query results, filled in batches so a large query paints
/// its first screenful immediately.
-class ThreadListModel : public QAbstractTableModel
+///
+/// A tree rather than a table since item 20: a thread's replies are child rows
+/// under it. The tree is at most two levels deep in the MODEL (a thread, then
+/// its messages) even though the messages carry a reply depth of their own; the
+/// visual nesting beyond the first level comes from that depth, not from
+/// further parent-child structure. A deeper model would buy nothing and make
+/// every index calculation recursive.
+class ThreadListModel : public QAbstractItemModel
{
Q_OBJECT
public:
- /// No tags column: spelling out a dozen tags per row cost most of the
- /// list's width and was unreadable. Functional tags moved to a chip strip
- /// under the message pane, and the account tag renders as a chip in front
- /// of the subject.
- enum Column {
- /// A paperclip when the thread has an attachment, so it is visible
- /// without opening the thread. Icon only and deliberately narrow;
- /// it carries no text.
- AttachmentColumn = 0,
-
- /// A star when the thread carries the flagged tag. Beside the
- /// paperclip and the same shape: icon only, narrow, no text.
- FlagColumn,
-
- DateColumn,
- AuthorsColumn,
- SubjectColumn,
- ColumnCount,
- };
-
enum Role {
/// The thread id behind a row. Views hand out QModelIndexes, but the
/// worker speaks thread ids, so the mapping belongs on the model
@@ -77,23 +64,86 @@ public:
/// model because it owns the TagColors instance; a delegate reading
/// config itself would be a second source of truth.
PillColoursRole,
+
+ /// True when the row is a MESSAGE row rather than a thread root.
+ /// Drives both the action scope and whether the view paints a tag
+ /// strip under the row.
+ IsMessageRole,
+
+ /// The message id behind a message row. Empty on a thread root.
+ MessageIdRole,
+
+ /// The message's reply depth, for the view's indentation. 1 for a
+ /// direct reply, since depth 0 is the root row itself.
+ MessageDepthRole,
+
+ /// True when the row is a thread that has replies to show.
+ ///
+ /// Read by SubjectDelegate, which draws the expander itself: the
+ /// delegate cannot call hasChildren without the model, and the same
+ /// answer has to reach the cell that reserves room for the glyph.
+ HasRepliesRole,
+
+ /// The tags this MESSAGE carries that its thread does not.
+ ///
+ /// A reply card shows these and nothing else. Showing a reply's full
+ /// tag set instead was measured against the user's own database and
+ /// rejected: of 48691 messages, 7 carry `unread` and 75 carry
+ /// `flagged`, and both are already drawn another way (the sender's
+ /// weight, and the mark on line 2). Every other tag is applied to a
+ /// whole thread and is identical on all its messages, so full sets
+ /// would repeat the thread's own chips down the entire expansion,
+ /// which is the striping the old row-wide strip existed to avoid.
+ ///
+ /// Empty on a thread row, which has no thread to differ from.
+ MessageOwnTagsRole,
+
+ /// The colours for MessageOwnTagsRole, in the same order. Supplied by
+ /// the model for the same reason as PillColoursRole: it owns the
+ /// TagColors instance, and a delegate reading config itself would be a
+ /// second source of truth.
+ MessageOwnColoursRole,
+
+ /// The card's own fields, by role rather than by column.
+ ///
+ /// Five columns used to answer these through Qt::DisplayRole. One
+ /// column cannot, and a card needs all five values at once, so each
+ /// gets a role and Qt::DisplayRole answers the subject alone (which is
+ /// what keyboard search and accessibility read).
+ SubjectRole,
+ SendersRole,
+ DateRole, ///< A QDateTime. The delegate formats it.
+ HasAttachmentRole, ///< bool
+ IsFlaggedRole, ///< bool
+ ReplyCountRole, ///< int; 0 when a thread has no replies.
};
- /// Row fill for a thread tagged `deleted`, and for one tagged `spam`.
- /// Muted rather than saturated: a bulk delete paints every selected row,
- /// and a wall of pure red is harder to read than the list it replaces.
- /// Exposed so a test names the same colour the model uses.
- /// The character shown in AttachmentColumn for a thread that has one.
- /// A paperclip when the system font can draw it, "*" otherwise.
+ /// The mark drawn on a card's second line when the message has an
+ /// attachment. A paperclip when the system font can draw it, "*" otherwise.
static QString attachmentGlyph();
- /// The character shown in FlagColumn for a flagged thread.
+ /// The mark drawn on a card's second line when the message is flagged.
/// A star when the system font can draw it, "*" otherwise.
static QString flagGlyph();
+ /// Row fill for a thread tagged `deleted`, and for one tagged `spam`.
+ /// Muted rather than saturated: a bulk delete paints every selected row,
+ /// and a wall of pure red is harder to read than the list it replaces.
+ /// Exposed so a test names the same colour the model uses.
static QColor deletedColour();
static QColor spamColour();
+ /// Background for a reply row, so an expanded thread reads as one block
+ /// rather than as more table rows.
+ ///
+ /// Derived from the palette and deliberately subtle: it marks a grouping,
+ /// and a tint strong enough to notice on its own would compete with the
+ /// deleted and spam row colours, which carry real meaning.
+ static QColor replyBackground();
+
+ /// The line drawn down the left of an expanded thread's replies.
+ static QColor threadLineColour();
+
/// The dimmed text colour a READ thread carries.
///
/// Unread rows are left at the palette's own colour and read ones recede,
@@ -109,17 +159,59 @@ public:
/// Without one, chips fall back to a colour generated from the tag name.
void setTagColors(const TagColors *colours) { m_tagColors = colours; }
+ QModelIndex index(int row, int column,
+ const QModelIndex &parent = {}) const override;
+ QModelIndex parent(const QModelIndex &child) const override;
+
int rowCount(const QModelIndex &parent = {}) const override;
int columnCount(const QModelIndex &parent = {}) const override;
+
+ /// Whether a thread row should offer an expander.
+ ///
+ /// Answered from totalCount rather than from the loaded children, and that
+ /// is what makes lazy loading possible at all: rowCount is 0 until the
+ /// worker has walked the thread, so a view left to infer this from rowCount
+ /// alone draws no expander, the user can never expand, and the replies are
+ /// never asked for. The count is already in the summary, so this costs
+ /// nothing.
+ bool hasChildren(const QModelIndex &parent = {}) const override;
QVariant data(const QModelIndex &index, int role) const override;
- QVariant headerData(int section, Qt::Orientation orientation,
- int role) const override;
void appendBatch(const QVector<ThreadSummary> &batch);
void clear();
ThreadSummary threadAt(int row) const;
+ /// Fills in a thread's message rows once the worker has walked its tree.
+ ///
+ /// The depth-0 message is dropped: it is the thread's first message and the
+ /// ROOT row already stands for it. Keeping it would show a thread of seven
+ /// as one root and seven children, contradicting the reply count the row
+ /// advertises. Calling again replaces the rows rather than appending, so a
+ /// thread reloaded after a sync does not list its replies twice.
+ void setThreadMessages(const QString &threadId,
+ const QVector<MessageNode> &nodes);
+
+ /// True when the index is a message row rather than a thread root.
+ bool isMessageRow(const QModelIndex &index) const;
+
+ /// The message row's node, or a default-constructed one for any index that
+ /// is not a message row.
+ MessageNode messageAt(const QModelIndex &index) const;
+
+ /// The thread a loaded message row belongs to, or empty when no expanded
+ /// thread holds it. Only expanded threads have message rows at all, so a
+ /// message the user could select is always findable here.
+ QString threadIdForMessage(const QString &messageId) const;
+
+ /// Resolves a selection into what an action should touch.
+ ///
+ /// Mixed selections are honoured as given: a thread root and an unrelated
+ /// reply act on that whole thread and that one message. Nothing is
+ /// escalated or narrowed silently, which is the point of the scope being
+ /// visible in the first place.
+ ActionScope scopeFor(const QModelIndexList &selection) const;
+
/// The account keys behind a thread's account tags, for item 49's
/// per-account sync.
///
@@ -137,6 +229,33 @@ public:
const QStringList &removed);
private:
- QVector<ThreadSummary> m_threads;
+ /// One thread root and the message rows expanded under it.
+ ///
+ /// Children live beside the summary rather than in a separate map keyed by
+ /// thread id, so a row and its expansion are appended, cleared and
+ /// destroyed together. The model is rebuilt wholesale on every query, so
+ /// nothing here has to survive a reset.
+ struct ThreadNode
+ {
+ ThreadSummary summary;
+ QVector<MessageNode> children; ///< Empty until the thread is expanded.
+
+ /// The thread's FIRST message, which the root card itself draws.
+ ///
+ /// Kept because the root card is that message: selecting it must
+ /// render one message rather than the whole conversation, and without
+ /// this the first message of every thread is unreachable, since the
+ /// only rows offering a message are the replies and it is not one of
+ /// them. Empty until the replies are loaded.
+ MessageNode first;
+
+ /// Distinguishes "this thread has no replies" from "its replies have
+ /// not been asked for yet". Without it an expander would be drawn over
+ /// every thread, including the ones that turn out to be single
+ /// messages.
+ bool loaded = false;
+ };
+
+ QVector<ThreadNode> m_threads;
const TagColors *m_tagColors = nullptr;
};
diff --git a/src/threadlistview.cpp b/src/threadlistview.cpp
index ef80e09..5a3ce77 100644
--- a/src/threadlistview.cpp
+++ b/src/threadlistview.cpp
@@ -18,123 +18,38 @@
#include "threadlistview.h"
-#include "tagchip.h"
+#include "carddelegate.h"
#include "threadlistmodel.h"
-#include <QPaintEvent>
-#include <QPainter>
-#include <QScrollBar>
+#include <QMouseEvent>
-void ThreadListView::paintEvent(QPaintEvent *event)
+void ThreadListView::mousePressEvent(QMouseEvent *event)
{
- QTableView::paintEvent(event);
-
- if (!model())
- return;
-
- QPainter painter(viewport());
-
- // Two fonts, deliberately. The row's own font fixes where the text band
- // ends, and the pills are drawn a size smaller: at the same size they read
- // as a second row of content competing with the subject, rather than as
- // annotation beneath it.
- const QFontMetrics rowMetrics(font());
- const QFont pillFont = SubjectDelegate::pillFont(font());
- const QFontMetrics metrics(pillFont);
- painter.setFont(pillFont);
-
- // Only the rows actually on screen. Walking the whole model would paint
- // thousands of strips outside the viewport on a large query.
- const int first = rowAt(0);
- const int last = rowAt(viewport()->height() - 1);
- const int lastRow = last >= 0 ? last : model()->rowCount() - 1;
-
- for (int row = qMax(0, first); row <= lastRow; ++row) {
- const QModelIndex index =
- model()->index(row, ThreadListModel::SubjectColumn);
-
- const int rowTop = rowViewportPosition(row);
- const int height = rowHeight(row);
- if (height <= 0)
- continue;
-
- // The strip's band, filled to match the row before anything is drawn
- // on it.
- //
- // A QTableView paints alternating colours and the selection PER CELL,
- // so nothing paints the width to the right of the last column, and
- // nothing paints the band at all where a column does not reach. Left
- // unfilled, an alternate-coloured or selected row shows the viewport
- // background in a strip across its lower half. Filled for every
- // visible row, not only tagged ones, since an untagged row has the
- // same band to account for.
- // Starting at the date column, NOT at the viewport edge. The two
- // leading columns hold the attachment and flag glyphs, centred in the
- // full row height, so a band drawn over them cuts those glyphs in half.
- const int bandLeft =
- columnViewportPosition(ThreadListModel::DateColumn);
- const QRect band(bandLeft, rowTop + SubjectDelegate::kRowPadding
- + rowMetrics.height(),
- viewport()->width() - bandLeft,
- height - SubjectDelegate::kRowPadding
- - rowMetrics.height());
-
- // The model's own row colour wins where it has one: a deleted or spam
- // thread fills its cells with crimson or orange, and painting the base
- // colour across the band beneath them would cut the row in half.
- const QVariant background = index.data(Qt::BackgroundRole);
-
- if (background.isValid())
- painter.fillRect(band, background.value<QBrush>());
- else if (selectionModel() && selectionModel()->isRowSelected(row))
- painter.fillRect(band, palette().brush(QPalette::Highlight));
- else if (alternatingRowColors() && (row % 2))
- painter.fillRect(band, palette().brush(QPalette::AlternateBase));
- else
- painter.fillRect(band, palette().brush(QPalette::Base));
-
- const QStringList tags =
- index.data(ThreadListModel::PillTagsRole).toStringList();
- if (tags.isEmpty())
- continue;
-
- const QVariantList colours =
- index.data(ThreadListModel::PillColoursRole).toList();
-
- // The band the cells leave free, below the text they draw in the
- // upper one. Measured from SubjectDelegate by both sides, so neither
- // can drift into the other's half. The row's own font metrics set the
- // text band; the strip's smaller font must not be used for it, or the
- // pills ride up over the date and sender.
- const int top = rowTop + SubjectDelegate::kRowPadding
- + rowMetrics.height() + TagChip::kSpacing;
-
- // Aligned with the first text column rather than the viewport edge:
- // the two leading columns are narrow markers for the attachment and
- // flag glyphs, and a strip starting at x=0 paints straight over them.
- // Indented past the date column's own left edge rather than flush with
- // it: a chip starting exactly where the column does reads as part of
- // the column rather than as a strip laid under the row.
- int x = columnViewportPosition(ThreadListModel::DateColumn)
- + TagChip::kSpacing * 2;
- const int available = viewport()->width() - TagChip::kSpacing;
-
- for (int i = 0; i < tags.size(); ++i) {
- const QSize size = TagChip::sizeFor(metrics, tags.at(i));
-
- // Stop rather than wrap or elide. A row that grew to fit its tags
- // would break the uniform height the list depends on, and half a
- // chip reads as a rendering fault.
- if (x + size.width() > available)
- break;
-
- const QColor colour = i < colours.size()
- ? colours.at(i).value<QColor>()
- : QColor(0x55, 0x55, 0x5f);
-
- TagChip::paint(&painter, QRect(x, top, size.width(), size.height()),
- tags.at(i), colour);
- x += size.width() + TagChip::kSpacing;
+ const QModelIndex index = indexAt(event->pos());
+
+ // The reply count IS the expander. Anything outside its rect selects the
+ // card and opens it, which is what the rest of the card is for.
+ if (event->button() == Qt::LeftButton && index.isValid()
+ && index.data(ThreadListModel::ReplyCountRole).toInt() > 0) {
+
+ QStyleOptionViewItem option;
+ initViewItemOption(&option);
+ option.rect = visualRect(index);
+ // State_Open decides which way the glyph points, and the rect is the
+ // same either way, but pass it so the layout sees the true state.
+ if (isExpanded(index))
+ option.state |= QStyle::State_Open;
+
+ if (CardDelegate::expanderRectFor(option, index)
+ .contains(event->pos())) {
+ setExpanded(index, !isExpanded(index));
+ // Swallowed, so expanding does not also load the thread into the
+ // message pane: it is a request to see the thread's shape, not to
+ // read it.
+ event->accept();
+ return;
}
}
+
+ QTreeView::mousePressEvent(event);
}
diff --git a/src/threadlistview.h b/src/threadlistview.h
index 0b4eafc..3215153 100644
--- a/src/threadlistview.h
+++ b/src/threadlistview.h
@@ -18,30 +18,33 @@
#pragma once
-#include <QTableView>
+#include <QTreeView>
-/// The thread list, with a row-wide strip of tag chips under each row's cells.
+/// The thread list.
///
-/// The strip is painted by the VIEW rather than by a delegate, and that is the
-/// whole reason this class exists. A delegate is handed one cell's rectangle
-/// and cannot paint outside its column, so pills drawn from the subject
-/// column's delegate stop at that column's edge, losing the last tags of a
-/// well-tagged thread, and start at that column's left edge, which puts them
-/// under the subject instead of under the row. Painting after the cells lets
-/// the strip run the full width, which is what the layout asks for:
+/// It exists for ONE reason now: the expander is drawn by CardDelegate, and a
+/// delegate gets no click of its own without an editor, so the view has to own
+/// the hit-test. Everything else it used to do is gone.
///
-/// [ date ][ from ][ subject ...................... ]
-/// [ pill ][ pill ][ pill ]
-///
-/// The cells confine themselves to the upper band so the lower one is free;
-/// SubjectDelegate::kRowPadding and rowHeightFor() are the shared measurements
-/// that keep the two halves agreeing.
-class ThreadListView : public QTableView
+/// Until item 53 this class also painted a row-wide strip of tag chips after
+/// the cells, because a delegate cannot paint outside its column and the strip
+/// spanned all five. With one column and one delegate painting the whole card,
+/// that reason is gone and so is the paintEvent, along with the two faults it
+/// kept producing: a deleted row cut in half, and every other row showing a
+/// bare stripe, both from the view having to re-honour alternating colours,
+/// the selection and BackgroundRole across cells it did not own.
+class ThreadListView : public QTreeView
{
Q_OBJECT
public:
- using QTableView::QTableView;
+ using QTreeView::QTreeView;
protected:
- void paintEvent(QPaintEvent *event) override;
+ /// Toggles a thread when its reply count is clicked.
+ ///
+ /// Being VISIBLE and being CLICKABLE are separate properties:
+ /// setRootIsDecorated(false), needed to stop the style drawing its own
+ /// indicator underneath, also removed the style's hit area, so an expander
+ /// once painted correctly and did nothing at all.
+ void mousePressEvent(QMouseEvent *event) override;
};
diff --git a/src/types.h b/src/types.h
index 999d3f3..d04670e 100644
--- a/src/types.h
+++ b/src/types.h
@@ -63,6 +63,63 @@ struct MessageRef
bool matched = true;
};
+/// One message as a row in the thread list.
+///
+/// Separate from MessageRef, which exists for RENDERING a thread and carries
+/// only what the message pane needs. A row has to be drawn without opening the
+/// message at all, so the display facts live here.
+struct MessageNode
+{
+ QString messageId;
+ QString threadId; ///< The thread this message belongs to.
+ QString from;
+ QString subject;
+ QDateTime date;
+ QStringList tags;
+ QString filePath;
+
+ /// Reply depth within the thread. 0 is the thread's first message, which
+ /// occupies the ROOT row rather than a child row: the user's model is
+ /// "N replies", so a thread of 7 shows 1 root and 6 descendants.
+ int depth = 0;
+
+ bool isUnread() const { return tags.contains(QStringLiteral("unread")); }
+ bool isFlagged() const { return tags.contains(QStringLiteral("flagged")); }
+
+ /// notmuch applies "attachment" while indexing, so this needs no MIME
+ /// parsing, exactly as on ThreadSummary.
+ bool hasAttachment() const
+ {
+ return tags.contains(QStringLiteral("attachment"));
+ }
+};
+
+/// What an action is about to touch, resolved from the selection.
+///
+/// Exists because the thread list holds two kinds of row since item 20, so a
+/// keypress alone no longer says whether it hit one message or seven. Actions
+/// take one of these rather than a bare list of thread ids, and the status bar
+/// reports it: this project's answer to that ambiguity is to make the scope
+/// visible, not to add a confirmation dialog. See CLAUDE.md on why.
+struct ActionScope
+{
+ QStringList threadIds; ///< Whole threads to act on.
+ QStringList messageIds; ///< Individual messages to act on.
+
+ /// Messages the action will touch in total, for the status bar. A whole
+ /// thread contributes all of its messages, a message row contributes one.
+ int messageCount = 0;
+
+ /// True when any whole thread is in scope, which drives the
+ /// "(whole thread)" suffix in the status bar.
+ bool wholeThread = false;
+
+ bool isEmpty() const
+ {
+ return threadIds.isEmpty() && messageIds.isEmpty();
+ }
+};
+
/// One tag mutation, kept so it can be inverted for undo.
struct TagChange
{
@@ -93,5 +150,6 @@ struct DatabaseStats
Q_DECLARE_METATYPE(ThreadSummary)
Q_DECLARE_METATYPE(MessageRef)
+Q_DECLARE_METATYPE(MessageNode)
Q_DECLARE_METATYPE(TagChange)
Q_DECLARE_METATYPE(DatabaseStats)
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index a7bb670..4ecc04d 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -41,6 +41,7 @@ add_qtmaildir_test(interceptor)
add_qtmaildir_test(htmlbuilder)
add_qtmaildir_test(notmuchworker)
add_qtmaildir_test(tagcolors)
+add_qtmaildir_test(cardlayout)
add_qtmaildir_test(threadlistmodel)
add_qtmaildir_test(mailsync)
add_qtmaildir_test(syncmonitor)
diff --git a/tests/test_cardlayout.cpp b/tests/test_cardlayout.cpp
new file mode 100644
index 0000000..fdc18bf
--- /dev/null
+++ b/tests/test_cardlayout.cpp
@@ -0,0 +1,383 @@
+/*
+ * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs
+ * Copyright (C) 2026 Danilo M. <danix@danix.xyz>
+ *
+ * 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 "cardlayout.h"
+
+#include <QFont>
+#include <QLocale>
+#include <QTest>
+
+class TestCardLayout : public QObject
+{
+ Q_OBJECT
+
+private slots:
+ void everyCardIsTheSameHeight();
+ void threeLinesStackWithoutOverlapping();
+ void replyIndentsByDepth();
+ void aDepthZeroReplyStillIndents();
+ void indentStopsAtTheCap();
+ void expanderSitsOnTheSecondLine();
+ void expanderIsEmptyWithoutReplies();
+ void theExpanderReadsAsAPillWithAWord();
+ void dateIsFlushRight();
+ void threadCardCarriesAnAccentBar();
+ void replyCardCarriesNoAccentBar();
+ void theDateFitsWhenTheCardIsBold();
+ void theDateFollowsTheSystemLocale();
+};
+
+namespace {
+
+CardLayout::Input threadInput()
+{
+ CardLayout::Input in;
+ in.isMessage = false;
+ in.depth = 0;
+ in.replyCount = 3;
+ return in;
+}
+
+CardLayout::Input replyInput(int depth)
+{
+ CardLayout::Input in;
+ in.isMessage = true;
+ in.depth = depth;
+ in.replyCount = 0;
+ return in;
+}
+
+} // namespace
+
+void TestCardLayout::everyCardIsTheSameHeight()
+{
+ const QFont font;
+ const int thread = CardLayout::heightFor(font);
+
+ // The uniform height is the whole reason setUniformRowHeights(true)
+ // survives this design, so it is asserted directly rather than inferred
+ // from two cards happening to look alike.
+ //
+ // Note what is NOT varied here: the tag list. CardLayout reserves line 3
+ // unconditionally and never sees the tags, which is exactly the property
+ // being asserted. A version of this test that passed a tag list in would
+ // be testing a parameter that does not exist.
+ const CardLayout threadCard =
+ CardLayout::compute(threadInput(), QRect(0, 0, 400, thread), font);
+ const CardLayout deepReply =
+ CardLayout::compute(replyInput(3), QRect(0, 0, 400, thread), font);
+ CardLayout::Input noRepliesIn = threadInput();
+ noRepliesIn.replyCount = 0;
+ const CardLayout noReplies =
+ CardLayout::compute(noRepliesIn, QRect(0, 0, 400, thread), font);
+
+ QCOMPARE(threadCard.totalHeight, thread);
+ QCOMPARE(deepReply.totalHeight, thread);
+ QCOMPARE(noReplies.totalHeight, thread);
+
+ // The third line exists on every card, including one with nothing to put
+ // there. That blank band is the cost the uniform height was bought with.
+ QCOMPARE(noReplies.tagRect.height(), threadCard.tagRect.height());
+}
+
+void TestCardLayout::threeLinesStackWithoutOverlapping()
+{
+ const QFont font;
+ const int h = CardLayout::heightFor(font);
+ const CardLayout card =
+ CardLayout::compute(threadInput(), QRect(0, 0, 400, h), font);
+
+ QVERIFY(card.senderRect.height() > 0);
+ QVERIFY(card.subjectRect.height() > 0);
+ QVERIFY(card.tagRect.height() > 0);
+
+ // Guard: these must actually be three stacked bands. A layout that
+ // collapsed them all to the same rect would satisfy any assertion that
+ // only checked they exist.
+ QVERIFY(card.senderRect.bottom() <= card.subjectRect.top());
+ QVERIFY(card.subjectRect.bottom() <= card.tagRect.top());
+ QVERIFY(card.tagRect.bottom() <= h);
+}
+
+void TestCardLayout::replyIndentsByDepth()
+{
+ const QFont font;
+ const int h = CardLayout::heightFor(font);
+ const QRect rect(0, 0, 400, h);
+
+ const CardLayout root = CardLayout::compute(threadInput(), rect, font);
+ const CardLayout d1 = CardLayout::compute(replyInput(1), rect, font);
+ const CardLayout d2 = CardLayout::compute(replyInput(2), rect, font);
+
+ QVERIFY(d1.contentLeft > root.contentLeft);
+ QVERIFY(d2.contentLeft > d1.contentLeft);
+
+ // One spine per depth level, so the count is the depth itself.
+ QCOMPARE(root.spines.size(), 0);
+ QCOMPARE(d1.spines.size(), 1);
+ QCOMPARE(d2.spines.size(), 2);
+
+ // Each spine runs the full height of the card, which is what makes an
+ // expansion read as one continuous block rather than as dashes.
+ for (const QRect &spine : d2.spines) {
+ QCOMPARE(spine.top(), rect.top());
+ QCOMPARE(spine.bottom(), rect.bottom());
+ }
+}
+
+void TestCardLayout::aDepthZeroReplyStillIndents()
+{
+ // A reply in a FLAT thread carries depth 0, because notmuch reports every
+ // message of a thread with no usable In-Reply-To as a top-level message.
+ // It is still a reply: it is a child row under the root card, and it has
+ // to read as one.
+ //
+ // Treating depth 0 as "no nesting" left those replies flush against their
+ // thread with no spine, while a nested thread's replies indented normally,
+ // so the list showed two different shapes for the same relationship.
+ const QFont font;
+ const int h = CardLayout::heightFor(font);
+ const QRect rect(0, 0, 400, h);
+
+ CardLayout::Input flatReply;
+ flatReply.isMessage = true;
+ flatReply.depth = 0;
+
+ const CardLayout root = CardLayout::compute(threadInput(), rect, font);
+ const CardLayout reply = CardLayout::compute(flatReply, rect, font);
+
+ QVERIFY2(reply.contentLeft > root.contentLeft,
+ "a depth-0 reply sits flush with its thread, so a flat thread's "
+ "replies look like more threads");
+ QVERIFY2(!reply.spines.isEmpty(),
+ "a depth-0 reply has no spine, so nothing joins it to the thread "
+ "above it");
+
+ // And it lands at the same place a depth-1 reply does: the two are the
+ // same relationship and notmuch's numbering is the only difference.
+ const CardLayout nested = CardLayout::compute(replyInput(1), rect, font);
+ QCOMPARE(reply.contentLeft, nested.contentLeft);
+ QCOMPARE(reply.spines.size(), nested.spines.size());
+}
+
+void TestCardLayout::indentStopsAtTheCap()
+{
+ const QFont font;
+ const int h = CardLayout::heightFor(font);
+ const QRect rect(0, 0, 400, h);
+
+ const CardLayout d4 = CardLayout::compute(replyInput(4), rect, font);
+ const CardLayout d5 = CardLayout::compute(replyInput(5), rect, font);
+ const CardLayout d9 = CardLayout::compute(replyInput(9), rect, font);
+
+ QCOMPARE(d5.contentLeft, d4.contentLeft);
+ QCOMPARE(d9.contentLeft, d4.contentLeft);
+ QCOMPARE(d5.spines.size(), d4.spines.size());
+ QCOMPARE(d9.spines.size(), d4.spines.size());
+
+ // Guard: the cap must not be so low that it has already bitten at depth 3,
+ // which would make the three assertions above true for the wrong reason.
+ const CardLayout d3 = CardLayout::compute(replyInput(3), rect, font);
+ QVERIFY(d3.contentLeft < d4.contentLeft);
+}
+
+void TestCardLayout::expanderSitsOnTheSecondLine()
+{
+ const QFont font;
+ const int h = CardLayout::heightFor(font);
+ const CardLayout card =
+ CardLayout::compute(threadInput(), QRect(0, 0, 400, h), font);
+
+ QVERIFY(!card.expanderRect.isEmpty());
+ // It is the reply count, so it belongs on the line the reply count is on.
+ QVERIFY(card.expanderRect.top() >= card.subjectRect.top());
+ QVERIFY(card.expanderRect.bottom() <= card.subjectRect.bottom());
+ // And it is on the right, where the count is drawn, not in a left gutter.
+ QVERIFY(card.expanderRect.left() > 400 / 2);
+}
+
+void TestCardLayout::expanderIsEmptyWithoutReplies()
+{
+ const QFont font;
+ const int h = CardLayout::heightFor(font);
+ CardLayout::Input in = threadInput();
+ in.replyCount = 0;
+
+ const CardLayout card = CardLayout::compute(in, QRect(0, 0, 400, h), font);
+ QVERIFY(card.expanderRect.isEmpty());
+}
+
+void TestCardLayout::theExpanderReadsAsAPillWithAWord()
+{
+ // A bare "3" beside the subject reads as an unexplained number and gives
+ // no hint that it can be clicked. The label carries the word, and the rect
+ // carries padding for the pill drawn behind it.
+ QCOMPARE(CardLayout::expanderLabel(3, false),
+ QStringLiteral("\u25b8 3 replies"));
+ QCOMPARE(CardLayout::expanderLabel(3, true),
+ QStringLiteral("\u25be 3 replies"));
+
+ // Singular, because "1 replies" is the kind of detail that makes an
+ // interface look unfinished.
+ QCOMPARE(CardLayout::expanderLabel(1, false),
+ QStringLiteral("\u25b8 1 reply"));
+
+ const QFont font;
+ const int h = CardLayout::heightFor(font);
+ const CardLayout card =
+ CardLayout::compute(threadInput(), QRect(0, 0, 400, h), font);
+ const QFontMetrics small(CardLayout::smallFont(font));
+
+ // The rect must hold the label AND its padding, or the pill's background
+ // is narrower than the text sitting on it.
+ QVERIFY2(card.expanderRect.width()
+ >= small.horizontalAdvance(CardLayout::expanderLabel(3, false))
+ + CardLayout::kPillPaddingX * 2,
+ "the expander rect is too narrow for its own label and padding");
+
+ // And it must NOT change width when the card opens: a pill that resized on
+ // click would shift the subject's elision under the pointer.
+ CardLayout::Input open = threadInput();
+ const CardLayout expanded =
+ CardLayout::compute(open, QRect(0, 0, 400, h), font);
+ QCOMPARE(expanded.expanderRect.width(), card.expanderRect.width());
+}
+
+void TestCardLayout::dateIsFlushRight()
+{
+ const QFont font;
+ const int h = CardLayout::heightFor(font);
+ const QRect rect(0, 0, 400, h);
+ const CardLayout card = CardLayout::compute(threadInput(), rect, font);
+
+ // Compared as exclusive edges. QRect::right() is inclusive (left + width -
+ // 1), so asserting card.dateRect.right() == rect.right() - kPaddingX
+ // demands a gap of kPaddingX - 1 pixels and is off by one against the
+ // padding the constant names.
+ QCOMPARE(card.dateRect.right() + 1, rect.right() + 1 - CardLayout::kPaddingX);
+ // The sender must stop before the date starts, or a long sender overwrites
+ // it. This is the assertion that fails if the two are laid out
+ // independently.
+ QVERIFY(card.senderRect.right() <= card.dateRect.left());
+}
+
+void TestCardLayout::threadCardCarriesAnAccentBar()
+{
+ const QFont font;
+ const int h = CardLayout::heightFor(font);
+ const QRect rect(0, 0, 400, h);
+ const CardLayout card = CardLayout::compute(threadInput(), rect, font);
+
+ QCOMPARE(card.accentRect.left(), rect.left());
+ QCOMPARE(card.accentRect.width(), CardLayout::kAccentWidth);
+ // Full height, so a run of cards from one account reads as a continuous
+ // edge rather than as dashes.
+ QCOMPARE(card.accentRect.top(), rect.top());
+ QCOMPARE(card.accentRect.bottom(), rect.bottom());
+
+ // Nothing may be drawn on top of the colour.
+ QVERIFY(card.contentLeft >= card.accentRect.right());
+}
+
+void TestCardLayout::replyCardCarriesNoAccentBar()
+{
+ const QFont font;
+ const int h = CardLayout::heightFor(font);
+ const CardLayout reply =
+ CardLayout::compute(replyInput(1), QRect(0, 0, 400, h), font);
+
+ // A reply's account is its thread's, stated once at the head. The spine
+ // carries the accent instead, so the gutter never holds two lines.
+ QVERIFY(reply.accentRect.isEmpty());
+ QCOMPARE(reply.spines.size(), 1);
+}
+
+void TestCardLayout::theDateFollowsTheSystemLocale()
+{
+ const QDateTime when(QDate(2025, 8, 10), QTime(6, 26));
+
+ // The system locale's own rendering, whatever it is. Asserting a specific
+ // string would only restate the hardcoded pattern this replaced, and would
+ // fail on any machine but the one that wrote it.
+ QCOMPARE(CardLayout::formatDate(when),
+ QLocale::system().toString(when, QLocale::ShortFormat));
+
+ // The specific fault: an ISO-looking pattern on a desktop that does not
+ // use one. Guarded so this test says nothing on a locale that genuinely
+ // formats that way.
+ if (QLocale::system().toString(when, QLocale::ShortFormat)
+ != QStringLiteral("2025-08-10 06:26")) {
+ QVERIFY2(CardLayout::formatDate(when)
+ != QStringLiteral("2025-08-10 06:26"),
+ "the date is hardcoded to yyyy-MM-dd hh:mm rather than "
+ "following the desktop's locale");
+ }
+
+ // And the reserved width has to follow the same formatter, or a locale
+ // whose dates are longer clips them exactly as the bold font did.
+ QFont font;
+ const int h = CardLayout::heightFor(font);
+ const CardLayout card =
+ CardLayout::compute(threadInput(), QRect(0, 0, 400, h), font);
+ QFont bold = font;
+ bold.setBold(true);
+ QVERIFY2(card.dateRect.width()
+ >= QFontMetrics(bold).horizontalAdvance(
+ CardLayout::formatDate(when)),
+ "the reserved date width is narrower than this locale's own "
+ "formatting of a date");
+}
+
+void TestCardLayout::theDateFitsWhenTheCardIsBold()
+{
+ // An UNREAD card draws BOLD, and bold is wider. The layout is computed from
+ // option.font, which is the view's regular font, while the text is painted
+ // with the font initStyleOption resolved from the model's Qt::FontRole. So
+ // a date measured regular and drawn bold overflows its rect: measured at
+ // 154px reserved against 170px needed, which clipped the leading digit of
+ // the year off every unread card.
+ //
+ // The fix is in the layout rather than in the delegate: it reserves the
+ // BOLD width whatever font it is handed, so the two can never disagree.
+ QFont regular;
+ regular.setBold(false);
+ QFont bold = regular;
+ bold.setBold(true);
+
+ const QString sample = QStringLiteral("2025-08-10 06:26");
+ const int boldWidth = QFontMetrics(bold).horizontalAdvance(sample);
+
+ // Guard: bold must actually be wider here, or this asserts nothing.
+ QVERIFY2(boldWidth > QFontMetrics(regular).horizontalAdvance(sample),
+ "bold is not wider than regular in this environment, so this test "
+ "cannot detect the overflow it exists for");
+
+ const int h = CardLayout::heightFor(regular);
+ const CardLayout card =
+ CardLayout::compute(threadInput(), QRect(0, 0, 400, h), regular);
+
+ QVERIFY2(card.dateRect.width() >= boldWidth,
+ qPrintable(QStringLiteral("a layout computed from the REGULAR "
+ "font reserves %1px, and the date needs "
+ "%2px when the card draws bold")
+ .arg(card.dateRect.width())
+ .arg(boldWidth)));
+}
+
+QTEST_MAIN(TestCardLayout)
+#include "test_cardlayout.moc"
diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp
index 5031ead..922705c 100644
--- a/tests/test_mainwindow.cpp
+++ b/tests/test_mainwindow.cpp
@@ -38,6 +38,7 @@
#include <QSplitter>
#include <QTableView>
#include <QToolBar>
+#include <QTreeView>
#include <QTimer>
#include "config.h"
@@ -45,8 +46,16 @@
#include "mainwindow.h"
#include "messageview.h"
#include "notmuchworker.h"
+#include "carddelegate.h"
+#include "cardlayout.h"
+
+#include <QImage>
+#include <QPainter>
+#include <QComboBox>
+#include <QScrollBar>
#include "tagchip.h"
#include "threadlistmodel.h"
+#include "threadlistview.h"
/// MainWindow is mostly wiring, and the parts that need a real database are
/// still verified manually. What is checked here is the action registry: the
@@ -93,7 +102,22 @@ private slots:
void anUnobservableLockTableLeavesTheSyncButtonUsable();
void theStatusBarFollowsTheSyncPhase();
void aSelectedReadThreadIsNotDimmedIntoTheHighlight();
- void thePillRowSpansTheWholeWidthNotOneColumn();
+ void childRowsAreIndentedUnderTheirThread();
+ void aThreadWithRepliesDrawsAVisibleExpander();
+ void cardsNeverScrollSideways();
+ void selectingARootCardKeepsItsThreadForMarkRead();
+ void nextThreadLeavesTheLastReply();
+ void altDownSkipsReplies();
+ void bothThreadStepBindingsReachTheAction();
+ void sortChoiceSurvivesRestart();
+ void accountEntriesCarryTheirColour();
+ void replyRowsKeepTheirTextUnderTheThreadLine();
+ void clickingTheExpanderTogglesTheThread();
+ void selectingAMessageRowTargetsThatMessageNotItsThread();
+ void selectingAThreadRowNamesHowManyMessagesItStandsFor();
+ void selectingAMessageRowReportsNoBulkCount();
+ void anActionOnAThreadRowSaysItHitTheWholeThread();
+ void anActionOnAMessageRowTagsThatMessageNotTheThread();
void markAllReadIsDisabledUntilTheQueryFinishes();
void markAllReadActsOnEveryRowAndUndoesInOneStep();
void markAllReadDoesNothingWhenNothingIsUnread();
@@ -439,11 +463,12 @@ void TestMainWindow::headerStateFromADifferentColumnLayoutIsDiscarded()
window.close();
}
- // Forge a state file from an older layout: same blob, wrong column count.
+ // Forge a state file from the five-column layout. Nothing reads these keys
+ // any more, and that is exactly what must be verified: a blob saved by an
+ // older version has to be ignored rather than applied to a one-column view.
{
QSettings state(MainWindow::uiStatePath(), QSettings::IniFormat);
- state.setValue(QStringLiteral("threadlist/columns"),
- int(ThreadListModel::ColumnCount) - 1);
+ state.setValue(QStringLiteral("threadlist/columns"), 5);
state.setValue(QStringLiteral("threadlist/header"),
QByteArray("not a header this model could have saved"));
}
@@ -452,11 +477,9 @@ void TestMainWindow::headerStateFromADifferentColumnLayoutIsDiscarded()
const Config config;
MainWindow reopened(config);
- auto *view = reopened.findChild<QTableView *>();
+ auto *view = reopened.findChild<QTreeView *>();
QVERIFY(view);
- QCOMPARE(view->columnWidth(ThreadListModel::AttachmentColumn), 28);
- QCOMPARE(view->columnWidth(ThreadListModel::DateColumn), 130);
- QCOMPARE(view->columnWidth(ThreadListModel::SubjectColumn), 520);
+ QCOMPARE(view->model()->columnCount(), 1);
QFile::remove(MainWindow::uiStatePath());
QStandardPaths::setTestModeEnabled(false);
@@ -503,6 +526,27 @@ void TestMainWindow::returnInTheQueryBarRunsTheQueryNotOpenThread()
QVERIFY(!actionFired);
}
+/// Selects a top-level THREAD row, replacing QTableView::selectRow which a
+/// QTreeView does not have.
+///
+/// Not merely a rename: setCurrentIndex alone leaves the selection model empty,
+/// and select() alone leaves current invalid, so every test asserting on either
+/// would break in a different way. Both are set here, exactly as
+/// QTableView::selectRow did.
+static void selectThreadRow(QTreeView *view, int row)
+{
+ const QModelIndex index = view->model()->index(row, 0, QModelIndex());
+ view->selectionModel()->select(
+ index, QItemSelectionModel::ClearAndSelect | QItemSelectionModel::Rows);
+ view->setCurrentIndex(index);
+}
+
+/// The height of a top-level row, replacing QTableView::rowHeight(int).
+static int threadRowHeight(QTreeView *view, int row)
+{
+ return view->visualRect(view->model()->index(row, 0, QModelIndex())).height();
+}
+
/// A thread summary carrying the tags a test needs. Enough to drive selection;
/// nothing here touches a database.
static ThreadSummary makeThread(const QString &id, const QStringList &tags)
@@ -515,81 +559,870 @@ static ThreadSummary makeThread(const QString &id, const QStringList &tags)
return thread;
}
-void TestMainWindow::thePillRowSpansTheWholeWidthNotOneColumn()
+void TestMainWindow::childRowsAreIndentedUnderTheirThread()
+{
+ const Config config;
+ MainWindow window(config);
+
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<QTreeView *>();
+ QVERIFY2(view, "the thread list is not a QTreeView, so it cannot indent");
+
+ // With an account tag, so the thread row draws the chip that a reply row
+ // does not. That asymmetry is the whole reason the indent has to be wide,
+ // and a test against an untagged thread never sees it.
+ model->appendBatch({ makeThread(
+ QStringLiteral("t1"),
+ QStringList{ TagColors::tagForAccountKey(QStringLiteral("work")) }) });
+
+ MessageNode first;
+ first.messageId = QStringLiteral("m0@example.org");
+ first.threadId = QStringLiteral("t1");
+ first.depth = 0;
+ MessageNode reply;
+ reply.messageId = QStringLiteral("m1@example.org");
+ reply.threadId = QStringLiteral("t1");
+ reply.from = QStringLiteral("A Replier <replier@example.org>");
+ reply.depth = 1;
+ model->setThreadMessages(QStringLiteral("t1"), { first, reply });
+
+ window.resize(1400, 300);
+ window.show();
+ QVERIFY(QTest::qWaitForWindowExposed(&window));
+
+ const QModelIndex root = model->index(0, 0, QModelIndex());
+ view->expand(root);
+ QApplication::processEvents();
+
+ // One column, and setIndentation(0): Qt indents nothing, CardLayout draws
+ // the indent itself. So visualRect reports the SAME rect for a thread and
+ // its reply, and the indent has to be read off the layout rather than off
+ // the geometry. That is the trap CLAUDE.md records in reverse: there,
+ // visualRect reported an indent the text did not have; here it reports
+ // none while the text is indented.
+ const QModelIndex rootCell = model->index(0, 0, QModelIndex());
+ const QModelIndex child = model->index(0, 0, root);
+ QVERIFY(child.isValid());
+
+ // Guards before the claim: a probe that cannot see both rows can report
+ // anything it likes about their relative position.
+ QVERIFY2(view->visualRect(rootCell).height() > 0,
+ "the thread row has no height, so nothing about it is measurable");
+ QVERIFY2(view->visualRect(child).height() > 0,
+ "the reply row has no height: it is collapsed or off-screen, and "
+ "an indent test against it would pass without drawing anything");
+
+ // The indent is NOT in the geometry. setIndentation(0) means visualRect
+ // reports the same left edge for both rows, deliberately: CardLayout draws
+ // the indent inside the card's own rect. Asserting on visualRect here
+ // would fail against a perfectly indented list, which is the mirror of the
+ // trap CLAUDE.md records for item 20, where visualRect reported an indent
+ // the text did not have.
+ //
+ // So the real property, as before: where the TEXT lands. It is read off
+ // the layout, which is what the delegate paints from.
+ CardLayout::Input threadIn;
+ threadIn.isMessage = false;
+ threadIn.depth = 0;
+ CardLayout::Input replyIn;
+ replyIn.isMessage = true;
+ replyIn.depth =
+ model->data(child, ThreadListModel::MessageDepthRole).toInt();
+ QVERIFY2(replyIn.depth > 0,
+ "the reply reports depth 0, so there is no nesting to measure");
+
+ const QRect rect = view->visualRect(rootCell);
+ const CardLayout threadCard =
+ CardLayout::compute(threadIn, rect, view->font());
+ const CardLayout replyCard =
+ CardLayout::compute(replyIn, rect, view->font());
+
+ QVERIFY2(replyCard.contentLeft > threadCard.contentLeft,
+ qPrintable(QStringLiteral("the reply's text starts at x=%1, not "
+ "right of the thread's at x=%2: the "
+ "nesting is invisible")
+ .arg(replyCard.contentLeft)
+ .arg(threadCard.contentLeft)));
+
+ // And the spine that makes the nesting read as one block rather than as an
+ // arbitrary offset.
+ QCOMPARE(replyCard.spines.size(), replyIn.depth);
+}
+
+namespace {
+
+/// Two threads, the first with one reply, expanded. The shared fixture for the
+/// two navigation tests below.
+struct NavFixture
+{
+ QTreeView *view = nullptr;
+ ThreadListModel *model = nullptr;
+ QModelIndex root;
+ QModelIndex reply;
+};
+
+NavFixture buildNavFixture(MainWindow &window)
+{
+ NavFixture f;
+ f.view = window.findChild<QTreeView *>();
+ f.model = window.findChild<ThreadListModel *>();
+
+ // unread, so a selection arms the mark-read timer: scheduleMarkRead()
+ // returns early for a thread that is already read, and a fixture without
+ // it would make a mark-read assertion pass for the wrong reason.
+ ThreadSummary first = makeThread(QStringLiteral("T1"),
+ QStringList{ QStringLiteral("inbox"),
+ QStringLiteral("unread") });
+ first.totalCount = 2;
+ ThreadSummary second = makeThread(QStringLiteral("T2"),
+ QStringList{ QStringLiteral("inbox") });
+ second.totalCount = 1;
+ f.model->appendBatch({ first, second });
+
+ MessageNode rootNode;
+ rootNode.messageId = QStringLiteral("M1");
+ rootNode.threadId = QStringLiteral("T1");
+ rootNode.depth = 0;
+ MessageNode replyNode;
+ replyNode.messageId = QStringLiteral("M2");
+ replyNode.threadId = QStringLiteral("T1");
+ replyNode.depth = 1;
+ f.model->setThreadMessages(QStringLiteral("T1"), { rootNode, replyNode });
+
+ f.root = f.model->index(0, 0);
+ f.view->expand(f.root);
+ f.reply = f.model->index(0, 0, f.root);
+ return f;
+}
+
+} // namespace
+
+void TestMainWindow::selectingARootCardKeepsItsThreadForMarkRead()
+{
+ // A root card is BOTH a message and a thread: it renders the thread's
+ // first message, and it is still the thread that gets marked read and
+ // repainted on a tag change. The message-row path deliberately clears the
+ // current thread id; doing that here too would silently disable mark-read
+ // and the tag-change repaint for every thread root in the list.
+ const Config config;
+ MainWindow window(config);
+ window.show();
+ QVERIFY(QTest::qWaitForWindowExposed(&window));
+
+ const NavFixture f = buildNavFixture(window);
+ f.view->setCurrentIndex(f.root);
+ QApplication::processEvents();
+
+ // Guard: the fixture loads replies, so the root knows its own message and
+ // the branch under test is the one that runs.
+ QVERIFY2(!f.model->data(f.root, ThreadListModel::MessageIdRole)
+ .toString().isEmpty(),
+ "the root card does not know its first message, so this exercises "
+ "the fallback rather than the path it is written for");
+
+ // A mark-read timer armed for the thread is what proves the thread id
+ // survived: scheduleMarkRead() is only reached on the thread-row path.
+ auto *timer = window.findChild<QTimer *>(QStringLiteral("markReadTimer"));
+ QVERIFY(timer);
+ QVERIFY2(timer->isActive(),
+ "no mark-read timer for a selected root card: its thread id was "
+ "cleared along with the switch to rendering one message");
+}
+
+void TestMainWindow::nextThreadLeavesTheLastReply()
+{
+ const Config config;
+ MainWindow window(config);
+ window.show();
+ QVERIFY(QTest::qWaitForWindowExposed(&window));
+
+ const NavFixture f = buildNavFixture(window);
+ QVERIFY(f.reply.isValid());
+ QVERIFY2(f.view->isExpanded(f.root),
+ "the thread is collapsed, so this test would arrow down a flat "
+ "list and pass against the bug it exists to catch");
+
+ f.view->setCurrentIndex(f.reply);
+
+ // The defect (item 60): selectRow(current.row() + 1) asked for row 1 UNDER
+ // T1, which does not exist, so the action did nothing at all.
+ window.findChild<QAction *>(QStringLiteral("next_thread"))->trigger();
+
+ QCOMPARE(f.view->currentIndex().data(ThreadListModel::ThreadIdRole)
+ .toString(),
+ QStringLiteral("T2"));
+}
+
+void TestMainWindow::altDownSkipsReplies()
+{
+ const Config config;
+ MainWindow window(config);
+ window.show();
+ QVERIFY(QTest::qWaitForWindowExposed(&window));
+
+ const NavFixture f = buildNavFixture(window);
+ QVERIFY(f.view->isExpanded(f.root));
+
+ // From the thread ROOT with its replies showing: one step must land on the
+ // next THREAD, not on the first reply. That is what makes the action mean
+ // thread-to-thread while plain Up/Down still steps message-to-message.
+ f.view->setCurrentIndex(f.root);
+ window.findChild<QAction *>(QStringLiteral("next_thread"))->trigger();
+
+ QCOMPARE(f.view->currentIndex().data(ThreadListModel::ThreadIdRole)
+ .toString(),
+ QStringLiteral("T2"));
+ QVERIFY(!f.view->currentIndex().data(ThreadListModel::IsMessageRole)
+ .toBool());
+
+ // And back, which is the mirror case the old arithmetic also failed.
+ window.findChild<QAction *>(QStringLiteral("prev_thread"))->trigger();
+ QCOMPARE(f.view->currentIndex().data(ThreadListModel::ThreadIdRole)
+ .toString(),
+ QStringLiteral("T1"));
+ QVERIFY(!f.view->currentIndex().data(ThreadListModel::IsMessageRole)
+ .toBool());
+}
+
+void TestMainWindow::bothThreadStepBindingsReachTheAction()
+{
+ const Config config;
+ MainWindow window(config);
+
+ // Two bindings per action, which needs setShortcuts rather than
+ // setShortcut: Ctrl+J/K for a neomutt hand, Alt+Up/Down for a mouse one.
+ // Alt because Shift+arrows is QTreeView's built-in extend-selection that
+ // multi-row tagging depends on, and a bare arrow cannot be a window
+ // shortcut without breaking every text field in the window.
+ for (const auto &pair : { std::pair<const char *, const char *>{
+ "next_thread", "Alt+Down" },
+ { "prev_thread", "Alt+Up" } }) {
+ auto *action =
+ window.findChild<QAction *>(QString::fromLatin1(pair.first));
+ QVERIFY2(action, pair.first);
+ const QList<QKeySequence> shortcuts = action->shortcuts();
+ QVERIFY2(shortcuts.size() >= 2,
+ qPrintable(QStringLiteral("%1 carries %2 shortcut(s), so the "
+ "second binding is unreachable")
+ .arg(QString::fromLatin1(pair.first))
+ .arg(shortcuts.size())));
+ QVERIFY2(shortcuts.contains(
+ QKeySequence(QString::fromLatin1(pair.second))),
+ pair.second);
+ }
+}
+
+void TestMainWindow::sortChoiceSurvivesRestart()
+{
+ QStandardPaths::setTestModeEnabled(true);
+ QFile::remove(MainWindow::uiStatePath());
+
+ {
+ const Config config;
+ MainWindow window(config);
+ auto *sort = window.findChild<QComboBox *>(QStringLiteral("sortOrder"));
+ QVERIFY(sort);
+ QCOMPARE(sort->count(), 2);
+ QCOMPARE(sort->currentIndex(), 0); // Newest first by default.
+ sort->setCurrentIndex(1);
+ window.close();
+ }
+
+ const Config config;
+ MainWindow second(config);
+ auto *sort = second.findChild<QComboBox *>(QStringLiteral("sortOrder"));
+ QVERIFY(sort);
+ QCOMPARE(sort->currentIndex(), 1);
+
+ // A stale or hand-edited file can hold anything, which is the lesson item
+ // 58 recorded: an out-of-range value must fall back rather than select a
+ // row that does not exist.
+ {
+ QSettings state(MainWindow::uiStatePath(), QSettings::IniFormat);
+ state.setValue(QStringLiteral("threadlist/sortOrder"), 47);
+ }
+ MainWindow third(config);
+ auto *thirdSort = third.findChild<QComboBox *>(QStringLiteral("sortOrder"));
+ QCOMPARE(thirdSort->currentIndex(), 0);
+
+ QFile::remove(MainWindow::uiStatePath());
+ QStandardPaths::setTestModeEnabled(false);
+}
+
+void TestMainWindow::accountEntriesCarryTheirColour()
+{
+ // Its own config, not the environment's. Reading the real one made this
+ // SKIP wherever no accounts are configured, which is a test that asserts
+ // nothing while reporting success.
+ QTemporaryDir dir;
+ const QString path = dir.filePath(QStringLiteral("qtmaildir.conf"));
+ {
+ QSettings s(path, QSettings::IniFormat);
+ s.beginGroup(QStringLiteral("account.work"));
+ s.setValue(QStringLiteral("maildir"), QStringLiteral("work"));
+ s.setValue(QStringLiteral("color"), QStringLiteral("#3d7fd1"));
+ s.endGroup();
+ s.beginGroup(QStringLiteral("account.personal"));
+ s.setValue(QStringLiteral("maildir"), QStringLiteral("personal"));
+ s.endGroup();
+ }
+
+ Config config;
+ config.load(path);
+ QCOMPARE(config.accounts().size(), 2);
+
+ MainWindow window(config);
+ auto *box = window.findChild<QComboBox *>(QStringLiteral("accountBox"));
+ QVERIFY(box);
+ QCOMPARE(box->count(), 3);
+
+ // "All accounts" is not an account and carries no swatch.
+ QVERIFY(!box->itemData(0, Qt::DecorationRole).isValid());
+
+ // Every real account does, including the one with no color= key:
+ // colourFor() never fails, deriving a stable colour from the tag name, so
+ // adding an account and forgetting to colour it degrades to something
+ // usable rather than to nothing.
+ QSet<QRgb> seen;
+ for (int i = 1; i < box->count(); ++i) {
+ const QVariant swatch = box->itemData(i, Qt::DecorationRole);
+ QVERIFY2(swatch.isValid(),
+ qPrintable(QStringLiteral("account %1 carries no swatch")
+ .arg(box->itemText(i))));
+ const QColor colour = swatch.value<QColor>();
+ QVERIFY(colour.isValid());
+ seen.insert(colour.rgb());
+ }
+
+ // Guard: two accounts sharing one colour would make the swatches useless
+ // as a key to the accent bars, and would let a broken lookup pass.
+ QCOMPARE(seen.size(), 2);
+}
+
+void TestMainWindow::cardsNeverScrollSideways()
+{
+ const Config config;
+ MainWindow window(config);
+ window.show();
+ QVERIFY(QTest::qWaitForWindowExposed(&window));
+
+ auto *view = window.findChild<ThreadListView *>();
+ QVERIFY(view);
+
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ // A long subject, so the guard below is not vacuous: this is exactly the
+ // content that used to make the subject column wider than the viewport.
+ model->appendBatch({ makeThread(
+ QStringLiteral("t1"),
+ QStringList{ QStringLiteral("inbox") }) });
+ QApplication::processEvents();
+
+ // Item 51: clicking a row used to scroll the list sideways, because the
+ // subject column was wider than the viewport and auto-scroll brought the
+ // clicked index fully into view. A card is exactly viewport width, so
+ // there is nowhere to scroll to.
+ QVERIFY2(view->visualRect(model->index(0, 0)).height() > 0,
+ "no card is drawn, so there is no layout to assert about");
+ QCOMPARE(view->horizontalScrollBar()->minimum(),
+ view->horizontalScrollBar()->maximum());
+}
+
+void TestMainWindow::aThreadWithRepliesDrawsAVisibleExpander()
{
- // The pills are a row-wide strip under the cells, not content of the
- // subject cell. Drawn from the subject column's delegate they stop at that
- // column's edge, so a thread with several tags loses the last of them; and
- // they inherit the column's left edge, which puts them under the subject
- // rather than under the row.
+ // The expander is the ONLY thing saying a thread can be opened, and it took
+ // four wrong attempts to get on screen before item 53, each of which looked
+ // correct in code and none of which a geometry or role assertion could see.
+ // So this counts painted pixels.
//
- // The property: pills appear to the LEFT of where the subject column
- // starts, which no per-cell delegate on that column could produce.
+ // Painted through the DELEGATE rather than through viewport()->render().
+ // The viewport render returns a blank image here: CLAUDE.md records that it
+ // does so in several ordinary situations, and this test proved it again,
+ // reporting zero ink over a card the delegate demonstrably paints 2183
+ // pixels into. A probe that sees nothing cannot report on anything.
const Config config;
MainWindow window(config);
auto *model = window.findChild<ThreadListModel *>();
QVERIFY(model);
- auto *view = window.findChild<QTableView *>();
+ auto *view = window.findChild<QTreeView *>();
QVERIFY(view);
- ThreadSummary thread = makeThread(QStringLiteral("t1"), {});
- thread.tags = QStringList{ QStringLiteral("mailing-list/SBo"),
- QStringLiteral("signed") };
- model->appendBatch({ thread });
+ // Two threads: one with replies, one without. The second is the control,
+ // and without it a test that counts ink would pass on any card.
+ ThreadSummary withReplies = makeThread(
+ QStringLiteral("t1"),
+ QStringList{ TagColors::tagForAccountKey(QStringLiteral("work")) });
+ withReplies.totalCount = 3;
+ ThreadSummary lone = makeThread(
+ QStringLiteral("t2"),
+ QStringList{ TagColors::tagForAccountKey(QStringLiteral("work")) });
+ lone.totalCount = 1;
+ model->appendBatch({ withReplies, lone });
+
+ const QModelIndex first = model->index(0, 0, QModelIndex());
+ const QModelIndex second = model->index(1, 0, QModelIndex());
+
+ // Guards: the model agrees about which thread has replies, and only that
+ // one is offered an expander at all.
+ QCOMPARE(model->data(first, ThreadListModel::ReplyCountRole).toInt(), 2);
+ QCOMPARE(model->data(second, ThreadListModel::ReplyCountRole).toInt(), 0);
+
+ const QFont font = view->font();
+ const int height = CardLayout::heightFor(font);
+
+ const auto inkInExpander = [&](const QModelIndex &index) {
+ QImage shot(400, height, QImage::Format_ARGB32);
+ shot.fill(Qt::white);
+ QPainter painter(&shot);
+ QStyleOptionViewItem option;
+ option.rect = QRect(0, 0, 400, height);
+ option.font = font;
+ option.palette = QApplication::palette();
+ option.state = QStyle::State_Enabled;
+ CardDelegate delegate;
+ delegate.paint(&painter, option, index);
+ painter.end();
+
+ const QRect rect = CardDelegate::expanderRectFor(option, index);
+ int found = 0;
+ for (int y = rect.top(); y <= rect.bottom() && y < shot.height(); ++y) {
+ for (int x = rect.left(); x <= rect.right() && x < shot.width();
+ ++x) {
+ if ((shot.pixel(x, y) | 0xff000000) != 0xffffffffu)
+ ++found;
+ }
+ }
+
+ // Guard on the probe itself: prove it can see the card's own text
+ // before trusting it about the expander. A probe that finds no ink
+ // anywhere reports "nothing was drawn" whatever the delegate did.
+ int anyInk = 0;
+ for (int y = 0; y < shot.height(); ++y)
+ for (int x = 0; x < shot.width(); ++x)
+ if ((shot.pixel(x, y) | 0xff000000) != 0xffffffffu)
+ ++anyInk;
+ return std::pair<int, int>(found, anyInk);
+ };
+
+ const auto [drawn, drawnAnywhere] = inkInExpander(first);
+ const auto [control, controlAnywhere] = inkInExpander(second);
+
+ QVERIFY2(drawnAnywhere > 0 && controlAnywhere > 0,
+ "the probe finds no ink on either card, so it cannot report on "
+ "the expander either");
+
+ QVERIFY2(drawn > 12,
+ qPrintable(QStringLiteral("only %1 pixels in the expander's rect: "
+ "the reply count is clipped or painted "
+ "over").arg(drawn)));
+ QVERIFY2(control == 0,
+ qPrintable(QStringLiteral("a thread with no replies drew %1 "
+ "pixels where an expander would go")
+ .arg(control)));
+}
+
+void TestMainWindow::selectingAThreadRowNamesHowManyMessagesItStandsFor()
+{
+ // With two kinds of row selectable, one selected row no longer says how
+ // much an action will touch. CLAUDE.md forbids a confirmation dialog for
+ // tag mutations, so the scope is made visible instead: this is the "before"
+ // half of that, and the count has to come from the thread's own total, not
+ // from whatever happens to be expanded.
+ const Config config;
+ MainWindow window(config);
+
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<QTreeView *>();
+ QVERIFY(view);
+ auto *status = window.findChild<QLabel *>(QStringLiteral("statusMessage"));
+ QVERIFY(status);
+
+ ThreadSummary t = makeThread(QStringLiteral("t1"), {});
+ t.totalCount = 7;
+ model->appendBatch({ t });
+
+ window.show();
+ QVERIFY(QTest::qWaitForWindowExposed(&window));
+
+ // Guard: nothing is expanded, so a count taken from the loaded children
+ // would read 0 and this test would be measuring the wrong source.
+ QCOMPARE(model->rowCount(model->index(0, 0, QModelIndex())), 0);
+
+ selectThreadRow(view, 0);
+ QApplication::processEvents();
+
+ QVERIFY2(status->text().contains(QStringLiteral("7")),
+ qPrintable(QStringLiteral("the status bar says '%1', which does "
+ "not name the 7 messages the thread "
+ "stands for")
+ .arg(status->text())));
+}
+
+void TestMainWindow::selectingAMessageRowReportsNoBulkCount()
+{
+ // Reading one message is not a bulk action, so it gets no count. A message
+ // row reporting "1 thread selected" would be actively wrong about what an
+ // action would touch.
+ const Config config;
+ MainWindow window(config);
+
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<QTreeView *>();
+ QVERIFY(view);
+ auto *status = window.findChild<QLabel *>(QStringLiteral("statusMessage"));
+ QVERIFY(status);
+
+ ThreadSummary t = makeThread(QStringLiteral("t1"), {});
+ t.totalCount = 3;
+ model->appendBatch({ t });
+
+ MessageNode root;
+ root.messageId = QStringLiteral("m0@example.org");
+ root.threadId = QStringLiteral("t1");
+ root.depth = 0;
+ MessageNode reply;
+ reply.messageId = QStringLiteral("m1@example.org");
+ reply.threadId = QStringLiteral("t1");
+ reply.depth = 1;
+ model->setThreadMessages(QStringLiteral("t1"), { root, reply });
+
+ window.show();
+ QVERIFY(QTest::qWaitForWindowExposed(&window));
+
+ const QModelIndex threadRow = model->index(0, 0, QModelIndex());
+ view->expand(threadRow);
+ QApplication::processEvents();
+
+ const QModelIndex messageRow = model->index(0, 0, threadRow);
+ QVERIFY(model->isMessageRow(messageRow));
+
+ view->selectionModel()->select(
+ messageRow,
+ QItemSelectionModel::ClearAndSelect | QItemSelectionModel::Rows);
+ view->setCurrentIndex(messageRow);
+ QApplication::processEvents();
+
+ QVERIFY2(!status->text().contains(QStringLiteral("thread")),
+ qPrintable(QStringLiteral("a single message row reports '%1', "
+ "which claims a thread-wide scope it "
+ "does not have")
+ .arg(status->text())));
+}
+
+void TestMainWindow::anActionOnAThreadRowSaysItHitTheWholeThread()
+{
+ // The "after" half. Undo is the safety net this project chose over a
+ // confirmation dialog, and undo is only usable if the user can tell that
+ // something bigger than they intended just happened.
+ const Config config;
+ MainWindow window(config);
+
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<QTreeView *>();
+ QVERIFY(view);
+ auto *status = window.findChild<QLabel *>(QStringLiteral("statusMessage"));
+ QVERIFY(status);
+
+ ThreadSummary t = makeThread(QStringLiteral("t1"), {});
+ t.totalCount = 7;
+ model->appendBatch({ t });
+
+ window.show();
+ QVERIFY(QTest::qWaitForWindowExposed(&window));
+
+ selectThreadRow(view, 0);
+ QApplication::processEvents();
+
+ auto *archive = window.findChild<QAction *>(QStringLiteral("archive"));
+ QVERIFY2(archive, "no archive action to trigger");
+ archive->trigger();
+
+ // Read BEFORE processEvents, deliberately. This binary has no worker
+ // (backlog item 36), so the queued applyTagsToThreads reaches a throwaway
+ // database that has never heard of thread t1 and answers with
+ // errorOccurred, which overwrites the status bar. Draining the event loop
+ // here would assert on that error rather than on the scope message, and
+ // the test would fail against correct code.
+ const QString message = status->text();
+
+ QVERIFY2(message.contains(QStringLiteral("7")),
+ qPrintable(QStringLiteral("after archiving a 7-message thread the "
+ "status bar says '%1', which does not "
+ "say how much was touched")
+ .arg(message)));
+
+ // And it must say the whole thread went, not merely how many messages: the
+ // count alone does not distinguish "7 messages you picked" from "7 messages
+ // because you picked their thread".
+ QVERIFY2(message.contains(QStringLiteral("whole thread")),
+ qPrintable(QStringLiteral("the status bar says '%1', which does "
+ "not say the action took the whole "
+ "thread")
+ .arg(message)));
+}
+
+void TestMainWindow::anActionOnAMessageRowTagsThatMessageNotTheThread()
+{
+ // The routing itself, which nothing else here can see. A message row sent
+ // down the THREAD path produces the same undo depth and the same status
+ // text while tagging every sibling in the conversation: a mutation that did
+ // exactly that passed the entire suite, so this test exists because that
+ // gap was found rather than because the path looked risky.
+ const Config config;
+ MainWindow window(config);
+
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<QTreeView *>();
+ QVERIFY(view);
+
+ ThreadSummary t = makeThread(QStringLiteral("t1"), {});
+ t.totalCount = 3;
+ model->appendBatch({ t });
+
+ MessageNode root;
+ root.messageId = QStringLiteral("m0@example.org");
+ root.threadId = QStringLiteral("t1");
+ root.depth = 0;
+ MessageNode reply;
+ reply.messageId = QStringLiteral("m1@example.org");
+ reply.threadId = QStringLiteral("t1");
+ reply.depth = 1;
+ model->setThreadMessages(QStringLiteral("t1"), { root, reply });
+
+ window.show();
+ QVERIFY(QTest::qWaitForWindowExposed(&window));
+
+ const QModelIndex threadRow = model->index(0, 0, QModelIndex());
+ view->expand(threadRow);
+ QApplication::processEvents();
+
+ const QModelIndex messageRow = model->index(0, 0, threadRow);
+ QVERIFY(model->isMessageRow(messageRow));
+
+ view->selectionModel()->select(
+ messageRow,
+ QItemSelectionModel::ClearAndSelect | QItemSelectionModel::Rows);
+ view->setCurrentIndex(messageRow);
+ QApplication::processEvents();
+
+ auto *archive = window.findChild<QAction *>(QStringLiteral("archive"));
+ QVERIFY(archive);
+ archive->trigger();
+
+ // The change must carry the MESSAGE id and no thread id. Sent as a thread
+ // id it would archive the root and every other reply along with it.
+ QCOMPARE(window.pendingMessageIdsForTesting(),
+ QStringList{ QStringLiteral("m1@example.org") });
+ QVERIFY2(window.pendingThreadIdsForTesting().isEmpty(),
+ qPrintable(QStringLiteral("the action was sent for thread(s) %1: a "
+ "message row must not tag its siblings")
+ .arg(window.pendingThreadIdsForTesting()
+ .join(QStringLiteral(", ")))));
+
+ // And it is undoable, on its own terms rather than the thread's.
+ QCOMPARE(window.undoDepthForTesting(), 1);
+}
+
+void TestMainWindow::selectingAMessageRowTargetsThatMessageNotItsThread()
+{
+ // test_mainwindow has no worker (backlog item 36), so this cannot assert on
+ // what the pane renders. What it CAN assert is the decision the UI makes:
+ // a message row must stop tracking a current thread, or a reply arriving
+ // for either kind of selection cannot tell which one it belongs to.
+ //
+ // The trap this covers is specific. threadAt() takes a TOP-LEVEL row
+ // number, and a child's row number indexes its siblings, so handing a
+ // message row's number to it loads whichever thread happens to sit at that
+ // position in the list. Row 0 under a thread is a plausible-looking wrong
+ // answer, which is why the fixture puts the reply under the SECOND thread.
+ const Config config;
+ MainWindow window(config);
+
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<QTreeView *>();
+ QVERIFY(view);
+
+ ThreadSummary first = makeThread(QStringLiteral("t1"), {});
+ ThreadSummary second = makeThread(QStringLiteral("t2"), {});
+ second.totalCount = 2;
+ model->appendBatch({ first, second });
+
+ MessageNode root;
+ root.messageId = QStringLiteral("m0@example.org");
+ root.threadId = QStringLiteral("t2");
+ root.depth = 0;
+ MessageNode reply;
+ reply.messageId = QStringLiteral("m1@example.org");
+ reply.threadId = QStringLiteral("t2");
+ reply.depth = 1;
+ model->setThreadMessages(QStringLiteral("t2"), { root, reply });
window.resize(1400, 300);
window.show();
QVERIFY(QTest::qWaitForWindowExposed(&window));
+
+ // Start on a thread row, so the transition to a message row is what is
+ // being observed rather than the initial state.
+ const QModelIndex threadRow = model->index(1, 0, QModelIndex());
+ selectThreadRow(view, 1);
QApplication::processEvents();
+ QCOMPARE(window.currentThreadId(), QStringLiteral("t2"));
- const int subjectLeft =
- view->columnViewportPosition(ThreadListModel::SubjectColumn);
- QVERIFY2(subjectLeft > 40,
- qPrintable(QStringLiteral("the subject column starts at x=%1, too "
- "close to the left edge to tell a "
- "row-wide strip from a subject-cell one")
- .arg(subjectLeft)));
- // The strip must have somewhere to paint that the subject cell does not
- // reach, or this test cannot fail.
- QVERIFY2(subjectLeft < view->viewport()->width(),
- qPrintable(QStringLiteral("the subject column is off-screen "
- "(x=%1, viewport %2), so nothing it "
- "draws is measurable")
- .arg(subjectLeft)
- .arg(view->viewport()->width())));
+ view->expand(threadRow);
+ QApplication::processEvents();
+
+ const QModelIndex messageRow = model->index(0, 0, threadRow);
+ QVERIFY(messageRow.isValid());
+ QVERIFY2(model->isMessageRow(messageRow),
+ "the fixture did not produce a message row, so this test would "
+ "assert nothing about one");
+
+ view->selectionModel()->select(
+ messageRow,
+ QItemSelectionModel::ClearAndSelect | QItemSelectionModel::Rows);
+ view->setCurrentIndex(messageRow);
+ QApplication::processEvents();
+
+ // The thread is no longer what the pane is about. Left set, a late
+ // loadThread reply would repaint the whole conversation over the single
+ // message the user asked for.
+ QVERIFY2(window.currentThreadId().isEmpty(),
+ qPrintable(QStringLiteral("selecting a reply left the current "
+ "thread set to '%1': the pane is still "
+ "tracking the conversation")
+ .arg(window.currentThreadId())));
+}
+
+void TestMainWindow::clickingTheExpanderTogglesTheThread()
+{
+ // The glyph being VISIBLE and the glyph being CLICKABLE are separate
+ // properties, and the pixel test for the first passes happily against a
+ // triangle nothing can hit. Turning off rootIsDecorated to stop the style
+ // drawing its own dot under ours also removed the style's hit area, so the
+ // expander rendered perfectly and did nothing.
+ const Config config;
+ MainWindow window(config);
+
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<QTreeView *>();
+ QVERIFY(view);
+
+ ThreadSummary t = makeThread(
+ QStringLiteral("t1"),
+ QStringList{ TagColors::tagForAccountKey(QStringLiteral("work")) });
+ t.totalCount = 3;
+ model->appendBatch({ t });
+
+ window.resize(1400, 300);
+ window.show();
+ QVERIFY(QTest::qWaitForWindowExposed(&window));
+ QApplication::processEvents();
+
+ const QModelIndex root = model->index(0, 0, QModelIndex());
+ const QModelIndex subject =
+ model->index(0, 0, QModelIndex());
+ const QRect rect = view->visualRect(subject);
+
+ // Guards: the row is drawn, it claims to have replies, and it starts
+ // collapsed. Without the last one a toggle test can pass by doing nothing.
+ QVERIFY2(rect.height() > 0, "the thread row is not on screen");
+ QVERIFY(model->data(subject, ThreadListModel::HasRepliesRole).toBool());
+ QVERIFY(!view->isExpanded(root));
+
+ // Aimed at the rect the delegate reports, not at one reconstructed here:
+ // the drawn target and the clickable one cannot drift if both come from
+ // the same call.
+ QStyleOptionViewItem option;
+ option.rect = rect;
+ option.font = view->font();
+ const QRect expander = CardDelegate::expanderRectFor(option, subject);
+ QVERIFY2(!expander.isEmpty(), "the card offers no expander to click");
+ const QPoint hit = expander.center();
+
+ QTest::mouseClick(view->viewport(), Qt::LeftButton, Qt::NoModifier, hit);
+ QApplication::processEvents();
+ QVERIFY2(view->isExpanded(root),
+ "clicking the expander did not open the thread");
+
+ QTest::mouseClick(view->viewport(), Qt::LeftButton, Qt::NoModifier, hit);
+ QApplication::processEvents();
+ QVERIFY2(!view->isExpanded(root),
+ "clicking the expander again did not close the thread");
+}
+
+void TestMainWindow::replyRowsKeepTheirTextUnderTheThreadLine()
+{
+ // paintEvent runs AFTER the cells, so anything it fills across a reply row
+ // covers the text the delegate just drew. The tint and the thread line are
+ // both painted there, which makes this the obvious way to ship a block of
+ // blank rows.
+ const Config config;
+ MainWindow window(config);
+
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<QTreeView *>();
+ QVERIFY(view);
+
+ ThreadSummary t = makeThread(
+ QStringLiteral("t1"),
+ QStringList{ TagColors::tagForAccountKey(QStringLiteral("work")) });
+ t.totalCount = 2;
+ model->appendBatch({ t });
+
+ MessageNode first;
+ first.messageId = QStringLiteral("m0@example.org");
+ first.threadId = QStringLiteral("t1");
+ first.depth = 0;
+ MessageNode reply;
+ reply.messageId = QStringLiteral("m1@example.org");
+ reply.threadId = QStringLiteral("t1");
+ reply.from = QStringLiteral("A Replier <replier@example.org>");
+ reply.subject = QStringLiteral("Re: a subject");
+ reply.depth = 1;
+ model->setThreadMessages(QStringLiteral("t1"), { first, reply });
+
+ window.resize(1400, 300);
+ window.show();
+ QVERIFY(QTest::qWaitForWindowExposed(&window));
+
+ const QModelIndex root = model->index(0, 0, QModelIndex());
+ view->expand(root);
+ QApplication::processEvents();
+
+ const QModelIndex child =
+ model->index(0, 0, root);
+ const QRect rect = view->visualRect(child);
+ QVERIFY2(rect.height() > 0, "the reply row is not on screen");
QImage shot(view->viewport()->size(), QImage::Format_ARGB32);
shot.fill(Qt::transparent);
view->viewport()->render(&shot);
- // Count pixels matching the tag colours EXACTLY, not "saturated" pixels.
- // A looser test counts the antialiased edge of the selection highlight
- // blending into the background, which is several hundred distinct
- // near-background colours and passes whatever the strip does. Both earlier
- // versions of this test did precisely that.
- QSet<QRgb> pillColours;
- const QVariantList colours =
- model->index(0, ThreadListModel::SubjectColumn)
- .data(ThreadListModel::PillColoursRole).toList();
- QVERIFY2(!colours.isEmpty(), "the model supplied no pill colours");
- for (const QVariant &colour : colours)
- pillColours.insert(colour.value<QColor>().rgb());
-
- const int rowHeight = view->rowHeight(0);
- QVERIFY(rowHeight > 0);
-
- int chipPixels = 0;
- for (int y = 0; y < qMin(rowHeight, shot.height()); ++y) {
- for (int x = 0; x < qMin(subjectLeft, shot.width()); ++x) {
- if (pillColours.contains(shot.pixel(x, y) | 0xff000000))
- ++chipPixels;
+ // Count pixels in the sender cell that differ from the row's own tint.
+ // Text is the only thing that can produce them.
+ const QRgb tint = ThreadListModel::replyBackground().rgb() | 0xff000000;
+ int textPixels = 0;
+ for (int y = rect.top(); y < qMin(rect.bottom(), shot.height()); ++y) {
+ for (int x = rect.left(); x < qMin(rect.right(), shot.width()); ++x) {
+ if ((shot.pixel(x, y) | 0xff000000) != tint)
+ ++textPixels;
}
}
- QVERIFY2(chipPixels > 0,
- "no pill-coloured pixels left of the subject column: the strip is "
- "still confined to that cell rather than spanning the row");
+ QVERIFY2(textPixels > 20,
+ qPrintable(QStringLiteral("only %1 non-background pixels in the "
+ "reply's sender cell: the row was "
+ "painted over after its text was drawn")
+ .arg(textPixels)));
}
void TestMainWindow::aSelectedReadThreadIsNotDimmedIntoTheHighlight()
@@ -607,7 +1440,7 @@ void TestMainWindow::aSelectedReadThreadIsNotDimmedIntoTheHighlight()
auto *model = window.findChild<ThreadListModel *>();
QVERIFY(model);
- auto *view = window.findChild<QTableView *>();
+ auto *view = window.findChild<QTreeView *>();
QVERIFY(view);
// Both rows READ, so both are dimmed and neither is bold: the only thing
@@ -633,10 +1466,10 @@ void TestMainWindow::aSelectedReadThreadIsNotDimmedIntoTheHighlight()
// dimmed row switches it to the highlight's own text colour, so the two
// rows MUST differ; comparing two identically-styled rows would pass
// against a delegate that did nothing at all.
- view->selectRow(0);
+ selectThreadRow(view, 0);
QApplication::processEvents();
- const int rowHeight = view->rowHeight(0);
+ const int rowHeight = threadRowHeight(view, 0);
QVERIFY(rowHeight > 0);
QImage shot(view->viewport()->size(), QImage::Format_ARGB32);
@@ -657,15 +1490,15 @@ void TestMainWindow::aSelectedReadThreadIsNotDimmedIntoTheHighlight()
QVERIFY2(delegate, "the thread view has no styled delegate");
const QModelIndex index =
- model->index(0, ThreadListModel::SubjectColumn);
+ model->index(0, 0);
// initStyleOption is protected, so the resolved palette is reached the way
// the painter does: through a subclass that exposes it.
- struct Probe : SubjectDelegate {
- using SubjectDelegate::initStyleOption;
+ struct Probe : CardDelegate {
+ using CardDelegate::initStyleOption;
};
const auto *probe = static_cast<const Probe *>(
- static_cast<const SubjectDelegate *>(delegate));
+ static_cast<const CardDelegate *>(delegate));
probe->initStyleOption(&selected, index);
probe->initStyleOption(&unselected, index);
@@ -724,7 +1557,7 @@ void TestMainWindow::markAllReadActsOnEveryRowAndUndoesInOneStep()
auto *model = window.findChild<ThreadListModel *>();
QVERIFY(model);
- auto *view = window.findChild<QTableView *>();
+ auto *view = window.findChild<QTreeView *>();
QVERIFY(view);
auto *action = window.findChild<QAction *>(QStringLiteral("mark_all_read"));
QVERIFY(action);
@@ -746,7 +1579,7 @@ void TestMainWindow::markAllReadActsOnEveryRowAndUndoesInOneStep()
// One row selected, to prove the action ignores the selection rather than
// acting on it.
- view->selectRow(0);
+ selectThreadRow(view, 0);
action->trigger();
@@ -831,7 +1664,7 @@ void TestMainWindow::markReadTimerRestartsRatherThanStacking()
QVERIFY(model);
auto *timer = window.findChild<QTimer *>(QStringLiteral("markReadTimer"));
QVERIFY(timer);
- auto *view = window.findChild<QTableView *>();
+ auto *view = window.findChild<QTreeView *>();
QVERIFY(view);
model->appendBatch({ makeThread(QStringLiteral("t1"),
@@ -841,13 +1674,13 @@ void TestMainWindow::markReadTimerRestartsRatherThanStacking()
makeThread(QStringLiteral("t3"),
{ QStringLiteral("unread") }) });
- view->selectRow(0);
+ selectThreadRow(view, 0);
QVERIFY2(timer->isActive(), "no timer armed for an unread thread");
// Move on before it can fire. One timer stays armed, not three.
- view->selectRow(1);
+ selectThreadRow(view, 1);
QVERIFY(timer->isActive());
- view->selectRow(2);
+ selectThreadRow(view, 2);
QVERIFY(timer->isActive());
// Exactly one timer exists at all, which is what "restarted, not stacked"
@@ -867,7 +1700,7 @@ void TestMainWindow::markReadTimerIsNotArmedForAReadThread()
QVERIFY(model);
auto *timer = window.findChild<QTimer *>(QStringLiteral("markReadTimer"));
QVERIFY(timer);
- auto *view = window.findChild<QTableView *>();
+ auto *view = window.findChild<QTreeView *>();
QVERIFY(view);
model->appendBatch({ makeThread(QStringLiteral("read"),
@@ -875,16 +1708,16 @@ void TestMainWindow::markReadTimerIsNotArmedForAReadThread()
makeThread(QStringLiteral("unread"),
{ QStringLiteral("unread") }) });
- view->selectRow(0);
+ selectThreadRow(view, 0);
QVERIFY2(!timer->isActive(), "armed a timer for an already-read thread");
// And the unread one still arms, so this is not "never arms".
- view->selectRow(1);
+ selectThreadRow(view, 1);
QVERIFY(timer->isActive());
// Moving back to a read thread disarms it again, rather than leaving the
// previous thread's timer running to fire against the wrong row.
- view->selectRow(0);
+ selectThreadRow(view, 0);
QVERIFY(!timer->isActive());
}
@@ -909,12 +1742,12 @@ void TestMainWindow::markReadCanBeDisabled()
QVERIFY(model);
auto *timer = window.findChild<QTimer *>(QStringLiteral("markReadTimer"));
QVERIFY(timer);
- auto *view = window.findChild<QTableView *>();
+ auto *view = window.findChild<QTreeView *>();
QVERIFY(view);
model->appendBatch({ makeThread(QStringLiteral("t1"),
{ QStringLiteral("unread") }) });
- view->selectRow(0);
+ selectThreadRow(view, 0);
QVERIFY2(!timer->isActive(),
"a negative mark_read_delay_ms must disable the timer");
@@ -1091,7 +1924,7 @@ void TestMainWindow::selectAllIsBoundAndSelectsEveryRow()
auto *model = window.findChild<ThreadListModel *>();
QVERIFY(model);
- auto *view = window.findChild<QTableView *>();
+ auto *view = window.findChild<QTreeView *>();
QVERIFY(view);
model->appendBatch({ makeThread(QStringLiteral("t1"), {}),
@@ -1120,7 +1953,7 @@ void TestMainWindow::aMultiRowSelectionDoesNotArmTheMarkReadTimer()
auto *model = window.findChild<ThreadListModel *>();
QVERIFY(model);
- auto *view = window.findChild<QTableView *>();
+ auto *view = window.findChild<QTreeView *>();
QVERIFY(view);
auto *timer = window.findChild<QTimer *>(QStringLiteral("markReadTimer"));
QVERIFY(timer);
@@ -1134,7 +1967,7 @@ void TestMainWindow::aMultiRowSelectionDoesNotArmTheMarkReadTimer()
// Sweep down as Shift+arrow does: current moves onto a row while the
// selection already spans more than one.
- view->selectRow(0);
+ selectThreadRow(view, 0);
view->selectionModel()->select(
model->index(1, 0),
QItemSelectionModel::Select | QItemSelectionModel::Rows);
@@ -1159,7 +1992,7 @@ void TestMainWindow::growingASelectionCancelsAnAlreadyArmedTimer()
auto *model = window.findChild<ThreadListModel *>();
QVERIFY(model);
- auto *view = window.findChild<QTableView *>();
+ auto *view = window.findChild<QTreeView *>();
QVERIFY(view);
auto *timer = window.findChild<QTimer *>(QStringLiteral("markReadTimer"));
QVERIFY(timer);
@@ -1169,7 +2002,7 @@ void TestMainWindow::growingASelectionCancelsAnAlreadyArmedTimer()
makeThread(QStringLiteral("t2"),
{ QStringLiteral("unread") }) });
- view->selectRow(0);
+ selectThreadRow(view, 0);
QVERIFY2(timer->isActive(), "no timer armed for a single unread thread");
// Extend to a second row, as Shift+click would.
@@ -1191,7 +2024,7 @@ void TestMainWindow::collapsingBackToOneRowLoadsThatThreadAgain()
auto *model = window.findChild<ThreadListModel *>();
QVERIFY(model);
- auto *view = window.findChild<QTableView *>();
+ auto *view = window.findChild<QTreeView *>();
QVERIFY(view);
auto *timer = window.findChild<QTimer *>(QStringLiteral("markReadTimer"));
QVERIFY(timer);
@@ -1205,7 +2038,7 @@ void TestMainWindow::collapsingBackToOneRowLoadsThatThreadAgain()
QVERIFY(!timer->isActive());
// Back to one row, as a plain click would leave it.
- view->selectRow(1);
+ selectThreadRow(view, 1);
QVERIFY2(timer->isActive(),
"collapsing back to one row did not resume mark-read");
@@ -1221,7 +2054,7 @@ void TestMainWindow::theStatusBarReportsAMultiRowSelection()
auto *model = window.findChild<ThreadListModel *>();
QVERIFY(model);
- auto *view = window.findChild<QTableView *>();
+ auto *view = window.findChild<QTreeView *>();
QVERIFY(view);
auto *status = window.findChild<QLabel *>(QStringLiteral("statusMessage"));
QVERIFY2(status, "no status label to report into");
@@ -1246,7 +2079,7 @@ void TestMainWindow::clearSelectionBlanksThePaneAndDeselects()
auto *model = window.findChild<ThreadListModel *>();
QVERIFY(model);
- auto *view = window.findChild<QTableView *>();
+ auto *view = window.findChild<QTreeView *>();
QVERIFY(view);
model->appendBatch({ makeThread(QStringLiteral("t1"), {}),
@@ -1256,7 +2089,7 @@ void TestMainWindow::clearSelectionBlanksThePaneAndDeselects()
// and what CLAUDE.md requires: selectAll() on a fresh view emits no
// currentRowChanged at all, so a test starting there passes against a
// missing guard.
- view->selectRow(0);
+ selectThreadRow(view, 0);
QCOMPARE(view->selectionModel()->selectedRows().size(), 1);
auto *action = window.findChild<QAction *>(QStringLiteral("clear_selection"));
@@ -1302,13 +2135,13 @@ void TestMainWindow::clearPaneLeavesTheSelectionAlone()
auto *model = window.findChild<ThreadListModel *>();
QVERIFY(model);
- auto *view = window.findChild<QTableView *>();
+ auto *view = window.findChild<QTreeView *>();
QVERIFY(view);
model->appendBatch({ makeThread(QStringLiteral("t1"), {}),
makeThread(QStringLiteral("t2"), {}) });
- view->selectRow(0);
+ selectThreadRow(view, 0);
QCOMPARE(view->selectionModel()->selectedRows().size(), 1);
auto *action = window.findChild<QAction *>(QStringLiteral("clear_pane"));
@@ -1421,7 +2254,7 @@ void TestMainWindow::theThreadListOffersAContextMenu()
const Config config;
MainWindow window(config);
- auto *view = window.findChild<QTableView *>();
+ auto *view = window.findChild<QTreeView *>();
QVERIFY(view);
QCOMPARE(view->contextMenuPolicy(), Qt::CustomContextMenu);
@@ -1464,7 +2297,7 @@ void TestMainWindow::aSecondRowBlanksThePaneNotOnlyAThird()
auto *model = window.findChild<ThreadListModel *>();
QVERIFY(model);
- auto *view = window.findChild<QTableView *>();
+ auto *view = window.findChild<QTreeView *>();
QVERIFY(view);
auto *timer = window.findChild<QTimer *>(QStringLiteral("markReadTimer"));
QVERIFY(timer);
@@ -1477,7 +2310,7 @@ void TestMainWindow::aSecondRowBlanksThePaneNotOnlyAThird()
{ QStringLiteral("unread") }) });
// One row: ordinary reading, so a timer is armed and a thread is current.
- view->selectRow(0);
+ selectThreadRow(view, 0);
QCOMPARE(view->selectionModel()->selectedRows().size(), 1);
QVERIFY(timer->isActive());
@@ -1829,13 +2662,13 @@ void TestMainWindow::escapeBlanksTheMessagePane()
auto *model = window.findChild<ThreadListModel *>();
QVERIFY(model);
- auto *view = window.findChild<QTableView *>();
+ auto *view = window.findChild<QTreeView *>();
QVERIFY(view);
model->appendBatch({ makeThread(QStringLiteral("t1"), {}),
makeThread(QStringLiteral("t2"), {}) });
- view->selectRow(0);
+ selectThreadRow(view, 0);
QVERIFY2(!window.currentThreadId().isEmpty(),
"no thread was opened to blank");
@@ -1856,14 +2689,14 @@ void TestMainWindow::deleteTogglesOnAnAlreadyDeletedThread()
auto *model = window.findChild<ThreadListModel *>();
QVERIFY(model);
- auto *view = window.findChild<QTableView *>();
+ auto *view = window.findChild<QTreeView *>();
QVERIFY(view);
auto *action = window.findChild<QAction *>(QStringLiteral("delete"));
QVERIFY(action);
model->appendBatch({ makeThread(QStringLiteral("t1"),
{ QStringLiteral("deleted") }) });
- view->selectRow(0);
+ selectThreadRow(view, 0);
action->trigger();
@@ -1884,7 +2717,7 @@ void TestMainWindow::deleteOnAMixedSelectionDeletesRatherThanSplittingIt()
auto *model = window.findChild<ThreadListModel *>();
QVERIFY(model);
- auto *view = window.findChild<QTableView *>();
+ auto *view = window.findChild<QTreeView *>();
QVERIFY(view);
auto *action = window.findChild<QAction *>(QStringLiteral("delete"));
QVERIFY(action);
@@ -1935,7 +2768,7 @@ void TestMainWindow::theSelectionCountIsStateAndDoesNotExpire()
auto *model = window.findChild<ThreadListModel *>();
QVERIFY(model);
- auto *view = window.findChild<QTableView *>();
+ auto *view = window.findChild<QTreeView *>();
QVERIFY(view);
auto *status = window.findChild<QLabel *>(QStringLiteral("statusMessage"));
QVERIFY(status);
@@ -2059,13 +2892,13 @@ void TestMainWindow::anEditDuringABackgroundSyncIsNotSentYet()
auto *model = window.findChild<ThreadListModel *>();
QVERIFY(model);
- auto *view = window.findChild<QTableView *>();
+ auto *view = window.findChild<QTreeView *>();
QVERIFY(view);
auto *action = window.findChild<QAction *>(QStringLiteral("flag"));
QVERIFY2(action, "no flag action registered");
model->appendBatch({ makeThread(QStringLiteral("t1"), {}) });
- view->selectRow(0);
+ selectThreadRow(view, 0);
// A cron sync takes the lock.
QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged",
@@ -2090,13 +2923,13 @@ void TestMainWindow::aHeldEditIsSentWhenTheBackgroundSyncEnds()
auto *model = window.findChild<ThreadListModel *>();
QVERIFY(model);
- auto *view = window.findChild<QTableView *>();
+ auto *view = window.findChild<QTreeView *>();
QVERIFY(view);
auto *action = window.findChild<QAction *>(QStringLiteral("flag"));
QVERIFY(action);
model->appendBatch({ makeThread(QStringLiteral("t1"), {}) });
- view->selectRow(0);
+ selectThreadRow(view, 0);
QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged",
Q_ARG(SyncMonitor::State,
@@ -2125,7 +2958,7 @@ void TestMainWindow::aHeldEditCountsAsUnsynced()
auto *model = window.findChild<ThreadListModel *>();
QVERIFY(model);
- auto *view = window.findChild<QTableView *>();
+ auto *view = window.findChild<QTreeView *>();
QVERIFY(view);
auto *action = window.findChild<QAction *>(QStringLiteral("flag"));
QVERIFY(action);
@@ -2134,7 +2967,7 @@ void TestMainWindow::aHeldEditCountsAsUnsynced()
QVERIFY2(label->isHidden(), "the indicator starts hidden at zero");
model->appendBatch({ makeThread(QStringLiteral("t1"), {}) });
- view->selectRow(0);
+ selectThreadRow(view, 0);
QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged",
Q_ARG(SyncMonitor::State,
@@ -2162,7 +2995,7 @@ void TestMainWindow::anUnreadableLockTableStillSendsTheEdit()
auto *model = window.findChild<ThreadListModel *>();
QVERIFY(model);
- auto *view = window.findChild<QTableView *>();
+ auto *view = window.findChild<QTreeView *>();
QVERIFY(view);
auto *action = window.findChild<QAction *>(QStringLiteral("flag"));
QVERIFY(action);
@@ -2173,7 +3006,7 @@ void TestMainWindow::anUnreadableLockTableStillSendsTheEdit()
QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged",
Q_ARG(SyncMonitor::State,
SyncMonitor::State::Running));
- view->selectRow(0);
+ selectThreadRow(view, 0);
action->trigger();
QVERIFY2(window.hasEditAwaitingSend(),
"the edit was not held during a running sync, so this test is not "
@@ -2189,7 +3022,7 @@ void TestMainWindow::anUnreadableLockTableStillSendsTheEdit()
"platform without /proc/locks");
// And a NEW edit is sent rather than held.
- view->selectRow(1);
+ selectThreadRow(view, 1);
action->trigger();
QVERIFY2(!window.hasEditAwaitingSend(),
"an unreadable lock table held a new edit, so writes never resume");
@@ -2206,7 +3039,7 @@ void TestMainWindow::aRejectedWriteKeepsEarlierUndoHistory()
auto *model = window.findChild<ThreadListModel *>();
QVERIFY(model);
- auto *view = window.findChild<QTableView *>();
+ auto *view = window.findChild<QTreeView *>();
QVERIFY(view);
auto *flag = window.findChild<QAction *>(QStringLiteral("flag"));
QVERIFY(flag);
@@ -2215,7 +3048,7 @@ void TestMainWindow::aRejectedWriteKeepsEarlierUndoHistory()
model->appendBatch({ makeThread(QStringLiteral("t1"),
{ QStringLiteral("inbox") }) });
- view->selectRow(0);
+ selectThreadRow(view, 0);
// One edit that succeeds, so there is history worth keeping.
archive->trigger();
@@ -2609,7 +3442,7 @@ void TestMainWindow::theImportantActionStillWritesTheFlaggedTag()
auto *model = window.findChild<ThreadListModel *>();
QVERIFY(model);
- auto *view = window.findChild<QTableView *>();
+ auto *view = window.findChild<QTreeView *>();
QVERIFY(view);
model->appendBatch({ makeThread(QStringLiteral("t1"),
@@ -2619,7 +3452,7 @@ void TestMainWindow::theImportantActionStillWritesTheFlaggedTag()
// below would pass against an action that did nothing at all.
QVERIFY(!model->threadAt(0).isFlagged());
- view->selectRow(0);
+ selectThreadRow(view, 0);
auto *action = window.findChild<QAction *>(QStringLiteral("flag"));
QVERIFY(action);
diff --git a/tests/test_notmuchworker.cpp b/tests/test_notmuchworker.cpp
index b419915..88dcf0b 100644
--- a/tests/test_notmuchworker.cpp
+++ b/tests/test_notmuchworker.cpp
@@ -39,6 +39,8 @@ private slots:
void malformedQueryYieldsNoThreads();
void unreadableConfigEmitsError();
void queryPassesGenerationThrough();
+ void oldestFirstReversesTheOrder();
+ void theSortOrderCrossesAQueuedCall();
void loadThreadReturnsMessagesOldestFirst();
void loadThreadMarksMatchedMessages();
@@ -58,6 +60,11 @@ private slots:
void requestAllTagsReturnsSortedTags();
void requestAllTagsOnUnreadableConfigEmitsError();
+ void loadMessageReturnsOnlyThatMessage();
+ void loadMessageOnAnUnknownIdReturnsNothing();
+ void loadThreadTreeReportsReplyDepth();
+ void loadThreadTreeCarriesTheFactsARowNeeds();
+
void requestCountsAnswersOneCountPerQuery();
void requestCountsKeepsPositionOnAnInvalidQuery();
void requestDatabaseStatsCountsMessagesNotThreads();
@@ -68,7 +75,9 @@ private:
QStringList tagsOf(const QString &messageId);
QVector<MessageRef> messagesOfThread(const QString &threadId,
const QString &matchQuery = QString());
- QVector<ThreadSummary> runQuery(const QString &query);
+ QVector<ThreadSummary> runQuery(
+ const QString &query,
+ NotmuchWorker::SortOrder sort = NotmuchWorker::NewestFirst);
QString threadIdOf(const QString &subject);
NotmuchFixture m_fixture;
@@ -108,13 +117,14 @@ void TestNotmuchWorker::initTestCase()
QVERIFY2(m_fixture.index(), qPrintable(m_fixture.error()));
}
-QVector<ThreadSummary> TestNotmuchWorker::runQuery(const QString &query)
+QVector<ThreadSummary> TestNotmuchWorker::runQuery(
+ const QString &query, NotmuchWorker::SortOrder sort)
{
NotmuchWorker worker(m_fixture.configPath());
QSignalSpy ready(&worker, &NotmuchWorker::threadsReady);
QSignalSpy finished(&worker, &NotmuchWorker::queryFinished);
- worker.runQuery(query, 1);
+ worker.runQuery(query, 1, sort);
QVector<ThreadSummary> all;
for (const QList<QVariant> &args : ready)
@@ -158,6 +168,91 @@ QStringList TestNotmuchWorker::tagsOf(const QString &messageId)
return {};
}
+void TestNotmuchWorker::loadMessageReturnsOnlyThatMessage()
+{
+ // a2 is a reply in a two-message thread. Selecting a reply row must render
+ // that message alone; loadThread would hand back the whole thread and the
+ // pane would show the conversation the user was trying to look inside.
+ NotmuchWorker worker(m_fixture.configPath());
+ QSignalSpy loaded(&worker, &NotmuchWorker::messageLoaded);
+ worker.loadMessage(QStringLiteral("a2@example.org"), 1);
+
+ QCOMPARE(loaded.count(), 1);
+ const auto messages = loaded.first().at(0).value<QVector<MessageRef>>();
+
+ QCOMPARE(messages.size(), 1);
+ QCOMPARE(messages.first().messageId, QStringLiteral("a2@example.org"));
+ QVERIFY(!messages.first().filePath.isEmpty());
+
+ // matched, so the pane renders it expanded rather than as a stub. The user
+ // asked for this message by clicking it, which is as matched as it gets.
+ QVERIFY(messages.first().matched);
+}
+
+void TestNotmuchWorker::loadMessageOnAnUnknownIdReturnsNothing()
+{
+ // Empty rather than an error: a stale row after a reindex is an ordinary
+ // race, not a failure worth a message in the status bar.
+ NotmuchWorker worker(m_fixture.configPath());
+ QSignalSpy loaded(&worker, &NotmuchWorker::messageLoaded);
+ QSignalSpy errors(&worker, &NotmuchWorker::errorOccurred);
+
+ worker.loadMessage(QStringLiteral("nonexistent@example.org"), 1);
+
+ QCOMPARE(loaded.count(), 1);
+ QVERIFY(loaded.first().at(0).value<QVector<MessageRef>>().isEmpty());
+ QCOMPARE(errors.count(), 0);
+}
+
+void TestNotmuchWorker::loadThreadTreeReportsReplyDepth()
+{
+ // Thread A is a root plus one reply carrying In-Reply-To, which is what
+ // notmuch threads on. Without that header the two would be separate threads
+ // and this test would assert nothing about depth.
+ const QString threadId = threadIdOf(QStringLiteral("Release notes"));
+ QVERIFY(!threadId.isEmpty());
+
+ NotmuchWorker worker(m_fixture.configPath());
+ QSignalSpy loaded(&worker, &NotmuchWorker::threadTreeLoaded);
+ worker.loadThreadTree(threadId, QString(), 1);
+
+ QCOMPARE(loaded.count(), 1);
+ const auto nodes = loaded.first().at(0).value<QVector<MessageNode>>();
+
+ QCOMPARE(nodes.size(), 2);
+ QCOMPARE(nodes.at(0).messageId, QStringLiteral("a1@example.org"));
+ QCOMPARE(nodes.at(0).depth, 0);
+ QCOMPARE(nodes.at(1).messageId, QStringLiteral("a2@example.org"));
+ QCOMPARE(nodes.at(1).depth, 1);
+}
+
+void TestNotmuchWorker::loadThreadTreeCarriesTheFactsARowNeeds()
+{
+ // A row is drawn without opening the message, so the walk has to read the
+ // headers. loadThread does not, which is why a separate signal exists.
+ const QString threadId = threadIdOf(QStringLiteral("Release notes"));
+ QVERIFY(!threadId.isEmpty());
+
+ NotmuchWorker worker(m_fixture.configPath());
+ QSignalSpy loaded(&worker, &NotmuchWorker::threadTreeLoaded);
+ worker.loadThreadTree(threadId, QString(), 1);
+
+ QCOMPARE(loaded.count(), 1);
+ const auto nodes = loaded.first().at(0).value<QVector<MessageNode>>();
+ QCOMPARE(nodes.size(), 2);
+
+ const MessageNode &reply = nodes.at(1);
+ QVERIFY(reply.from.contains(QStringLiteral("bob@example.org")));
+ QCOMPARE(reply.subject, QStringLiteral("Re: Release notes"));
+ QVERIFY(reply.date.isValid());
+ QVERIFY(!reply.filePath.isEmpty());
+
+ // Every node names its thread, so a batch does not need the caller to keep
+ // track of which thread it asked about.
+ QCOMPARE(reply.threadId, threadId);
+ QCOMPARE(nodes.at(0).threadId, threadId);
+}
+
void TestNotmuchWorker::queryReturnsAllThreads()
{
const QVector<ThreadSummary> threads = runQuery(QStringLiteral("*"));
@@ -234,6 +329,45 @@ void TestNotmuchWorker::queryPassesGenerationThrough()
QCOMPARE(finished.first().at(1).value<quint64>(), quint64(42));
}
+void TestNotmuchWorker::oldestFirstReversesTheOrder()
+{
+ const QVector<ThreadSummary> newest = runQuery(QStringLiteral("*"));
+ const QVector<ThreadSummary> oldest =
+ runQuery(QStringLiteral("*"), NotmuchWorker::OldestFirst);
+
+ QCOMPARE(oldest.size(), newest.size());
+
+ // The guard: with fewer than two threads, or with every thread carrying
+ // the same date, a reversal is indistinguishable from no sorting at all
+ // and every assertion below would pass against a hardcoded order.
+ QVERIFY(newest.size() >= 2);
+ QVERIFY(newest.first().date != newest.last().date);
+
+ QCOMPARE(oldest.first().threadId, newest.last().threadId);
+ QCOMPARE(oldest.last().threadId, newest.first().threadId);
+}
+
+void TestNotmuchWorker::theSortOrderCrossesAQueuedCall()
+{
+ // MainWindow reaches the worker with invokeMethod(..., QueuedConnection)
+ // across a thread boundary, and a Q_ARG whose type the meta-object system
+ // does not know FAILS AT RUNTIME with a warning, not at compile time. So
+ // the enum's registration is asserted here rather than assumed from Q_ENUM.
+ QVERIFY2(QMetaType::fromName("NotmuchWorker::SortOrder").isValid(),
+ "SortOrder is not a registered metatype, so the queued runQuery "
+ "call will drop its sort argument at runtime");
+
+ NotmuchWorker worker(m_fixture.configPath());
+ QSignalSpy ready(&worker, &NotmuchWorker::threadsReady);
+
+ // The real call shape, invoked by NAME exactly as MainWindow does.
+ QVERIFY(QMetaObject::invokeMethod(
+ &worker, "runQuery", Qt::DirectConnection,
+ Q_ARG(QString, QStringLiteral("*")), Q_ARG(quint64, 1),
+ Q_ARG(NotmuchWorker::SortOrder, NotmuchWorker::OldestFirst)));
+ QCOMPARE(ready.size(), 1);
+}
+
void TestNotmuchWorker::loadThreadReturnsMessagesOldestFirst()
{
const QString threadId = threadIdOf(QStringLiteral("Release notes"));
diff --git a/tests/test_threadlistmodel.cpp b/tests/test_threadlistmodel.cpp
index f84bbab..aa71080 100644
--- a/tests/test_threadlistmodel.cpp
+++ b/tests/test_threadlistmodel.cpp
@@ -27,6 +27,20 @@ class TestThreadListModel : public QObject
{
Q_OBJECT
private slots:
+ void messageNodeHoldsDisplayFacts();
+ void rootRowsSurviveTheTreeConversion();
+ void repliesBecomeChildRowsUnderTheirThread();
+ void messageRowsShowTheirOwnSenderAndSubject();
+ void modelHasOneColumn();
+ void aFlatThreadStillListsItsReplies();
+ void theRootCardKnowsItsOwnMessage();
+ void replyShowsOnlyItsOwnTags();
+ void replySharingEveryThreadTagShowsNone();
+ void reloadingAThreadReplacesItsRepliesRatherThanRepeatingThem();
+ void anUnexpandedMultiMessageThreadOffersAnExpander();
+ void scopeFollowsTheSelectedRowKind();
+ void scopeCountsEveryMessageOfAnUnexpandedThread();
+ void scopeHonoursAMixedSelectionWithoutEscalating();
void startsEmpty();
void accountKeysComeFromTheAccountTags();
void accountKeysCoverAThreadSpanningTwoAccounts();
@@ -35,22 +49,20 @@ private slots:
void appendingEmptyBatchIsNoOp();
void clearResetsModel();
void reportsSubjectAndAuthors();
- void subjectShowsMessageCountOnlyForRealThreads();
+ void theReplyCountExcludesTheRootMessage();
void unreadThreadsRenderBold();
void readThreadsAreDimmedAndUnreadAreNot();
void flaggedThreadsShowAStar();
void pillTagsExcludeWhatTheRowAlreadyShows();
- void theStarColumnIsNarrowAndCarriesNoText();
void theUnreadCueDoesNotDependOnFontWeight();
void aDoomedThreadKeepsItsContrastEvenWhenRead();
- void tagsAreTheFirstColumnAndSubjectTheLast();
void accountTagBecomesAChipLabel();
void unreadStylingSurvivesAnAccountChip();
void accountChipUsesTheConfiguredColour();
void deletedThreadsAreRedAndStruckThrough();
- void attachmentColumnIsFirstAndMarksOnlyTaggedThreads();
+ void attachmentIsMarkedOnlyOnTaggedThreads();
void spamThreadsAreOrangeAndStruckThrough();
- void doomedStylingCoversEveryColumn();
+ void doomedStylingCoversTheWholeCard();
void ordinaryThreadsCarryNoRowColour();
void threadIdIsReachableFromAnIndex();
void invalidIndexesReturnNothing();
@@ -76,6 +88,238 @@ static ThreadSummary makeThread(const QString &id, const QString &subject)
return t;
}
+static MessageNode makeNode(const QString &id, int depth,
+ const QString &from = QStringLiteral("Alice"),
+ const QString &subject = QStringLiteral("Re: Hi"))
+{
+ MessageNode n;
+ n.messageId = id;
+ n.threadId = QStringLiteral("t1");
+ n.from = from;
+ n.subject = subject;
+ n.date = QDateTime::fromSecsSinceEpoch(1750000000);
+ n.depth = depth;
+ return n;
+}
+
+void TestThreadListModel::repliesBecomeChildRowsUnderTheirThread()
+{
+ ThreadListModel model;
+ model.appendBatch({ makeThread(QStringLiteral("t1"),
+ QStringLiteral("A subject")) });
+
+ // Depth 0 is the thread's FIRST message and belongs on the root row, not in
+ // the children: the user's model is "N replies", so a thread of three shows
+ // one root and two children.
+ model.setThreadMessages(QStringLiteral("t1"),
+ { makeNode(QStringLiteral("m0@example.org"), 0),
+ makeNode(QStringLiteral("m1@example.org"), 1),
+ makeNode(QStringLiteral("m2@example.org"), 2) });
+
+ const QModelIndex root = model.index(0, 0, QModelIndex());
+ QCOMPARE(model.rowCount(root), 2);
+
+ const QModelIndex child =
+ model.index(0, 0, root);
+ QVERIFY(child.isValid());
+ QCOMPARE(model.parent(child), model.index(0, 0, QModelIndex()));
+
+ QVERIFY(model.data(child, ThreadListModel::IsMessageRole).toBool());
+ QCOMPARE(model.data(child, ThreadListModel::MessageIdRole).toString(),
+ QStringLiteral("m1@example.org"));
+
+ // A message row still belongs to a thread, so a caller that only needs the
+ // containing thread does not have to walk up itself.
+ QCOMPARE(model.data(child, ThreadListModel::ThreadIdRole).toString(),
+ QStringLiteral("t1"));
+
+ // A thread root is not a message ROW, but it does carry a message id: the
+ // root card is the thread's first message, and selecting it renders that
+ // message alone. It used to answer nothing here, which is what made the
+ // first message of every thread unreachable.
+ QVERIFY(!model.data(root, ThreadListModel::IsMessageRole).toBool());
+ QCOMPARE(model.data(root, ThreadListModel::MessageIdRole).toString(),
+ QStringLiteral("m0@example.org"));
+
+ QAbstractItemModelTester tester(
+ &model, QAbstractItemModelTester::FailureReportingMode::Warning);
+ Q_UNUSED(tester);
+}
+
+void TestThreadListModel::messageRowsShowTheirOwnSenderAndSubject()
+{
+ // A reply's row shows the REPLY's sender, not the thread's author summary.
+ // Reading the thread's fields for a child row is the obvious mistake and
+ // would look almost right, since the first sender is usually in both.
+ ThreadListModel model;
+ model.appendBatch({ makeThread(QStringLiteral("t1"),
+ QStringLiteral("A subject")) });
+ model.setThreadMessages(
+ QStringLiteral("t1"),
+ { makeNode(QStringLiteral("m0@example.org"), 0),
+ makeNode(QStringLiteral("m1@example.org"), 1,
+ QStringLiteral("Bob <bob@example.org>"),
+ QStringLiteral("Re: A subject")) });
+
+ const QModelIndex root = model.index(0, 0, QModelIndex());
+ const QModelIndex reply = model.index(0, 0, root);
+
+ QCOMPARE(model.data(reply, ThreadListModel::SendersRole).toString(),
+ QStringLiteral("Bob <bob@example.org>"));
+ QCOMPARE(model.data(reply, ThreadListModel::SubjectRole).toString(),
+ QStringLiteral("Re: A subject"));
+
+ // No tag strip under a child row. The strip is a row-wide band carrying the
+ // THREAD's tags; one under every reply would stripe the list and repeat the
+ // same tags down the whole expansion.
+ QVERIFY(model.data(reply, ThreadListModel::PillTagsRole)
+ .toStringList().isEmpty());
+}
+
+void TestThreadListModel::reloadingAThreadReplacesItsRepliesRatherThanRepeatingThem()
+{
+ // A thread reloaded after a sync must not end up listing its replies twice.
+ ThreadListModel model;
+ model.appendBatch({ makeThread(QStringLiteral("t1"),
+ QStringLiteral("A subject")) });
+
+ const QVector<MessageNode> nodes{
+ makeNode(QStringLiteral("m0@example.org"), 0),
+ makeNode(QStringLiteral("m1@example.org"), 1)
+ };
+
+ model.setThreadMessages(QStringLiteral("t1"), nodes);
+ const QModelIndex root = model.index(0, 0, QModelIndex());
+ QCOMPARE(model.rowCount(root), 1);
+
+ model.setThreadMessages(QStringLiteral("t1"), nodes);
+ QCOMPARE(model.rowCount(root), 1);
+
+ QAbstractItemModelTester tester(
+ &model, QAbstractItemModelTester::FailureReportingMode::Warning);
+ Q_UNUSED(tester);
+}
+
+void TestThreadListModel::anUnexpandedMultiMessageThreadOffersAnExpander()
+{
+ // This is what makes lazy loading work at all. rowCount is 0 until the
+ // worker has walked the thread, so a view inferring the expander from
+ // rowCount alone draws none, the user can never expand, and the replies are
+ // never requested. hasChildren answers from the summary's count instead.
+ ThreadListModel model;
+ ThreadSummary many = makeThread(QStringLiteral("t1"),
+ QStringLiteral("Has replies"));
+ many.totalCount = 4;
+ ThreadSummary lone = makeThread(QStringLiteral("t2"),
+ QStringLiteral("Single message"));
+ lone.totalCount = 1;
+ model.appendBatch({ many, lone });
+
+ const QModelIndex withReplies = model.index(0, 0, QModelIndex());
+ const QModelIndex single = model.index(1, 0, QModelIndex());
+
+ // Guard: neither is expanded, so this really is the unloaded case.
+ QCOMPARE(model.rowCount(withReplies), 0);
+ QCOMPARE(model.rowCount(single), 0);
+
+ QVERIFY(model.hasChildren(withReplies));
+ QVERIFY(!model.hasChildren(single));
+
+ // Once loaded the children are the truth, including "there are none": a
+ // thread whose count included duplicates must stop offering an expander
+ // that opens onto nothing.
+ model.setThreadMessages(QStringLiteral("t1"),
+ { makeNode(QStringLiteral("m0@example.org"), 0) });
+ QVERIFY(!model.hasChildren(withReplies));
+
+ // A message row is always a leaf.
+ model.setThreadMessages(QStringLiteral("t1"),
+ { makeNode(QStringLiteral("m0@example.org"), 0),
+ makeNode(QStringLiteral("m1@example.org"), 1) });
+ QVERIFY(model.hasChildren(withReplies));
+ QVERIFY(!model.hasChildren(model.index(0, 0, withReplies)));
+}
+
+void TestThreadListModel::scopeFollowsTheSelectedRowKind()
+{
+ ThreadListModel model;
+ ThreadSummary t = makeThread(QStringLiteral("t1"),
+ QStringLiteral("A subject"));
+ t.totalCount = 3;
+ model.appendBatch({ t });
+ model.setThreadMessages(QStringLiteral("t1"),
+ { makeNode(QStringLiteral("m0@example.org"), 0),
+ makeNode(QStringLiteral("m1@example.org"), 1) });
+
+ const QModelIndex root = model.index(0, 0, QModelIndex());
+ const QModelIndex child = model.index(0, 0, root);
+
+ // A thread root acts on the whole thread, and reports every message it
+ // stands for so the status bar can say so.
+ const ActionScope threadScope = model.scopeFor({ root });
+ QCOMPARE(threadScope.threadIds, QStringList{ QStringLiteral("t1") });
+ QVERIFY(threadScope.messageIds.isEmpty());
+ QCOMPARE(threadScope.messageCount, 3);
+ QVERIFY(threadScope.wholeThread);
+
+ // A message row acts on that message alone.
+ const ActionScope messageScope = model.scopeFor({ child });
+ QVERIFY(messageScope.threadIds.isEmpty());
+ QCOMPARE(messageScope.messageIds,
+ QStringList{ QStringLiteral("m1@example.org") });
+ QCOMPARE(messageScope.messageCount, 1);
+ QVERIFY(!messageScope.wholeThread);
+}
+
+void TestThreadListModel::scopeCountsEveryMessageOfAnUnexpandedThread()
+{
+ // totalCount, not the loaded children. A thread that was never expanded
+ // still has all of its messages, and counting only what happens to be on
+ // screen would understate what the action is about to do.
+ ThreadListModel model;
+ ThreadSummary t = makeThread(QStringLiteral("t1"),
+ QStringLiteral("A subject"));
+ t.totalCount = 7;
+ model.appendBatch({ t });
+
+ const QModelIndex root = model.index(0, 0, QModelIndex());
+ QCOMPARE(model.rowCount(root), 0); // guard: nothing expanded
+
+ const ActionScope scope = model.scopeFor({ root });
+ QCOMPARE(scope.messageCount, 7);
+}
+
+void TestThreadListModel::scopeHonoursAMixedSelectionWithoutEscalating()
+{
+ // Selecting a thread root and an unrelated reply acts on that whole thread
+ // AND that one message. Nothing is escalated to thread scope or narrowed to
+ // message scope silently, which is the point of the scope being visible.
+ ThreadListModel model;
+ ThreadSummary t1 = makeThread(QStringLiteral("t1"), QStringLiteral("One"));
+ t1.totalCount = 2;
+ ThreadSummary t2 = makeThread(QStringLiteral("t2"), QStringLiteral("Two"));
+ t2.totalCount = 5;
+ model.appendBatch({ t1, t2 });
+
+ MessageNode reply = makeNode(QStringLiteral("m1@example.org"), 1);
+ reply.threadId = QStringLiteral("t2");
+ model.setThreadMessages(QStringLiteral("t2"),
+ { makeNode(QStringLiteral("m0@example.org"), 0),
+ reply });
+
+ const QModelIndex firstRoot = model.index(0, 0, QModelIndex());
+ const QModelIndex secondRoot = model.index(1, 0, QModelIndex());
+ const QModelIndex reply1 = model.index(0, 0, secondRoot);
+
+ const ActionScope scope = model.scopeFor({ firstRoot, reply1 });
+ QCOMPARE(scope.threadIds, QStringList{ QStringLiteral("t1") });
+ QCOMPARE(scope.messageIds, QStringList{ QStringLiteral("m1@example.org") });
+
+ // 2 from the whole thread plus 1 for the lone message.
+ QCOMPARE(scope.messageCount, 3);
+ QVERIFY(scope.wholeThread);
+}
+
void TestThreadListModel::accountKeysComeFromTheAccountTags()
{
// Item 49 reads this to decide which mbsync channels a sync needs. Only
@@ -116,11 +360,69 @@ void TestThreadListModel::accountKeysAreEmptyForAnUnknownThread()
QVERIFY(model.accountKeysForThread(QStringLiteral("nope")).isEmpty());
}
+void TestThreadListModel::messageNodeHoldsDisplayFacts()
+{
+ // A message ROW has to be drawn without opening the message, so the display
+ // facts live on the node itself. MessageRef, which exists for rendering a
+ // thread into the pane, carries none of them.
+ MessageNode node;
+ node.messageId = QStringLiteral("id@example.org");
+ node.from = QStringLiteral("A Sender <sender@example.org>");
+ node.subject = QStringLiteral("Re: a subject");
+ node.date = QDateTime::fromSecsSinceEpoch(1000);
+ node.depth = 2;
+ node.tags = QStringList{ QStringLiteral("unread") };
+
+ QCOMPARE(node.depth, 2);
+ QVERIFY(node.isUnread());
+ QCOMPARE(node.from, QStringLiteral("A Sender <sender@example.org>"));
+ QCOMPARE(node.subject, QStringLiteral("Re: a subject"));
+
+ // Depth 0 is the thread's first message, which the ROOT row stands for.
+ // Defaulting to 0 rather than 1 keeps "is this the root" a plain check.
+ const MessageNode fresh;
+ QCOMPARE(fresh.depth, 0);
+ QVERIFY(!fresh.isUnread());
+}
+
+void TestThreadListModel::rootRowsSurviveTheTreeConversion()
+{
+ // The point of this test is NOT the tree. It is that converting the base
+ // class from QAbstractTableModel changed nothing a thread row does: a table
+ // answers index() and parent() too, just trivially, and every existing test
+ // in this file is the real regression net beside it.
+ ThreadListModel model;
+ model.appendBatch({ makeThread(QStringLiteral("t1"),
+ QStringLiteral("A subject")) });
+
+ // A tree model reports its roots under an INVALID parent.
+ QCOMPARE(model.rowCount(QModelIndex()), 1);
+ QCOMPARE(model.columnCount(QModelIndex()), 1);
+
+ const QModelIndex root =
+ model.index(0, 0, QModelIndex());
+ QVERIFY(root.isValid());
+ QVERIFY(!model.parent(root).isValid());
+ QCOMPARE(model.data(root, ThreadListModel::ThreadIdRole).toString(),
+ QStringLiteral("t1"));
+
+ // No children until a thread's messages are asked for. An expander drawn
+ // over a thread whose replies were never loaded would open onto nothing.
+ QCOMPARE(model.rowCount(root), 0);
+
+ // Qt's own conformance check. It walks index/parent/rowCount for
+ // consistency and catches the classic tree-model faults, such as a parent()
+ // that does not round-trip, which a hand-written assertion misses.
+ QAbstractItemModelTester tester(
+ &model, QAbstractItemModelTester::FailureReportingMode::Warning);
+ Q_UNUSED(tester);
+}
+
void TestThreadListModel::startsEmpty()
{
ThreadListModel model;
QCOMPARE(model.rowCount(), 0);
- QCOMPARE(model.columnCount(), ThreadListModel::ColumnCount);
+ QCOMPARE(model.columnCount(), 1);
}
void TestThreadListModel::appendsBatches()
@@ -165,21 +467,21 @@ void TestThreadListModel::reportsSubjectAndAuthors()
ThreadListModel model;
model.appendBatch({ makeThread(QStringLiteral("t1"), QStringLiteral("hello")) });
- const QModelIndex authors = model.index(0, ThreadListModel::AuthorsColumn);
- QCOMPARE(model.data(authors, Qt::DisplayRole).toString(),
+ // One index, every field, by role. The card draws them all at once, so
+ // reading them through Qt::DisplayRole as five columns did is no longer
+ // possible: DisplayRole answers the subject alone.
+ const QModelIndex card = model.index(0, 0);
+ QCOMPARE(model.data(card, ThreadListModel::SendersRole).toString(),
QStringLiteral("Alice"));
+ QVERIFY(model.data(card, ThreadListModel::DateRole).toDateTime().isValid());
+ QCOMPARE(model.data(card, ThreadListModel::SubjectRole).toString(),
+ QStringLiteral("hello"));
- const QModelIndex date = model.index(0, ThreadListModel::DateColumn);
- QVERIFY(!model.data(date, Qt::DisplayRole).toString().isEmpty());
-
- // Tags are no longer a column; they reach the strip under the message
- // pane through a role instead.
- const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn);
- QCOMPARE(model.data(subject, ThreadListModel::TagsRole).toStringList(),
+ QCOMPARE(model.data(card, ThreadListModel::TagsRole).toStringList(),
QStringList({ QStringLiteral("inbox"), QStringLiteral("unread") }));
}
-void TestThreadListModel::subjectShowsMessageCountOnlyForRealThreads()
+void TestThreadListModel::theReplyCountExcludesTheRootMessage()
{
ThreadListModel model;
@@ -189,12 +491,18 @@ void TestThreadListModel::subjectShowsMessageCountOnlyForRealThreads()
multi.totalCount = 4;
model.appendBatch({ single, multi });
- QCOMPARE(model.data(model.index(0, ThreadListModel::SubjectColumn),
- Qt::DisplayRole).toString(),
- QStringLiteral("alone"));
- QCOMPARE(model.data(model.index(1, ThreadListModel::SubjectColumn),
- Qt::DisplayRole).toString(),
- QStringLiteral("group (4)"));
+ // The count used to be a "(4)" suffix on the subject. It is the expander
+ // on the card's second line now, and it counts REPLIES: totalCount
+ // includes the root message, which is the card itself.
+ QCOMPARE(model.data(model.index(0, 0),
+ ThreadListModel::ReplyCountRole).toInt(), 0);
+ QCOMPARE(model.data(model.index(1, 0),
+ ThreadListModel::ReplyCountRole).toInt(), 3);
+
+ // And the subject is bare, with no count spliced into it.
+ QCOMPARE(model.data(model.index(1, 0),
+ ThreadListModel::SubjectRole).toString(),
+ QStringLiteral("group"));
}
void TestThreadListModel::unreadThreadsRenderBold()
@@ -205,11 +513,11 @@ void TestThreadListModel::unreadThreadsRenderBold()
model.appendBatch({ read, makeThread(QStringLiteral("t2"), QStringLiteral("unread")) });
const QVariant readFont =
- model.data(model.index(0, ThreadListModel::SubjectColumn), Qt::FontRole);
+ model.data(model.index(0, 0), Qt::FontRole);
QVERIFY(!readFont.isValid());
const QVariant unreadFont =
- model.data(model.index(1, ThreadListModel::SubjectColumn), Qt::FontRole);
+ model.data(model.index(1, 0), Qt::FontRole);
QVERIFY(unreadFont.isValid());
QVERIFY(unreadFont.value<QFont>().bold());
}
@@ -232,10 +540,10 @@ void TestThreadListModel::readThreadsAreDimmedAndUnreadAreNot()
{ read, makeThread(QStringLiteral("t2"), QStringLiteral("unread")) });
const QVariant readFg =
- model.data(model.index(0, ThreadListModel::SubjectColumn),
+ model.data(model.index(0, 0),
Qt::ForegroundRole);
const QVariant unreadFg =
- model.data(model.index(1, ThreadListModel::SubjectColumn),
+ model.data(model.index(1, 0),
Qt::ForegroundRole);
QVERIFY2(readFg.isValid(), "a read thread carries no dimming");
@@ -257,16 +565,17 @@ void TestThreadListModel::flaggedThreadsShowAStar()
QStringLiteral("flagged") };
model.appendBatch({ plain, starred });
- const QString none =
- model.data(model.index(0, ThreadListModel::FlagColumn),
- Qt::DisplayRole).toString();
- const QString star =
- model.data(model.index(1, ThreadListModel::FlagColumn),
- Qt::DisplayRole).toString();
-
- QVERIFY2(none.isEmpty(), "an unflagged thread shows something in the column");
- QVERIFY2(!star.isEmpty(), "a flagged thread shows nothing");
- QCOMPARE(star, ThreadListModel::flagGlyph());
+ QVERIFY2(!model.data(model.index(0, 0),
+ ThreadListModel::IsFlaggedRole).toBool(),
+ "an unflagged thread reports itself flagged");
+ QVERIFY2(model.data(model.index(1, 0),
+ ThreadListModel::IsFlaggedRole).toBool(),
+ "a flagged thread does not report itself flagged");
+
+ // The glyph the delegate draws from that flag must be something a font can
+ // render: an unrenderable codepoint shows as tofu, which reads as
+ // breakage rather than as a mark.
+ QVERIFY(!ThreadListModel::flagGlyph().isEmpty());
}
void TestThreadListModel::pillTagsExcludeWhatTheRowAlreadyShows()
@@ -290,7 +599,7 @@ void TestThreadListModel::pillTagsExcludeWhatTheRowAlreadyShows()
model.appendBatch({ thread });
const QStringList pills =
- model.data(model.index(0, ThreadListModel::SubjectColumn),
+ model.data(model.index(0, 0),
ThreadListModel::PillTagsRole).toStringList();
QVERIFY2(pills.contains(QStringLiteral("SBo")), qPrintable(pills.join(',')));
@@ -320,29 +629,6 @@ void TestThreadListModel::pillTagsExcludeWhatTheRowAlreadyShows()
QCOMPARE(pills, sorted);
}
-void TestThreadListModel::theStarColumnIsNarrowAndCarriesNoText()
-{
- // A marker column, like the paperclip beside it: centred, and never
- // carrying the subject or anything else that would want width.
- ThreadListModel model;
- ThreadSummary starred = makeThread(QStringLiteral("t1"),
- QStringLiteral("starred"));
- starred.tags = QStringList{ QStringLiteral("flagged") };
- model.appendBatch({ starred });
-
- const QModelIndex index = model.index(0, ThreadListModel::FlagColumn);
- QCOMPARE(model.data(index, Qt::TextAlignmentRole).toInt(),
- int(Qt::AlignCenter));
-
- // The glyph is one character, whether it is the star or its fallback: a
- // column sized for a marker cannot hold a word.
- QCOMPARE(ThreadListModel::flagGlyph().size(), 1);
-
- // And it says what it means, for anyone who cannot tell the glyph apart
- // from the paperclip beside it.
- QVERIFY(!model.data(index, Qt::ToolTipRole).toString().isEmpty());
-}
-
void TestThreadListModel::theUnreadCueDoesNotDependOnFontWeight()
{
// The property that matters, stated directly: strip every font from the
@@ -355,17 +641,14 @@ void TestThreadListModel::theUnreadCueDoesNotDependOnFontWeight()
model.appendBatch(
{ read, makeThread(QStringLiteral("t2"), QStringLiteral("unread")) });
- for (int column = 0; column < ThreadListModel::ColumnCount; ++column) {
- const QVariant readFg =
- model.data(model.index(0, column), Qt::ForegroundRole);
- const QVariant unreadFg =
- model.data(model.index(1, column), Qt::ForegroundRole);
+ const QVariant readFg =
+ model.data(model.index(0, 0), Qt::ForegroundRole);
+ const QVariant unreadFg =
+ model.data(model.index(1, 0), Qt::ForegroundRole);
- QVERIFY2(readFg != unreadFg,
- qPrintable(QStringLiteral("column %1 renders read and unread "
- "identically once the font is "
- "ignored").arg(column)));
- }
+ QVERIFY2(readFg != unreadFg,
+ "read and unread cards render identically once the font is "
+ "ignored");
}
void TestThreadListModel::aDoomedThreadKeepsItsContrastEvenWhenRead()
@@ -382,32 +665,11 @@ void TestThreadListModel::aDoomedThreadKeepsItsContrastEvenWhenRead()
model.applyTagChange(QStringLiteral("t1"), { QStringLiteral("deleted") }, {});
- const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn);
+ const QModelIndex subject = model.index(0, 0);
QCOMPARE(model.data(subject, Qt::ForegroundRole).value<QBrush>().color(),
QColor(Qt::white));
}
-void TestThreadListModel::tagsAreTheFirstColumnAndSubjectTheLast()
-{
- // Subject stretches to fill the view, so whatever sits after it is pushed
- // off-screen. Tags used to be there, which is why acting on a thread
- // looked like it did nothing: the only column that changed was invisible.
- QCOMPARE(ThreadListModel::SubjectColumn, ThreadListModel::ColumnCount - 1);
-
- ThreadListModel model;
- model.appendBatch({ makeThread(QStringLiteral("t1"), QStringLiteral("hello")) });
- QCOMPARE(model.headerData(ThreadListModel::SubjectColumn, Qt::Horizontal,
- Qt::DisplayRole).toString(),
- QStringLiteral("Subject"));
-
- // No tags column at all: spelling out a dozen tags per row consumed most
- // of the list's width and was unreadable.
- for (int column = 0; column < ThreadListModel::ColumnCount; ++column) {
- QVERIFY(model.headerData(column, Qt::Horizontal, Qt::DisplayRole)
- .toString() != QStringLiteral("Tags"));
- }
-}
-
void TestThreadListModel::accountTagBecomesAChipLabel()
{
// The account tag is a different taxonomy from a functional one: which
@@ -419,7 +681,7 @@ void TestThreadListModel::accountTagBecomesAChipLabel()
QStringLiteral("account-webmail-personal") };
model.appendBatch({ thread });
- const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn);
+ const QModelIndex subject = model.index(0, 0);
QCOMPARE(model.data(subject, ThreadListModel::AccountLabelRole).toString(),
QStringLiteral("webmail-personal"));
QVERIFY(model.data(subject, ThreadListModel::AccountColourRole)
@@ -430,7 +692,7 @@ void TestThreadListModel::accountTagBecomesAChipLabel()
ThreadSummary untagged = makeThread(QStringLiteral("t2"), QStringLiteral("hi"));
untagged.tags = QStringList{ QStringLiteral("inbox") };
plain.appendBatch({ untagged });
- QVERIFY(plain.data(plain.index(0, ThreadListModel::SubjectColumn),
+ QVERIFY(plain.data(plain.index(0, 0),
ThreadListModel::AccountLabelRole).toString().isEmpty());
}
@@ -446,7 +708,7 @@ void TestThreadListModel::unreadStylingSurvivesAnAccountChip()
QStringLiteral("account-webmail-personal") };
model.appendBatch({ thread });
- const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn);
+ const QModelIndex subject = model.index(0, 0);
QVERIFY(!model.data(subject, ThreadListModel::AccountLabelRole)
.toString().isEmpty());
@@ -469,7 +731,7 @@ void TestThreadListModel::accountChipUsesTheConfiguredColour()
thread.tags = QStringList{ QStringLiteral("account-webmail-personal") };
model.appendBatch({ thread });
- QCOMPARE(model.data(model.index(0, ThreadListModel::SubjectColumn),
+ QCOMPARE(model.data(model.index(0, 0),
ThreadListModel::AccountColourRole).value<QColor>(),
QColor(QStringLiteral("#cc0000")));
}
@@ -481,7 +743,7 @@ void TestThreadListModel::deletedThreadsAreRedAndStruckThrough()
thread.tags = QStringList{ QStringLiteral("inbox") };
model.appendBatch({ thread });
- const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn);
+ const QModelIndex subject = model.index(0, 0);
QVERIFY(!model.data(subject, Qt::BackgroundRole).isValid());
model.applyTagChange(QStringLiteral("t1"), { QStringLiteral("deleted") }, {});
@@ -506,7 +768,7 @@ void TestThreadListModel::spamThreadsAreOrangeAndStruckThrough()
model.applyTagChange(QStringLiteral("t1"), { QStringLiteral("spam") }, {});
- const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn);
+ const QModelIndex subject = model.index(0, 0);
QCOMPARE(model.data(subject, Qt::BackgroundRole).value<QBrush>().color(),
ThreadListModel::spamColour());
QVERIFY(model.data(subject, Qt::FontRole).value<QFont>().strikeOut());
@@ -515,10 +777,12 @@ void TestThreadListModel::spamThreadsAreOrangeAndStruckThrough()
QVERIFY(ThreadListModel::spamColour() != ThreadListModel::deletedColour());
}
-void TestThreadListModel::doomedStylingCoversEveryColumn()
+void TestThreadListModel::doomedStylingCoversTheWholeCard()
{
- // A cue on one column would vanish the moment that column scrolled out of
- // view, which is the bug this whole change exists to fix.
+ // The cue is on the card itself. It used to be asserted per column,
+ // because a cue on one column vanished the moment that column scrolled out
+ // of view; one column cannot scroll away, but the roles still have to be
+ // answered or a deleted card looks untouched.
ThreadListModel model;
ThreadSummary thread = makeThread(QStringLiteral("t1"), QStringLiteral("doomed"));
thread.tags = QStringList{ QStringLiteral("inbox") };
@@ -526,13 +790,11 @@ void TestThreadListModel::doomedStylingCoversEveryColumn()
model.applyTagChange(QStringLiteral("t1"), { QStringLiteral("deleted") }, {});
- for (int column = 0; column < ThreadListModel::ColumnCount; ++column) {
- const QModelIndex index = model.index(0, column);
- QVERIFY2(model.data(index, Qt::BackgroundRole).isValid(),
- qPrintable(QStringLiteral("column %1 has no background").arg(column)));
- QVERIFY2(model.data(index, Qt::FontRole).value<QFont>().strikeOut(),
- qPrintable(QStringLiteral("column %1 is not struck through").arg(column)));
- }
+ const QModelIndex index = model.index(0, 0);
+ QVERIFY2(model.data(index, Qt::BackgroundRole).isValid(),
+ "a deleted card has no background");
+ QVERIFY2(model.data(index, Qt::FontRole).value<QFont>().strikeOut(),
+ "a deleted card is not struck through");
}
void TestThreadListModel::ordinaryThreadsCarryNoRowColour()
@@ -546,7 +808,7 @@ void TestThreadListModel::ordinaryThreadsCarryNoRowColour()
model.applyTagChange(QStringLiteral("t1"), { QStringLiteral("deleted") }, {});
model.applyTagChange(QStringLiteral("t1"), {}, { QStringLiteral("deleted") });
- const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn);
+ const QModelIndex subject = model.index(0, 0);
QVERIFY(!model.data(subject, Qt::BackgroundRole).isValid());
const QVariant font = model.data(subject, Qt::FontRole);
QVERIFY(!font.isValid() || !font.value<QFont>().strikeOut());
@@ -569,7 +831,7 @@ void TestThreadListModel::threadIdIsReachableFromAnIndex()
model.appendBatch({ makeThread(QStringLiteral("t1"), QStringLiteral("one")),
makeThread(QStringLiteral("t2"), QStringLiteral("two")) });
- const QModelIndex index = model.index(1, ThreadListModel::SubjectColumn);
+ const QModelIndex index = model.index(1, 0);
QCOMPARE(model.data(index, ThreadListModel::ThreadIdRole).toString(),
QStringLiteral("t2"));
}
@@ -587,7 +849,7 @@ void TestThreadListModel::invalidIndexesReturnNothing()
// the reset in clear() before data() ever sees it. data() still checks its
// own bounds, but that guard is unreachable defence, not something these
// assertions can falsify.
- QVERIFY(!model.index(0, ThreadListModel::ColumnCount).isValid());
+ QVERIFY(!model.index(0, 1).isValid());
QVERIFY(!model.index(5, 0).isValid());
QVERIFY(!model.index(-1, 0).isValid());
@@ -651,9 +913,9 @@ void TestThreadListModel::tagChangeSignalsExactlyTheChangedRow()
const QModelIndex bottomRight = changed.first().at(1).value<QModelIndex>();
QCOMPARE(topLeft.row(), 1);
QCOMPARE(bottomRight.row(), 1);
+ // One column, so the range is a single index: the card repaints whole.
QCOMPARE(topLeft.column(), 0);
- // The whole row repaints: unread state changes the font of every column.
- QCOMPARE(bottomRight.column(), ThreadListModel::ColumnCount - 1);
+ QCOMPARE(bottomRight.column(), 0);
}
void TestThreadListModel::tagChangeForUnknownThreadIsIgnored()
@@ -704,12 +966,11 @@ void TestThreadListModel::modelPassesQtTester()
model.clear();
}
-void TestThreadListModel::attachmentColumnIsFirstAndMarksOnlyTaggedThreads()
+void TestThreadListModel::attachmentIsMarkedOnlyOnTaggedThreads()
{
- // Leftmost, and narrow: the point is to see an attachment without opening
- // the thread, which only works if the column is never scrolled away.
- QCOMPARE(ThreadListModel::AttachmentColumn, 0);
-
+ // The mark is drawn on the card's second line by CardDelegate. What the
+ // model owes it is the flag and the glyph, which is what this asserts:
+ // the column that used to carry it is gone.
ThreadSummary plain = makeThread(QStringLiteral("t1"),
QStringLiteral("no attachment"));
ThreadSummary withFile = makeThread(QStringLiteral("t2"),
@@ -722,13 +983,12 @@ void TestThreadListModel::attachmentColumnIsFirstAndMarksOnlyTaggedThreads()
model.appendBatch({ plain, withFile });
const QModelIndex plainCell =
- model.index(0, ThreadListModel::AttachmentColumn);
+ model.index(0, 0);
const QModelIndex fileCell =
- model.index(1, ThreadListModel::AttachmentColumn);
+ model.index(1, 0);
- QVERIFY(model.data(plainCell, Qt::DisplayRole).toString().isEmpty());
- QCOMPARE(model.data(fileCell, Qt::DisplayRole).toString(),
- ThreadListModel::attachmentGlyph());
+ QVERIFY(!model.data(plainCell, ThreadListModel::HasAttachmentRole).toBool());
+ QVERIFY(model.data(fileCell, ThreadListModel::HasAttachmentRole).toBool());
// The glyph must be something a font can draw. An unrenderable codepoint
// shows as a tofu box, which reads as breakage rather than as a marker.
@@ -738,11 +998,178 @@ void TestThreadListModel::attachmentColumnIsFirstAndMarksOnlyTaggedThreads()
// have an attachment on hover.
QVERIFY(model.data(plainCell, Qt::ToolTipRole).toString().isEmpty());
QVERIFY(!model.data(fileCell, Qt::ToolTipRole).toString().isEmpty());
+}
+
+void TestThreadListModel::modelHasOneColumn()
+{
+ ThreadListModel model;
+ ThreadSummary thread;
+ thread.threadId = QStringLiteral("T1");
+ thread.subject = QStringLiteral("Build fails");
+ thread.authors = QStringLiteral("alice@example.org");
+ thread.date = QDateTime::currentDateTime();
+ thread.totalCount = 1;
+ model.appendBatch({ thread });
+
+ QCOMPARE(model.columnCount(), 1);
+
+ // Every field the five columns used to answer is still reachable, by role
+ // rather than by column, because the card draws them all.
+ const QModelIndex index = model.index(0, 0);
+ QCOMPARE(index.data(ThreadListModel::SubjectRole).toString(),
+ QStringLiteral("Build fails"));
+ QCOMPARE(index.data(ThreadListModel::SendersRole).toString(),
+ QStringLiteral("alice@example.org"));
+ QVERIFY(index.data(ThreadListModel::DateRole).toDateTime().isValid());
+
+ // A single-message thread offers no expander: totalCount includes the root
+ // message, which is the card itself.
+ QCOMPARE(index.data(ThreadListModel::ReplyCountRole).toInt(), 0);
+}
+
+void TestThreadListModel::aFlatThreadStillListsItsReplies()
+{
+ // A thread whose messages carry no reply structure: notmuch returns them
+ // all from get_toplevel_messages at depth 0, which is what happens when the
+ // mail has no usable In-Reply-To. Measured in the user's own database:
+ // of 396 inbox threads, three are like this, one of them nine messages
+ // deep, and every one of them showed a reply count that expanded to
+ // nothing because the model kept only nodes with depth > 0.
+ ThreadListModel model;
+ ThreadSummary thread = makeThread(QStringLiteral("t1"),
+ QStringLiteral("flat thread"));
+ thread.totalCount = 3;
+ model.appendBatch({ thread });
+
+ model.setThreadMessages(QStringLiteral("t1"),
+ { makeNode(QStringLiteral("m0@example.org"), 0),
+ makeNode(QStringLiteral("m1@example.org"), 0),
+ makeNode(QStringLiteral("m2@example.org"), 0) });
+
+ const QModelIndex root = model.index(0, 0);
+
+ // Two children, not zero: the FIRST message is the root card itself, and
+ // the rest are its replies however flat the thread is.
+ QCOMPARE(model.rowCount(root), 2);
+ QCOMPARE(model.index(0, 0, root).data(ThreadListModel::MessageIdRole)
+ .toString(),
+ QStringLiteral("m1@example.org"));
+
+ // And the count the card advertises must agree with the rows beneath it,
+ // or the expander opens onto nothing.
+ QCOMPARE(root.data(ThreadListModel::ReplyCountRole).toInt(),
+ model.rowCount(root));
+}
+
+void TestThreadListModel::theRootCardKnowsItsOwnMessage()
+{
+ // The root card IS the thread's first message, so it has to be able to say
+ // which message that is. Without this the pane renders the whole thread
+ // when the root is selected, and the first message is unreachable: the
+ // only rows offering it are the replies, and it is not one of them.
+ ThreadListModel model;
+ ThreadSummary thread = makeThread(QStringLiteral("t1"),
+ QStringLiteral("a subject"));
+ thread.totalCount = 2;
+ model.appendBatch({ thread });
+
+ const QModelIndex root = model.index(0, 0);
+
+ // Before the replies are loaded there is nothing to report, and the caller
+ // must fall back to loading the whole thread rather than a wrong message.
+ QVERIFY(root.data(ThreadListModel::MessageIdRole).toString().isEmpty());
+
+ model.setThreadMessages(QStringLiteral("t1"),
+ { makeNode(QStringLiteral("m0@example.org"), 0),
+ makeNode(QStringLiteral("m1@example.org"), 1) });
+
+ QCOMPARE(root.data(ThreadListModel::MessageIdRole).toString(),
+ QStringLiteral("m0@example.org"));
+
+ // And it is the FIRST message, not just any of them: the reply must still
+ // report its own.
+ QCOMPARE(model.index(0, 0, root).data(ThreadListModel::MessageIdRole)
+ .toString(),
+ QStringLiteral("m1@example.org"));
+}
+
+void TestThreadListModel::replyShowsOnlyItsOwnTags()
+{
+ ThreadListModel model;
+ ThreadSummary thread;
+ thread.threadId = QStringLiteral("T1");
+ thread.subject = QStringLiteral("Build fails");
+ thread.totalCount = 2;
+ thread.tags = { QStringLiteral("inbox"), QStringLiteral("work") };
+ model.appendBatch({ thread });
+
+ MessageNode reply;
+ reply.messageId = QStringLiteral("M2");
+ reply.threadId = QStringLiteral("T1");
+ reply.from = QStringLiteral("bob@example.org");
+ reply.depth = 1;
+ // Two the thread already has, one it does not.
+ reply.tags = { QStringLiteral("inbox"), QStringLiteral("work"),
+ QStringLiteral("todo") };
+
+ // Led by the thread's FIRST message, which is what the worker sends and
+ // what the root card draws. setThreadMessages drops it by position.
+ MessageNode root;
+ root.messageId = QStringLiteral("M1");
+ root.threadId = QStringLiteral("T1");
+ root.depth = 0;
+ model.setThreadMessages(QStringLiteral("T1"), { root, reply });
+
+ const QModelIndex threadIndex = model.index(0, 0);
+ QVERIFY(model.hasChildren(threadIndex));
+ const QModelIndex replyIndex = model.index(0, 0, threadIndex);
+ QVERIFY(replyIndex.isValid());
+
+ const QStringList own =
+ replyIndex.data(ThreadListModel::MessageOwnTagsRole).toStringList();
+ QCOMPARE(own, QStringList{ QStringLiteral("todo") });
+
+ // The colours must line up with the names one for one, or the delegate
+ // walks the two lists together and paints a chip in another tag's colour.
+ const QVariantList colours =
+ replyIndex.data(ThreadListModel::MessageOwnColoursRole).toList();
+ QCOMPARE(colours.size(), own.size());
+ QVERIFY(colours.first().value<QColor>().isValid());
+}
+
+void TestThreadListModel::replySharingEveryThreadTagShowsNone()
+{
+ ThreadListModel model;
+ ThreadSummary thread;
+ thread.threadId = QStringLiteral("T1");
+ thread.totalCount = 2;
+ thread.tags = { QStringLiteral("inbox"), QStringLiteral("work") };
+ model.appendBatch({ thread });
- // The header carries no text: a label would set a minimum width far wider
- // than the icon and defeat the narrow column.
- QVERIFY(model.headerData(ThreadListModel::AttachmentColumn, Qt::Horizontal,
- Qt::DisplayRole).toString().isEmpty());
+ MessageNode reply;
+ reply.messageId = QStringLiteral("M2");
+ reply.threadId = QStringLiteral("T1");
+ reply.depth = 1;
+ reply.tags = { QStringLiteral("inbox"), QStringLiteral("work") };
+
+ MessageNode root;
+ root.messageId = QStringLiteral("M1");
+ root.threadId = QStringLiteral("T1");
+ root.depth = 0;
+ model.setThreadMessages(QStringLiteral("T1"), { root, reply });
+
+ const QModelIndex replyIndex = model.index(0, 0, model.index(0, 0));
+ QVERIFY(replyIndex.isValid());
+ QVERIFY(replyIndex.data(ThreadListModel::MessageOwnTagsRole)
+ .toStringList()
+ .isEmpty());
+
+ // A thread row has no thread to differ from, so it never answers these:
+ // its own chips come from PillTagsRole.
+ QVERIFY(model.index(0, 0)
+ .data(ThreadListModel::MessageOwnTagsRole)
+ .toStringList()
+ .isEmpty());
}
QTEST_MAIN(TestThreadListModel)