Conversation
A single rule block can have comma-separated selectors that target
different ::object-id subparts, e.g.
node|z16-[addr:housenumber][addr:street],
node|z16-[addr:housenumber][addr:street]::int_name,
{text: none;}
Previously StyleChooser.updateStyles asked testChains for the *first*
matching (rule, object_id) and applied the body only to that one. The
::int_name selector here would silently fall through, keeping whatever
text the cascade inherited from earlier choosers. In Organic Maps this
surfaced as:
ERROR: priority is not set for caption building-address::int_name
because the ::int_name entry retained its default text=int_name and
then hit the standalone-caption priority lookup with no priority
defined for that subpart.
Iterate every matching ruleChain (deduped by object_id) and apply the
body to each. Runtime conditions are now also checked per-rule rather
than per-chooser, which is more correct when individual selectors carry
different runtime conditions.
The previously-asserted "only first ::class wins" behaviour in
test_update_styles_by_class was an artefact of the bug — the test now
reflects MapCSS-standard semantics.
Generated with LLM assistance.
Signed-off-by: Alexander Borsuk <me@alex.bio>
Signed-off-by: Alexander Borsuk <me@alex.bio>
Member
|
Show the diff txt fragment when applied to the current OM master repo? |
strump
reviewed
May 27, 2026
| # Apply the body to every matching subpart (deduped by object-id), not | ||
| # just the first one — otherwise the other selectors silently keep | ||
| # whatever values the cascade brought in from earlier choosers. | ||
| seen_object_ids = set() |
Collaborator
There was a problem hiding this comment.
Why do we need this seen_object_ids? Should we ignored repeated object_id ?
Collaborator
|
@vng I ran |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes a bug with int names, needed for experiments with cycling and other map styles.
A single MapCSS rule block can have comma-separated selectors that target different
::object-idsubparts:StyleChooser.updateStyles previously asked testChains for the first
matching (rule, object_id) and applied the body only to that one. The
::int_name selector silently fell through and kept whatever value the
cascade had inherited from earlier choosers.
In Organic Maps this surfaces as:
ERROR: priority is not set for caption building-address::int_name
because the ::int_name entry retains the default text: int_name from
the imported base style, then hits the standalone-caption priority lookup
in libkomwm.py with no priority defined for that subpart.
This PR iterates every matching ruleChain (deduped by object_id) and
applies the body to each. Runtime conditions are now checked per rule
rather than per chooser, which is the correct semantic when individual
selectors carry different runtime conditions (verified that no current
Organic Maps style relies on the old "first rule's runtime cond gates
the whole chooser" behaviour — there are zero hits for runtime
conditions in data/styles/).
The previously-asserted "only first ::class wins" behaviour in
test_update_styles_by_class was an artefact of the bug; the test now
reflects standard MapCSS semantics.
Test plan
— clean
binaries successfully (priority entries grow slightly: 1929 → 1935
overlays, 365 → 374 FG — these are subpart overrides that the old
bug silently dropped)
suppression of the form shown above now builds cleanly instead of
raising the "priority not set for building-address::int_name"
error
the housenumber-style case directly