diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-08 11:36:06 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-10 08:23:42 +0200 |
| commit | dbca3a604470dbd500f02325dd6a0501ca008ec3 (patch) | |
| tree | a68d06b64a682b635d341449a9eb55fb1b1d5152 | |
| parent | 7c3648676e188344dabb24e084f91b2b47e87633 (diff) | |
| download | qtmaildir-dbca3a604470dbd500f02325dd6a0501ca008ec3.tar.gz qtmaildir-dbca3a604470dbd500f02325dd6a0501ca008ec3.zip | |
docs: record message rows, and the user's verdict on them
CLAUDE.md described a QTableView over a table model, which has not been true
since the view port. Updated with the traps the port produced, each of which
shipped a plausible-looking broken build before being caught:
- A tree numbers rows per parent, so nothing may be keyed on a row NUMBER.
- drawBranches runs before the row's cells, so an expander on a content
column is painted over by the delegate's background.
- setRootIsDecorated(false) removes the style's HIT AREA along with its
indicator, leaving a glyph that renders and does nothing.
- isExpanded and setExpanded are keyed on column 0.
- A reply's indent must beat the account chip's width, and visualRect
reports the indent correctly even when nothing is visibly indented.
- paintEvent runs after the cells, so a full-row fill erases their text.
Also the notmuch ownership rule, which is a double-free if undone: messages
reached through a thread are freed with it, so walkReplies holds them raw
against this file's own RAII convention.
Item 20 is marked built, not done, and item 53 records why. The user's verdict
on the finished result was that the table view does not fit the use, said with
every cue in and working. That is a design finding rather than a defect: the
item shipped exactly what its four decisions specified, and all four were the
user's own choices. Recording it as a defect would misattribute the cause;
recording nothing would leave the next session building on a rejected design.
Item 53 carries the cause verified in code rather than guessed. A message row
fills the same five columns as a thread row (threadlistmodel.cpp:275-283
mirroring :428-431), so replies land on the same rigid column boundaries as the
threads around them, and the eye reads columns before indentation or tint. The
reference the user gave has no column rules through its reply rows at all, and
that absence is the one thing three added cues cannot supply.
| -rw-r--r-- | CHANGELOG.md | 13 | ||||
| -rw-r--r-- | CLAUDE.md | 73 |
2 files changed, 77 insertions, 9 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index fdbb08f..1444c3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,6 +62,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 @@ -42,8 +42,9 @@ 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) + │ thread rows, expanding to message rows; RowStyleDelegate every + │ column, SubjectDelegate on Subject (chip, subject, expander) └ MessageView (header QLabel, QWebEngineView, attachment bar, TagStrip) Config (INI) KeyMap MailSync (QProcess) MimeParser (GMime) @@ -63,19 +64,73 @@ 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. +every column (a `Q_ASSERT` catches this); 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. + +**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 a flat `0..N` walk +paints the first thread's strip over every one of them. The strip walk goes by +index, alternating colour follows visual position rather than `index.row()`, and +`QTableView::isRowSelected(int)` has no equivalent — use +`selectionModel()->isSelected(index)`. Row height comes from +`setUniformRowHeights` plus the delegate's `sizeHint`, since a tree has no +vertical header to carry a default section size. + +**Four 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). `SubjectDelegate` draws it instead, from BOTH of its branches — +calling it only from the no-chip branch leaves every real row without one, since +every real row has an account chip. `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. And `isExpanded`/`setExpanded` are keyed on **column 0**, so +asking them about the subject-column index always answers false and every click +expands again instead of toggling. + +**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. + +**A reply row's indent must beat the account chip's width.** A thread row draws +a chip before its subject and a reply row does not, so a reply's text starts +roughly a chip-width to the LEFT of its thread's before any indent applies. +Qt's 20px default is swallowed entirely by that difference and the replies read +as flush or outdented. `SubjectDelegate::kReplyIndent` is 72px for this reason. +Note that `visualRect` reports the indent correctly the whole time, so a +geometry probe endorses a layout with no visible nesting: assert on where the +TEXT lands. + +**`paintEvent` runs AFTER the cells.** Anything it fills across a row covers the +text the delegate just drew: the reply tint filled the full row height in its +first version and erased every sender and subject, measured at zero surviving +text pixels. The fill and the thread-line stub stay in the band below the text, +where the tag strip lives on thread rows. **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 |
