-
-
Notifications
You must be signed in to change notification settings - Fork 33.7k
doc: add description for inspector-only console methods. #17004
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
Conversation
doc/api/console.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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. |
8f59344 to
7ca8aa7
Compare
|
@Trott Thanks for the input! Just did it, hope it's okay |
doc/api/console.md
Outdated
There was a problem hiding this comment.
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()`][]
doc/api/console.md
Outdated
There was a problem hiding this comment.
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()`][]
doc/api/console.md
Outdated
There was a problem hiding this comment.
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}
doc/api/console.md
Outdated
There was a problem hiding this comment.
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.
7ca8aa7 to
02a3493
Compare
doc/api/console.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
having inspector be a link to debugger.html and/or inspector.html would be good here. debugger.html is the best choice.
02a3493 to
99e297d
Compare
|
Thanks! Changed according to each review :) |
doc/api/console.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Remove tab.
doc/api/console.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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()...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would like to see the recent round of nits addressed, but LGTM with or without them.
99e297d to
bcfe296
Compare
|
@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! |
|
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>
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 (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 |
Description inspired by dev tools reference and inspector err messages
Added:
Fixes: #16755
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
doc