Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

lib/readline.js: Fix backspace for multi-line(\n) #7128

Open
wants to merge 2 commits into
from

Conversation

Projects
None yet
6 participants

Add additional parameter to cursorTo in _refreshLine to ensure that
the cursor moves to the top left row as well as the far left column
This prevents multi-line output from repeating previous lines.

Calculate the cursor position relative to the last line by splitting
the prompt text on newline characters.

Fixes joyent/node#6415

@Benjamin-Waters Benjamin-Waters lib/readline.js: Fix backspace for multi-line(\n)
Add additional parameter to cursorTo in _refreshLine to ensure that
the cursor moves to the top left row as well as the far left column
This prevents multi-line output from repeating previous lines.

Calculate the cursor position relative to the last line by splitting
the prompt text on newline characters.

Fixes nodejs/node-v0.x-archive#6415
b5cc0f2
Owner

indutny commented Feb 17, 2014

Could you please try running make jslint and fix all the issues in submitted changed that it'll point to? Otherwise it looks good, but would be cool to see a test before landing it.

Updated as per 'make jslint'

I'll look into creating a test for it.

@trevnorris trevnorris added the readline label Feb 18, 2014

@Benjamin-Waters Benjamin-Waters lib/readline.js: Fix backspace for multi-line(\n)
Add additional parameter to cursorTo in _refreshLine to ensure that
the cursor moves to the top left row as well as the far left column
This prevents multi-line output from repeating previous lines.

Calculate the cursor position relative to the last line by splitting
the prompt text on newline characters.

Fixes nodejs/node-v0.x-archive#6415
eed85d7

Any status on this? I've run into this building cli tools as well.

Owner

indutny commented Jul 23, 2014

Sorry for a long waiting... Could you please provide a test for it? Please let me know if you need any help with this.

Owner

jasnell commented Aug 13, 2015

@balupton ... just wanted to check on this. @indutny had asked for a test case to be added before landing. It appears to have stalled out at that point. If this is something that you'd still like to pursue, it would likely be best to (a) add the test case then (b) close this here and open a new PR against master on http://github.com/nodejs/node (the current active development branch).

@Fishrock123 Fishrock123 self-assigned this Jan 14, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment