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
37 changes: 36 additions & 1 deletion lib/yuidoc.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ YUI.add('yuidoc', function (Y) {
version: '0.1.0',
paths: [],
themedir: path.join(__dirname, 'themes', 'default'),
syntaxtype: 'js'
syntaxtype: 'js',
preprocessor: undefined
Copy link
Member

Choose a reason for hiding this comment

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

not need to define an undefined value.

};

/**
Expand Down Expand Up @@ -245,8 +246,40 @@ YUI.add('yuidoc', function (Y) {
}
},

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

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

for (p=0; p<preprocessors.length; p++) {
preprocessor=preprocessors[p];

// Check if path is absolute, from:
// http://stackoverflow.com/questions/21698906/how-to-check-if-a-path-is-absolute-or-relative
if (path.resolve(preprocessor)===preprocessor) {
preprocessorAbsFile=preprocessor;
}

else {
preprocessorAbsFile=path.join(process.cwd(),preprocessor);
}

preprocessorModule=require(preprocessorAbsFile);
Copy link
Member

Choose a reason for hiding this comment

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

you don't need any of the conditions above, just do:

var pathToConfigFile = (/* we need to figure the path of the config which is not necessarily the process.cwd()*/) || process.cwd();
preprocessorModule=require(path.resolve(pathToConfigFile, preprocessor));

that's what will guarantee that in the config you can reference relative paths.

the other option is to use npm modules for each processor, in which case require(preprocessor) will be just enough. although this is problematic for people who only want to use yuidocjs without having to deal with npm stuff.

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 @@ -260,6 +293,8 @@ YUI.add('yuidoc', function (Y) {

data = parser.data;

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.


data.warnings = parser.warnings;

if (self.selleck && self.options.selleck && data.files && data.modules) {
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);