Why transform WellKnownName property to lowercase?

[jahow]

Question

When doing some experiments with mark symbolizers using font glyphs instead of standard shapes, I realized the WellKnownName property of the symbolizer was modified to lowercase here:

geostyler-sld-parser/src/SldStyleParser.ts

Line 1702 in e19a1e9

markSymbolizer.wellKnownName.toLowerCase()

This looks like it is necessary because the Geostyler format expects shapes to be named Circle, Cross etc. and the SLD standard works with circle, cross etc.

Unfortunately, this results in a very strong constraint where no upper case character can be used in well known names at all, which makes it impossible to use font glyphs like so: <WellKnownName>ttf://Webdings#0x0064</WellKnownName>.

Without the upper case, the previous example will not render correctly in GeoServer.

Would it be possible to consider making the logic more fine-grained, and for example only make the first character lowercase? or have an association of well known names such as Circle -> circle etc?

If this modification receives approval, I will create a Pull Request in that sense.

Note that this is linked to the issue on the OL parser here: geostyler/geostyler-openlayers-parser#230

Thanks a lot!

[KaiVolland]

Maybe we should modify the geostyler-style instead. I don't see any reason why we need have it uppercase. So if we use lowercase we don't to transform the wellKnonwnName at all.
Any thoughts @jansule

[jansule]

I don't see any reason why we need have it uppercase

Me neither.

Maybe we should modify the geostyler-style instead

This should be the first thing to do. Changes should then also be applied to the other parsers as well.

[jahow]

Ok, I guess I can help with that. Also, now that I think of it, the geostyler format does restrict the well known name property to a list of accepted ones, so this also implicitly discourages the use of other formats:
https://github.com/geostyler/geostyler-style/blob/ef5be29cbfb38bbf9a9f8be805ba33e3b166d7d2/index.d.ts#L154-L157

Maybe this should be structured differently, with a list of "default" well known names provided but not mandatory?

[KaiVolland]

If we want to support the fonts we have to switch to type string anyhow. There is no thing like regex validation for the types. At least as far as i know.

Theres a proposal though: microsoft/TypeScript#6579

[jahow]

Last thing: if we remove the toLowercase step, this should be advertised as a potentially breaking change, right?

[KaiVolland]

Yes.
I think its not a breaking change to the geostyler-style as the old values should still work here.
'shape://slash' is still a string.

But it might be a breaking change to the geostlyer-sld-parser.

[jahow]

Also it looks like it will break many things in the https://github.com/geostyler/geostyler components which use the uppercased well known names like Circle extensively.

I think the SLD parser should have a specific logic to keep backwards compatibility, either:

1. do not lowercase the WellKnownName if it starts with a scheme (ttf://, shape:// etc.)
2. only lowercase the first character (sounds a bit more hackish)

See #278 for a proposed modification of the SLD parser which keeps backwards compatibility.

[KaiVolland]

I think this is fixed with geostyler/geostyler-style#143

[jansule]

Unfortunately, this issue cannot be closed yet, as this also affects other parser, e.g. geostyler-openlayers-parser (https://github.com/geostyler/geostyler-openlayers-parser/blob/master/src/OlStyleParser.ts#L761)

[KaiVolland]

geostyler/geostyler-style#171 transforms the representation of WellKnownName in the geostyler-style. This may affect this issue.