Skip to content

Conversation

@parasyte
Copy link

I discovered a bug with filename extension removal in dot.process when using additional . characters in the filename. Example directory structure:

src/video/webgl/glsl/
|-- fragment.glsl.dot
`-- vertex.glsl.dot

Processed with:

console.log(Object.keys(dot.process({
    src : "src/video/webgl/glsl/"
})));

Outputs the following keys:

["fragment", "vertex"]

The expected "glsl" extension has been removed. The patch fixes this issue by using String.prototype.replace instead of locating the first . character.

@epoberezkin
Copy link
Collaborator

@parasyte it can be considered after tests for doT.process are added.

@obiot
Copy link

obiot commented Nov 3, 2016

+1 ! :)

@parasyte
Copy link
Author

parasyte commented Nov 8, 2016

@epoberezkin index.js is not currently tested, and compileAll is not really testable because it uses the native fs module. I attempted to mock it with mock-fs, but ran into too many problems with mocha when using it.

How would you like to proceed?

@parasyte
Copy link
Author

parasyte commented Nov 8, 2016

Here's a index.test.js that seems to work (tested with the patch rebased against master):

'use strict';

var mockfs = require("mock-fs");
var assert = require("assert")
var doT = require("../index");
doT.log = false;

describe('index', function() {
    describe('process', function() {
        it('should preserve filenames', function() {
            mockfs({
                './mock-fs': {
                    'foo.dot': '<html>{{= bar }}</html>',
                    'foo.bar.dot': '<xml>{{= bar }}</xml>',
                    'bar.def': 'included',
                    'baz.jst': 'function(){}'
                }
            });

            var compiled = doT.process({
                'path': './mock-fs'
            });

            mockfs.restore();

            assert.equal(typeof compiled.foo, 'function');
            assert.equal(typeof compiled['foo.bar'], 'function');
        });
    });
});

It's ugly, but it's pretty much the best I could do with the limited mocking for fs.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants