-
Notifications
You must be signed in to change notification settings - Fork 30.1k
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
doc: readline.emitKeypressEvents and raw mode #6628
Conversation
@@ -362,6 +362,12 @@ Move cursor to the specified position in a given TTY stream. | |||
Causes `stream` to begin emitting `'keypress'` events corresponding to its | |||
input. | |||
|
|||
Note that the stream need to be in raw mode: |
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.
needs
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.
Thanks.
@nodejs/documentation |
Might wanna add a check that the stream is a TTY in the example? if (process.stdin.isTTY) {
// might not be a TTY if spawned from another node process
process.stdin.setRawMode(true)
} |
@@ -362,6 +362,12 @@ Move cursor to the specified position in a given TTY stream. | |||
Causes `stream` to begin emitting `'keypress'` events corresponding to its | |||
input. | |||
|
|||
Note that the stream needs to be in raw mode: |
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.
“… the stream, if it is a TTY, needs to be…”?
@arve0 +1 to adding that check to the example. LGTM apart from that. |
`readline.emitKeypressEvents` needs `stream` to be in raw mode, ref nodejs#6626
Updated. |
LGTM |
Landed in 1ba5a56. Thank you! |
Checklist
Affected core subsystem(s)
doc
Description of change
readline.emitKeypressEvents
needsstream
to be in raw mode, fixes #6626