Skip to content
New issue

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

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

util: improve .inspect() array grouping #28070

Closed
wants to merge 2 commits into from
Closed

util: improve .inspect() array grouping #28070

wants to merge 2 commits into from

Conversation

BridgeAR
Copy link
Member

@BridgeAR BridgeAR commented Jun 5, 2019

Please have a look at the commit messages for a detailed description and check the changed tests for the result of these changes.

Refs: #27690

// cc @nodejs/util

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

@BridgeAR BridgeAR requested review from addaleax and devsnek June 5, 2019 13:15
@BridgeAR BridgeAR force-pushed the inspect-array-grouping-improvements branch from 721f471 to 7e8c779 Compare June 9, 2019 11:02
@BridgeAR BridgeAR mentioned this pull request Jun 11, 2019
Closed
@BridgeAR
util: improve .inspect() array grouping
2f6c2e9 
This improves a couple minor things:

* Arrays that contain entries other than `number` or `bigint` are
  ordered to the left instead of the right.
* The bias towards more columns got increased. That mainly increases
  the number of columns for arrays that contain lots of short entries.
* Columns are now more dense in case they would otherwise have extra
  whitespace in-between two columns.
* The maximum columns got increased from 10 to 15.
* The maximum number of columns per `compact` was increased from
  3 to 4.
@BridgeAR
util: use average bias while grouping arrays
9e7f4e2 
This makes sure that strongly deviating entry length are taken into
account while grouping arrays.
@BridgeAR BridgeAR force-pushed the inspect-array-grouping-improvements branch from 7e8c779 to 9e7f4e2 Compare June 11, 2019 22:27
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jun 11, 2019 •
edited
Loading

CI: https://ci.nodejs.org/job/node-test-pull-request/23808/
Resumed build: https://ci.nodejs.org/job/node-test-pull-request/23813/
CI: https://ci.nodejs.org/job/node-test-pull-request/23813/

@BridgeAR
Copy link
Member Author

@nodejs/util PTAL. This is open since a week without receiving any reviews so far.

@BridgeAR BridgeAR added the util Issues and PRs related to the built-in util module. label Jun 12, 2019
@BridgeAR
Copy link
Member Author

@addaleax you suggested to change the entries alignment in case the array contains non-numbers. This PR implements this suggestion plus some minor other modifications.

@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jun 15, 2019
@BridgeAR
Copy link
Member Author

Landed in 87a22cf, b97b003 🎉

@BridgeAR BridgeAR closed this Jun 17, 2019
BridgeAR added a commit to BridgeAR/node that referenced this pull request Jun 17, 2019
@BridgeAR
util: improve .inspect() array grouping
87a22cf 
This improves a couple minor things:

* Arrays that contain entries other than `number` or `bigint` are
  ordered to the left instead of the right.
* The bias towards more columns got increased. That mainly increases
  the number of columns for arrays that contain lots of short entries.
* Columns are now more dense in case they would otherwise have extra
  whitespace in-between two columns.
* The maximum columns got increased from 10 to 15.
* The maximum number of columns per `compact` was increased from
  3 to 4.

PR-URL: nodejs#28070
Refs: nodejs#27690
Reviewed-By: James M Snell <jasnell@gmail.com>
BridgeAR added a commit to BridgeAR/node that referenced this pull request Jun 17, 2019
@BridgeAR
util: use average bias while grouping arrays
b97b003 
This makes sure that strongly deviating entry length are taken into
account while grouping arrays.

PR-URL: nodejs#28070
Refs: nodejs#27690
Reviewed-By: James M Snell <jasnell@gmail.com>
BridgeAR added a commit that referenced this pull request Jun 17, 2019
@BridgeAR
util: improve .inspect() array grouping
8f4bb19 
This improves a couple minor things:

* Arrays that contain entries other than `number` or `bigint` are
  ordered to the left instead of the right.
* The bias towards more columns got increased. That mainly increases
  the number of columns for arrays that contain lots of short entries.
* Columns are now more dense in case they would otherwise have extra
  whitespace in-between two columns.
* The maximum columns got increased from 10 to 15.
* The maximum number of columns per `compact` was increased from
  3 to 4.

PR-URL: #28070
Refs: #27690
Reviewed-By: James M Snell <jasnell@gmail.com>
BridgeAR added a commit that referenced this pull request Jun 17, 2019
@BridgeAR
util: use average bias while grouping arrays
225fb1b 
This makes sure that strongly deviating entry length are taken into
account while grouping arrays.

PR-URL: #28070
Refs: #27690
Reviewed-By: James M Snell <jasnell@gmail.com>
@BridgeAR BridgeAR mentioned this pull request Jun 17, 2019
Merged
targos pushed a commit that referenced this pull request Jun 18, 2019
@BridgeAR @targos
util: improve .inspect() array grouping
7e61760 
This improves a couple minor things:

* Arrays that contain entries other than `number` or `bigint` are
  ordered to the left instead of the right.
* The bias towards more columns got increased. That mainly increases
  the number of columns for arrays that contain lots of short entries.
* Columns are now more dense in case they would otherwise have extra
  whitespace in-between two columns.
* The maximum columns got increased from 10 to 15.
* The maximum number of columns per `compact` was increased from
  3 to 4.

PR-URL: #28070
Refs: #27690
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Jun 18, 2019
@BridgeAR @targos
util: use average bias while grouping arrays
b6f113b 
This makes sure that strongly deviating entry length are taken into
account while grouping arrays.

PR-URL: #28070
Refs: #27690
Reviewed-By: James M Snell <jasnell@gmail.com>
BridgeAR added a commit that referenced this pull request Jun 26, 2019
@BridgeAR
2019-06-18, Version 12.5.0 (Current)
4360273 
Notable changes:

This release contains `semver-major` commits. These are in fact not
`semver-major` due to follow-up commits that remove all breaking changes.

* build:
  * The startup time is reduced by enabling V8 snapshots by default
    #28181
* deps:
  * Updated `V8` to 7.5.288.22 #27375
    * The numeric separator (v8.dev/features/numeric-separators) feature is now
      enabled by default
  * Updated `OpenSSL` to 1.1.1c #28211
* inspector:
  * The `--inspect-publish-uid` flag was added to specify ways of the inspector
    web socket url exposure #27741
* n-api:
  * Accessors on napi_define_* are now ECMAScript-compliant
    #27851
* report:
  * The cpu info got added to the report output
    #28188
* src:
  * Restore the original state of the stdio file descriptors on exit to prevent
    leaving stdio in raw or non-blocking mode
    #24260
* tools,gyp:
  * Introduce MSVS 2019 #27375
* util:
  * inspect:
    * Array grouping became more compact and uses more columns than before
      #28059
      #28070
    * Long strings will not be split at 80 characters anymore. Instead they will
      be split on new lines #28055
* worker:
  * `worker.terminate()` now returns a promise and using the callback is
    deprecated #28021

PR-URL: #28268
BridgeAR added a commit that referenced this pull request Jun 26, 2019
@BridgeAR
2019-06-18, Version 12.5.0 (Current)
65829a2 
Notable changes:

* build:
  * The startup time is reduced by enabling V8 snapshots by default
    #28181
* deps:
  * Updated `V8` to 7.5.288.22 #27375
    * The numeric separator (v8.dev/features/numeric-separators) feature is now
      enabled by default
  * Updated `OpenSSL` to 1.1.1c #28211
* inspector:
  * The `--inspect-publish-uid` flag was added to specify ways of the inspector
    web socket url exposure #27741
* n-api:
  * Accessors on napi_define_* are now ECMAScript-compliant
    #27851
* report:
  * The cpu info got added to the report output
    #28188
* src:
  * Restore the original state of the stdio file descriptors on exit to prevent
    leaving stdio in raw or non-blocking mode
    #24260
* tools,gyp:
  * Introduce MSVS 2019 #27375
* util:
  * inspect:
    * Array grouping became more compact and uses more columns than before
      #28059
      #28070
    * Long strings will not be split at 80 characters anymore. Instead they will
      be split on new lines #28055
* worker:
  * `worker.terminate()` now returns a promise and using the callback is
    deprecated #28021

PR-URL: #28268
BridgeAR added a commit that referenced this pull request Jun 26, 2019
@BridgeAR
2019-06-26, Version 12.5.0 (Current)
f5e0b17 
Notable changes:

* build:
  * The startup time is reduced by enabling V8 snapshots by default
    #28181
* deps:
  * Updated `V8` to 7.5.288.22 #27375
    * The numeric separator (v8.dev/features/numeric-separators) feature is now
      enabled by default
  * Updated `OpenSSL` to 1.1.1c #28211
* inspector:
  * The `--inspect-publish-uid` flag was added to specify ways of the inspector
    web socket url exposure #27741
* n-api:
  * Accessors on napi_define_* are now ECMAScript-compliant
    #27851
* report:
  * The cpu info got added to the report output
    #28188
* src:
  * Restore the original state of the stdio file descriptors on exit to prevent
    leaving stdio in raw or non-blocking mode
    #24260
* tools,gyp:
  * Introduce MSVS 2019 #27375
* util:
  * inspect:
    * Array grouping became more compact and uses more columns than before
      #28059
      #28070
    * Long strings will not be split at 80 characters anymore. Instead they will
      be split on new lines #28055
* worker:
  * `worker.terminate()` now returns a promise and using the callback is
    deprecated #28021

PR-URL: #28268
BridgeAR added a commit that referenced this pull request Jun 27, 2019
@BridgeAR
2019-06-27, Version 12.5.0 (Current)
01b404f 
Notable changes:

* build:
  * The startup time is reduced by enabling V8 snapshots by default
    #28181
* deps:
  * Updated `V8` to 7.5.288.22 #27375
    * The numeric separator (v8.dev/features/numeric-separators) feature is now
      enabled by default
  * Updated `OpenSSL` to 1.1.1c #28211
* inspector:
  * The `--inspect-publish-uid` flag was added to specify ways of the inspector
    web socket url exposure #27741
* n-api:
  * Accessors on napi_define_* are now ECMAScript-compliant
    #27851
* report:
  * The cpu info got added to the report output
    #28188
* src:
  * Restore the original state of the stdio file descriptors on exit to prevent
    leaving stdio in raw or non-blocking mode
    #24260
* tools,gyp:
  * Introduce MSVS 2019 #27375
* util:
  * inspect:
    * Array grouping became more compact and uses more columns than before
      #28059
      #28070
    * Long strings will not be split at 80 characters anymore. Instead they will
      be split on new lines #28055
* worker:
  * `worker.terminate()` now returns a promise and using the callback is
    deprecated #28021

PR-URL: #28268
BridgeAR added a commit to BridgeAR/node that referenced this pull request Jun 27, 2019
@BridgeAR
2019-06-27, Version 12.5.0 (Current)
af3cc71 
Notable changes:

* build:
  * The startup time is reduced by enabling V8 snapshots by default
    nodejs#28181
* deps:
  * Updated `V8` to 7.5.288.22 nodejs#27375
    * The numeric separator (v8.dev/features/numeric-separators) feature is now
      enabled by default
  * Updated `OpenSSL` to 1.1.1c nodejs#28211
* inspector:
  * The `--inspect-publish-uid` flag was added to specify ways of the inspector
    web socket url exposure nodejs#27741
* n-api:
  * Accessors on napi_define_* are now ECMAScript-compliant
    nodejs#27851
* report:
  * The cpu info got added to the report output
    nodejs#28188
* src:
  * Restore the original state of the stdio file descriptors on exit to prevent
    leaving stdio in raw or non-blocking mode
    nodejs#24260
* tools,gyp:
  * Introduce MSVS 2019 nodejs#27375
* util:
  * inspect:
    * Array grouping became more compact and uses more columns than before
      nodejs#28059
      nodejs#28070
    * Long strings will not be split at 80 characters anymore. Instead they will
      be split on new lines nodejs#28055
* worker:
  * `worker.terminate()` now returns a promise and using the callback is
    deprecated nodejs#28021

PR-URL: nodejs#28268
@BridgeAR BridgeAR deleted the inspect-array-grouping-improvements branch January 20, 2020 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasnell jasnell jasnell approved these changes

@addaleax addaleax Awaiting requested review from addaleax

@devsnek devsnek Awaiting requested review from devsnek

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. util Issues and PRs related to the built-in util module.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@BridgeAR @nodejs-github-bot @jasnell