util: allow symbol-based custom inspection methods #8174

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
6 participants
@addaleax
Member

addaleax commented Aug 18, 2016

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.

@jasnell

View changes

lib/util.js
+ maybeCustomInspect !== exports.inspect &&
+ // Also filter out any prototype objects using the circular check.
+ !(value.constructor && value.constructor.prototype === value)) {
+ let ret = maybeCustomInspect.call(value, recurseTimes, ctx);

This comment has been minimized.

@jasnell

jasnell Aug 18, 2016

Member

nit: looks like the indent is off here

@jasnell

jasnell Aug 18, 2016

Member

nit: looks like the indent is off here

This comment has been minimized.

@addaleax

addaleax Aug 18, 2016

Member

@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.

@addaleax

addaleax Aug 18, 2016

Member

@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.

This comment has been minimized.

@Trott

Trott Aug 18, 2016

Member

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!!!

@Trott

Trott Aug 18, 2016

Member

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!!!

This comment has been minimized.

@jasnell

jasnell Aug 18, 2016

Member

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

@jasnell

jasnell Aug 18, 2016

Member

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

This comment has been minimized.

@addaleax

addaleax Aug 18, 2016

Member

@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

addaleax Aug 18, 2016

Member

@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. :)

This comment has been minimized.

@Trott

Trott Aug 18, 2016

Member

@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

Trott Aug 18, 2016

Member

@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();
}

This comment has been minimized.

@addaleax

addaleax Aug 18, 2016

Member

@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.

@addaleax

addaleax Aug 18, 2016

Member

@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.

This comment has been minimized.

@Trott

Trott Aug 19, 2016

Member

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

@Trott

Trott Aug 19, 2016

Member

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

This comment has been minimized.

@Trott

Trott Aug 19, 2016

Member

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

@Trott

Trott Aug 19, 2016

Member

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

This comment has been minimized.

@addaleax

addaleax Aug 19, 2016

Member

Again, no need to apologize. :)

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

#holy-war ? ;)

@addaleax

addaleax Aug 19, 2016

Member

Again, no need to apologize. :)

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

#holy-war ? ;)

@jasnell

View changes

doc/api/util.md
return { bar: 'baz' };
};
util.inspect(obj);
// "{ bar: 'baz' }"
```
+The same functionality can also be provided by `inspect(depth, opts)` when
+no `util.inspect.Custom` method is present on an object.

This comment has been minimized.

@jasnell

jasnell Aug 18, 2016

Member

Perhaps instead:

A custom inspection method can alternatively be provided by exposing
an `inspect(depth, opts)` method on the object:

Then include the equivalent example.

@jasnell

jasnell Aug 18, 2016

Member

Perhaps instead:

A custom inspection method can alternatively be provided by exposing
an `inspect(depth, opts)` method on the object:

Then include the equivalent example.

@jasnell

View changes

doc/api/util.md
+-->
+
+A Symbol that can be used to declare custom inspect functions, as described
+above.

This comment has been minimized.

@jasnell

jasnell Aug 18, 2016

Member

Perhaps make this a link to the section rather than a relative marker.

@jasnell

jasnell Aug 18, 2016

Member

Perhaps make this a link to the section rather than a relative marker.

@jasnell

View changes

lib/buffer.js
@@ -500,7 +500,8 @@ Buffer.prototype.equals = function equals(b) {
};
-// Inspect
+// Override how buffers are presented by util.inspect().
+Buffer.prototype[internalUtil.inspectSymbol] =
Buffer.prototype.inspect = function inspect() {

This comment has been minimized.

@jasnell

jasnell Aug 18, 2016

Member

Just a nit but... Perhaps instead:

Buffer.prototype[internalUtil.inspectSymbol] = function inspect() { ... };
Buffer.prototype.inspect = Buffer.prototype[internalUtil.inspectSymbol];

I know they accomplish the same thing, but this would make it a bit more straightforward to deprecate and eventually remove the Buffer.prototype.inspect variant later on.

@jasnell

jasnell Aug 18, 2016

Member

Just a nit but... Perhaps instead:

Buffer.prototype[internalUtil.inspectSymbol] = function inspect() { ... };
Buffer.prototype.inspect = Buffer.prototype[internalUtil.inspectSymbol];

I know they accomplish the same thing, but this would make it a bit more straightforward to deprecate and eventually remove the Buffer.prototype.inspect variant later on.

@jasnell

This comment has been minimized.

Show comment
Hide comment
@jasnell

jasnell Aug 18, 2016

Member

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.

Member

jasnell commented Aug 18, 2016

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.

@addaleax

This comment has been minimized.

Show comment
Hide comment
@addaleax

addaleax Aug 18, 2016

Member

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().

Member

addaleax commented Aug 18, 2016

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().

@jasnell

This comment has been minimized.

Show comment
Hide comment
@jasnell

jasnell Aug 19, 2016

Member

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

Member

jasnell commented Aug 19, 2016

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

@targos

This comment has been minimized.

Show comment
Hide comment
@targos

targos Aug 19, 2016

Member

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

Member

targos commented Aug 19, 2016

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

@addaleax

This comment has been minimized.

Show comment
Hide comment
@addaleax

addaleax Aug 21, 2016

Member

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

Member

addaleax commented Aug 21, 2016

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

@jasnell

This comment has been minimized.

Show comment
Hide comment
@jasnell

jasnell Aug 22, 2016

Member

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
.

Member

jasnell commented Aug 22, 2016

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
.

@addaleax

This comment has been minimized.

Show comment
Hide comment
@addaleax

addaleax Aug 22, 2016

Member

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

Member

addaleax commented Aug 22, 2016

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

@jasnell

This comment has been minimized.

Show comment
Hide comment
@jasnell

jasnell Aug 22, 2016

Member

LGTM

Member

jasnell commented Aug 22, 2016

LGTM

addaleax added some commits Aug 18, 2016

util: allow symbol-based custom inspection methods
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
util: allow returning `this` from custom inspect
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.
@addaleax

This comment has been minimized.

Show comment
Hide comment
@addaleax

addaleax Aug 24, 2016

Member

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

Member

addaleax commented Aug 24, 2016

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

@targos

This comment has been minimized.

Show comment
Hide comment
@targos

targos Aug 24, 2016

Member

LGTM

Member

targos commented Aug 24, 2016

LGTM

@addaleax

This comment has been minimized.

Show comment
Hide comment
@addaleax

addaleax Aug 25, 2016

Member

Landed in 59714cb & a60ed89

Member

addaleax commented Aug 25, 2016

Landed in 59714cb & a60ed89

@addaleax addaleax closed this Aug 25, 2016

@addaleax addaleax deleted the addaleax:util-inspect-symbol2 branch Aug 25, 2016

addaleax added a commit that referenced this pull request Aug 25, 2016

util: allow symbol-based custom inspection methods
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>

addaleax added a commit that referenced this pull request Aug 25, 2016

util: allow returning `this` from custom inspect
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>

@Fishrock123 Fishrock123 referenced this pull request Sep 6, 2016

Closed

v6.6.0 pre-proposal #8428

addaleax added a commit to addaleax/node that referenced this pull request Sep 7, 2016

util: allow symbol-based custom inspection methods
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>

addaleax added a commit to addaleax/node that referenced this pull request Sep 7, 2016

util: allow returning `this` from custom inspect
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>

Fishrock123 added a commit to Fishrock123/node that referenced this pull request Sep 8, 2016

util: allow symbol-based custom inspection methods
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>

Fishrock123 added a commit to Fishrock123/node that referenced this pull request Sep 8, 2016

util: allow returning `this` from custom inspect
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>

Fishrock123 added a commit to Fishrock123/node that referenced this pull request Sep 8, 2016

util: allow symbol-based custom inspection methods
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>

Fishrock123 added a commit to Fishrock123/node that referenced this pull request Sep 8, 2016

util: allow returning `this` from custom inspect
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>

Fishrock123 added a commit that referenced this pull request Sep 9, 2016

util: allow symbol-based custom inspection methods
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>

Fishrock123 added a commit that referenced this pull request Sep 9, 2016

util: allow returning `this` from custom inspect
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>

@Fishrock123 Fishrock123 referenced this pull request Sep 9, 2016

Merged

v6.6.0 proposal #8466

Fishrock123 added a commit that referenced this pull request Sep 14, 2016

2016-09-14, Version 6.6.0 (Current)
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

Fishrock123 added a commit to Fishrock123/node that referenced this pull request Sep 15, 2016

2016-09-14, Version 6.6.0 (Current)
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

imyller added a commit to imyller/meta-nodejs that referenced this pull request Sep 15, 2016

2016-09-14, Version 6.6.0 (Current)
    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>

imyller added a commit to imyller/meta-nodejs that referenced this pull request Sep 15, 2016

2016-09-14, Version 6.6.0 (Current)
    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>

@black-snow black-snow referenced this pull request in szwacz/fs-jetpack Sep 16, 2016

Closed

Console.log crashes fs-jetpack #29

@MylesBorins

This comment has been minimized.

Show comment
Hide comment
@MylesBorins

MylesBorins Nov 18, 2016

Member

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

edit: if so it needs to include #9289

Member

MylesBorins commented Nov 18, 2016

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

edit: if so it needs to include #9289

@addaleax

This comment has been minimized.

Show comment
Hide comment
@addaleax

addaleax Nov 18, 2016

Member

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

EDIT: #9688

Member

addaleax commented Nov 18, 2016

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

EDIT: #9688

addaleax added a commit to addaleax/node that referenced this pull request Nov 18, 2016

util: allow symbol-based custom inspection methods
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>

addaleax added a commit to addaleax/node that referenced this pull request Nov 18, 2016

util: allow returning `this` from custom inspect
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 addaleax referenced this pull request Nov 18, 2016

Closed

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

4 of 4 tasks complete

@ljharb ljharb referenced this pull request Jan 15, 2018

Closed

util: deprecate obj.inspect for custom inspection #15631

3 of 3 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment