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

doc: improve node.1 manual page #5497

Closed
wants to merge 0 commits into from

Conversation

Fishrock123
Copy link
Contributor

Pull Request check-list

Please make sure to review and check all of these items:

  • make lint (ccplint)
  • Is the commit message formatted according to CONTRIBUTING.md?

Affected core subsystem(s)

doc,src

Description of change

Edited the man page to use better troff formatting and cover some previously missing options.

Also removed the v8 options from the man page. They really bloat the man page, and don't even belong there. Use --v8-options to see them.

@Fishrock123 Fishrock123 added the doc Issues and PRs related to the documentations. label Feb 29, 2016
.\ Man syntax is somewhat obscure, but the important part is is that .<letter>
.\ specifies <letter>'s syntax for that line, and \f<letter> specifies it for
.\ the characters that follow.
.\ See http://liw.fi/manpages/ for more info.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe I should get a better link?

@Fishrock123
Copy link
Contributor Author

Imo it would be ideal if on on unix systems node -h would open the man page. Not sure if we want that or how to do that yet.

.I arguments
]
.B node
[\fBdebug \fR|\fB \-\-\fR\fBdebug \fR|\fB \-\-debug-brk\fR] \fI...
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@indutny How much of this should we list here? I'm not really sure how the debug options work. :/

Copy link
Contributor

Choose a reason for hiding this comment

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

Each one is different. And even debug has multiple options:

$ node debug
Usage: node debug script.js
       node debug <host>:<port>
       node debug -p <pid>

@trevnorris
Copy link
Contributor

overall nice changes. though i'm not sure about removal of the v8 flags. only b/c they have "always" been there. but also not sure how many would care if they left.

@trevnorris
Copy link
Contributor

In the man output, first line is showing up like so:

Node.js  is  a  set  of  libraries  for  JavaScript

Notice there are double spaces between several of those words.

@Fishrock123
Copy link
Contributor Author

@trevnorris blame troff -- it likes to justify the content. Probably some option for man to configure it.

@trevnorris
Copy link
Contributor

This is also missing the debugger environment variables list. e.g.

$ NODE_DEBUG=net node -e 'require("net").createServer((c) => c.end("bye\n")).listen(8080)'
NET 19373: listen2 null 8080 4 false undefined
NET 19373: _listen2: create a handle
NET 19373: bind to ::

@trevnorris
Copy link
Contributor

@Fishrock123

blame troff -- it likes to justify the content. Probably some option for man to configure it.

Yup. Must be it. Resizing terminal gives different results.

@Fishrock123 Fishrock123 added the wip Issues and PRs that are still a work in progress. label Mar 1, 2016
@jasnell
Copy link
Member

jasnell commented Mar 2, 2016

Generally LGTM

@benjamingr
Copy link
Member

I'm +1 on removing the v8 flags there, it's just one more place to synchronize them in and it already shows how to print them. LGTM.

@Fishrock123
Copy link
Contributor Author

Updated. Should be about good to land. I've left out anything debug related for now.

@benjamingr, @jasnell, @trevnorris ptal

@Fishrock123 Fishrock123 removed the wip Issues and PRs that are still a work in progress. label Mar 14, 2016
@Fishrock123
Copy link
Contributor Author

maybe cc @nodejs/documentation ? Although I think this is almost a bit out of the regular documentation domain.

@Fishrock123
Copy link
Contributor Author

@jasnell Also this intentionally ignores --security-revert=, I figure the people who actually need that will know about it.

@jasnell
Copy link
Member

jasnell commented Mar 15, 2016

+1 ... Yeah --security-revert is not something we want to encourage too
many people to be using so leaving it undocumented is a good thing.
On Mar 15, 2016 7:25 AM, "Jeremiah Senkpiel" notifications@github.com
wrote:

@jasnell https://github.com/jasnell Also this intentionally ignores
--security-revert=, I figure the people who actually need that will know
about it.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#5497 (comment)

@Fishrock123
Copy link
Contributor Author

Anyone else able to review the updated version of this?

@benjamingr
Copy link
Member

Sorry, I read this yesterday and forgot to LGTM. LGTM.

@Fishrock123 Fishrock123 force-pushed the improve-node.1 branch 2 times, most recently from ba1b714 to f4f52fa Compare March 16, 2016 16:38
@Fishrock123
Copy link
Contributor Author

Rebased to add --zero-fill-buffers from 85ab4a5

Fishrock123 added a commit to Fishrock123/node that referenced this pull request Mar 16, 2016
Uses better troff formatting.
Removes v8 options from the man page.

Also edits `node -h` in node.cc slightly.

PR-URL: nodejs#5497
Reviewed-By: James Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@Fishrock123
Copy link
Contributor Author

Landed in 3bca5d3

@benjamingr
Copy link
Member

Nice Work 👏

Fishrock123 added a commit to Fishrock123/node that referenced this pull request Mar 16, 2016
Uses better troff formatting.
Removes v8 options from the man page.

Also edits `node -h` in node.cc slightly.

PR-URL: nodejs#5497
Reviewed-By: James Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@Fishrock123
Copy link
Contributor Author

Actually landed at f9e6191 because I screwed something up

@MylesBorins
Copy link
Contributor

@Fishrock123 is this applicable to LTS?

@jasnell
Copy link
Member

jasnell commented Mar 17, 2016

hmm.. I can't imagine this breaking anyone but let's hold off on porting to v4 for now.

@Fishrock123
Copy link
Contributor Author

This should land in LTS but you'll probably need to update to make sure no new options get accidentally added with it.

I can do that if needed.

@jasnell
Copy link
Member

jasnell commented Mar 17, 2016

oh.. heh, yeah, this one is good for LTS. For some reason I was thinking this was the other change you're looking at in terms of having --help output the man page.

@MylesBorins
Copy link
Contributor

@Fishrock123 probably best for you to port it.

Fishrock123 added a commit that referenced this pull request Mar 22, 2016
Uses better troff formatting.
Removes v8 options from the man page.

Also edits `node -h` in node.cc slightly.

PR-URL: #5497
Reviewed-By: James Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

Conflicts:
	doc/node.1
MylesBorins pushed a commit that referenced this pull request Mar 30, 2016
Uses better troff formatting.
Removes v8 options from the man page.

Also edits `node -h` in node.cc slightly.

PR-URL: #5497
Reviewed-By: James Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
MylesBorins pushed a commit that referenced this pull request Mar 30, 2016
Uses better troff formatting.
Removes v8 options from the man page.

Also edits `node -h` in node.cc slightly.

PR-URL: #5497
Reviewed-By: James Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants