Skip to content

fix: preserve prompt position after ExternalBreak#1042

Merged
fdncred merged 2 commits intonushell:mainfrom
eitsupi:fix/external-break-preserve-prompt-position
Mar 20, 2026
Merged

fix: preserve prompt position after ExternalBreak#1042
fdncred merged 2 commits intonushell:mainfrom
eitsupi:fix/external-break-preserve-prompt-position

Conversation

@eitsupi
Copy link
Copy Markdown
Contributor

@eitsupi eitsupi commented Mar 19, 2026 •
edited
Loading

Follow-up to #1035.

In #1035, when a caller handles ExternalBreak by calling read_line() again (e.g., to resume editing after inspecting the buffer), the prompt jumps down one line each time.
This is because move_cursor_to_end() unconditionally prints a CRLF before returning ExternalBreak.

This PR replaces move_cursor_to_end() with saving suspended_state, matching the pattern used by ExecuteHostCommand.
The updated example demonstrates resuming read_line() after ExternalBreak.

@eitsupi eitsupi marked this pull request as ready for review March 19, 2026 15:23
@fdncred
Copy link
Copy Markdown
Contributor

fdncred commented Mar 19, 2026

How does this affect normal use?

@eitsupi eitsupi changed the title feat: preserve prompt position after ExternalBreak fix: preserve prompt position after ExternalBreak Mar 19, 2026
@eitsupi
Copy link
Copy Markdown
Contributor Author

eitsupi commented Mar 19, 2026

How does this affect normal use?

This change only affects callers that use the break_signal feature (introduced in #1035) and call read_line() again after receiving ExternalBreak.
For normal nushell use, there is no impact.
For break_signal users, the behavior change is: after ExternalBreak, the next read_line() call reuses the original prompt position instead of advancing one line. This matches how ExecuteHostCommand already works.

Apologies for the late fix. I should have caught this issue when working on #1035.

@fdncred
Copy link
Copy Markdown
Contributor

fdncred commented Mar 19, 2026

Am I misreading this? I thought this change was for everyone?
https://github.com/nushell/reedline/pull/1042/changes#diff-ff278ec599adfd033d35a083ebe8948db8bf177c9817dee8ddc4f3a9e3f5728eR819-R820

@eitsupi eitsupi mentioned this pull request Mar 19, 2026
Merged
@eitsupi
Copy link
Copy Markdown
Contributor Author

eitsupi commented Mar 19, 2026

Am I misreading this? I thought this change was for everyone?
#1042 (changes)

The changed code is inside the if let Some(ref signal) = self.break_signal block, which is only entered when a caller has configured a break signal via with_break_signal().
Without that, self.break_signal is None and this path is never reached.

The core fix is replacing move_cursor_to_end() with suspended_state saving (same pattern as ExecuteHostCommand).
The last_render_snapshot = None on line 819 was added for consistency with ExecuteHostCommand.
it has no functional impact since the snapshot is only used for mouse-click handling and is refreshed on the next repaint().

eitsupi added a commit to eitsupi/arf that referenced this pull request Mar 19, 2026
@eitsupi
feat: add experimental JSON-RPC IPC server for external tool access (#…
24fe4b3 
…113)

## Summary

Add an experimental JSON-RPC 2.0 IPC server for AI agents, IDE extensions, and other external tools to interact with a running arf R session.

### Features

- **Transport**: Unix domain sockets (Linux/macOS), Windows named pipes
- **Protocol**: JSON-RPC 2.0 over HTTP (curl-compatible) or raw JSON
- **`evaluate` method**: Silent or visible evaluation with captured stdout/value/error via WriteConsoleEx interception
- **`user_input` method**: Inject code into the REPL via reedline's `ExternalBreak` signal (nushell/reedline#1035), with full echo, history, spinner, and duration tracking
- **`send` method**: Alias for `user_input`
- **`:ipc` meta command**: Start/stop/check server status at runtime
- **`arf ipc` subcommand**: CLI client for eval/send/list/status from another terminal
- **Session discovery**: `~/.cache/arf/sessions/<pid>.json`, cleaned up on exit

### Safety

- Mutual exclusion with console input: IPC requests are rejected with `USER_IS_TYPING` (-32003) when the user has typed something in the buffer
- Alternate screen modes (help pager, history browser, shell mode): IPC requests are rejected immediately with `R_NOT_AT_PROMPT` (-32001)
- Prompt position is preserved after rejected ExternalBreak (reedline fix: nushell/reedline#1042)

### Tests

- 7 cross-platform IPC unit tests (protocol, evaluate, user_input)
- 6 PTY integration tests (evaluate capture, visible/silent modes, user_input, rejection, prompt position)
@fdncred
Copy link
Copy Markdown
Contributor

fdncred commented Mar 19, 2026

ah, ok. I missed that. I guess this isn't an issue anymore since you closed it?

@eitsupi eitsupi reopened this Mar 20, 2026
@eitsupi
Copy link
Copy Markdown
Contributor Author

eitsupi commented Mar 20, 2026

I'm sorry, it was an accident.
I accidentally closed from my repository that was referencing this pull request😓

@fdncred fdncred merged commit 1e017e8 into nushell:main Mar 20, 2026
14 checks passed
@fdncred
Copy link
Copy Markdown
Contributor

fdncred commented Mar 20, 2026

Thanks

@eitsupi eitsupi deleted the fix/external-break-preserve-prompt-position branch March 20, 2026 12:39
@eitsupi
Copy link
Copy Markdown
Contributor Author

eitsupi commented Mar 20, 2026

Thanks for the quick review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@eitsupi @fdncred