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

preprocessor config, fix for #212 #222

Merged
merged 9 commits into from
Apr 22, 2014
Merged
27 changes: 27 additions & 0 deletions lib/yuidoc.js
Original file line number Diff line number Diff line change
Expand Up @@ -245,8 +245,33 @@ YUI.add('yuidoc', function (Y) {
}
},

/**
* Applies preprocessors to the data tree.
* @method applyPreprocessors
* @private
*/
applyPreprocessors: function (data) {
var preprocessors,
preprocessorsRelativeTo;

// If the preprocessor paths are relative they will be searched for relative
// to the process working directory. This is consistent with how other paths
// are treated by yuidoc, such as the config options 'paths' and 'outdir'.
preprocessorsRelativeTo = process.cwd();

if (this.options.preprocessor) {
preprocessors = [].concat(this.options.preprocessor);

preprocessors.forEach(function (preprocessor) {
var preprocessorModule = require(path.resolve(preprocessorsRelativeTo, preprocessor));
Copy link
Member

Choose a reason for hiding this comment

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

I think we should also consider passing preprocessors in a form of nodejs modules, saying I have a nodejs module that export a function, and I want to install that at the top level as devDependencies, then passing that name into the preprocessor option. For that, I think we can do:

try {
   preprocessorModule = require(preprocessor);
} catch (e) {
   preprocessorModule = require(path.resolve(preprocessorsRelativeTo, preprocessor));
}

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 makes sense. This way it will be possible to create "yuidoc plugins" and distribute them as separate npm modules as well, so I will fix this...

preprocessorModule(data);
});
}
},

/**
* Writes the parser JSON data to disk.
* Applies preprocessors, if any.
* @method writeJSON
* @param {Object} parser The DocParser instance to use
* @private
Expand All @@ -262,6 +287,8 @@ YUI.add('yuidoc', function (Y) {

data.warnings = parser.warnings;

this.applyPreprocessors(data);
Copy link
Member

Choose a reason for hiding this comment

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

applyPreprocessors seems like a weird name for a method that mutates the data. I will recommend to call it augmentData(), or simply make that method to return the mutated data so you can do something like:

data = this.runPreprocessors(data);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, I can change that name.

In a sense I also agree that It would make sense to have it return the modified data instead of mutating it. However, this would not be the case for the actual preprocessor modules. Consider for example a module like this that does nothing:

module.exports=function(data) {
}

I think that it would be less error prune for the user if this function would just do nothing, rather than break the whole documentation build process because it has a missing return statement.

Still, I agree that it would be cleaner if the actual function runPreprocessors would return the new data rather than mutate it. Maybe a way to get "the best of both worlds" would be for the runPreprocessors to first clone the data and then pass it to the preprocessors? I will test this and see what it will look like.


if (self.selleck && self.options.selleck && data.files && data.modules) {
Object.keys(self.selleck).forEach(function (file) {
Object.keys(data.files).forEach(function (f) {
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
},
"scripts": {
"pretest": "jshint ./lib/*.js ./tests/*.js",
"test": "istanbul cover --print=both --yui ytestrunner -- --include ./tests/options.js --include ./tests/builder.js --include ./tests/parser.js --include ./tests/parser_coffee.js --include ./tests/files.js --include ./tests/utils.js"
"test": "istanbul cover --print=both --yui ytestrunner -- --include ./tests/options.js --include ./tests/builder.js --include ./tests/parser.js --include ./tests/parser_coffee.js --include ./tests/files.js --include ./tests/utils.js --include ./tests/preprocessor.js"
},
"preferGlobal": "true",
"licenses":[
Expand Down
3 changes: 2 additions & 1 deletion scripts/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ wait
./builder.js \
./options.js \
./utils.js \
./files.js
./files.js \
./preprocessor.js

exit $?

5 changes: 5 additions & 0 deletions tests/input/preprocessor/preprocessortest.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
/**
* This class is for testing the preprocessor option.
* @class TestPreprocessor
* @customtag hello
*/
9 changes: 9 additions & 0 deletions tests/lib/testpreprocessor.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
module.exports=function(data) {
global.testPreprocessorCallCount++;

for (className in data.classes) {
classData=data.classes[className];

classData.customtagPlusStar=classData.customtag+"*";
}
}
71 changes: 71 additions & 0 deletions tests/preprocessor.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/*global Y:true */
var YUITest = require('yuitest'),
Assert = YUITest.Assert,
path = require('path'),
Y = require(path.join(__dirname, '../', 'lib', 'index'));

//Move to the test dir before running the tests.
process.chdir(__dirname);

var suite = new YUITest.TestSuite({
name: 'Preprocessor Test Suite',
setUp: function () {
process.chdir(__dirname);
}
});

suite.add(new YUITest.TestCase({
name: "Preprocessor",
'test: single preprocessor': function () {
global.testPreprocessorCallCount=0;

var json = (new Y.YUIDoc({
quiet: true,
paths: ['input/preprocessor'],
outdir: './out',
preprocessor: 'lib/testpreprocessor.js'
})).run();

Assert.isObject(json);
Assert.areSame(global.testPreprocessorCallCount,1,"the preprocessor was not called");
},
'test: single preprocessor with absolute path': function () {
global.testPreprocessorCallCount=0;

var json = (new Y.YUIDoc({
quiet: true,
paths: ['input/preprocessor'],
outdir: './out',
preprocessor: path.join(process.cwd(),'lib/testpreprocessor.js')
})).run();

Assert.isObject(json);
Assert.areSame(global.testPreprocessorCallCount,1,"the preprocessor was not called when an absolute path was used");
},
'test: several preprocessors': function () {
global.testPreprocessorCallCount=0;

var json = (new Y.YUIDoc({
quiet: true,
paths: ['input/preprocessor'],
outdir: './out',
preprocessor: ['lib/testpreprocessor.js','./lib/testpreprocessor']
})).run();

Assert.isObject(json);
Assert.areSame(global.testPreprocessorCallCount,2,"the preprocessor was not called twice");
},
'test: the test preprocessor does its job': function () {
var json = (new Y.YUIDoc({
quiet: true,
paths: ['input/preprocessor'],
outdir: './out',
preprocessor: 'lib/testpreprocessor.js'
})).run();

Assert.isObject(json);
Assert.areSame(json.classes.TestPreprocessor.customtagPlusStar,"hello*","the preprocessor did not modify the data");
}
}));

YUITest.TestRunner.add(suite);