doc: refactor manpage to use mdoc(7) macros #18559

Alhadis · 2018-02-04T11:06:58Z

NOTE:
A correctly-written commit message will be amended and force-pushed once a maintainer gives their approval. Due to the rather ambitious nature of this PR, and the fact it targets an ancillary part of Node's codebase, I expect this to be rejected. I thought to try nonetheless.

Checklist

documentation is changed or added
commit message follows commit guidelines

Description

This is a refactoring of Node's manpage to use mdoc macros instead of the legacy man macros. Both are widely supported, and both are used to format manual-pages for terminal display. The key difference is that mdoc is semantic rather than presentational. It offers less control over arbitrary formatting, but generates very consistent output. Its syntax is a little unwieldy but leaves less room for error. See mdoc(7) for reference.

Realistic preview of changes

Before (using man macros)
After (using mdoc macros)

Some minor editorial changes were made so the page conforms to the structure and conventions of manpages. In addition, the --experimental-modules and --experimental-vm-modules switches were added to the OPTIONS section, which are currently listed in the output of node --help.

Fixes: #18434 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

gibfahn · 2018-02-05T07:29:59Z

I'm not particularly knowledgeable about man pages, but as long as mdoc works on all our supported platforms, +1 on moving to it. Less room for error sounds good, and I don't think we need arbitrary formatting.

I'm not really sure who to cc for this, I'm not sure we have anyone who is particularly knowledgeable about the Node manpage. Maybe cc/ @silverwind @bnoordhuis @sam-github @strugee (from the commit history).

What would be really helpful is a way to evaluate PRs to the manpage. A rendered before/after might be adequate, how did you generate yours?

bnoordhuis · 2018-02-05T09:08:55Z

I'm not an mdoc expert (wrote a lot of roff in my time but not mdoc specifically) but the changes look good to me. @Alhadis Can you wrap lines at 80 columns and spice up the commit log?

as long as mdoc works on all our supported platforms

I'm not 100% sure but I think that's the case. It's probably an improvement over the current man page because I doubt it renders nicely when your roff isn't groff.

Alhadis · 2018-02-05T09:48:59Z

Can you wrap lines at 80 columns

I can, but having one sentence per line enables Troff (or mandoc) to generate nicer-looking paragraphs with better justification. Compare:

ONE SENTENCE PER LINE:
     Node.js is a set of libraries for JavaScript which allows it to be used
     outside of the browser.  It is primarily focused on creating simple,
     easy-to-build network clients and servers.

LINES WRAPPED TO 80 COLUMNS:
     Node.js is a set of libraries for JavaScript which allows it to be used
     outside of the browser. It is primarily focused on creating simple, easy-
     to-build network clients and servers.

Not a huge issue (unless you're a typography nerd like me), but you'll notice easy-to-build was broken across two lines, and extra spacing doesn't separate the two sentences. mandoc -T lint doc/node.1 will also warn you about this sort of thing:

λ node (docfix): mandoc -T lint doc/node.1 
mandoc: doc/node.1:28:17: WARNING: new sentence, new line

@bnoordhuis How should I proceed?

What would be really helpful is a way to evaluate PRs to the manpage. A rendered before/after might be adequate, how did you generate yours?

Using this, which started from this. Don't want this PR to become an advertisement for my own handiwork, but suffice to say if you're an Atom user, you'll damn well enjoy having a live previewer for editing Roff documents. It's not strictly reliant on Atom to work, either: its non-graphical output can be used from within Node alone:

groff -mandoc -tZTutf8 /path/to/man/page.1 | html-tty > wanky-preview.html

With html-tty being this. The styling was added manually, since the HTML-ified nroff output is intended to be styled from inside Atom.

Note: The previewer isn't finished, although it's 95% there. I'm itching to finish it.

silverwind · 2018-02-05T18:32:36Z

doc/node.1

-.TP
-.BR \-\-zero\-fill\-buffers
+.
+.It Fl -redirect-warnings Ar file


Can you add the equal signs back on arguments like −−redirect−warnings=file? From a quick glance, it looks like the parser is strict about them.

What do you mean? Every time I pass --redirect-warnings=file (with or without an equals-sign), I get the same error telling me it was given a bad option.

I think the option may be actually broken, let me check for another example.

Better example: node --inspect=1234 works while node --inspect 1234 does not. (Yeah we should really fix our argparser)

#18580 for the --redirect-warnings issue.

#18580 resolved. The HTML you generated includes
U+2212 minus signs which of course throws our parser off as it expects ASCII. The issue appears to not be present in the manpage.

"The issue appears to not be present in the manpage"

When you say it's not present, which manpage and manpage version are you specifically referring to? If you're testing them locally, compare the output of these two lines, please:

groff -mandoc -tTascii ./node.1 | less # Boring old ASCII groff -mandoc -tTutf8 ./node.1 | less # UTF-8

Either way, it seems the = is necessary, so @Alhadis would you mind adding it back in?

Wouldn't it be better to explicitly mention the option requires an equals sign? It isn't immediately clear to the reader that the discrepancy is deliberate.

Wouldn't it be better to explicitly mention the option requires an equals sign?

Not sure I follow. Do you mean adding a "This option requires a equal sign" to each option that does? I'd rather just print it as part of the option title, much more concise.

silverwind · 2018-02-05T18:36:04Z

doc/node.1

+.It Fl \-experimental-vm-modules
+Enable experimental ES module support in VM module.
+.
+.It \-


Shouldn't this - be highlighted as a flag?

It's not a real flag, as such. Both - and -- probably shouldn't even be documented here. I'd move them back up to the DESCRIPTION block, next to the line about no arguments enabling the REPL.

Sounds fine to me, they aren't really options, but I see some value having them documented.

OTOH, I just noticed that for example git seems to also list -- under options, see git-commit(1). So maybe it's not that uncommon to list -- and - as options.

If it is going to stay here (which seems fine to me, especially given the git-commit example) then I guess bolding the - and -- makes sense.

silverwind · 2018-02-05T18:44:56Z

doc/node.1

-.\" See http://liw.fi/manpages/ for an overview, or http://www.troff.org/54.pdf
-.\" for detailed language reference.
-
-.\" Macro to display an underlined URL in bold


I'd suggest retaining a informational header about the file format because many people are not familar with the arcane art of writing manpages. A few links to the format and maybe a example on how to lint and preview should be enough

I somehow missed this comment. And yep, I'll definitely add a header with some links to informative resources... I held off doing this for the same reason as the commit message: I wasn't sure how it would be received (and I'm absurdly OCD about well-written commit messages, sometimes to a point of being manic). 😅

silverwind · 2018-02-05T18:45:24Z

Overall, I like the new concise rendering 👍

@Alhadis By the way, which tool would you recommend for linting? I see you've been using mandoc -T lint but the mandoc tools does not look readily available in Linux at least (For example, Arch Linux only has it in the user repository).

Alhadis · 2018-02-05T19:04:22Z

By the way, which tool would you recommend for linting? I see you've been using mandoc -T lint but the mandoc tools does not look readily available in Linux at least

Which Linux are you referring to? I had no problems installing it on Debian, and the project's site has a pretty comprehensive listing of ports and supported platforms.

For typesetter-roff (troff), well, your best bet is this:

troff -tTutf8 -wall /path/to/page.1 >/dev/null

silverwind · 2018-02-05T19:16:05Z

Which Linux are you referring to

Arch Linux does not have it in the base repos, just in AUR, but that shouldn't be an issue for a possible future integration of a manpage linter if our lint job node-test-linter runs on a Debian derivate, but I'm not certain right now.

gibfahn · 2018-02-06T10:45:38Z

Linter job currently runs on FreeBSD, but I don't think moving to debian will be an issue.

Alhadis · 2018-02-06T13:27:29Z

Anybody want me to chuck this in as a bonus? 😀 It's a few years out-of-date and needs updating with V8's new command-switch set, but I've been meaning to hack a Perl script together to sync the v8.1 page with latest releases.

The page was hard-written but works beautifully.

silverwind · 2018-02-06T17:16:17Z

Anybody want me to chuck this in as a bonus?

I wouldn't be against adding v8 options to the man page if you can get the updating integrated into our v8 update workflow (I think this is done using https://github.com/targos/update-v8). Would probably have to be written in JS, thought :)

cc: @targos

Alhadis · 2018-02-06T17:36:47Z

Updating a manual-page by comparing two lists of options gets a bit slippery because many of the manpage option-descriptions have been polished up in grammar or wording, possibly with marked-up links to other sections across pages, or what-have-you. =) (I'm hoping that makes sense).

FYI, it wouldn't take me very long to smash together a Node program that generates the API manpages from JSON source. 😉

silverwind · 2018-02-06T17:57:08Z

Linter job currently runs on FreeBSD

Given that it's a BSD, it might just have the mandoc tool preinstalled. I don't have a FreeBSD available, can someone check?

many of the manpage option-descriptions have been polished up

I see that most v8 options are pretty inconsistently labeled. One suggestion might be to maintain a list of polished up descriptions and fallback to the default description in case no mapping is found.

gibfahn · 2018-02-07T15:01:15Z

Anybody want me to chuck this in as a bonus?

Sounds useful, but it might delay this PR, maybe just raise a separate one.

FYI, it wouldn't take me very long to smash together a Node program that generates the API manpages from JSON source. 😉

Please do!

Alhadis · 2018-02-07T15:20:37Z

Sounds useful, but it might delay this PR, maybe just raise a separate one.

Will do. ;)

Just waiting on @bnoordhuis to clarify what he wants me to do regarding that line-wrapping matter. Then I'll amend my commit and force-push something merge-worthy. 👍

bnoordhuis · 2018-02-07T17:09:12Z

It's fine but can you add a comment to the document explaining why it has long lines? Otherwise someone will clean it up sooner or later.

Alhadis · 2018-02-07T17:26:37Z

Wilco. =) I'll include a note about why blank lines are bad, too.

Alhadis · 2018-02-09T10:10:17Z

Alright, done. Does that look adequate?

I started with the .ig command for embedding the notes at the top:

.ig
This manpage is written in mdoc(7).

 * Language reference:
   https://man.openbsd.org/mdoc.7

 * Atom editor support:
   https://atom.io/packages/language-roff

 * Linting changes:
   mandoc -Wall -Tlint /path/to/this.file  # BSD
   groff -w all -z /path/to/this.file      # GNU/Linux, macOS


 Before making changes, please note the following:

 * In Roff, each new sentence should begin on a new line. This gives
   the Roff formatter better control over text-spacing, line-wrapping,
   and paragraph justification.

 * Do not leave blank lines in the markup. If whitespace is desired
   for readability, put a dot in the first column to indicate a null/empty
   command. Comments and horizontal whitespace may optionally follow: each
   of these lines are an example of a null command immediately followed by
   a comment.
..

Of course, then I had to explain (tersely) what the .ig (Ignore text) command does, and why those lines were an exception to what they were explaining. 😞 Since there was no concise way to do so, I resorted to using the regular .\" comments just to simplify things for readers.

gibfahn · 2018-02-11T19:13:20Z

Out of interest did you edit your commit message to make every line exactly 72 columns? If so then I'm kind of amazed by the commitment 😁 .

Like man(7), mdoc(7) is a macro package for marking up computer manuals.
The main difference is that mdoc is semantic rather than presentational,
providing authors with a full DSL to abstract page markup from low-level
Roff commands. By contrast, `man` is minimalist and leaves formatting to
the author, who is expected to have a working amount of Roff knowledge.

Therefore the use of `mdoc` for marking up Node's manpage is a decidedly
better choice than bare `man`. Less room is left for error and mandoc(1)
offers very robust error-checking and linting.

Also do you have an updated rendered link to compare against?

Alhadis · 2018-02-12T07:35:22Z

Out of interest did you edit your commit message to make every line exactly 72 columns? If so then I'm kind of amazed by the commitment 😁 .

That's, uh, literally every commit message I've ever written. 😅 I'm not even making this up.

Think that's absurd? Scroll down.

Also do you have an updated rendered link to compare against?

Output is still the same. 😉 All I did was add comments:

Before: d6eab8d
After: 375f46f

gibfahn

LGTM once @silverwind 's comments are addressed

gibfahn · 2018-02-12T14:06:04Z

doc/node.1

+.It Fl \-experimental-vm-modules
+Enable experimental ES module support in VM module.
+.
+.It \-


If it is going to stay here (which seems fine to me, especially given the git-commit example) then I guess bolding the - and -- makes sense.

gibfahn · 2018-02-12T14:08:19Z

doc/node.1

-.TP
-.BR \-\-zero\-fill\-buffers
+.
+.It Fl -redirect-warnings Ar file


Either way, it seems the = is necessary, so @Alhadis would you mind adding it back in?

silverwind · 2018-02-13T17:52:22Z

Yeah, I'd approve as well once those two points are adressed (boldening of - and -- and adding the = on options that require it).

Like man(7), mdoc(7) is a macro package for marking up computer manuals. The main difference is that mdoc is semantic rather than presentational, providing authors with a full DSL to abstract page markup from low-level Roff commands. By contrast, `man` is minimalist and leaves formatting to the author, who is expected to have a working amount of Roff knowledge. Therefore the use of `mdoc` for marking up Node's manpage is a decidedly better choice than bare `man`. Less room is left for error and mandoc(1) offers very robust error-checking and linting.

Alhadis · 2018-02-16T04:21:40Z

Very well. Done. Here's the updated rendered view.

To explain the Ns = Ns weirdness: the Ns macro is used for suppressing the preceding space that would otherwise be inserted automatically (Ns = "No Space").

It's necessary to stop --inspect=[host:]port coming out like --inspect = [host:]port.

BridgeAR · 2018-02-16T22:16:47Z

Light-CI https://ci.nodejs.org/job/node-test-commit-light/280/

Like man(7), mdoc(7) is a macro package for marking up computer manuals. The main difference is that mdoc is semantic rather than presentational, providing authors with a full DSL to abstract page markup from low-level Roff commands. By contrast, `man` is minimalist and leaves formatting to the author, who is expected to have a working amount of Roff knowledge. Therefore the use of `mdoc` for marking up Node's manpage is a decidedly better choice than bare `man`. Less room is left for error and mandoc(1) offers very robust error-checking and linting. PR-URL: nodejs#18559 Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>

BridgeAR · 2018-02-17T14:45:27Z

Landed in ef7f5a1 🎉

@Alhadis thanks a lot for your hard work.

Like man(7), mdoc(7) is a macro package for marking up computer manuals. The main difference is that mdoc is semantic rather than presentational, providing authors with a full DSL to abstract page markup from low-level Roff commands. By contrast, `man` is minimalist and leaves formatting to the author, who is expected to have a working amount of Roff knowledge. Therefore the use of `mdoc` for marking up Node's manpage is a decidedly better choice than bare `man`. Less room is left for error and mandoc(1) offers very robust error-checking and linting. PR-URL: #18559 Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>

Like man(7), mdoc(7) is a macro package for marking up computer manuals. The main difference is that mdoc is semantic rather than presentational, providing authors with a full DSL to abstract page markup from low-level Roff commands. By contrast, `man` is minimalist and leaves formatting to the author, who is expected to have a working amount of Roff knowledge. Therefore the use of `mdoc` for marking up Node's manpage is a decidedly better choice than bare `man`. Less room is left for error and mandoc(1) offers very robust error-checking and linting. PR-URL: nodejs#18559 Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>

nodejs-github-bot added the doc Issues and PRs related to the documentations. label Feb 4, 2018

silverwind reviewed Feb 5, 2018

View reviewed changes

silverwind mentioned this pull request Feb 7, 2018

--redirect-warnings option appears to be broken #18580

Closed

Alhadis mentioned this pull request Feb 8, 2018

Export globally mocha and runner instances Alhadis/Atom-Mocha#4

Merged

Alhadis mentioned this pull request Feb 9, 2018

Don't replace ASCII minus sign with U+2122 Alhadis/Roff.js#1

Closed

gibfahn reviewed Feb 12, 2018

View reviewed changes

silverwind approved these changes Feb 16, 2018

View reviewed changes

jasnell approved these changes Feb 16, 2018

View reviewed changes

gibfahn approved these changes Feb 16, 2018

View reviewed changes

BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 16, 2018

BridgeAR closed this Feb 17, 2018

Alhadis deleted the docfix branch February 17, 2018 16:16

MylesBorins mentioned this pull request Feb 21, 2018

v9.6.0 proposal #18902

Merged

MylesBorins added the dont-land-on-v8.x label Aug 7, 2018

Alhadis mentioned this pull request Jan 2, 2022

Rewrite manual page in mdoc(7) rui314/mold#230

Merged

doc: refactor manpage to use mdoc(7) macros #18559

doc: refactor manpage to use mdoc(7) macros #18559

Conversation

Alhadis commented Feb 4, 2018 • edited Loading

Checklist

Description

Realistic preview of changes

gibfahn commented Feb 5, 2018

bnoordhuis commented Feb 5, 2018

Alhadis commented Feb 5, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

silverwind Feb 7, 2018 • edited Loading

Choose a reason for hiding this comment

Alhadis Feb 7, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

silverwind commented Feb 5, 2018

Alhadis commented Feb 5, 2018

silverwind commented Feb 5, 2018

gibfahn commented Feb 6, 2018

Alhadis commented Feb 6, 2018

silverwind commented Feb 6, 2018

Alhadis commented Feb 6, 2018 • edited Loading

silverwind commented Feb 6, 2018 • edited Loading

gibfahn commented Feb 7, 2018

Alhadis commented Feb 7, 2018

bnoordhuis commented Feb 7, 2018

Alhadis commented Feb 7, 2018

Alhadis commented Feb 9, 2018 • edited Loading

gibfahn commented Feb 11, 2018

Alhadis commented Feb 12, 2018

gibfahn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

silverwind commented Feb 13, 2018

Alhadis commented Feb 16, 2018

BridgeAR commented Feb 16, 2018

BridgeAR commented Feb 17, 2018

Alhadis commented Feb 4, 2018 •

edited

Loading

Alhadis commented Feb 5, 2018 •

edited

Loading

silverwind Feb 7, 2018 •

edited

Loading

Alhadis Feb 7, 2018 •

edited

Loading

Alhadis commented Feb 6, 2018 •

edited

Loading

silverwind commented Feb 6, 2018 •

edited

Loading

Alhadis commented Feb 9, 2018 •

edited

Loading