Merge pull request #983 from GitbookIO/fix/982

Fix #982: correctly handle templating in imported content

7 files changed, 169 insertions, 88 deletions

diff --git a/lib/conrefs_loader.js b/lib/conrefs_loader.js
index a6c2049..255bf06 100644
--- a/lib/conrefs_loader.js
+++ b/lib/conrefs_loader.js
@@ -1,15 +1,19 @@
-var path = require("path");
-var nunjucks = require("nunjucks");
+var _ = require('lodash');
+var path = require('path');
+var nunjucks = require('nunjucks');
 
-var git = require("./utils/git");
-var fs = require("./utils/fs");
-var pathUtil = require("./utils/path");
+var git = require('./utils/git');
+var fs = require('./utils/fs');
+var pathUtil = require('./utils/path');
 
 // The loader should handle relative and git url
 var BookLoader = nunjucks.Loader.extend({
     async: true,
 
-    init: function(book) {
+    init: function(book, opts) {
+        this.opts = _.defaults(opts || {}, {
+            interpolate: _.identity
+        });
         this.book = book;
     },
 
@@ -20,14 +24,20 @@ var BookLoader = nunjucks.Loader.extend({
         .then(function(filepath) {
             // Is local file
             if (!filepath) filepath = path.resolve(fileurl);
-            else that.book.log.debug.ln("resolve from git", fileurl, "to", filepath);
+            else that.book.log.debug.ln('resolve from git', fileurl, 'to', filepath);
 
             //  Read file from absolute path
             return fs.readFile(filepath)
             .then(function(source) {
+                return that.opts.interpolate(filepath, source.toString());
+            })
+            .then(function(source) {
                 return {
-                    src: source.toString(),
-                    path: filepath
+                    src: source,
+                    path: filepath,
+
+                    // We disable cache sincde content is modified (shortcuts, ...)
+                    noCache: true
                 };
             });
         })
@@ -53,7 +63,7 @@ var BookLoader = nunjucks.Loader.extend({
         return path.resolve(path.dirname(from), to);
     },
 
-    // Handle all files as relative, so that nunjucks pass responsability to "resolve"
+    // Handle all files as relative, so that nunjucks pass responsability to 'resolve'
     // Only git urls are considered as absolute
     isRelative: function(filename) {
         return !git.checkUrl(filename);
diff --git a/lib/template.js b/lib/template.js
index d9d3a2f..dac1201 100644
--- a/lib/template.js
+++ b/lib/template.js
@@ -1,12 +1,14 @@
-var _ = require("lodash");
-var Q = require("q");
-var nunjucks = require("nunjucks");
-var escapeStringRegexp = require("escape-string-regexp");
-
-var batch = require("./utils/batch");
-var pkg = require("../package.json");
-var defaultBlocks = require("./blocks");
-var BookLoader = require("./conrefs_loader");
+var _ = require('lodash');
+var Q = require('q');
+var path = require('path');
+var nunjucks = require('nunjucks');
+var parsers = require('gitbook-parsers');
+var escapeStringRegexp = require('escape-string-regexp');
+
+var batch = require('./utils/batch');
+var pkg = require('../package.json');
+var defaultBlocks = require('./blocks');
+var BookLoader = require('./conrefs_loader');
 
 // Normalize result from a block
 function normBlockResult(blk) {
@@ -16,24 +18,37 @@ function normBlockResult(blk) {
 
 
 var TemplateEngine = function(book) {
+    var that = this;
+
     this.book = book;
     this.log = this.book.log;
 
+    // Template loader
+    this.loader = new BookLoader(book, {
+        // Replace shortcuts in imported files
+        interpolate: function(filepath, source) {
+            var parser = parsers.get(path.extname(filepath));
+            var type = parser? parser.name : null;
+
+            return  that.applyShortcuts(type, source);
+        }
+    });
+
     // Nunjucks env
     this.env = new nunjucks.Environment(
-        new BookLoader(book),
+        this.loader,
         {
             // Escaping is done after by the markdown parser
             autoescape: false,
 
             // Tags
             tags: {
-                blockStart: "{%",
-                blockEnd: "%}",
-                variableStart: "{{",
-                variableEnd: "}}",
-                commentStart: "{###",
-                commentEnd: "###}"
+                blockStart: '{%',
+                blockEnd: '%}',
+                variableStart: '{{',
+                variableEnd: '}}',
+                commentStart: '{###',
+                commentEnd: '###}'
             }
         }
     );
@@ -60,20 +75,20 @@ TemplateEngine.prototype.processBlock = function(blk) {
         parse: false,
         post: undefined
     });
-    blk.id = _.uniqueId("blk");
+    blk.id = _.uniqueId('blk');
 
     var toAdd = (!blk.parse) || (blk.post !== undefined);
 
     // Add to global map
     if (toAdd) this.blockBodies[blk.id] = blk;
 
-    //Parsable block, just return it
+    // Parsable block, just return it
     if (blk.parse) {
         return blk.body;
     }
 
     // Return it as a position marker
-    return "@%@"+blk.id+"@%@";
+    return '@%@'+blk.id+'@%@';
 };
 
 // Replace position markers of blocks by body after processing
@@ -110,11 +125,13 @@ TemplateEngine.prototype.bindContext = function(func) {
 TemplateEngine.prototype.addFilter = function(filterName, func) {
     try {
         this.env.getFilter(filterName);
-        this.log.warn.ln("conflict in filters, \""+filterName+"\" is already set");
+        this.log.warn.ln('conflict in filters, \''+filterName+'\' is already set');
         return false;
-    } catch(e) {}
+    } catch(e) {
+        // Filter doesn't exist
+    }
 
-    this.log.debug.ln("add filter \""+filterName+"\"");
+    this.log.debug.ln('add filter \''+filterName+'\'');
     this.env.addFilter(filterName, this.bindContext(function() {
         var ctx = this;
         var args = Array.prototype.slice.apply(arguments);
@@ -138,7 +155,7 @@ TemplateEngine.prototype.addFilters = function(filters) {
 
 // Return nunjucks extension name of a block
 TemplateEngine.prototype.blockExtName = function(name) {
-    return "Block"+name+"Extension";
+    return 'Block'+name+'Extension';
 };
 
 // Test if a block is defined
@@ -167,7 +184,7 @@ TemplateEngine.prototype.addBlock = function(name, block) {
 
     block = _.defaults(block || {}, {
         shortcuts: [],
-        end: "end"+name,
+        end: 'end'+name,
         process: _.identity,
         blocks: []
     });
@@ -175,13 +192,13 @@ TemplateEngine.prototype.addBlock = function(name, block) {
     extName = this.blockExtName(name);
 
     if (this.hasBlock(name) && !defaultBlocks[name]) {
-        this.log.warn.ln("conflict in blocks, \""+name+"\" is already defined");
+        this.log.warn.ln('conflict in blocks, \''+name+'\' is already defined');
     }
 
     // Cleanup previous block
     this.removeBlock(name);
 
-    this.log.debug.ln("add block \""+name+"\"");
+    this.log.debug.ln('add block \''+name+'\'');
     this.blocks[name] = block;
 
     Ext = function () {
@@ -238,7 +255,7 @@ TemplateEngine.prototype.addBlock = function(name, block) {
                 bodies.push(subbodies[blockName][0].body);
             });
 
-            return new nodes.CallExtensionAsync(this, "run", args, bodies);
+            return new nodes.CallExtensionAsync(this, 'run', args, bodies);
         };
 
         this.run = function(context) {
@@ -287,7 +304,7 @@ TemplateEngine.prototype.addBlock = function(name, block) {
     // Add shortcuts
     if (!_.isArray(block.shortcuts)) block.shortcuts = [block.shortcuts];
     _.each(block.shortcuts, function(shortcut) {
-        this.log.debug.ln("add template shortcut from \""+shortcut.start+"\" to block \""+name+"\" for parsers ", shortcut.parsers);
+        this.log.debug.ln('add template shortcut from \''+shortcut.start+'\' to block \''+name+'\' for parsers ', shortcut.parsers);
         this.shortcuts.push({
             block: name,
             parsers: shortcut.parsers,
@@ -314,7 +331,7 @@ TemplateEngine.prototype.applyBlock = function(name, blk, ctx) {
     var func, block, r;
 
     block = this.blocks[name];
-    if (!block) throw new Error("Block not found \""+name+"\"");
+    if (!block) throw new Error('Block not found \''+name+'\'');
     if (_.isString(blk)) {
         blk = {
             body: blk
@@ -339,14 +356,19 @@ TemplateEngine.prototype.applyBlock = function(name, blk, ctx) {
 TemplateEngine.prototype._applyShortcut = function(parser, content, shortcut) {
     if (!_.contains(shortcut.parsers, parser)) return content;
     var regex = new RegExp(
-        escapeStringRegexp(shortcut.start) + "([\\s\\S]*?[^\\$])" + escapeStringRegexp(shortcut.end),
-       "g"
+        escapeStringRegexp(shortcut.start) + '([\\s\\S]*?[^\\$])' + escapeStringRegexp(shortcut.end),
+       'g'
     );
     return content.replace(regex, function(all, match) {
-        return "{% "+shortcut.tag.start+" %}"+ match + "{% "+shortcut.tag.end+" %}";
+        return '{% '+shortcut.tag.start+' %}'+ match + '{% '+shortcut.tag.end+' %}';
     });
 };
 
+// Apply all shortcuts to some template string
+TemplateEngine.prototype.applyShortcuts = function(type, content) {
+    return _.reduce(this.shortcuts, _.partial(this._applyShortcut.bind(this), type), content);
+};
+
 // Render a string from the book
 TemplateEngine.prototype.renderString = function(content, context, options) {
     context = _.extend({}, context, {
@@ -367,14 +389,18 @@ TemplateEngine.prototype.renderString = function(content, context, options) {
         type: null
     });
     if (options.path) options.path = this.book.resolve(options.path);
+    if (!options.type && options.path) {
+        var parser = parsers.get(path.extname(options.path));
+        options.type = parser? parser.name : null;
+    }
 
     // Replace shortcuts
-    content = _.reduce(this.shortcuts, _.partial(this._applyShortcut.bind(this), options.type), content);
+    content = this.applyShortcuts(options.type, content);
 
     return Q.nfcall(this.env.renderString.bind(this.env), content, context, options)
     .fail(function(err) {
         if (_.isString(err)) err = new Error(err);
-        err.message = err.message.replace(/^Error: /, "");
+        err.message = err.message.replace(/^Error: /, '');
 
         throw err;
     });
@@ -398,7 +424,7 @@ TemplateEngine.prototype.renderPage = function(page) {
 
     return that.book.statFile(page.path)
     .then(function(stat) {
-       var context = {
+        var context = {
             // infos about the file
             file: {
                 path: page.path,
diff --git a/test/books/conrefs/README.md b/test/books/conrefs/README.md
index 804a77a..324ee1f 100644
--- a/test/books/conrefs/README.md
+++ b/test/books/conrefs/README.md
@@ -1,5 +1,8 @@
 # Readme
 
+<p id="test-plugin-block-shortcuts-1">$$test_block1$$</p>
+<p id="test-plugin-block-shortcuts-2">{% include "./block.md" %}</p>
+
 ### Relative
 
 <p id="t1">{% include "./hello.md" %}</p>
diff --git a/test/books/conrefs/block.md b/test/books/conrefs/block.md
new file mode 100644
index 0000000..3910cb6
--- /dev/null
+++ b/test/books/conrefs/block.md
@@ -0,0 +1 @@
+$$test_block2$$
diff --git a/test/conrefs.js b/test/conrefs.js
index 4f654b0..32e4058 100644
--- a/test/conrefs.js
+++ b/test/conrefs.js
@@ -1,66 +1,66 @@
-var fs = require("fs");
-var path = require("path");
+var fs = require('fs');
+var path = require('path');
 
-describe("ConRefs", function () {
+describe('ConRefs', function () {
     var book, readme;
 
     before(function() {
-        return books.generate("conrefs", "website")
+        return books.generate('conrefs', 'website')
             .then(function(_book) {
                 book = _book;
 
                 readme = fs.readFileSync(
-                    path.join(book.options.output, "index.html"),
-                    { encoding: "utf-8" }
+                    path.join(book.options.output, 'index.html'),
+                    { encoding: 'utf-8' }
                 );
             });
     });
 
-    it("should handle local references", function() {
+    it('should handle local references', function() {
         readme.should.be.html({
-            ".page-inner p#t1": {
+            '.page-inner p#t1': {
                 count: 1,
-                text: "Hello World",
+                text: 'Hello World',
                 trim: true
             }
         });
     });
 
-    it("should handle local references with absolute paths", function() {
+    it('should handle local references with absolute paths', function() {
         readme.should.be.html({
-            ".page-inner p#t2": {
+            '.page-inner p#t2': {
                 count: 1,
-                text: "Hello World",
+                text: 'Hello World',
                 trim: true
             }
         });
     });
 
-    it("should correctly include file from git reference", function() {
+    it('should correctly include file from git reference', function() {
         readme.should.be.html({
-            ".page-inner p#t3": {
+            '.page-inner p#t3': {
                 count: 1,
-                text: "Hello from git",
+                text: 'Hello from git',
                 trim: true
             }
         });
     });
 
-    it("should correctly handle deep include in git reference", function() {
+    it('should correctly handle deep include in git reference', function() {
         readme.should.be.html({
-            ".page-inner p#t4": {
+            '.page-inner p#t4': {
                 count: 1,
-                text: "First Hello. Hello from git",
+                text: 'First Hello. Hello from git',
                 trim: true
             }
         });
     });
 
-    it("should correctly handle absolute include in git reference", function() {
+    it('should correctly handle absolute include in git reference', function() {
         readme.should.be.html({
-            ".page-inner p#t5": {
+            '.page-inner p#t5': {
                 count: 1,
-                text: "First Hello. Hello from git",
+                text: 'First Hello. Hello from git',
                 trim: true
             }
         });
diff --git a/test/helper.js b/test/helper.js
index 95619df..bbe82de 100644
--- a/test/helper.js
+++ b/test/helper.js
@@ -1,13 +1,13 @@
-var os = require("os");
-var path = require("path");
-var Q = require("q");
-var _ = require("lodash");
+var os = require('os');
+var path = require('path');
+var Q = require('q');
+var _ = require('lodash');
 
-var fsUtil = require("../lib/utils/fs");
-var Book = require("../").Book;
-var LOG_LEVELS = require("../").LOG_LEVELS;
+var fsUtil = require('../lib/utils/fs');
+var Book = require('../').Book;
+var LOG_LEVELS = require('../').LOG_LEVELS;
 
-require("./assertions");
+require('./assertions');
 
 
 var BOOKS = {};
@@ -20,7 +20,7 @@ function generateBook(bookId, test, opts) {
         prepare: function() {}
     });
 
-    return parseBook(bookId, test)
+    return parseBook(bookId, test, opts)
     .then(function(book) {
 
         return Q(opts.prepare(book))
@@ -32,21 +32,27 @@ function generateBook(bookId, test, opts) {
 }
 
 // Generate and return a book
-function parseBook(bookId, test) {
-    test = test || "website";
+function parseBook(bookId, test, opts) {
+    opts = _.defaults(opts || {}, {
+        testId: ''
+    });
+
+    test = test || 'website';
+    var testId = [test, opts.testId].join('-');
+
     BOOKS[bookId] = BOOKS[bookId] || {};
-    if (BOOKS[bookId][test]) return Q(BOOKS[bookId][test]);
+    if (BOOKS[bookId][testId]) return Q(BOOKS[bookId][testId]);
 
-    BOOKS[bookId][test] = new Book(path.resolve(__dirname, "books", bookId), {
+    BOOKS[bookId][testId] = new Book(path.resolve(__dirname, 'books', bookId), {
         logLevel: LOG_LEVELS.DISABLED,
         config: {
-            output: path.resolve(TMPDIR, bookId+"-"+test)
+            output: path.resolve(TMPDIR, bookId+'-'+testId)
         }
     });
 
-    return BOOKS[bookId][test].parse()
+    return BOOKS[bookId][testId].parse()
     .then(function() {
-        return BOOKS[bookId][test];
+        return BOOKS[bookId][testId];
     });
 }
 
diff --git a/test/plugins.js b/test/plugins.js
index db2d225..1600d0d 100644
--- a/test/plugins.js
+++ b/test/plugins.js
@@ -1,4 +1,5 @@
 var _ = require('lodash');
+var fs = require('fs');
 var should = require('should');
 var path = require('path');
 
@@ -193,15 +194,49 @@ describe('Plugins', function () {
                 });
         });
 
-        it('should correctly accept shortcuts', function() {
-            return testTpl('$$hello$$', {}, {
-                type: 'markdown'
-            })
-            .then(function(content) {
-                content.should.equal('testhellotest');
+        describe('Shortcuts', function() {
+            it('should correctly accept shortcuts', function() {
+                return testTpl('$$hello$$', {}, {
+                    type: 'markdown'
+                })
+                .then(function(content) {
+                    content.should.equal('testhellotest');
+                });
+            });
+
+            it('should correctly apply shortcuts to included file', function() {
+                return books.generate('conrefs', 'website', {
+                    testId: 'include-plugins',
+                    prepare: function(bookConref) {
+                        plugin = new Plugin(bookConref, 'blocks');
+                        plugin.load('./blocks', PLUGINS_ROOT);
+
+                        return bookConref.plugins.load(plugin);
+                    }
+                })
+                .then(function(bookConref) {
+                    var readme = fs.readFileSync(
+                        path.join(bookConref.options.output, 'index.html'),
+                        { encoding: 'utf-8' }
+                    );
+
+                    readme.should.be.html({
+                        '.page-inner p#test-plugin-block-shortcuts-1': {
+                            count: 1,
+                            text: 'testtest_block1test',
+                            trim: true
+                        },
+                        '.page-inner p#test-plugin-block-shortcuts-2': {
+                            count: 1,
+                            text: 'testtest_block2test',
+                            trim: true
+                        }
+                    });
+                });
             });
         });
 
+
         it('should correctly extend template blocks with defined end', function() {
             return testTpl('{% test2 %}hello{% endtest2end %}')
                 .then(function(content) {