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

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

Closed
wants to merge 4 commits into from
Closed

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

wants to merge 4 commits into from

Conversation

addaleax
Copy link
Member

@addaleax addaleax commented Sep 7, 2016

Backport #8189 (because the rest depends on that) and #8174, with one require() line added that is present in master but missing in v6.x.

/cc @Fishrock123

Trott and others added 4 commits September 7, 2016 23:29
@Trott @addaleax
test: refactor test-util-inspect
cdf0234 
* favor `assert.strictEqual()` and friends of `assert.equal()` etc.
* favor `.includes()` over `.indexOf()` for existence checks

PR-URL: nodejs#8189
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@addaleax
util: allow symbol-based custom inspection methods
5647c8c 
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
[squash] fixup for v6 by adding missing require()
763e8b9 
steal a line from nodejs#7207 to make things work
@addaleax
util: allow returning this from custom inspect
b0f14e6 
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>
@nodejs-github-bot nodejs-github-bot added buffer Issues and PRs related to the buffer subsystem. util Issues and PRs related to the built-in util module. v6.x labels Sep 7, 2016
@addaleax
Copy link
Member Author

addaleax commented Sep 7, 2016 •
edited
Loading

CI: https://ci.nodejs.org/job/node-test-commit/4948/
CITGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/380/ (because of the added internal/ reference in lib/buffer.js)

New CI, the last one got stuck: https://ci.nodejs.org/job/node-test-commit/4956/

@mscdex mscdex mentioned this pull request Sep 7, 2016
Closed
@bnoordhuis bnoordhuis added the semver-minor PRs that contain new features and should be released in the next minor version. label Sep 8, 2016
@jasnell
Copy link
Member

jasnell commented Sep 8, 2016

LGTM

Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request Sep 8, 2016
@Trott @Fishrock123
test: refactor test-util-inspect
f52d838 
* favor `assert.strictEqual()` and friends of `assert.equal()` etc.
* favor `.includes()` over `.indexOf()` for existence checks

PR-URL: nodejs#8189
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

PR-URL: nodejs#8437
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request Sep 8, 2016
@addaleax @Fishrock123
util: allow symbol-based custom inspection methods
1f2b871 
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 pushed a commit to Fishrock123/node that referenced this pull request Sep 8, 2016
@addaleax @Fishrock123
fixup for v6 by adding missing require()
5a3f574 
steal a line from nodejs#7207 to make things work

PR-URL: nodejs#8437
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request Sep 8, 2016
@addaleax @Fishrock123
util: allow returning this from custom inspect
f19bf16 
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
Copy link
Contributor

Thanks, CI seems green enough and fixes my problem so landed in Fishrock123/node@f829660...f19bf16

@Fishrock123 Fishrock123 closed this Sep 8, 2016
@Fishrock123
Copy link
Contributor

Err, make that f829660...f19bf16 on the actual repo here

@addaleax addaleax deleted the backport-8174 branch September 8, 2016 21:51
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request Sep 8, 2016
@Trott @Fishrock123
test: refactor test-util-inspect
6f269be 
* favor `assert.strictEqual()` and friends of `assert.equal()` etc.
* favor `.includes()` over `.indexOf()` for existence checks

PR-URL: nodejs#8189
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

Refs: nodejs#8437
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request Sep 8, 2016
@addaleax @Fishrock123
util: allow symbol-based custom inspection methods
954eeb1 
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 pushed a commit to Fishrock123/node that referenced this pull request Sep 8, 2016
@addaleax @Fishrock123
fixup for v6 by adding missing require()
9d4698c 
steal a line from nodejs#7207 to make things work

PR-URL: nodejs#8437
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request Sep 8, 2016
@addaleax @Fishrock123
util: allow returning this from custom inspect
c21bfae 
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
Copy link
Contributor

now f829660...c21bfae as I fixed a thing

@Fishrock123 Fishrock123 mentioned this pull request Sep 8, 2016
Closed
Fishrock123 pushed a commit that referenced this pull request Sep 9, 2016
@addaleax @Fishrock123
util: allow symbol-based custom inspection methods
4cb55ff 
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 pushed a commit that referenced this pull request Sep 9, 2016
@addaleax @Fishrock123
util: allow returning this from custom inspect
e9c4805 
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
buffer Issues and PRs related to the buffer subsystem. 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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@addaleax @jasnell @Fishrock123 @bnoordhuis @nodejs-github-bot @Trott