search

dart-lang / csslib

A library for parsing CSS.
https://pub.dev/packages/csslib
BSD 3-Clause "New" or "Revised" License
95 stars 38 forks source link

Fix upper cases in media expressions, fix test failures related to platform-dependent error output with glyphs, fix failure to comma-separate keyframes percentage list #140

Closed zzwx closed 2 years ago

zzwx commented 3 years ago

Attempt 2, taking into account that some platforms may have unicode glyphs to describe the source spans, and some may not.

zzwx commented 3 years ago

Trying to fix dart-lang/tools#1172 as well. I guess I'm not correctly creating pull requests

zzwx commented 2 years ago

I've tested it on both Windows and Linux, tests don't fail on both. I'm hoping you'll give it a green light to run the checks here.

natebosch commented 2 years ago

Thanks for the PR!

We prefer to keep refactoring and other fixes in separate PRs from the main fix. I also think we probably want to disable ascii glyphs for all the tests instead of patching the expectations. I'll start looking at that in https://github.com/dart-lang/csslib/pull/142

zzwx commented 2 years ago

Thanks for the PR!

We prefer to keep refactoring and other fixes in separate PRs from the main fix. I also think we probably want to disable ascii glyphs for all the tests instead of patching the expectations. I'll start looking at that in dart-lang/csslib#142

Sure, as long as it fixes the bugs!

natebosch commented 2 years ago

@zzwx - I don't have rights to push to your fork (when you open a PR with the same account that owns the repository, it by default allows pushes from the repo maintainers, but using a separate account for the repo may have interefered with that).

https://github.com/dart-lang/csslib/commit/4140cca8c10946b1c3ef26a52069a61cb9b0c51a has the merge if you want to bring it to your PR branch.

zzwx commented 2 years ago

Will this allow you to resolve the conflicts? What a mess I have made :)

natebosch commented 2 years ago

Will this allow you to resolve the conflicts? What a mess I have made :)

If it's feeling like a mess I'm happy to take over 😄

I can start this in a new PR based on this one so that I can merge with the conflicts resolved.

natebosch commented 2 years ago

Follow along in https://github.com/dart-lang/csslib/pull/143