diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-10 09:33:44 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-10 09:33:44 +0200 |
| commit | 01419de209c2b5e2ae7b996e6b5ff1baa2efb3da (patch) | |
| tree | 25718fd36e87eac721f41b02cca46b6fb07e94d9 /CLAUDE.md | |
| parent | f72dba9f6c463c6823d85701e51d8be38dd22a62 (diff) | |
| parent | e1dba2987a9a1e87b92801959df9c9d4f1375d2f (diff) | |
| download | qtmaildir-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.
Diffstat (limited to 'CLAUDE.md')
| -rw-r--r-- | CLAUDE.md | 119 |
1 files changed, 102 insertions, 17 deletions
@@ -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 |
