FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

console: change doc to follow major browser console doc by aks- · Pull Request #3584 · nodejs/node · GitHub

/ node Public

console: change doc to follow major browser console doc - #3584

Closed
aks- wants to merge 1 commit into
nodejs:masterfrom
aks-:change-doc-to-follow-major-browser-console-API
ConversationCommits (1)ChecksFiles changed
Closed

console: change doc to follow major browser console doc#3584
aks- wants to merge 1 commit into
nodejs:masterfrom
aks-:change-doc-to-follow-major-browser-console-API

Conversation

aks- commented Oct 29, 2015

Copy link
Copy Markdown
Member

Trott added the console Issues and PRs related to the console subsystem. label Oct 29, 2015
aks- mentioned this pull request Oct 29, 2015
Closed
silverwind reviewed Oct 29, 2015
View reviewed changes
Comment thread doc/api/console.markdown Outdated

silverwind Oct 29, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

We recently changed this in #3166 to match MDN. I'd say we ought to try changing the spec (which is linked on MDN, ironically).

silverwind Oct 29, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

According to MDN history, it's called timerName on there since 2012 when that article was created, thought calling it label seems somewhat common too.

If we agree on calling it label, I can change it on MDN, supposedly.

cc: @targos

targos Oct 29, 2015

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@silverwind I don't really care. I changed it after your suggestion.
If you want to change it back, just run git revert 8c043c1

silverwind Oct 29, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks, revert filed in #3590. I'm sorry for the trouble I've caused.

@aks- could you leave this rename of that argument out of this PR?

aks- Oct 30, 2015

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@silverwind done.

aks- force-pushed the change-doc-to-follow-major-browser-console-API branch from 8215ae2 to d48a9c3 Compare October 30, 2015 05:54

silverwind commented Oct 30, 2015

Copy link
Copy Markdown
Contributor

LGTM

targos reviewed Oct 30, 2015
View reviewed changes
Comment thread doc/api/console.markdown Outdated

targos Oct 30, 2015

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

The first argument is optional too. You can call console.log() to print a blank line.

silverwind Oct 30, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I've tested this before, Node and Firefox print an empty line, Chrome does not. Maybe it should be brought up with the spec authors?

Fishrock123 Oct 30, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Meh, what actually matters here is what Node does.

Fishrock123 Oct 30, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Should be console.log([object, ...]), I think.

targos Oct 30, 2015

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Or console.log([object [, ...]]) ?

targos Oct 30, 2015

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Like in PHP doc: http://php.net/manual/en/function.printf.php

silverwind commented Oct 30, 2015

Copy link
Copy Markdown
Contributor

@Fishrock123 agree, this isn't correctly documenting how it currently works in node. I filed DeveloperToolsWG/console-object#36 to hopefully clarify what the expected behaviour is.

domenic commented Oct 30, 2015

Copy link
Copy Markdown
Contributor

That "spec" is a reasonable attempt but not really anything normative. I would not base anything on it. Console behaviour is not really interoperable.

Fishrock123 commented Nov 3, 2015

Copy link
Copy Markdown
Contributor

@domenic are you saying that our console shouldn't try to be similar to a browser console?

domenic commented Nov 4, 2015

Copy link
Copy Markdown
Contributor

It definitely should. But that document and browser console behavior are unrelated. And browsers don't really agree on console behavior in various ways.

jasnell added the stalled Issues and PRs that are stalled. label Mar 22, 2016

jasnell commented Mar 22, 2016

Copy link
Copy Markdown
Member

Marked as stalled. Will close if there's no new activity in the next few weeks.

jasnell commented Apr 12, 2016

Copy link
Copy Markdown
Member

Closing due to further lack of activity. This might still be worthwhile, however, so can definitely reopen if necessary.

jasnell closed this Apr 12, 2016
This was referenced Dec 7, 2023
Open
Open
Open
leonardoadame mentioned this pull request Dec 8, 2023
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

console Issues and PRs related to the console subsystem. stalled Issues and PRs that are stalled.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

Back | FazBrowse Home | New Git URL