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

Allow empty argument in functional pseudos #220

Merged

Conversation

bramus
Copy link
Contributor

@bramus bramus commented Dec 15, 2022

Fixes #219.

Marking this PR as draft for now, as I’m not sure what to do nth-child and its variants. Should relaxed parsing – allowing an empty argument – also be allowed there? If so, it’s a matter of changing the fixture to not mark "" as invalid.

@lahmatiy
Copy link
Member

@bramus

If so, it’s a matter of changing the fixture to not mark "" as invalid.

A failed test is a little weird. It tests parsing <an-plus-b> but through :nth-child() pseudo. So empty string is invalid for <an-plus-b>, but that's fine for :nth-child() content. I think you may change the test to ignore empty string case in fixture.invalid.forEach block.

Everything else looks good to me 👍

@bramus bramus marked this pull request as ready for review December 15, 2022 21:21
@coveralls
Copy link

coveralls commented Dec 15, 2022

Coverage Status

Coverage increased (+0.0004%) to 98.932% when pulling 96d9ded on bramus:allow-empty-argument-in-functional-pseudos into 593bf37 on csstree:master.

@bramus
Copy link
Contributor Author

bramus commented Dec 15, 2022

Updated tests to reflect resolution as per #220 (comment)

@lahmatiy lahmatiy merged commit 4d55558 into csstree:master Dec 19, 2022
@lahmatiy
Copy link
Member

@bramus Thank you!

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.

Empty argument pseudos such as :has() throw
3 participants