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

Add support for parsing CSS variables #28

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

Caellian
Copy link

@Caellian Caellian commented Nov 3, 2024

Adds support for parsing CSS Variables Level 1.

This PR is a prerequisite for resvg#821.

Caellian

This comment was marked as outdated.

@Caellian
Copy link
Author

Caellian commented Nov 3, 2024

Should I actually add anything besides just bare VariableFunction parsing?

It feels like adding support for var() to all parsers is bad design. At the same time, Paint does handle url() and also handles fallback for it. url() fallback could be a var(). Values in Transform can also contain variable functions for instance.

Browsers store the whole AST and then resolve variables later, but the idea of svgtypes is to return the simplest uniform representation which doesn't work when variables are used.

Looking at it from resvg perspective, it has some state with current variable values and it could resolve variables. But svgtypes has no access to this information and it can only return some function that evaluates to Result<T, Error> when provided some variable state.

- Document VariableFunction fields.

Signed-off-by: Tin Švagelj <[email protected]>
@RazrFalcon
Copy link
Collaborator

Sorry, but I would not have time reviewing your PR anytime soon.
But I would suggest starting with simplecss. svgtypes isn't designed to parse CSS. It has to be done beforehand.

@Caellian
Copy link
Author

Caellian commented Nov 3, 2024

Sorry, but I would not have time reviewing your PR anytime soon.

That's alright, it can wait, not sure I'll finish it anytime soon either. 😄
Finishing doesn't block the review though as I asked most questions already so if I forget, a review bump will be a good encouragement to implement variables for other types.

svgtypes isn't designed to parse CSS. It has to be done beforehand.

Took another look at the specification and you're right, CSS variables are only supported by browser implementation of SVG DOM because it's treated as HTML. However, having a way to modify values like Paint after they've been parsed (what would be compute time in browsers) is really useful for external programs.

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

Successfully merging this pull request may close these issues.

2 participants