Fix setRawMode on Windows for non-stdin TTYs - #32153
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughWindows TTY support for reopened CONIN$ streams is enhanced. The module conditionally loads the native TTY binding on Windows, ReadStream constructor allocates a native wrapper for non-stdin TTY fds, setRawMode coordinates with the binding using numeric mode values, and a test verifies raw mode state transitions on reopened CONIN$ streams. ChangesWindows TTY raw mode support
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
|
Cross-referencing #34788, which addresses the same |
What does this PR do?
Fixes Windows
tty.ReadStream#setRawMode()for reopened console input handles such as\\\\.\\CONIN$.When stdin is a pipe, tools such as hunk should be able to reopen
CONIN$to recover interactive keyboard input. On Bun, creatingnew tty.ReadStream(fd)for that reopened console fd reportedisTTY === true, butsetRawMode(true)failed because the stream did not have a native TTY wrapper. This PR creates a WindowsTTYWrapfor non-stdin TTY read streams and routes raw-mode changes through it, while preserving Bun’s existing stdin path.It also adds a Windows regression test covering
fs.openSync("\\\\.\\CONIN$", "r+"),new tty.ReadStream(fd), and toggling raw mode on/off. The analogous test works in Node; see nodejs/node#63856How did you verify your code works?
Built Bun locally on Windows:
Ran the focused tty test:
Verified the original pipe/interactive scenario using Hunk:
Checked patch whitespace:
git diff --checkI had issues running the full test suite, but I think those errors were unrelated.