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.

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

Implement console.trace() #2780

Merged
merged 3 commits into from Aug 17, 2019

Conversation

@kevinkassimo
Copy link
Contributor

commented Aug 14, 2019

Noticed that we have never implemented this.
https://console.spec.whatwg.org/#trace

const message = stringifyArgs(args, {
indentLevel: this.indentLevel,
collapsedAt: this.collapsedAt,
noTrailingNewline: true

This comment has been minimized.

Copy link
@ry

ry Aug 14, 2019

Collaborator

Why is noTrailingNewline necessary?

This comment has been minimized.

Copy link
@kevinkassimo

kevinkassimo Aug 14, 2019

Author Contributor

It is just that without this settings there will be a newline appended to message by stringifyArgs and thus leaving a blank line between message and trace on printing err.stack.

(There is definitely no specification saying this is not allow, just I feel it would be kind of ugly...)

This comment has been minimized.

Copy link
@ry

ry Aug 15, 2019

Collaborator

I appreciate it being formatted as you have it.
Rather, I'm suggesting that instead of adding noTrailingNewline, you make it so stringifyArgs doesn't add a trailing \n and lift that into the existing callers.

This comment has been minimized.

Copy link
@kevinkassimo

kevinkassimo Aug 15, 2019

Author Contributor

I reviewed the structure of console.ts and realized that there are some complexity introduced by console.groupCollapsed(). I believe the original intention of introducing it was to allow synchronous write to stdout, which is available now since we added File::writeSync.

Technically console.groupCollapsed() behavior we have is non-standard, as the definition of being collapsed refers to something like a dropdown. Node simply aliases groupCollapsed to group

If we were to make console.groupCollapsed to alias console.group, then things could simplify quite a bit.

kevinkassimo and others added 2 commits Aug 15, 2019
groupCollapsed alias to group, remove noTrailingNewline, move newline…
… out of stringifyArgs, fix console.dir, add tests, and fix a repl log quirk

For repl logging quirks, I believe we should not indent repl logging. If
we really want such indentation, we probably also want to indent "> "
prompts.
@ry
ry approved these changes Aug 17, 2019
Copy link
Collaborator

left a comment

LGTM - nice clean up with this added feature.

@ry ry merged commit 9acb177 into denoland:master Aug 17, 2019

3 checks passed

Travis CI - Pull Request Build Passed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
license/cla Contributor License Agreement is signed.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can’t perform that action at this time.