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

doc: note about custom inspect functions by thefourtheye · Pull Request #2142 · nodejs/node · GitHub

/ node Public

doc: note about custom inspect functions - #2142

Closed
thefourtheye wants to merge 6 commits into
nodejs:masterfrom
thefourtheye:repl-doc-improvement-inspect
ConversationCommits (6)ChecksFiles changed
Closed

doc: note about custom inspect functions#2142
thefourtheye wants to merge 6 commits into
nodejs:masterfrom
thefourtheye:repl-doc-improvement-inspect

Conversation

thefourtheye commented Jul 9, 2015

Copy link
Copy Markdown
Contributor

See: #1798

When an Object is printed in REPL, the actual representation can be
overriden by defining inspect method on the objects. This patch
includes a note about the same in the REPL documentation

mscdex added doc Issues and PRs related to the documentations. repl Issues and PRs related to the REPL subsystem. labels Jul 9, 2015
targos reviewed Jul 10, 2015
View reviewed changes
Comment thread doc/api/repl.markdown Outdated

targos Jul 10, 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

... defined an inspect function ...

thefourtheye Jul 10, 2015

Copy link
Copy Markdown
Contributor 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

Changed it now, thanks :-)

doc: note about custom inspect functions
b08202a 
See: nodejs#1798

When an Object is printed in REPL, the actual representation can be
overriden by defining `inspect` method on the objects. This patch
includes a note about the same in the REPL documentation
thefourtheye force-pushed the repl-doc-improvement-inspect branch from eec353d to b08202a Compare July 10, 2015 21:23

thefourtheye commented Jul 15, 2015

Copy link
Copy Markdown
Contributor Author

bump

Fishrock123 reviewed Jul 16, 2015
View reviewed changes
Comment thread doc/api/repl.markdown Outdated

Fishrock123 Jul 16, 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

Maybe remove depth?

thefourtheye Jul 17, 2015

Copy link
Copy Markdown
Contributor 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

Ah, copy/paste mistake. Thanks :-)

Fishrock123 commented Jul 16, 2015

Copy link
Copy Markdown
Contributor

LGTM Otherwise.

thefourtheye commented Jul 17, 2015

Copy link
Copy Markdown
Contributor Author

@Fishrock123 Addressed the comments. PTAL :-)

thefourtheye commented Jul 22, 2015

Copy link
Copy Markdown
Contributor Author

Bump!

cjihrig reviewed Jul 22, 2015
View reviewed changes
Comment thread doc/api/repl.markdown Outdated

cjihrig Jul 22, 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

Maybe call this Customizing Object displays in the REPL

thefourtheye commented Jul 22, 2015

Copy link
Copy Markdown
Contributor Author

@cjihrig Thanks for the review. I changed the text now. PTAL.

cjihrig reviewed Jul 22, 2015
View reviewed changes
Comment thread doc/api/repl.markdown Outdated

cjihrig Jul 22, 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

Period after function

thefourtheye Jul 22, 2015

Copy link
Copy Markdown
Contributor 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

I just used :. Should this be a .?

cjihrig commented Jul 22, 2015

Copy link
Copy Markdown
Contributor

LGTM minus a few comments.

cjihrig reviewed Jul 22, 2015
View reviewed changes
Comment thread doc/api/repl.markdown Outdated

cjihrig Jul 22, 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 think either a period or colon should be fine here. I originally said period because your sentence was coming to a finish, but you also have a following code block, so either should be fine.

thefourtheye Jul 22, 2015

Copy link
Copy Markdown
Contributor 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

Cool, thanks :-)

thefourtheye commented Jul 22, 2015

Copy link
Copy Markdown
Contributor Author

@Fishrock123 Can you PTAL and give LGTM, so that I can land this?

EDIT: CC @targos

targos reviewed Jul 22, 2015
View reviewed changes
Comment thread doc/api/repl.markdown Outdated

targos Jul 22, 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 link should be relative. You can see how it's done in https://raw.githubusercontent.com/nodejs/io.js/master/doc/api/console.markdown

thefourtheye commented Jul 22, 2015

Copy link
Copy Markdown
Contributor Author

Thanks for pointing it out @targos :-) PTAL now.

targos commented Jul 22, 2015

Copy link
Copy Markdown
Member

LGTM

Fishrock123 reviewed Jul 22, 2015
View reviewed changes
Comment thread doc/api/repl.markdown Outdated

Fishrock123 Jul 22, 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 this be referred to as inspect()? I wish we had guidelines for that kind of thing... cc @nodejs/documentation

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

doc Issues and PRs related to the documentations. repl Issues and PRs related to the REPL subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

Back | FazBrowse Home | New Git URL