fix: ensure stdin state is correct for searchable multi-select prompt - #620
fix: ensure stdin state is correct for searchable multi-select prompt#620yongnianliu wants to merge 6 commits into
Conversation
📝 WalkthroughWalkthroughThe changes fix a stdin state bug affecting arrow key input in searchableMultiSelect after the welcome screen. The welcome-screen no longer manages stdin state post-entry, while searchable-multi-select now explicitly prepares stdin (raw mode and resume) before initialization. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 🧪 Unit Test Generation v2 is now available!We have significantly improved our unit test generation capabilities. To enable: Add this to your reviews:
finishing_touches:
unit_tests:
enabled: trueTry it out by using the Have feedback? Share your thoughts on our Discord thread! 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 |
Review CompleteYour review story is ready! Comment !reviewfast on this PR to re-generate the story. |
Greptile OverviewGreptile SummaryFixed stdin state management issue where arrow keys didn't work in Root cause: Changes:
The fix follows a defensive programming approach where Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant InitCommand
participant WelcomeScreen
participant stdin as process.stdin
participant SearchableMultiSelect
Note over User,SearchableMultiSelect: Before Fix: stdin left in paused state
User->>InitCommand: Run init command
InitCommand->>WelcomeScreen: showWelcomeScreen()
WelcomeScreen->>stdin: setRawMode(true)
WelcomeScreen->>stdin: resume()
User->>WelcomeScreen: Press Enter
Note over WelcomeScreen,stdin: OLD: setRawMode(wasRaw) + pause()
WelcomeScreen-->>InitCommand: Return
InitCommand->>SearchableMultiSelect: searchableMultiSelect()
Note over SearchableMultiSelect,stdin: Arrow keys don't work (stdin paused)
User->>SearchableMultiSelect: Press Enter first
Note over SearchableMultiSelect: Inquirer resumes stdin
User->>SearchableMultiSelect: Arrow keys now work
Note over User,SearchableMultiSelect: After Fix: stdin state properly managed
User->>InitCommand: Run init command
InitCommand->>WelcomeScreen: showWelcomeScreen()
WelcomeScreen->>stdin: setRawMode(true)
WelcomeScreen->>stdin: resume()
User->>WelcomeScreen: Press Enter
Note over WelcomeScreen: NEW: Don't restore/pause stdin
WelcomeScreen-->>InitCommand: Return
InitCommand->>SearchableMultiSelect: searchableMultiSelect()
SearchableMultiSelect->>stdin: Check isTTY
SearchableMultiSelect->>stdin: setRawMode(true) if not raw
SearchableMultiSelect->>stdin: resume()
Note over SearchableMultiSelect: stdin now in correct state
User->>SearchableMultiSelect: Arrow keys work immediately
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Additional Comments (1)
Prompt To Fix With AIThis is a comment left during a code review.
Path: src/ui/welcome-screen.ts
Line: 90:90
Comment:
Variable `wasRaw` captured but never used after removing the cleanup code
How can I resolve this? If you propose a fix, please make it concise. |
TabishB
left a comment
TabishB
left a comment
There was a problem hiding this comment.
Nice find on the root cause. One suggestion below.
| stdin.removeListener('data', onData); | ||
| stdin.setRawMode(wasRaw); | ||
| stdin.pause(); | ||
| // Don't restore raw mode or pause stdin here - let the next prompt handle stdin state |
There was a problem hiding this comment.
The searchableMultiSelect() guard already fixes this. I'd revert this change so waitForEnter() keeps cleaning up after itself. No need for both.
|
@yongnianliu Let me know if you can take a look at the above comment |
|
Closing as already fixed on #619 is closed as completed, fixed by #1175 (
async function waitForEnter(): Promise<void> {
if (!process.stdin.isTTY) return;
// Keep all interactive input on Inquirer's keypress lifecycle. Mixing a raw
// `data` listener between Inquirer prompts breaks arrow/space keys on Windows.
const { createPrompt, isEnterKey, useKeypress } = await import('@inquirer/core');
const prompt = createPrompt<void, Record<string, never>>((_config, done) => {
useKeypress((key) => {
if (key.ctrl && key.name === 'c') { process.stdout.write('\n'); process.exit(0); }
if (isEnterKey(key)) done(undefined);
});
return '';
});
await prompt({});
}That code comment names exactly the failure mode this PR was working around. Why the shipped fix took a different shape, and why it matters here: this PR left the raw listener in place and compensated on both sides — Since then Reopen if arrow keys still fail after the welcome screen on current |
3 participants
#619
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.