-
Notifications
You must be signed in to change notification settings - Fork 30k
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: include reference anchor for circular structures #27685
Conversation
This adds a reference anchor to circular structures when using `util.inspect`. That way it's possible to identify with what object the circular reference corresponds too.
c1c7595
to
59adf42
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it’s sufficient to either pick the asterisk or ref
as a symbol, using both seems to make the output a bit verbose, but either way 👍
@addaleax I played around with it and this just seemed the most obvious and hopefully straight forward to understand. It does also look nice to just use I also thought about using |
@nodejs/tsc @nodejs/releasers I currently wonder if it would make sense to postpone landing this PR shortly before the next semver-major release date instead of in the next few days. The reason for this consideration is that this PR is not possible to be backported in a semver-patch manner, even with a compatibility patch (almost all changed code lines are test changes and most other changed lines would also have to be reverted). That would reduce the number of conflicts during the regular releases and that's something we try to achieve in the release working group at the moment. We try to take new measures against conflicts (as e.g., backporting semver-major releases with compatibility patches). I am not sure if this is something we might want to give a try or not but I thought it's worth bringing it up and it would be great to get some feedback! |
It's might be just as much work as resolving the conflicts to land the semver-major PRs later (including a minimal new review). So it might not be worth it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! I like the idea.
@BridgeAR I think it’s fine to land this. If you want to wait, I think it’s best to switch the labels |
This adds a reference anchor to circular structures when using `util.inspect`. That way it's possible to identify with what object the circular reference corresponds too. PR-URL: nodejs#27685 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Landed in 9f71dbc 🎉 |
This adds a reference anchor to circular structures when using `util.inspect`. That way it's possible to identify with what object the circular reference corresponds too. PR-URL: nodejs#27685 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
This patch makes sure no breaking changes from the referenced semver-major PR are backported. Refs: nodejs#27685
This adds a reference anchor to circular structures when using `util.inspect`. That way it's possible to identify with what object the circular reference corresponds too. PR-URL: #27685 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
…rcular refs In nodejs/node#27685 (part of node v14), how objects with circular references are stringified with `util.inspect` changed.
…rcular refs In nodejs/node#27685 (part of node v14), how objects with circular references are stringified with `util.inspect` changed.
This adds a reference anchor to circular structures when using
util.inspect
. That way it's possible to identify with what objectthe circular reference corresponds too.
// cc @nodejs/util
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes