Allow variables to be set within conditional sections

[csytsma]

It appears that variables can only be set in a stand-alone line. For example: @color: red;

It would be great if the variable assignment could happen within a conditional, so the value could be set based upon the element being rendered. For example, with [name] being set in the layer data:

[name='trek'] { @color: red; } [name='bike'] { @color: blue; } [name='canoe'] { @color: green; } line-color: @color;

[nebulon42]

What would be the improvement in contrast to writing:

[name='trek'] {
  line-color: red;
}
[name='bike'] {
  line-color: blue;
}
[name='canoe'] {
  line-color: green;
}

Variables are meant to ease the re-use of a definition, but in your example I don't see the added value of using variables. Could you come up with an example that better illustrates your point?

[csytsma]

The situation that lead me to want this, was that I was styling a line, with a marker. I ended up having to specify the color 4 times: line-color, text-fill, marker-line-color and marker-fill.

[name='trek'] { line-color: red; text-fill: red; marker-line-color: red; marker-fill: red; } [name='bike'] { line-color: blue; text-fill: blue; marker-line-color: blue; marker-fill: blue; } [name='canoe'] { line-color: green; text-fill: green; marker-line-color: green; marker-fill: green; }

Compared to: [name='trek'] { @color: red; } [name='bike'] { @color: blue; } [name='canoe'] { @color: green; } line-color: @color; text-fill: @color; marker-line-color: @color; marker-fill: @color;

Makes for changing a color easier in the future. The variable can be at the top of the section, easy to find, and only change in one place.

Thanks

[nebulon42]

Ok, makes more sense now. :)

[nebulon42]

Related to #338.

[nebulon42]

This is nearly working, but there is a tricky thing left that may lead to (a lot more) architectural changes.

Some architectural changes were already needed, I'm documenting them here for reference. I had to

• establish parent - child relationships between rulesets and between ruleset and rules
• establish a relationship between a rule and its building blocks (expressions, vars, dimensions, ...)
• extend specificity with zoom level (this gets tricky when a zoom level filter itself contains a variable, then we could easily go into an infinite evaluation loop, therefore I disabled zoom level within specificity for those cases)
• compute specificity for variable definitions, compute specificity for the rule that is evaluating the variable and use that to decide which variable definition to select

What is working:

@var: 'foo';

#world {
  @var: 'oof';
  [zoom >= 5] {
      @var: 'roof';
      text-name: @var + 'bar';
  }
  text-face-name: 'Arial';
  text-name: @var + 'bar';
}

Note the duplicated text-name rule.

What is (not yet) working:

@var: 'foo';

#world {
  @var: 'oof';
  [zoom >= 5] {
      @var: 'roof';
  }
  text-face-name: 'Arial';
  text-name: @var + 'bar';
}

The reason for that is the following: When the evaluating rule is not part of the filter in question it also does not inherit its specificity options. At render time this is transformed to two separate rules with the right zoom levels set. But at parse time where the variables are evaluated this is not yet done. There are no two separate text-name rules but only one. This one rule has to resolve to one variable value the first time and to another one the second time.

I don't know yet how to solve this. One idea would be to move the duplication from render to parse time and generate two rules. One for zoom >= 5 and one for the rest. Then the variables could be evaluated to the right values. But maybe this requires too many core changes.

This problem does not apply to Less because they only have selectors and no filters.

Branch: vars-everywhere

[nebulon42]

When looking closer it turned out that my first explanation above was not quite correct (as is the case most of the time when I'm dealing with carto :)). When inheriting rules from one definition to another I have to make sure that all relationships (parent - child, rule) I introduced are updated accordingly. Additionally, the zoom filter of the inherited rule has to be updated. Then I have to make sure that folding the style (removing rules that cannot be reached with filter-mode="first") does not remove a second rule with identical filter (none) but different zoom filter.

[csytsma]

Thanks for your work on this Michael. Does it seem achievable? Anything I can help with?

-Cory

On Jul 2, 2017, at 12:30 PM, Michael Glanznig notifications@github.com wrote:

When looking closer it turned out that my first explanation above was not quite correct (as is the case most of the time when I'm dealing with carto :)). When inheriting rules from one definition to another I have to make sure that all relationships (parent - child, rule) I introduced are updated accordingly. Additionally, the zoom filter of the inherited rule has to be updated. Then I have to make sure that folding the style (removing rules that cannot be reached with filter-mode="first") does not remove a second rule with identical filter (none) but different zoom filter.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/mapbox/carto/issues/461#issuecomment-312512039, or mute the thread https://github.com/notifications/unsubscribe-auth/AAH3RD81YsxOjZIYo0gTNRuX9nbm7oRzks5sJ-_qgaJpZM4LjclV.

[nebulon42]

I think its achievable. I made the examples above work, but broke 7 other tests that have to be fixed again. Afterwards it has to be tested against real stylesheets if there are no regressions. And the changes probably make the whole thing a bit slower.

There can still be surprises. I once already had to revert a fix for a bug that unacceptably degraded performance for osm-carto. That said, there is not much what can be done now. But once I'm finished it is still important to try it out and test it further.

[nebulon42]

A little update as I have to hold myself back from applying rough patches over cracks that have appeared on the last mile. This would make things only worse. The inheritance/folding topic starts to break depending on the style rules applied which is a bad thing. It seems that the current way of inheriting rules and removing unnecessary ones is not really compatible with the changes required for evaluating variables at arbitrary positions in the tree.

I think the issue here is that filters and zoom levels are not handled properly when inheriting and folding. While zoom level is also a filter it is treated differently and only filters (without zoom level) are taken into account when inheriting and folding. This needs to change as zoom level is also important when deciding which variable definition to use to evaluate an expression.

What I did until now was that I patched the logic, but apparently did not understand it fully or did not patch the right things. So I think I have to fully rewrite the inheritance/folding logic. If I'm lucky this also helps us with #20 or #213. I'm postponing this a bit, current status is still at the vars-everywhere branch.