doc: add description for inspector-only console methods. by Tiriel · Pull Request #17004 · nodejs/node

Tiriel · 2017-11-13T23:26:15Z

Description inspired by dev tools reference and inspector err messages

Added:

intro
console.debug()
console.dirxml()
console.markTimeline()
console.profile()
console.profileEnd()
console.table()
console.timeStamp()
console.timeline()
console.timelineEnd()

Fixes: #16755

Checklist

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

Affected core subsystem(s)

doc

Trott · 2017-11-14T00:29:21Z

doc/api/console.md

I'd spell out what you mean by non-op rather than trust that readers will know what that means. So instead of are non-op, perhaps do not display anything or something like that.

Trott · 2017-11-14T00:30:29Z

Would it be better for users if we repeat the "these functions don't do anything if you're not in the inspector" warning for each method rather than trusting that users will see the notice at the top? I know I often only read the text that is directly under the function that I care about in the docs.

Tiriel · 2017-11-14T12:52:21Z

@Trott Thanks for the input! Just did it, hope it's okay

vsemozhetbyt · 2017-11-14T14:04:03Z

doc/api/console.md

A nit: undefined reference [`console.profileEnd()`][]

vsemozhetbyt · 2017-11-14T14:05:49Z

doc/api/console.md

A nit: undefined reference [`console.profile()`][]

vsemozhetbyt · 2017-11-14T14:11:50Z

doc/api/console.md

A nit: non-primitive types are usually noted in title case: {Array|Object}

vsemozhetbyt · 2017-11-14T14:18:06Z

doc/api/console.md

A nit: this reference should go after the [`console.timeEnd()`], ABC-wise.

jasnell · 2017-11-14T19:06:11Z

doc/api/console.md

having inspector be a link to debugger.html and/or inspector.html would be good here. debugger.html is the best choice.

Tiriel · 2017-11-14T19:26:22Z

Thanks! Changed according to each review :)

Trott · 2017-11-14T21:23:51Z

doc/api/console.md

Nit: Remove tab.

Trott · 2017-11-14T21:25:20Z

doc/api/console.md

Nit: This also needs the "does not display anything" line. Although I guess we could easily implement console.debug() if it's just an alias to console.log()...

Well, I can't say for sure, but that is what I saw in the inspector and what seems to indicate the DevTools ref: https://developers.google.com/web/tools/chrome-devtools/console/console-reference#consoledebugobject_object

Wouldn't it be better then if I leave this line like this and open another issue/PR to implement the method?

Trott · 2017-11-14T21:26:13Z

@Tiriel Thanks for your patience with all the nits. Doc updates can be like that.

Trott

Would like to see the recent round of nits addressed, but LGTM with or without them.

Tiriel · 2017-11-14T21:43:23Z

@Trott No problem at all, having correct api docs seems of vital importance to me. That's the source of truth for many users/devs, they need to be the best we can provide.

Removed the 'tab's!

tniessen · 2017-11-20T15:51:18Z

New CI: https://ci.nodejs.org/job/node-test-pull-request/11550/

Tiriel · 2017-11-20T17:30:38Z

Awesome, thanks!

Description inspired by dev tools reference and inspector err messages Added: * intro * console.debug() * console.dirxml() * console.markTimeline() * console.profile() * console.profileEnd() * console.table() * console.timeStamp() * console.timeline() * console.timelineEnd() PR-URL: #17004 Fixes: #16755 Reviewed-By: Khaidi Chu <i@2333.moe> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

jasnell · 2017-11-22T17:15:27Z

Landed in 982c674

Tiriel · 2017-11-22T20:32:35Z

Thanks!

Adds the console.debug() method, alias for console.log(). This method is exposed by V8 and was only available in inspector until now. Also adds matching test and documentation. PR-URL: #17033 Refs: #17004 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Khaidi Chu <i@2333.moe> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Timothy Gu <timothygu99@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

Description inspired by dev tools reference and inspector err messages Added: * intro * console.debug() * console.dirxml() * console.markTimeline() * console.profile() * console.profileEnd() * console.table() * console.timeStamp() * console.timeline() * console.timelineEnd() PR-URL: #17004 Fixes: #16755 Reviewed-By: Khaidi Chu <i@2333.moe> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

Adds the console.debug() method, alias for console.log(). This method is exposed by V8 and was only available in inspector until now. Also adds matching test and documentation. PR-URL: #17033 Refs: #17004 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Khaidi Chu <i@2333.moe> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Timothy Gu <timothygu99@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

Trott · 2018-12-08T17:38:37Z

Turns out this should have landed in v8.x. See #24909 (comment).

Miraculously, 982c674 still cherry-picks cleanly to the v8.x-staging branch. I've removed the dont-land-on-v8.x label. Is that enough for it to get cherry-picked into the next v8.x release?

(Pinging the last three people who did v8.x releases for an answer to that last question: @rvagg @BethGriggs @MylesBorins.)

Description inspired by dev tools reference and inspector err messages Added: * intro * console.debug() * console.dirxml() * console.markTimeline() * console.profile() * console.profileEnd() * console.table() * console.timeStamp() * console.timeline() * console.timelineEnd() PR-URL: #17004 Fixes: #16755 Reviewed-By: Khaidi Chu <i@2333.moe> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

MylesBorins · 2018-12-10T18:09:34Z

I've landed this in 8.x-staging

@BethGriggs can you rebase this into the current release proposal

nodejs-github-bot added console Issues and PRs related to the console subsystem. doc Issues and PRs related to the documentations. labels Nov 13, 2017

XadillaX approved these changes Nov 13, 2017

View reviewed changes

Trott reviewed Nov 14, 2017

View reviewed changes

Tiriel force-pushed the doc-add-console-noops branch from 8f59344 to 7ca8aa7 Compare November 14, 2017 12:51

vsemozhetbyt reviewed Nov 14, 2017

View reviewed changes

doc/api/console.md Outdated

Copy link

Contributor

vsemozhetbyt Nov 14, 2017

Choose a reason for hiding this comment

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

A nit: undefined reference [`console.profileEnd()`][]

vsemozhetbyt reviewed Nov 14, 2017

View reviewed changes

doc/api/console.md Outdated

Copy link

Contributor

vsemozhetbyt Nov 14, 2017

Choose a reason for hiding this comment

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

A nit: undefined reference [`console.profile()`][]

Tiriel reacted with thumbs up emoji

vsemozhetbyt reviewed Nov 14, 2017

View reviewed changes

doc/api/console.md Outdated

Copy link

Contributor

vsemozhetbyt Nov 14, 2017

Choose a reason for hiding this comment

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

A nit: non-primitive types are usually noted in title case: {Array|Object}

vsemozhetbyt approved these changes Nov 14, 2017

View reviewed changes

vsemozhetbyt reviewed Nov 14, 2017

View reviewed changes

doc/api/console.md Outdated

Copy link

Contributor

vsemozhetbyt Nov 14, 2017 •

edited

Loading

Choose a reason for hiding this comment

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

A nit: this reference should go after the [`console.timeEnd()`], ABC-wise.

Tiriel force-pushed the doc-add-console-noops branch from 7ca8aa7 to 02a3493 Compare November 14, 2017 14:22

jasnell reviewed Nov 14, 2017

View reviewed changes

jasnell approved these changes Nov 14, 2017

View reviewed changes

Tiriel force-pushed the doc-add-console-noops branch from 02a3493 to 99e297d Compare November 14, 2017 19:23

Trott reviewed Nov 14, 2017

View reviewed changes

doc/api/console.md Outdated

Copy link

Member

Trott Nov 14, 2017

Choose a reason for hiding this comment

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

Nit: Remove tab.

Trott reviewed Nov 14, 2017

View reviewed changes

Trott approved these changes Nov 14, 2017

View reviewed changes

Tiriel force-pushed the doc-add-console-noops branch from 99e297d to bcfe296 Compare November 14, 2017 21:42

cjihrig approved these changes Nov 20, 2017

View reviewed changes

joyeecheung added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 21, 2017

jasnell closed this Nov 22, 2017

addaleax removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 28, 2017

MylesBorins mentioned this pull request Dec 12, 2017

v9.3.0 proposal #17631

Merged

MylesBorins added the dont-land-on-v6.x label Dec 19, 2017

gibfahn added the dont-land-on-v8.x label Dec 19, 2017

Trott removed the dont-land-on-v8.x label Dec 8, 2018

Trott mentioned this pull request Dec 8, 2018

bug: console.table doesn't print #24909

Closed

MylesBorins added the land-on-v8.x label Dec 10, 2018

Uh oh!

Conversation

Tiriel commented Nov 13, 2017

Checklist

Affected core subsystem(s)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Trott commented Nov 14, 2017

Uh oh!

Tiriel commented Nov 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vsemozhetbyt Nov 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Tiriel commented Nov 14, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Trott commented Nov 14, 2017

Uh oh!

Trott left a comment

Choose a reason for hiding this comment

Uh oh!

Tiriel commented Nov 14, 2017

Uh oh!

tniessen commented Nov 20, 2017

Uh oh!

Tiriel commented Nov 20, 2017

Uh oh!

jasnell commented Nov 22, 2017

Uh oh!

Tiriel commented Nov 22, 2017

Uh oh!

Trott commented Dec 8, 2018

Uh oh!

MylesBorins commented Dec 10, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

13 participants

Tiriel commented Nov 14, 2017 •

edited

Loading

vsemozhetbyt Nov 14, 2017 •

edited

Loading