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

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

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

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.

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.

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

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

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

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

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

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

Thanks! Changed according to each review :)

Nit: Remove tab.

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?

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

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

@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!

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

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>

Landed in 982c674

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>

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>

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

vsemozhetbyt approved these changes Nov 14, 2017

View reviewed changes

vsemozhetbyt reviewed Nov 14, 2017

View reviewed changes

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

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 outstanding review comments, 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 outstanding review comments, 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

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

Uh oh!

Uh oh!

doc: add description for inspector-only console methods. - #17004

doc: add description for inspector-only console methods.#17004Tiriel wants to merge 12 commits intonodejs:masternodejs/node:masterfrom Tiriel:doc-add-console-noopsTiriel/node:doc-add-console-noopsCopy head branch name to clipboard

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

doc: add description for inspector-only console methods.#17004
Tiriel wants to merge 12 commits into
nodejs:masterfrom
Tiriel:doc-add-console-noops

Tiriel commented Nov 14, 2017 •
edited
Loading

vsemozhetbyt Nov 14, 2017 •
edited
Loading