| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
Item 28, reported by the user: open a thread, let the automatic
mark-read remove `unread`, then press Ctrl+U to put it back. The
indicator read "2 unsynced change(s)" with the mail store exactly where
it started.
The counter incremented per confirmed write and never decremented, so
any add-then-remove of the same tag inflated it. Mark-read is simply the
path that fires without being asked, which is why it surfaced there.
The user's call was net state: an edit and its inverse are zero
outstanding changes, because what the indicator answers is whether
quitting now would strand work. A QHash keyed "<messageId>\n<tag>"
replaces the int, and a pair that reverts is erased rather than stored
with the new direction, so the map cannot grow without bound across a
long session of tagging and untagging.
Keyed per (message, tag) rather than per message: removing `unread` and
adding `flagged` on one message are independent changes and must not
cancel each other. A change carrying no message ids cannot be netted
against anything and is counted separately, since dropping it would
understate the indicator, which is the direction that costs work.
Both properties item 18 established still hold: a successful sync clears
everything, a failed one clears nothing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Items 16 and 33.
Delete now removes the `deleted` tag when every selected thread already
carries it, so pressing it twice puts a thread back. One direction for
the whole selection, never per row: toggling each independently would
leave a single keystroke with the selection in two states, which is
worse than either outcome.
Status messages are classified rather than blanket-timed, which is the
substance of item 33. Events expire after six seconds and fall back to
the last query's thread count: "Sync complete", "Nothing to undo", the
skip notice, the per-action "Archive: 3 threads". State does not expire:
"Searching...", "Syncing...", the selection count, and "Sync failed
(exit N)", because an error must not vanish before it is read.
A test caught a mistake in that routing. Making the per-action message
transient armed the timer during select-all, since tagSelected() runs on
a selection onSelectionChanged() had just described, and the count would
then be replaced while it was still true. Writing the count now cancels
any transient still counting down.
QStatusBar::showMessage() would give the same behaviour but the label is
added with addWidget() beside permanent widgets, so adopting it means
reworking that arrangement. One timer beside the label is the smaller
change.
Both fixes verified by reverting them and watching the tests fail.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Items 29 and 32.
29 was a constraint item 27 specified and that shipped unbuilt: while a
cron sync held the lock the Sync button stayed clickable, and pressing it
could only produce the EX_TEMPFAIL skip. The progress bar and the button
are now written by one updateSyncControls() taking both sync sources,
which the item asked for by name: two independent assignments, one per
path, means whichever finishes second wins, so a background sync ending
would re-enable the button in the middle of a local run.
Unknown re-enables the button, deliberately. It means /proc/locks could
not be read and nothing was observed, so leaving the button disabled
would strand it permanently wherever the lock cannot be seen.
32 adds a clear_pane action on Esc. It clears m_currentThreadId with the
pane, not merely alongside it, or a threadLoaded still in flight would
paint the thread straight back; and it cancels any pending mark-read,
since a thread blanked from view must not be marked read two seconds
later. The selection, the query and the undo stack are untouched.
The one real risk in 32 was Escape being stolen from the query
completer, the way Return was once lost to a window shortcut. Probed
rather than reasoned about: a popup consumes the key before a
window-level shortcut sees it, so the completer still dismisses.
Every test here was verified by reverting the code it covers.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Reported from hand testing: a manual sync ended with "Sync finished
elsewhere" stamped over its own result.
Ownership of a lock period was being decided when the lock was RELEASED,
by asking MailSync::isRunning(). That question cannot be answered then:
the process exits, so isRunning() goes false, and only afterwards does
the next poll observe the lock gone. The guard therefore suppressed the
message while the sync ran and let it through at the end, up to two
seconds after onSyncFinished() had already said what happened.
Ownership is now latched when the lock APPEARS, which is the moment
isRunning() can still answer, and the matching release is swallowed.
onSyncFinished() hands the latch back when it sees exit 75, because a
skip means the lock was never ours: if a manual run and the cron run
start inside one poll interval, the lock would otherwise be latched as
local and that other run's completion swallowed with it.
Also renames the messages to "Background sync running/completed" per the
user: "finished elsewhere" reads as though the application does not know
what is syncing the Maildir, when in fact it is the same script.
The tests added here cover the external path and the Unknown state. They
do NOT reproduce the reported bug, and were checked against a reverted
fix to confirm that: staging it needs isRunning() true at the Running
transition and false at the Idle one, which cannot be arranged in
test_mainwindow without a configured sync command and a live child
process. That was tried and abandoned, it left a process running for the
length of the suite and popped a dialog. The ordering and the fix were
instead verified against a standalone model of both code paths.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
The user's cron runs mailsync.sh every ten minutes, so mail arrives and
tags change while the window sits idle, and nothing here noticed. The
script already holds an flock for the whole run, so that lock is the
signal: no status file is needed, and a kernel lock cannot go stale
because it dies with the process holding it.
The observation method is the part that matters, and two of the three
plausible ones are wrong. Both were probed on Linux 6.18 before any of
this was written:
- flock -n acquires in order to test, so polling every two seconds
would open a window every two seconds in which a starting
mailsync.sh is refused the lock and exits 75. It would cause the
very skips the script reports.
- fcntl(F_OFD_GETLK) never acquires and looks ideal, but reports
UNLOCKED against a lock held by flock(2): separate lock namespaces
in the kernel, which cannot see each other. A silent false negative.
- /proc/locks is a pure read. It observes flock(2) correctly, and
since it takes no lock at all it can never contend with the Xapian
write lock notmuch new holds during the same run. Confirmed: 200
reads left the lock table unchanged and this process holding
nothing.
SyncMonitor keeps the parsing separate from the polling so the parsing
is testable, and it reports Unknown rather than Idle where /proc/locks
cannot be read: "no sync is running" is the claim that would let the
window quit, so it must never be guessed. Verified against a real flock
end to end, not only against synthetic content.
It reports rather than refreshes. runCurrentQuery() clears the undo
stack, the selection and the message pane, which is right for a query
the user typed and hostile for one a cron timer fired: it would discard
undo history and close the thread being read up to six times an hour,
with no action from the user.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Multi-select already worked by Ctrl+click and Shift+click, but nothing in
the UI said so and every tag action was keyboard-only, so the Ctrl+T tag
dialog could not be reached with a mouse at all.
Adds a select_all action on Ctrl+A, registered like every other action so
it reaches the Edit menu, the shortcut reference and [keys]; a right-click
menu on the thread list built from the same QActions rather than parallel
copies; a selection count in the status bar, which is the part that
actually teaches the feature by acknowledging a selection while it is
being built; and a note in the shortcut dialog for the mouse gestures,
which are view behaviour and cannot appear in the generated table.
A selection gesture must not open mail or mutate it. Selecting several
rows now blanks the message pane and cancels any pending mark-read,
rather than rendering each row swept through and queueing it to be marked
read.
Two Qt behaviours shaped this, both established by probe rather than from
memory:
- selectAll() emits no currentRowChanged at all and leaves the current
index invalid.
- currentRowChanged is emitted BEFORE the selection model is updated.
The second one caused two distinct faults. Collapsing a multi-row
selection back to one row reported the old count, so the guard swallowed
the load and the pane stayed blank; that case is handled in
onSelectionChanged, which sees the true count. And a Ctrl+click taking
the selection from one row to two also reported one, so the thread was
loaded, blanked, and then painted back when the queued reply returned
from the worker. By the third row the id was already cleared and the
reply was discarded, which is why the fault presented as an off-by-one in
the threshold rather than as a race.
Tests cover the synchronous half. The late-reply guard has no test:
MainWindow in tests has no worker, so threadLoaded never fires and the
repaint cannot be reproduced in process. Verified by hand instead.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Four changes to the sync UI, three of them from using it.
The log pane could not be dismissed. It is hidden at construction and
shown on failure, and nothing ever hid it again, so a single failed sync
left it on screen until the application restarted. It now sits in a
container with its own Close button, and is 200px rather than 120,
because mbsync's output is wide and repetitive and the shorter pane
showed too little of it to read. It still appears only on failure, which
the user confirmed is what they want.
A sync gives no feedback while it runs. The status bar now carries an
indeterminate progress bar for the duration, and the Sync button is
disabled rather than left looking live. The bar is indeterminate on
purpose: mbsync reports no percentage and the script's output is
unstructured, so a bar filling left to right would be inventing a
fraction nobody knows. The log is also cleared at the start of each run,
since leaving the previous run's lines in place makes a stale failure
look like the current one.
The lock skip was reported as a failure. mailsync.sh exits when another
run holds the lock, and that was exit 1, which qtmaildir reads as "sync
failed": it showed the log pane and, on the exit path, told the user
their changes were still unsynced. With a cron timer every ten minutes,
a click landing inside a run is routine and none of that is true. The
script now exits 75 (EX_TEMPFAIL) and the window reports it as its own
case, saying a sync is already running. On the exit path it stays open
and says plainly that the other run is most likely carrying the changes
over but that this window cannot see it finish, rather than guessing
either way.
That last hedge is what item 27 records: the application cannot see a
sync it did not start. The user chose continuous polling of the lock file
over the narrower "only while quitting" version, and the entry notes that
the lock is already the signal, so no status file is needed, and that a
kernel lock cannot go stale where a written file can.
Verified against stub binaries: a second run while the lock is held exits
75 and says SKIPPED, while the run holding it completes at 0.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Five hardcoded tags were the only ones reachable from the UI: archive,
delete, spam, flag and toggle_unread. For an application whose purpose is
organising mail by tag, applying any other one meant leaving for a
terminal. Item 26 of the usability backlog, raised by the user asking how
to add a tag and finding they could not.
One dialog rather than separate add and remove actions, at the user's
choice: filing something under a new tag while dropping inbox is one
thought, not two. Type tags to add or remove, comma separated, or clear a
checkbox to drop a tag already on the selection without retyping its
name.
Both fields complete against the tag list MainWindow already holds for
the query completer. Completion is a guard against typing shoppping
beside shopping, never a whitelist: inventing a tag is the entire point,
so any valid name goes through whether or not it exists yet.
Tri-state checkboxes carry the multi-thread case, and are where the risk
is. A tag on some selected threads shows partially checked, and leaving
it alone changes nothing; the opposite reading would silently tag threads
the user never looked at. A tag already on every thread and left checked
is likewise not a change and is not sent as one.
Tag names are validated before anything is applied, through a free
function so the rules are testable on their own. Empty, a leading dash
(notmuch's CLI reads it as removal, making such a tag a trap), whitespace
and control characters are refused by name and reason. Nothing is applied
until the whole set passes, since the user cannot tell which half of a
partial change landed.
TagDialog is pure UI: handed the vocabulary and the current state,
returning two lists, contacting no worker. That is what lets its fifteen
tests run without a notmuch database. Integration is a single call to the
existing tagSelected(), so undo, the optimistic model update, the
combined multi-row query and the completer refresh for a brand-new tag
all come for free.
One test assumption was wrong and the code was right: a case asserted
that QStringLiteral("null\0byte") truncates at the null and reads as
empty. It does not, so the null is caught as a control character. The
test was corrected rather than the validator.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Tagging changes the notmuch index at once, but the mail store only hears
about it on the next sync, and nothing said so. Quitting with tagging
outstanding was silent. Items 18 and 19 of the usability backlog, built
together because the second needs the first's counter.
The counter cannot be QUndoStack::isClean(), which is the obvious
candidate and the wrong one: the undo stack is cleared on every query,
since its entries refer to rows the new result set discards. Tag a
thread, run any query, and the stack is empty while the change is still
unsynced. m_pendingEdits is its own count, incremented where a write is
CONFIRMED rather than where one is sent, so an optimistic update the
worker later rejects cannot leave the indicator claiming an edit that
never landed. Only a successful sync resets it: clearing on failure would
assert the changes had reached the mail store when the sync is exactly
what failed to put them there.
It is shown in the status bar, hidden entirely at zero, and described as
a lower bound rather than a guarantee, since an external notmuch run can
carry changes over without this application noticing.
On exit, sync_on_exit in [general] takes ask, always or never. Three
values rather than a bool because "prompt me", "just do it" and "do
nothing" are three behaviours and true/false expresses two; an unknown
value warns by name, since a typo there silently changes what happens to
unsynced work. The prompt offers three buttons for the same reason: a
user who hit Quit by mistake needs a way back that is not "sync". A sync
started at exit holds the window open until it finishes rather than being
killed mid-run, and a sync that FAILS does not quit, because quitting
there would discard the user's choice silently. With no sync command
configured the prompt degrades to a plain warning instead of offering a
sync that cannot run.
This is not a destructive-action confirmation of the kind CLAUDE.md
forbids. Those cover tag mutations, which keep undo instead of a dialog.
This asks about losing work at the one point where undo cannot help.
The tagsApplied lambda became a named slot, which is better structure and
also what lets a test drive it: the worker is deliberately parentless
because it moves to its own thread, so reaching it with findChild to emit
the real signal cannot work, and contorting the test to try was the wrong
instinct. Testing a modal needed its own care. A test that sends a close
event hangs forever if an unexpected dialog opens, because the modal
spins its own event loop; CloseProbe polls for activeModalWidget, closes
it and records that one appeared, turning "a dialog opened" into an
assertion rather than a hang.
Also removes a stray qDebug left in the open_thread action by the earlier
Enter-key investigation, which had reached two commits.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Opening a thread left it tagged unread, so the unread count never
matched what had actually been read and the app was quietly wrong every
day it was used. Item 6 of the usability backlog.
A single-shot timer, armed when a thread is selected and restarted rather
than stacked, so arrowing down a list marks only the thread still
selected when it fires and not every one passed through. Configurable
through mark_read_delay_ms in [general], defaulting to 2000: zero marks
read at once, and any negative value disables the behaviour, which is why
the value is neither clamped nor warned about at either end.
The automatic change deliberately does NOT go on the undo stack. It
routes through sendThreadTagChange() rather than tagSelected(), because
undoing an action the user never took is worse than leaving a thread
read, and toggle_unread already gives them a direct way back. It still
funnels through the single applyTags path; what differs is only whether
the inverse is pushed, which is a window-level decision above the worker.
An explicit toggle_unread cancels any pending timer, or marking a thread
unread by hand would be reversed a moment later and the key would look
broken.
Two guards beyond the plan, both from asking what happens when a timer
outlives the thread it was armed for. Arming is skipped for a thread that
is not unread, so no write is scheduled that would change nothing, and
the handler re-checks that its thread is still selected and still unread
before writing, so a stale timer does nothing rather than tagging the
wrong thread.
The plan expected the rapid-arrow case to need a database and a manual
check. It needs neither: ThreadListModel takes threads through
appendBatch(), so the case is unit-tested. All three tests were confirmed
to fail against deliberately broken versions, one arming for read threads
and one creating a timer per selection instead of restarting one.
Item 7 is closed in the same pass. The user verified against real mail
that HTML messages already open as HTML, which is what the item asked
for, so it is recorded as done with no code changed. The prefer_html key
it floated was not added: nobody has asked to default to plain text, and
Ctrl+H already switches a thread by hand.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Pressing Return in the query bar moved focus to the thread list and left
the query unrun, so a query typed at the keyboard could not be executed
at all.
Return is bound to open_thread as a Qt::WindowShortcut, and a shortcut is
dispatched before the focused widget ever sees the key. Qt withholds a
plain-LETTER shortcut from an editable widget, which is why every other
binding in the map was safe here, but Return is not a letter and gets no
such protection: the action fired from inside the bar, its handler called
setFocus() on the thread list, and QLineEdit::returnPressed was never
emitted.
Accept the ShortcutOverride for Return and Enter on the query bar, which
tells Qt the focused widget wants the key as ordinary input and stops the
shortcut being dispatched. Narrow by design, one widget and one key, so
open_thread keeps working everywhere else in the window.
Three earlier hypotheses were tested and disproven before this one, and
two synthetic probes wrongly reported the binding as harmless: real input
sends ShortcutOverride first and only dispatches the shortcut if nothing
claims it, while QTest::keyClick skips that round trip entirely. The new
test drives the override exchange rather than the keystroke and fails
against the old code. The comment claiming no filter was needed said the
letter rule covered this case; it did not, and it now says so.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Adds the complete_query action, whose registration the assertion in
registerActions() has been demanding since keymap gained the name: a
known action nothing implements would have been a silently dead binding.
Also lands the QueryCompleter half of the manual trigger, which the
keymap commit left behind: triggerCompletion() and the focus-in filter
gated on completion_on_focus.
Tags refresh at startup, after a sync, and after a mutation introduces a
tag not already known, since that is the tag most likely to be typed
again. onAllTagsReady deliberately drops the generation the signal
carries: a tag list is not an ordered query result, so a later one is
always at least as good as an earlier one and discarding on staleness
could only throw away a good list.
|
|
Resizing the window, the splitter or a thread-list column was undone by
the next launch. State now round-trips through a separate settings file
at ~/.local/state/qtmaildir/uistate.conf, written on close and read at
startup.
The state file is deliberately not the user's config: a base64 geometry
blob does not belong in a hand-edited file, and rewriting that file on
exit would drop its comments and key order, which QSettings does not
preserve.
Two details that are easy to get wrong:
QStandardPaths::StateLocation appends both the organization and the
application name, and both are "qtmaildir" here, so it resolves to
~/.local/state/qtmaildir/qtmaildir. The path is built from
GenericStateLocation instead, matching Config::defaultPath().
Restore runs after buildMenus() rather than at the end of buildUi():
QMainWindow::restoreState() matches toolbars by object name and silently
drops the position of one that does not exist yet.
Every restore is conditional on a non-empty blob, so a missing or
rejected state file leaves the built-in defaults instead of producing a
zero-size window.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Two optional keys in an [account.<key>] stanza. color fills the chip,
label sets its text.
Both belong to the account rather than to [tagcolors] because an account
tag is a different taxonomy: which mailbox a thread arrived in, not what
state it is in.
label is display only. "account-provider-work" is a lot of
row for one bit of information, but the notmuch tag is never renamed, so
existing queries and external tagging are unaffected. Unset falls back to
the account key, and an empty label is ignored rather than rendering a
blank chip.
|
|
Actions were a QHash of std::function dispatched by an event filter,
which nothing could put in a menu. They are QActions now, bound from
KeyMap so a [keys] override reaches the menus as well as the keyboard.
Menu bar covers every action; the toolbar carries only Sync, Archive,
Delete and Undo. Help > Keyboard shortcuts is generated from the actions,
so it shows what the keys really do rather than a copy that drifts.
spam and load_remote gained defaults, having been unreachable without a
hand-written binding.
The event filter is gone. Probing showed QAction shortcuts are dispatched
before the focused widget sees the key, so they beat QAbstractItemView's
type-to-search without one, and Qt already suppresses plain-letter
shortcuts while an editable widget has focus. Dropping the filter's
blanket guard also lets Ctrl+Q work while the query bar has focus.
registeredActionNames() is derived from the actions rather than
hand-maintained, so the two drift tests it needed are replaced by checks
that a configured binding reaches its action.
No confirmation dialogs: tag mutations still answer to undo.
|
|
Confirmed with the maintainer as v2-only rather than v2-or-later. LICENSE is
the official text from gnu.org. Every file under src/ and tests/ carries the
matching notice.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Wires the worker thread, thread list, message pane, sync process and undo
stack together, and replaces the placeholder main() with real startup:
custom URL schemes registered before QApplication, a libnotmuch ABI check,
and config loading.
Four fixes against the drafted version:
- onWorkerError() only set a status label. Its own comment elsewhere
claimed it reverted the optimistic update, and the spec requires that;
it did not, so a rejected write left the list showing a tag the database
never received. The pending change is now recorded and rolled back, and
a confirmed tagsApplied clears it so a later unrelated error cannot undo
a write that succeeded.
- runCurrentQuery() cleared the model but left the undo stack pointing at
rows that no longer exist. Undoing after a new query would have written
to the database while the visible list stayed put. The stack is cleared
with the model.
- m_currentMessages was assigned on every thread load and never read.
Removed.
- buildUi() connected sync output to m_syncLog and errors to m_statusLabel
before either existed. Both are constructed before the wiring now.
cidPrefix generation lives here, this being its only producer in the
application, and is pinned by tests: it must never contain '!' and must be
distinct per message, which are the invariants the cid: namespacing rests
on. A second test holds registeredActionNames() against
KeyMap::knownActions(), since those two hand-maintained lists drifting
either way silently breaks a user's key binding. Mutation-verified that
dropping an action fails the test by name.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
