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

Clarify inconsistencies between spec and grammar #77

Open
maxcollombin opened this issue Aug 7, 2024 · 8 comments
Open

Clarify inconsistencies between spec and grammar #77

maxcollombin opened this issue Aug 7, 2024 · 8 comments

Comments

@maxcollombin
Copy link
Collaborator

  • name, symbolizer and nestedRules are present in the spec but not in the grammar
@jerstlouis
Copy link
Member

jerstlouis commented Aug 7, 2024

@maxcollombin the stylingRuleList? is the nested rules and the (propertyAssignmentList SEMI)? is the symbolizer.

Like for the CartoSym-JSON, how the grammar / schema is defined does not necessarily need to use the exact same terms as the UML, since it expresses something different.

But we could try to make this a bit more consistent/obvious, by renaming to these to e.g., nestedStylingRulesList and symbolizerPropertyAssignments ?

But the same stylingRuleList also applies the top-level styling rules of the style, that's why it's not called nestedStylingRulesList.

By name, I imagine you mean the name of the stylingRule for the legend labeling purpose? That one is missing right now.

@maxcollombin
Copy link
Collaborator Author

Same remark/question for the name property of the IdentifierExpression class relative to the Expression class.
Does the name property correspond to a grammar element? Sorry for the obvious questions (still learning😆)

@jerstlouis
Copy link
Member

@maxcollombin

Does the name property correspond to a grammar element? Sorry for the obvious questions (still learning😆)

There is not currently a single element corresponding to that in the CartoSym-CSS grammar.

It corresponds primarily to the IDENTIFIER when used specifically in this rule:

idOrConstant: IDENTIFIER | expConstant;

We probably need to clarify whether the . object member operator and the [ ] array indexing operators are folded in and considered part of an IdentifierExpression, or their own expression types. In our CMSS implementation, these are separate classes of expressions, but these concepts do not exist yet in CQL2.

So whether you consider expression DOT IDENTIFIER in this rule a single IdentifierExpression and that whole object.member is its name is not yet well defined (same for array[10] in this rule).

@maxcollombin
Copy link
Collaborator Author

It corresponds primarily to the IDENTIFIER when used specifically in this rule:
idOrConstant: IDENTIFIER | expConstant;

Thanks for your feedback @jerstlouis . This is the result of a problem with my implementation that I have just reported here.

@maxcollombin
Copy link
Collaborator Author

I'm also having some difficulty relating this class diagram:

Figure 1 — Symbology Core Classes UML Diagram

to this part of the grammar:

///////////////////////////////
// High level style sheet rules
///////////////////////////////

styleSheet: metadata* stylingRuleList;

metadata:
    '.' IDENTIFIER CHARACTER_LITERAL;

stylingRuleList:
     stylingRule
   | stylingRuleList stylingRule;

stylingRule:
   ( selector )*
   LCBR
      (propertyAssignmentList SEMI)?
      stylingRuleList?
   RCBR;

selector:
     IDENTIFIER
   | LSBR expression RSBR ;

///////////////////////////////

@maxcollombin
Copy link
Collaborator Author

maxcollombin commented Aug 16, 2024

Regardin the Symbology Core Classes UML Diagram, I'd rather see something like this:

TL4zJyCm4DtzAwoCaLerQwPAm0P2ecF5uCP7Q-7ObUy2f1N_dNCKsQc1ASdxU7dFtYAfG4DlHMSLejpHQ65t0Y1v5LBsG4ehKdApNjUI0MJUkEFBOE7H8Cb6V8lP-ZHh9oCPij2oFB1wp-xcDTerO1VCcOntjoHpQg2J3xZ4wuY_m_ZfW_vD7C7VAr9tyUzGqgdMhcXylLtHB

@maxcollombin
Copy link
Collaborator Author

Here is the associated PlantUML code syntax:

StyleSheet : metadata 0..* 
StyleSheet : stylingRuleList 1

Metadata : title: string[0..1] 
Metadata : description: string[0..1]
Metadata : authors: string[0..*]
Metadata : keywords: string[0..*]
Metadata : geoDataClasses: string[0..*]

StylingRuleList : stylingRule: StylingRule[1..*]

StylingRule : name: string
StylingRule : symbolizer: Symbolizer[0..1]
StylingRule : selector: Expression[0..1]
StylingRule : nestedRules: StylingRuleList[0..1]

Symbolizer : visibility: bool
Symbolizer : opacity: float
Symbolizer : zOrder: integer


StyleSheet "1" -- "*" Metadata
StyleSheet "1" -- "1" StylingRuleList
StylingRuleList "0" *-- "*" StylingRule
StylingRule "0" *-- "1" Expression
StylingRule "0" *-- "*" Symbolizer

@jerstlouis
Copy link
Member

@maxcollombin

I'd rather see something like this:

In my view, conceptually a class with only one element is unnecessary.
It's cleaner to see it as a StylingRule with a 0..* multiplicity.

The stylingRuleList grammar rule is just a convenience for describing the grammar.

In my view, the grammar for the encoding does not directly relate to the conceptual / logical model at all (e.g., none of symbolizer properties / system identifier etc. appear at all in the grammar).

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

No branches or pull requests

2 participants