Skip to content

Conversation

@thefourtheye
Copy link
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 mscdex added doc Issues and PRs related to the documentations. repl Issues and PRs related to the REPL subsystem. labels Jul 9, 2015
Copy link
Member

Choose a reason for hiding this comment

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

... defined an inspect function ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed it now, thanks :-)

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 thefourtheye force-pushed the repl-doc-improvement-inspect branch from eec353d to b08202a Compare July 10, 2015 21:23
@thefourtheye
Copy link
Contributor Author

bump

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe remove depth?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, copy/paste mistake. Thanks :-)

@Fishrock123
Copy link
Contributor

LGTM Otherwise.

@thefourtheye
Copy link
Contributor Author

@Fishrock123 Addressed the comments. PTAL :-)

@thefourtheye
Copy link
Contributor Author

Bump!

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe call this Customizing Object displays in the REPL

@thefourtheye
Copy link
Contributor Author

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Period after function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just used :. Should this be a .?

@cjihrig
Copy link
Contributor

cjihrig commented Jul 22, 2015

LGTM minus a few comments.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool, thanks :-)

@thefourtheye
Copy link
Contributor Author

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

EDIT: CC @targos

Copy link
Member

Choose a reason for hiding this comment

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

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

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

@targos
Copy link
Member

targos commented Jul 22, 2015

LGTM

Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be referred to as inspect()? I wish we had guidelines for that kind of thing... cc @nodejs/documentation

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd recommend using However instead of But here. It's slightly more formal english.

@Fishrock123
Copy link
Contributor

Nits but LGTM

@thefourtheye
Copy link
Contributor Author

@Fishrock123 I incorporated the comments now.

This already got 3 LGTMs. If no other suggestions, I ll merge this by 9:00 PM IST 24th Jul 2015

thefourtheye added a commit that referenced this pull request Jul 24, 2015
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.

PR-URL: #2142
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
@thefourtheye
Copy link
Contributor Author

Thanks people, landed at d9f857d :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants