● just ci green; line-mode fallback unchanged (it never touches vim-line); 29 unit tests. #41

Merged

navicore merged 2 commits from repl-slice-B-phase-2 into main 2026-06-04 14:36:57 +00:00

Conversation 1 Commits 2 Files changed 5 +481 -325

navicore commented 2026-06-04 13:59:58 +00:00

Owner

Copy link

Slice B Phase 2 — vim-line integration:

• app.rs — App.editor: VimLineEditor added; App.cursor field replaced with cursor() method delegating to
  editor.cursor(). insert_char / backspace / move_* removed — vim-line owns text mutation now. New
  apply_vim_key(vim_line::Key): hands the key to editor.handle_key, applies returned TextEdits to prompt in
  reverse order (vim-line's documented invariant), then dispatches Action::Submit (→ quit./halt. SWI check, then
  history record + push + submit), HistoryPrev/Next (→ recall_prev/next), Cancel (→ should_quit = true).
  recall_prev/next and the editor reset sync the editor cursor via set_cursor(self.prompt.len(), &self.prompt).
• keys.rs — Action shrunk to { NextSolution, StopStreaming, VimKey(vim_line::Key), Quit, Ignore }. New
  convert_key(KeyEvent) -> Option translates crossterm → vim-line. Ctrl-J → KeyCode::Enter with modifiers
  stripped (the crossterm-#371 fix carries forward — vim-line's Normal-mode Enter is Submit). Ctrl-C now passes
  through to vim-line (which emits Action::Cancel → quit) instead of being short-circuited; the test enforces that
  contract so a future refactor can't accidentally reintroduce the short-circuit.
• tui.rs — dispatch collapses to five arms. The old phase-1 commit_goal helper moved into
  App::handle_vim_action::Submit.
• ui.rs — cursor sourced from app.cursor(). New right-aligned mode badge ("NORMAL" / "INSERT" / "-- VISUAL --")
  via app.editor.status() (requires use vim_line::LineEditor). Streaming hides the badge.
• take_prompt removed entirely — vim-line absorbs that flow; the line-mode driver never used it.

Test reshape (29 total):

• Dropped phase-1 insert_char / backspace / arrow_moves — vim-line owns those invariants.
• New typing_in_insert_mode_appends_to_prompt, enter_in_normal_mode_submits_via_apply_vim_key,
  ctrl_c_through_vim_line_quits, quit_dot_typed_and_submitted_exits drive App through apply_vim_key with real
  vim_line::Keys.
• History recall + streaming tests carry forward unchanged.
• keys::tests reshaped — editing_mode_wraps_chars_as_vim_keys, streaming_mode_handles_*_literally,
  ctrl_d_quits_only_on_empty_prompt, ctrl_j_rewrites_to_vim_enter, ctrl_c_passes_through_to_vim_line.

REPL now opens in Normal mode by vim convention — press i to start typing.

Slice B Phase 2 — vim-line integration: - app.rs — App.editor: VimLineEditor added; App.cursor field replaced with cursor() method delegating to editor.cursor(). insert_char / backspace / move_* removed — vim-line owns text mutation now. New apply_vim_key(vim_line::Key): hands the key to editor.handle_key, applies returned TextEdits to prompt in reverse order (vim-line's documented invariant), then dispatches Action::Submit (→ quit./halt. SWI check, then history record + push + submit), HistoryPrev/Next (→ recall_prev/next), Cancel (→ should_quit = true). recall_prev/next and the editor reset sync the editor cursor via set_cursor(self.prompt.len(), &self.prompt). - keys.rs — Action shrunk to { NextSolution, StopStreaming, VimKey(vim_line::Key), Quit, Ignore }. New convert_key(KeyEvent) -> Option<VlKey> translates crossterm → vim-line. Ctrl-J → KeyCode::Enter with modifiers stripped (the crossterm-#371 fix carries forward — vim-line's Normal-mode Enter is Submit). Ctrl-C now passes through to vim-line (which emits Action::Cancel → quit) instead of being short-circuited; the test enforces that contract so a future refactor can't accidentally reintroduce the short-circuit. - tui.rs — dispatch collapses to five arms. The old phase-1 commit_goal helper moved into App::handle_vim_action::Submit. - ui.rs — cursor sourced from app.cursor(). New right-aligned mode badge ("NORMAL" / "INSERT" / "-- VISUAL --") via app.editor.status() (requires use vim_line::LineEditor). Streaming hides the badge. - take_prompt removed entirely — vim-line absorbs that flow; the line-mode driver never used it. Test reshape (29 total): - Dropped phase-1 insert_char / backspace / arrow_moves — vim-line owns those invariants. - New typing_in_insert_mode_appends_to_prompt, enter_in_normal_mode_submits_via_apply_vim_key, ctrl_c_through_vim_line_quits, quit_dot_typed_and_submitted_exits drive App through apply_vim_key with real vim_line::Keys. - History recall + streaming tests carry forward unchanged. - keys::tests reshaped — editing_mode_wraps_chars_as_vim_keys, streaming_mode_handles_*_literally, ctrl_d_quits_only_on_empty_prompt, ctrl_j_rewrites_to_vim_enter, ctrl_c_passes_through_to_vim_line. REPL now opens in Normal mode by vim convention — press i to start typing.

navicore added 1 commit 2026-06-04 13:59:58 +00:00

● just ci green; line-mode fallback unchanged (it never touches vim-line); 29 unit tests. ... cf1601fa0d

Slice B Phase 2 — vim-line integration:

  - app.rs — App.editor: VimLineEditor added; App.cursor field replaced with cursor() method delegating to
  editor.cursor(). insert_char / backspace / move_* removed — vim-line owns text mutation now. New
  apply_vim_key(vim_line::Key): hands the key to editor.handle_key, applies returned TextEdits to prompt in
  reverse order (vim-line's documented invariant), then dispatches Action::Submit (→ quit./halt. SWI check, then
  history record + push + submit), HistoryPrev/Next (→ recall_prev/next), Cancel (→ should_quit = true).
  recall_prev/next and the editor reset sync the editor cursor via set_cursor(self.prompt.len(), &self.prompt).
  - keys.rs — Action shrunk to { NextSolution, StopStreaming, VimKey(vim_line::Key), Quit, Ignore }. New
  convert_key(KeyEvent) -> Option<VlKey> translates crossterm → vim-line. Ctrl-J → KeyCode::Enter with modifiers
  stripped (the crossterm-#371 fix carries forward — vim-line's Normal-mode Enter is Submit). Ctrl-C now passes
  through to vim-line (which emits Action::Cancel → quit) instead of being short-circuited; the test enforces that
  contract so a future refactor can't accidentally reintroduce the short-circuit.
  - tui.rs — dispatch collapses to five arms. The old phase-1 commit_goal helper moved into
  App::handle_vim_action::Submit.
  - ui.rs — cursor sourced from app.cursor(). New right-aligned mode badge ("NORMAL" / "INSERT" / "-- VISUAL --")
  via app.editor.status() (requires use vim_line::LineEditor). Streaming hides the badge.
  - take_prompt removed entirely — vim-line absorbs that flow; the line-mode driver never used it.

  Test reshape (29 total):
  - Dropped phase-1 insert_char / backspace / arrow_moves — vim-line owns those invariants.
  - New typing_in_insert_mode_appends_to_prompt, enter_in_normal_mode_submits_via_apply_vim_key,
  ctrl_c_through_vim_line_quits, quit_dot_typed_and_submitted_exits drive App through apply_vim_key with real
  vim_line::Keys.
  - History recall + streaming tests carry forward unchanged.
  - keys::tests reshaped — editing_mode_wraps_chars_as_vim_keys, streaming_mode_handles_*_literally,
  ctrl_d_quits_only_on_empty_prompt, ctrl_j_rewrites_to_vim_enter, ctrl_c_passes_through_to_vim_line.

  REPL now opens in Normal mode by vim convention — press i to start typing.

navicore commented 2026-06-04 14:22:23 +00:00

Author

Owner

Copy link

Review

What it does: vim-line integration. The editor owns Editing-mode keys (modal Normal/Insert/Visual, motions, operators); App hosts the text per vim-line's caller-owns-text contract and dispatches its Actions; keys.rs shrinks to a crossterm→vim-line translation plus the Streaming literals; mode badge in the prompt row.

Verified locally (scratch copy of the branch): 29 tests pass, clippy clean, and under a pty the full modal flow works — i → type → Esc → Enter submits, ; streams, . commits, iquit.-Esc-Enter exits, INSERT/NORMAL badges render. Ctrl-D-on-empty quit confirmed working end-to-end (incidentally — script's EOF arrives as a literal ^D). Also read the pinned vim-line 7.5.7 source: the reverse-order edit application in apply_vim_key matches the real upstream contract (the r-command multi-edit is built for it), r restores Normal mode safely, Cancel fires only from Normal-mode Ctrl-C so the Cancel→quit mapping can't misfire, and SubmitCommand is genuinely unreachable with command mode defaulted off. All four #40 land-now items are in (Press filter, stdin+stdout TTY check, current_solution with three regression tests, Ctrl-D/Ctrl-J). The ctrl_c_passes_through_to_vim_line contract test is a thoughtful touch.

🟡 Worth addressing

1. Insert-mode Enter inserts an invisible \n instead of submitting — demonstrated under a pty: type i p(1). and press Enter; nothing submits, the newline enters the single-line prompt buffer, the rendered prompt goes blank/mangled (ratatui won't render \n inside a height-1 span), and the caret floats at a byte-offset column that counts the invisible newline. To a user with REPL muscle memory (Enter = submit, which is every REPL including SWI), the app looks dead — badge still INSERT, no feedback. o/O and Shift-Enter inject \n the same way. Two-part fix:
   • Submit on Insert-mode Enter. Cleanest per the design doc's upstream-first policy: a with_insert_enter_submits(true) opt-in on vim-line (same owner; seqr keeps its multi-line chat behavior). Host-side interception in apply_vim_key works as a stopgap.
   • Defense in depth: after applying edits, self.prompt = self.prompt.replace('\n', " ") — it's byte-length-preserving so every cursor offset stays valid, and it degrades o/O/Shift-Enter gracefully. (Single-line prompt is the design doc's stated initial-cut scope.)
2. Ctrl-C is dead during Streaming: map_streaming falls through to Ignore for it, so a user mid-stream gets nothing from Ctrl-C (phase 1 quit from both modes). Mapping it to StopStreaming matches every REPL's "Ctrl-C = get me back to the prompt" instinct. Related accuracy note: Insert-mode Ctrl-C only exits to Normal in vim-line (vim-correct), so "Ctrl-C → Cancel → quit" holds from Normal only — quitting from Insert is Ctrl-C ×2. Fine behavior, but the PR description overstates it and users should learn it from somewhere (see 3).
3. Discoverability: the REPL now opens in Normal mode, and the only place that says "press i to start typing" is the PR description. A first-time user types a goal and watches motions fire (or nothing). Suggest a one-time transcript hint at startup ("press i to edit · Enter in NORMAL submits · quit. exits") and the README keybindings section the design doc already lists as README content — it doesn't exist yet, and with modal editing it's now load-bearing.

🟢 Minor

• editor.selection() is never rendered — Visual mode shows the badge but no highlight, so v + motions look inert. Phase-3 candidate alongside the cursor-column work.
• editor.set_cursor(0, &self.prompt) immediately after editor.reset() is redundant — reset() already zeroes the cursor.
• PR description says the badge shows -- VISUAL --; vim-line's status() returns VISUAL (and d.../c.../r... for pending states — those rendering in the badge is actually a nice freebie).
• Upstream doc drift worth an upstream PR since you own vim-line: EditResult.edits is documented "in order," while the crate example and the ReplaceChar comment both require reverse-order application. This PR got it right by following the example; the next consumer may follow the field doc instead.

Verdict

Clean integration — the host/editor split follows vim-line's design exactly, the dropped phase-1 editing tests are correctly replaced by driving real vim_line::Keys through apply_vim_key, and everything claimed checks out under a pty. Item 1 is the one real user-facing trap and deserves a fix before this ships to anyone who hasn't read the PR description; 2 and 3 are small and round it out.

---

Review by Claude Code

# Review **What it does:** vim-line integration. The editor owns Editing-mode keys (modal Normal/Insert/Visual, motions, operators); App hosts the text per vim-line's caller-owns-text contract and dispatches its `Action`s; keys.rs shrinks to a crossterm→vim-line translation plus the Streaming literals; mode badge in the prompt row. **Verified locally** (scratch copy of the branch): 29 tests pass, clippy clean, and under a pty the full modal flow works — `i` → type → Esc → Enter submits, `;` streams, `.` commits, `iquit.`-Esc-Enter exits, INSERT/NORMAL badges render. Ctrl-D-on-empty quit confirmed working end-to-end (incidentally — `script`'s EOF arrives as a literal `^D`). Also read the pinned vim-line 7.5.7 source: the reverse-order edit application in `apply_vim_key` matches the real upstream contract (the `r`-command multi-edit is built for it), `r` restores Normal mode safely, `Cancel` fires only from Normal-mode Ctrl-C so the Cancel→quit mapping can't misfire, and `SubmitCommand` is genuinely unreachable with command mode defaulted off. All four #40 land-now items are in (Press filter, stdin+stdout TTY check, `current_solution` with three regression tests, Ctrl-D/Ctrl-J). The `ctrl_c_passes_through_to_vim_line` contract test is a thoughtful touch. ## 🟡 Worth addressing 1. **Insert-mode Enter inserts an invisible `\n` instead of submitting** — demonstrated under a pty: type `i p(1).` and press Enter; nothing submits, the newline enters the single-line prompt buffer, the rendered prompt goes blank/mangled (ratatui won't render `\n` inside a height-1 span), and the caret floats at a byte-offset column that counts the invisible newline. To a user with REPL muscle memory (Enter = submit, which is *every* REPL including SWI), the app looks dead — badge still INSERT, no feedback. `o`/`O` and Shift-Enter inject `\n` the same way. Two-part fix: - Submit on Insert-mode Enter. Cleanest per the design doc's upstream-first policy: a `with_insert_enter_submits(true)` opt-in on vim-line (same owner; seqr keeps its multi-line chat behavior). Host-side interception in `apply_vim_key` works as a stopgap. - Defense in depth: after applying edits, `self.prompt = self.prompt.replace('\n', " ")` — it's byte-length-preserving so every cursor offset stays valid, and it degrades `o`/`O`/Shift-Enter gracefully. (Single-line prompt is the design doc's stated initial-cut scope.) 2. **Ctrl-C is dead during Streaming**: `map_streaming` falls through to `Ignore` for it, so a user mid-stream gets nothing from Ctrl-C (phase 1 quit from both modes). Mapping it to `StopStreaming` matches every REPL's "Ctrl-C = get me back to the prompt" instinct. Related accuracy note: Insert-mode Ctrl-C only exits to Normal in vim-line (vim-correct), so "Ctrl-C → Cancel → quit" holds *from Normal only* — quitting from Insert is Ctrl-C ×2. Fine behavior, but the PR description overstates it and users should learn it from somewhere (see 3). 3. **Discoverability:** the REPL now opens in Normal mode, and the only place that says "press `i` to start typing" is the PR description. A first-time user types a goal and watches motions fire (or nothing). Suggest a one-time transcript hint at startup ("press i to edit · Enter in NORMAL submits · quit. exits") and the README keybindings section the design doc already lists as README content — it doesn't exist yet, and with modal editing it's now load-bearing. ## 🟢 Minor - `editor.selection()` is never rendered — Visual mode shows the badge but no highlight, so `v` + motions look inert. Phase-3 candidate alongside the cursor-column work. - `editor.set_cursor(0, &self.prompt)` immediately after `editor.reset()` is redundant — `reset()` already zeroes the cursor. - PR description says the badge shows `-- VISUAL --`; vim-line's `status()` returns `VISUAL` (and `d...`/`c...`/`r...` for pending states — those rendering in the badge is actually a nice freebie). - Upstream doc drift worth an upstream PR since you own vim-line: `EditResult.edits` is documented "in order," while the crate example and the ReplaceChar comment both require reverse-order application. This PR got it right by following the example; the next consumer may follow the field doc instead. ## Verdict Clean integration — the host/editor split follows vim-line's design exactly, the dropped phase-1 editing tests are correctly replaced by driving real `vim_line::Key`s through `apply_vim_key`, and everything claimed checks out under a pty. Item 1 is the one real user-facing trap and deserves a fix before this ships to anyone who hasn't read the PR description; 2 and 3 are small and round it out. --- *Review by Claude Code*

navicore referenced this pull request from a commit 2026-06-04 14:35:28 +00:00

● All four PR #41 items addressed; just ci green; 32 unit tests.

navicore added 1 commit 2026-06-04 14:35:28 +00:00

● All four PR #41 items addressed; just ci green; 32 unit tests. ... 35c67b1c9f

- 🟡 1 Insert-mode Enter now short-circuits to Action::Submit before reaching vim-line (vim-line would otherwise
  insert a literal \n per its multi-line contract). Plus the byte-length-preserving prompt.replace('\n', ' ')
  defense after every edit-apply, so o/O/Shift-Enter degrade to "insert a space" instead of leaving the prompt
  with an invisible newline ratatui can't render. New tests enter_in_insert_mode_submits and
  o_command_does_not_break_single_line_prompt.
  - 🟡 2 map_streaming now maps Ctrl-C → Action::StopStreaming — universal "back to the prompt" instinct.
  Quit-from-Editing still works via vim-line's Action::Cancel. New test ctrl_c_in_streaming_stops_streaming.
  - 🟡 3 New TranscriptEntry::Info variant; App construction pushes a one-time dim hint: "press i to edit · Enter
  submits · ; for next solution · . to commit · quit. to exit". README gets a full Keybindings section per the
  design doc (Editing / Streaming / Exiting tables). app_constructs_with_startup_hint test replaces the earlier
  "transcript is empty" assertion.
  - 🟢 Dropped the redundant editor.set_cursor(0, &self.prompt) after editor.reset() in the Submit arm — reset()
  already zeros it.

  Also rolled the three submit/advance tests forward to use post-hint indexing (let before = app.transcript.len()
  then assert against transcript[before]) instead of hard-coding index 0.

navicore merged commit f3b1439652 into main 2026-06-04 14:36:57 +00:00

navicore deleted branch repl-slice-B-phase-2 2026-06-04 14:36:57 +00:00

navicore referenced this pull request from a commit 2026-06-04 14:36:57 +00:00

Merge pull request '● just ci green; line-mode fallback unchanged (it never touches vim-line); 29 unit tests.' (#41) from repl-slice-B-phase-2 into main

Commenting is not possible because the repository is archived.

Reviewers

No reviewers

Labels

Clear labels   

bug

Something isn't working

  

documentation

Improvements or additions to documentation

  

duplicate

This issue or pull request already exists

  

enhancement

New feature or request

  

good first issue

Good for newcomers

  

help wanted

Extra attention is needed

  

invalid

This doesn't seem right

  

question

Further information is requested

  

wontfix

This will not be worked on

No labels

bug

documentation

duplicate

enhancement

good first issue

help wanted

invalid

question

wontfix

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

navicore/patch-prolog-v1!41

No description provided.