doc: changed util.inspect signature #23216

siddhant1 · 2018-10-02T11:13:51Z

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
documentation is changed or added
commit message follows commit guidelines

BridgeAR · 2018-10-02T11:25:31Z

doc/api/util.md

@@ -356,7 +356,7 @@ stream.on('data', (data) => {
 stream.write('With ES6');
 ```

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


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.

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

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.

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

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

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.

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 · 2018-10-02T12:41:04Z

Refs: #23205

siddhant1 · 2018-10-02T15:15:17Z

Should I add a legacy or deprecated patch to the legacy one?

refack · 2018-10-02T15:59:18Z

Should I add a legacy or deprecated patch to the legacy one?

IMHO it should be documented as having two signatures. I thought I saw an example of such polymorphism in our docs, but I can't find it at the moment.

P.S. I found a not-so-good example https://nodejs.org/api/fs.html#fs_fs_readfilesync_path_options
But IMHO util.inspect should be documented as having two distinct signatures.

siddhant1 · 2018-10-02T16:38:34Z

Okay I will take a look at previously documented API's and work on this one.

vsemozhetbyt · 2018-10-02T18:13:14Z

Polymorphic signatures:
https://nodejs.org/api/console.html#console_new_console_stdout_stderr_ignoreerrors
https://nodejs.org/api/https.html#https_https_get_options_callback
https://nodejs.org/api/https.html#https_https_request_options_callback
https://nodejs.org/api/http.html#http_http_request_options_callback
https://nodejs.org/api/http.html#http_http_get_options_callback

siddhant1 · 2018-10-02T18:48:05Z

Thanks for the help dude! , I am just starting out and I am glad to see such a healthy community.

vsemozhetbyt · 2018-10-02T19:06:30Z

doc/api/util.md

@@ -356,7 +357,9 @@ stream.on('data', (data) => {
 stream.write('With ES6');
 ```

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


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

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

vsemozhetbyt · 2018-10-02T19:08:17Z

doc/api/util.md

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


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

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

Or this one if optionality is incremental:

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

vsemozhetbyt · 2018-10-02T19:09:26Z

doc/api/util.md

@@ -435,6 +438,7 @@ changes:
    function, it is used as a [compare function][].
 * Returns: {string} The representation of passed object

+


This empty line seems redundant)

vsemozhetbyt · 2018-10-06T13:02:55Z

doc/api/util.md

@@ -356,7 +357,9 @@ stream.on('data', (data) => {
 stream.write('With ES6');
 ```

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


Nit: two spaces after the ## produce unneeded diff.

doc/api/util.md

vsemozhetbyt · 2018-10-06T13:05:30Z

doc/api/util.md

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


So should not this be ## util.inspect(object[, showHidden[, depth[, colors]]])?

I am doing all these changes and squashing the commits into one

siddhant1 · 2018-10-07T10:14:26Z

@vsemozhetbyt can you check them now!

vsemozhetbyt · 2018-10-07T10:18:06Z

Thank you. Some extra empty lines can be deleted on landing.

CI-lite: https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/1154/ (green)

vsemozhetbyt · 2018-10-07T20:18:23Z

Dear reviewers, PTAL.

vsemozhetbyt · 2018-10-08T20:20:39Z

Landed in 5e63cf2
Thank you!

PR-URL: #23216 Fixes: #23205 Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: James M Snell <[email protected]>

siddhant1 added 2 commits October 2, 2018 16:21

doc : changed util.inspect signature

605a893

doc: changed util.inspect signature

127ef47

nodejs-github-bot added doc Issues and PRs related to the documentations. util Issues and PRs related to the built-in util module. labels Oct 2, 2018

BridgeAR reviewed Oct 2, 2018

View reviewed changes

Update util.md

4bc07c2

doc: changed util.inspect signature

cf6409f

vsemozhetbyt reviewed Oct 2, 2018

View reviewed changes

doc: changed util.inspect signature

3c391c1

vsemozhetbyt reviewed Oct 6, 2018

View reviewed changes

doc/api/util.md Show resolved Hide resolved

vsemozhetbyt reviewed Oct 6, 2018

View reviewed changes

doc/api/util.md Show resolved Hide resolved

vsemozhetbyt reviewed Oct 6, 2018

View reviewed changes

doc: changed util.inspect signature

57cc11a

vsemozhetbyt approved these changes Oct 7, 2018

View reviewed changes

jasnell approved these changes Oct 8, 2018

View reviewed changes

vsemozhetbyt added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Oct 8, 2018

vsemozhetbyt closed this Oct 8, 2018

vsemozhetbyt pushed a commit that referenced this pull request Oct 8, 2018

doc: add util.inspect() legacy signature

5e63cf2

PR-URL: #23216 Fixes: #23205 Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: James M Snell <[email protected]>

targos pushed a commit that referenced this pull request Oct 10, 2018

doc: add util.inspect() legacy signature

eee71d6

PR-URL: #23216 Fixes: #23205 Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: James M Snell <[email protected]>

targos mentioned this pull request Oct 10, 2018

Release proposal: v10.12.0 #23313

Merged

jasnell pushed a commit that referenced this pull request Oct 17, 2018

doc: add util.inspect() legacy signature

80a86e6

PR-URL: #23216 Fixes: #23205 Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: James M Snell <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

doc: changed util.inspect signature #23216

doc: changed util.inspect signature #23216

siddhant1 commented Oct 2, 2018 •

edited

Loading

BridgeAR Oct 2, 2018

siddhant1 Oct 2, 2018 •

edited

Loading

Trott Oct 2, 2018

Trott Oct 2, 2018

siddhant1 Oct 2, 2018

Trott Oct 2, 2018

bnb Oct 2, 2018

vsemozhetbyt commented Oct 2, 2018

siddhant1 commented Oct 2, 2018

refack commented Oct 2, 2018 •

edited

Loading

siddhant1 commented Oct 2, 2018

vsemozhetbyt commented Oct 2, 2018

siddhant1 commented Oct 2, 2018

vsemozhetbyt Oct 2, 2018 •

edited

Loading

vsemozhetbyt Oct 2, 2018

vsemozhetbyt Oct 2, 2018

vsemozhetbyt Oct 2, 2018

vsemozhetbyt Oct 6, 2018

vsemozhetbyt Oct 6, 2018

siddhant1 Oct 6, 2018

siddhant1 commented Oct 7, 2018

vsemozhetbyt commented Oct 7, 2018 •

edited

Loading

vsemozhetbyt commented Oct 7, 2018

vsemozhetbyt commented Oct 8, 2018

		@@ -435,6 +438,7 @@ changes:
		function, it is used as a [compare function][].
		* Returns: {string} The representation of passed object

doc: changed util.inspect signature #23216

doc: changed util.inspect signature #23216

Conversation

siddhant1 commented Oct 2, 2018 • edited Loading

Checklist

Choose a reason for hiding this comment

siddhant1 Oct 2, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vsemozhetbyt commented Oct 2, 2018

siddhant1 commented Oct 2, 2018

refack commented Oct 2, 2018 • edited Loading

siddhant1 commented Oct 2, 2018

vsemozhetbyt commented Oct 2, 2018

siddhant1 commented Oct 2, 2018

vsemozhetbyt Oct 2, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

siddhant1 commented Oct 7, 2018

vsemozhetbyt commented Oct 7, 2018 • edited Loading

vsemozhetbyt commented Oct 7, 2018

vsemozhetbyt commented Oct 8, 2018

siddhant1 commented Oct 2, 2018 •

edited

Loading

siddhant1 Oct 2, 2018 •

edited

Loading

refack commented Oct 2, 2018 •

edited

Loading

vsemozhetbyt Oct 2, 2018 •

edited

Loading

vsemozhetbyt commented Oct 7, 2018 •

edited

Loading