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

feat: basic class selector parsing #307

Merged
merged 2 commits into from
Sep 18, 2023
Merged

feat: basic class selector parsing #307

merged 2 commits into from
Sep 18, 2023

Conversation

ematipico
Copy link
Member

@ematipico ematipico commented Sep 16, 2023

Summary

This PR adds basic parsing of CSS class selector.

I removed some nodes from the grammar because they weren't needed.

Test Plan

Added basic tests

@ematipico ematipico temporarily deployed to Website deployment September 16, 2023 19:21 — with GitHub Actions Inactive
@github-actions github-actions bot added A-Core Area: core A-Parser Area: parser A-Tooling Area: internal tools L-CSS Language: CSS labels Sep 16, 2023
@github-actions
Copy link
Contributor

Parser conformance results on

js/262

Test result main count This PR count Difference
Total 48863 48863 0
Passed 47808 47808 0
Failed 1055 1055 0
Panics 0 0 0
Coverage 97.84% 97.84% 0.00%

jsx/babel

Test result main count This PR count Difference
Total 40 40 0
Passed 37 37 0
Failed 3 3 0
Panics 0 0 0
Coverage 92.50% 92.50% 0.00%

symbols/microsoft

Test result main count This PR count Difference
Total 6212 6212 0
Passed 1764 1764 0
Failed 4448 4448 0
Panics 0 0 0
Coverage 28.40% 28.40% 0.00%

ts/babel

Test result main count This PR count Difference
Total 639 639 0
Passed 573 573 0
Failed 66 66 0
Panics 0 0 0
Coverage 89.67% 89.67% 0.00%

ts/microsoft

Test result main count This PR count Difference
Total 17224 17224 0
Passed 13121 13121 0
Failed 4103 4103 0
Panics 0 0 0
Coverage 76.18% 76.18% 0.00%

)
.is_err()
{
m.abandon(p);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please share your strategy for situations where we might need to abandon and return as absent, rather than completing and returning bogus kind?

Copy link
Member Author

@ematipico ematipico Sep 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have a clear strategy in mind yet, but I've had some ideas while starting this work, still it's best if we discuss them.

For rules, I thought we could bogus+recover in the prelude and in the block.

This would allow us to parse cases like:

.foo }
.bar {}
.686 {}
.bar {}

Although, I'm not sure about the prelude of the rule.

Or maybe we should have a big CssBogusRule? What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with you, let's try to use a recovery strategy.

@ematipico ematipico temporarily deployed to Website deployment September 17, 2023 17:17 — with GitHub Actions Inactive
@ematipico ematipico merged commit ef41e50 into main Sep 18, 2023
16 checks passed
@ematipico ematipico deleted the chore/css-test-suite branch September 18, 2023 07:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Core Area: core A-Parser Area: parser A-Tooling Area: internal tools L-CSS Language: CSS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants