Skip to content

Semantic distinction between an IDENT and a STRING within the CSSExpressionMemberTermSimple #75

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
shagkur opened this issue Jan 10, 2022 · 6 comments
Assignees

Comments

@shagkur
Copy link
Contributor

shagkur commented Jan 10, 2022

It'd quite useful to be able to distinct between an IDENT and a STRING on CSSExpressionMemberTermSimple member (needed for instance for the image() function, where one can pass a url as a simple string literal along with a color to be used, i.e. image('dark.png', blue))
This distinction might be useful at ohter places too.

@phax phax self-assigned this Jan 10, 2022
@phax
Copy link
Owner

phax commented Jan 10, 2022

That's a tricky one, because that means, that I need to use different data types internally, which will bloat the data model a bit more. But I can understand why it would make sense. As a heurisitc you can say everything that starts with ' or " is a String, whereas the other things are identifiers. Does that help? I could also provide you with a quick sanity method in CSSExpressionMemberTermSimple if needed...

@shagkur
Copy link
Contributor Author

shagkur commented Jan 11, 2022

That's what i acutally do, i check whether the value is surrounded by ' or " to identify it as a String. I ofcourse understand your concerns, and i already realized this is being a rather big change.
Perhaps having an enum, whether it's a String or an Ident value in the CSSExpressionMemberTermSimple, which is resolved upon construction of this member would already help?

shagkur added a commit to unblu/ph-css that referenced this issue Jan 12, 2022
@shagkur
Copy link
Contributor Author

shagkur commented Jan 12, 2022

Here's a PR (if you like)
#76

@phax
Copy link
Owner

phax commented Jan 12, 2022

Thanks for providing this patch - is that urgent for you? Than I can build a 6.4.2 version

@shagkur
Copy link
Contributor Author

shagkur commented Jan 12, 2022

No worries. Do you normal release schedule. I can work with my fork. In the long run, i once want to switch to your release build rather than using the local fork.
Thanks alot for your quick reaction on all these issues & PRs i put :)

@phax
Copy link
Owner

phax commented Jan 12, 2022

Great thanks. Closing this issue for now.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

2 participants