Skip to content

Conversation

@JazzGlobal
Copy link
Contributor

@JazzGlobal JazzGlobal commented Oct 27, 2025

Fixes #5539

Sending CTRL+Z will now cause the shell to close on Windows.

The EOF will still be stored in the history. This is how it works when running python on CMD and Powershell so I figured I'd leave it alone. Not sure if that's a python feature or a feature of those particular shells 😄

Summary by CodeRabbit

  • Bug Fixes
    • CTRL+Z on Windows now correctly signals end-of-file in the interactive REPL. Trailing carriage returns/newlines are trimmed so an explicit EOF input is recognized when running in a terminal. Non-Windows platforms and other input paths retain previous behavior; public interfaces remain unchanged.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 27, 2025

Walkthrough

Windows-only REPL readline handling: bind Ctrl+Z to insert an EOF character, trim CR/LF on input, and return EOF when the trimmed line equals the EOF character and stdin is a terminal.

Changes

Cohort / File(s) Summary
Windows EOF handling
vm/src/readline.rs
Added Windows-only EOF handling: introduced an EOF_CHAR constant, bound Ctrl+Z to insert that char in the Rustyline editor, trim trailing CR/LF from input on Windows, and return ReadlineResult::Eof when the trimmed input equals the EOF char and stdin is a terminal.

Sequence Diagram

sequenceDiagram
    participant User
    participant Editor
    participant Readline
    participant Shell

    rect rgb(230,240,255)
    Note over User,Editor: Windows: Ctrl+Z inserts EOF
    User->>Editor: Press Ctrl+Z
    Editor->>Readline: Insert EOF char (0x1A)
    end

    rect rgb(235,255,235)
    Note over User,Readline: Enter submits line
    User->>Readline: Press Enter
    Readline->>Readline: Trim CR/LF
    Readline->>Readline: If line == EOF_CHAR && stdin.isatty()
    Readline-->>Shell: Return ReadlineResult::Eof
    Shell->>Shell: Exit REPL
    end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Single-file, platform-conditional change.
  • Check Windows compilation guards, correct EOF char value/encoding, keybinding scope, and terminal detection (isatty) behavior.

Poem

🐰 I nudged 0x1A into the stream, a tiny zap,
A carrot tap, a soft Ctrl+Z nap,
CR and LF I sweep away,
The REPL bows, then hops away. 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "5539 - CTRL+Z then Enter will now close shell on Windows" is clear, specific, and directly summarizes the main change in the changeset. The title references the issue number and concisely describes the primary functionality being added: enabling Ctrl+Z to exit the shell on Windows. The title is concrete enough that a teammate reviewing the git history would immediately understand the nature of the change without ambiguity.
Linked Issues Check ✅ Passed The code changes directly address the requirement specified in issue #5539. The implementation adds Windows-specific handling to bind CTRL+Z to an EOF_CHAR constant and detects when this character is entered to return ReadlineResult::Eof, which closes the shell. The changes confirm the expected behavior stated in the issue where "Ctrl-Z plus Return to exit" should work, and the PR description explicitly states "Sending CTRL+Z will now cause the RustPython interactive shell to close on Windows," which aligns perfectly with the issue's objective to fix the non-functional Ctrl-Z exit method on Windows.
Out of Scope Changes Check ✅ Passed All changes in the pull request are directly related to the scope of issue #5539. The modifications are explicitly Windows-specific, including the EOF_CHAR constant, CTRL+Z binding, and EOF detection logic. The summary confirms that existing behavior for non-Windows platforms and existing code paths are preserved, indicating no unnecessary or tangential changes were introduced. No changes to exported or public entity signatures were made, and the changes are minimal and focused solely on fixing the Ctrl+Z exit functionality on Windows.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e6089ab and 7cabf8e.

📒 Files selected for processing (1)
  • vm/src/readline.rs (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • vm/src/readline.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
  • GitHub Check: Run tests under miri
  • GitHub Check: Run snippets and cpython tests on wasm-wasi
  • GitHub Check: Run snippets and cpython tests (ubuntu-latest)
  • GitHub Check: Check the WASM package and demo
  • GitHub Check: Run snippets and cpython tests (windows-latest)
  • GitHub Check: Run snippets and cpython tests (macos-latest)
  • GitHub Check: Run rust tests (ubuntu-latest)
  • GitHub Check: Run rust tests (windows-latest)
  • GitHub Check: Check Rust code with rustfmt and clippy
  • GitHub Check: Ensure compilation on various targets
  • GitHub Check: Run rust tests (macos-latest)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@JazzGlobal JazzGlobal marked this pull request as ready for review October 27, 2025 19:26
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
vm/src/readline.rs (2)

128-140: LGTM with optional refactor suggestion.

The EOF detection logic is correct and well-defended:

  • Properly trims Windows line endings before comparison
  • Validates stdin is a terminal to prevent unintended EOF from piped input
  • Returns the original untrimmed line to preserve history behavior (as noted in PR description)

Optionally, if the EOF character constant is extracted as suggested in the initialization block, update this check to reference it:

-                            if trimmed == "\u{001A}" && io::stdin().is_terminal() {
+                            if trimmed == EOF_CHAR && io::stdin().is_terminal() {

91-99: API verification confirmed; consider extracting EOF character constant for maintainability.

The rustyline 17.0.1 API provides all required types (KeyEvent, Modifiers, EventHandler, Cmd) accessible via the wildcard import at line 81. The CTRL+Z binding pattern is correct, and the Windows-specific EOF detection logic properly guards with both a terminal check and line-ending trimming.

The magic string "\u{001A}" appears in two places (lines 97 and 139). For improved maintainability and readability, extract it to a module-level constant:

const EOF_CHAR: &str = "\u{001A}";

Then use EOF_CHAR.into() at line 97 and reference EOF_CHAR at line 139.

📜 Review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9a2792a and 2b3ee61.

📒 Files selected for processing (1)
  • vm/src/readline.rs (2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.rs: Format Rust code with the default rustfmt style (run cargo fmt)
Run clippy and fix any warnings or lints introduced by your changes
Follow Rust best practices for error handling and memory management

Files:

  • vm/src/readline.rs
{vm,stdlib}/**/*.rs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use RustPython macros (pyclass, pymodule, pyfunction, etc.) when implementing Python functionality in Rust

Files:

  • vm/src/readline.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
  • GitHub Check: Run snippets and cpython tests (macos-latest)
  • GitHub Check: Run snippets and cpython tests (ubuntu-latest)
  • GitHub Check: Run snippets and cpython tests (windows-latest)
  • GitHub Check: Ensure compilation on various targets
  • GitHub Check: Run rust tests (ubuntu-latest)
  • GitHub Check: Run rust tests (macos-latest)
  • GitHub Check: Run rust tests (windows-latest)

Copy link
Member

@youknowone youknowone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much! 👍

@youknowone youknowone merged commit 6b25fe5 into RustPython:main Oct 28, 2025
12 checks passed
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.

Ctrl-Z on windows does not exit the shell

2 participants