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.

Sign up for GitHub

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

Add task for building docs only, using existing Node #3888

Closed
wants to merge 1 commit into from
Closed

Add task for building docs only, using existing Node #3888

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions BUILDING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,20 @@ $ make test-npm

To build the documentation:

This will build Node.js first (if necessary) and then use it to build the docs:

```text
$ make doc
```

If you have an existing Node.js you can build just the docs with:

```text
$ NODE=node make doc-only
Copy link
Contributor

Choose a reason for hiding this comment

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

NODE=/path/to/node?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback. I'm going to suggest that the current version is more ideal because I expect the 80% case (really more like 99.9%) when doing make doc-only is that the user wants to use the node in their path. I did include this note right underneath that:

(Where node is the path to your executable.)

That could be expanded if it's not clear enough:

(Where node can be a pathname: NODE=/path/to/node make doc-only)

I suggested the possibility of npm run doc for the 80% case and if that happened I could see documenting it like that:

# Use the `node` in the path
npm run doc

# Point to the binary you want to use
NODE=/path/to/node make doc-only

```

(Where `node` is the path to your executable.)

To read the documentation:

```text
Expand Down
14 changes: 8 additions & 6 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ BUILDTYPE_LOWER := $(shell echo $(BUILDTYPE) | tr '[A-Z]' '[a-z]')
EXEEXT := $(shell $(PYTHON) -c \
"import sys; print('.exe' if sys.platform == 'win32' else '')")

NODE ?= ./node$(EXEEXT)
NODE_EXE = node$(EXEEXT)
NODE ?= ./$(NODE_EXE)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this change necessary? It seems to me that they are creating the exact same result (testing this too)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not strictly necessary. Yes, they're meant to create the same result. The purpose of that change was to reduce duplication since the default value for NODE literally duplicates the entire right hand side of NODE_EXE and prefixes it. I just consolidated it to incorporate the value via variable expansion instead of literal duplication.

NODE_G_EXE = node_g$(EXEEXT)

# Flags for packaging.
Expand Down Expand Up @@ -258,7 +258,9 @@ apidoc_dirs = out/doc out/doc/api/ out/doc/api/assets

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

doc: $(apidoc_dirs) $(apiassets) $(apidocs) tools/doc/ $(NODE_EXE)
doc-only: $(apidoc_dirs) $(apiassets) $(apidocs) tools/doc/

doc: $(NODE_EXE) doc-only

$(apidoc_dirs):
mkdir -p $@
Expand All @@ -269,11 +271,11 @@ out/doc/api/assets/%: doc/api_assets/% out/doc/api/assets/
out/doc/%: doc/%
cp -r $< $@

out/doc/api/%.json: doc/api/%.md $(NODE_EXE)
out/doc/api/%.json: doc/api/%.md
$(NODE) tools/doc/generate.js --format=json $< > $@

out/doc/api/%.html: doc/api/%.md $(NODE_EXE)
$(NODE) tools/doc/generate.js --format=html --template=doc/template.html $< > $@
out/doc/api/%.html: doc/api/%.md
$(NODE) tools/doc/generate.js --node-version=$(FULLVERSION) --format=html --template=doc/template.html $< > $@

docopen: out/doc/api/all.html
-google-chrome out/doc/api/all.html
Expand Down Expand Up @@ -663,5 +665,5 @@ endif
blog blogclean tar binary release-only bench-http-simple bench-idle \
bench-all bench bench-misc bench-array bench-buffer bench-net \
bench-http bench-fs bench-tls cctest run-ci test-v8 test-v8-intl \
test-v8-benchmarks test-v8-all v8 lint-ci bench-ci jslint-ci \
test-v8-benchmarks test-v8-all v8 lint-ci bench-ci jslint-ci doc-only \
$(TARBALL)-headers
13 changes: 9 additions & 4 deletions tools/doc/generate.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const args = process.argv.slice(2);
let format = 'json';
let template = null;
let inputFile = null;
let nodeVersion = null;

args.forEach(function(arg) {
if (!arg.match(/^\-\-/)) {
Expand All @@ -18,23 +19,22 @@ args.forEach(function(arg) {
format = arg.replace(/^\-\-format=/, '');
} else if (arg.match(/^\-\-template=/)) {
template = arg.replace(/^\-\-template=/, '');
} else if (arg.match(/^\-\-node\-version=/)) {
nodeVersion = arg.replace(/^\-\-node\-version=/, '');
}
});


if (!inputFile) {
throw new Error('No input file specified');
}


console.error('Input file = %s', inputFile);
fs.readFile(inputFile, 'utf8', function(er, input) {
if (er) throw er;
// process the input for @include lines
processIncludes(inputFile, input, next);
});


function next(er, input) {
if (er) throw er;
switch (format) {
Expand All @@ -46,7 +46,12 @@ function next(er, input) {
break;

case 'html':
require('./html.js')(input, inputFile, template, function(er, html) {
require('./html.js')({
input: input,
filename: inputFile,
template: template,
nodeVersion: nodeVersion,
}, function(er, html) {
if (er) throw er;
console.log(html);
});
Expand Down
27 changes: 22 additions & 5 deletions tools/doc/html.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,12 @@ var gtocPath = path.resolve(path.join(
var gtocLoading = null;
var gtocData = null;

function toHTML(input, filename, template, cb) {
/**
* opts: input, filename, template, nodeVersion.
*/
function toHTML(opts, cb) {
var template = opts.template;

if (gtocData) {
return onGtocLoaded();
}
Expand All @@ -51,10 +56,15 @@ function toHTML(input, filename, template, cb) {
}

function onGtocLoaded() {
var lexed = marked.lexer(input);
var lexed = marked.lexer(opts.input);
fs.readFile(template, 'utf8', function(er, template) {
if (er) return cb(er);
render(lexed, filename, template, cb);
render({
lexed: lexed,
filename: opts.filename,
template: template,
nodeVersion: opts.nodeVersion,
}, cb);
});
}
}
Expand All @@ -81,7 +91,14 @@ function toID(filename) {
.replace(/-+/g, '-');
}

function render(lexed, filename, template, cb) {
/**
* opts: lexed, filename, template, nodeVersion.
*/
function render(opts, cb) {
var lexed = opts.lexed;
var filename = opts.filename;
var template = opts.template;

// get the section
var section = getSection(lexed);

Expand All @@ -100,7 +117,7 @@ function render(lexed, filename, template, cb) {
template = template.replace(/__ID__/g, id);
template = template.replace(/__FILENAME__/g, filename);
template = template.replace(/__SECTION__/g, section);
template = template.replace(/__VERSION__/g, process.version);
template = template.replace(/__VERSION__/g, opts.nodeVersion);
template = template.replace(/__TOC__/g, toc);
template = template.replace(
/__GTOC__/g,
Expand Down