The keybinding parser validated strings, not whether any producer could emit the event. So config.rs shipped ("mouse:dblclick(sidebar-row)", "AttachHighlighted") as a default binding that could never fire — and nothing failed, because InputKey::parse was happy with it.
MouseKind::DblClick appeared only in the enum and in parse/format; canonicalise emits Down/Drag/ScrollUp/ScrollDown and nothing else. Adds a ClickTracker that promotes a second click at the same cell within 400ms.
canonicalise stays a pure function of one event — it is well tested and making it stateful for this would have been a shame. The tracker is separate and takes now: Instant as a parameter, so the timing rule is tested without sleeping: fast-second-click promotes, slow one doesn't, different cell doesn't, and a promoted click resets the tracker so three rapid clicks are click / double / click rather than two doubles.
Region::Sidebar and Region::SplitDivider are deleted. region_at can only return Modal, Tab, SidebarRow, Pane or Unknown, so the parser was accepting region names the renderer cannot produce. It now rejects them, with a test that pins exactly that.
Shift-Tab is wired to overlays::prev_field, which existed with no production caller — the spawn modal could only be cycled forwards. crossterm reports Shift-Tab as KeyCode::BackTab, not a modified Tab (my first attempt got this wrong), so InputKey gains a BackTab variant which also forwards \x1b[Z to the pane in Insert mode.
Left alone deliberately:Action::FocusPane(Direction) parses, displays and tests a direction the dispatcher discards. The review flagged it, but that is v1 having one pane per tab — a documented limitation, not a defect. Rejecting directions would break configs for no gain.
Verification
cargo fmt --all -- --check ok
cargo clippy --all-targets -- -D warnings 0
cargo test --all 317 passed, 0 failed
pnpm -C web test 36 passed
Ninth and last of the architecture review series (#40–#47 merged).
The keybinding parser validated *strings*, not whether any producer could emit the event. So `config.rs` shipped `("mouse:dblclick(sidebar-row)", "AttachHighlighted")` as a **default binding that could never fire** — and nothing failed, because `InputKey::parse` was happy with it.
**`MouseKind::DblClick`** appeared only in the enum and in parse/format; `canonicalise` emits `Down`/`Drag`/`ScrollUp`/`ScrollDown` and nothing else. Adds a `ClickTracker` that promotes a second click at the same cell within 400ms.
`canonicalise` stays a pure function of one event — it is well tested and making it stateful for this would have been a shame. The tracker is separate and takes `now: Instant` as a parameter, so the timing rule is tested without sleeping: fast-second-click promotes, slow one doesn't, different cell doesn't, and a promoted click resets the tracker so three rapid clicks are click / double / click rather than two doubles.
**`Region::Sidebar` and `Region::SplitDivider`** are deleted. `region_at` can only return `Modal`, `Tab`, `SidebarRow`, `Pane` or `Unknown`, so the parser was accepting region names the renderer cannot produce. It now rejects them, with a test that pins exactly that.
**Shift-Tab** is wired to `overlays::prev_field`, which existed with no production caller — the spawn modal could only be cycled forwards. crossterm reports Shift-Tab as `KeyCode::BackTab`, not a modified `Tab` (my first attempt got this wrong), so `InputKey` gains a `BackTab` variant which also forwards `\x1b[Z` to the pane in Insert mode.
**Left alone deliberately:** `Action::FocusPane(Direction)` parses, displays and tests a direction the dispatcher discards. The review flagged it, but that is v1 having one pane per tab — a documented limitation, not a defect. Rejecting directions would break configs for no gain.
<details>
<summary>Verification</summary>
```
cargo fmt --all -- --check ok
cargo clippy --all-targets -- -D warnings 0
cargo test --all 317 passed, 0 failed
pnpm -C web test 36 passed
```
</details>
**Ninth and last** of the architecture review series (#40–#47 merged).
The keybinding parser validated strings, not whether any producer could
emit the event — so the shipped default `mouse:dblclick(sidebar-row)`
could never fire, and nothing failed.
`MouseKind::DblClick` appeared only in the enum and in parse/format;
`canonicalise` emits Down/Drag/ScrollUp/ScrollDown. Adds a `ClickTracker`
that promotes a second click at the same cell within 400ms, keeping
`canonicalise` a pure function of one event — the tracker takes `now` as a
parameter, so the timing rule is tested without sleeping.
`Region::Sidebar` and `Region::SplitDivider` are deleted: `region_at` can
only return Modal, Tab, SidebarRow, Pane or Unknown, so the parser was
accepting names the renderer cannot produce. It now rejects them.
Shift-Tab is wired to `overlays::prev_field`, which existed with no
caller, leaving the spawn modal cycleable in one direction only. crossterm
reports it as `KeyCode::BackTab` rather than a modified Tab, so `InputKey`
gains a `BackTab` variant that also forwards `\x1b[Z` to the pane.
Left alone: `Action::FocusPane(Direction)` discards its direction, but
that is v1 having one pane per tab, not a defect.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A drag leaves last intact, so a click, drag, and another click at the original cell within 400 ms becomes dblclick. Clear the pending click when a drag (or any intervening mouse event) arrives.
A drag leaves `last` intact, so a click, drag, and another click at the original cell within 400 ms becomes `dblclick`. Clear the pending click when a drag (or any intervening mouse event) arrives.
<!-- agent-meta: {"model":"gpt-5.6","session":"01a029c5"} -->
promote() early-returned for any non-click and left last intact, so click -> drag -> click at the same cell inside the window came back as dblclick. A scroll or a keystroke between the two did the same thing.
Fixed in #49: any non-click input clears the pending click. Three regression tests cover drag, keystroke and scroll.
One trade-off I'd rather state than hide: a double-click where the mouse jitters enough for the terminal to emit a Drag between the two downs now registers as two clicks. That is the right way round — a spurious dblclick fires AttachHighlighted and opens a session, whereas a missed one costs a second click.
Good catch — that's a real bug and it was mine.
`promote()` early-returned for any non-click and left `last` intact, so click -> drag -> click at the same cell inside the window came back as `dblclick`. A scroll or a keystroke between the two did the same thing.
Fixed in #49: any non-click input clears the pending click. Three regression tests cover drag, keystroke and scroll.
One trade-off I'd rather state than hide: a double-click where the mouse jitters enough for the terminal to emit a `Drag` between the two downs now registers as two clicks. That is the right way round — a spurious `dblclick` fires `AttachHighlighted` and opens a session, whereas a missed one costs a second click.
This repo is archived. You cannot comment on pull requests.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
The keybinding parser validated strings, not whether any producer could emit the event. So
config.rsshipped("mouse:dblclick(sidebar-row)", "AttachHighlighted")as a default binding that could never fire — and nothing failed, becauseInputKey::parsewas happy with it.MouseKind::DblClickappeared only in the enum and in parse/format;canonicaliseemitsDown/Drag/ScrollUp/ScrollDownand nothing else. Adds aClickTrackerthat promotes a second click at the same cell within 400ms.canonicalisestays a pure function of one event — it is well tested and making it stateful for this would have been a shame. The tracker is separate and takesnow: Instantas a parameter, so the timing rule is tested without sleeping: fast-second-click promotes, slow one doesn't, different cell doesn't, and a promoted click resets the tracker so three rapid clicks are click / double / click rather than two doubles.Region::SidebarandRegion::SplitDividerare deleted.region_atcan only returnModal,Tab,SidebarRow,PaneorUnknown, so the parser was accepting region names the renderer cannot produce. It now rejects them, with a test that pins exactly that.Shift-Tab is wired to
overlays::prev_field, which existed with no production caller — the spawn modal could only be cycled forwards. crossterm reports Shift-Tab asKeyCode::BackTab, not a modifiedTab(my first attempt got this wrong), soInputKeygains aBackTabvariant which also forwards\x1b[Zto the pane in Insert mode.Left alone deliberately:
Action::FocusPane(Direction)parses, displays and tests a direction the dispatcher discards. The review flagged it, but that is v1 having one pane per tab — a documented limitation, not a defect. Rejecting directions would break configs for no gain.Verification
Ninth and last of the architecture review series (#40–#47 merged).
@@ -291,0 +318,4 @@row,} = keyelse {return key;A drag leaves
lastintact, so a click, drag, and another click at the original cell within 400 ms becomesdblclick. Clear the pending click when a drag (or any intervening mouse event) arrives.Good catch — that's a real bug and it was mine.
promote()early-returned for any non-click and leftlastintact, so click -> drag -> click at the same cell inside the window came back asdblclick. A scroll or a keystroke between the two did the same thing.Fixed in #49: any non-click input clears the pending click. Three regression tests cover drag, keystroke and scroll.
One trade-off I'd rather state than hide: a double-click where the mouse jitters enough for the terminal to emit a
Dragbetween the two downs now registers as two clicks. That is the right way round — a spuriousdblclickfiresAttachHighlightedand opens a session, whereas a missed one costs a second click.