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

property: PropertyNames is case sensitive #25

Open
Jontem opened this issue Nov 2, 2020 · 4 comments
Open

property: PropertyNames is case sensitive #25

Jontem opened this issue Nov 2, 2020 · 4 comments

Comments

@Jontem
Copy link

Jontem commented Nov 2, 2020

Property names in the javascript implementation is case sensitive. This is a problem when serialized and used between the earlier C# implementation which was case insensitive.

For example when validating property filters Size and size are different properties.

The issue exists in promaster app and edit:
image

Should we have case sensitive or case insensitive property names?

@jonaskello
Copy link
Member

From what I recall we decided somewhere back in time that property names should be case insensitive. Perhaps someone else remembers more. @AdamLuotonen do you remember anything about this?

@AdamLuotonen
Copy link

Yes, I believe we said that they should be case insensitive. However that raises a few other questions how the API should work. For example should GetPropertyNames() return with all lowercase or should casing be kept there? Or should all names be transformed already when parsing PVS/PF? In the end I think having them case sensitive is easier to reason about and would be my choice. I think some applications make use of PascalCase/camelCase and then it is easier to keep their casing for debugging reasons etc.

@Jontem
Copy link
Author

Jontem commented Nov 9, 2020

If we want to preserve the case the only thing i can come up with is to store original parsed property names at a specified key in the PropertyValueSet object

@jonaskello
Copy link
Member

Could we not just keep the original property names and just make the string comparison case-insensitive?

@jonaskello jonaskello changed the title PropertyNames is case sensitive property: PropertyNames is case sensitive Dec 8, 2021
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

3 participants