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.group(), console.groupEnd() #9276

Closed
wants to merge 1 commit into from

Conversation

ajnirp
Copy link
Contributor

@ajnirp ajnirp commented Jan 12, 2016

Fixes #9274

Review on Reviewable

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Jan 12, 2016
@wafflespeanut wafflespeanut added the S-needs-rebase There are merge conflict errors. label Jan 12, 2016
@ajnirp
Copy link
Contributor Author

ajnirp commented Jan 12, 2016

rebased

@wafflespeanut wafflespeanut removed the S-needs-rebase There are merge conflict errors. label Jan 12, 2016
@jdm
Copy link
Member

jdm commented Jan 12, 2016

I doubt we want to apply the indentation to the message before sending it to the devtools client, but I haven't looked carefully at this code yet. Have you tried out the changes with the Firefox webconsole connected?

@KiChjang KiChjang added the S-awaiting-answer Someone asked a question that requires an answer. label Jan 13, 2016
@paulrouget
Copy link
Contributor

Also, please add groupCollapsed.

@jdm
Copy link
Member

jdm commented Jan 13, 2016

Note, instructions for interacting with the Firefox devtools exist in the wiki.

@bors-servo
Copy link
Contributor

☔ The latest upstream changes (presumably #9339) made this pull request unmergeable. Please resolve the merge conflicts.

@highfive highfive added the S-needs-rebase There are merge conflict errors. label Jan 18, 2016
@asajeffrey asajeffrey self-assigned this Feb 4, 2016
@asajeffrey
Copy link
Member

Some comments on reviewable...


Reviewed 4 of 4 files at r1.
Review status: all files reviewed at latest revision, 4 unresolved discussions, some commit checks failed.


components/devtools/lib.rs, line 339 [r1] (raw file):
This is adding 2 spaces per indent level, and the indent level is incremented/decremented by 2 on each Group/GroupEnd, so there's 4 spaces per event. Is this intended?


components/devtools/lib.rs, line 352 [r1] (raw file):
There should be a way to do this without copying the indent string.


components/devtools_traits/lib.rs, line 220 [r1] (raw file):
It's a bit odd that Group and GroupEnd are considered to be LogLevels. Is this what devtools does?


components/script/dom/console.rs, line 47 [r1] (raw file):
Should all messages be printed to stdout, irrespective of log level?


Comments from the review on Reviewable.io

@asajeffrey asajeffrey removed the S-awaiting-review There is new code that needs to be reviewed. label Feb 5, 2016
@jdm jdm added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. and removed S-awaiting-answer Someone asked a question that requires an answer. labels Feb 5, 2016
@jdm
Copy link
Member

jdm commented Feb 26, 2016

@wenderen Are you planning to continue working on this?

@asajeffrey
Copy link
Member

@wenderen is this PR still open?

@ajnirp
Copy link
Contributor Author

ajnirp commented Mar 10, 2016

@jdm @asajeffrey It is. I'm very sorry but I cannot work on this because of a lack of free time. Please assign this to someone else if need be.

@asajeffrey
Copy link
Member

@wenderen OK, thanks for letting us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-needs-code-changes Changes have not yet been made that were requested by a reviewer. S-needs-rebase There are merge conflict errors.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants