util: allow symbol-based custom inspection methods by addaleax · Pull Request #8174 · nodejs/node · GitHub

Checklist

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

Affected core subsystem(s)

util

Description of change

Add a util.inspect.Custom Symbol which can be used to customize util.inspect() output. Providing obj[util.inspect.Custom] works like providing obj.inspect, except that the former allows avoiding name clashes with other inspect() methods.

Fixes: #8071

I’d appreciate feedback both on the symbol’s name and the way I changed the docs on custom inspection functions.

nit: looks like the indent is off here

@jasnell This line is indented two spaces more than the opening if ( is, I think that’s the general rule. I know the code looks a bit weird, yes.

Alas, this is probably a result of our style choice of aligning the separate lines of the conditional.

At the risk of going off on a low-value irrelevant tangent: I have come to greatly dislike all the alignment stuff and wouldn't mind (at a minimum) removing the custom rules in ESLint that enforces it.

The main problems with alignment (from my perspective) are:

Does not allow for modification in situations like this where alignment (which can wind up at any column) diminishes ease of scanning and understanding due to clashing with fixed indentation. Fixed indentation is more important than alignment IMO and so we should just ditch alignment and use fixed indentation or (more likely, given the churn) no rules at all everywhere that we are using alignment.
As others have pointed out: Alignment looks great when you first do it. Then one line of the fourteen that are aligned gets changed, and you have to change all the others, resulting in more churn.

By the way, I'm pretty sure I wrote the custom rules that enforce alignment (after I got a nit or three about my failure to align stuff, probably), so I'll add: SORRY!!!

ok, I see it now... visually it looks off at first glance

@Trott For the most part I think the alignment rules are okay, I see no need to change that or apologize for them. ;)

What I like to do in cases like this is actually not changing the alignment, but pulling the opening { into its own line… I assume people here (and the linter) aren’t fans of that either, so it’s probably okay to leave it as it is right now. :)

@addaleax I like to put the first part of the conditional on its own line so that it gets fixed indentation, but I'm also not sure others are a fan of that:

if (
  somethingReallyLong &&
  somethingElseThatIsAlsoReallyLong &&
  oneMoreReallyLongThing
) {
  doSomething();
}

@Trott … yeah … makes me wonder if Python is the only lang that got it right ;)

I’m going to leave this as it is if everyone’s okay with that.

Yeah, totally OK with that. Sorry for the distracting rant.

(Another rant for another time: Don't limit the line length to 80 characters?)

Again, no need to apologize. :)

(Another rant for another time: Don't limit the line length to 80 characters?)

#holy-war ? ;)

Generally LGTM, I'd add the example of using inspect() back into the docs then maybe include a statement that using the inspect() approach is no longer recommended and may become deprecated in the future.

Updated with nits addressed… CI: https://ci.nodejs.org/job/node-test-pull-request/3745/

I don’t know about recommending to no longer use inspect() – I doubt this is going to make it into v4.x, so anybody who wants to support v4 will need to stick to using inspect() for now or provide both methods, and there isn’t exactly something wrong with using inspect().

Ok, yep, you're right. No need to rush it

I find it weird that the Symbol's name starts with an uppercase. I don't know about other uses or best practice but it's not the case for well-known symbols like Symbol.iterator

I’m happy with an all-lowercase variant too, util.inspect.custom or something like that. Does anybody else have opinions?

Lowercase works for me

On Sunday, August 21, 2016, Anna Henningsen notifications@github.com
wrote:

I’m happy with an all-lowercase variant too, util.inspect.custom or
something like that. Does anybody else have opinions?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#8174 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAa2eRG7iU5PKxju3tGI4ztyUk-emQiaks5qiNx6gaJpZM4Jn_-o
.

Updated using util.inspect.custom, new CI: https://ci.nodejs.org/job/node-test-commit/4708/

LGTM

Add a `util.inspect.custom` Symbol which can be used to customize `util.inspect()` output. Providing `obj[util.inspect.custom]` works like providing `obj.inspect`, except that the former allows avoiding name clashes with other `inspect()` methods. Fixes: nodejs#8071

If a custom inspection function returned `this`, use that value for further formatting instead of going into infinite recursion. This is particularly useful when combined with `util.inspect.custom` because returning `this` from such a method makes it easy to have an `inspect()` function that is ignored by `util.inspect` without actually having to provide an alternative for custom inspection.

Rebased to resolve conflicts with #8189 … new CI: https://ci.nodejs.org/job/node-test-commit/4747/

LGTM

Landed in 59714cb & a60ed89

Add a `util.inspect.custom` Symbol which can be used to customize `util.inspect()` output. Providing `obj[util.inspect.custom]` works like providing `obj.inspect`, except that the former allows avoiding name clashes with other `inspect()` methods. Fixes: #8071 PR-URL: #8174 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michaël Zasso <mic.besace@gmail.com>

If a custom inspection function returned `this`, use that value for further formatting instead of going into infinite recursion. This is particularly useful when combined with `util.inspect.custom` because returning `this` from such a method makes it easy to have an `inspect()` function that is ignored by `util.inspect` without actually having to provide an alternative for custom inspection. PR-URL: #8174 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michaël Zasso <mic.besace@gmail.com>

Add a `util.inspect.custom` Symbol which can be used to customize `util.inspect()` output. Providing `obj[util.inspect.custom]` works like providing `obj.inspect`, except that the former allows avoiding name clashes with other `inspect()` methods. Fixes: nodejs#8071 PR-URL: nodejs#8174 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michaël Zasso <mic.besace@gmail.com>

If a custom inspection function returned `this`, use that value for further formatting instead of going into infinite recursion. This is particularly useful when combined with `util.inspect.custom` because returning `this` from such a method makes it easy to have an `inspect()` function that is ignored by `util.inspect` without actually having to provide an alternative for custom inspection. PR-URL: nodejs#8174 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michaël Zasso <mic.besace@gmail.com>

Add a `util.inspect.custom` Symbol which can be used to customize `util.inspect()` output. Providing `obj[util.inspect.custom]` works like providing `obj.inspect`, except that the former allows avoiding name clashes with other `inspect()` methods. Fixes: nodejs#8071 PR-URL: nodejs#8174 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michaël Zasso <mic.besace@gmail.com> PR-URL: nodejs#8437 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>

If a custom inspection function returned `this`, use that value for further formatting instead of going into infinite recursion. This is particularly useful when combined with `util.inspect.custom` because returning `this` from such a method makes it easy to have an `inspect()` function that is ignored by `util.inspect` without actually having to provide an alternative for custom inspection. PR-URL: nodejs#8174 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michaël Zasso <mic.besace@gmail.com> PR-URL: nodejs#8437 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>

Add a `util.inspect.custom` Symbol which can be used to customize `util.inspect()` output. Providing `obj[util.inspect.custom]` works like providing `obj.inspect`, except that the former allows avoiding name clashes with other `inspect()` methods. Fixes: nodejs#8071 PR-URL: nodejs#8174 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michaël Zasso <mic.besace@gmail.com> Refs: nodejs#8437 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>

If a custom inspection function returned `this`, use that value for further formatting instead of going into infinite recursion. This is particularly useful when combined with `util.inspect.custom` because returning `this` from such a method makes it easy to have an `inspect()` function that is ignored by `util.inspect` without actually having to provide an alternative for custom inspection. PR-URL: nodejs#8174 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michaël Zasso <mic.besace@gmail.com> Refs: nodejs#8437 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>

Add a `util.inspect.custom` Symbol which can be used to customize `util.inspect()` output. Providing `obj[util.inspect.custom]` works like providing `obj.inspect`, except that the former allows avoiding name clashes with other `inspect()` methods. Fixes: #8071 PR-URL: #8174 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michaël Zasso <mic.besace@gmail.com> Refs: #8437 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>

If a custom inspection function returned `this`, use that value for further formatting instead of going into infinite recursion. This is particularly useful when combined with `util.inspect.custom` because returning `this` from such a method makes it easy to have an `inspect()` function that is ignored by `util.inspect` without actually having to provide an alternative for custom inspection. PR-URL: #8174 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michaël Zasso <mic.besace@gmail.com> Refs: #8437 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>

Notable changes: * crypto: Added `crypto.timingSafeEqual()`. (not-an-aardvark) #8304 * events: Made the "max event listeners" memory leak warning more accessible. (Anna Henningsen) #8298 * promises: Unhandled rejections now emit a process warning after the first tick. (Benjamin Gruenbaum) #8223 * repl: Added auto alignment for `.editor` mode. (Prince J Wesley) #8241 * util: Some functionality has been added to `util.inspect()`: - Returning `this` from a custom inspect function now works. (Anna Henningsen) #8174 - Added support for Symbol-based custom inspection methods. (Anna Henningsen) #8174 Refs: #8428 Refs: #8457 PR-URL: #8466

Notable changes: * crypto: Added `crypto.timingSafeEqual()`. (not-an-aardvark) nodejs#8304 * events: Made the "max event listeners" memory leak warning more accessible. (Anna Henningsen) nodejs#8298 * promises: Unhandled rejections now emit a process warning after the first tick. (Benjamin Gruenbaum) nodejs#8223 * repl: Added auto alignment for `.editor` mode. (Prince J Wesley) nodejs#8241 * util: Some functionality has been added to `util.inspect()`: - Returning `this` from a custom inspect function now works. (Anna Henningsen) nodejs#8174 - Added support for Symbol-based custom inspection methods. (Anna Henningsen) nodejs#8174 Refs: nodejs#8428 Refs: nodejs#8457 PR-URL: nodejs#8466

Notable changes: * crypto: Added `crypto.timingSafeEqual()`. (not-an-aardvark) nodejs/node#8304 * events: Made the "max event listeners" memory leak warning more accessible. (Anna Henningsen) nodejs/node#8298 * promises: Unhandled rejections now emit a process warning after the first tick. (Benjamin Gruenbaum) nodejs/node#8223 * repl: Added auto alignment for `.editor` mode. (Prince J Wesley) nodejs/node#8241 * util: Some functionality has been added to `util.inspect()`: - Returning `this` from a custom inspect function now works. (Anna Henningsen) nodejs/node#8174 - Added support for Symbol-based custom inspection methods. (Anna Henningsen) nodejs/node#8174 Signed-off-by: Ilkka Myller <ilkka.myller@nodefield.com>

@addaleax would we want to backport this to v4.x?

edit: if so it needs to include #9289

@thealphanerd I’ll open a backport PR and we can talk about that then (but generally I’d be in favour)

EDIT: #9688

Add a `util.inspect.custom` Symbol which can be used to customize `util.inspect()` output. Providing `obj[util.inspect.custom]` works like providing `obj.inspect`, except that the former allows avoiding name clashes with other `inspect()` methods. Fixes: nodejs#8071 PR-URL: nodejs#8174 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michaël Zasso <mic.besace@gmail.com>

If a custom inspection function returned `this`, use that value for further formatting instead of going into infinite recursion. This is particularly useful when combined with `util.inspect.custom` because returning `this` from such a method makes it easy to have an `inspect()` function that is ignored by `util.inspect` without actually having to provide an alternative for custom inspection. PR-URL: nodejs#8174 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michaël Zasso <mic.besace@gmail.com>

addaleax added util Issues and PRs related to the built-in util module. semver-minor PRs that contain new features and should be released in the next minor version. labels Aug 18, 2016

nodejs-github-bot added the buffer Issues and PRs related to the buffer subsystem. label Aug 18, 2016

addaleax mentioned this pull request Aug 18, 2016

console.log calls inspect function on passed object #8071

Closed

jasnell reviewed Aug 18, 2016
View reviewed changes

addaleax force-pushed the util-inspect-symbol2 branch from e85abcb to 4eb7232 Compare August 18, 2016 23:44

addaleax force-pushed the util-inspect-symbol2 branch 2 times, most recently from 3ef0b4c to 1124ead Compare August 22, 2016 07:51

addaleax added 2 commits August 24, 2016 10:54

addaleax force-pushed the util-inspect-symbol2 branch from 1124ead to 2dc6163 Compare August 24, 2016 08:54

addaleax closed this Aug 25, 2016

addaleax deleted the util-inspect-symbol2 branch August 25, 2016 05:22

Fishrock123 mentioned this pull request Sep 6, 2016

v6.6.0 pre-proposal #8428

Closed

addaleax mentioned this pull request Sep 7, 2016

Backport 8174 (symbol-based custom inspection methods) #8437

Closed

Fishrock123 mentioned this pull request Sep 9, 2016

v6.6.0 proposal #8466

Merged

black-snow mentioned this pull request Sep 16, 2016

Console.log crashes fs-jetpack szwacz/fs-jetpack#29

Closed

MylesBorins added the lts-watch-v4.x label Nov 18, 2016

addaleax mentioned this pull request Nov 18, 2016

v4.x: backport util.inspect.custom #9688

Closed

4 tasks

addaleax added dont-land-on-v4.x and removed lts-watch-v4.x labels Nov 18, 2016

ljharb mentioned this pull request Jan 15, 2018

util: deprecate obj.inspect for custom inspection #15631

Closed

3 tasks

util: allow symbol-based custom inspection methods by addaleax · Pull Request #8174 · nodejs/node · GitHub

Uh oh!

Uh oh!

util: allow symbol-based custom inspection methods - #8174

util: allow symbol-based custom inspection methods#8174addaleax wants to merge 2 commits intonodejs:masternodejs/node:masterfrom addaleax:util-inspect-symbol2addaleax/node:util-inspect-symbol2Copy head branch name to clipboard

Conversation

addaleax commented Aug 18, 2016

Checklist

Affected core subsystem(s)

Description of change

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jasnell commented Aug 18, 2016

Uh oh!

addaleax commented Aug 18, 2016

Uh oh!

jasnell commented Aug 19, 2016

Uh oh!

targos commented Aug 19, 2016

Uh oh!

addaleax commented Aug 21, 2016

Uh oh!

jasnell commented Aug 22, 2016

Uh oh!

addaleax commented Aug 22, 2016

Uh oh!

jasnell commented Aug 22, 2016

Uh oh!

addaleax commented Aug 24, 2016

Uh oh!

targos commented Aug 24, 2016

Uh oh!

addaleax commented Aug 25, 2016

Uh oh!

MylesBorins commented Nov 18, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

addaleax commented Nov 18, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

util: allow symbol-based custom inspection methods#8174
addaleax wants to merge 2 commits into
nodejs:masterfrom
addaleax:util-inspect-symbol2

MylesBorins commented Nov 18, 2016 •
edited
Loading

addaleax commented Nov 18, 2016 •
edited
Loading