Repository navigation
Readline keypress event is triggered only after pressing Esc key 3 times #7379
Description
- Version: 6.2.2 and 4.4.5
- Platform: macOS 10.11.5
- Subsystem: readline
Test case
const readline = require('readline');
readline.emitKeypressEvents(process.stdin);
process.stdin.setRawMode(true);
process.stdin.on('keypress', console.log);Expected
After pressing Esc once, I would get this output:
undefined { sequence: '\u001b',
name: 'escape',
ctrl: false,
meta: true,
shift: false }
Actual
After pressing Esc three times, I get this output:
undefined { sequence: '\u001b\u001b\u001b',
name: 'escape',
ctrl: false,
meta: true,
shift: false }
It works fine on 0.10.44 and 0.12.13. After pressing Esc once:
{ name: 'escape',
ctrl: false,
meta: false,
shift: false,
sequence: '\u001b' }
Also notice that meta is false here.
Activity
improve key handling
MylesBorins commentedon Jun 23, 2016
I've dug into this a bit and can confirm the bug exists in
v4.xandv5.xIt would appear to have been introduced in
v2.0.2I'll dig into that release and see where it came fromMylesBorins commentedon Jun 23, 2016
So it would appear that escape started acting weird after aed6bce906 landed.
Doesn't appear to be easily revertable... I'm digging into what is going on with escape
MylesBorins commentedon Jun 23, 2016
Found the offending section
https://github.com/nodejs/node/blob/master/lib/internal/readline.js#L144-L151
The original PR introduced this logic which yields to more results if the first character is an escape... which is problematic if your entire payload is an escape. It allows for double escaped characters, which is why things work on the third press
Not sure the best way to approach this, but I'll revisit in the morning
princejwesley commentedon Jun 23, 2016
@thealphanerd It was mentioned in the PR itself.
MylesBorins commentedon Jun 23, 2016
@princejwesley sigh... I wish I had read the pr instead of just the commit and then digging into the code.
@cjihrig you have the blame on most of
internal/readline.jswould you specify this as awont-fixrather than as a bug?princejwesley commentedon Jun 23, 2016
@thealphanerd I'll fix it!
3 remaining items
silverwind commentedon Jun 23, 2016
@cjihrig I accepted that drawback as the primary use of esc in a terminal is to start an escape sequence, but I now see the issue with applications relying on that key.
@princejwesley I guess a timeout-based approach to triggering the key is the way to go. I just don't know if 700ms is the correct value, seems a tad high.
princejwesley commentedon Jun 23, 2016
@silverwind
Default timeout values for
zshis400msand for vim & ncurses, its1sec. @sindresorhus suggested to kepp the middle value between400 - 1000. Shall I update with the middle value of '400 - 700' (550ms) or400ms? One second is too high.silverwind commentedon Jun 23, 2016
How about 500ms, like in GNU readline (http://man7.org/linux/man-pages/man3/readline.3.html)?
princejwesley commentedon Jun 23, 2016
@silverwind Done
readline: keypress event trigger for escape character
readline: keypress trigger for escape character
readline: keypress trigger for escape character
MylesBorins commentedon Sep 30, 2016
This has been backported to v4.x in 68a2979
readline: keypress trigger for escape character
readline: keypress trigger for escape character
readline: keypress trigger for escape character