Skip to content

doc: repl: add defineComand and displayPrompt#3765

Closed
bengl wants to merge 1 commit into
nodejs:masterfrom
bengl:definecommanddocs
Closed

doc: repl: add defineComand and displayPrompt#3765
bengl wants to merge 1 commit into
nodejs:masterfrom
bengl:definecommanddocs

Conversation

@bengl

@bengl bengl commented Nov 11, 2015

Copy link
Copy Markdown
Member

Also some minor edits so the additions make sense.

I've used defineCommand several times now, figuring out how to use it by viewing the source. I figured it probably should be documented.

@thefourtheye thefourtheye added doc Issues and PRs related to the documentations. repl Issues and PRs related to the REPL subsystem. labels Nov 11, 2015
Comment thread doc/api/repl.markdown

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.

Nit: const instead of var?

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.

Other pre-existing examples in the doc use var for such things, so I thought it was best to keep with the style already in this doc. Perhaps instead a future PR can convert them all in one fell swoop?

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.

Sure. It's a nit. Ignore it.

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.

Cool beans.

@Trott

Trott commented Nov 11, 2015

Copy link
Copy Markdown
Member

LGTM with one typo correction that should happen and a bunch of nits that can be ignored if you wish because they are, you know, just nits.

@bengl bengl force-pushed the definecommanddocs branch from a7d042d to 5c80312 Compare November 11, 2015 07:13
@bengl

bengl commented Nov 11, 2015

Copy link
Copy Markdown
Member Author

Cool, yeah, I think the vars should stay for consistency until a full overhaul is done. Other than that, those nits are addressed now I think.

@jasnell

jasnell commented Nov 12, 2015

Copy link
Copy Markdown
Member

LGTM

Also some minor edits so the additions make sense.
@bengl bengl force-pushed the definecommanddocs branch from 5c80312 to 1640a2f Compare November 13, 2015 06:45
@bengl

bengl commented Nov 13, 2015

Copy link
Copy Markdown
Member Author

OK, @Trott helpfully pointed out a conflict with master and it seems due to 8a245ea, but that was easy to resolve, so here's a rebased version. I also restored the link URLs to the bottom of the file.

@Trott

Trott commented Nov 13, 2015

Copy link
Copy Markdown
Member

LGTM

Trott pushed a commit to Trott/io.js that referenced this pull request Nov 13, 2015
Also some minor edits so the additions make sense.

PR-URL: nodejs#3765
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@Trott

Trott commented Nov 13, 2015

Copy link
Copy Markdown
Member

Landed in 061b2c8

@Trott Trott closed this Nov 13, 2015
@tflanagan

Copy link
Copy Markdown
Contributor

Not to be a doc-nazi, but could we put the class definition first, and then resort alpha?

@Trott

Trott commented Nov 14, 2015

Copy link
Copy Markdown
Member

@tflanagan Whoops! Did we just mess up a bit of the alphabetizing work that you and jasnell did? Sorry about that. Since this is now merged with master, we'll want a separate PR to re-alphabetize. Do you want to put together a PR on this file for it? I'm happy to do if you'd rather not.

@tflanagan

Copy link
Copy Markdown
Contributor

@Trott no worries! I figured this would happen once or twice :) I'll take care of it on Monday

Fishrock123 pushed a commit that referenced this pull request Nov 17, 2015
Also some minor edits so the additions make sense.

PR-URL: #3765
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 17, 2015
Also some minor edits so the additions make sense.

PR-URL: #3765
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@MylesBorins

Copy link
Copy Markdown
Contributor

landed in v4.x-staging in 5254fda

This was referenced Mar 22, 2022
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.

6 participants