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

doc: changed util.inspect signature #23216

Closed
wants to merge 6 commits into from
Closed

doc: changed util.inspect signature #23216

Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
605a893
doc : changed util.inspect signature
siddhant1 Oct 2, 2018
127ef47
doc: changed util.inspect signature
siddhant1 Oct 2, 2018
4bc07c2
Update util.md
siddhant1 Oct 2, 2018
cf6409f
doc: changed util.inspect signature
siddhant1 Oct 2, 2018
3c391c1
doc: changed util.inspect signature
siddhant1 Oct 3, 2018
57cc11a
doc: changed util.inspect signature
siddhant1 Oct 7, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion doc/api/util.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@

vsemozhetbyt marked this conversation as resolved.
Show resolved Hide resolved
# Util

<!--introduced_in=v0.10.0-->
Expand Down Expand Up @@ -356,7 +357,9 @@ stream.on('data', (data) => {
stream.write('With ES6');
```

## util.inspect(object[, options])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not seem to be completely correct. You want to add the legacy signature which is:

## util.inspect(object[, showHidden[, depth[, colors]]])

The options signature is not compatible with it since it's not possible to combine the legacy one and the new one. So a separate entry would be required in this case. However, I am not fond of actually documenting it. It uses boolean arguments and those are difficult to grasp without actively looking into the documentation.

Copy link
Contributor Author

@siddhant1 siddhant1 Oct 2, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Somebody rose an issue on this one , are you sure you don't want it documented

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However, I am not fond of actually documenting it. It uses boolean arguments and those are difficult to grasp without actively looking into the documentation.

If the signature is "difficult to grasp without actively looking into the documentation", that would seem to argue for documenting it. I'd certainly be in favor of applying a doc-only deprecation to that signature (if it's not already deprecated) though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Specifically: If someone comes across util.inspect() being used this way, they should be able to find the signature in the documentation. The deprecation would be to discourage its further use this way. But people should still be able to use the docs to help them figure out code that already exists.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you tell me the correct signature so I can make another pr on this one?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe there's needs to be two separate signatures, one for when there is an options object and one for when there are the other arguments.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I'm guessing the answer to @siddhant1's question is:

## util.inspect(object[, showHidden[, depth[, colors]]])

AND

## util.inspect(object[, options])

Is that right @Trott, @refack?


vsemozhetbyt marked this conversation as resolved.
Show resolved Hide resolved
## util.inspect(object[,options]
Copy link
Contributor

@vsemozhetbyt vsemozhetbyt Oct 2, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing space after the comma and missing closing parenthesis) Should be the same line:

## util.inspect(object[, options])

## util.inspect(object, [showHidden], [depth], [colors])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We usually enclose commas and spaces inside brackets for optional parameters:

## util.inspect(object[, showHidden][, depth][, colors])

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or this one if optionality is incremental:

## util.inspect(object[, showHidden[, depth[, colors]]])

<!-- YAML
added: v0.3.0
changes:
Expand Down Expand Up @@ -435,6 +438,7 @@ changes:
function, it is used as a [compare function][].
* Returns: {string} The representation of passed object


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This empty line seems redundant)

The `util.inspect()` method returns a string representation of `object` that is
intended for debugging. The output of `util.inspect` may change at any time
and should not be depended upon programmatically. Additional `options` may be
Expand Down