Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

roads.mss carto code style error #2468

Open
pnorman opened this issue Nov 27, 2016 · 6 comments
Open

roads.mss carto code style error #2468

pnorman opened this issue Nov 27, 2016 · 6 comments

Comments

@pnorman
Copy link
Collaborator

pnorman commented Nov 27, 2016

At line 1342

    [feature = 'highway_residential'],
    [feature = 'highway_unclassified'] {
      [zoom = 12][feature = 'highway_residential'] {
        line-color: @unimportant-road;
        line-width: 0.4;
      }
      [zoom = 12][feature = 'highway_unclassified'] {
        line-color: @unimportant-road;
        line-width: 1;
      }

zoom = should never be used

@matthijsmelissen
Copy link
Collaborator

Might be a nice exercise for people new to contributing.

@jojo4u
Copy link

jojo4u commented Nov 27, 2016

I tried, but i also included my changes from another of my branches (simplify-capital). Yes git is not that easy... Will try again!
master...jojo4u:road-zoom (deleted)

@nebulon42
Copy link
Contributor

In this particular case I see no sense in enforcing this. While the gist of the rule is to prevent style problems if higher/lower zooms are left out here the next rule anyway starts with zoom >= 13.

What sense does it make to change

[zoom = 12][feature = 'highway_residential'] {
  line-color: @unimportant-road;
  line-width: 0.4;
}
[zoom = 12][feature = 'highway_unclassified'] {
  line-color: @unimportant-road;
  line-width: 1;
}
[zoom >= 13] {

to

[zoom >= 12][zoom < 13][feature = 'highway_residential'] {
  line-color: @unimportant-road;
  line-width: 0.4;
}
[zoom >= 12][zoom < 13][feature = 'highway_unclassified'] {
  line-color: @unimportant-road;
  line-width: 1;
}
[zoom >= 13] {

I'm against dogmatic applying of style rules.

@matthijsmelissen
Copy link
Collaborator

See #410 for the previous time we had this discussion. I personally have no strong opinion either way.

@pnorman
Copy link
Collaborator Author

pnorman commented Nov 28, 2016

The standard way to write the logic here is

[feature = 'highway_residential'],
[feature = 'highway_unclassified'] {
  [zoom >= 12] {
    [feature = 'highway_residential'] { ... }
    [feature = 'highway_unclassified'] { ... }
  }
  [zoom >= 13] { ... }
  ...
}

@kocio-pl kocio-pl added this to the Bugs and improvements milestone Nov 29, 2016
@matkoniecz
Copy link
Contributor

Note that it is done this way in part because proper way (presented in the comment above) resulted in a weird glitch.

At that time a glitch was appearing solely in TileMill (but not if export to file was used). Due to poor reproducibility I never successfully tracked down whatever it was truly caused by this change and whatever it was bug in style code or in Tilemill or in something else.

For this reason I will will remove "easy" label - if there is really something fishy going here it may be a poor introduction.

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

No branches or pull requests

6 participants