summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-07 17:52:05 +0200
committerDanilo M. <danix@danix.xyz>2026-08-07 17:52:05 +0200
commit5f77cb8374b983b17268dfd571afa65e2678147b (patch)
treedd9476f9439268b609dc7d41e6ac5055d6b1c376
parent334b510e2673a6ab3875ffa7a4c5b3b2dd09a369 (diff)
downloadqtmaildir-5f77cb8374b983b17268dfd571afa65e2678147b.tar.gz
qtmaildir-5f77cb8374b983b17268dfd571afa65e2678147b.zip
docs: record four items the usage notes had and the backlog did not
The notes are the upstream source and keep growing, so the backlog goes stale on its own between sessions. This pass found three unrecorded entries, each with its cause verified in the code rather than copied from the note: - 49, sync always runs every account regardless of what changed. The account set is not a parameter anywhere on the path, and the shipped script hardcodes the whole-store sweep. Needs a decision first about whether an account key is also an mbsync channel name. - 50, Esc blanks the pane but leaves the row selected. Built as specified by item 32; the user now wants both, as two actions rather than one. - 51, clicking a subject scrolls the list sideways. Ordinary view auto-scroll, cosmetic. 52 came from the work itself rather than from the notes: the suite's verdict depends on how it is invoked. test_querycompleter grabs a popup and Wayland refuses a grabbing popup for a window that has never received input, so it fails under ctest and passes when run with the offscreen platform by hand. It cost a wrong diagnosis on the day it was found, an unrelated change blamed for a failure that reproduces on a clean checkout, because the comparison was run under two different plugins. Recorded with the transientParent fix preferred over forcing the platform, since a test that runs under the real plugin is worth more than one that opts out.
-rw-r--r--docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md168
1 files changed, 168 insertions, 0 deletions
diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md
index e82c8fc..cefdfc3 100644
--- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md
+++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md
@@ -97,6 +97,10 @@ taking that too literally.
| 46 | `uiStateSurvivesARestart` fails under the offscreen platform | testing | XS | **done** |
| 47 | The query bar looks unfinished, and cannot be cleared by mouse | presentation | XS | **done** |
| 48 | Removing a tag suggests every tag, not the thread's own | workflow | XS | **done** |
+| 49 | Sync runs every account regardless of what changed | workflow | M | open |
+| 50 | Esc blanks the pane but leaves the row selected | workflow | XS | open |
+| 51 | Clicking a subject scrolls the list sideways | presentation | XS | open |
+| 52 | `test_querycompleter` fails under Wayland, passes offscreen | testing | XS | open |
Sizes are rough: XS under an hour, S a sitting, M a session.
@@ -2774,6 +2778,170 @@ assert the remove field's completer offers only those. The keys must be typed,
not `setText()`, since `setText` does not drive a completer at all, which
`CLAUDE.md` records.
+## 49. Sync always runs every account, even when one account was touched
+
+**Observed (user, 2026-08-07):** "make the manual and closing sync operation
+work only on affected accounts (it now runs for all accounts, independent from
+actual modifications present). If no modifications exists, assume it's for all
+accounts and run normally."
+
+**Cause (verified in code):** the account set is not a parameter anywhere on the
+path. `MailSync::start()` (`src/mailsync.cpp:170`) takes no arguments; it splits
+the configured `sync_command` string and runs it verbatim
+(`src/mailsync.cpp:177-184`), so the command line is fixed at config time and
+identical for every run. The shipped script then hardcodes the whole-store
+sweep: `mbsync -V -a` (`assets/mailsync.sh:83`), where `-a` means all channels.
+Nothing between the tag edit and mbsync carries which account changed.
+
+**Approach.** Three pieces, and the first is the real work.
+
+- **Track which accounts have unsynced edits.** Item 18's `m_pendingEdits` is a
+ bare count; this needs the set of account tags behind it, accumulated in the
+ same `onTagsApplied()` handler and cleared on the same successful-sync reset.
+ `ThreadSummary` already carries the account tag that `ThreadListModel` renders
+ as the account chip, so the mapping exists and needs no worker query.
+- **Let `start()` take accounts.** An optional `QStringList` appended to the
+ configured command's arguments, empty meaning today's behaviour.
+- **Let the script accept channel names.** `mailsync.sh` passes them to mbsync in
+ place of `-a` when given, keeping `-a` when not.
+
+**Constraints.**
+
+- **Empty set means all accounts, per the user's own wording.** A sync with
+ nothing pending is a fetch, and fetching one account because that is where the
+ last edit happened to be would be wrong.
+- **A notmuch account tag is not necessarily an mbsync channel name.** The
+ config already separates the two ideas (`[account.<key>]` has a notmuch tag and
+ a display `label`); mapping to a channel needs either a new per-account key or
+ an explicit decision that the key IS the channel. Settle this with the user
+ before building, it is the one design question in the item.
+- **`notmuch new` still runs over everything**, and must. Restricting the fetch
+ does not restrict the index.
+- **The script's two shipped properties survive**: it prints to stdout as well as
+ its log, and it exits with the real status. `CLAUDE.md` records why.
+- Item 42's status-bar parsing reads the account name out of `mbsync -V` output;
+ a narrowed run must keep `-V` or that regresses.
+
+**Verification:** by hand, since it ends in a real fetch. Tag in one account,
+sync, and confirm from the log that only that channel ran. Then sync with
+nothing pending and confirm all channels run.
+
+## 50. Esc blanks the pane but leaves the row selected
+
+**Observed (user, 2026-08-07):** "esc in the main window should deselect whatever
+is selected in the left pane while still blanking the right pane. Two actions
+instead of one."
+
+**Cause (verified in code):** deliberate, and now reconsidered. The `clear_pane`
+action (`src/mainwindow.cpp:772`) is bound to `Esc` by default
+(`src/keymap.cpp:88`), and its own comment states the intent: "A view change,
+not a mail change: the selection, the query and the undo stack are all left
+alone." It calls `m_messageView->clear()` and drops `m_currentThreadId`
+(`src/mainwindow.cpp:783`) and touches the selection model not at all. Item 32
+built exactly what was asked for then; the user now wants the selection cleared
+as well.
+
+**Approach.** The note asks for "two actions instead of one", so this is not a
+matter of adding `clearSelection()` to the existing one:
+
+- `clear_pane` keeps its current behaviour and its name, still available to a
+ user who binds it.
+- A second action clears the pane AND the thread list selection, and takes the
+ `Esc` default.
+
+**Constraints.**
+
+- **Clearing the selection must not resurrect the pane.** The selection handler
+ reacts to `selectionChanged`, so clearing it fires that path; confirm it
+ leaves the pane blank rather than re-rendering or repainting a placeholder,
+ and order the two operations accordingly. This is the whole risk in the item.
+- **Interacts with item 30.** Once the blank pane shows a placeholder, "blank"
+ means "show the placeholder", and a multi-selection message is one of the
+ things item 30 specifies the placeholder saying. Build 30 first or the two
+ will disagree about what an empty pane looks like.
+- The in-flight-load guard must keep working: `m_currentThreadId` is cleared
+ with the pane precisely so a `threadLoaded` arriving afterwards is dropped.
+- Both actions need entries in the shortcut reference, which is generated, so
+ the descriptions must distinguish them in one line.
+
+## 51. Clicking a subject scrolls the list sideways
+
+**Observed (user, 2026-08-07):** "when selecting a row by clicking on the subject,
+the table scrolls horizontally to accomodate the whole subject column into view.
+Minor UX."
+
+**Cause (verified in code):** `QAbstractItemView`'s auto-scroll, which is on by
+default and scrolls the clicked index fully into view. The Subject column
+stretches and holds long text, so the view has somewhere to scroll to;
+`src/mainwindow.cpp:548` sets `setHorizontalScrollMode(ScrollPerPixel)`, which
+makes the movement smooth rather than by whole columns but does not cause it.
+Nothing calls `scrollTo()` explicitly, and `setAutoScroll` is not set anywhere.
+
+**Approach.** Cheapest first, and check it is enough before going further:
+`setAutoScroll(false)` on the thread view suppresses the scroll-into-view on
+click. If that proves too blunt, override `scrollTo()` to ignore the horizontal
+component and defer to the base class for the vertical one.
+
+**Constraints.**
+
+- **Keyboard navigation must still scroll vertically.** Arrowing past the bottom
+ of the viewport has to follow the current row, and `setAutoScroll(false)` is
+ the flag that governs that too. Verify with the keyboard, not only the mouse;
+ if it breaks, the `scrollTo()` override is the answer rather than the flag.
+- Drag-select auto-scroll rides on the same flag. Selecting past the edge of the
+ viewport is a real gesture on a list this long.
+- The item is cosmetic and must not grow into column-sizing work. The Subject
+ column being wider than the viewport is the precondition, not the defect.
+
+## 52. `test_querycompleter` fails under Wayland, passes offscreen
+
+**Observed 2026-08-07**, while verifying an unrelated change: `ctest` reports
+`theDescriptionSurvivesAModestPopupWidth` failing, while running the same binary
+directly passes. Reproduced on a clean checkout, so it predates that work.
+
+**Cause (verified):** the test grabs the completer popup and asserts the image
+is not null (`tests/test_querycompleter.cpp:824`). Under the Wayland platform
+plugin the grab returns a null pixmap, with the warning
+
+```
+qt.qpa.wayland: Failed to create grabbing popup. Ensure popup ... has a
+transientParent set and that parent window has received input.
+```
+
+Wayland will not create a grabbing popup for a window that has never received
+input, which a test window has not. `ctest` sets no `QT_QPA_PLATFORM`, so it
+inherits the session's Wayland plugin, whereas a developer running the binary
+by hand usually exports `offscreen` and never sees it.
+
+**Why this matters more than one red test.** The suite's result depends on how
+it is invoked. A test that only passes under a platform plugin nobody sets is
+not protecting anything, and worse, it trains you to read a real failure as
+environmental. It cost a wrong diagnosis on the day it was found: the failure
+was initially attributed to the change in flight, because the clean-tree
+comparison was run under `offscreen` while `ctest` ran under Wayland. Comparing
+two environments and reading the difference as a regression is exactly the
+mistake this item exists to stop repeating.
+
+**Approach.** Pin the platform for the tests that need one rather than for all
+of them, the same shape as item 46:
+
+- `set_tests_properties(... ENVIRONMENT QT_QPA_PLATFORM=offscreen)` for the
+ tests that grab widgets, so `ctest` is deterministic however the session is
+ configured.
+- Preferable if it works: give the popup a `transientParent` and show the parent
+ first, which fixes the grab under Wayland rather than avoiding it. Try this
+ before reaching for the environment override, since a test that can run under
+ the real plugin is worth more than one that opts out.
+
+**Constraints.**
+
+- **Do not simply delete the assertion.** It exists because a null grab is
+ precisely how this class of rendering test silently passes, which `CLAUDE.md`
+ records at length. Weakening it to `if (!shot.isNull())` would make it pass
+ everywhere and check nothing.
+- Whatever is chosen must hold for `test_mainwindow` and `test_messageview` too,
+ which create widgets and could grow the same dependency.
+
## Deferred, unsized, or split out
Items noted while triaging but not part of the original list. Same numbering