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.

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

util: improve util inspect output #26984

Closed

Conversation

BridgeAR
Copy link
Member

Please have a look at the commit messages for details.

@nodejs/util PTAL

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

This fixes a proportion calculation for lots of short array entries
with at least one bigger one that alone makes up for more than one
fifth of all other entries together.
This makes sure that errors that contain extra properties show those
properties on a separate line.
@nodejs-github-bot
Copy link
Collaborator

@BridgeAR Sadly, an error occurred when I tried to trigger a build. :(

@nodejs-github-bot nodejs-github-bot added the util Issues and PRs related to the built-in util module. label Mar 29, 2019
@BridgeAR
Copy link
Member Author

I just pushed one small commit that adds one more check to prevent unnecessary work in the default case (but no changed behavior).

@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 30, 2019
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Mar 31, 2019

@BridgeAR BridgeAR requested review from jasnell and mcollina April 1, 2019 15:01
@mcollina
Copy link
Member

mcollina commented Apr 1, 2019

Can you please explain how this improves the output?

@BridgeAR
Copy link
Member Author

BridgeAR commented Apr 1, 2019

@mcollina did you see the commit messages?

util: improve `inspect()` compact number mode

This fixes a proportion calculation for lots of short array entries
with at least one bigger one that alone makes up for more than one
fifth of all other entries together.

util: improve error property inspection

This makes sure that errors that contain extra properties show those
properties on a separate line.

Without the first commit the corresponding test would just create a single
line for each array entry instead of grouping them together.

Without the second commit the corresponding tests would not add a linebreak
in case an error has extra properties.

const err = new Error();
err.foo = true;
console.log(err);
// Only the last line:

// Before
//     at REPLServer.Interface._line (readline.js:675:8) foo: true }

// Now
//     at REPLServer.Interface._line (readline.js:675:8)
//   foo: true }

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

LGTM

BridgeAR added a commit to BridgeAR/node that referenced this pull request Apr 3, 2019
This fixes a proportion calculation for lots of short array entries
with at least one bigger one that alone makes up for more than one
fifth of all other entries together.

PR-URL: nodejs#26984
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
BridgeAR added a commit to BridgeAR/node that referenced this pull request Apr 3, 2019
This makes sure that errors that contain extra properties show those
properties on a separate line.

PR-URL: nodejs#26984
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@BridgeAR
Copy link
Member Author

BridgeAR commented Apr 3, 2019

Landed in 14b2db0, f7c9685 🎉

@BridgeAR BridgeAR closed this Apr 3, 2019
BethGriggs pushed a commit that referenced this pull request Apr 4, 2019
This fixes a proportion calculation for lots of short array entries
with at least one bigger one that alone makes up for more than one
fifth of all other entries together.

PR-URL: #26984
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
BethGriggs pushed a commit that referenced this pull request Apr 4, 2019
This makes sure that errors that contain extra properties show those
properties on a separate line.

PR-URL: #26984
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

4 participants