| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
One markdown file per signature under ~/.config/qtmaildir/signatures/,
spliced into the composer's buffer and switched from a control on the
editor bar.
MessageBuilder needs no change: it already builds text/plain from
markdownBody verbatim and text/html from MarkdownRenderer over the same
string, so one markdown signature in the buffer yields both forms
correctly. That is the "transparent to the user" requirement the note
asked for, and it is why a two-file text/HTML variant was dropped after
being chosen: it buys designed HTML signatures at the cost of the
signature no longer being visible while composing.
The switch stays stateless. seedBody() deliberately refuses to track
"my text" and "the quote" as separate pieces, and a toggle cannot duck
that question the way the quote did; it answers it by scanning for the
last "-- " block not followed by quoted lines, so nothing can desync
from the undo stack. That same scan is what lets signature_position
offer both end (the default) and above_quote over one implementation.
The per-account key does not reopen the note's constraint: an account
seeds the choice, the switch keeps every signature reachable, and the
automatic follow stops once the user picks one.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KEcn3u19xPqv6ggD15PG4c
|
|
Item 94. The query row is the six built-in filters (Unread, Inbox,
Important, Sent, Drafts, Trash), which compose with the account
dropdown, and every saved query lives in the More queries menu. Nothing
has to decide which of the user's queries get button space, which is the
question item 93 would otherwise have had to answer.
SavedQuery::pinned is gone from the struct, the reader, the writer, the
save dialog's checkbox and the pin/unpin context action.
The stored key is stripped rather than left ignored, at the user's
choice. That has one non-obvious requirement: `pinned` stays named in
loadSavedQueries' `known` list precisely so it is NOT collected as an
unknown field, since those are preserved and written straight back. A
mutation removing that name puts the key in the file for ever.
Confirmed with the user before starting that the built-in set covers
their use, since removing pinning removes the escape hatch this item was
blocked on.
Tests: four pinning tests replaced by two on the new rule, four more
converted from buttons to menu entries. migrationPinsEveryEntry and
aStoredGeneratedQueryIsUnpinnedNotDropped are rewritten around the
property that outlived the flag rather than deleted: an entry must be
KEPT, which is what both assertions were really guarding.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KEcn3u19xPqv6ggD15PG4c
|
|
Item 157, the half item 153 did not close. A draft was editable by
double-click and by a Message-menu entry, neither of which is where the
user looks while reading one. populateMessageBar() swaps the reply pair
for edit_draft on a displayed draft.
Three things came out of hand-testing it, each invisible to the tests
written before them.
The bar keyed on currentIndex(), which a query leaves valid on a row of
the discarded result, so it kept the draft button after switching to the
inbox and the reply pair after switching to drafts. This is item 150's
trap one level up. It answers from m_currentMessageId/m_currentThreadId
now, which every blanking route clears, refilled from
showPlaceholderPane(), the one site all five of those routes share.
That exposed a defect predating the bar: updateComposeActions() ran only
from the two selection handlers, so Reply and Forward stayed enabled over
a blank pane. Invisible while they sat on the main toolbar among
always-on actions.
The bar is hidden over an empty pane, so it comes and goes with the
subject and the details button rather than hovering over the logo. That
in turn broke the showing half: setBarActions() runs before showThread()
fills m_items, so the first message opened after a blanking left the bar
hidden and the second showed it from stale items, one selection behind
for the life of the view. updateHeader() shows it, beside the details
button it rides with.
The test missed the last one by asserting before the render landed,
measuring the placeholder; it waits on showingPlaceholder() now.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KEcn3u19xPqv6ggD15PG4c
|
|
Item 153. DraftStore had a write() and no reader, and nothing opened a
composer from an existing message, so a draft rendered like ordinary mail
and could never be finished or sent.
ComposeContextBuilder::forDraft() reads one back. A new Kind::Draft seeds
every field verbatim: the subject takes no Re:/Fwd: prefix, and the body
goes in exactly as it was left, with none of seedBody()'s quote framing. It
is reachable by double-click and by an edit_draft action in the Message
menu.
Three things the shape of this depends on.
A resumed draft must OWN its file. Maildir has no in-place edit, so an
autosave writes a new file and unlinks the old one; a composer that did not
know its own path would leave the original behind and one message would
become two. ComposeContext::draftPath carries it into m_draftPath, which the
autosave already knew how to replace.
MimeParser had no bcc, and nothing had ever needed one. MessageBuilder
writes Bcc into the draft file deliberately and explains why, so a resumed
draft that ignored it would drop every blind recipient from the message the
user then finishes and sends, reporting nothing.
edit_draft is gated on the file being inside a configured drafts folder,
matched on the PATH. A `draft` tag is not enough: notmuch surfaces the
Maildir D flag as one, and a message flagged by another client sits in the
inbox. Offered on ordinary mail, the composer would own a file it did not
write and the first autosave would delete a received message.
And a live defect found on the way, which is most of why this took as long
as it did. updateComposeActions() ran only from onSelectionChanged. Both
signals fire for an ordinary click, so nothing had noticed; but running a
query and setting the current index emits currentRowChanged ALONE, so the
enablement was computed against the previously selected row. Edit draft
stayed disabled on a draft selected that way, and the reply family had the
same blind spot with no test that could see it. Now connected to both.
Reading currentRowChanged is safe here for the reason CLAUDE.md gives: it
answers "which row is current", and no count is read.
WorkerBackedWindow::AccountSpec gains a drafts field, which the two new
tests need and which no fixture could express before.
|
|
Item 138 gave drafts a button and immediately surfaced that they cannot be
edited: DraftStore has a write() and no reader, and no action opens a
composer from an existing message, so a draft renders as ordinary mail and
can never be finished. Filed as 153, a defect rather than an enhancement.
The user's note splits it in two, "double-clicking a draft should open the
editor" and "there's no edit action anywhere". They are one item: the second
is the general case and the first is one route into it.
154 to 156 are the rest of that note's list, all v2 and all unspecified to
different degrees. 154 and 156 are deliberately separate: a read receipt is
the reader's client to honour, a delivery receipt the sending server's, and
whether the latter can be requested at all depends on the send_command.
152 gains the constraint the user added today: a signature is not tied to an
account and is switched from the editor bar, which rules out a plain
[account.*] key as the whole answer.
|
|
Items 138 and 148.
The query row carried Unread, Inbox, Important, Sent and Trash, and no
Drafts, though the composer has been autosaving into each account's drafts
folder since compose shipped. Reaching them meant typing a query by hand.
Smaller than its size suggested: Account::draftsQuery() and
Config::allDraftsQuery() already existed for the placeholder pane's drafts
count, and builtinFilters() derives the row from kQueryGenerators, so the
work was the generator entry, two resolvedQuery branches, a label and an
icon.
It follows TRASH rather than Sent. Folder-matched like both, because `draft`
is a Maildir flag notmuch surfaces as a tag while the folder is what the
user means and what the composer actually writes into. But NOT flat: Sent is
flat so a thread cannot fold the user's own message back into the
conversation it answers, and a draft reply belongs with its conversation for
the same reason a trashed message does.
An account with no drafts folder shows no button, per item 103's rule. The
existing row test surfaced that by failing until its fixture configured one,
which is the rule working rather than a defect.
Ctrl+W closes the composer, which bound nothing at all: the only way out was
the title bar. The action is parented to the composer, so it is a
WindowShortcut dispatched to the active one only and the main window's
namespace is untouched, exactly like the formatting shortcuts. It calls
close() rather than doing anything of its own, since closeEvent() already
decides whether the draft is saved and a second route out that skipped it
would lose the message.
The Italian gains "Bozze"; lrelease reports 478 finished, 0 unfinished.
|
|
Items 142, 143, 144 and 145, to the layout the user described.
The composer had one addToolBar carrying three scopes at once: text
formatting, message composition, and the terminal action. It read as a menu
bar that is not one. There is now no window toolbar at all.
From: [.............] +--------+
To: [.........] [v Cc/Bcc] | Send |
Subject: [...........................] +
[B][I][</>][S][link]["] [Attach] [Send as HTML]
+---------------------------------------------+
| message text |
+---------------------------------------------+
[Remove] * report.pdf <- only when attached
Send is a large icon-above-text button beside the headers: it is the
terminal action and carries the weight to match. Formatting is a toolbar
widget in the central column directly above the text it formats, icon-only
with the words kept as tooltips, which is where a tooltip stops being
decoration. Attach and the HTML toggle ride the right end of that bar, past
a stretch, because neither formats text. Remove attachment sits with the
list it acts on and appears only once something is attached.
"Also send a formatted copy" becomes "Send as HTML": the old label described
a mechanism without naming it, leaving the reader to infer that "formatted"
meant HTML and that "copy" meant a MIME part rather than a second message.
Cc and Bcc hide behind a disclosure beside To:. revealCcBccIfUsed() only
ever shows, never hides, so nothing but the user's own click can make a
field holding an address invisible: a hidden recipient is a message going
somewhere the sender cannot see, which is worse than the clutter this
removes. The label is hidden with each field, since a QFormLayout holds the
two as separate items and hiding the line edit alone strands a "Cc:" over
empty space.
Two send-lock faults, one predicted and one not. The backlog warned that
setInputsEnabled() disabled the single toolbar wholesale, so the send-path
test was strengthened to name every control BEFORE the split; it then caught
Attach live during a countdown, where a file appended after MessageBuilder
has run is either dropped or added to bytes already sent, silently either
way. With every control named it failed again on format_bold: disabling a
QToolBar greys its buttons but leaves each QAction enabled, so Ctrl+B during
a send would have edited a message already being built, through a button
that looked unavailable. setInputsEnabled() now walks the bar's actions too.
The Italian translation is refreshed; lrelease reports 477 finished, 0
unfinished.
|
|
Three corrections from looking at the built bar.
Compose returns to the main toolbar. The split this was built to, "about a
message" against "about the list", does not survive contact: what matters is
what the action NEEDS. Reply and Forward are meaningless without a message on
display, while Compose needs none and is disabled only when no account can
send. So the pane's bar holds exactly the two actions that depend on what it
is showing, and Compose sits with the window-wide ones.
The bar moves below the subject and details rows, directly above the web
view. At the top of the pane it read as window chrome rather than as
belonging to the message. The transient notice bars stay above it: they
explain the message rather than offer an action on it.
Its icons were the style's own default, 16px, which is tiny beside a 32px
toolbar. They are now 7/8 of toolbar_icon_size, which is the 28 the user
asked for at their 32, derived rather than hardcoded so the relation holds
if that key changes. The test asserts the relation as well as the value,
since a bare 28 would stop meaning anything the moment the key moved.
m_headerLabel gains an object name so the placement test can find the row it
must sit below.
|
|
Items 139, 140 and 141, built together because the seam between them is
wasted work: 140 needs a container and 141 is that container.
The main toolbar had grown to mix two scopes. Sync, Archive, Delete, Mark
all read and Undo act on the list or the selection; Compose, Reply and
Forward are about one message. With everything in one row the distinction
was invisible, and Forward was on no toolbar at all, reachable only from the
Message menu, which is item 139.
Compose, Reply and Forward now sit on a bar above the message pane, and
LEAVE the main toolbar rather than gaining a second home: that is what makes
the toolbar's remaining contents mean one thing. Toggle HTML joins them at
the right end, separated by an expanding spacer, since changing how a
message is displayed is a different scope from acting on it. That layout was
the open design question item 141 recorded, and it was settled with the user
rather than guessed.
The actions are MainWindow's own QAction objects shown a second time, never
copies: a duplicate would carry its own enablement and drift from the menu
entry updateComposeActions() keeps in step. MessageView::setBarActions() is
the seam, so the pane still knows nothing about the window's action map.
Two things worth recording:
QToolBar has no addStretch(), so the separation is an expanding spacer
widget. A test asserting only on action ORDER passes with that spacer
deleted, measured, so it asserts on the spacer's size policy instead.
noTwoActionsShareAnIcon looked up the toolbar with an unnamed
findChild<QToolBar*>(). There are two toolbars now, so it is pinned to
main_toolbar: pointed at the pane's bar it would have asserted that a
thread action is absent from a bar that never holds any, and passed while
the rule it exists for went unchecked.
|
|
The previous commit fixed the cursor within each quote_position branch and
the user still saw the old layout, because the branches were already right:
what was wrong was the DEFAULT. quote_position shipped as `above`, and the
layout asked for is exactly what `below` already produced, a blank line at
the top with the quote underneath.
So the default flips, along with the fallback for a malformed value and the
warning naming it. Nothing needs an Upgrading note: compose has not been
released, so no config in the wild sets this.
Focus goes to the body whenever To: is already filled, which a Reply and a
Forward always are. The form's first widget took it otherwise, so the user
had to click into the editor before typing. A New message keeps the default,
since an empty To: is genuinely the first thing to fill in.
The focus test asserts on the window's focusWidget() rather than on
QWidget::hasFocus(): an unshown window is never active, so hasFocus() reads
false whatever the code does and would fail against a correct fix. Both
directions are mutation-checked, since focusing unconditionally passes the
reply case while breaking the new-message one.
|
|
The three out-of-band bars in the message pane were plain labels on the
pane's own background, so they read as part of the page they were trying to
interrupt. They now carry a severity, as the user asked for:
- yellow for a warning that explains a limitation and offers nothing to do
about it (the receive-only ribbon)
- blue for one offering an action (remote content blocked, stale thread),
with the button moved right of a stretch, where the eye ends up after
reading the sentence
Each severity carries its own light and dark set rather than one tint
dimmed, and the theme is read off QPalette::Base, the same surface
HtmlBuilder reads, so a bar and the message under it cannot disagree about
which way round the theme is. Every ground states its own text colour: the
palette's may be near-white, which is unreadable on a pale tint.
The blocked row had to become a widget on the way. It was a bare
QHBoxLayout, which has nothing to paint a ground on, and once wrapped, the
six sites that hid its label and button individually had to hide the
wrapper instead or an empty painted strip would have been left behind.
The test asserts on the stylesheet string rather than on pixels, per
CLAUDE.md on rendering probes: an unshown widget offscreen renders nothing,
so a pixel test would pass whatever the code does. It compares the COLOURS
the sheets name rather than the sheets themselves, since each names its own
widget and would differ by that alone.
|
|
MessageView::clear() resets the blocked-content bar, the stale notice and
the attachment bar by hand, and forgot the receive-only ribbon. Only
setReceiveOnlyAccount() ever hid it, and that is reached from
updateComposeActions(), which runs on a SELECTION change.
So the ribbon survived every route to a blank pane that is not one:
clear_pane, clear_selection, a new query, and a multi-row selection. It sat
over a blank pane, or over another account's mail, naming an account that
was no longer on screen and contradicting the live Reply button beside it.
Worth recording because it cost a test: the first one written for this
selected receive-only mail and then mail from a sending account, which is
how the report reads. It passed against the unfixed code, because that
gesture is precisely the one path that was already covered. The test now
drives clear_pane and a new query as well.
Its own trap, in the test rather than the code: clear_pane leaves the row
CURRENT, so re-selecting it emits no change and the ribbon is never
re-raised. The test moves away and back instead.
|
|
quote_position names where the QUOTE goes, so the reply belongs on the
other side of it and the cursor has to follow the reply rather than the
buffer. seedBody() moved the cursor to Start under both positions, which
is correct only for Below: under Above, the shipped default, it landed on
the "On ... wrote:" attribution line, so every reply had to have room made
before it could be typed.
End under Above, Start under Below.
The existing theQuotePositionDecidesWhereTheQuoteLands passed throughout
the defect and still does, because the quote was in the right place all
along; only the cursor was not. The new test asserts the cursor's block is
blank, and that typing lands on the correct side of the quote, so a fix
that freed the cursor by inverting the position would not satisfy it.
Also re-measures item 136 in the backlog, which is not the intermittent
race it was filed as: undoMovesTheMessageBack fails 6 runs in 6 when named
alone and passes in the full suite, on a clean tree, so it depends on the
tests before it.
|
|
Task 13 of item 123, the documentation that was left when the code was
finished.
The README claimed "sending is not implemented. Compose, reply, forward and
send are planned for v2 and need a companion send script that does not exist
yet", which had been false for three days. It also documented none of
`trash`, `send_command` or the `[compose]` section: `trash` has been effectively
mandatory since item 103 and its absence breaks Delete, so a user reading only
the README could not configure a working install. Both are item 122, which
closes with this.
Every default in the new [compose] block was read out of config.h rather than
from the prose around it, which was worth doing: send_html defaults to TRUE
and the first draft of this documented it as false, and the key is
autosave_interval_ms rather than the autosave_delay_ms that reads more
naturally.
The v1/v2 language is retired rather than updated. The project follows semver
on its user-visible surface and those labels described a scope split that
stopped being true when compose shipped. CLAUDE.md says so explicitly rather
than deleting the sentence, since the older spec and plan documents still use
the phrase and are historical records: read it there as "before compose".
CLAUDE.md's architecture diagram listed none of the compose units. It does
now, and it distinguishes the classes from the namespaces, because most of
them are namespaces of free functions over values: the markdown, the MIME
assembly and the account-picking are all testable without a widget. There is
no FormatToolbar class, which the first draft of this diagram invented, and
that is the exact mistake the paragraph under it already warns about for
QueryBar and AttachmentBar.
Also records item 148 from the notes: Ctrl+W does not close the composer.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q2koFevoSxTLhfexJTZWQd
|
|
The 2026-08-23 pass over the user's notes. Ten rows, eight sections: two of
the ten are duplicates of items already open and get a row saying so rather
than a second entry.
Six of the eight are the compose interface, which had never been looked at by
a human until this week. They are presentation rather than defect, and every
cause is verified in the code rather than copied from the note: one toolbar
carrying formatting, Attach and Send together (142), text buttons where an
editor uses icons (143), a checkbox label that describes a mechanism without
naming it (144), and Cc and Bcc as unconditional form rows (145).
The other two are the message actions. Forward has no button anywhere (139),
and the user's own answer to that is bigger than a missing button: Compose,
Reply and Forward belong over the message pane rather than on a toolbar that
otherwise acts on the list (140), which needs a bar that does not exist yet
(141). 139 closes inside 140 if that is built.
138 is the closest thing to a defect here: every account already configures a
drafts folder and the query row has no generator for it, so the one folder the
composer writes to is the one folder with no button.
Item 21 gains a pointer rather than a plan. The user is drafting the shortcut
table in their own notes and it is the specification this item has been
waiting for, so the entry says to read it and not to propose one. It records
what the draft settles (Ctrl+Return for Send, which closes an open question
from item 123 task 11), the two collisions its proposal creates (Ctrl+R is
restore today, Ctrl+F is coupled to Find moving to `/`), and the trap under
that last one: a `/` registered as a shortcut is dispatched before any
editable widget sees it, so it needs the event filter Return already uses.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q2koFevoSxTLhfexJTZWQd
|
|
The reply family is disabled on mail that arrived at an account with no
send_command, behind a ribbon in MessageView naming the account and the key
to add. save_message is deliberately never disabled: it is the escape hatch
for exactly that case.
The ribbon is a WIDGET in the pane's layout, never markup inside the web
view. Composing HTML from configuration into the one document that renders
input from strangers is the wrong direction, and the header row is already a
widget for the same reason.
Compose itself is disabled only when NO account can send, and that state is
not warned about at startup: an installation with no send_command anywhere
is a valid read-only installation.
Every reply resolves through messageScopeFor(), not threadFor(): a thread
row means the one message its card shows. Replying to a thread is
meaningless; a reply answers a message. The context is built from the
DATABASE rather than the model, the rule Restore already follows, because a
row whose state has not been re-queried carries stale values and a reply
built from one would carry the wrong recipients.
The mail root crosses from the worker as its own signal. There was no route
for it at all: mailRootOf() is file-static in notmuchworker.cpp, and item
124 records that composing a destination from database.path writes into the
Xapian tree under a split index. The test uses NotmuchFixture::splitIndex(),
the only layout where the two accessors disagree.
A thread row's path is RELATIVE to the mail root while a message row's is
absolute, so the account lookup matched nothing and the reply family was
dead on mail from an account that could send. Found by the positive guard
test rather than the negative one, which passed throughout for the wrong
reason.
The quit path checks the failed-save case FIRST. In the ordinary case
nothing is lost by saving; there, saving is what is already not working, so
the dialog says plainly that quitting loses that text rather than offering a
save that will fail again. Both dialogs name the composers, and the ordinary
one asks once whatever the count, because three modals in a row is worse
than a coarse answer. Its wording says drafts already saved stay in the
folder, so Discard cannot read as 'delete my three messages'.
The Save loop holds QPointers, not raw pointers. A deleteLater() posted
while a nested exec() runs IS processed by that nested loop, measured in a
standalone program: the guard nulls before the modal returns. Closing a
composer while the quit dialog is up therefore freed a window the loop then
called saveDraftNow() on, crashing at the exact moment the application
promised to preserve that text.
A compose request that matches nothing clears itself and says so. It was
cleared only on a match, so a message deleted between selection and Reply
left the request armed for the session: Reply did nothing, and the next
ordinary click on that message opened a composer nobody asked for while the
pane stayed blank.
Forward carries the original's attachments, which the context has always had
a field for and nothing ever filled, and seeds its HTML toggle from
[compose] send_html. Only Reply seeds that from the original.
save_message keeps its filename inside the chosen directory and no longer
overwrites a file already there. The check was correct and untested: the
test asserted through Attachment's helpers rather than through the function
production calls, so deleting the containment check outright left it green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TvwDptMWxjqhbCmjxwcSZ2
|
|
Found while running the suite during item 123 task 10, and checked rather
than assumed to be unrelated: with the branch's work stashed out, on a clean
tree, it still fails 1 run in 6. A failure that appears during unrelated work
gets blamed on the change in front of it unless someone measures.
Sized ? deliberately. The race is either in the test's wait or in the
Maildir move Delete performs and Undo reverses, and the two have very
different consequences: a test that waits wrongly is noise, while a move
that races is mail landing in the wrong folder, which this document already
records as reaching the mail server.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FXF741wz4SY7j5dqvAxMU5
|
|
MarkdownFormat, task 8 of the compose-and-send plan. Three free functions
over (text, selection start, selection end) returning the new text and the
selection that follows it, so the grammar is tested without a widget.
Three gaps in the plan's draft, each now pinned by a test checked against the
mutation that breaks it:
- QString::lastIndexOf INCLUDES the position it is given, so quoting with the
cursor at the end of a line found that line's newline and quoted the
FOLLOWING one. The draft's fixtures never placed a cursor there.
- A backwards selection was normalised but never tested, so the swap was
unguarded; a right-to-left drag is an ordinary gesture and Qt reports the
anchor after the cursor. normalise() now swaps and clamps in one place.
- A blank line inside a quoted range produced "> " with trailing whitespace,
which editors and mail clients strip anyway. It is written bare.
Two further defects came out of review:
- quote()'s selectionStart was unasserted for any block not starting at line
zero. Hardcoding it to 0 passed all nineteen tests, because the one test
naming the property quoted the first line, where right and wrong coincide.
A wrong selection there means a second press quotes a line the user never
selected, and a following Bold bolds the wrong text.
- A selection splitting a surrogate pair split the character across the
inserted tokens, leaving invalid UTF-16. Not reachable from the toolbar,
where arrow keys and mouse hit-testing both move in whole clusters, but
reachable by any code computing a position arithmetically. normalise()
nudges off a low surrogate; a collapsed cursor moves back on both ends,
since widening would turn "insert an empty pair here" into "wrap the emoji".
The buttons stack rather than toggle: a second Bold press gives ****this****,
and a second Quote press nests. That is what the spec specifies, and the
preserved selection exists so a second press can apply a SECOND token. A
toggle was built during this task at the user's request and reverted on
finding it contradicts the spec at two sites; it is recorded as backlog item
135, where the unanswered question is what replaces bold-then-italic.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LoaLBowZ6w1JNx6SEhDP1L
|
|
ComposeContext, task 7 of the compose-and-send plan. Address parsing,
recipient derivation, the References chain, subject prefixing and account
resolution, as free functions over values so they test without a painter.
Recipient derivation was designed from the spec rather than transcribed: the
plan's draft omitted it and its tests could not compile, calling
QVERIFY(config.load(path)) against a void return.
Six defects found in review, each pinned by a test checked against the
mutation that breaks it:
- Message-ids reached GMime bare, and GMime writes an EMPTY header for a bare
addr-spec rather than complaining. In-Reply-To and References both shipped
blank, so every reply would have arrived as an orphan thread with nothing
wrong to see locally. MessageBuilder now brackets on write, in the one place
that composes those headers rather than in each caller.
- internet_address_to_string was called with FALSE for the encode flag, so a
display name carrying a raw newline rendered with the newline intact. That
is a header-injection primitive.
- A reply to the user's own message addressed the user. It now goes to that
message's original recipients, mirroring their To/Cc split, which is what
the Sent view and a follow-up on unanswered mail need.
- A From parsing to no mailbox left To empty, reachable from real mail
("From: Mailer Daemon"). MessageBuilder treats an empty recipient list as
success, so the message would have been handed to the send command with
nobody to deliver to and filed in Sent looking sent.
- The References header was split on whitespace alone, so a client's
non-conformant "<a@x>,<b@y>" became one token and the bracket strip produced
the fabricated id "a@x>,<b@y".
- Reply and forward prefixes were recognised in English only, doubling every
AW:, SV:, WG: and Re[2]: a mixed-locale mailbox receives. Single-letter
spellings are deliberately excluded: with R: recognised, "R: report on Q3"
reads as a prefix and a genuine first reply threads nowhere.
The mailbox-only guard in parseAddressHeader survived its first mutation
check, because removing it still yields no recipients: the invalid GObject
cast makes GMime's own assertion return NULL. That is undefined behaviour
papered over by an assertion G_DISABLE_CHECKS compiles out, so the test now
asserts on the emitted critical rather than on the count. Registering the log
handler on a NULL domain catches nothing; the criticals carry "GLib-GObject"
and "gmime".
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LoaLBowZ6w1JNx6SEhDP1L
|
|
MessageSender runs the configured command with the message on stdin and
judges the result by its exit status alone. Nothing here waits on the
event loop, so a send does not block the GUI thread; a 1.6MB payload was
probed through a reading stub without deadlocking the pipe buffer.
The command is split and passed to QProcess as a program and an argument
list, never through a shell. A test asserts that by giving the command
shell metacharacters and checking that the marker file a shell would have
created does not exist, so the property fails a mutation rather than
resting on a comment.
Four corrections to the plan's draft. splitCommand handles double quotes
only, so a single-quoted argument splits wrongly and the header now says
so. A crashing command delivers finished(11, CrashExit) and would have
been reported as "exited with status 11", so a crash branch was added. A
command that exits without draining a large stdin emits WriteError before
finished(), which the draft handled correctly and by luck, untested. And
an empty send_command is checked after trimming.
Two contract gaps found in review, both about what this class promises
rather than what it does.
The exactly-once guarantee covers the EMIT, not what a caller receives: a
long-lived sender plus a connect() inside each send accumulates
receivers, and the second result then runs the first send's lambda too,
filing a sent copy of the wrong message. The header now scopes the
promise and requires Qt::SingleShotConnection. The plan's Task 11 call
site already had that flag, sixty-nine lines below the connect and
outside anything a reader would see, so the plan gained a note where
someone retyping it will read it.
And destruction mid-send killed the command with no report, announced
only by a Qt warning: a live SMTP conversation abandoned, possibly
partially delivered, while the user believes it was cancelled. The
destructor now closes stdin, waits a bounded five seconds, and only then
kills. It emits nothing either way, because the outcome after a kill is
genuinely unknown and reporting "not sent" for a message that may have
gone out is the mailsync.sh mistake pointing the other way. Claiming
m_reported before kill() is what makes that true, since kill() delivers
finished(CrashExit), which would otherwise emit exactly that untruth.
No timeout on the send itself: killing a slow but working send is worse
than waiting. Task 10 owns the popup, and deliberately offers no cancel
after commit, so this class promises none either.
Also refreshes the translations Task 5 left out. That gap was invisible
because test_translations builds its rows from the .ts file, so a string
that never entered it is never asserted on.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QP2g3b3kuLx6AYFCNEz6UR
|
|
Task 4's code review measured it: a large attachment is read and base64
encoded on the calling thread, and autosave calls build() from a GUI-thread
timer. The directory hang that review found is fixed, but the blocking read
is by design and will be felt in the composer.
Recorded against Task 11 rather than fixed, because nothing in the composer
crosses the worker boundary and adding a second threading model for one call
is worse than the stall.
|
|
Task 2's code review found that the README's sample config documents every
other key, including recently added ones, and has nothing for send_command
or the [compose] section. Without it those keys ship undiscoverable: a user
has no way to learn that sending exists at all.
That is a gap in the plan rather than a deviation by the task, since no task
claimed the README, so it becomes a step in the close-out where the rest of
the documentation is written.
|
|
Thirteen tasks, ninety-nine steps, against the spec committed earlier on this
branch. Written on master so it is readable from either branch; the
implementation goes on compose-and-send.
Every API assumption was verified against this machine rather than written
from memory, which found five things the spec had wrong or unstated:
libcmark-gfm-extensions ships NO pkg-config file although libcmark-gfm does,
so CMake needs find_library beside pkg_check_modules. All three enabled
extensions live in that second library, so finding only the first yields a
build that compiles and silently renders plain CommonMark.
GMime defaults to iso-8859-1, emits no Date or Message-ID unless asked, and
g_mime_text_part_set_text() encodes with whatever charset is set when it is
called, so setting the charset afterwards produces a part labelled utf-8
carrying latin-1 bytes. All three fail only on accented text, which for this
user is every message. The plan builds the content stream directly and
carries a working probe's output as evidence.
MessageNode has no body or date field, so quoting takes a ParsedMessage.
ThreadListModel::messageScopeFor() takes a QModelIndexList rather than a
single index. There is no Config::maildirPath(): the mail root comes from
notmuch_config_get(NOTMUCH_CONFIG_MAIL_ROOT) via a file-static helper in the
worker, and item 124 records that composing a destination from the wrong
root would write into the Xapian tree.
Two spec statements are corrected in the plan rather than followed. It calls
for a new top-level Message menu and one already exists at
mainwindow.cpp:1156. And it requires a shortcut per action, which item 132
changed while this was being planned, so save_message ships without one.
|
|
everyActionHasAShortcut() was written when the action list was short and
every action plausibly deserved a chord. Item 123 adds six more, and under
that rule each one consumes a key sequence whether or not anyone would ever
press it. Rarely-used actions were being given chords to satisfy a test
rather than because a user wanted them.
everyActionIsReachableFromAMenu() is the rule that actually matters, and it
already has the right shape: it is what stops an action shipping invisible,
which is the defect item 103 found when `restore` was reachable by a chord
and by nothing a user could see. Discoverability comes from the menu. A
shortcut is an accelerator for the things done often.
Nothing replaces the deleted test and nothing else needed changing:
showShortcutReference() already prints `(unbound)` for an empty sequence, so
the code anticipated this and only the test forbade it.
Verified rather than assumed: with `tag_rules` unbound in defaultBindings(),
an action that is registered, menu-reachable and carries an icon but has no
chord at all, the full suite passes. Before this commit it failed.
CLAUDE.md's "adding an action is FIVE places" paragraph is updated, including
its count of how many are test-enforced, which drops from four to three.
|
|
Three rows in every state, so nothing reflows: status label, bar,
right-aligned Undo. The bar changes mode rather than place, determinate
and draining during the countdown because that has measurable progress,
indeterminate once send_command starts because a send does not.
Undo stays visible after it disables. A control that vanishes re-lays
out the popup mid-operation, and a greyed one says why cancelling is no
longer possible where an absent one looks like it was never offered.
The status label sizes from the longest string it can hold in the
current language rather than from its content: Italian "Rimozione della
bozza..." is longer than "Removing draft...", so a content-sized label
resizes the popup between stages, which is the jumping the fixed layout
exists to prevent.
Item 134 gains a requirement from this: the extracted widget must expose
both bar modes, not only the indeterminate one MainWindow happens to
need today.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YDq53rMd3AQp7QmcZzpuBM
|
|
The user asked for Gmail's undo-send, and it answers a question the
spec had left open: what Cancel means during a send.
It means nothing, if offered while send_command is running. Killing an
SMTP client mid-transaction leaves an unknown send, since the message
may have reached the server in full before the kill, and that is worse
than either clean outcome. Moving the cancel window before the command
starts makes Undo mean genuinely nothing happened.
The popup owns the whole operation, countdown through completion,
rather than a countdown popup handing over to a status bar. One widget
changing state in one place, and it keeps the eye-catching surface the
user asked for. Modal to the composer only, so a second composer and
the main window stay usable. No close button and no Escape: during the
countdown a dismissal cannot say whether it means cancel or send now.
send_delay_ms defaults to 5000, and zero skips it.
The test for this asserts a negative: Undo leaves the stub command never
run. A test asserting only that the composer reopened would pass against
a design that ran the command and discarded the result, which is exactly
what the delay exists to prevent.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YDq53rMd3AQp7QmcZzpuBM
|
|
Two corrections from the user, both of which the spec had wrong.
A failed sent-copy write was put in the main window's status bar, on
the reasoning that the composer closes so the message needs somewhere
persistent. Wrong instinct: the fix for "the window is gone" is a
dialog, not a quieter surface. It is the one failure here that produces
a silent divergence between what the recipient received and what the
local archive holds, and nobody discovers that from a line that showed
for a few seconds. It gets a modal.
A failed autosave stays in the composer but as a persistent banner
rather than a status-area line, since the quit path already escalates
that state to a dialog and depends on it surviving.
Stated as a rule at the head of the section, because the user's point
was general: modal for silent divergence, banner for mid-task, status
bar only for what is already obvious.
Second correction: the composer's busy indicator is not built inline. A
second instance of MainWindow's progress-bar-plus-label pairing is
where a widget class earns itself, and "this codebase builds small UI
inline" describes what the code does rather than justifying repeating
it. Item 134 extracts it and converts MainWindow.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YDq53rMd3AQp7QmcZzpuBM
|
|
The spec said "disabled with a spinner" without saying where, which
leaves a popup as a reasonable reading of it. A popup is wrong here: it
would be modal over a window that is already disabled, and it can be
dismissed while the operation continues, which is the indicator
ambiguity items 18, 19, 28 and 54 each closed once.
Progress goes in the composer's own status bar, through the three
stages the operation actually has, since a failure filing the sent copy
means something different from a failure sending. The window closing is
the success message.
The indicator is an indeterminate QProgressBar built inline, matching
MainWindow's m_syncProgress rather than factoring out a shared widget:
this codebase builds small UI inline, and two progress bars do not
justify a third class.
Also settles what the staged display implies for a sent-copy write that
fails after a successful send: the composer still closes, because
holding it open for a message already sent invites sending it twice,
and the warning goes to the main window's status bar instead.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YDq53rMd3AQp7QmcZzpuBM
|
|
The spec said "the editor is plain text" and left it there, which reads
as "you are on your own with the syntax". Storage format and editing
affordances are separate decisions and only the first was stated.
The toolbar is text transformation over the markdown source, not
rich-text editing: bold, italic, code, strikethrough, link and quote,
selection-aware, with the cursor landing between the tokens when there
is no selection.
Its shortcuts belong to the composer window's own scope and do not
touch KeyMap, which matters for item 132: the two namespaces should not
be conflated when that rule is revisited.
Live syntax highlighting is a follow-up (item 133) rather than part of
this: agreeing with the grammar about nesting and about code spans is
the expensive half, and it is better judged after living with the
toolbar.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YDq53rMd3AQp7QmcZzpuBM
|
|
The spec called it a new dependency needing a SlackBuild REQUIRES entry.
It is a new dependency, but /var/log/packages/ shows
cmark-gfm-0.29.0.gfm.13-x86_64-3 with no _danix tag, so it is stock and
REQUIRES lists only non-stock dependencies.
Also records the staleness cost accepted with it: cmark-gfm tracks an
older CommonMark base (0.29 era) than the stock plain cmark (0.31.2).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YDq53rMd3AQp7QmcZzpuBM
|
|
Brainstormed with the user. Design only, no code, which is what the
item's #plan-only tag asked for.
The decision that shaped everything: there is no MTA on the machine, so
"an external script on the same model as mailsync.sh" had no model to
copy. Send becomes a per-account send_command taking the message on
stdin, exactly as [sync] command already works, which keeps the
no-network-protocol rule intact without naming an MTA.
An account with no send_command is receive-only by construction, which
is how one of the five accounts is meant to work. Reply, reply-all and
forward are disabled on its mail behind a ribbon that says why.
The body is markdown parsed by cmark-gfm rather than a hand-written
parser for a limited set: the two share no code, so the small one is
deleted wholesale the moment the set widens.
Four new units, three of them widget-free and testable without a
painter. MessageSender is deliberately a separate unit rather than a
method on the composer, so a future outbox wraps the funnel instead of
reworking it.
Item 123's section is replaced by a pointer to the spec, per this
document's own rule for a fully specified item. The brainstorm opened
items 128 to 132, including a review of the every-action-has-a-shortcut
rule, which the user raised: six more actions takes it past the point
where a chord for everything is useful.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YDq53rMd3AQp7QmcZzpuBM
|
|
The entry said sending should be an external script on the same model as
mailsync.sh. Measured on the machine, there is no such model to copy: the
fetch side has mbsync and the send side has nothing.
No MTA is installed at all, msmtp and sendmail are both absent, and neomutt
sends over its own built-in SMTP configured per account in
~/.config/neomutt/accounts/*.rc. So the working setup this application
mirrors has no external send path either.
That makes the first question a non-UX one, ahead of everything the note
lists: sending needs either an MTA the user chooses to install and
configure, which is the only shape that keeps the no-network-protocol rule
intact, or a decision to relax that rule. Recorded so the brainstorm does
not assume the tidier answer, since installing and configuring an MTA is
work he has not asked for and the credentials already exist elsewhere.
Also notes that all five accounts already configure a drafts folder, so the
draft half has somewhere to live before anything is decided.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YDq53rMd3AQp7QmcZzpuBM
|
|
Not a regression and not something we removed: Save image is item 114,
still open. Item 127 removed Save LINK and deliberately left this one.
The circumstances the user reported sharpen it twice. The image had already
had its remote content loaded, so m_allowRemote was still true at the click.
That flag is live on the shared interceptor and cleared by the next
showThread(), so a download handler is subject to whatever it says at the
moment of the click rather than at render time. A naive handler therefore
looks perfect in exactly this case and fails once the grant is gone, which
makes "it worked when I tried it" worthless as evidence. The entry records
that both cases must be tested against a message whose grant has been
cleared.
The second is a corollary of item 127. downloadRequested is per-profile, so
connecting it lights up every download entry Chromium offers at once,
including the Save link just removed from the menu. An entry being absent
from a menu is not the same as the capability being absent, so the handler
must decide per request rather than merely exist.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YDq53rMd3AQp7QmcZzpuBM
|
|
Reported by hand after the item 127 fix: right-clicking a link still
offered Save link.
It had been deferred to item 114 alongside Save image, on the grounds that
both are inert without a downloadRequested handler. That is true and it was
the wrong conclusion, because the two are not the same question.
Save image is content the message already carries, and item 114 is about
making it work. Save link fetches a remote URL chosen by the sender,
through the pane's profile, which is the one profile in this application
that must never fetch remote content: that is what m_allowRemote and the
interceptor exist to prevent. Answering it with a download handler would
put a network fetch of attacker-controlled content behind one context-menu
entry.
Saving what the user actually wants already has a path that never touches
the network: saveAttachment(), which writes a MIME part already parsed into
memory and sanitises the filename.
So it is removed rather than implemented, and the test asserts its absence.
Item 114 now carries the constraint that follows: a downloadRequested
handler added to make Save image work must not make Save link reachable
again, which the natural per-profile implementation would do by default.
Mutation checked: dropping the entry from the filter fails the test with
"a link action survived: Save link".
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YDq53rMd3AQp7QmcZzpuBM
|
|
Items 126 and 127, in one sitting because the second is only safe after
the first.
126: an anchor carrying target="_blank" did nothing when clicked, with no
error and nothing on screen. Chromium routes such a click to
QWebEnginePage::createWindow() rather than to acceptNavigationRequest, and
MessagePage did not override it, so the base implementation returned
nullptr and the URL was discarded before any of our code saw it. Plain
anchors were unaffected and already worked, which is why this presented as
"HTML mail is broken" while a text mail's links opened: marketing HTML sets
_blank on practically every anchor.
createWindow() receives a WebWindowType and no URL, so an override cannot
simply read the target: it arrives afterwards as a navigation on whatever
page is returned. LinkRelayPage is that page. It has no view, hands the URL
to the same handler the plain-link path uses, refuses the navigation, and
deletes itself. Nothing is ever fetched and no second QWebEngineView is
created.
127: OpenLinkInNewTab, OpenLinkInNewWindow and OpenLinkInThisWindow join
removeBrowserActions()'s list. Item 100's list is the PAGE actions and was
tested by right-clicking the page; these appear only over a link, so it
never saw them. CopyLinkToClipboard stays, being the fallback for any link
that will not open. The order matters: 126 gives the page a working
createWindow(), so those entries would have stopped being dead and started
opening links into a tab that does not exist.
Testing needed two seams. The click cannot be synthesised, since JavaScript
is off in this profile (measured: runJavaScript returns an invalid QVariant)
and a synthetic press would depend on the anchor's rect and the desktop's
fonts; setUrl() is no substitute because it arrives as NavigationTypeTyped.
clickLinkForTest() and relayBlankTargetForTest() drive the real overrides on
the real page, and setLinkOpener() substitutes a recorder for
QDesktopServices::openUrl.
Both routes are asserted rather than only the broken one, since they share a
handler now. Three mutations checked and caught, including the filter also
removing CopyLinkToClipboard, which a later sweep of "dead link actions"
would otherwise take silently.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YDq53rMd3AQp7QmcZzpuBM
|
|
The first diagnosis was wrong and the user's own follow-up disproved it: a
plain-text GitHub mail opens its links correctly while an HTML newsletter
does not. If RequestInterceptor blocking https were the cause, neither
would work.
Verified against the two messages named. The difference is target="_blank".
An anchor with no target navigates the main frame and reaches
acceptNavigationRequest, which hands it to QDesktopServices::openUrl; that
path works today. An anchor asking for a new window is routed by Chromium
to QWebEnginePage::createWindow(), which MessagePage does not override, so
the base implementation returns nullptr and the click is discarded before
any existing code observes it. Marketing HTML uses _blank almost
universally, which is what makes it read as "HTML mail is broken".
Both messages render HTML, so this was never a text-versus-HTML
distinction: the GitHub mail is multipart/alternative and its HTML part is
what the pane shows.
The entry also drops the proposal to let main-frame navigations through the
interceptor. That would have weakened the remote-content protection to fix
something it was not causing. Nothing here needs m_allowRemote relaxed: the
URL goes to an external browser and the pane fetches nothing.
Records the trap that decides the fix's shape: createWindow() receives no
URL, only a WebWindowType, so an override returning nullptr discards the
target before it can be read.
Item 127 is updated to match. OpenLinkInNewTab and OpenLinkInNewWindow fail
through the same missing createWindow(), so fixing 126 may make them start
working, which is worse rather than better.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YDq53rMd3AQp7QmcZzpuBM
|
|
Two defects found by hand, related but separate.
126: clicking a link in a message does nothing. The handler is already
there and correct, calling QDesktopServices::openUrl from
acceptNavigationRequest, and it has presumably never run. RequestInterceptor
denies http and https whenever m_allowRemote is false, which is the default
for every message, and it runs on the request before the page is asked
whether to accept the navigation. The click is dropped at the network layer
with no error, no navigation and no browser. That is the remote-content
protection working as designed; the bug is that a deliberate click is
indistinguishable from a resource the document fetched itself, at the layer
where the decision is currently made.
127: a link's context menu still offers Open in new tab, Open in new
window, Save link and Copy link. Item 100 removed the page-level actions
and its list names four of them; the link actions are different WebAction
values that Chromium adds only over a link, so item 100 never saw them.
Two are dead (there are no tabs, and a second view is deliberately never
created), one belongs with item 114's missing download handler, and Copy
link works and is currently the user's whole workaround for 126. It
follows 126, since a working click makes one "Open link" entry the right
answer rather than a removal.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YDq53rMd3AQp7QmcZzpuBM
|
|
Item 124 shipped and is proven: the index moved from a 7200rpm platter to
NVMe with the mail staying at /data/Mail. Cold start went from 38618 ms to
668 ms for a complete walk, and 2008 ms to 50 ms for the first rows.
Counts held at 49174 messages / 5594 inbox / 100 tags at every step, and
Delete then Restore round-tripped through the account's trash by hand.
Item 121 stays open, and its entry now says why. The measured platter
figures are the evidence FOR building the indicator, not against it: a
mechanical disk is the cheap configuration, not an exotic one, and a user
with a large Maildir on spinning rust has nowhere to migrate to. Fixing
one developer's hardware is not fixing the application. The constraint
that pointed at item 124 as the answer is replaced by one saying the
opposite, and prefaulting stays rejected on its own merits since it is
worst on the low-memory machines most likely to have a slow disk.
Item 125 is new, found by hand during the migration. mailsync.sh exits 75
(EX_TEMPFAIL) when another run holds the lock, and the sync indicator
never clears; because an edit made during a sync is held until the sync
ends, a Delete sat queued for a completion that could not arrive and
looked like it had done nothing. Nothing was lost, since held edits reach
the disk, but the user cannot tell that.
Also records in CLAUDE.md that notmuch_database_get_path() is not the mail
root, that database.hook_dir defaults into the index directory and
silently stops post-new under a split config, and that the ordinary
fixture layout cannot tell the two accessors apart.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YDq53rMd3AQp7QmcZzpuBM
|
|
notmuch can be configured with `mail_root` and `path` as separate keys,
which puts the Xapian index outside the Maildir. Under that layout
notmuch_database_get_path() returns the INDEX directory, and the worker
treated it as the mail root at four sites.
The consequences are not symmetric. Message paths resolved to `../..`
escapes that match no account prefix, which is a display defect. But
moveMessages() composes its destination from the same root, so Delete
would have written into the Xapian tree: outside the Maildir, invisible
to mbsync, and gone from every other client. That is the stranded-mail
failure of item 103 with a new cause.
notmuch_config_get(NOTMUCH_CONFIG_MAIL_ROOT) is correct under both
layouts, so no conditional is needed. Verified against the live database:
with only `path` set it returns the same string as get_path(), making
this a no-op for the current configuration.
The fixture gains an opt-in splitIndex(). That is load-bearing rather
than convenience: in the ordinary layout the index lives inside the mail
root and both accessors return the same string, so a test written
against it passes whichever one the code uses. All three new tests fail
against the old accessor, confirmed by mutation.
Also records the finding as backlog item 124, and corrects item 121's
timings, which had been copied from item 74 rather than measured. A cold
run seven minutes after boot, with the index verifiably unread, gives
2008 ms to the first rows and 38618 ms to a complete list, against the
642 ms and 5714 ms recorded there.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YDq53rMd3AQp7QmcZzpuBM
|
|
Item 115's closed section, which the previous commit failed to update: its
python edit asserted on wording that did not match the file and aborted, so the
code shipped and the record did not.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Items 115 and 117, both from the user's notes.
Select all was never in Chromium's menu for this pane, measured by hand with a
selection active and against a build with removeBrowserActions() reverted, so
the filter is not what removed it. MessageView::addPaneActions() supplies it,
static and taking the menu, mirroring removeBrowserActions() beside it. Two
comments claiming the standard menu already offered it are corrected; either
would have sent the next reader down the same three wrong theories the item
records.
The copy entries all worked and none of them said so. Four now report through
the pane's existing statusMessage, each naming what it copied rather than saying
"Copied", which is the item's own constraint when three of them sit together in
one menu. Connected to the page's own QActions, so the report follows the entry
wherever it is triggered from.
The two differ in what can be tested, and the tests say so rather than papering
over it. The copy path is fully covered: triggering the action runs the
production path, and mutations for a duplicated message and an unwired entry
both fail. addPaneActions() is covered, but showBodyContextMenu() CALLING it is
not and cannot be, since createStandardContextMenu() returns nothing outside a
real context-menu event; a mutation deleting that call leaves the suite green,
measured. The call site is a hand test and the test file records that so nobody
adds an assertion that appears to cover it.
The copy strings are QT_TR_NOOP inside an array, which CLAUDE.md warns extracts
nothing at file scope. Verified rather than assumed: lupdate found all four
under the MessageView context, because the array sits inside a member function.
387 finished, 0 unfinished.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Two entries in the notes had no item here. Both causes verified in the code
rather than copied from the note.
Item 119, the unsynced-changes count being clickable, is bigger than it reads.
pendingEditCount() sums four sources and one of them, m_unnettablePendingEdits,
is a bare int by design: it counts confirmed changes carrying no message ids, so
a dialog built from what is currently kept can list three groups and then owes
the user a remainder it cannot describe.
Item 120 goes to the deferred table, matching where the user filed it. It is not
plannable as it stands: nothing records which rule tagged a message, so the
information does not exist to display, and creating it means the post-new hook
storing something per message, which is a shared-format change across both
repos.
Everything else in the notes maps to an existing item.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Every step of the plan is done. Item 103's section moves to the closed file on
the same commit, per this repo's own rule, with its outcome recorded: what was
built, the ten defects hand testing found that the suite did not, and the two
process gaps closed alongside them.
The fact worth carrying forward is the one that damaged real mail. Under
mbsync's Create Both, a wrongly named origin folder propagates to the mail
server, so any code composing a folder name reaches the server whether it means
to or not.
Item 118, emptying the trash, remains deferred at the user's request.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Nine tasks, TDD throughout. The ordering mutation in task 4 is the
load-bearing check: indexing the new path before removing the old one is
what preserves a message's tags, and the wrong order passes every other
test while silently destroying them.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Item 103's measurement is done, so its section carries the finding and the
three constraints that decide whether to open the spec, rather than the
design inline.
Item 118 is blocked on 103 and is the first action that would destroy mail
with no undo, which is why it is filed separately rather than folded in.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Measures what Delete does today (a notmuch tag and nothing else, verified
against the tag-to-flag table and a probe on a throwaway database) and
specifies the move-to-trash behaviour that replaces it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
The reconciliation against the user's own notes found ZERO unrecorded
entries, the first clean pass: the 2026-08-16 sweep added items 98 to 104 and
those absorbed the whole current "Not done yet" list.
Closed this session, sections moved to the closed-items file: 98, 100, 102.
Dropped:
- 78, at the user's request. Never a defect. Items 85, 23 and 81 already give
the whole journey (right-click a value, search it, save the query, make a
rule from it); this was only a shortcut across it, and the entry had already
said to gather usage evidence first. That evidence never appeared.
- 116, the same day it was raised, and its section is kept for the process
failure rather than the non-bug. Copy image was reported as copying markup
instead of pixels. Two explanations were eliminated by real evidence, and
the conclusion drawn was that something more interesting must be wrong; the
actual answer was that the measurement distinguishing them was broken. A
wl-paste reading taken minutes after the copy showed text flavours only, was
explicitly labelled unreliable in the entry, and was then reasoned from
anyway. Run immediately after a copy it reports image/png and 30 more, and
pasting into GIMP immediately works. A caveat that does not stop the
reasoning it qualifies is decoration.
Opened:
- 112, Toggle unread on a whole thread cannot reach "all unread" on a
partly-read thread. The direction comes from notmuch's UNION over the
thread, so one unread message anywhere makes the action pick "mark read" and
no input reaches the other branch. Third defect from that union after 110.
- 113, view source as our own plain-text dialog.
- 114, Save image is offered and does nothing: no downloadRequested handler
exists anywhere. The user corrected the first proposal, which would have
refused remote images on security grounds; once remote content is granted
the bytes are already fetched, so saving them is a local copy and blocking
it protects nothing.
- 115, no confirmation when a copy succeeds.
- 117, the pane offers no Select all. NOT caused by item 100: verified against
a build with that filter reverted. Three wrong theories preceded that
measurement, and the lesson is one item 100 had already written down: a menu
built by hand proves nothing about the menu Chromium builds.
The changelog's Unreleased section gains Important-as-a-toggle, the rules
Note column, the menu fix, and two Upgrading notes.
|
|
A thread's card has rendered one message since item 66, but every tag
action still acted on the entire conversation. Delete, Archive,
Important, Mark spam and Toggle unread now act on the message the card
shows; the whole-thread versions move to a "Whole thread" submenu in the
Message menu and the thread list's context menu, on Ctrl+Alt+<key>.
Closes items 87, 88, 105, 106, 107, 108, 109, 110 and 111.
The defects fixed along the way, several found by reading rather than by
report:
- threadAt(current.row()) answered about the wrong thread for a reply
row, because a tree numbers rows per parent. The audit found four live
sites, not the one reported: Delete and Toggle unread each chose their
DIRECTION from an unrelated thread, and the tag dialog counted the
wrong thread's tags. threadFor(index) replaces them.
- A message-scoped write made no optimistic model update and no reply
row carried a doomed cue, so acting on a reply moved the pending-edit
count and changed nothing on screen.
- Both toggles read the state of a reply's THREAD, which a
message-scoped write never changes, so they were one-way: the second
press re-sent a tag the message already had.
- flushHeldEdits() re-sent only thread-scoped edits, so a tag change
made on one message during a sync was applied to the row, counted as
unsynced, and then dropped without ever being written.
- applyTagChange() updated a thread's summary but not its loaded
replies, leaving an expanded thread's rows describing a state the
database no longer held.
- A thread's first message is not among its children, so both
message-scoped lookups missed it: acting on a root card repainted
nothing and emptied the message pane's chip row.
- ThreadSummary::tags is notmuch's union over the thread, so a card
standing for one message drew tags belonging to its siblings. The
worker now reads that message's own tags in the walk that already
finds its id, so the split is known before a row is ever opened.
The card shows both tiers: its own message's tags at full size, the rest
of the conversation's smaller and muted, so nothing appears to vanish
when a row is selected.
Auto mark-read is message-scoped as a result, and now arms for a reply,
which it never did. With maildir.synchronize_flags on, the old
thread-wide write reached the server for mail that had never been
displayed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Double-clicking any row drills into its thread: the list becomes that thread
alone, expanded, and the pane shows the double-clicked row's own message. A
reply therefore opens its WHOLE thread with itself selected, never itself alone,
which is what the user asked for and is not the obvious reading of "open it by
itself".
This is recoverStaleThread() triggered by a gesture. That function already ran
thread:<id>, expanded the thread when the row arrived, selected the target
message once the replies landed, and fell back to the root when the message had
gone; all three cases are existing paths through it, so the new code resolves a
row to a thread id and a message id and hands both over.
The row is reached through the INDEX and never through index.row(): a tree
numbers rows per parent, so threadAt(row) on a reply answers about an unrelated
thread. That is item 88's trap, avoided here by construction.
The first click of a double-click arms the mark-read timer, and the handler
cancels it, because a gesture that navigates must not mutate mail. The timer is
armed again for whichever row the recovery lands on, so only the arming for the
row being left is cancelled. Its test asserts the timer was active beforehand,
so it cannot pass by the timer never having been armed at all.
The expander keeps its own double-click: ThreadListView::mousePressEvent accepts
a press inside its rect and returns, so Qt never pairs one into a double-click
there.
Nothing is built for getting back. The filter buttons already are that, per the
user.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
The entry said a double-click runs a query naming the row, an id: for a message
and a thread: for a thread. That is wrong for a reply, which the user wants to
drill to its THREAD with itself selected, not to itself alone. The view is
always the whole thread, expanded; only which message the pane shows changes.
It also assumed a thread: query would show the conversation. Nothing in the tree
auto-expands, so it lands on one collapsed card and the replies still need a
click.
Both are already solved by recoverStaleThread(), which runs the query, expands
the thread, selects the target reply when the replies arrive, and falls back to
the thread when the message has gone. Item 91 is that mechanism triggered by a
gesture rather than by the stale-thread notice, so the approach is to reuse it
rather than write a second selection-after-query path.
Recorded alongside: a double-click delivers a single click first, which arms the
mark-read timer, so the handler must cancel it rather than marking a message
read that the user only passed through.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
