repl refactor #518

Merged
navicore merged 1 commit from refactor-repl into main 2026-05-25 02:30:19 +00:00
Owner

This session's applied changes across the crate: ir_pane.rs (two per-line
styler extractions killing depth-8 nesting), highlight.rs (lex_number
extraction), run.rs (deduped success branch), ir/stack_art.rs (deleted 3
dead methods, re-gated 2 test-only to #[cfg(test)]), completion.rs
(word_start + show_items dedups), layout.rs (format allocation-free
middle). Every change verified clippy-clean with 87 seq-repl tests passing.

This session's applied changes across the crate: ir_pane.rs (two per-line styler extractions killing depth-8 nesting), highlight.rs (lex_number extraction), run.rs (deduped success branch), ir/stack_art.rs (deleted 3 dead methods, re-gated 2 test-only to #[cfg(test)]), completion.rs (word_start + show_items dedups), layout.rs (format allocation-free middle). Every change verified clippy-clean with 87 seq-repl tests passing.
repl refactor
All checks were successful
CI - Linux / CI - Linux x86_64 (pull_request) Successful in 4m1s
94c84e4e60
This session's applied changes across the crate: ir_pane.rs (two per-line
  styler extractions killing depth-8 nesting), highlight.rs (lex_number
  extraction), run.rs (deduped success branch), ir/stack_art.rs (deleted 3
  dead methods, re-gated 2 test-only to #[cfg(test)]), completion.rs
  (word_start + show_items dedups), layout.rs (format allocation-free
  middle). Every change verified clippy-clean with 87 seq-repl tests passing.
Author
Owner

Review — approve, no blockers

Refactor across crates/repl/ (+516/−524, net −8). just ci green on the branch: 87 seq-repl tests + all workspace crates pass. Reviewed each file; all behavior-preserving.

app.rs (the largest change — note: not mentioned in the PR description)

Two refactors, both faithful:

  1. SearchState field grouping — the five search_* fields on App (search_mode, search_pattern, search_matches, search_match_index, search_original_input) collapse into one #[derive(Default)] struct SearchState (active/pattern/matches/match_index/original_input). Both constructors use SearchState::default(). This drives the app/tests.rs churn (pure field renames — no assertion or expected-value changes).
  2. handle_key decomposition — the giant inline match is split into handle_completion_popup_key / handle_search_key / try_enter_search / handle_ctrl_shortcut / handle_function_key / handle_command_line_tab (each returns bool = consumed) plus refresh_search / step_search_match helpers. Verified arm-for-arm:
    • dispatch order unchanged (completion → search → enter-search → ctrl → fn-keys → cmdline-tab → Tab-completion → insert-enter → normal-jk → editor);
    • the completion-popup _ arm and cmdline-tab _ arm correctly return false to preserve the original fall-through;
    • step_search_match keeps preview_current_match()+update_search_status() after the index update, gated by the early return on empty (matches the old if !empty { … });
    • Tab/BackTab/Shift+Tab forward-vs-backward direction preserved.

Other files (agent-surveyed, spot-confirmed)

  • ir/stack_art.rs — 3 deleted methods (StackValue::is_rest, Stack::values, Stack::has_concrete_values) confirmed to have zero references anywhere in the crate; the 2 re-gated from #[allow(dead_code)] to #[cfg(test)] (Stack::empty, render_sequence) are referenced only from #[cfg(test)] code — and CI's non-test release build passing proves the gating is correct.
  • ui/ir_pane.rs — the two per-line styler extractions (style_stack_art_line, compact_line) are verbatim lifts; same char tests → same spans/styles. &String&str param is the only delta.
  • ui/highlight.rs (lex_number), completion.rs (word_start/show_items), run.rs (merged success branch — same stdout-on-success/stderr-on-fail selection), ui/layout.rs (allocation-free middle) — all CLEAN.

Minor note (non-blocking)

app.rs is the biggest change in the PR but isn't in the description (which covers ir_pane/highlight/run/stack_art/completion/layout). Same pattern as #515's ast/program.rs. Worth a line in the description so the diff isn't surprising to a future reader — the change itself is clean.

## Review — approve, no blockers Refactor across `crates/repl/` (+516/−524, net −8). `just ci` green on the branch: 87 seq-repl tests + all workspace crates pass. Reviewed each file; all behavior-preserving. ### `app.rs` (the largest change — note: not mentioned in the PR description) Two refactors, both faithful: 1. **`SearchState` field grouping** — the five `search_*` fields on `App` (`search_mode`, `search_pattern`, `search_matches`, `search_match_index`, `search_original_input`) collapse into one `#[derive(Default)] struct SearchState` (`active`/`pattern`/`matches`/`match_index`/`original_input`). Both constructors use `SearchState::default()`. This drives the `app/tests.rs` churn (pure field renames — no assertion or expected-value changes). 2. **`handle_key` decomposition** — the giant inline `match` is split into `handle_completion_popup_key` / `handle_search_key` / `try_enter_search` / `handle_ctrl_shortcut` / `handle_function_key` / `handle_command_line_tab` (each returns `bool` = consumed) plus `refresh_search` / `step_search_match` helpers. Verified arm-for-arm: - dispatch order unchanged (completion → search → enter-search → ctrl → fn-keys → cmdline-tab → Tab-completion → insert-enter → normal-jk → editor); - the completion-popup `_` arm and cmdline-tab `_` arm correctly return `false` to preserve the original fall-through; - `step_search_match` keeps `preview_current_match()`+`update_search_status()` after the index update, gated by the early `return` on empty (matches the old `if !empty { … }`); - Tab/BackTab/Shift+Tab forward-vs-backward direction preserved. ### Other files (agent-surveyed, spot-confirmed) - **`ir/stack_art.rs`** — 3 deleted methods (`StackValue::is_rest`, `Stack::values`, `Stack::has_concrete_values`) confirmed to have **zero references** anywhere in the crate; the 2 re-gated from `#[allow(dead_code)]` to `#[cfg(test)]` (`Stack::empty`, `render_sequence`) are referenced **only** from `#[cfg(test)]` code — and CI's non-test release build passing proves the gating is correct. - **`ui/ir_pane.rs`** — the two per-line styler extractions (`style_stack_art_line`, `compact_line`) are verbatim lifts; same char tests → same spans/styles. `&String`→`&str` param is the only delta. - **`ui/highlight.rs`** (`lex_number`), **`completion.rs`** (`word_start`/`show_items`), **`run.rs`** (merged success branch — same stdout-on-success/stderr-on-fail selection), **`ui/layout.rs`** (allocation-free `middle`) — all CLEAN. ### Minor note (non-blocking) `app.rs` is the biggest change in the PR but isn't in the description (which covers ir_pane/highlight/run/stack_art/completion/layout). Same pattern as #515's `ast/program.rs`. Worth a line in the description so the diff isn't surprising to a future reader — the change itself is clean.
navicore deleted branch refactor-repl 2026-05-25 02:30:19 +00:00
Sign in to join this conversation.
No description provided.