Skip to content

refactor: migrate search panel to SidebarSystem (#323, #333, #334) - #342

Merged
JDonaghy merged 3 commits into
developfrom
issue-323-search-sidebar-system
May 10, 2026
Merged

refactor: migrate search panel to SidebarSystem (#323, #333, #334)#342
JDonaghy merged 3 commits into
developfrom
issue-323-search-sidebar-system

Conversation

@JDonaghy

Copy link
Copy Markdown
Owner

Summary

Closes #323, closes #333, closes #334

Test plan

  • TUI: type query → Enter → results grouped by file
  • TUI: j/k navigate results, Enter opens file at match line
  • TUI: click text input → focuses with cursor
  • TUI: click result row → selects, click file header → collapses/expands
  • TUI: scroll wheel + scrollbar track click + thumb drag
  • TUI: Tab/Escape switch form↔results, printable char re-enters form
  • GTK: all of the above
  • GTK: toggle buttons (Aa/Ab|/.*) and action buttons (Find/Repl/All) clickable
  • GTK: Alt+C/W/R shortcuts toggle search options
  • GTK: Ctrl+V paste into focused input
  • Both: toggling case/word/regex auto-reruns search
  • Both: switching to another panel clears search focus

🤖 Generated with Claude Code

JDonaghy and others added 3 commits May 10, 2026 15:07
Replace hand-rolled search panel scroll/selection/collapse with a
2-section SidebarSystem (Form + Tree). Unified key dispatch via
dispatch_search_sidebar_key_unified() eliminates ~120 lines of
per-backend key handling. TUI duplicate focus state removed (#333).
Printable-char re-enters input mode on both backends (#334).

Net -120 lines across 12 files. Toggles auto-rerun search. GTK gets
wire_da_events for scroll/drag, Alt modifier on Msg::KeyPress, and
proper search_has_focus routing in handle_key_press.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Deduplicate 6 identical search panel activation blocks (4 activation,
2 results-switch) across TUI, GTK, and Win-GUI into two shared engine
methods. Eliminates ~40 lines of per-backend copy-paste.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@JDonaghy
JDonaghy merged commit f00161b into develop May 10, 2026
JDonaghy added a commit that referenced this pull request Sep 3, 2026
… both backends

`render::route_terminal_key` states the rung once; both backends call it and
have nothing bespoke left. `render::route_terminal_resize` does the same for
the "window resized → resize the PTYs" half.

**quadraui first (`CLAUDE.md`, and the issue's own note).** quadraui#343 has
landed on the pinned rev (`TerminalSession::paste` /
`bracketed_paste_enabled`), so the bracketed-paste rung is now an *adoption*:
`Engine::terminal_paste` delegates to it instead of wrapping unconditionally,
which stops literal `[200~` bytes leaking into programs that don't strip them.
quadraui#342 (the keyboard→PTY encoder) has **not** landed, so the encoder
stays in `core::engine::terminal_ops::key_to_pty_bytes` — platform-neutral
`core`, not per-backend code. When #342 lands, that fn is the single call site
to swap and neither backend changes.

**What diverged.** TUI hand-rolled ~75 lines inside `handle_key_pressed`. GTK's
twin was deleted outright by the #540 Relm4→ShellApp cutover — the whole
`if engine.borrow().terminal_has_focus { … }` block lived in the `view!`'s
`EventControllerKey` closure and nothing replaced it. Since #540, **every key
typed into a focused terminal on GTK fell through to `Engine::handle_key` and
ran a vim command on the editor buffer** (#471): Ctrl+F opened the editor's
find/replace overlay, `x` deleted a character from the file. The other half of
#471 was that old arm's `sender.input(Msg::Resize)` after each keypress, whose
handler resized *every* pane to the full panel width — in split mode that
reflowed the half-width panes while they were still painted narrow, so freshly
typed text in the right pane wrapped out of view. The router performs no
resize, and `route_terminal_resize` is split-aware, so neither half can return.

Also reconciled: `canonical_terminal_key_name` accepts both backends' spellings
(`PageUp`/`Page_Up`, `BackTab`/`ISO_Left_Tab`, `Enter`/`Return`) and strips the
`Shift_` prefix `translate_key` adds for the editor — which is why the old TUI
arm bypassed `translate_key` and re-derived names from the raw `KeyCode`. That
bypass is gone. The GTK rung sits directly below the focus owners and *above*
the debug F-keys, matching TUI's ladder: `vim`/`htop` inside the terminal must
get F5 themselves.

Black-box coverage, both backends, asserting on rendered output only:
- `terminal_ctrl_f_opens_the_painted_find_bar{,_via_shell_app}` — the painted
  `" FIND: …"` toolbar text (`render::build_terminal_toolbar`), then the typed
  query reaching it. Never `terminal_find_active`.
- `focused_terminal_swallows_editor_keys{_on_gtk,_via_shell_app}` — the painted
  buffer line, with a positive control (same fixture, terminal unfocused, the
  identical key must delete the character).
- `route_terminal_resize_keeps_a_split_split` — the PTYs' own `cols()`, which
  is what the painter reads back.
- `canonical_terminal_key_name_reconciles_both_backends_spellings`.

**Verified RED against unfixed `develop`** (`CLAUDE.md` rule 2): with the two
`route_terminal_key` calls and the `route_terminal_resize` body removed, all
five behaviour tests fail; restored, all pass.

Entry-point production lines (`handle_key_press` + `dispatch_gtk_panel_
accelerator` + `handle_key_pressed` + `handle_focus_owner_key` +
`dispatch_panel_accelerator_sizeless`): **1,423 → 1,389**, a strict decrease
even though GTK *gained* a rung it did not have.

`cargo build && cargo clippy -- -D warnings && cargo fmt --check` EXIT=0.

Beyond the issue's file list: `core/engine/terminal_ops.rs` (new
`Engine::terminal_paste`) and `core/engine/keys.rs` (`route_paste`'s terminal
branch now calls it) — required for the quadraui#343 adoption.

Refs #758, #734, #351, #471, quadraui#342, quadraui#343.

Co-Authored-By: Claude Opus 5 <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