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

Fix nunjucks for windows: relative paths and tests #391

Merged
merged 10 commits into from
Mar 11, 2015
Merged
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
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 0 additions & 8 deletions Makefile

This file was deleted.

27 changes: 27 additions & 0 deletions appveyor.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Fix line endings in Windows. (runs before repo cloning)
init:
- git config --global core.autocrlf input

# Test against these versions of Node.js.
environment:
matrix:
- nodejs_version: "0.11"
- nodejs_version: "0.10"

# Install scripts. (runs after repo cloning)
install:
# Get the latest stable version of Node.js or io.js
- ps: Install-Product node $env:nodejs_version
# install modules
- npm install

# Post-install test scripts.
test_script:
# Output useful info for debugging.
- node --version
- npm --version
# run tests
- npm test

# Don't actually build.
build: off
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@
"node": "*"
},
"scripts": {
"test": "make test"
"test": "./node_modules/.bin/istanbul cover ./node_modules/mocha/bin/_mocha -- -b -R tap tests",
"browserfiles": "./bin/bundle browser/nunjucks; SLIM=1 ./bin/bundle browser/nunjucks-slim"
},
"bin": {
"nunjucks-precompile": "./bin/precompile"
Expand Down
2 changes: 1 addition & 1 deletion src/compiler.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ var Compiler = Object.extend({
},

_templateName: function() {
return this.templateName == null? 'undefined' : '"'+this.templateName.replace(/"/g, '\\"')+'"';
return this.templateName == null? 'undefined' : JSON.stringify(this.templateName);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why use JSON.stringify here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because we need to escape the templateName to be work in a javascript string, I think it's most secure way to do it, we need to handle \, ", ', ...
By using .replace, we may forget one case or not handle the combinaison.

What is you suggestion?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One other solution could be the use of https://github.com/joliss/js-string-escape

Copy link
Contributor

Choose a reason for hiding this comment

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

Sound good.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Which solution?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think JSON.stringify is ok in this case that is only replace " originally.

},

_bufferAppend: function(func) {
Expand Down
43 changes: 21 additions & 22 deletions src/precompile.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,27 @@ function precompile(input, opts) {

var pathStats = fs.existsSync(input) && fs.statSync(input);
var output = '';
var templates = [];

function addTemplates(dir) {
var files = fs.readdirSync(dir);

for(var i=0; i<files.length; i++) {
var filepath = path.join(dir, files[i]);
var subpath = filepath.substr(path.join(input, '/').length);
var stat = fs.statSync(filepath);

if(stat && stat.isDirectory()) {
subpath += '/';
if (!match(subpath, opts.exclude)) {
addTemplates(filepath);
}
}
else if(match(subpath, opts.include)) {
templates.push(filepath);
}
}
}

if(opts.isString) {
if(!opts.name) {
Expand All @@ -56,28 +77,6 @@ function precompile(input, opts) {
opts.asFunction);
}
else if(pathStats.isDirectory()) {
var templates = [];

function addTemplates(dir) {
var files = fs.readdirSync(dir);

for(var i=0; i<files.length; i++) {
var filepath = path.join(dir, files[i]);
var subpath = filepath.substr(path.join(input, '/').length);
var stat = fs.statSync(filepath);

if(stat && stat.isDirectory()) {
subpath += '/';
if (!match(subpath, opts.exclude)) {
addTemplates(filepath);
}
}
else if(match(subpath, opts.include)) {
templates.push(filepath);
}
}
}

addTemplates(input);

for(var i=0; i<templates.length; i++) {
Expand Down
6 changes: 4 additions & 2 deletions tests/api.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
(function() {
'use strict';

var expect, Environment, Loader, templatesPath;
var expect, util, Environment, Loader, templatesPath;
var path = require('path');
var os = require('os');

if(typeof require !== 'undefined') {
expect = require('expect.js');
util = require('./util');
Environment = require('../src/environment').Environment;
Loader = require('../src/node-loaders').FileSystemLoader;
templatesPath = 'tests/templates';
Expand Down Expand Up @@ -41,7 +43,7 @@

var test = env.getTemplate('relative/test-cache.html');

expect(test.render()).to.be('Test1\nTest2');
expect(util.normEOL(test.render())).to.be('Test1\nTest2');
});

it('should handle correctly relative paths in renderString', function() {
Expand Down
22 changes: 22 additions & 0 deletions tests/precompile.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
(function() {
'use strict';

var expect, precompile, precompileString;

if(typeof require !== 'undefined') {
expect = require('expect.js');
precompile = require('../src/precompile').precompile;
precompileString = require('../src/precompile').precompileString;
}
else {
expect = window.expect;
precompile = nunjucks.precompile;
precompileString = nunjucks.precompileString
}

describe('precompile', function() {
it('should return a string', function() {
expect(precompileString("{{ test }}", { name: "test.html" })).to.be.an('string');
});
});
})();
11 changes: 9 additions & 2 deletions tests/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,11 @@
}
}

function normEOL(str) {
if (!str) return str;
return str.replace(/\r\n|\r/g, "\n");
}

function render(str, ctx, opts, cb) {
if(!opts) {
cb = ctx;
Expand Down Expand Up @@ -95,7 +100,7 @@
throw err;
}

cb(err, res);
cb(err, normEOL(res));

doneAsyncs++;

Expand All @@ -110,12 +115,14 @@
module.exports.render = render;
module.exports.equal = equal;
module.exports.finish = finish;
module.exports.normEOL = normEOL;
}
else {
window.util = {
render: render,
equal: equal,
finish: finish
finish: finish,
normEOL: normEOL
};
}
})();