From f9fcffeebafdb4c90f28d1587708457f4b0827de Mon Sep 17 00:00:00 2001 From: Tom MacWright Date: Mon, 15 Oct 2012 10:16:27 -0700 Subject: [PATCH 1/4] Fix nested class behavior. This changes the output of complex_cascades and changes current behavior, but to the best of my sleep-deprived cognition I think we were testing for incorrect behavior. --- lib/carto/renderer.js | 3 +++ lib/carto/tree/ruleset.js | 32 +++++++++++++---------- test/rendering/combined_class.mml | 27 +++++++++++++++++++ test/rendering/combined_class.mss | 11 ++++++++ test/rendering/combined_class.result | 36 ++++++++++++++++++++++++++ test/rendering/complex_cascades.result | 10 +++---- test/rendering/nesting_class.mml | 27 +++++++++++++++++++ test/rendering/nesting_class.mss | 10 +++++++ test/rendering/nesting_class.result | 36 ++++++++++++++++++++++++++ 9 files changed, 174 insertions(+), 18 deletions(-) create mode 100644 test/rendering/combined_class.mml create mode 100644 test/rendering/combined_class.mss create mode 100644 test/rendering/combined_class.result create mode 100644 test/rendering/nesting_class.mml create mode 100644 test/rendering/nesting_class.mss create mode 100644 test/rendering/nesting_class.result diff --git a/lib/carto/renderer.js b/lib/carto/renderer.js index d3541b1..83aa132 100644 --- a/lib/carto/renderer.js +++ b/lib/carto/renderer.js @@ -215,6 +215,9 @@ function inheritRules(definitions, env) { return result; } +// Sort styles by the minimum index of their rules. +// This sorts a slice of the styles, so it returns a sorted +// array but does not change the input. function sortStyles(styles, env) { styles.forEach(function(style) { style.index = Infinity; diff --git a/lib/carto/tree/ruleset.js b/lib/carto/tree/ruleset.js index 214cb56..2bc6d4a 100644 --- a/lib/carto/tree/ruleset.js +++ b/lib/carto/tree/ruleset.js @@ -8,8 +8,9 @@ tree.Ruleset = function Ruleset(selectors, rules) { this.is = 'ruleset'; }; tree.Ruleset.prototype = { - eval: function(env) { - var ruleset = new tree.Ruleset(this.selectors, this.rules.slice(0)); + 'eval': function(env) { + var i, + ruleset = new tree.Ruleset(this.selectors, this.rules.slice(0)); ruleset.root = this.root; // push the current ruleset to the frames stack @@ -17,7 +18,7 @@ tree.Ruleset.prototype = { // Evaluate imports if (ruleset.root) { - for (var i = 0; i < ruleset.rules.length; i++) { + for (i = 0; i < ruleset.rules.length; i++) { if (ruleset.rules[i] instanceof tree.Import) { Array.prototype.splice .apply(ruleset.rules, [i, 1].concat(ruleset.rules[i].eval(env))); @@ -26,7 +27,7 @@ tree.Ruleset.prototype = { } // Evaluate everything else - for (var i = 0, rule; i < ruleset.rules.length; i++) { + for (i = 0, rule; i < ruleset.rules.length; i++) { rule = ruleset.rules[i]; ruleset.rules[i] = rule.eval ? rule.eval(env) : rule; } @@ -71,7 +72,8 @@ tree.Ruleset.prototype = { this.rulesets().forEach(function(rule) { if (rule !== self) { for (var j = 0; j < rule.selectors.length; j++) { - if (match = selector.match(rule.selectors[j])) { + match = selector.match(rule.selectors[j]); + if (match) { if (selector.elements.length > 1) { Array.prototype.push.apply(rules, rule.find( new tree.Selector(null, null, selector.elements.slice(1)), self)); @@ -86,18 +88,21 @@ tree.Ruleset.prototype = { return this._lookups[key] = rules; }, flatten: function(result, parents, env) { - var selectors = []; + var selectors = [], i, j; if (this.selectors.length === 0) { env.frames = env.frames.concat(this.rules); } - for (var i = 0; i < this.selectors.length; i++) { + for (i = 0; i < this.selectors.length; i++) { var child = this.selectors[i]; - // This is an invalid filterset. - if (!child.filters) continue; + if (!child.filters) { + // TODO: is this internal inconsistency? + // This is an invalid filterset. + continue; + } if (parents.length) { - for (var j = 0; j < parents.length; j++) { + for (j = 0; j < parents.length; j++) { var parent = parents[j]; var mergedFilters = parent.filters.cloneWith(child.filters); @@ -106,7 +111,8 @@ tree.Ruleset.prototype = { // filters. This means that we only have to clone when // the zoom levels or the attachment is different too. if (parent.zoom === (parent.zoom & child.zoom) && - parent.attachment === child.attachment) { + parent.attachment === child.attachment && + parent.elements.join() === child.elements.join()) { continue; } else { mergedFilters = parent.filters; @@ -135,7 +141,7 @@ tree.Ruleset.prototype = { } var rules = []; - for (var i = 0; i < this.rules.length; i++) { + for (i = 0; i < this.rules.length; i++) { var rule = this.rules[i]; if (rule instanceof tree.Ruleset) { @@ -148,7 +154,7 @@ tree.Ruleset.prototype = { } var index = rules.length ? rules[0].index : false; - for (var i = 0; i < selectors.length; i++) { + for (i = 0; i < selectors.length; i++) { // For specificity sort, use the position of the first rule to allow // defining attachments that are under current element as a descendant // selector. diff --git a/test/rendering/combined_class.mml b/test/rendering/combined_class.mml new file mode 100644 index 0000000..0a3a3fa --- /dev/null +++ b/test/rendering/combined_class.mml @@ -0,0 +1,27 @@ +{ + "srs": "+proj=merc +a=6378137 +b=6378137 +lat_ts=0.0 +lon_0=0.0 +x_0=0.0 +y_0=0.0 +k=1.0 +units=m +nadgrids=@null +wktext +no_defs +over", + "Stylesheet": [ + "combined_class.mss" + ], + "Layer": [ + { + "name": "just_land", + "class": "land", + "srs": "+proj=merc +a=6378137 +b=6378137 +lat_ts=0.0 +lon_0=0.0 +x_0=0.0 +y_0=0.0 +k=1.0 +units=m +nadgrids=@null +wktext +no_defs +over", + "Datasource": { + "file": "http://tilemill-data.s3.amazonaws.com/test_data/shape_demo.zip", + "type": "shape" + } + }, + { + "name": "lakes", + "id": "lakes", + "class": "land", + "srs": "+proj=merc +a=6378137 +b=6378137 +lat_ts=0.0 +lon_0=0.0 +x_0=0.0 +y_0=0.0 +k=1.0 +units=m +nadgrids=@null +wktext +no_defs +over", + "Datasource": { + "file": "http://tilemill-data.s3.amazonaws.com/test_data/shape_demo.zip", + "type": "shape" + } + } + ] +} diff --git a/test/rendering/combined_class.mss b/test/rendering/combined_class.mss new file mode 100644 index 0000000..f035dca --- /dev/null +++ b/test/rendering/combined_class.mss @@ -0,0 +1,11 @@ +/* Applies to all layers with .land class */ +.land { + line-color: #ccc; + line-width: 0.5; + polygon-fill: #eee; +} +.land#lakes { + line-color: #ccc; + line-width: 0.5; + polygon-fill: #000; +} diff --git a/test/rendering/combined_class.result b/test/rendering/combined_class.result new file mode 100644 index 0000000..24dea77 --- /dev/null +++ b/test/rendering/combined_class.result @@ -0,0 +1,36 @@ + + + + + + + + just_land + + + + + + + + + lakes + + + + + + + diff --git a/test/rendering/complex_cascades.result b/test/rendering/complex_cascades.result index 29a34ee..cab5c7b 100644 --- a/test/rendering/complex_cascades.result +++ b/test/rendering/complex_cascades.result @@ -1,6 +1,6 @@ - + + srs="+proj=merc +a=6378137 +b=6378137 +lat_ts=0.0 +lon_0=0.0 +x_0=0.0 +y_0=0.0 +k=1.0 +units=m +nadgrids=@null +wktext +no_defs +over"> countries diff --git a/test/rendering/nesting_class.mml b/test/rendering/nesting_class.mml new file mode 100644 index 0000000..4fd9d8c --- /dev/null +++ b/test/rendering/nesting_class.mml @@ -0,0 +1,27 @@ +{ + "srs": "+proj=merc +a=6378137 +b=6378137 +lat_ts=0.0 +lon_0=0.0 +x_0=0.0 +y_0=0.0 +k=1.0 +units=m +nadgrids=@null +wktext +no_defs +over", + "Stylesheet": [ + "nesting_class.mss" + ], + "Layer": [ + { + "name": "foo", + "class": "land", + "srs": "+proj=merc +a=6378137 +b=6378137 +lat_ts=0.0 +lon_0=0.0 +x_0=0.0 +y_0=0.0 +k=1.0 +units=m +nadgrids=@null +wktext +no_defs +over", + "Datasource": { + "file": "http://tilemill-data.s3.amazonaws.com/test_data/shape_demo.zip", + "type": "shape" + } + }, + { + "name": "lakes", + "id": "lakes", + "class": "land", + "srs": "+proj=merc +a=6378137 +b=6378137 +lat_ts=0.0 +lon_0=0.0 +x_0=0.0 +y_0=0.0 +k=1.0 +units=m +nadgrids=@null +wktext +no_defs +over", + "Datasource": { + "file": "http://tilemill-data.s3.amazonaws.com/test_data/shape_demo.zip", + "type": "shape" + } + } + ] +} diff --git a/test/rendering/nesting_class.mss b/test/rendering/nesting_class.mss new file mode 100644 index 0000000..7f7e3e4 --- /dev/null +++ b/test/rendering/nesting_class.mss @@ -0,0 +1,10 @@ +/* Applies to all layers with .land class */ +.land { + line-color: #ccc; + line-width: 0.5; + polygon-fill: #eee; + /* Applies to #lakes.land */ + #lakes { + polygon-fill: #000; + } +} diff --git a/test/rendering/nesting_class.result b/test/rendering/nesting_class.result new file mode 100644 index 0000000..1db146f --- /dev/null +++ b/test/rendering/nesting_class.result @@ -0,0 +1,36 @@ + + + + + + + + foo + + + + + + + + + lakes + + + + + + + From 840e16a3f2f8ba4c788c601110046f00ca608b3a Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Mon, 15 Oct 2012 13:14:47 -0700 Subject: [PATCH 2/4] improve carto command line to be able to handle just converting mss chunks - mostly for easy testing --- bin/carto | 94 ++++++++++++++++++++++++++++++------------- lib/carto/renderer.js | 6 +++ 2 files changed, 71 insertions(+), 29 deletions(-) diff --git a/bin/carto b/bin/carto index 238a733..4aadf76 100755 --- a/bin/carto +++ b/bin/carto @@ -1,9 +1,13 @@ #!/usr/bin/env node -var path = require('path'), - fs = require('fs'), - util = require('util'), - carto = require('carto'); +var path = require('path'); +var fs = require('fs'); +var util = require('util'); +var carto = require('carto'); + +var url = require('url'); +var _ = require('underscore'); +var existsSync = require('fs').existsSync || require('path').existsSync var args = process.argv.slice(1); var options = { nosymlink:false }; @@ -28,7 +32,6 @@ args = args.filter(function (arg) { case 'nosymlink': options.nosymlink = true; break; - default: util.puts("Usage: carto "); util.puts("Options:"); @@ -54,37 +57,18 @@ if (options.benchmark) { var start = +new Date; } -try { - var data = fs.readFileSync(input, 'utf-8'); -} catch(err) { - util.puts("carto: " + err.message.replace(/^[A-Z]+, /, '')); - process.exit(1); -} +var ext = path.extname(input); -try { - data = JSON.parse(data); -} catch(err) { - util.puts("carto: " + err.message.replace(/^[A-Z]+, /, '')); +if (!ext) { + util.puts("carto: please pass either a .mml file or .mss file"); process.exit(1); } -var millstone = undefined; - -try { - require.resolve('millstone'); - millstone = require('millstone'); -} catch (err) { - util.puts('carto: Millstone not found. ' + err.message.replace(/^[A-Z]+, /, '')); +if (!existsSync(input)) { + util.puts("carto: file does not exist: '" + input + "'"); process.exit(1); } -millstone.resolve({ - mml: data, - base: path.dirname(input), - cache: path.join(path.dirname(input), 'cache'), - nosymlink: options.nosymlink -}, compile); - function compile(err, data) { if (err) throw err; try { @@ -113,3 +97,55 @@ function compile(err, data) { } } }; + +try { + var data = fs.readFileSync(input, 'utf-8'); +} catch(err) { + util.puts("carto: " + err.message.replace(/^[A-Z]+, /, '')); + process.exit(1); +} + +if (ext == '.mml') { + try { + data = JSON.parse(data); + } catch(err) { + util.puts("carto: " + err.message.replace(/^[A-Z]+, /, '')); + process.exit(1); + } + + var millstone = undefined; + try { + require.resolve('millstone'); + millstone = require('millstone'); + } catch (err) { + util.puts('carto: Millstone not found. ' + err.message.replace(/^[A-Z]+, /, '')); + process.exit(1); + } + millstone.resolve({ + mml: data, + base: path.dirname(input), + cache: path.join(path.dirname(input), 'cache'), + nosymlink: options.nosymlink + }, compile); +} else if (ext == '.mss') { + var env = _({}).defaults({ + benchmark: false, + validation_data: false, + effects: [] + }); + var output = []; + var parser = (carto.Parser(env)).parse(data); + var rules = parser.toList(env); + var inherited = carto.inheritRules(rules,env); + var sorted = carto.sortStyles(inherited,env); + _(sorted).each(function(rule) { + var style = new carto.tree.Style('layer', rule.attachment, rule); + if (style) { + // env.effects can be modified by this call + output.push(style.toXML(env)); + } + }); + console.log(output.join('\n')); +} else { + util.puts("carto: please pass either a .mml file or .mss file"); +} diff --git a/lib/carto/renderer.js b/lib/carto/renderer.js index d3541b1..d3c1b13 100644 --- a/lib/carto/renderer.js +++ b/lib/carto/renderer.js @@ -31,6 +31,9 @@ carto.Renderer.prototype.render = function render(m, callback) { // Transform stylesheets into rulesets. var rulesets = _(m.Stylesheet).chain() .map(function(s) { + if (typeof s == 'string') { + throw new Error("Stylesheet object is expected not a string: '" + s + "'"); + } // Passing the environment from stylesheet to stylesheet, // allows frames and effects to be maintained. env = _(env).extend({filename:s.id}); @@ -270,3 +273,6 @@ function getMapProperties(m, rulesets, env) { } module.exports = carto; +module.exports.addRules = addRules; +module.exports.inheritRules = inheritRules; +module.exports.sortStyles = sortStyles; From 8fc2c06b458d7cc4d661948d7a6b048f2be5ecaf Mon Sep 17 00:00:00 2001 From: Tom MacWright Date: Sun, 21 Oct 2012 17:57:45 -0400 Subject: [PATCH 3/4] Evaluate calls before keywords. Fixes #203 --- lib/carto/parser.js | 4 ++-- test/rendering/gray_function.mml | 14 ++++++++++++++ test/rendering/gray_function.mss | 3 +++ test/rendering/gray_function.result | 20 ++++++++++++++++++++ 4 files changed, 39 insertions(+), 2 deletions(-) create mode 100644 test/rendering/gray_function.mml create mode 100644 test/rendering/gray_function.mss create mode 100644 test/rendering/gray_function.result diff --git a/lib/carto/parser.js b/lib/carto/parser.js index 0e1876c..ff1dd8f 100644 --- a/lib/carto/parser.js +++ b/lib/carto/parser.js @@ -587,11 +587,11 @@ carto.Parser = function Parser(env) { // and can be found inside a rule's value. // entity: function() { - return $(this.entities.literal) || + return $(this.entities.call) || + $(this.entities.literal) || $(this.entities.field) || $(this.entities.variable) || $(this.entities.url) || - $(this.entities.call) || $(this.entities.keyword); }, diff --git a/test/rendering/gray_function.mml b/test/rendering/gray_function.mml new file mode 100644 index 0000000..ac72dbf --- /dev/null +++ b/test/rendering/gray_function.mml @@ -0,0 +1,14 @@ +{ + "srs": "+proj=merc +a=6378137 +b=6378137 +lat_ts=0.0 +lon_0=0.0 +x_0=0.0 +y_0=0.0 +k=1.0 +units=m +nadgrids=@null +wktext +no_defs +over", + "Stylesheet": [ + "gray_function.mss" + ], + "Layer": [{ + "name": "world", + "srs": "+proj=merc +a=6378137 +b=6378137 +lat_ts=0.0 +lon_0=0.0 +x_0=0.0 +y_0=0.0 +k=1.0 +units=m +nadgrids=@null +wktext +no_defs +over", + "Datasource": { + "file": "http://tilemill-data.s3.amazonaws.com/test_data/shape_demo.zip", + "type": "shape" + } + }] +} diff --git a/test/rendering/gray_function.mss b/test/rendering/gray_function.mss new file mode 100644 index 0000000..e2bdcfc --- /dev/null +++ b/test/rendering/gray_function.mss @@ -0,0 +1,3 @@ +#world { + polygon-fill:gray; +} diff --git a/test/rendering/gray_function.result b/test/rendering/gray_function.result new file mode 100644 index 0000000..9891f15 --- /dev/null +++ b/test/rendering/gray_function.result @@ -0,0 +1,20 @@ + + + + + + + + world + + + + + + + From 1eaf7e4fbfd2e76a331ef23d1e9315312e51731b Mon Sep 17 00:00:00 2001 From: Tom MacWright Date: Sun, 21 Oct 2012 21:08:11 -0400 Subject: [PATCH 4/4] Remove & from old-style less combinators in the parser --- lib/carto/parser.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/carto/parser.js b/lib/carto/parser.js index ff1dd8f..e778010 100644 --- a/lib/carto/parser.js +++ b/lib/carto/parser.js @@ -722,7 +722,7 @@ carto.Parser = function Parser(env) { var name, value, c = input.charAt(i); save(); - if (c === '.' || c === '#' || c === '&') { return } + if (c === '.' || c === '#') { return; } if (name = $(this.variable) || $(this.property)) { value = $(this.value);