FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

readline: make tab size configurable by BridgeAR · Pull Request #31318 · nodejs/node · GitHub

/ node Public

readline: make tab size configurable - #31318

Closed
BridgeAR wants to merge 1 commit into
nodejs:masterfrom
BridgeAR:2020-01-11-readline-make-tab-size-configurable
ConversationCommits (1)ChecksFiles changed
Closed

readline: make tab size configurable#31318
BridgeAR wants to merge 1 commit into
nodejs:masterfrom
BridgeAR:2020-01-11-readline-make-tab-size-configurable

Conversation

BridgeAR commented Jan 11, 2020

Copy link
Copy Markdown
Member

This adds the tabSize option to readline to allow different tab
sizes. It also cleans up some code and unifies the linebreak RegExp.

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

nodejs-github-bot added the readline Issues and PRs related to the built-in readline module. label Jan 11, 2020
BridgeAR added the semver-minor PRs that contain new features and should be released in the next minor version. label Jan 11, 2020

This comment has been minimized.

Sign in to view

This comment has been minimized.

Sign in to view
bnoordhuis reviewed Jan 12, 2020
View reviewed changes

bnoordhuis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

The historySize changes should be a separate commit and arguably a separate PR because it's unrelated.

Comment thread lib/readline.js Outdated

This comment has been minimized.

Sign in to view
BridgeAR force-pushed the 2020-01-11-readline-make-tab-size-configurable branch from 60f32a5 to 77effd2 Compare January 12, 2020 20:14

BridgeAR commented Jan 12, 2020

Copy link
Copy Markdown
Member Author

@bnoordhuis I removed the refactoring.

BridgeAR force-pushed the 2020-01-11-readline-make-tab-size-configurable branch from 77effd2 to 5f1947d Compare January 12, 2020 21:04

This comment has been minimized.

Sign in to view

nodejs-github-bot commented Jan 12, 2020 •
edited by BridgeAR
Loading

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/28419/ ✔️

BridgeAR commented Jan 13, 2020

Copy link
Copy Markdown
Member Author

@nodejs/readline @nodejs/repl PTAL (this is the basis to add the same option to the repl).

Trott reviewed Jan 14, 2020
View reviewed changes
Comment thread test/parallel/test-readline-interface.js Outdated

Trott commented Jan 14, 2020

Copy link
Copy Markdown
Member

Not opposed to this, but curious what the motivation is. Is this something you've needed but didn't have? Is it something someone has asked for?

BridgeAR commented Jan 14, 2020 •
edited
Loading

Copy link
Copy Markdown
Member Author

@Trott neither. The reason is that it's possible to change the tab size in terminals.

BridgeAR commented Jan 17, 2020

Copy link
Copy Markdown
Member Author

@nodejs/repl @nodejs/readline this could use some reviews.

mcollina reviewed Jan 20, 2020
View reviewed changes

mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I would prefer if this did not edit existings tests as well, it makes the review harder.

BridgeAR commented Jan 20, 2020

Copy link
Copy Markdown
Member Author

I updated the tests to be independent but I want to note that we just copy 95% of the test code all the time.

BridgeAR added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Jan 20, 2020

nodejs-github-bot commented Jan 20, 2020

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Jan 20, 2020

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Jan 20, 2020

Copy link
Copy Markdown
Collaborator

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

antsmartian approved these changes Jan 21, 2020
View reviewed changes
Comment thread lib/readline.js Outdated
BridgeAR force-pushed the 2020-01-11-readline-make-tab-size-configurable branch from d7eddc1 to 0b9ec1d Compare January 22, 2020 14:58

nodejs-github-bot commented Jan 22, 2020

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Jan 25, 2020

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Jan 26, 2020

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Jan 26, 2020

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Jan 26, 2020

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Jan 27, 2020

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Feb 5, 2020

Copy link
Copy Markdown
Collaborator

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

addaleax removed the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Feb 5, 2020
addaleax approved these changes Feb 5, 2020
View reviewed changes

addaleax left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This needs a rebase

BridgeAR force-pushed the 2020-01-11-readline-make-tab-size-configurable branch from 0b9ec1d to 27983d4 Compare February 6, 2020 19:46
readline: make tab size configurable
c7571e2 
This adds the `tabSize` option to readline to allow different tab
sizes.
BridgeAR force-pushed the 2020-01-11-readline-make-tab-size-configurable branch from 27983d4 to c7571e2 Compare February 6, 2020 19:48

nodejs-github-bot commented Feb 6, 2020

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Feb 6, 2020

Copy link
Copy Markdown
Collaborator

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

addaleax pushed a commit that referenced this pull request Feb 7, 2020
readline: make tab size configurable
f9a27ea 
This adds the `tabSize` option to readline to allow different tab
sizes.

PR-URL: #31318
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

addaleax commented Feb 7, 2020

Copy link
Copy Markdown
Member

Landed in f9a27ea

addaleax closed this Feb 7, 2020
codebytere pushed a commit that referenced this pull request Feb 17, 2020
readline: make tab size configurable
148dfde 
This adds the `tabSize` option to readline to allow different tab
sizes.

PR-URL: #31318
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
codebytere mentioned this pull request Feb 17, 2020
Merged
codebytere added a commit that referenced this pull request Feb 18, 2020
2020-02-18, Version 13.9.0 (Current)
1f0f58c 
Notable changes:

* async_hooks
  * add executionAsyncResource (Matteo Collina) #30959
* crypto
  * add crypto.diffieHellman (Tobias Nießen) #31178
  * add DH support to generateKeyPair (Tobias Nießen) #31178
  * simplify DH groups (Tobias Nießen) #31178
  * add key type 'dh' (Tobias Nießen) #31178
* test
  * skip keygen tests on arm systems (Tobias Nießen) #31178
* perf_hooks
  * add property flags to GCPerformanceEntry (Kirill Fomichev) #29547
* process
  * report ArrayBuffer memory in `memoryUsage()` (Anna Henningsen) #31550
* readline
  * make tab size configurable (Ruben Bridgewater) #31318
* report
  * add support for Workers (Anna Henningsen) #31386
* worker
  * add ability to take heap snapshot from parent thread (Anna Henningsen) #31569
* added new collaborators
  * add ronag to collaborators (Robert Nagy) #31498

PR-URL: #31837
codebytere added a commit that referenced this pull request Feb 18, 2020
2020-02-18, Version 13.9.0 (Current)
0c3c0e7 
Notable changes:

* async_hooks
  * add executionAsyncResource (Matteo Collina) #30959
* crypto
  * add crypto.diffieHellman (Tobias Nießen) #31178
  * add DH support to generateKeyPair (Tobias Nießen) #31178
  * simplify DH groups (Tobias Nießen) #31178
  * add key type 'dh' (Tobias Nießen) #31178
* test
  * skip keygen tests on arm systems (Tobias Nießen) #31178
* perf_hooks
  * add property flags to GCPerformanceEntry (Kirill Fomichev) #29547
* process
  * report ArrayBuffer memory in `memoryUsage()` (Anna Henningsen) #31550
* readline
  * make tab size configurable (Ruben Bridgewater) #31318
* report
  * add support for Workers (Anna Henningsen) #31386
* worker
  * add ability to take heap snapshot from parent thread (Anna Henningsen) #31569
* added new collaborators
  * add ronag to collaborators (Robert Nagy) #31498

PR-URL: #31837
targos mentioned this pull request Apr 18, 2020
Merged
2 tasks

targos commented Apr 25, 2020

Copy link
Copy Markdown
Member

Depends on #31423 to land on v12.x

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

mcollina mcollina left review comments
bnoordhuis bnoordhuis left review comments
jasnell jasnell approved these changes
Trott Trott approved these changes
addaleax addaleax approved these changes
targos Awaiting requested review from targos
+2 more reviewers
lundibundi lundibundi approved these changes
antsmartian antsmartian approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

readline Issues and PRs related to the built-in readline module. semver-minor PRs that contain new features and should be released in the next minor version.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

10 participants

Back | FazBrowse Home | New Git URL