Readline: workaround for #3860 was removed by commit 94284e7d (incorrect prompt on vt control chars) #5628

Open
OJezu opened this Issue Jun 4, 2013 · 5 comments

3 participants

@OJezu

The following code:

var rl = require('readline').createInterface(process.stdin, process.stdout);
rl.setPrompt('\u001b[31m> \u001b[39m', '> '.length);
rl.prompt();
rl.on('line', function(){
        rl.prompt();
});

in v0.10.6 will put the cursor in correct position (right after '> '), and will not in v0.11.2 due to second argument of rl.setPrompt being removed.
While I agree that second argument of setPrompt is redundant, it should not be removed until the prompt length calculation handles all cases, vt control chars included.

See issue #3860, commit 94284e7 and pull request #4994 for reference

Also, in v0.10.6 there was some weirdness event when promptLength was provided, due to readline code sometimes referencing this._prompt.length instead of this._promptLenght (line wrap) and this._promptLength being lost after rl.question call

@OJezu OJezu pushed a commit that referenced this issue Jun 7, 2013
OhJeez readline: strip ctrl chars for prompt width calc
Use regular expression to strip vt ansi escape codes from display when
calulating prompt display width and cursor position

Fixes #3860 and #5628
2cbb876
@bnoordhuis bnoordhuis added a commit that referenced this issue Jun 17, 2013
@OJezu OJezu readline: strip ctrl chars for prompt width calc
Use regular expression to strip vt ansi escape codes from display when
calulating prompt display width and cursor position

Fixes #3860 and #5628.
ffcd8b9
@tmpfs

I think that the length argument to setPrompt() should not be removed. I am currently using it and if I know the length of the string unformatted it should be respected.

However the quick fix is a modification of the regexp:

var functionKeyCodeReAnywhere = new RegExp('(?:\x1b+)(O|N|\\[|\\[\\[)(?:' + [
  '(\\d+)(?:;(\\d+))?([~^$m])', // added 'm'
  '(?:M([@ #!a`])(.)(.))', // mouse
  '(?:1;)?(\\d+)?([a-zA-Z])'
].join('|') + ')');

I have a patched version that reinstates the length argument and fixes this issue if it helps:

https://github.com/freeformsystems/cli-input/blob/master/lib/patch/readline.js#L968

@jasnell
Node.js Foundation member

@OhJeez ... is this still an issue for you?

@OJezu

In v0.12 everything seems ok. In v0.10 there are still problems, as far as I can tell.

@jasnell
Node.js Foundation member

@OhJeez ... what is the behavior you are seeing in v0.10? When I run using v0.10.39~pre, the behavior is identical to what I see under v0.12.3~pre.

@OJezu

In v0.10.38 things get broken with colored prompt when line wraps due to its length. Text does not appear on new line (overwrites bottom-most line), moving cursor and removing text is broken. It's OK in v0.12.3.

I cannot find tag or branch for v0.10.39-pre in repo - I don't know how to test it 😕

EDIT: v0.10.38 behaviour gif:
prompt-v0 10 38
Type some a's, left arrow a few times, type some b's, and then some backspace

@jasnell jasnell added the readline label Jun 25, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment