Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

json doc generator now generates _index.json as it creates files. #3696

Open
wants to merge 4 commits into from

6 participants

@iapain

generate.js now accepts --output=<file> which tools/doc/json.js uses to write the output. If output file is missing then it executes callback. This was required to get the directory for new _index.json. Ideally, I left it for you to decide what to call this _index.json. There is already an index.json file which should not be generated at all. Additionally it sets title as first heading node text if applicable.

TODO

  • Add exclude files to avoid generation of index.json
  • Dependency map in Makefile (if subchapters like structure is required).
@iapain iapain Added output file option to gentest.
json doc generator now accepts output file as well.
json doc generator now generates index file automatically.
5152bf5
@iapain iapain closed this
@iapain iapain reopened this
@iapain

Damn I cannot attach it to the issue. It's for #3668

@mainerror

What's up with this pull request? Is it going to get merged?

@iapain

It still needs some API related decisions from @isaacs. He urged me to write this patch but probably too busy with other stuff.

@mainerror

I reckoned, just wanted to bump this a little.

@isaacs
Owner

The index should be written to index.json, not _index.json.

Also, it's a bit odd having url be a an object. Maybe it should be just html: 'foo.html', json: 'foo.json' or something?

@iapain

@isaacs The reason I kept _index.json was index.markdown translates into index.json but I guess it's no longer required. In last commit I have renamed it to index.json. Moreover url.html, url.json is now just html, json as you suggested.

@Nodejs-Jenkins

Can one of the admins verify this patch?

@mainerror

Indeed, it would be awesome to get this in, I don't quite understand what's taking eight months to review ...

@bnoordhuis

Yeah, because we're all just twiddling our thumbs while raking in the big paychecks...

Look at the number of open issues and pull requests, now realize that there are only two or three people working on node.js full time.

If you want to do something constructive, start triaging bug reports and reviewing PRs.

@mainerror

@bnoordhuis I didn't say that you guys weren't doing something but eight months for such a minor PR?

I'm not actively working on Node.js, I have other projects I'm actively contributing to.

@iapain

I must agree with @mainerror being a contributor it's very discouraging. I tried to bring it up few times on IRC to @isaacs in vain.

@bnoordhuis I totally respect what you guys are doing it's pretty cool and cutting edge but eventually it's core team who needs to organize and expand if it's required.

tools/doc/json.js
@@ -146,9 +159,62 @@ function doJSON(input, filename, cb) {
finishSection(current, stack[stack.length - 1]);
}
- return cb(null, root)
+ if (outfile) {
+ writeOututToFile(root, filename, outfile, indexfile, writeToIndexFile);
@isaacs Owner
isaacs added a note

Typo? Should this be writeOutputToFile?

@iapain
iapain added a note

Sorry that's a typo. Will rectify in next commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
tools/doc/json.js
((4 lines not shown))
- return cb(null, root)
+ if (outfile) {
+ writeOututToFile(root, filename, outfile, indexfile, writeToIndexFile);
+ }
+ else {
+ return cb(null, root)
+ }
+}
+
+// write output object to outfile
+function writeOututToFile(obj, sourcefile, outfile, indexfile, cb) {
+ fs.writeFile(outfile, JSON.stringify(obj, null, 2), function(err) {
+ if(err) {
+ throw new Error('error saving file - '+ err);
+ }
+ cb(obj, sourcefile, path.join(path.dirname(outfile), indexfile));
@isaacs Owner
isaacs added a note

API here is a bit weird. cb functions should pass err as their first argument. Better to just call fs.writeFile with the supplied cb, than to force a throw.

@iapain
iapain added a note

I tried to keep the way it worked with console.log version. I remember there it threw an error as well but I can see you're right and in this setup it doesn't make any sense to throw an error. I will clean up this in a moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@isaacs
Owner

In general, it is better to light candles than to curse darkness. @mainerror, if you have comments on this patch, then share them. We welcome reviews from all parties. Though it does take some core team member to actually land it, you can help things along by providing feedback.

@iapain I'm a bit confused about what happens if just one of the files changes. Ie, if you've generated the index.json, and then we change doc/api/url.markdown, for instance. Will it generate a new index.json containing all of the previous files as well? How does it know to do index.json last?

@iapain

@isaacs I'm not tracking anything, it just regenerates index.json. Should it keep track for changes?

@iapain

@isaacs changes checked-in. I think now I got it what you were trying to say in previous comment. Actually writeToIndexFile checks if there is already an index.json file. It also check (in a way) that if it's proper index.json and then it update or create proper chapter entry into main json object which is written back to index.json

@chrisdickinson chrisdickinson commented on the diff
tools/doc/json.js
@@ -146,9 +159,64 @@ function doJSON(input, filename, cb) {
finishSection(current, stack[stack.length - 1]);
}
- return cb(null, root)
+ if (outfile) {
+ writeOutputToFile(root, filename, outfile, indexfile, writeToIndexFile);
@chrisdickinson Owner

Okay, I see the source of my confusion. writeToIndexFile performs async actions, but never calls cb. This probably needs to be something like the following:

writeOutputToFile(root, filename, outfile, indexfile, function(err, root, sourcefile, outfile) {
  if (err) return cb(err);
  writeToIndexFile(root, sourcefile, outfile, cb);
})
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@chrisdickinson chrisdickinson commented on the diff
tools/doc/json.js
((11 lines not shown))
}
+// write output object to outfile
+function writeOutputToFile(obj, sourcefile, outfile, indexfile, cb) {
+ fs.writeFile(outfile, JSON.stringify(obj, null, 2), function(err) {
+ cb(err, obj, sourcefile, path.join(path.dirname(outfile), indexfile));
+ });
+}
+
+// make an entry into index file
+function writeToIndexFile(err, root, sourcefile, outfile) {
+ // check if there was an error writing file
+ if (err) {
+ throw new Error('error writing file - '+ e);
+ }
@chrisdickinson Owner

this error handling should happen in doJSON -- including the err argument that writeToIndexFile takes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@chrisdickinson chrisdickinson commented on the diff
tools/doc/json.js
((23 lines not shown))
+ if (err) {
+ throw new Error('error writing file - '+ e);
+ }
+
+ // default type of an index
+ var obj = {"type":"index"};
+ var entry = {"source":sourcefile};
+
+ // check if indexfile already exists
+ if (fs.existsSync(outfile)) {
+ var data = fs.readFileSync(outfile);
+ try {
+ obj = JSON.parse(data.toString());
+ }
+ catch(e) {
+ throw new Error('invalid json data - '+ e);
@chrisdickinson Owner

If there's invalid JSON data, we should ignore it, and go with our default object.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@chrisdickinson chrisdickinson commented on the diff
tools/doc/json.js
((29 lines not shown))
+ var entry = {"source":sourcefile};
+
+ // check if indexfile already exists
+ if (fs.existsSync(outfile)) {
+ var data = fs.readFileSync(outfile);
+ try {
+ obj = JSON.parse(data.toString());
+ }
+ catch(e) {
+ throw new Error('invalid json data - '+ e);
+ }
+ }
+ // check if index file is valid
+ if (obj.type !== "index") {
+ throw new Error('invalid index file - '+ outfile);
+ }
@chrisdickinson Owner

I think this check is maybe unnecessary. Ultimately I think our goal should be to merge the chapters property into whatever's already there, or if nothing's there, create it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@chrisdickinson chrisdickinson commented on the diff
tools/doc/json.js
((44 lines not shown))
+ }
+ // construct an entry object
+ entry.title = root.title;
+ entry.html = path.basename(sourcefile).replace(/\.(markdown|md)/i, ".html");
+ entry.json = entry.html.replace(/\.html/i, ".json");
+
+ // append mode
+ if (obj.chapters && typeof obj.chapters === "object") {
+ obj.chapters.push(entry);
+ }
+ else {
+ obj.chapters = [entry];
+ }
+ fs.writeFile(outfile, JSON.stringify(obj, null, 2), function(err) {
+ if(err) {
+ throw new Error('error saving file - '+ err);
@chrisdickinson Owner

return cb(new Error('[...]')) vs. throwing here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@chrisdickinson

@iapain Thanks for your patience. I went through and reviewed the changes and had a few comments, and will merge once they're addressed -- or, if you'd prefer I can take over this PR. Sorry you had to wait so long!

@iapain

@chrisdickinson Please take over this PR. Excited to see that it's finally coming :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jul 12, 2012
  1. @iapain

    Added output file option to gentest.

    iapain authored
    json doc generator now accepts output file as well.
    json doc generator now generates index file automatically.
Commits on Oct 3, 2012
  1. @iapain

    Now json doc generator writes index.json file.\n Renamed url.html and…

    iapain authored
    … url.json to html and json
Commits on Mar 14, 2013
  1. @iapain
  2. @iapain
This page is out of date. Refresh to see the latest.
Showing with 76 additions and 5 deletions.
  1. +1 −1  Makefile
  2. +4 −1 tools/doc/generate.js
  3. +71 −3 tools/doc/json.js
View
2  Makefile
@@ -176,7 +176,7 @@ out/doc/%: doc/%
cp -r $< $@
out/doc/api/%.json: doc/api/%.markdown node
- out/Release/node tools/doc/generate.js --format=json $< > $@
+ out/Release/node tools/doc/generate.js --format=json $< --output=$@
out/doc/api/%.html: doc/api/%.markdown node
out/Release/node tools/doc/generate.js --format=html --template=doc/template.html $< > $@
View
5 tools/doc/generate.js
@@ -31,6 +31,7 @@ var args = process.argv.slice(2);
var format = 'json';
var template = null;
var inputFile = null;
+var outputFile = null;
args.forEach(function (arg) {
if (!arg.match(/^\-\-/)) {
@@ -39,6 +40,8 @@ args.forEach(function (arg) {
format = arg.replace(/^\-\-format=/, '');
} else if (arg.match(/^\-\-template=/)) {
template = arg.replace(/^\-\-template=/, '');
+ } else if (arg.match(/^\-\-output=/)) {
+ outputFile = arg.replace(/^\-\-output=/, '');
}
})
@@ -100,7 +103,7 @@ function next(er, input) {
if (er) throw er;
switch (format) {
case 'json':
- require('./json.js')(input, inputFile, function(er, obj) {
+ require('./json.js')(input, inputFile, outputFile, function(er, obj) {
console.log(JSON.stringify(obj, null, 2));
if (er) throw er;
});
View
74 tools/doc/json.js
@@ -23,16 +23,25 @@ module.exports = doJSON;
// Take the lexed input, and return a JSON-encoded object
// A module looks like this: https://gist.github.com/1777387
-
+var fs = require('fs');
+var path = require('path');
var marked = require('marked');
-function doJSON(input, filename, cb) {
+function doJSON(input, filename, outfile, cb) {
var root = {source: filename};
+ // Default index file
+ var indexfile = "index.json";
var stack = [root];
var depth = 0;
var current = root;
var state = null;
var lexed = marked.lexer(input);
+
+ // If outfile is same as index file do nothing
+ if (outfile && outfile.slice(-indexfile.length) === indexfile) {
+ return;
+ }
+
lexed.forEach(function (tok) {
var type = tok.type;
var text = tok.text;
@@ -55,6 +64,10 @@ function doJSON(input, filename, cb) {
return cb(new Error('Inappropriate heading level\n'+
JSON.stringify(tok)));
}
+ // set first heading to title
+ if (current && !current.title) {
+ current.title = tok.text;
+ }
// Sometimes we have two headings with a single
// blob of description. Treat as a clone.
@@ -146,9 +159,64 @@ function doJSON(input, filename, cb) {
finishSection(current, stack[stack.length - 1]);
}
- return cb(null, root)
+ if (outfile) {
+ writeOutputToFile(root, filename, outfile, indexfile, writeToIndexFile);
@chrisdickinson Owner

Okay, I see the source of my confusion. writeToIndexFile performs async actions, but never calls cb. This probably needs to be something like the following:

writeOutputToFile(root, filename, outfile, indexfile, function(err, root, sourcefile, outfile) {
  if (err) return cb(err);
  writeToIndexFile(root, sourcefile, outfile, cb);
})
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ }
+ else {
+ return cb(null, root)
+ }
}
+// write output object to outfile
+function writeOutputToFile(obj, sourcefile, outfile, indexfile, cb) {
+ fs.writeFile(outfile, JSON.stringify(obj, null, 2), function(err) {
+ cb(err, obj, sourcefile, path.join(path.dirname(outfile), indexfile));
+ });
+}
+
+// make an entry into index file
+function writeToIndexFile(err, root, sourcefile, outfile) {
+ // check if there was an error writing file
+ if (err) {
+ throw new Error('error writing file - '+ e);
+ }
@chrisdickinson Owner

this error handling should happen in doJSON -- including the err argument that writeToIndexFile takes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
+ // default type of an index
+ var obj = {"type":"index"};
+ var entry = {"source":sourcefile};
+
+ // check if indexfile already exists
+ if (fs.existsSync(outfile)) {
+ var data = fs.readFileSync(outfile);
+ try {
+ obj = JSON.parse(data.toString());
+ }
+ catch(e) {
+ throw new Error('invalid json data - '+ e);
@chrisdickinson Owner

If there's invalid JSON data, we should ignore it, and go with our default object.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ }
+ }
+ // check if index file is valid
+ if (obj.type !== "index") {
+ throw new Error('invalid index file - '+ outfile);
+ }
@chrisdickinson Owner

I think this check is maybe unnecessary. Ultimately I think our goal should be to merge the chapters property into whatever's already there, or if nothing's there, create it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ // construct an entry object
+ entry.title = root.title;
+ entry.html = path.basename(sourcefile).replace(/\.(markdown|md)/i, ".html");
+ entry.json = entry.html.replace(/\.html/i, ".json");
+
+ // append mode
+ if (obj.chapters && typeof obj.chapters === "object") {
+ obj.chapters.push(entry);
+ }
+ else {
+ obj.chapters = [entry];
+ }
+ fs.writeFile(outfile, JSON.stringify(obj, null, 2), function(err) {
+ if(err) {
+ throw new Error('error saving file - '+ err);
@chrisdickinson Owner

return cb(new Error('[...]')) vs. throwing here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ }
+ });
+}
// go from something like this:
// [ { type: 'list_item_start' },
Something went wrong with that request. Please try again.