-
Notifications
You must be signed in to change notification settings - Fork 1.4k
5539 - CTRL+Z then Enter will now close shell on Windows. #6223
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughWindows-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
Sequence DiagramsequenceDiagram
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
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
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. Comment |
There was a problem hiding this 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
lineto 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 referenceEOF_CHARat line 139.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 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 (runcargo 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)
youknowone
left a comment
There was a problem hiding this 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! 👍
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