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

Handling escape symbols #93

Closed
ai opened this issue Dec 19, 2019 · 9 comments
Closed

Handling escape symbols #93

ai opened this issue Dec 19, 2019 · 9 comments

Comments

@ai
Copy link

ai commented Dec 19, 2019

Seems like we forget to add escape symbols support.

Here is an example: fo\a o should be parsed as a single token.

Here is a good description of escape symbols in CSS.

Tell me if you need anything from PostCSS core for this issue.

How Do We Reproduce?

Here is an example: fo\a o should be parsed as a single token.

Expected Behavior

Should be parsed as a single token.

Actual Behavior

Parses as multiple tokens.

@shellscape
Copy link
Owner

Thanks, I'll look into this as soon as possible.

@shellscape
Copy link
Owner

@nex3 I sat down to give this a look, but I'm going to need more context, some use cases, something real that I can write a test around. I can't look for \ being surrounded by anything in particular because that'd be too brittle.

@nex3
Copy link
Contributor

nex3 commented Apr 17, 2020

This isn't really about matching specific use cases, it's about accurately parsing according to the CSS spec.

@shellscape
Copy link
Owner

This isn't really about matching specific use cases

I'd argue the opposite. We only want to build in features that present from use cases. That prevents bloat. Minimal Viable Product very much applies here.

the next input code point has already been verified to be part of a valid escape

That's really what we're interested in there. So we're going to need a comprehensive list of valid escapes before we can move forward. From that we can build tests.

@nex3
Copy link
Contributor

nex3 commented Apr 17, 2020

I'd argue the opposite. We only want to build in features that present from use cases. That prevents bloat. Minimal Viable Product very much applies here.

The point of a parser for a well-specified format is to parse all possible documents in that format, not the 80%-use-case subset of that format. Across a wide enough swath of code, all corners of the syntax are going to be touched eventually.

If you're saying that postcss-values-parser intentionally doesn't correctly parse some valid CSS values, that makes it extremely difficult for us to justify our continued use and investment of engineering time in this package.

That's really what we're interested in there. So we're going to need a comprehensive list of valid escapes before we can move forward. From that we can build tests.

The uses of the "consume an escaped code point" algorithm describe how to verify this. They are:

@shellscape
Copy link
Owner

If you're saying that postcss-values-parser intentionally doesn't correctly parse some valid CSS values, that makes it extremely difficult for us to justify our continued use and investment of engineering time in this package.

That's the second time you've tossed that out there. I wrote this package for a need that I had, and some folks picked it up to use. I OWE YOU ABSOLUTELY NOTHING Threatening me with "we'll go elsewhere" is meaningless - it hold absolutely no meaning and does not sway me in the slightest.

Without a valid use case I won't be implementing this feature.

@nex3
Copy link
Contributor

nex3 commented Apr 17, 2020

I'm sorry, I didn't mean to level that as a threat. As an open-source maintainer, I understand how frustrating it can be for someone to say "well then I'll take my ball and go home", and I should have avoided doing that myself.

The point I was trying to make was more that we've been trying to provide value back to this package, as I hope I've demonstrated from my work on pull requests. That work benefits you and all your users, and we'd like to continue doing it, but we have some inescapable requirements—one of which is that our tools at least have the intention of following the CSS spec. As you may imagine, the set of CSS files across all of Google is vast, and we very much want to avoid any unnecessary pitfalls for authors who (reasonably) expect any standard CSS to work.

@shellscape
Copy link
Owner

I recommend that you fork or choose another project.

Repository owner locked as resolved and limited conversation to collaborators Apr 17, 2020
@jonathantneal
Copy link
Collaborator

jonathantneal commented Apr 18, 2020

Hey, all! I looked into this, and the PostCSS Tokenizer splits a string like fo\a o into 3 words. It will be important to fix this in PostCSS. Thanks for identifying this @nex3, and thanks for bringing it here as an issue, @ai.

In my limited corner of web development, the most common use case that I’ve seen for escape characters are when they are used to produce Atomic CSS libraries.

Take a look at the examples from https://acss.io/#pseudo-classes. In that section, the source for the C(brandColor) selector will appear as C\(brandColor\). That should parse as a single identifier. Similarly, the :h used to identify a style that applies on hover would put a slash before the colon in its source (as \:h).

We can validate this expectation @ https://rawgit.com/tabatkins/parse-css/master/example.html

Here is the good news. This tool extends the parser (and tokenizer) directly from PostCSS. This means that fixing the issue in PostCSS also means fixing the issue here.

We can continue the discussion and resolution in postcss/postcss#1349

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants