Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
15 changes: 14 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -476,12 +476,13 @@ DOCS_ANALYTICS ?=
apidoc_sources = $(wildcard doc/api/*.md)
apidocs_html = $(apidoc_dirs) $(apiassets) $(addprefix out/,$(apidoc_sources:.md=.html))
apidocs_json = $(apidoc_dirs) $(apiassets) $(addprefix out/,$(apidoc_sources:.md=.json))
apidocs_man = $(apidoc_dirs) $(apiassets) $(addprefix out/,$(apidoc_sources:.md=.3))

apidoc_dirs = out/doc out/doc/api/ out/doc/api/assets

apiassets = $(subst api_assets,api/assets,$(addprefix out/,$(wildcard doc/api_assets/*)))

doc-only: $(apidocs_html) $(apidocs_json)
doc-only: $(apidocs_html) $(apidocs_json) $(apidocs_man)
doc: $(NODE_EXE) doc-only

$(apidoc_dirs):
Expand Down Expand Up @@ -515,6 +516,18 @@ out/doc/api/%.json: doc/api/%.md
out/doc/api/%.html: doc/api/%.md
$(call gen-doc, $(gen-html))

# check if ./node is actually set, else use user pre-installed binary
gen-man = tools/doc/generate.js --format=man $< > $@
out/doc/api/%.3: doc/api/%.md
@[ -e tools/doc/node_modules/js-yaml/package.json ] || \
[ -e tools/eslint/node_modules/js-yaml/package.json ] || \
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 copied this from the HTML build logic, but it's now duplicated three times. Should we refactor this? I think yes, but my Make-fu is not very good...

if [ -x $(NODE) ]; then \
cd tools/doc && ../../$(NODE) ../../$(NPM) install; \
else \
cd tools/doc && node ../../$(NPM) install; \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we actually need npm install since node_modules is vendored?

Copy link
Member

Choose a reason for hiding this comment

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

would definitely prefer not having npm install here.

fi
[ -x $(NODE) ] && $(NODE) $(gen-man) || node $(gen-man)

docopen: $(apidocs_html)
@$(PYTHON) -mwebbrowser file://$(PWD)/out/doc/api/all.html

Expand Down
7 changes: 7 additions & 0 deletions tools/doc/generate.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,13 @@ function next(er, input) {
);
break;

case 'man':
require('./man.js')(input, inputFile, function(er, manSrc) {
console.log(manSrc);
if (er) throw er;
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be before the console.log()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mscdex lol, you'd think :D

I was looking at case 'json': when I wrote that, trying to mirror surrounding code style; perhaps that one's wrong too?

Copy link
Contributor

Choose a reason for hiding this comment

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

Probably.

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'll push a fix tomorrow then

});
break;

default:
throw new Error('Invalid format: ' + format);
}
Expand Down
133 changes: 133 additions & 0 deletions tools/doc/man.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
// Copyright AJ Jordan and other Node contributors.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this correct, given that I wrote this file from scratch?

//
// Permission is hereby granted, free of charge, to any person obtaining a
// copy of this software and associated documentation files (the
// "Software"), to deal in the Software without restriction, including
// without limitation the rights to use, copy, modify, merge, publish,
// distribute, sublicense, and/or sell copies of the Software, and to permit
// persons to whom the Software is furnished to do so, subject to the
// following conditions:
//
// The above copyright notice and this permission notice shall be included
// in all copies or substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
// USE OR OTHER DEALINGS IN THE SOFTWARE.

'use strict';

const assert = require('assert');
const path = require('path');
const remark = require('remark');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that with this change we now have two Markdown engines in use. This is unideal obviously, but truthfully I don't think it's a huge issue. I also think it probably makes sense to convert other uses of marked to Remark anyway, since the latter has much better tooling. But we can do that later.

const man = require('remark-man');
const parser = remark().use(fixupHeader).use(synopsis).use(stability).use(description).use(man);
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 should make this line not so long but it's 1 AM and I want to go to bed. Tomorrow!

const dontBuild = ['_toc', 'all', 'addons', 'async_hooks', 'cli', 'deprecations', 'documentation', 'domain', 'errors', 'globals', 'http2', 'index', 'inspector', 'intl', 'n-api', 'process', 'punycode', 'synopsis', 'tracing', 'v8'];

module.exports = toMan;

function fixupHeader(opts) {
return function _fixupHeader(ast, file, next) {
const header = ast.children[0];

assert.strictEqual(header.type, 'heading', 'First node is not a header');
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 actually like this style better (and functionally it doesn't super matter since we just throw either way) but it seems like this should probably just pass an Error to next()... if people disagree with that assessment though I'd prefer to keep it this way. It's more concise and obvious what's happening.

assert.strictEqual(header.depth, 1, 'Header is not of depth 1');

const moduleName = header.children[0].value;
header.children[0].value = `node-${moduleName.toLowerCase()}(3)`;
header.children[0].value += ` -- Node.js ${moduleName} module`;

// Attach the module name for other plugins in this file to use
file.moduleName = moduleName;

next();
};
}

function synopsis(opts) {
return function _synopsis(ast, file, next) {
const moduleName = file.moduleName.toLowerCase();

const heading = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This way of building nodes is kinda fugly but I'm not sure there's a better way. https://github.com/eush77/unist-builder perhaps?

type: 'heading',
depth: 2,
children: [
{
type: 'text',
value: 'SYNOPSIS'
}
]
};

const text = {
type: 'paragraph',
children: [
{
type: 'inlineCode',
value: `const ${moduleName} = require('${moduleName}');`
}
]
};

ast.children.splice(1, 0, heading);
ast.children.splice(2, 0, text);

next();
};
}

function stability(opts) {
return function _stability(ast, file, next) {
const node = ast.children[4];

assert.equal(node.type, 'blockquote', 'Stability information in unexpected location');

const heading = {
type: 'heading',
depth: 2,
children: [
{
type: 'text',
value: 'STABILITY'
}
]
};

ast.children.splice(3, 0, heading);
// Unwrap the paragraph inside the blockquote
ast.children[4] = node.children[0];

next();
};
}

function description(opts) {
return function _description(ast, file, next) {
const heading = {
type: 'heading',
depth: 2,
children: [
{
type: 'text',
value: 'DESCRIPTION'
}
]
};

ast.children.splice(5, 0, heading);

next();
};
}
function toMan(input, inputFile, cb) {
// Silently skip things we can't/shouldn't build
if (dontBuild.includes(path.parse(inputFile).name)) return;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is really hacky and leaves around empty files. Not good.


parser.process(input, function(er, file) {
cb(er, String(file));
});
}
21 changes: 21 additions & 0 deletions tools/doc/node_modules/array-iterate/LICENSE

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

43 changes: 43 additions & 0 deletions tools/doc/node_modules/array-iterate/index.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

102 changes: 102 additions & 0 deletions tools/doc/node_modules/array-iterate/package.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading