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

v2 Change befaviour when working with selector #160

Open
stoivo opened this issue May 29, 2024 · 3 comments
Open

v2 Change befaviour when working with selector #160

stoivo opened this issue May 29, 2024 · 3 comments

Comments

@stoivo
Copy link
Contributor

stoivo commented May 29, 2024

I am looking at fixing a bug when paring css with selector div.not(.a, .b). The Problem is that in RuleSet we parse the selector. https://github.com/stoivo/css_parser/blob/935308c5955b57a2d826d99ae7291902deccdcd5/lib/css_parser/rule_set.rb#L453-L462

We do two different operations here. First think is to split by , which I am confident we do because a ruleset might have a selector like .one,.two which means .one or .two and when we want to calculate specificity we can do it on an or. Also we want it split so if you search for selector .one you will find it. The other operation is to replace subsequent spaces with one space. I think we do that so if someone has a stylesheet with .one .two(many spaces) we collapse it so you can search for it by .one .two (one space)

So to my problems with this and proposed solution

I think we should stop to manipulate the selectors. If someone has a style sheet with the selector .a > .b (double spaces). We shrink it into .a > .b, but the most compact way to write this selector is .a>.b, esbuild demo. We have those who have rules on multiple lines, thy would need to match the selector exactly. I think this is fine because I don't understand why people would use this to look for a specific rule.

Another issue with this stripping is that if someone has a selector like input[name="Joe Doe"] esbuild example we would remove the spaces from the selector which changes the rule which I don't think is expected.

I would like to stop doing the removing of white spaces. If users want to have there selector minifies, minify the css before passing it to css_parser. That's my suggestion. And for the splitting on ,, it's not that hard when we have the crass parser to split on comma on top level tokens.

stoivo added a commit to stoivo/css_parser that referenced this issue May 29, 2024
stoivo added a commit to stoivo/css_parser that referenced this issue May 29, 2024
@stoivo stoivo mentioned this issue May 29, 2024
1 task
@stoivo
Copy link
Contributor Author

stoivo commented May 29, 2024

I created PR #161. What do you think?

@grosser
Copy link
Contributor

grosser commented May 29, 2024

yeah just stripping everything sounds like a bad idea 👍
... sounds like worst case we emit a few extra whitespaces 🤷

@stoivo
Copy link
Contributor Author

stoivo commented May 29, 2024

Exactly. I can only think of rule like .a,.b{color:red;} where we would add two spaces, after , and after b. I think think thats bad. Also if you want to minify use a minifier like esbuild or terser

stoivo added a commit to stoivo/css_parser that referenced this issue Jun 3, 2024
stoivo added a commit to stoivo/css_parser that referenced this issue Jun 4, 2024
stoivo added a commit to stoivo/css_parser that referenced this issue Jun 12, 2024
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

2 participants