Skip to content

fix: dedupe terminal pane focus + selection into engine method (#429) - #445

Merged
JDonaghy merged 1 commit into
developfrom
issue-429-terminal-pane-click-dedup
May 17, 2026
Merged

fix: dedupe terminal pane focus + selection into engine method (#429)#445
JDonaghy merged 1 commit into
developfrom
issue-429-terminal-pane-click-dedup

Conversation

@JDonaghy

Copy link
Copy Markdown
Owner

Summary

Both backends were inlining the same three steps after a content click on a non-split terminal pane:

```rust
engine.terminal_has_focus = true;
engine.terminal_scroll_reset();
if let Some(term) = engine.active_terminal_mut() {
term.selection = Some(TermSelection { ..click_cell });
}
```

Plus `handle_terminal_split_click` did the same work twice (once for `LeftPane`, once for `RightPane`) — its body literally repeated the three-step sequence after toggling `terminal_active`.

Extract `Engine::handle_terminal_pane_click(col, row)` as the single source of truth:

  • TUI mouse handler consumes it in the non-split fallback (`mouse.rs:2069` → 5 lines collapsed to 1 engine call).
  • `handle_terminal_split_click` LeftPane / RightPane arms delegate after setting `terminal_active`.

Net: `terminal_ops.rs` +3 / -3 (collapsed both arms), `tui_main/mouse.rs` -11 / -3.

Out of scope

GTK consumption (`src/gtk/mod.rs:6798-6811` has the same inline pattern) is a follow-up per the issue brief. Engine method is ready for GTK to consume in 1 line when that lands.

Test plan

  • `cargo build`
  • `cargo test --no-default-features --lib` — 1966 pass (no regressions). No new tests: the helper does 3 trivial state mutations and is covered transitively by the existing split-click smoke paths.
  • `cargo fmt --check` on touched files
  • TUI smoke (user-tested on server):
    • Click in non-split terminal — focuses, resets scroll, starts selection ✓
    • Click in left split pane — activates pane 0 + selection ✓
    • Click in right split pane — activates pane 1 + selection ✓
    • Found a pre-existing drag bug on right split (selection extends to wrong column) — not caused by this PR, filed as TUI right-split terminal: drag-select extends to wrong column (editor-relative vs pane-relative mismatch) #444. My PR preserves the existing click coord convention; the bug is in drag, which uses editor-relative col while click uses pane-relative.

Refs #429

🤖 Generated with Claude Code

Both backends were inlining the same three steps after a content click
on a non-split terminal pane:

  engine.terminal_has_focus = true;
  engine.terminal_scroll_reset();
  if let Some(term) = engine.active_terminal_mut() {
      term.selection = Some(TermSelection { ..click_cell });
  }

Plus `handle_terminal_split_click` did the same work twice (once for
LeftPane, once for RightPane).

Extract `Engine::handle_terminal_pane_click(col, row)` as the single
source of truth. TUI mouse handler consumes it in the non-split
fallback (`mouse.rs:2069`). The split method's LeftPane / RightPane
arms now delegate to it after setting `terminal_active`.

Per the issue scope, GTK consumption (mod.rs:6798-6811) is a follow-up;
no GTK changes in this PR.

Refs #429

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@JDonaghy
JDonaghy merged commit e3bedf8 into develop May 17, 2026
@JDonaghy
JDonaghy deleted the issue-429-terminal-pane-click-dedup branch May 17, 2026 15:29
JDonaghy added a commit that referenced this pull request Jul 18, 2026
…ndor

quadraui#452 vendored vt100 0.16.2 with its unicode-width bound loosened
(vendor/vt100-0.16.2-patched/) and patches it via [patch.crates-io] in
quadraui's OWN Cargo.toml to resolve a conflict with ratatui 0.29's exact
unicode-width=0.2.0 pin. But Cargo only honors [patch] sections declared
in the root manifest of the workspace actually being built — quadraui's
patch has zero effect on vimcode's separate build, so vimcode's own
Cargo.toml needs a matching [patch.crates-io] entry pointing at the same
vendored copy. Confirmed by quadraui#452's own commit message, which
explicitly flagged this vimcode-side companion change as needed and out
of scope for that session.

With this, `cargo build --features gui` gets past dependency resolution
(previously failed here with a unicode-width version conflict) — but
still does not succeed end-to-end. It now hits a separate, unrelated,
much larger issue: quadraui's `tui` feature currently pulls in ratatui
0.30 (bumped independently of #452 or #445, at some point between
quadraui ebb4ab7 and current develop), which split Frame/Buffer/Rect
into a new `ratatui-core` crate. vimcode's own TUI rendering code
(src/tui_main/panels.rs, render_impl.rs, quadraui_tui.rs, mod.rs — ~46
call sites total) is still written directly against ratatui 0.29's
types and is incompatible with quadraui's now-ratatui-core-based
`Backend::enter_frame_scope` signature. That's a real, separate
migration (bump vimcode's own ratatui pin and update every touched
call site) — tracked separately, not attempted here.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
JDonaghy added a commit that referenced this pull request Jul 19, 2026
The #587 regression (GTK global accelerators — Ctrl+Shift+P command
palette, Ctrl+B sidebar, Ctrl+P quick-open — silently do nothing) has
repeatedly slipped through Test-stage smoke as a build-against-stale-
quadraui trap: quadraui#445's accelerator-dispatch fix is a behavioural
change with no new public symbol, so a checkout that predates it still
compiles vimcode cleanly and only fails at runtime.

The vimcode side of #587 is already complete and correct on this branch
(register_panel_accelerators wired into ShellApp::setup, UiEvent::
Accelerator handled in ShellApp::handle) — nothing there needed changing.
The remaining failure mode was purely environmental: an artifact built
against a quadraui checkout lacking #445.

Add a build.rs guard that reads the sibling quadraui GTK key-controller
source and hard-fails the build with actionable remediation if it does
not call `match_keypress` (the Backend method #445 consults on the live
key path). Converts the silent runtime breakage into a loud build error.
Falls back to a warning (never a false failure) if the path dep points
elsewhere. Verified: passes against quadraui develop tip (has #445);
full lib suite green on both gui and default/TUI feature sets.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
JDonaghy added a commit that referenced this pull request Jul 20, 2026
…ndor

quadraui#452 vendored vt100 0.16.2 with its unicode-width bound loosened
(vendor/vt100-0.16.2-patched/) and patches it via [patch.crates-io] in
quadraui's OWN Cargo.toml to resolve a conflict with ratatui 0.29's exact
unicode-width=0.2.0 pin. But Cargo only honors [patch] sections declared
in the root manifest of the workspace actually being built — quadraui's
patch has zero effect on vimcode's separate build, so vimcode's own
Cargo.toml needs a matching [patch.crates-io] entry pointing at the same
vendored copy. Confirmed by quadraui#452's own commit message, which
explicitly flagged this vimcode-side companion change as needed and out
of scope for that session.

With this, `cargo build --features gui` gets past dependency resolution
(previously failed here with a unicode-width version conflict) — but
still does not succeed end-to-end. It now hits a separate, unrelated,
much larger issue: quadraui's `tui` feature currently pulls in ratatui
0.30 (bumped independently of #452 or #445, at some point between
quadraui ebb4ab7 and current develop), which split Frame/Buffer/Rect
into a new `ratatui-core` crate. vimcode's own TUI rendering code
(src/tui_main/panels.rs, render_impl.rs, quadraui_tui.rs, mod.rs — ~46
call sites total) is still written directly against ratatui 0.29's
types and is incompatible with quadraui's now-ratatui-core-based
`Backend::enter_frame_scope` signature. That's a real, separate
migration (bump vimcode's own ratatui pin and update every touched
call site) — tracked separately, not attempted here.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
JDonaghy added a commit that referenced this pull request Jul 20, 2026
The #587 regression (GTK global accelerators — Ctrl+Shift+P command
palette, Ctrl+B sidebar, Ctrl+P quick-open — silently do nothing) has
repeatedly slipped through Test-stage smoke as a build-against-stale-
quadraui trap: quadraui#445's accelerator-dispatch fix is a behavioural
change with no new public symbol, so a checkout that predates it still
compiles vimcode cleanly and only fails at runtime.

The vimcode side of #587 is already complete and correct on this branch
(register_panel_accelerators wired into ShellApp::setup, UiEvent::
Accelerator handled in ShellApp::handle) — nothing there needed changing.
The remaining failure mode was purely environmental: an artifact built
against a quadraui checkout lacking #445.

Add a build.rs guard that reads the sibling quadraui GTK key-controller
source and hard-fails the build with actionable remediation if it does
not call `match_keypress` (the Backend method #445 consults on the live
key path). Converts the silent runtime breakage into a loud build error.
Falls back to a warning (never a false failure) if the path dep points
elsewhere. Verified: passes against quadraui develop tip (has #445);
full lib suite green on both gui and default/TUI feature sets.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
JDonaghy added a commit that referenced this pull request Jul 20, 2026
…ndor

quadraui#452 vendored vt100 0.16.2 with its unicode-width bound loosened
(vendor/vt100-0.16.2-patched/) and patches it via [patch.crates-io] in
quadraui's OWN Cargo.toml to resolve a conflict with ratatui 0.29's exact
unicode-width=0.2.0 pin. But Cargo only honors [patch] sections declared
in the root manifest of the workspace actually being built — quadraui's
patch has zero effect on vimcode's separate build, so vimcode's own
Cargo.toml needs a matching [patch.crates-io] entry pointing at the same
vendored copy. Confirmed by quadraui#452's own commit message, which
explicitly flagged this vimcode-side companion change as needed and out
of scope for that session.

With this, `cargo build --features gui` gets past dependency resolution
(previously failed here with a unicode-width version conflict) — but
still does not succeed end-to-end. It now hits a separate, unrelated,
much larger issue: quadraui's `tui` feature currently pulls in ratatui
0.30 (bumped independently of #452 or #445, at some point between
quadraui ebb4ab7 and current develop), which split Frame/Buffer/Rect
into a new `ratatui-core` crate. vimcode's own TUI rendering code
(src/tui_main/panels.rs, render_impl.rs, quadraui_tui.rs, mod.rs — ~46
call sites total) is still written directly against ratatui 0.29's
types and is incompatible with quadraui's now-ratatui-core-based
`Backend::enter_frame_scope` signature. That's a real, separate
migration (bump vimcode's own ratatui pin and update every touched
call site) — tracked separately, not attempted here.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
JDonaghy added a commit that referenced this pull request Jul 20, 2026
The #587 regression (GTK global accelerators — Ctrl+Shift+P command
palette, Ctrl+B sidebar, Ctrl+P quick-open — silently do nothing) has
repeatedly slipped through Test-stage smoke as a build-against-stale-
quadraui trap: quadraui#445's accelerator-dispatch fix is a behavioural
change with no new public symbol, so a checkout that predates it still
compiles vimcode cleanly and only fails at runtime.

The vimcode side of #587 is already complete and correct on this branch
(register_panel_accelerators wired into ShellApp::setup, UiEvent::
Accelerator handled in ShellApp::handle) — nothing there needed changing.
The remaining failure mode was purely environmental: an artifact built
against a quadraui checkout lacking #445.

Add a build.rs guard that reads the sibling quadraui GTK key-controller
source and hard-fails the build with actionable remediation if it does
not call `match_keypress` (the Backend method #445 consults on the live
key path). Converts the silent runtime breakage into a loud build error.
Falls back to a warning (never a false failure) if the path dep points
elsewhere. Verified: passes against quadraui develop tip (has #445);
full lib suite green on both gui and default/TUI feature sets.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant