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

Making util.inspect recurse indefinitely by passing Infinity instead of null #20366

Closed
MarkTiedemann opened this issue Apr 27, 2018 · 7 comments
Labels
doc Issues and PRs related to the documentations. util Issues and PRs related to the built-in util module.

Comments

@MarkTiedemann
Copy link
Contributor

MarkTiedemann commented Apr 27, 2018

  • Version: v10.0.0
  • Platform: Microsoft Windows [Version 10.0.16299.371]
  • Subsystem: util

I'm wondering about the depth option for util.inspect.

The documentation states "To make it recurse indefinitely, pass null."

I think it would be more intuitive to pass Infinity to make it recurse indefinitely.

Would a change like this be accepted?

@BridgeAR
Copy link
Member

A documentation update like that would indeed be welcome. It should still say that null is supported though.

@BridgeAR BridgeAR added doc Issues and PRs related to the documentations. util Issues and PRs related to the built-in util module. labels Apr 27, 2018
@MarkTiedemann
Copy link
Contributor Author

MarkTiedemann commented Apr 27, 2018

@BridgeAR Should the code be updated, too? For example, should the default options be changed?

node/lib/util.js

Lines 81 to 83 in 2b85127

const inspectDefaultOptions = Object.seal({
showHidden: false,
depth: null,

Haven't looked into it too deeply yet, but I suspect there might be other places that probably should be updated, too.

@ryzokuken
Copy link
Contributor

Personal opinion, while we still need to support null for backwards compatibility, it does send the wrong message, and I believe we should use Infinity over null for signifying Infinite depth.

@jvelezpo
Copy link
Contributor

jvelezpo commented Apr 28, 2018

Looking at the current doc on master looks like this was already done, it also changes depth default from 2 to Infinity
👍
image

@MarkTiedemann
Copy link
Contributor Author

Second time contributor here. Sorry I missed that.

Seems like that change was already done as part of #17907, which is a semver major change (if I understood correctly, because the default depth was changed from 2 to Infinity) that didn't make it into v10.0.0, but is now in master.

Do you still think it's a good idea to just change the docs and change the internal implementation so it is a non-breaking change (more or less, if (depth === null) depth = Infinity;), without changing the default depth? I think, if that is done first in a patch release now, it could prepare people for the upcoming semver-major change. :)

@BridgeAR
Copy link
Member

@MarkTiedemann

I think, if that is done first in a patch release now, it could prepare people for the upcoming semver-major change.

It is already possible to do that right now and there should not be any difference for a user if that is documented or not. I suggest to target 10.x to update the docs there.

Seems like that change was already done as part of #17907

That is correct and it is currently unclear if that will be reverted or not. It is definitely going to get a change, how ever that will look like.

@BridgeAR
Copy link
Member

Closing this as the current documentation already mentions Infinity.

Offroaders123 added a commit to Offroaders123/NBTify that referenced this issue Jan 19, 2024
Now the format of the output file is combined into a full object, which is then used by the main file, rather than importing all of the arguments as individual flags.

Added a flag to specify the SNBT indentation spacing! It was only set to 2 before, now you can have fully minified SNBT, or use any indentation you'd like. Like the programming NBTify API, it accepts a number or a string.

Removed the `--pipe` flag, in favor of simply specifying whether you want to use an NBT or SNBT output to stdout. If neither `--nbt` or `--snbt` are passed as flags, it will simply log out the structure of the file, with nice pretty-printing, colors, and such, as it currently has been doing thus far.

The pretty-print logging now fully logs out the NBT file's tree, other than for TypedArray-based tags. For everything else though, you see all of it's content, rather than being abbreviated (the normal behavior for `console.log()` in Node.js).

Learned about the difference between `isNan()` and `Number.isNaN()`. The first one coerces the input value, checks if it's `NaN`, while the other simply checks what it is currently (doesn't convert to `number` first).

#25

microsoft/TypeScript#31025
https://stackoverflow.com/questions/56143158/how-to-use-util-promisify-and-bind-functions-in-nodejs (I think I ran into this issue previously, but I didn't know that function binding would work nicely here!)
https://stackoverflow.com/questions/43362222/nodejs-short-alias-for-process-stdout-write
https://askubuntu.com/questions/510890/how-do-i-redirect-command-output-to-vim-in-bash (Looked into this again, was curious in whether it's viable to edit SNBT with Vim again)

Oh yeah, that reminds me, I also made it so SNBT is added to stdout with a trailing `\n`, so it nicer looking in the terminal, as well as in the editor, as most editors utilize trailing new lines. I think those are finally catching on with me! They really bugged me for some reason, since the start of my programming lol. Now I'm getting Linux'ed haha.

nodejs/node#20366
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/isNaN
https://www.google.com/search?q=comments+in+powershell
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. util Issues and PRs related to the built-in util module.
Projects
None yet
Development

No branches or pull requests

4 participants