From e1878050b5e0148947054275b14773cfc054e535 Mon Sep 17 00:00:00 2001 From: Stanley Stuart Date: Sat, 28 Dec 2013 18:55:36 -0600 Subject: [PATCH 01/19] add line numbers to nodes when parsing closes #691 --- lib/handlebars/compiler/ast.js | 70 ++++++++++--- spec/ast.js | 186 ++++++++++++++++++++++++++++++++- src/handlebars.yy | 52 ++++----- 3 files changed, 266 insertions(+), 42 deletions(-) diff --git a/lib/handlebars/compiler/ast.js b/lib/handlebars/compiler/ast.js index 64cd3b7d2..88d64e3a2 100644 --- a/lib/handlebars/compiler/ast.js +++ b/lib/handlebars/compiler/ast.js @@ -1,20 +1,48 @@ import Exception from "../exception"; +function LocationInfo(locInfo){ + locInfo = locInfo || {}; + this.firstLine = locInfo.first_line; + this.firstColumn = locInfo.first_column; + this.lastColumn = locInfo.last_column; + this.lastLine = locInfo.last_line; +} + var AST = { - ProgramNode: function(statements, inverseStrip, inverse) { + ProgramNode: function(statements, inverseStrip, inverse, locInfo) { + var inverseLocationInfo, firstInverseNode; + if (arguments.length === 3) { + inverse = null; + locInfo = arguments[arguments.length - 1]; + } else if (arguments.length === 2 ) { + locInfo = arguments[1]; + } + LocationInfo.call(this, locInfo); this.type = "program"; this.statements = statements; this.strip = {}; if(inverse) { - this.inverse = new AST.ProgramNode(inverse, inverseStrip); + firstInverseNode = inverse[0]; + if (firstInverseNode) { + inverseLocationInfo = { + first_line: firstInverseNode.firstLine, + last_line: firstInverseNode.lastLine, + last_column: firstInverseNode.lastColumn, + first_column: firstInverseNode.firstColumn + }; + this.inverse = new AST.ProgramNode(inverse, inverseStrip, inverseLocationInfo); + } else { + this.inverse = new AST.ProgramNode(inverse, inverseStrip); + } this.strip.right = inverseStrip.left; } else if (inverseStrip) { this.strip.left = inverseStrip.right; } }, - MustacheNode: function(rawParams, hash, open, strip) { + MustacheNode: function(rawParams, hash, open, strip, locInfo) { + LocationInfo.call(this, locInfo); this.type = "mustache"; this.hash = hash; this.strip = strip; @@ -45,19 +73,22 @@ var AST = { // pass or at runtime. }, - PartialNode: function(partialName, context, strip) { + PartialNode: function(partialName, context, strip, locInfo) { + LocationInfo.call(this, locInfo); this.type = "partial"; this.partialName = partialName; this.context = context; this.strip = strip; }, - BlockNode: function(mustache, program, inverse, close) { + BlockNode: function(mustache, program, inverse, close, locInfo) { if(mustache.id.original !== close.path.original) { throw new Exception(mustache.id.original + " doesn't match " + close.path.original); } - this.type = "block"; + LocationInfo.call(this, locInfo); + + this.type = 'block'; this.mustache = mustache; this.program = program; this.inverse = inverse; @@ -75,17 +106,20 @@ var AST = { } }, - ContentNode: function(string) { + ContentNode: function(string, locInfo) { + LocationInfo.call(this, locInfo); this.type = "content"; this.string = string; }, - HashNode: function(pairs) { + HashNode: function(pairs, locInfo) { + LocationInfo.call(this, locInfo); this.type = "hash"; this.pairs = pairs; }, - IdNode: function(parts) { + IdNode: function(parts, locInfo) { + LocationInfo.call(this, locInfo); this.type = "ID"; var original = "", @@ -116,37 +150,43 @@ var AST = { this.stringModeValue = this.string; }, - PartialNameNode: function(name) { + PartialNameNode: function(name, locInfo) { + LocationInfo.call(this, locInfo); this.type = "PARTIAL_NAME"; this.name = name.original; }, - DataNode: function(id) { + DataNode: function(id, locInfo) { + LocationInfo.call(this, locInfo); this.type = "DATA"; this.id = id; }, - StringNode: function(string) { + StringNode: function(string, locInfo) { + LocationInfo.call(this, locInfo); this.type = "STRING"; this.original = this.string = this.stringModeValue = string; }, - IntegerNode: function(integer) { + IntegerNode: function(integer, locInfo) { + LocationInfo.call(this, locInfo); this.type = "INTEGER"; this.original = this.integer = integer; this.stringModeValue = Number(integer); }, - BooleanNode: function(bool) { + BooleanNode: function(bool, locInfo) { + LocationInfo.call(this, locInfo); this.type = "BOOLEAN"; this.bool = bool; this.stringModeValue = bool === "true"; }, - CommentNode: function(comment) { + CommentNode: function(comment, locInfo) { + LocationInfo.call(this, locInfo); this.type = "comment"; this.comment = comment; } diff --git a/spec/ast.js b/spec/ast.js index ae10a3772..0feb3f511 100644 --- a/spec/ast.js +++ b/spec/ast.js @@ -4,6 +4,25 @@ describe('ast', function() { return; } + var LOCATION_INFO = { + last_line: 0, + first_line: 0, + first_column: 0, + last_column: 0 + }; + + function testLocationInfoStorage(node){ + var properties = [ 'firstLine', 'lastLine', 'firstColumn', 'lastColumn' ], + property, + propertiesLen = properties.length, + i; + + for (i = 0; i < propertiesLen; i++){ + property = properties[0]; + equals(node[property], 0); + } + } + describe('MustacheNode', function() { function testEscape(open, expected) { var mustache = new handlebarsEnv.AST.MustacheNode([{}], {}, open, false); @@ -13,7 +32,7 @@ describe('ast', function() { it('should store args', function() { var id = {isSimple: true}, hash = {}, - mustache = new handlebarsEnv.AST.MustacheNode([id, 'param1'], hash, '', false); + mustache = new handlebarsEnv.AST.MustacheNode([id, 'param1'], hash, '', false, LOCATION_INFO); equals(mustache.type, 'mustache'); equals(mustache.hash, hash); equals(mustache.escaped, true); @@ -21,6 +40,7 @@ describe('ast', function() { equals(mustache.params.length, 1); equals(mustache.params[0], 'param1'); equals(!!mustache.isHelper, true); + testLocationInfoStorage(mustache); }); it('should accept token for escape', function() { testEscape('{{', true); @@ -53,6 +73,17 @@ describe('ast', function() { new handlebarsEnv.AST.BlockNode({id: {original: 'foo'}}, {}, {}, {path: {original: 'bar'}}); }, Handlebars.Exception, "foo doesn't match bar"); }); + + it('stores location info', function(){ + var block = new handlebarsEnv.AST.BlockNode({strip: {}, id: {original: 'foo'}}, + {strip: {}}, {strip: {}}, + { + strip: {}, + path: {original: 'foo'} + }, + LOCATION_INFO); + testLocationInfoStorage(block); + }); }); describe('IdNode', function() { it('should throw on invalid path', function() { @@ -78,5 +109,158 @@ describe('ast', function() { ]); }, Handlebars.Exception, "Invalid path: foothis"); }); + + it('stores location info', function(){ + var idNode = new handlebarsEnv.AST.IdNode([], LOCATION_INFO); + testLocationInfoStorage(idNode); + }); + }); + + describe("HashNode", function(){ + + it('stores location info', function(){ + var hash = new handlebarsEnv.AST.HashNode([], LOCATION_INFO); + testLocationInfoStorage(hash); + }); + }); + + describe("ContentNode", function(){ + + it('stores location info', function(){ + var content = new handlebarsEnv.AST.ContentNode("HI", LOCATION_INFO); + testLocationInfoStorage(content); + }); + }); + + describe("CommentNode", function(){ + + it('stores location info', function(){ + var comment = new handlebarsEnv.AST.CommentNode("HI", LOCATION_INFO); + testLocationInfoStorage(comment); + }); + }); + + describe("IntegerNode", function(){ + + it('stores location info', function(){ + var integer = new handlebarsEnv.AST.IntegerNode("6", LOCATION_INFO); + testLocationInfoStorage(integer); + }); + }); + + describe("StringNode", function(){ + + it('stores location info', function(){ + var string = new handlebarsEnv.AST.StringNode("6", LOCATION_INFO); + testLocationInfoStorage(string); + }); + }); + + describe("BooleanNode", function(){ + + it('stores location info', function(){ + var bool = new handlebarsEnv.AST.BooleanNode("true", LOCATION_INFO); + testLocationInfoStorage(bool); + }); + }); + + describe("DataNode", function(){ + + it('stores location info', function(){ + var data = new handlebarsEnv.AST.DataNode("YES", LOCATION_INFO); + testLocationInfoStorage(data); + }); + }); + + describe("PartialNameNode", function(){ + + it('stores location info', function(){ + var pnn = new handlebarsEnv.AST.PartialNameNode({original: "YES"}, LOCATION_INFO); + testLocationInfoStorage(pnn); + }); + }); + + describe("PartialNode", function(){ + + it('stores location info', function(){ + var pn = new handlebarsEnv.AST.PartialNode("so_partial", {}, {}, LOCATION_INFO); + testLocationInfoStorage(pn); + }); + }); + describe("ProgramNode", function(){ + + describe("storing location info", function(){ + it("stores when `inverse` argument isn't passed", function(){ + var pn = new handlebarsEnv.AST.ProgramNode([], LOCATION_INFO); + testLocationInfoStorage(pn); + }); + + it("stores when `inverse` or `stripInverse` arguments passed", function(){ + var pn = new handlebarsEnv.AST.ProgramNode([], {strip: {}}, undefined, LOCATION_INFO); + testLocationInfoStorage(pn); + + var clone = { + strip: {}, + firstLine: 0, + lastLine: 0, + firstColumn: 0, + lastColumn: 0 + }; + var pn = new handlebarsEnv.AST.ProgramNode([], {strip: {}}, [ clone ], LOCATION_INFO); + testLocationInfoStorage(pn); + + // Assert that the newly created ProgramNode has the same location + // information as the inverse + testLocationInfoStorage(pn.inverse); + }); + }); + }); + + describe("Line Numbers", function(){ + var ast, statements; + + function testColumns(node, firstLine, lastLine, firstColumn, lastColumn){ + equals(node.firstLine, firstLine); + equals(node.lastLine, lastLine); + equals(node.firstColumn, firstColumn); + equals(node.lastColumn, lastColumn); + } + + ast = Handlebars.parse("line 1 {{line1Token}}\n line 2 {{line2token}}\n line 3 {{#blockHelperOnLine3}}\nline 4{{line4token}}\n" + + "line5{{else}}\n{{line6Token}}\n{{/blockHelperOnLine3}}"); + statements = ast.statements; + + it('gets ContentNode line numbers', function(){ + var contentNode = statements[0]; + testColumns(contentNode, 1, 1, 0, 7); + }); + + it('gets MustacheNode line numbers', function(){ + var mustacheNode = statements[1]; + testColumns(mustacheNode, 1, 1, 7, 21); + }); + + it('gets line numbers correct when newlines appear', function(){ + var secondContentNode = statements[2]; + testColumns(secondContentNode, 1, 2, 21, 8); + }); + + it('gets MustacheNode line numbers correct across newlines', function(){ + var secondMustacheNode = statements[3]; + testColumns(secondMustacheNode, 2, 2, 8, 22); + }); + + it('gets the block helper information correct', function(){ + var blockHelperNode = statements[5]; + testColumns(blockHelperNode, 3, 7, 8, 23); + }); + + it('correctly records the line numbers of an inverse of a block helper', function(){ + var blockHelperNode = statements[5], + inverse = blockHelperNode.inverse; + + testColumns(inverse, 5, 6, 13, 0); + }); }); }); + diff --git a/src/handlebars.yy b/src/handlebars.yy index 93797f5ae..63de17b08 100644 --- a/src/handlebars.yy +++ b/src/handlebars.yy @@ -16,17 +16,17 @@ function stripFlags(open, close) { %% root - : statements EOF { return new yy.ProgramNode($1); } - | EOF { return new yy.ProgramNode([]); } + : statements EOF { return new yy.ProgramNode($1, @$); } + | EOF { return new yy.ProgramNode([], @$); } ; program - : simpleInverse statements -> new yy.ProgramNode([], $1, $2) - | statements simpleInverse statements -> new yy.ProgramNode($1, $2, $3) - | statements simpleInverse -> new yy.ProgramNode($1, $2, []) - | statements -> new yy.ProgramNode($1) - | simpleInverse -> new yy.ProgramNode([]) - | "" -> new yy.ProgramNode([]) + : simpleInverse statements -> new yy.ProgramNode([], $1, $2, @$) + | statements simpleInverse statements -> new yy.ProgramNode($1, $2, $3, @$) + | statements simpleInverse -> new yy.ProgramNode($1, $2, [], @$) + | statements -> new yy.ProgramNode($1, @$) + | simpleInverse -> new yy.ProgramNode([], @$) + | "" -> new yy.ProgramNode([], @$) ; statements @@ -35,20 +35,20 @@ statements ; statement - : openInverse program closeBlock -> new yy.BlockNode($1, $2.inverse, $2, $3) - | openBlock program closeBlock -> new yy.BlockNode($1, $2, $2.inverse, $3) + : openInverse program closeBlock -> new yy.BlockNode($1, $2.inverse, $2, $3, @$) + | openBlock program closeBlock -> new yy.BlockNode($1, $2, $2.inverse, $3, @$) | mustache -> $1 | partial -> $1 - | CONTENT -> new yy.ContentNode($1) - | COMMENT -> new yy.CommentNode($1) + | CONTENT -> new yy.ContentNode($1, @$) + | COMMENT -> new yy.CommentNode($1, @$) ; openBlock - : OPEN_BLOCK inMustache CLOSE -> new yy.MustacheNode($2[0], $2[1], $1, stripFlags($1, $3)) + : OPEN_BLOCK inMustache CLOSE -> new yy.MustacheNode($2[0], $2[1], $1, stripFlags($1, $3), @$) ; openInverse - : OPEN_INVERSE inMustache CLOSE -> new yy.MustacheNode($2[0], $2[1], $1, stripFlags($1, $3)) + : OPEN_INVERSE inMustache CLOSE -> new yy.MustacheNode($2[0], $2[1], $1, stripFlags($1, $3), @$) ; closeBlock @@ -58,13 +58,13 @@ closeBlock mustache // Parsing out the '&' escape token at AST level saves ~500 bytes after min due to the removal of one parser node. // This also allows for handler unification as all mustache node instances can utilize the same handler - : OPEN inMustache CLOSE -> new yy.MustacheNode($2[0], $2[1], $1, stripFlags($1, $3)) - | OPEN_UNESCAPED inMustache CLOSE_UNESCAPED -> new yy.MustacheNode($2[0], $2[1], $1, stripFlags($1, $3)) + : OPEN inMustache CLOSE -> new yy.MustacheNode($2[0], $2[1], $1, stripFlags($1, $3), @$) + | OPEN_UNESCAPED inMustache CLOSE_UNESCAPED -> new yy.MustacheNode($2[0], $2[1], $1, stripFlags($1, $3), @$) ; partial - : OPEN_PARTIAL partialName path? CLOSE -> new yy.PartialNode($2, $3, stripFlags($1, $4)) + : OPEN_PARTIAL partialName path? CLOSE -> new yy.PartialNode($2, $3, stripFlags($1, $4), @$) ; simpleInverse @@ -78,14 +78,14 @@ inMustache param : path -> $1 - | STRING -> new yy.StringNode($1) - | INTEGER -> new yy.IntegerNode($1) - | BOOLEAN -> new yy.BooleanNode($1) + | STRING -> new yy.StringNode($1, @$) + | INTEGER -> new yy.IntegerNode($1, @$) + | BOOLEAN -> new yy.BooleanNode($1, @$) | dataName -> $1 ; hash - : hashSegment+ -> new yy.HashNode($1) + : hashSegment+ -> new yy.HashNode($1, @$) ; hashSegment @@ -93,17 +93,17 @@ hashSegment ; partialName - : path -> new yy.PartialNameNode($1) - | STRING -> new yy.PartialNameNode(new yy.StringNode($1)) - | INTEGER -> new yy.PartialNameNode(new yy.IntegerNode($1)) + : path -> new yy.PartialNameNode($1, @$) + | STRING -> new yy.PartialNameNode(new yy.StringNode($1, @$), @$) + | INTEGER -> new yy.PartialNameNode(new yy.IntegerNode($1, @$)) ; dataName - : DATA path -> new yy.DataNode($2) + : DATA path -> new yy.DataNode($2, @$) ; path - : pathSegments -> new yy.IdNode($1) + : pathSegments -> new yy.IdNode($1, @$) ; pathSegments From 2f0c96b61873ce3b729c2d13ff8ae719608c1d40 Mon Sep 17 00:00:00 2001 From: Blake Embrey Date: Sat, 28 Dec 2013 21:35:40 -0500 Subject: [PATCH 02/19] Make the environment reusable. --- lib/handlebars.js | 4 +++- lib/handlebars/compiler/compiler.js | 21 +++++++++------------ spec/env/runtime.js | 16 +++++++++++++++- 3 files changed, 27 insertions(+), 14 deletions(-) diff --git a/lib/handlebars.js b/lib/handlebars.js index c8956b401..ffa9c7ab2 100644 --- a/lib/handlebars.js +++ b/lib/handlebars.js @@ -14,7 +14,9 @@ var create = function() { hb.compile = function(input, options) { return compile(input, options, hb); }; - hb.precompile = precompile; + hb.precompile = function (input, options) { + return precompile(input, options, hb); + }; hb.AST = AST; hb.Compiler = Compiler; diff --git a/lib/handlebars/compiler/compiler.js b/lib/handlebars/compiler/compiler.js index daea3072c..259c543a5 100644 --- a/lib/handlebars/compiler/compiler.js +++ b/lib/handlebars/compiler/compiler.js @@ -1,7 +1,4 @@ import Exception from "../exception"; -import { parse } from "./base"; -import JavaScriptCompiler from "./javascript-compiler"; -import AST from "./ast"; export function Compiler() {} @@ -423,8 +420,8 @@ Compiler.prototype = { } }; -export function precompile(input, options) { - if (input == null || (typeof input !== 'string' && input.constructor !== AST.ProgramNode)) { +export function precompile(input, options, env) { + if (input == null || (typeof input !== 'string' && input.constructor !== env.AST.ProgramNode)) { throw new Exception("You must pass a string or Handlebars AST to Handlebars.precompile. You passed " + input); } @@ -433,13 +430,13 @@ export function precompile(input, options) { options.data = true; } - var ast = parse(input); - var environment = new Compiler().compile(ast, options); - return new JavaScriptCompiler().compile(environment, options); + var ast = env.parse(input); + var environment = new env.Compiler().compile(ast, options); + return new env.JavaScriptCompiler().compile(environment, options); } export function compile(input, options, env) { - if (input == null || (typeof input !== 'string' && input.constructor !== AST.ProgramNode)) { + if (input == null || (typeof input !== 'string' && input.constructor !== env.AST.ProgramNode)) { throw new Exception("You must pass a string or Handlebars AST to Handlebars.compile. You passed " + input); } @@ -452,9 +449,9 @@ export function compile(input, options, env) { var compiled; function compileInput() { - var ast = parse(input); - var environment = new Compiler().compile(ast, options); - var templateSpec = new JavaScriptCompiler().compile(environment, options, undefined, true); + var ast = env.parse(input); + var environment = new env.Compiler().compile(ast, options); + var templateSpec = new env.JavaScriptCompiler().compile(environment, options, undefined, true); return env.template(templateSpec); } diff --git a/spec/env/runtime.js b/spec/env/runtime.js index 68e40b058..e73d11115 100644 --- a/spec/env/runtime.js +++ b/spec/env/runtime.js @@ -8,11 +8,21 @@ var _ = require('underscore'), global.Handlebars = undefined; vm.runInThisContext(fs.readFileSync(__dirname + '/../../dist/handlebars.runtime.js'), 'dist/handlebars.runtime.js'); +var parse = require('../../dist/cjs/handlebars/compiler/base').parse; var compiler = require('../../dist/cjs/handlebars/compiler/compiler'); +var JavaScriptCompiler = require('../../dist/cjs/handlebars/compiler/javascript-compiler')['default']; global.CompilerContext = { compile: function(template, options) { - var templateSpec = compiler.precompile(template, options); + // Hack the compiler on to the environment for these specific tests + handlebarsEnv.precompile = function(template, options) { + return compiler.precompile(template, options, handlebarsEnv); + }; + handlebarsEnv.parse = parse; + handlebarsEnv.Compiler = compiler.Compiler; + handlebarsEnv.JavaScriptCompiler = JavaScriptCompiler; + + var templateSpec = handlebarsEnv.precompile(template, options); return handlebarsEnv.template(safeEval(templateSpec)); }, compileWithPartial: function(template, options) { @@ -20,6 +30,10 @@ global.CompilerContext = { handlebarsEnv.compile = function(template, options) { return compiler.compile(template, options, handlebarsEnv); }; + handlebarsEnv.parse = parse; + handlebarsEnv.Compiler = compiler.Compiler; + handlebarsEnv.JavaScriptCompiler = JavaScriptCompiler; + return handlebarsEnv.compile(template, options); } }; From 150e55aa009e3af02f585e921213bfcff9999427 Mon Sep 17 00:00:00 2001 From: Blake Embrey Date: Sat, 28 Dec 2013 22:31:45 -0500 Subject: [PATCH 03/19] Pull options out from param setup to allow easier extension. --- lib/handlebars/compiler/javascript-compiler.js | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/lib/handlebars/compiler/javascript-compiler.js b/lib/handlebars/compiler/javascript-compiler.js index 159a38bfc..607b03013 100644 --- a/lib/handlebars/compiler/javascript-compiler.js +++ b/lib/handlebars/compiler/javascript-compiler.js @@ -798,9 +798,7 @@ JavaScriptCompiler.prototype = { }; }, - // the params and contexts arguments are passed in arrays - // to fill in - setupParams: function(paramSize, params, useRegister) { + setupOptions: function(paramSize, params) { var options = [], contexts = [], types = [], param, inverse, program; options.push("hash:" + this.popStack()); @@ -846,14 +844,21 @@ JavaScriptCompiler.prototype = { options.push("data:data"); } - options = "{" + options.join(",") + "}"; + return options; + }, + + // the params and contexts arguments are passed in arrays + // to fill in + setupParams: function(paramSize, params, useRegister) { + var options = '{' + this.setupOptions(paramSize, params).join(',') + '}'; + if (useRegister) { this.register('options', options); params.push('options'); } else { params.push(options); } - return params.join(", "); + return params.join(', '); } }; From 14d1d4270f63957d4ac743b00afa178a26350cd1 Mon Sep 17 00:00:00 2001 From: kpdecker Date: Sun, 29 Dec 2013 17:16:59 -0600 Subject: [PATCH 04/19] Fix ProgramNode parameter handling under IE --- lib/handlebars/compiler/ast.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/handlebars/compiler/ast.js b/lib/handlebars/compiler/ast.js index 88d64e3a2..b6c3a0350 100644 --- a/lib/handlebars/compiler/ast.js +++ b/lib/handlebars/compiler/ast.js @@ -12,11 +12,13 @@ var AST = { ProgramNode: function(statements, inverseStrip, inverse, locInfo) { var inverseLocationInfo, firstInverseNode; if (arguments.length === 3) { + locInfo = inverse; inverse = null; - locInfo = arguments[arguments.length - 1]; - } else if (arguments.length === 2 ) { - locInfo = arguments[1]; + } else if (arguments.length === 2) { + locInfo = inverseStrip; + inverseStrip = null; } + LocationInfo.call(this, locInfo); this.type = "program"; this.statements = statements; From b09333db7946d20ba7dbc6d32d5496ab8295b8e1 Mon Sep 17 00:00:00 2001 From: machty Date: Tue, 24 Dec 2013 11:01:23 -0500 Subject: [PATCH 05/19] Added support for subexpressions Handlebars now supports subexpressions. {{foo (bar 3)}} Subexpressions are always evaluated as helpers; if `3` were omitted from the above example, `bar` would be invoked as a param-less helper, even though a top-levell `{{bar}}` would be considered ambiguous. The return value of a subexpression helper is passed in as a parameter of a parent subexpression helper, even in string params mode. Their type, as listed in `options.types` or `options.hashTypes` in string params mode, is "sexpr". The main conceptual change in the Handlebars code is that there is a new AST.SexprNode that manages the params/hash passed into a mustache, as well as the logic that governs whether that mustache is a helper invocation, property lookup, etc. MustacheNode, which used to manage this stuff, still exists, but only manages things like white-space stripping and whether the mustache is escaped or not. So, a MustacheNode _has_ a SexprNode. The introduction of subexpressions is fully backwards compatible, but a few things needed to change about the compiled output of a template in order to support subexpressions. The main one is that the options hash is no longer stashed in a local `options` var before being passed to either the helper being invoked or the `helperMissing` fallback. Rather, the options object is inlined in these cases. This does mean compiled template sizes will be a little bit larger, even those that don't make use of subexpressions, but shouldn't have any noticeable impact on performance when actually rendering templates as only one of these inlined objects will actually get evaluated. --- lib/handlebars/compiler/ast.js | 24 ++- lib/handlebars/compiler/compiler.js | 90 +++++----- .../compiler/javascript-compiler.js | 42 +++-- lib/handlebars/compiler/printer.js | 12 +- spec/ast.js | 14 +- spec/string-params.js | 16 ++ spec/subexpressions.js | 166 ++++++++++++++++++ spec/tokenizer.js | 27 +++ src/handlebars.l | 7 +- src/handlebars.yy | 16 +- 10 files changed, 329 insertions(+), 85 deletions(-) create mode 100644 spec/subexpressions.js diff --git a/lib/handlebars/compiler/ast.js b/lib/handlebars/compiler/ast.js index b6c3a0350..ce5ee1104 100644 --- a/lib/handlebars/compiler/ast.js +++ b/lib/handlebars/compiler/ast.js @@ -46,7 +46,6 @@ var AST = { MustacheNode: function(rawParams, hash, open, strip, locInfo) { LocationInfo.call(this, locInfo); this.type = "mustache"; - this.hash = hash; this.strip = strip; // Open may be a string parsed from the parser or a passed boolean flag @@ -58,6 +57,25 @@ var AST = { this.escaped = !!open; } + if (rawParams instanceof AST.SexprNode) { + this.sexpr = rawParams; + } else { + // Support old AST API + this.sexpr = new AST.SexprNode(rawParams, hash); + } + + // Support old AST API that stored this info in MustacheNode + this.id = this.sexpr.id; + this.params = this.sexpr.params; + this.hash = this.sexpr.hash; + this.eligibleHelper = this.sexpr.eligibleHelper; + this.isHelper = this.sexpr.isHelper; + }, + + SexprNode: function(rawParams, hash) { + this.type = "sexpr"; + this.hash = hash; + var id = this.id = rawParams[0]; var params = this.params = rawParams.slice(1); @@ -84,8 +102,8 @@ var AST = { }, BlockNode: function(mustache, program, inverse, close, locInfo) { - if(mustache.id.original !== close.path.original) { - throw new Exception(mustache.id.original + " doesn't match " + close.path.original); + if(mustache.sexpr.id.original !== close.path.original) { + throw new Exception(mustache.sexpr.id.original + " doesn't match " + close.path.original); } LocationInfo.call(this, locInfo); diff --git a/lib/handlebars/compiler/compiler.js b/lib/handlebars/compiler/compiler.js index 259c543a5..00353a8f5 100644 --- a/lib/handlebars/compiler/compiler.js +++ b/lib/handlebars/compiler/compiler.js @@ -156,12 +156,13 @@ Compiler.prototype = { inverse = this.compileProgram(inverse); } - var type = this.classifyMustache(mustache); + var sexpr = mustache.sexpr; + var type = this.classifySexpr(sexpr); if (type === "helper") { - this.helperMustache(mustache, program, inverse); + this.helperSexpr(sexpr, program, inverse); } else if (type === "simple") { - this.simpleMustache(mustache); + this.simpleSexpr(sexpr); // now that the simple mustache is resolved, we need to // evaluate it by executing `blockHelperMissing` @@ -170,7 +171,7 @@ Compiler.prototype = { this.opcode('emptyHash'); this.opcode('blockValue'); } else { - this.ambiguousMustache(mustache, program, inverse); + this.ambiguousSexpr(sexpr, program, inverse); // now that the simple mustache is resolved, we need to // evaluate it by executing `blockHelperMissing` @@ -198,6 +199,12 @@ Compiler.prototype = { } this.opcode('getContext', val.depth || 0); this.opcode('pushStringParam', val.stringModeValue, val.type); + + if (val.type === 'sexpr') { + // Subexpressions get evaluated and passed in + // in string params mode. + this.sexpr(val); + } } else { this.accept(val); } @@ -226,26 +233,17 @@ Compiler.prototype = { }, mustache: function(mustache) { - var options = this.options; - var type = this.classifyMustache(mustache); - - if (type === "simple") { - this.simpleMustache(mustache); - } else if (type === "helper") { - this.helperMustache(mustache); - } else { - this.ambiguousMustache(mustache); - } + this.sexpr(mustache.sexpr); - if(mustache.escaped && !options.noEscape) { + if(mustache.escaped && !this.options.noEscape) { this.opcode('appendEscaped'); } else { this.opcode('append'); } }, - ambiguousMustache: function(mustache, program, inverse) { - var id = mustache.id, + ambiguousSexpr: function(sexpr, program, inverse) { + var id = sexpr.id, name = id.parts[0], isBlock = program != null || inverse != null; @@ -257,8 +255,8 @@ Compiler.prototype = { this.opcode('invokeAmbiguous', name, isBlock); }, - simpleMustache: function(mustache) { - var id = mustache.id; + simpleSexpr: function(sexpr) { + var id = sexpr.id; if (id.type === 'DATA') { this.DATA(id); @@ -274,9 +272,9 @@ Compiler.prototype = { this.opcode('resolvePossibleLambda'); }, - helperMustache: function(mustache, program, inverse) { - var params = this.setupFullMustacheParams(mustache, program, inverse), - name = mustache.id.parts[0]; + helperSexpr: function(sexpr, program, inverse) { + var params = this.setupFullMustacheParams(sexpr, program, inverse), + name = sexpr.id.parts[0]; if (this.options.knownHelpers[name]) { this.opcode('invokeKnownHelper', params.length, name); @@ -287,6 +285,18 @@ Compiler.prototype = { } }, + sexpr: function(sexpr) { + var type = this.classifySexpr(sexpr); + + if (type === "simple") { + this.simpleSexpr(sexpr); + } else if (type === "helper") { + this.helperSexpr(sexpr); + } else { + this.ambiguousSexpr(sexpr); + } + }, + ID: function(id) { this.addDepth(id.depth); this.opcode('getContext', id.depth); @@ -349,14 +359,14 @@ Compiler.prototype = { } }, - classifyMustache: function(mustache) { - var isHelper = mustache.isHelper; - var isEligible = mustache.eligibleHelper; + classifySexpr: function(sexpr) { + var isHelper = sexpr.isHelper; + var isEligible = sexpr.eligibleHelper; var options = this.options; // if ambiguous, we can possibly resolve the ambiguity now if (isEligible && !isHelper) { - var name = mustache.id.parts[0]; + var name = sexpr.id.parts[0]; if (options.knownHelpers[name]) { isHelper = true; @@ -383,35 +393,27 @@ Compiler.prototype = { this.opcode('getContext', param.depth || 0); this.opcode('pushStringParam', param.stringModeValue, param.type); + + if (param.type === 'sexpr') { + // Subexpressions get evaluated and passed in + // in string params mode. + this.sexpr(param); + } } else { this[param.type](param); } } }, - setupMustacheParams: function(mustache) { - var params = mustache.params; - this.pushParams(params); - - if(mustache.hash) { - this.hash(mustache.hash); - } else { - this.opcode('emptyHash'); - } - - return params; - }, - - // this will replace setupMustacheParams when we're done - setupFullMustacheParams: function(mustache, program, inverse) { - var params = mustache.params; + setupFullMustacheParams: function(sexpr, program, inverse) { + var params = sexpr.params; this.pushParams(params); this.opcode('pushProgram', program); this.opcode('pushProgram', inverse); - if(mustache.hash) { - this.hash(mustache.hash); + if (sexpr.hash) { + this.hash(sexpr.hash); } else { this.opcode('emptyHash'); } diff --git a/lib/handlebars/compiler/javascript-compiler.js b/lib/handlebars/compiler/javascript-compiler.js index 159a38bfc..d920d52c7 100644 --- a/lib/handlebars/compiler/javascript-compiler.js +++ b/lib/handlebars/compiler/javascript-compiler.js @@ -244,9 +244,6 @@ JavaScriptCompiler.prototype = { var current = this.topStack(); params.splice(1, 0, current); - // Use the options value generated from the invocation - params[params.length-1] = 'options'; - this.pushSource("if (!" + this.lastHelper + ") { " + current + " = blockHelperMissing.call(" + params.join(", ") + "); }"); }, @@ -398,10 +395,14 @@ JavaScriptCompiler.prototype = { this.pushString(type); - if (typeof string === 'string') { - this.pushString(string); - } else { - this.pushStackLiteral(string); + // If it's a subexpression, the string result + // will be pushed after this opcode. + if (type !== 'sexpr') { + if (typeof string === 'string') { + this.pushString(string); + } else { + this.pushStackLiteral(string); + } } }, @@ -409,8 +410,8 @@ JavaScriptCompiler.prototype = { this.pushStackLiteral('{}'); if (this.options.stringParams) { - this.register('hashTypes', '{}'); - this.register('hashContexts', '{}'); + this.push('{}'); // hashContexts + this.push('{}'); // hashTypes } }, pushHash: function() { @@ -421,9 +422,10 @@ JavaScriptCompiler.prototype = { this.hash = undefined; if (this.options.stringParams) { - this.register('hashContexts', '{' + hash.contexts.join(',') + '}'); - this.register('hashTypes', '{' + hash.types.join(',') + '}'); + this.push('{' + hash.contexts.join(',') + '}'); + this.push('{' + hash.types.join(',') + '}'); } + this.push('{\n ' + hash.values.join(',\n ') + '\n }'); }, @@ -526,7 +528,7 @@ JavaScriptCompiler.prototype = { invokeAmbiguous: function(name, helperCall) { this.context.aliases.functionType = '"function"'; - this.pushStackLiteral('{}'); // Hash value + this.emptyHash(); var helper = this.setupHelper(0, name, helperCall); var helperName = this.lastHelper = this.nameLookup('helpers', name, 'helper'); @@ -805,6 +807,11 @@ JavaScriptCompiler.prototype = { options.push("hash:" + this.popStack()); + if (this.options.stringParams) { + options.push("hashTypes:" + this.popStack()); + options.push("hashContexts:" + this.popStack()); + } + inverse = this.popStack(); program = this.popStack(); @@ -838,22 +845,13 @@ JavaScriptCompiler.prototype = { if (this.options.stringParams) { options.push("contexts:[" + contexts.join(",") + "]"); options.push("types:[" + types.join(",") + "]"); - options.push("hashContexts:hashContexts"); - options.push("hashTypes:hashTypes"); } if(this.options.data) { options.push("data:data"); } - options = "{" + options.join(",") + "}"; - if (useRegister) { - this.register('options', options); - params.push('options'); - } else { - params.push(options); - } - return params.join(", "); + params.push("{" + options.join(",") + "}"); } }; diff --git a/lib/handlebars/compiler/printer.js b/lib/handlebars/compiler/printer.js index f91ff0216..ad55c7d4a 100644 --- a/lib/handlebars/compiler/printer.js +++ b/lib/handlebars/compiler/printer.js @@ -62,8 +62,8 @@ PrintVisitor.prototype.block = function(block) { return out; }; -PrintVisitor.prototype.mustache = function(mustache) { - var params = mustache.params, paramStrings = [], hash; +PrintVisitor.prototype.sexpr = function(sexpr) { + var params = sexpr.params, paramStrings = [], hash; for(var i=0, l=params.length; i@\[-\^`\{-~]+/{LOOKAHEAD} [\s\S]*?"--}}" strip(0,4); this.popState(); return 'COMMENT'; +"(" return 'OPEN_SEXPR'; +")" return 'CLOSE_SEXPR'; + "{{"{LEFT_STRIP}?">" return 'OPEN_PARTIAL'; "{{"{LEFT_STRIP}?"#" return 'OPEN_BLOCK'; "{{"{LEFT_STRIP}?"/" return 'OPEN_ENDBLOCK'; diff --git a/src/handlebars.yy b/src/handlebars.yy index 63de17b08..319b8efbf 100644 --- a/src/handlebars.yy +++ b/src/handlebars.yy @@ -44,11 +44,11 @@ statement ; openBlock - : OPEN_BLOCK inMustache CLOSE -> new yy.MustacheNode($2[0], $2[1], $1, stripFlags($1, $3), @$) + : OPEN_BLOCK sexpr CLOSE -> new yy.MustacheNode($2, null, $1, stripFlags($1, $3), @$) ; openInverse - : OPEN_INVERSE inMustache CLOSE -> new yy.MustacheNode($2[0], $2[1], $1, stripFlags($1, $3), @$) + : OPEN_INVERSE sexpr CLOSE -> new yy.MustacheNode($2, null, $1, stripFlags($1, $3), @$) ; closeBlock @@ -58,11 +58,10 @@ closeBlock mustache // Parsing out the '&' escape token at AST level saves ~500 bytes after min due to the removal of one parser node. // This also allows for handler unification as all mustache node instances can utilize the same handler - : OPEN inMustache CLOSE -> new yy.MustacheNode($2[0], $2[1], $1, stripFlags($1, $3), @$) - | OPEN_UNESCAPED inMustache CLOSE_UNESCAPED -> new yy.MustacheNode($2[0], $2[1], $1, stripFlags($1, $3), @$) + : OPEN sexpr CLOSE -> new yy.MustacheNode($2, null, $1, stripFlags($1, $3), @$) + | OPEN_UNESCAPED sexpr CLOSE_UNESCAPED -> new yy.MustacheNode($2, null, $1, stripFlags($1, $3), @$) ; - partial : OPEN_PARTIAL partialName path? CLOSE -> new yy.PartialNode($2, $3, stripFlags($1, $4), @$) ; @@ -71,9 +70,9 @@ simpleInverse : OPEN_INVERSE CLOSE -> stripFlags($1, $2) ; -inMustache - : path param* hash? -> [[$1].concat($2), $3] - | dataName -> [[$1], null] +sexpr + : path param* hash? -> new yy.SexprNode([$1].concat($2), $3) + | dataName -> new yy.SexprNode([$1], null) ; param @@ -82,6 +81,7 @@ param | INTEGER -> new yy.IntegerNode($1, @$) | BOOLEAN -> new yy.BooleanNode($1, @$) | dataName -> $1 + | OPEN_SEXPR sexpr CLOSE_SEXPR {$2.isHelper = true; $$ = $2;} ; hash From f2df220a1f8ecbf2f07ef5b9ba9a317a1d3d6dbc Mon Sep 17 00:00:00 2001 From: kpdecker Date: Tue, 31 Dec 2013 13:31:47 -0600 Subject: [PATCH 06/19] Add location tracking to sexpr --- lib/handlebars/compiler/ast.js | 4 +++- src/handlebars.yy | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/handlebars/compiler/ast.js b/lib/handlebars/compiler/ast.js index ce5ee1104..6ccd7f15b 100644 --- a/lib/handlebars/compiler/ast.js +++ b/lib/handlebars/compiler/ast.js @@ -72,7 +72,9 @@ var AST = { this.isHelper = this.sexpr.isHelper; }, - SexprNode: function(rawParams, hash) { + SexprNode: function(rawParams, hash, locInfo) { + LocationInfo.call(this, locInfo); + this.type = "sexpr"; this.hash = hash; diff --git a/src/handlebars.yy b/src/handlebars.yy index 319b8efbf..7bff5125a 100644 --- a/src/handlebars.yy +++ b/src/handlebars.yy @@ -71,8 +71,8 @@ simpleInverse ; sexpr - : path param* hash? -> new yy.SexprNode([$1].concat($2), $3) - | dataName -> new yy.SexprNode([$1], null) + : path param* hash? -> new yy.SexprNode([$1].concat($2), $3, @$) + | dataName -> new yy.SexprNode([$1], null, @$) ; param From 641358a905cd8103e7e1f9c2d343dc244a1309ef Mon Sep 17 00:00:00 2001 From: Nate Irwin Date: Tue, 31 Dec 2013 15:09:41 -0700 Subject: [PATCH 07/19] Fix for reserved keyword "default" --- lib/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/index.js b/lib/index.js index ca51d3a49..e150524bd 100644 --- a/lib/index.js +++ b/lib/index.js @@ -3,9 +3,9 @@ // var local = handlebars.create(); -var handlebars = require('../dist/cjs/handlebars').default; +var handlebars = require('../dist/cjs/handlebars')["default"]; -handlebars.Visitor = require('../dist/cjs/handlebars/compiler/visitor').default; +handlebars.Visitor = require('../dist/cjs/handlebars/compiler/visitor')["default"]; var printer = require('../dist/cjs/handlebars/compiler/printer'); handlebars.PrintVisitor = printer.PrintVisitor; From af3358d1950317db17737baddffdd39895cdb320 Mon Sep 17 00:00:00 2001 From: kpdecker Date: Tue, 31 Dec 2013 16:10:51 -0600 Subject: [PATCH 08/19] Fix multiple hash handling in subexpressions --- lib/handlebars/compiler/javascript-compiler.js | 6 +++++- spec/subexpressions.js | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/handlebars/compiler/javascript-compiler.js b/lib/handlebars/compiler/javascript-compiler.js index d920d52c7..7fbddf77c 100644 --- a/lib/handlebars/compiler/javascript-compiler.js +++ b/lib/handlebars/compiler/javascript-compiler.js @@ -76,6 +76,7 @@ JavaScriptCompiler.prototype = { this.stackSlot = 0; this.stackVars = []; this.registers = { list: [] }; + this.hashes = []; this.compileStack = []; this.inlineStack = []; @@ -415,11 +416,14 @@ JavaScriptCompiler.prototype = { } }, pushHash: function() { + if (this.hash) { + this.hashes.push(this.hash); + } this.hash = {values: [], types: [], contexts: []}; }, popHash: function() { var hash = this.hash; - this.hash = undefined; + this.hash = this.hashes.pop(); if (this.options.stringParams) { this.push('{' + hash.contexts.join(',') + '}'); diff --git a/spec/subexpressions.js b/spec/subexpressions.js index 9c06f9453..e8ade63ab 100644 --- a/spec/subexpressions.js +++ b/spec/subexpressions.js @@ -76,7 +76,7 @@ describe('subexpressions', function() { }); it("as hashes", function() { - var string = '{{blog fun=(equal true true)}}'; + var string = '{{blog fun=(equal (blog fun=1) "val is 1")}}'; var helpers = { blog: function(options) { From cbfec5b9e9e7e2459c36fbfce599a3d5c082c61e Mon Sep 17 00:00:00 2001 From: kpdecker Date: Tue, 31 Dec 2013 20:25:46 -0600 Subject: [PATCH 09/19] Set laxbreak jshint flag --- .jshintrc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.jshintrc b/.jshintrc index ecd80acfc..bb29c76e8 100644 --- a/.jshintrc +++ b/.jshintrc @@ -35,7 +35,7 @@ "evil": true, "forin": false, "immed": false, - "laxbreak": false, + "laxbreak": true, "newcap": true, "noarg": true, "noempty": false, From f4d337d252c8f1f390cd0bdebbf1f57497591721 Mon Sep 17 00:00:00 2001 From: kpdecker Date: Tue, 31 Dec 2013 21:17:16 -0600 Subject: [PATCH 10/19] Fix incorrect stack pop when replacing literals --- lib/handlebars/compiler/javascript-compiler.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/handlebars/compiler/javascript-compiler.js b/lib/handlebars/compiler/javascript-compiler.js index 7fbddf77c..22937f629 100644 --- a/lib/handlebars/compiler/javascript-compiler.js +++ b/lib/handlebars/compiler/javascript-compiler.js @@ -687,7 +687,8 @@ JavaScriptCompiler.prototype = { replaceStack: function(callback) { var prefix = '', inline = this.isInline(), - stack; + stack, + usedLiteral; // If we are currently inline then we want to merge the inline statement into the // replacement statement via ',' @@ -697,6 +698,7 @@ JavaScriptCompiler.prototype = { if (top instanceof Literal) { // Literals do not need to be inlined stack = top.value; + usedLiteral = true; } else { // Get or create the current stack name for use by the inline var name = this.stackSlot ? this.topStackName() : this.incrStack(); @@ -711,7 +713,7 @@ JavaScriptCompiler.prototype = { var item = callback.call(this, stack); if (inline) { - if (this.inlineStack.length || this.compileStack.length) { + if (!usedLiteral) { this.popStack(); } this.push('(' + prefix + item + ')'); From ddfe457abfe85b14ba4c95c3a9f434d04e02ec23 Mon Sep 17 00:00:00 2001 From: kpdecker Date: Tue, 31 Dec 2013 21:17:50 -0600 Subject: [PATCH 11/19] Fix stack id "leak" on replaceStack --- lib/handlebars/compiler/javascript-compiler.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/handlebars/compiler/javascript-compiler.js b/lib/handlebars/compiler/javascript-compiler.js index 22937f629..7ad400c43 100644 --- a/lib/handlebars/compiler/javascript-compiler.js +++ b/lib/handlebars/compiler/javascript-compiler.js @@ -688,6 +688,7 @@ JavaScriptCompiler.prototype = { var prefix = '', inline = this.isInline(), stack, + createdStack, usedLiteral; // If we are currently inline then we want to merge the inline statement into the @@ -701,7 +702,8 @@ JavaScriptCompiler.prototype = { usedLiteral = true; } else { // Get or create the current stack name for use by the inline - var name = this.stackSlot ? this.topStackName() : this.incrStack(); + createdStack = !this.stackSlot; + var name = !createdStack ? this.topStackName() : this.incrStack(); prefix = '(' + this.push(name) + ' = ' + top + '),'; stack = this.topStack(); @@ -716,6 +718,9 @@ JavaScriptCompiler.prototype = { if (!usedLiteral) { this.popStack(); } + if (createdStack) { + this.stackSlot--; + } this.push('(' + prefix + item + ')'); } else { // Prevent modification of the context depth variable. Through replaceStack From cd885bf855e914d27a511273f548ab01b5fd3b7e Mon Sep 17 00:00:00 2001 From: kpdecker Date: Tue, 31 Dec 2013 21:18:42 -0600 Subject: [PATCH 12/19] Add stack handling sanity checks --- lib/handlebars/compiler/javascript-compiler.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/handlebars/compiler/javascript-compiler.js b/lib/handlebars/compiler/javascript-compiler.js index 7ad400c43..526ddc1c5 100644 --- a/lib/handlebars/compiler/javascript-compiler.js +++ b/lib/handlebars/compiler/javascript-compiler.js @@ -1,4 +1,5 @@ import { COMPILER_REVISION, REVISION_CHANGES, log } from "../base"; +import Exception from "../exception"; function Literal(value) { this.value = value; @@ -104,6 +105,10 @@ JavaScriptCompiler.prototype = { // Flush any trailing content that might be pending. this.pushSource(''); + if (this.stackSlot || this.inlineStack.length || this.compileStack.length) { + throw new Exception('Compile completed with content left on stack'); + } + return this.createFunctionContext(asObject); }, @@ -771,6 +776,9 @@ JavaScriptCompiler.prototype = { return item.value; } else { if (!inline) { + if (!this.stackSlot) { + throw new Exception('Invalid stack pop'); + } this.stackSlot--; } return item; From c1a93d33e78a2e498f3f57b1189193f69671d0dc Mon Sep 17 00:00:00 2001 From: kpdecker Date: Tue, 31 Dec 2013 21:19:00 -0600 Subject: [PATCH 13/19] Use literal for data lookup --- lib/handlebars/compiler/javascript-compiler.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/handlebars/compiler/javascript-compiler.js b/lib/handlebars/compiler/javascript-compiler.js index 526ddc1c5..bc9636c52 100644 --- a/lib/handlebars/compiler/javascript-compiler.js +++ b/lib/handlebars/compiler/javascript-compiler.js @@ -385,7 +385,7 @@ JavaScriptCompiler.prototype = { // // Push the data lookup operator lookupData: function() { - this.push('data'); + this.pushStackLiteral('data'); }, // [pushStringParam] From e7e94dc69710ee6b4e77c4c992b54f29ef42b713 Mon Sep 17 00:00:00 2001 From: kpdecker Date: Tue, 31 Dec 2013 21:20:59 -0600 Subject: [PATCH 14/19] Whitespace cleanup --- lib/handlebars/compiler/javascript-compiler.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/handlebars/compiler/javascript-compiler.js b/lib/handlebars/compiler/javascript-compiler.js index bc9636c52..dc77ef6df 100644 --- a/lib/handlebars/compiler/javascript-compiler.js +++ b/lib/handlebars/compiler/javascript-compiler.js @@ -504,9 +504,9 @@ JavaScriptCompiler.prototype = { this.push(helper.name + ' || ' + nonHelper); this.replaceStack(function(name) { - return name + ' ? ' + name + '.call(' + - helper.callParams + ") " + ": helperMissing.call(" + - helper.helperMissingParams + ")"; + return name + ' ? ' + + name + '.call(' + helper.callParams + ") " + + " : helperMissing.call(" + helper.helperMissingParams + ")"; }); }, @@ -843,7 +843,7 @@ JavaScriptCompiler.prototype = { } if (!inverse) { - this.context.aliases.self = "this"; + this.context.aliases.self = "this"; inverse = "self.noop"; } From 4713abc8f1cbb3910cac878bbea1e14e329f6110 Mon Sep 17 00:00:00 2001 From: kpdecker Date: Wed, 1 Jan 2014 18:15:39 -0600 Subject: [PATCH 15/19] Add subexpression benchmark --- bench/templates/subexpression.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 bench/templates/subexpression.js diff --git a/bench/templates/subexpression.js b/bench/templates/subexpression.js new file mode 100644 index 000000000..261c22d01 --- /dev/null +++ b/bench/templates/subexpression.js @@ -0,0 +1,14 @@ +module.exports = { + helpers: { + echo: function(value) { + return 'foo ' + value; + }, + header: function() { + return "Colors"; + } + }, + handlebars: "{{echo (header)}}", + eco: "<%= @echo(@header()) %>" +}; + +module.exports.context = module.exports.helpers; From 3c85720137c039cefc38046541f05731d151a506 Mon Sep 17 00:00:00 2001 From: kpdecker Date: Wed, 1 Jan 2014 18:59:21 -0600 Subject: [PATCH 16/19] Remove duplication from generated subexpressions --- lib/handlebars/compiler/ast.js | 2 + lib/handlebars/compiler/compiler.js | 2 +- .../compiler/javascript-compiler.js | 50 ++++++++++++++----- 3 files changed, 41 insertions(+), 13 deletions(-) diff --git a/lib/handlebars/compiler/ast.js b/lib/handlebars/compiler/ast.js index 6ccd7f15b..09bf92718 100644 --- a/lib/handlebars/compiler/ast.js +++ b/lib/handlebars/compiler/ast.js @@ -64,6 +64,8 @@ var AST = { this.sexpr = new AST.SexprNode(rawParams, hash); } + this.sexpr.isRoot = true; + // Support old AST API that stored this info in MustacheNode this.id = this.sexpr.id; this.params = this.sexpr.params; diff --git a/lib/handlebars/compiler/compiler.js b/lib/handlebars/compiler/compiler.js index 00353a8f5..0b25ba51d 100644 --- a/lib/handlebars/compiler/compiler.js +++ b/lib/handlebars/compiler/compiler.js @@ -281,7 +281,7 @@ Compiler.prototype = { } else if (this.options.knownHelpersOnly) { throw new Error("You specified knownHelpersOnly, but used the unknown helper " + name); } else { - this.opcode('invokeHelper', params.length, name); + this.opcode('invokeHelper', params.length, name, sexpr.isRoot); } }, diff --git a/lib/handlebars/compiler/javascript-compiler.js b/lib/handlebars/compiler/javascript-compiler.js index dc77ef6df..cd75af931 100644 --- a/lib/handlebars/compiler/javascript-compiler.js +++ b/lib/handlebars/compiler/javascript-compiler.js @@ -496,18 +496,31 @@ JavaScriptCompiler.prototype = { // and pushes the helper's return value onto the stack. // // If the helper is not found, `helperMissing` is called. - invokeHelper: function(paramSize, name) { + invokeHelper: function(paramSize, name, isRoot) { this.context.aliases.helperMissing = 'helpers.helperMissing'; + this.useRegister('helper'); var helper = this.lastHelper = this.setupHelper(paramSize, name, true); var nonHelper = this.nameLookup('depth' + this.lastContext, name, 'context'); - this.push(helper.name + ' || ' + nonHelper); - this.replaceStack(function(name) { - return name + ' ? ' - + name + '.call(' + helper.callParams + ") " - + " : helperMissing.call(" + helper.helperMissingParams + ")"; - }); + var lookup = 'helper = ' + helper.name + ' || ' + nonHelper; + if (helper.paramsInit) { + lookup += ',' + helper.paramsInit; + } + + this.push( + '(' + + lookup + + ',helper ' + + '? helper.call(' + helper.callParams + ') ' + + ': helperMissing.call(' + helper.helperMissingParams + '))'); + + // Always flush subexpressions. This is both to prevent the compounding size issue that + // occurs when the code has to be duplicated for inlining and also to prevent errors + // due to the incorrect options object being passed due to the shared register. + if (!isRoot) { + this.flushInline(); + } }, // [invokeKnownHelper] @@ -536,6 +549,7 @@ JavaScriptCompiler.prototype = { // `knownHelpersOnly` flags at compile-time. invokeAmbiguous: function(name, helperCall) { this.context.aliases.functionType = '"function"'; + this.useRegister('helper'); this.emptyHash(); var helper = this.setupHelper(0, name, helperCall); @@ -545,8 +559,11 @@ JavaScriptCompiler.prototype = { var nonHelper = this.nameLookup('depth' + this.lastContext, name, 'context'); var nextStack = this.nextStack(); - this.pushSource('if (' + nextStack + ' = ' + helperName + ') { ' + nextStack + ' = ' + nextStack + '.call(' + helper.callParams + '); }'); - this.pushSource('else { ' + nextStack + ' = ' + nonHelper + '; ' + nextStack + ' = typeof ' + nextStack + ' === functionType ? ' + nextStack + '.call(' + helper.callParams + ') : ' + nextStack + '; }'); + if (helper.paramsInit) { + this.pushSource(helper.paramsInit); + } + this.pushSource('if (helper = ' + helperName + ') { ' + nextStack + ' = helper.call(' + helper.callParams + '); }'); + this.pushSource('else { helper = ' + nonHelper + '; ' + nextStack + ' = typeof helper === functionType ? helper.call(' + helper.callParams + ') : helper; }'); }, // [invokePartial] @@ -807,12 +824,13 @@ JavaScriptCompiler.prototype = { }, setupHelper: function(paramSize, name, missingParams) { - var params = []; - this.setupParams(paramSize, params, missingParams); + var params = [], + paramsInit = this.setupParams(paramSize, params, missingParams); var foundHelper = this.nameLookup('helpers', name, 'helper'); return { params: params, + paramsInit: paramsInit, name: foundHelper, callParams: ["depth0"].concat(params).join(", "), helperMissingParams: missingParams && ["depth0", this.quotedString(name)].concat(params).join(", ") @@ -870,7 +888,15 @@ JavaScriptCompiler.prototype = { options.push("data:data"); } - params.push("{" + options.join(",") + "}"); + options = "{" + options.join(",") + "}"; + if (useRegister) { + this.useRegister('options'); + params.push('options'); + return 'options=' + options; + } else { + params.push(options); + return ''; + } } }; From 6e4e1f84040d8c7ee415ef711be7572ec94f3eb6 Mon Sep 17 00:00:00 2001 From: kpdecker Date: Wed, 1 Jan 2014 19:14:51 -0600 Subject: [PATCH 17/19] Include line info in compiler thrown exceptions Fixes #636 --- lib/handlebars/base.js | 2 +- lib/handlebars/compiler/ast.js | 19 ++++++++++++------- lib/handlebars/compiler/compiler.js | 5 ++--- lib/handlebars/exception.js | 16 ++++++++++++++-- lib/handlebars/runtime.js | 6 +++--- spec/ast.js | 18 +++++++++--------- 6 files changed, 41 insertions(+), 25 deletions(-) diff --git a/lib/handlebars/base.js b/lib/handlebars/base.js index 027f05254..11667af9b 100644 --- a/lib/handlebars/base.js +++ b/lib/handlebars/base.js @@ -53,7 +53,7 @@ function registerDefaultHelpers(instance) { if(arguments.length === 2) { return undefined; } else { - throw new Error("Missing helper: '" + arg + "'"); + throw new Exception("Missing helper: '" + arg + "'"); } }); diff --git a/lib/handlebars/compiler/ast.js b/lib/handlebars/compiler/ast.js index 09bf92718..002fd3be6 100644 --- a/lib/handlebars/compiler/ast.js +++ b/lib/handlebars/compiler/ast.js @@ -106,12 +106,12 @@ var AST = { }, BlockNode: function(mustache, program, inverse, close, locInfo) { + LocationInfo.call(this, locInfo); + if(mustache.sexpr.id.original !== close.path.original) { - throw new Exception(mustache.sexpr.id.original + " doesn't match " + close.path.original); + throw new Exception(mustache.sexpr.id.original + " doesn't match " + close.path.original, this); } - LocationInfo.call(this, locInfo); - this.type = 'block'; this.mustache = mustache; this.program = program; @@ -155,11 +155,16 @@ var AST = { original += (parts[i].separator || '') + part; if (part === ".." || part === "." || part === "this") { - if (dig.length > 0) { throw new Exception("Invalid path: " + original); } - else if (part === "..") { depth++; } - else { this.isScoped = true; } + if (dig.length > 0) { + throw new Exception("Invalid path: " + original, this); + } else if (part === "..") { + depth++; + } else { + this.isScoped = true; + } + } else { + dig.push(part); } - else { dig.push(part); } } this.original = original; diff --git a/lib/handlebars/compiler/compiler.js b/lib/handlebars/compiler/compiler.js index 0b25ba51d..461f98b65 100644 --- a/lib/handlebars/compiler/compiler.js +++ b/lib/handlebars/compiler/compiler.js @@ -279,7 +279,7 @@ Compiler.prototype = { if (this.options.knownHelpers[name]) { this.opcode('invokeKnownHelper', params.length, name); } else if (this.options.knownHelpersOnly) { - throw new Error("You specified knownHelpersOnly, but used the unknown helper " + name); + throw new Exception("You specified knownHelpersOnly, but used the unknown helper " + name, sexpr); } else { this.opcode('invokeHelper', params.length, name, sexpr.isRoot); } @@ -316,7 +316,7 @@ Compiler.prototype = { DATA: function(data) { this.options.data = true; if (data.id.isScoped || data.id.depth) { - throw new Exception('Scoped data references are not supported: ' + data.original); + throw new Exception('Scoped data references are not supported: ' + data.original, data); } this.opcode('lookupData'); @@ -350,7 +350,6 @@ Compiler.prototype = { }, addDepth: function(depth) { - if(isNaN(depth)) { throw new Error("EWOT"); } if(depth === 0) { return; } if(!this.depths[depth]) { diff --git a/lib/handlebars/exception.js b/lib/handlebars/exception.js index 6de9cfdb4..8c5c2f669 100644 --- a/lib/handlebars/exception.js +++ b/lib/handlebars/exception.js @@ -1,13 +1,25 @@ var errorProps = ['description', 'fileName', 'lineNumber', 'message', 'name', 'number', 'stack']; -function Exception(/* message */) { - var tmp = Error.prototype.constructor.apply(this, arguments); +function Exception(message, node) { + var line; + if (node && node.firstLine) { + line = node.firstLine; + + message += ' - ' + line + ':' + node.firstColumn; + } + + var tmp = Error.prototype.constructor.call(this, message); // Unfortunately errors are not enumerable in Chrome (at least), so `for prop in tmp` doesn't work. for (var idx = 0; idx < errorProps.length; idx++) { this[errorProps[idx]] = tmp[errorProps[idx]]; } + + if (line) { + this.lineNumber = line; + this.column = node.firstColumn; + } } Exception.prototype = new Error(); diff --git a/lib/handlebars/runtime.js b/lib/handlebars/runtime.js index 5c636457c..5ae8d8a78 100644 --- a/lib/handlebars/runtime.js +++ b/lib/handlebars/runtime.js @@ -10,11 +10,11 @@ export function checkRevision(compilerInfo) { if (compilerRevision < currentRevision) { var runtimeVersions = REVISION_CHANGES[currentRevision], compilerVersions = REVISION_CHANGES[compilerRevision]; - throw new Error("Template was precompiled with an older version of Handlebars than the current runtime. "+ + throw new Exception("Template was precompiled with an older version of Handlebars than the current runtime. "+ "Please update your precompiler to a newer version ("+runtimeVersions+") or downgrade your runtime to an older version ("+compilerVersions+")."); } else { // Use the embedded version info since the runtime doesn't know about this revision yet - throw new Error("Template was precompiled with a newer version of Handlebars than the current runtime. "+ + throw new Exception("Template was precompiled with a newer version of Handlebars than the current runtime. "+ "Please update your runtime to a newer version ("+compilerInfo[1]+")."); } } @@ -24,7 +24,7 @@ export function checkRevision(compilerInfo) { export function template(templateSpec, env) { if (!env) { - throw new Error("No environment passed to template"); + throw new Exception("No environment passed to template"); } // Note: Using env.VM references rather than local var references throughout this section to allow diff --git a/spec/ast.js b/spec/ast.js index 0cb11030e..46f0131f7 100644 --- a/spec/ast.js +++ b/spec/ast.js @@ -78,8 +78,8 @@ describe('ast', function() { shouldThrow(function() { var sexprNode = new handlebarsEnv.AST.SexprNode([{ original: 'foo'}], null); var mustacheNode = new handlebarsEnv.AST.MustacheNode(sexprNode, null, '{{', {}); - new handlebarsEnv.AST.BlockNode(mustacheNode, {}, {}, {path: {original: 'bar'}}); - }, Handlebars.Exception, "foo doesn't match bar"); + new handlebarsEnv.AST.BlockNode(mustacheNode, {}, {}, {path: {original: 'bar'}}, {first_line: 2, first_column: 2}); + }, Handlebars.Exception, "foo doesn't match bar - 2:2"); }); it('stores location info', function(){ @@ -102,22 +102,22 @@ describe('ast', function() { {part: 'foo'}, {part: '..'}, {part: 'bar'} - ]); - }, Handlebars.Exception, "Invalid path: foo.."); + ], {first_line: 1, first_column: 1}); + }, Handlebars.Exception, "Invalid path: foo.. - 1:1"); shouldThrow(function() { new handlebarsEnv.AST.IdNode([ {part: 'foo'}, {part: '.'}, {part: 'bar'} - ]); - }, Handlebars.Exception, "Invalid path: foo."); + ], {first_line: 1, first_column: 1}); + }, Handlebars.Exception, "Invalid path: foo. - 1:1"); shouldThrow(function() { new handlebarsEnv.AST.IdNode([ {part: 'foo'}, {part: 'this'}, {part: 'bar'} - ]); - }, Handlebars.Exception, "Invalid path: foothis"); + ], {first_line: 1, first_column: 1}); + }, Handlebars.Exception, "Invalid path: foothis - 1:1"); }); it('stores location info', function(){ @@ -216,7 +216,7 @@ describe('ast', function() { firstColumn: 0, lastColumn: 0 }; - var pn = new handlebarsEnv.AST.ProgramNode([], {strip: {}}, [ clone ], LOCATION_INFO); + pn = new handlebarsEnv.AST.ProgramNode([], {strip: {}}, [ clone ], LOCATION_INFO); testLocationInfoStorage(pn); // Assert that the newly created ProgramNode has the same location From 2b9f3c195f697265f81210d3aa20455efd8cb5b7 Mon Sep 17 00:00:00 2001 From: kpdecker Date: Wed, 1 Jan 2014 22:09:50 -0600 Subject: [PATCH 18/19] Update release notes --- release-notes.md | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/release-notes.md b/release-notes.md index 8c1198838..3f0fac995 100644 --- a/release-notes.md +++ b/release-notes.md @@ -2,7 +2,21 @@ ## Development -[Commits](https://github.com/wycats/handlebars.js/compare/v1.2.1...master) +[Commits](https://github.com/wycats/handlebars.js/compare/v1.3.0...master) + +## v1.3.0 - January 1st, 2014 +- [#690](https://github.com/wycats/handlebars.js/pull/690) - Added support for subexpressions ([@machty](https://api.github.com/users/machty)) +- [#696](https://github.com/wycats/handlebars.js/pull/696) - Fix for reserved keyword "default" ([@nateirwin](https://api.github.com/users/nateirwin)) +- [#692](https://github.com/wycats/handlebars.js/pull/692) - add line numbers to nodes when parsing ([@fivetanley](https://api.github.com/users/fivetanley)) +- [#695](https://github.com/wycats/handlebars.js/pull/695) - Pull options out from param setup to allow easier extension ([@blakeembrey](https://api.github.com/users/blakeembrey)) +- [#694](https://github.com/wycats/handlebars.js/pull/694) - Make the environment reusable ([@blakeembrey](https://api.github.com/users/blakeembrey)) +- [#636](https://github.com/wycats/handlebars.js/issues/636) - Print line and column of errors ([@sgronblo](https://api.github.com/users/sgronblo)) +- Use literal for data lookup - c1a93d3 +- Add stack handling sanity checks - cd885bf +- Fix stack id "leak" on replaceStack - ddfe457 +- Fix incorrect stack pop when replacing literals - f4d337d + +[Commits](https://github.com/wycats/handlebars.js/compare/v1.2.1...v1.3.0) ## v1.2.1 - December 26th, 2013 - [#684](https://github.com/wycats/handlebars.js/pull/684) - Allow any number of trailing characters for valid JavaScript variable ([@blakeembrey](https://api.github.com/users/blakeembrey)) From 0c6829f8af189622afd726bb72c195b5323bd6f9 Mon Sep 17 00:00:00 2001 From: kpdecker Date: Wed, 1 Jan 2014 22:10:31 -0600 Subject: [PATCH 19/19] v1.3.0 --- components/bower.json | 2 +- components/handlebars.js.nuspec | 2 +- lib/handlebars/base.js | 2 +- package.json | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/components/bower.json b/components/bower.json index b55ebc502..25923a475 100644 --- a/components/bower.json +++ b/components/bower.json @@ -1,6 +1,6 @@ { "name": "handlebars", - "version": "1.2.1", + "version": "1.3.0", "main": "handlebars.js", "dependencies": {} } diff --git a/components/handlebars.js.nuspec b/components/handlebars.js.nuspec index 610ca2f31..e4a169103 100644 --- a/components/handlebars.js.nuspec +++ b/components/handlebars.js.nuspec @@ -2,7 +2,7 @@ handlebars.js - 1.2.1 + 1.3.0 handlebars.js Authors https://github.com/wycats/handlebars.js/blob/master/LICENSE https://github.com/wycats/handlebars.js/ diff --git a/lib/handlebars/base.js b/lib/handlebars/base.js index 11667af9b..00c8bf1ff 100644 --- a/lib/handlebars/base.js +++ b/lib/handlebars/base.js @@ -1,7 +1,7 @@ module Utils from "./utils"; import Exception from "./exception"; -export var VERSION = "1.2.1"; +export var VERSION = "1.3.0"; export var COMPILER_REVISION = 4; export var REVISION_CHANGES = { diff --git a/package.json b/package.json index 90b6be31a..6403c5d46 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "handlebars", "barename": "handlebars", - "version": "1.2.1", + "version": "1.3.0", "description": "Handlebars provides the power necessary to let you build semantic templates effectively with no frustration", "homepage": "http://www.handlebarsjs.com/", "keywords": [