repl refactor #518
No reviewers
Labels
No labels
bug
dependencies
documentation
duplicate
enhancement
good first issue
help wanted
invalid
question
refactor
rust
technical-debt
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
navicore/patch-seq!518
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "refactor-repl"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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.
Review — approve, no blockers
Refactor across
crates/repl/(+516/−524, net −8).just cigreen 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:
SearchStatefield grouping — the fivesearch_*fields onApp(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 useSearchState::default(). This drives theapp/tests.rschurn (pure field renames — no assertion or expected-value changes).handle_keydecomposition — the giant inlinematchis split intohandle_completion_popup_key/handle_search_key/try_enter_search/handle_ctrl_shortcut/handle_function_key/handle_command_line_tab(each returnsbool= consumed) plusrefresh_search/step_search_matchhelpers. Verified arm-for-arm:_arm and cmdline-tab_arm correctly returnfalseto preserve the original fall-through;step_search_matchkeepspreview_current_match()+update_search_status()after the index update, gated by the earlyreturnon empty (matches the oldif !empty { … });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→&strparam 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-freemiddle) — all CLEAN.Minor note (non-blocking)
app.rsis 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'sast/program.rs. Worth a line in the description so the diff isn't surprising to a future reader — the change itself is clean.