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: add util.inspect compact option #17576

Closed
wants to merge 7 commits into from

Conversation

BridgeAR
Copy link
Member

@BridgeAR BridgeAR commented Dec 9, 2017

I personally really dislike the formatting used in util.inspect and this should
improve the situation a lot. I am also working on a feature that requires a
more structural util.inspect output. In general, I do not argue about it being
perfect, so if anyone has some further input, I am all ears.

commit - util: add util.inspect structured option

The current default formatting is not ideal and this improves
the situation by formatting the output more intuitiv.

1) All object keys are now indented by 2 characters instead of
   something 2 and sometimes 3 characters.
2) Each object key will now use a individual line instead of
   sharing a line potentially with multiple object keys.
3) Long strings will now be split into multiple lines in case
   they exceed the "lineBreak" option length (including the
   current indentation).
4) Opening braces are now directly behind a object property
   instead of using a new line.

commit - util: rename util.inspect argument

util.inspect can actually receive any property and the description
was wrong so far. This fixes it by renaming the argument to
value and also updating the description.

commit - util: fix custom inspect description

Using a custom inspect function on the inspected object is
deprecated. Remove the reference from the option description
to make sure the user will read about the deprecation in the
more detailed description.

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

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
Affected core subsystem(s)

util

@nodejs-github-bot nodejs-github-bot added the util Issues and PRs related to the built-in util module. label Dec 9, 2017
@BridgeAR BridgeAR force-pushed the util-structured branch 2 times, most recently from 8c65749 to b44a6f8 Compare December 9, 2017 17:55
@BridgeAR
Copy link
Member Author

BridgeAR commented Dec 9, 2017

I pushed another commit that changes the base order of the inspected object.

@BridgeAR BridgeAR added the semver-minor PRs that contain new features and should be released in the next minor version. label Dec 10, 2017
@Trott
Copy link
Member

Trott commented Dec 10, 2017

Two small typos in the first commit message: something 2 -> sometimes 2 and a individual -> an individual. I normally don't flag typos in commit messages but that first one had me puzzled for a few moments longer than I want to admit.

The current default formatting is not ideal and this improves
the situation by formatting the output more intuitiv.

1) All object keys are now indented by 2 characters instead of
   sometimes 2 and sometimes 3 characters.
2) Each object key will now use an individual line instead of
   sharing a line potentially with multiple object keys.
3) Long strings will now be split into multiple lines in case
   they exceed the "lineBreak" option length (including the
   current indentation).
4) Opening braces are now directly behind a object property
   instead of using a new line.
util.inspect can actually receive any property and the description
was wrong so far. This fixes it by renaming the argument to
value and also updating the description.
Using a custom inspect function on the inspected object is
deprecated. Remove the reference from the option description
to make sure the user will read about the deprecation in the
more detailed description.
In case the structured option is used, it will now print

  "[Function: foo] {\n  property: 'data'\n}"

instead of

  "{ [Function: foo]\n  property: 'data'\n}"
@BridgeAR
Copy link
Member Author

I updated the commit message accordingly.

@BridgeAR BridgeAR mentioned this pull request Dec 11, 2017
4 tasks
@addaleax
Copy link
Member

@BridgeAR I’m a fan of this feature, but not so sure about the name – “structured” doesn’t say that much, I think… it’s hard to come up with something better. I’ve thought about “long”, which also seems to have potential for confusion but is more to the point, I think. Maybe onePropertyPerLine? It’s verbose but very unambiguous.

@BridgeAR
Copy link
Member Author

@addaleax yeah, the name is probably not ideal. What do you think about beautify? 😄
I am not a fan of very long option names and onePropertyPerLine would actually not describe everything that is changed.

@addaleax
Copy link
Member

@BridgeAR Me neither, but I’d rather have something that actually tells you what’s happening

And beauty is definitely in the eye of the beholder ;)

@Trott
Copy link
Member

Trott commented Dec 11, 2017

Maybe make an option called compact and have it default to true? Then, if you want this feature (what is called structured in this PR currently), set compact to false?

@BridgeAR
Copy link
Member Author

@Trott that would also work but it somehow sounds "less good". And I personally prefer the new formatting a lot over the current one. If it is abot me I would just replace it ^^. But even though the output is considered to be changed from time to time, I guess that would be to harsh.

@Trott
Copy link
Member

Trott commented Dec 11, 2017

@BridgeAR OK, but I do think we need a better term than structured. I gave my suggestion. :-D

@BridgeAR
Copy link
Member Author

BridgeAR commented Dec 12, 2017

So I thought about it again and what do you think about readerFriendly?

@addaleax
Copy link
Member

I think readerFriendly doesn’t say anything about the format, and is pretty subjective?

I actually like compact.

@BridgeAR
Copy link
Member Author

I agree that compact itself sounds nice but that is exactly the problem I have with it. Because compact = false sounds like you deactivate something positive. Otherwise I would would have already changed the name to it.

@@ -1213,3 +1213,129 @@ assert.doesNotThrow(() => util.inspect(process));
assert.strictEqual(util.inspect(new NotStringClass()),
'NotStringClass {}');
}

{
Copy link
Member

Choose a reason for hiding this comment

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

It looks like none of the examples here test objects with a custom inspect method. Can you add some coverage in for that?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@BridgeAR BridgeAR added the notable-change PRs with changes that should be highlighted in changelogs. label Dec 15, 2017
@BridgeAR
Copy link
Member Author

I changed the name to compact and I also updated the RegExp to handle line breaks better.

lib/util.js Outdated
const averageLineLength = Math.ceil(value.length / Math.ceil(value.length / minLineLength));
const divisor = Math.max(averageLineLength, MIN_LINE_LENGTH);
var res = '';
if (readableRegExps[divisor] === undefined)
Copy link
Member

Choose a reason for hiding this comment

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

Can you add {} for multi-line if bodies? :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@BridgeAR BridgeAR closed this Dec 21, 2017
@addaleax addaleax removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 29, 2017
@MylesBorins
Copy link
Contributor

This does not land cleanly on v9.x. Would someone be willing to backport?

BridgeAR added a commit to BridgeAR/node that referenced this pull request Mar 8, 2018
Using a custom inspect function on the inspected object is
deprecated. Remove the reference from the option description
to make sure the user will read about the deprecation in the
more detailed description.

PR-URL: nodejs#17576
Reviewed-By: Anna Henningsen <[email protected]>
BridgeAR added a commit to BridgeAR/node that referenced this pull request Mar 8, 2018
util.inspect can actually receive any property and the description
was wrong so far. This fixes it by renaming the argument to
value and also updating the description.

PR-URL: nodejs#17576
Reviewed-By: Anna Henningsen <[email protected]>
BridgeAR added a commit to BridgeAR/node that referenced this pull request Mar 8, 2018
The current default formatting is not ideal and this improves
the situation by formatting the output more intuitiv.

1) All object keys are now indented by 2 characters instead of
   sometimes 2 and sometimes 3 characters.
2) Each object key will now use an individual line instead of
   sharing a line potentially with multiple object keys.
3) Long strings will now be split into multiple lines in case
   they exceed the "lineBreak" option length (including the
   current indentation).
4) Opening braces are now directly behind a object property
   instead of using a new line.
5) Switch inspect "base" order. In case the compact option is set
   to `false`, inspect will now print
     "[Function: foo] {\n  property: 'data'\n}"
   instead of
     "{ [Function: foo]\n  property: 'data'\n}".

PR-URL: nodejs#17576
Reviewed-By: Anna Henningsen <[email protected]>
@BridgeAR BridgeAR mentioned this pull request Mar 8, 2018
4 tasks
@BridgeAR
Copy link
Member Author

BridgeAR commented Mar 8, 2018

Backport opened in #19230

MylesBorins pushed a commit that referenced this pull request Mar 15, 2018
Using a custom inspect function on the inspected object is
deprecated. Remove the reference from the option description
to make sure the user will read about the deprecation in the
more detailed description.

Backport-PR-URL: #19230
PR-URL: #17576
Reviewed-By: Anna Henningsen <[email protected]>
MylesBorins pushed a commit that referenced this pull request Mar 15, 2018
util.inspect can actually receive any property and the description
was wrong so far. This fixes it by renaming the argument to
value and also updating the description.

Backport-PR-URL: #19230
PR-URL: #17576
Reviewed-By: Anna Henningsen <[email protected]>
MylesBorins pushed a commit that referenced this pull request Mar 15, 2018
The current default formatting is not ideal and this improves
the situation by formatting the output more intuitiv.

1) All object keys are now indented by 2 characters instead of
   sometimes 2 and sometimes 3 characters.
2) Each object key will now use an individual line instead of
   sharing a line potentially with multiple object keys.
3) Long strings will now be split into multiple lines in case
   they exceed the "lineBreak" option length (including the
   current indentation).
4) Opening braces are now directly behind a object property
   instead of using a new line.
5) Switch inspect "base" order. In case the compact option is set
   to `false`, inspect will now print
     "[Function: foo] {\n  property: 'data'\n}"
   instead of
     "{ [Function: foo]\n  property: 'data'\n}".

Backport-PR-URL: #19230
PR-URL: #17576
Reviewed-By: Anna Henningsen <[email protected]>
@targos targos mentioned this pull request Mar 18, 2018
MylesBorins pushed a commit that referenced this pull request Mar 20, 2018
Using a custom inspect function on the inspected object is
deprecated. Remove the reference from the option description
to make sure the user will read about the deprecation in the
more detailed description.

Backport-PR-URL: #19230
PR-URL: #17576
Reviewed-By: Anna Henningsen <[email protected]>
MylesBorins pushed a commit that referenced this pull request Mar 20, 2018
util.inspect can actually receive any property and the description
was wrong so far. This fixes it by renaming the argument to
value and also updating the description.

Backport-PR-URL: #19230
PR-URL: #17576
Reviewed-By: Anna Henningsen <[email protected]>
MylesBorins pushed a commit that referenced this pull request Mar 20, 2018
The current default formatting is not ideal and this improves
the situation by formatting the output more intuitiv.

1) All object keys are now indented by 2 characters instead of
   sometimes 2 and sometimes 3 characters.
2) Each object key will now use an individual line instead of
   sharing a line potentially with multiple object keys.
3) Long strings will now be split into multiple lines in case
   they exceed the "lineBreak" option length (including the
   current indentation).
4) Opening braces are now directly behind a object property
   instead of using a new line.
5) Switch inspect "base" order. In case the compact option is set
   to `false`, inspect will now print
     "[Function: foo] {\n  property: 'data'\n}"
   instead of
     "{ [Function: foo]\n  property: 'data'\n}".

Backport-PR-URL: #19230
PR-URL: #17576
Reviewed-By: Anna Henningsen <[email protected]>
MylesBorins added a commit that referenced this pull request Mar 21, 2018
Notable changes:

* assert:
  - From now on all error messages produced by `assert` in strict mode
    will produce a error diff. (Ruben Bridgewater)
    #17615
  - From now on it is possible to use a validation object in throws
    instead of the other possibilities. (Ruben Bridgewater)
    #17584
* crypto:
  - allow passing null as IV unless required (Tobias Nießen)
    #18644
* fs:
  - support as and as+ flags in stringToFlags() (Sarat Addepalli)
    #18801
* tls:
  - expose Finished messages in TLSSocket (Anton Salikhmetov)
    #19102
* tty:
  - Add getColorDepth function to determine if terminal supports colors
    (Ruben Bridgewater) #17615
* util:
  - add util.inspect compact option (Ruben Bridgewater)
    #17576
* **Added new collaborators**
  - [watson](https://github.com/watson) Thomas Watson

PR-URL: #19428
MylesBorins added a commit that referenced this pull request Mar 21, 2018
Notable changes:

* assert:
  - From now on all error messages produced by `assert` in strict mode
    will produce a error diff. (Ruben Bridgewater)
    #17615
  - From now on it is possible to use a validation object in throws
    instead of the other possibilities. (Ruben Bridgewater)
    #17584
* crypto:
  - allow passing null as IV unless required (Tobias Nießen)
    #18644
* fs:
  - support as and as+ flags in stringToFlags() (Sarat Addepalli)
    #18801
* tls:
  - expose Finished messages in TLSSocket (Anton Salikhmetov)
    #19102
* tty:
  - Add getColorDepth function to determine if terminal supports colors
    (Ruben Bridgewater) #17615
* util:
  - add util.inspect compact option (Ruben Bridgewater)
    #17576
* **Added new collaborators**
  - [watson](https://github.com/watson) Thomas Watson

PR-URL: #19428
@MylesBorins
Copy link
Contributor

Requested backport to 8.x in #19230

@BridgeAR BridgeAR deleted the util-structured branch April 1, 2019 23:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
notable-change PRs with changes that should be highlighted in changelogs. semver-minor PRs that contain new features and should be released in the next minor version. util Issues and PRs related to the built-in util module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants