Repository navigation
Treat a type's forbidden [!key] as an absent key - #46
Conversation
A forbidden [!key] in a mapcss-mapping.csv selector matches features without the key, but kothic evaluated styles against key=no, which MapCSS treats as set. Now such types can exclude tunnels without inheriting the tunnel styles. The generated drawing rules do not change. See organicmaps/kothic#46. Signed-off-by: Alexander Borsuk <me@alex.bio>
d6e1353 to
18a86d1
Compare
A forbidden [!key] in a mapcss-mapping.csv selector matches features without the key, but kothic evaluated styles against key=no, which MapCSS treats as set. Now such types can exclude tunnels without inheriting the tunnel styles. The generated drawing rules do not change. See organicmaps/kothic#46. The update also brings organicmaps/kothic#44, which only touches kothic's tests. Signed-off-by: Alexander Borsuk <me@alex.bio>
| that occurs only in such conditions is not a static tag, and styles can't test it (an "Unknown tag" error). | ||
| """ | ||
| tags = OrderedDict() | ||
| for cond in selectors.split(',')[0].split('['): |
There was a problem hiding this comment.
I have doubts about this line:
selectors.split(',')[0]Why do we parse only the first selector? We have this code for a long time already. But let's put a comment to come back here later.
There was a problem hiding this comment.
Good question, and it is worth a comment - added one at that line.
Short answer: the remaining selectors are alternative OSM spellings of the same type, while the style has to be evaluated against a single canonical tag set, and the first selector is it. E.g. natural|water|intermittent matches [natural=water][intermittent=yes], [natural=water][seasonal?], [natural=water][basin=detention] - all the same classificator type, so its tags are those of the first one.
The consequence is a real blind spot: on the current mapcss-mapping.csv, 202 of 1378 live rows list several selectors and 179 of those carry keys the first selector lacks (37 distinct ones: seasonal, basin, leaf_type, location, ...). A style rule testing such a key is silently false for that type if another type makes it a static tag, and an "Unknown tag" error otherwise. Nothing in the current styles does that, so this is latent rather than broken.
So the comment states why only the first selector is used and leaves a TODO: either take the tags of every selector into account, or validate styles against them.
For the generator, a [!key] in mapcss-mapping.csv matches features where the key is absent or "no". Kothic put key=no into the type's tags that styles are evaluated against, but MapCSS treats key=no as set: [key] matched such a type and [!key] did not. So a type defined with e.g. [natural=water][intermittent=yes][!tunnel] inherited the tunnel rules. Leave such keys out of the type's tags instead. A key that occurs only as [!key] is then not a static tag, so styles can't test it; no such key exists now. The current Organic Maps styles produce the same drawing rules. Signed-off-by: Alexander Borsuk <me@alex.bio>
18a86d1 to
a8a2523
Compare
A forbidden [!key] in a mapcss-mapping.csv selector matches features without the key, but kothic evaluated styles against key=no, which MapCSS treats as set. Now such types can exclude tunnels without inheriting the tunnel styles. The generated drawing rules do not change. See organicmaps/kothic#46. The update also brings organicmaps/kothic#44, which only touches kothic's tests. Signed-off-by: Alexander Borsuk <me@alex.bio>
A forbidden [!key] in a mapcss-mapping.csv selector matches features without the key, but kothic evaluated styles against key=no, which MapCSS treats as set. Now such types can exclude tunnels without inheriting the tunnel styles. The generated drawing rules do not change. See organicmaps/kothic#46. The update also brings organicmaps/kothic#44, which only touches kothic's tests. Signed-off-by: Alexander Borsuk <me@alex.bio>
A forbidden [!key] in a mapcss-mapping.csv selector matches features without the key, but kothic evaluated styles against key=no, which MapCSS treats as set. Now such types can exclude tunnels without inheriting the tunnel styles. The generated drawing rules do not change. See organicmaps/kothic#46. The update also brings organicmaps/kothic#44, which only touches kothic's tests. Signed-off-by: Alexander Borsuk <me@alex.bio>
2 participants
A
[!key]in the first selector of amapcss-mapping.csvrow was put into the type's tags askey=no. MapCSS treatskey=noas set, so[key]style rules matched such a type and[!key]ones did not, unlike the generator, which matches[!key]for features without the key. Now such keys are left out of the type's tags. A key that occurs only as[!key]is then not a static tag, so styles can't test it ("Unknown tag"); no such key exists now.The current Organic Maps styles produce identical drawing rules. Needed by organicmaps/organicmaps#13543, which excludes tunnels with
[!tunnel].Tested: unit tests (new
test_get_type_tags, including the tags' order), ruff, andgenerate_drules.shin organicmaps without drules changes.