| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
Extends the reconciliation step rather than adding a second one: after
diffing the user's notes against the backlog, render what is open as a
table with a short description, the size, and one note saying what
decides whether the item can be picked up now.
Then stop. The user picks. Recommending a single item reads as though
the choice were already made.
Defects are flagged apart from enhancements, because they read alike in a
numbered list and do not deserve equal billing: item 28 sat as "a counter
is wrong" while the indicator was quietly lying about whether the user's
work was safe to quit on.
Records two gotchas found by running the grep: item 12 lives in the
deferred table, which has different columns and no size, and a status
cell is prose rather than a keyword, so "open, on demand" and "open,
unspecified" both mean open.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Corrections from using 0.8.0: net unsynced count, Delete as a toggle,
expiring status messages, Sync disabled during a background sync, Esc to
blank the pane, and a named default on the quit prompt.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
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>
|
|
Two 0.8.0 defects could not be given regression tests because
test_mainwindow has no notmuch database, so threadLoaded never fires:
the queued reply repainting a blanked pane, and a local sync reporting
itself as a background one. Both were verified by hand and by a
standalone model, and the first guard is still untested.
tests/notmuchfixture.h already does what is needed and test_notmuchworker
already uses it; test_mainwindow does not. So the item is wiring, not new
machinery.
Records that a corpus built from real mail was considered and rejected:
the rule against personal data in fixtures forbids it, hooks enforce it,
the repo is public, and one surviving In-Reply-To header names a real
correspondent. Minimal hand-written .eml files reproducing a message's
shape are the supported route, as truncated.eml already is.
Deferred by the user until a defect needs it. Wiring it with nothing to
test proves nothing.
The table also still showed items 24 and 25 open; both shipped in 0.8.0.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Item 31. The user could not tell which button Enter would press on the
unsynced-changes dialog.
The code was already correct: setDefaultButton() is called, and Qt agrees,
isDefault() and hasFocus() are both true on "Sync and quit". The active
style, qt6ct-style, simply draws no visible default-button decoration. The
GIMP dialog offered for comparison is GTK drawing its own focus ring, a
different toolkit.
Naming the default in the text rather than restyling the button:
overriding the appearance means fighting the user's theme, which is worse
than one word. The safe option was already the default, so no behaviour
changed.
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>
|
|
The backlog is downstream of the user's own notes, which they add to while
using the app. Comparing the two turned up nine entries with no item here,
two of them defects rather than enhancements:
- Re-adding `unread` after the automatic mark-read counts 2 unsynced
changes when the mail store is back where it started. The counter
counts writes and never decrements, so any add-then-remove of the
same tag inflates it (item 28).
- The Sync button stays enabled while a background sync holds the lock.
That was a written constraint of item 27 and shipped unbuilt, which
is the strongest argument for doing this comparison at all (item 29).
The rest are enhancements: a useful blank right pane, for which the user
has produced the two logo images added here; a default button on the quit
prompt, recorded as needing a repro because the code does set one; Esc to
blank the pane; expiring status messages; a Maildir overview; and an
automatic refresh after a sync.
Item 35 is the one 0.8.0 deliberately did not build. A background sync
reports rather than refreshes, because runCurrentQuery() clears the undo
stack, the selection and the message pane. A non-destructive refresh is
real work, so it is an item rather than a flag.
Item 27 gains its outcome, including that its own proposed approach was
the wrong one: flock -n acquires in order to test, so polling with it
would have caused the exact skips the sync script reports.
CLAUDE.md now carries the reconciliation step, since a note saying "X is
broken" is a bug report that will sit in a personal file indefinitely
unless someone goes looking.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Multi-select discoverability and awareness of background syncs.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Multi-select discoverability (items 24, 25) and awareness of syncs this
window did not start (item 27).
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>
|
|
currentRowChanged fires before the selection model updates, and
selectAll() emits it not at all. Both cost real debugging, and the first
caused two distinct faults in one change, so they belong beside the
setCompleter trap rather than only in commit history.
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>
|
|
Real Maildir account keys had reached comments and test data:
provider-and-mailbox names across three source files, one of them
carrying a surname, plus a real address used as example data in the
notmuch test fixture and the design spec.
The user's standing rule is that maildir and account names never reach a
commit, and this is about to become a public repository, which is what
makes it consequential rather than untidy. Replaced with generic keys
that carry the same shape, since the length is the point in every one of
these comments: a 33-character account tag is why the chip label exists
and why the tag column was removed.
The measurements stay. They are the evidence behind those decisions and
are not personal details.
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>
|
|
The obvious crontab line is the wrong one. The script writes to
~/.local/state/mailsync.log itself, so a caller appending to that same
file gets every line twice, which is what the first real run of the moved
script did.
Also records that rotation belongs to logrotate rather than the script,
and why: the two fought over the same file and the script overwrote a
compressed generation with an uncompressed one.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Two problems, both visible in the first real run of the moved script.
Every line was logged twice. The script writes to the log through tee,
and the user's crontab also ended in ">> mailsync.log 2>&1", so both put
the same output there. Reproduced: without the caller's redirect one run
writes a single RUN START, with it, two. The crontab redirect goes away
and the script keeps owning the log, which is what makes it behave the
same however it is invoked. A comment in the header now says so, since
the obvious cron line is the one that breaks it.
The internal size-based rotation is removed outright.
/etc/logrotate.d/mailsync already owns this file and keeps seven
compressed days, and the two were fighting: the script's
"mv $LOGFILE $LOGFILE.1" overwrites whatever logrotate had just placed
at .1, which is why that file was 357K and uncompressed while .2 through
.5 were properly gzipped. logrotate is better at this in every respect,
including recreating the file with the right ownership, so the script
should not have a second opinion.
Verified against stub mbsync and notmuch in a fake HOME: one RUN START
per run, stdout and the log agree, no rotation performed by the script,
and a failing mbsync or notmuch still produces its own exit code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
The reference sync script lived in the companion mailctl project, which
documents that it never calls it: sync and organization are separate
there on purpose. qtmaildir is the only thing that runs it
programmatically, and two of its behaviours were live bugs here, so it
belongs in this repo where the constraints on it are.
It printed nothing a caller could see. Everything was redirected to
$LOGFILE, so MailSync captured an empty stream and the sync log pane
stayed blank, a known limitation carried since 0.1.0. It now writes to
both through tee: cron keeps its log, and the pane has something to show.
It also ended in an unconditional exit 0, discarding statuses it had
already computed. qtmaildir believes that exit code: a failed mbsync
reported success, cleared the unsynced-changes count, and would have
allowed a sync-on-exit to quit over a sync that never happened, which is
exactly the case the exit prompt exists to prevent. It now exits with
mbsync's status, or notmuch's when mbsync succeeded.
The statuses move through files rather than shell variables because the
run block is piped into tee, which puts it in a subshell where an
assignment does not survive.
Verified against stub mbsync and notmuch binaries in a fake HOME, so no
mail or database was touched: output reaches stdout, the log still gets
every line, and a failing mbsync or notmuch produces its own exit code
rather than zero.
Both properties are now recorded in CLAUDE.md, since they exist for this
application's sake and an edit that looks like a cleanup would remove
them.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Arbitrary tagging from the UI and the unsynced-edits work are both new
features, so this takes the minor slot.
The SlackBuild's MD5SUM stays at the previous value until the 0.7.0
snapshot tarball exists, which is after the tag is pushed. Update it with
sbodl before building the package.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Reported by the user: in the Edit tags fields, the first tag suggested
existing tags and the second did not. Typing a comma, a space and a
letter offered nothing.
QLineEdit::setCompleter hands completion to the line edit, which
overwrites the completer's prefix with the widget's ENTIRE text on every
keystroke. These fields hold a comma-separated list, so once one reads
"unread, fl" that whole string is matched against the tag names, nothing
matches, and completion silently stops after the first tag. Confirmed
with a probe: the prefix really is "unread, fl" and the completion count
really is zero.
Attach with setWidget instead, which keeps the popup anchored without
ceding control of the prefix, and drive it from the token under the
cursor on every edit. Setting the prefix from a textEdited handler while
leaving setCompleter in place does NOT work, which was the first attempt:
the line edit sets it again afterwards.
Accepting a candidate needed the same treatment, and is the other half of
the fix. QCompleter's own insertion replaces the whole field, so taking
"flagged" from the popup would have discarded every tag already typed.
replaceCurrentToken() overwrites only the token under the cursor and
keeps the separator's spacing, so the result is "unread, flagged" rather
than "unread,flagged".
This is the same defect QueryCompleter hit in c98b179. Having now cost
two debugging rounds, it is written into CLAUDE.md as a Qt trap rather
than a property of either class, together with the reason a test using
setText() passes against it: setText does not drive a completer at all,
so the keys have to be typed.
Both new tests were confirmed to fail against setCompleter before the fix
was kept.
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>
|
|
The user asked how to add a tag to a message and the answer is that you
cannot: every tagging action writes a hardcoded name, so archive, delete,
spam, flag and toggle_unread are the only tags reachable from the UI. For
an application whose purpose is organising mail by tag, that is a hole
worth its own item rather than being folded into item 25.
The plumbing is already there. tagSelected() takes arbitrary add and
remove lists, applyTagsToThreads() handles multi-row selections in one
query, undo works, and the completer already holds every tag in the
database, which is what makes completing the dialog's input the obvious
guard against creating 'shoppping' beside 'shopping'.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Right-click actions on the thread list, and select-all for bulk tagging.
Item 24 records that there is no context menu anywhere in the
application: no contextMenuEvent override and no CustomContextMenu policy
in src/. The actions themselves already exist as QActions, so the work is
presentation, with one trap worth naming: right-clicking does not change
the selection in Qt, so a menu built naively would act on the row under
the cursor while the user is looking at several selected rows.
Item 25 is smaller than it sounds, and the entry says why. The thread
view is already ExtendedSelection and tagSelected() already acts on every
selected row through one combined query, so Ctrl+click bulk tagging works
today. What is missing is a select_all action, which does not exist for
the list, and any indication that multi-select is possible at all. It
also carries a caution: select-all over a 10k-thread query turns a rare
accident into a routine keystroke, and the combined-query design should
be measured against a real query before the binding ships rather than
assumed to scale.
Both are the user's own observations; the plan did not carry either.
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>
|
|
Four items from the user's own notes that this document did not carry.
Item 5 gains two concrete sub-items: a star column for flagged threads,
mirroring the paperclip, and the observation that everything looks
unread. The second is recorded with a warning rather than as a defect,
because bold is already conditional on isUnread() in the model; either
the list really is mostly unread or bold leaks through some other path,
and that has to be reproduced before the font logic is touched.
Item 21, better default shortcuts, records why the defaults moved once
already: a bare letter cannot be a menu accelerator, and a bare capital
parses to a key no keystroke emits. Both still constrain a second pass,
as does the rule against testing reachability with synthetic input.
Item 22 is the translatability audit CLAUDE.md has owed since the tr()
rule was written, plus the loading machinery, which does not exist at
all: no QTranslator, no .ts files, no CMake rule. Those are separate
sizes and the item says so.
Item 23, saving a query from the UI, carries the one real design question
with it: whether the write lands in the hand-edited config or the state
file. Saved queries are user intent rather than machine state, which
argues against the split item 1 established, so the decision is flagged
rather than assumed. It also notes that item 23 may answer postponed item
10 as a side effect, since account-scoped saved queries were exactly what
item 10 proposed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Message details and automatic mark-read are both new features, so this
takes the minor slot.
The SlackBuild's MD5SUM stays at the previous value until the 0.6.0
snapshot tarball exists, which is after the tag is pushed. Update it with
sbodl before building the package.
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>
|
|
MimeParser has filled To and Cc all along and HtmlBuilder simply never
interpolated them, so both were parsed on every message and then
discarded. The header strip showed the subject and a message count and
nothing else, which is item 2 of the usability backlog.
The header now adapts to what it can say honestly. A thread holding one
message shows From, To and Cc under the subject, where every field is
unambiguous. A thread holding several keeps showing the subject and the
count alone: the recipient differs message to message, and once the user
has replied there is no single address the thread is addressed to, so
naming one would be a guess presented as a fact. Per-message detail is
what the dialog is for.
That dialog lists Subject, From, To, Cc, Date and Message-Id for every
message, numbered when there is more than one, in a read-only plain-text
widget. Plain text is the security decision, not a stylistic one: these
values come from strangers and the dialog exists to show them verbatim,
so the format that cannot interpret markup is the right one. The header
label is RichText and every value interpolated into it is escaped, since
an unescaped From injects into the application's own chrome rather than
into the sandboxed page.
Reached by a Details... button beside the subject and by Ctrl+Shift+D.
Both, because a shortcut alone restates the complaint this backlog opened
with. The binding is shifted because Ctrl+D is delete, and the
destructive action keeps the key it already had rather than being moved
to make room.
An empty Cc omits its row instead of printing a label with nothing after
it. Both header shapes were rendered to PNG and inspected, not only
asserted.
The new button also exposed a latent flaw in an older test:
attachmentButtonLabels() identified attachment buttons by excluding the
one other button's label, so it counted the details button as an
attachment as soon as one existed. It now finds the bar by object name
and reads only its children.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Item 2 (message details) gains the user's decision on where the details
go. The header adapts to what it can say honestly: a single-message
thread shows From, To, Cc and Subject, while a multi-message thread keeps
showing only subject and count. Everything else moves to a popup behind a
button on the right of the header.
An earlier draft also put a recipient line on the thread header, which
forced a choice between the union of recipients and their intersection
and would have needed real address parsing to compute either. The user
called that overcomplicating and dropped it, so no address parsing is
needed and the item is UI work over strings MimeParser already fills.
The item's own "check before building" question is answered in place: To
and Cc are parsed at mimeparser.cpp:344-345 and then dropped at the
renderer, which never interpolates them. The larger task it warned about
does not exist.
Item 10 is postponed at the user's request rather than dropped: the
complaint was real and the cheap first fix it proposes still stands, it
is simply not wanted now. Only its startup-query half ever shipped.
Items 18 and 19 come from the user's own notes and were missing here: a
visual cue for unsynced edits, and a sync-on-exit prompt with a config
option. 18 records a finding that shapes both: the QUndoStack looks like
a record of pending edits but is cleared on every query, so it cannot
drive the indicator and a separate counter is needed.
Item 20 records, unspecified, that the user's mental model of the thread
view differs from what was built. Nothing is designed there yet; the next
step is asking what they pictured.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Query completion is a new feature, so this takes the minor slot rather
than a patch, the accompanying bug fixes notwithstanding.
The SlackBuild's MD5SUM is left at the 0.4.1 value on purpose: it can
only be computed once the 0.5.0 snapshot tarball exists, which is after
the tag is pushed. Update it with sbodl before the package is built.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
The delegate split each row evenly between the value and its
description. That reads as fair but spends the width on the wrong
column: the values are short, the longest prefix being "attachment:",
while the descriptions are ordinary prose. Measured against the built-in
vocabulary, ten of fifteen descriptions elided at a 400px popup and the
longest, "directory below the Maildir root", needed a ~650px popup to
appear in full, all while the value's half of the row sat mostly empty.
The descriptions are the whole reason the popup teaches the query
language, so a half they cannot use is a half wasted.
Measure the value and lend the description the remainder, capped at 65%
of the row. Every built-in description now fits at ~500px instead of
~700px. The cap is what keeps a genuinely long value legible, and the
value's own reservation is capped in turn so that
"application/vnd.oasis.opendocument.text" elides itself rather than
claiming the row and silencing the column that explains what it is.
The test renders the real popup and reads the pixels back, since the
delegate is private to the .cpp and legibility is a painting question.
It asserts the painted width against the width the text needs unelided,
not merely that something was drawn: a blank-or-not check passes against
the very rule this replaces. 550px is chosen deliberately, being a width
where the new rule paints the longest description in full and an even
split cannot; against the old code it fails with "painted 261px of the
285px it needs".
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
The configuration block is presented as something to copy, and every
optional key in it is commented out with ';' except two. Copying the
block therefore activated whatever those two happened to say: three
extra mimetypes nobody chose, and a sync command pointing at a path
that does not exist on the reader's machine.
Neither was harmful (extra_mimetypes only appends to the built-in list,
and an unresolvable sync command just disables the button with a
tooltip), but both are config acquired by accident rather than by
decision.
Comment them out verbatim. The extra_mimetypes line in particular has
to stay legible rather than be deleted: it is the only worked example
of the '|' and ',' separators, and the reason those two characters
differ is subtle enough that the prose above it needs the illustration.
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>
|
|
Pressing Tab or an arrow key with the completion popup open crashed the
application outright.
QCoreApplication::sendEvent re-runs APPLICATION-level event filters. The
navigation branch forwarded the key to the popup from inside a filter
installed on qApp, so the very same event came back to the filter that
had just sent it. The popup was still visible, the popupVisible() guard
still passed, and it forwarded again: unbounded recursion ending in a
stack overflow rather than in any diagnosable error. Reproduced at 9176
recursive QueryCompleter::eventFilter frames, with a standalone Qt probe
confirming the re-entry independently.
Guard the filter with m_forwarding, checked before the switch so it
covers every branch rather than the navigation keys alone. A key the
filter is itself redelivering now falls through to the popup instead of
being claimed a second time.
The existing tests missed this because they exercised the accept path
without ever forwarding an event. arrowNavigationDoesNotRecurse drives
Down through the grabbing popup and asserts the selection actually
moved, so it fails on a fix that merely swallows the key; against the
old code it takes the process down with it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Showing the popup takes focus away from the query bar and the popup
window grabs the keyboard, so keys pressed while it is up are delivered
to the popup. An event filter installed on the line edit therefore never
ran at the one moment it had to, leaving Tab to move focus to the next
widget and Return to reach the thread list and open a message.
Install the key filter on the application instead, which sees events
before any widget receives them. It returns immediately unless our own
popup is visible, so it cannot affect keyboard handling elsewhere. The
FocusIn filter stays on the line edit, where it is correctly scoped: it
only fires with the popup down.
Accepting a completion now also reopens the popup when the caret lands
somewhere more can be offered, so taking "tag:" goes straight on to the
tag list instead of needing a second complete_query. The chain stops on
a stem that is already a complete candidate, which is what every accept
produces. The mouse path chains identically.
The previous tests passed against the broken code because they posted
events straight to the line edit, bypassing the delivery path a real
keypress takes. The new tests route keys through the active popup and
run against a real X display; offscreen does not grab the keyboard and
cannot reproduce this class of bug.
|
|
QLineEdit::setCompleter hands completion to the line edit, which then
resets the completer's completionPrefix to the widget's entire text on
every keystroke. The prefix has to be the stem, so once the query grew
past its first token the whole-line prefix matched no candidate, the
popup stopped appearing, and the two reported symptoms followed: nothing
was there for Tab to accept, and Tab fell through to focus navigation.
Attach the completer with setWidget instead, which keeps the popup
anchored without ceding control of the prefix. complete() dereferences
widget() unconditionally, so leaving it unset segfaults rather than
degrading. Opening the popup then becomes ours to do on every edit.
Extend the existing event filter to route the keys the popup needs while
it is visible, and install it unconditionally now that it does more than
the completion_on_focus case. Enter accepts a completion only while the
popup is up, so returnPressed still runs the query when it is closed.
The existing tests called acceptCompletion() directly and so never
touched the widget, which is why neither bug was caught. Add four tests
that drive the real widget path plus one covering both settings of
completion_on_focus; the first two fail against the old code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Records what shipped rather than what the plan drafted: path: is the only
prefix beyond tag:/is:/date:/mimetype: that offers values, and completion_on_focus
defaults to false.
The extra_mimetypes syntax needs both separators explained, since ',' splitting
is QSettings' own behaviour and '|' exists only because a description may contain
a comma.
|
|
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.
|
|
The free-form date hint is a footer label rather than a model row: a row
would be filtered away by the first non-matching keystroke and could be
selected and inserted, producing a query that errors.
QCompleter::setPopup takes a QAbstractItemView, so the label cannot be laid
out beside the view in a container widget. The footer sits in space reserved
with setViewportMargins inside the list view instead.
setItemDelegate must run after setPopup, not before: setPopup installs a
plain QStyledItemDelegate of its own and discards whatever was already set,
which silently drops the description column.
Accepting replaces exactly the span the tokenizer identified rather than
QCompleter's own completion prefix, which is a different span once a prefix
or a range bound is involved. Four tests drive that path directly instead of
through synthetic key events, since whether a key needs Shift is a
keyboard-layout property and could not decide the question.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
tag: and is: share the tag list, since notmuch aliases them. path: offers
each account maildir in both bare and recursive forms, the latter being
what Account::scopedQuery builds and not something a user would guess.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Mimetypes are the one completion list with no enumerator, so the user can
extend it. Entries append to the built-ins and a malformed one is skipped
with a problem recorded rather than dropping the whole list.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
KeyMap::defaultBindings() is the single source of truth for shortcuts:
the menus, the shortcut reference dialog and loadDefaults() all read it,
so registering here makes the binding appear in the reference and stay
rebindable from [keys] without any of them disagreeing.
Ctrl+Space is a modifier plus a named key, so it sidesteps the bare
capital trap in normalizeSequence() and needs no Shift on any layout.
Verified it parses to a single non-empty combination that round-trips to
"Ctrl+Space", and it collides with no existing default.
Note test_mainwindow now fails its everyKnownActionIsRegistered()
assertion: MainWindow does not yet implement complete_query. That wiring
is a separate task, and the assertion firing is the intended signal.
|
|
Keywords are syntax and stay untranslated; the descriptions beside them are
prose and go through tr().
The tables are free functions with no QObject to inherit tr() from, so the
file declares a VocabularyStrings context with Q_DECLARE_TR_FUNCTIONS rather
than borrowing QObject::tr, which would file every string under the QObject
context.
|
|
Query bar completion cannot offer tag names without a way to enumerate
them, and libnotmuch had no call wired up for it. Follows the existing
generation-counter pattern; the result crosses the thread boundary as a
QStringList.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
date:las..today put the caret at the end of the lower bound, where the
whole-bound span and the typed-so-far span are both 3 characters long, so
the test passed even against a tokenizer that truncates at the caret and
never sees the range separator. Moving the caret mid-bound with
date:lastweek..today separates the two: 8 under correct code, 3 under the
truncating one. Mutation-verified in both directions.
|
|
Each side of '..' is an independent value against the same model. Entries
that are themselves ranges are withheld once a range exists, since
date:1week....today is malformed.
The token is read to its full extent rather than truncated at the caret:
the separator deciding which bound is being edited can sit to the right
of the caret. Stems stay caret-bounded so matching never uses untyped
text.
|
|
subject:"foo bar has the cursor in free text, where offering keywords
would be wrong.
|
|
The replace span covers the value only, so accepting a completion never
overwrites the prefix that selected it.
|
|
Prefix completion only so far: the token under the cursor, bounded by
whitespace or an opening parenthesis rather than by the start of the line.
|
