Allow setting of properties without generating rules #462

Open
pnorman opened this Issue Jan 20, 2017 · 5 comments

Projects

None yet

4 participants

@pnorman
Contributor
pnorman commented Jan 20, 2017

A common problem when you want to set some secondary properties (e.g. color) in common to multiple selectors, but the enclosing block may not be exclusive. An example based on recent work is

#foo {
  [feature = 'bar'] {
    line-color: red;
    line-width: 2;
  }
  [feature = 'baz'] {
    line-color: blue;
    line-width: 2;
  }
}

In real-world examples instead of just one line-color line there can be 5-10 properties set for all features, and dozens (or much more, for osm-carto) of selectors for different features.

A logical syntax to try is moving the line-width up a block to be common:

#foo {
  line-width: 2;
  [feature = 'bar'] {
    line-color: red;
  }
  [feature = 'baz'] {
    line-color: blue;
  }
}

But this doesn't work, because it generates rules for feature=bar, feature=baz, and feature != bar or baz, or the same as

#foo {
  line-color: black; // default value
  line-width: 2;
  [feature = 'bar'] {
    line-color: red;
  }
  [feature = 'baz'] {
    line-color: blue;
  }
}

Proposal

I propose allowing overriding the default value with none

#foo {
  line-color: none;
  line-width: 2;
  [feature = 'bar'] {
    line-color: red;
  }
  [feature = 'baz'] {
    line-color: blue;
  }
}

This would then not generate rules for where line-color: none applies.

Workarounds

I'm aware of two common workarounds

@foo-line-width: 2
#foo {
  [feature = 'bar'] {
    line-color: red;
    line-width: @foo-line-width;
  }
  [feature = 'baz'] {
    line-color: blue;
    line-width: @foo-line-width;
  }
}

This is what OSM-Carto uses. It makes it easier to change, but still has the same duplicate lines, and adds lots of variables.

#foo {
  [feature = 'bar'],
  [feature = 'baz'] {
    line-width: 2;
    [feature = 'bar'] {
      line-color: red;
    }
    [feature = 'baz'] {
      line-color: blue;
    }
  }
}

This avoids the extra rule, but then the rules are duplicated and need to be kept in sync, which is prone to failure. It also makes the MSS more complex.

Impacts

For a real-world example of the types of selectors, see https://github.com/gravitystorm/openstreetmap-carto/blob/master/amenity-points.mss. I estimate it would save 400 lines from .points in that file and 300 lines from text, reducing the file size by a third.

@nebulon42
Collaborator
nebulon42 commented Jan 20, 2017 edited

This comes from Mapnik, but I think the issue behind this is that some properties have implicit default values. Such as line-color has black or line-width has 1. It would also help if not specifying default values would not lead to rules. There are other cases where an error is thrown instead e.g. for text-face-name or shield-face-name.

Implementation-wise your proposal might be easier.

@pnorman
Contributor
pnorman commented Jan 20, 2017 edited

the issue behind this is that some properties have implicit default values

Yes - the point of the proposed none. Not generating rules when the "primary" property is unset (e.g. not generating a LineSymbolizer when line-width is unset) would also work, but then it'd be good to have a way to unset a property, and that would probably have to be a 1.0 change as it could break a number of styles.

@nebulon42 nebulon42 added this to the 1.0 milestone Jan 20, 2017
@HolgerJeromin

This would prevent some bugs with missing attributes, too.

@nebulon42
Collaborator

Related: #214

@Zverik
Zverik commented Feb 9, 2017

I agree with @nebulon42: setting default values to none and introducing this value should make style creation less wtfy.

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