repl,readline: improve unicode support and tab completion by BridgeAR · Pull Request #31288 · nodejs/node

BridgeAR · 2020-01-09T21:35:42Z

This reduces the number of write operations used during tab
completion.
The tab completion calculated the string width using the length
of the string instead of using the actual width. That is fixed.
The key decoder is now capable of handling characters composed
out of two code points. That reduces the number of "keypress"
events that are emitted which again lowers the amount of writes
triggered.

~~This is currently blocked by #31112 (moving the charLengthAt function).~~

// cc @nodejs/repl @nodejs/readline

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

1. This reduces the number of write operations used during tab completion. 2. The tab completion calculated the string width using the length of the string instead of using the actual width. That is fixed. 3. The key decoder is now capable of handling characters composed out of two code points. That reduces the number of "keypress" events that are emitted which again lowers the amount of writes triggered.

This simplifies code that was more complicated than it had to be and removes code that should never be reached.

BridgeAR · 2020-01-10T17:49:08Z

@jasnell I pushed another commit that cleans up some code. It would have conflicted with the former commit if I would have opened another PR for it instead. PTAL.

nodejs-github-bot · 2020-01-10T17:50:13Z

CI: https://ci.nodejs.org/job/node-test-pull-request/28353/

BridgeAR · 2020-01-10T17:52:16Z

lib/readline.js

-      getStringWidth(this.line[this.cursor]) > 1) {
-    rows++;
-    cols = 0;
-  }


Our cursor can't be between two code points. We move the cursor by the correct length since a while.

nodejs-github-bot · 2020-01-11T17:40:54Z

CI: https://ci.nodejs.org/job/node-test-pull-request/28363/

Trott

LGTM but I'm going to kick off a CITGM just to make sure the readline changes aren't breaking in an unexpected way.

Trott · 2020-01-11T17:43:07Z

CITGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/2218/

nodejs-github-bot · 2020-01-11T18:32:20Z

CI: https://ci.nodejs.org/job/node-test-pull-request/28365/

Trott · 2020-01-11T22:07:20Z

Landed in a523283...fe05818

1. This reduces the number of write operations used during tab completion. 2. The tab completion calculated the string width using the length of the string instead of using the actual width. That is fixed. 3. The key decoder is now capable of handling characters composed out of two code points. That reduces the number of "keypress" events that are emitted which again lowers the amount of writes triggered. PR-URL: #31288 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

This simplifies code that was more complicated than it had to be and removes code that should never be reached. PR-URL: #31288 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

1. This reduces the number of write operations used during tab completion. 2. The tab completion calculated the string width using the length of the string instead of using the actual width. That is fixed. 3. The key decoder is now capable of handling characters composed out of two code points. That reduces the number of "keypress" events that are emitted which again lowers the amount of writes triggered. PR-URL: #31288 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

This simplifies code that was more complicated than it had to be and removes code that should never be reached. PR-URL: #31288 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

codebytere · 2020-03-14T21:17:58Z

@BridgeAR should this go back to v12.x? I'll add the label but feel free to swap out.

1. This reduces the number of write operations used during tab completion. 2. The tab completion calculated the string width using the length of the string instead of using the actual width. That is fixed. 3. The key decoder is now capable of handling characters composed out of two code points. That reduces the number of "keypress" events that are emitted which again lowers the amount of writes triggered. PR-URL: nodejs#31288 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

This simplifies code that was more complicated than it had to be and removes code that should never be reached. PR-URL: nodejs#31288 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

1. This reduces the number of write operations used during tab completion. 2. The tab completion calculated the string width using the length of the string instead of using the actual width. That is fixed. 3. The key decoder is now capable of handling characters composed out of two code points. That reduces the number of "keypress" events that are emitted which again lowers the amount of writes triggered. PR-URL: #31288 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

This simplifies code that was more complicated than it had to be and removes code that should never be reached. PR-URL: #31288 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

BridgeAR added the blocked PRs that are blocked by other issues or PRs. label Jan 9, 2020

nodejs-github-bot added the readline Issues and PRs related to the built-in readline module. label Jan 9, 2020

BridgeAR added performance Issues and PRs related to the performance of Node.js. repl Issues and PRs related to the REPL subsystem. and removed performance Issues and PRs related to the performance of Node.js. labels Jan 9, 2020

jasnell approved these changes Jan 10, 2020

View reviewed changes

BridgeAR removed the blocked PRs that are blocked by other issues or PRs. label Jan 10, 2020

BridgeAR force-pushed the 2020-01-09-readline-improvements branch from 2e65b26 to a019c62 Compare January 10, 2020 08:18

repl,readline: code cleanup

23c754f

This simplifies code that was more complicated than it had to be and removes code that should never be reached.

BridgeAR force-pushed the 2020-01-09-readline-improvements branch from 9211679 to 23c754f Compare January 10, 2020 17:46

BridgeAR changed the title ~~readline: improve unicode support and tab completion~~ repl,readline: improve unicode support and tab completion Jan 10, 2020

BridgeAR requested a review from jasnell January 10, 2020 17:48

BridgeAR commented Jan 10, 2020

View reviewed changes

BridgeAR mentioned this pull request Jan 11, 2020

repl: prevent previews during pasting #31315

Closed

4 tasks

Trott approved these changes Jan 11, 2020

View reviewed changes

BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 11, 2020

Trott closed this Jan 11, 2020

codebytere mentioned this pull request Jan 16, 2020

v13.7.0 proposal #31382

Merged

codebytere added the backport-requested-v12.x label Mar 14, 2020

targos removed author ready PRs that have at least one approval, no pending requests for changes, and a CI started. backport-requested-v12.x labels Apr 25, 2020

targos mentioned this pull request May 2, 2020

v12.17.0 release proposal #33197

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

repl,readline: improve unicode support and tab completion#31288

repl,readline: improve unicode support and tab completion#31288
BridgeAR wants to merge 2 commits intonodejs:masterfrom
BridgeAR:2020-01-09-readline-improvements

BridgeAR commented Jan 9, 2020 •

edited

Loading

Uh oh!

BridgeAR commented Jan 10, 2020

Uh oh!

nodejs-github-bot commented Jan 10, 2020

Uh oh!

BridgeAR Jan 10, 2020

Uh oh!

nodejs-github-bot commented Jan 11, 2020

Uh oh!

Trott left a comment

Uh oh!

Trott commented Jan 11, 2020

Uh oh!

nodejs-github-bot commented Jan 11, 2020

Uh oh!

Trott commented Jan 11, 2020

Uh oh!

codebytere commented Mar 14, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Conversation

BridgeAR commented Jan 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

BridgeAR commented Jan 10, 2020

Uh oh!

nodejs-github-bot commented Jan 10, 2020

Uh oh!

BridgeAR Jan 10, 2020

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Jan 11, 2020

Uh oh!

Trott left a comment

Choose a reason for hiding this comment

Uh oh!

Trott commented Jan 11, 2020

Uh oh!

nodejs-github-bot commented Jan 11, 2020

Uh oh!

Trott commented Jan 11, 2020

Uh oh!

codebytere commented Mar 14, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

BridgeAR commented Jan 9, 2020 •

edited

Loading