gravitystorm / openstreetmap-carto

A general-purpose OpenStreetMap mapnik style, in CartoCSS
Other
1.51k stars 811 forks source link

Use common Double Quotes for Columns #4940

Closed MaikBusch78 closed 2 months ago

MaikBusch78 commented 3 months ago

Fixes: no officially reported issue

Changes proposed in this pull request:

Test rendering with links to the example places:

imagico commented 3 months ago

Same comments as in https://github.com/gravitystorm/openstreetmap-carto/pull/4939#pullrequestreview-1920960136 apply. In addition - some guidance would be important describing when double quotes are required and when not.

MaikBusch78 commented 3 months ago

Attribute quoting is needed iff the attribute name does not match the regex [a-zA-Z0-9_][a-zA-Z0-9_-]*, see here https://github.com/mapbox/carto/blob/305107201d4f8c4a2b9118c0077dde904f4224ed/build/tmlanguage_template.js#L50C78-L50C104.

MaikBusch78 commented 3 months ago

I have also tested this change now. I produced a project.xml with carto project.mml > project.xml.1 before my change and after my change with carto project.mml > project.xml.2. Both project.xml files are identical (i used md5sum). I have also greped into the changed files to be sure that the input is really different.

MaikBusch78 commented 3 months ago

So my own doubt that a rendering with the selector ['construction' = 'subway'] compared to a selector [construction = 'subway'] might be different, is at least for mapnik based rendering with a generated project.xml not valid.

MaikBusch78 commented 3 months ago

@imagico What can i do to get your approval here too ?