Extend CSS check to CSS variables #101813
Conversation
rustbot
added
the
T-rustdoc
rust-highfive
added
the
S-waiting-on-review
GuillaumeGomez
force-pushed
the
check-css-variables
branch
from
fd7ef0f
to
e7d8ad6
Compare
There was a problem hiding this comment.
More generally, I have concerns about producing non-rustdoc-specific warnings from the tool as it's written.
-
Good linters have support for localized silencing, like the
#[allow()]directives in Rust itself. There's none of that here. -
It's also fairly likely, since this parser hasn't really been tested against a production-grade battery of CSS test suite corner cases, that there's bugs in here.
For example, you can totally include semicolons and curley braces in
url({;}.png)without quotes. It looks like this parser tokenizes it incorrectly.
As long as it's just scanning for variables and selectors, it's probably okay, but if it's going to start producing more broadly-applicable warnings, it becomes more important to minimize false positives.
|
In other words, I'm happy with checking CSS variables are filled in with all the data rustdoc needs. I just don't want scope creep from adding other warnings. |
|
I added them without thinking. Removing them. |
Yes, bugs are very likely but the goal isn't to support the CSS syntax faithfully so we should be mostly fine on this part. As for why I didn't use an existing CSS parser: either they're way too big (in term of deps) or they provide way too advanced features, making it complicated to just query the information we want. But if you know one that isn't too big and covers just what we need, it'd be perfect! |
|
I added an extra handling for parens which fixes the issue you mentioned. |
|
@bors r=notriddle |
bors
added
S-waiting-on-bors
…llaumeGomez Rollup of 7 pull requests Successful merges: - rust-lang#101494 (rustdoc mobile: move notable traits to return type) - rust-lang#101813 (Extend CSS check to CSS variables) - rust-lang#101825 (Fix back RPIT changes) - rust-lang#101843 (Suggest associated const for incorrect use of let in traits) - rust-lang#101859 (Slight vertical formatting) - rust-lang#101868 (rustdoc: use more precise URLs for jump-to-definition links) - rust-lang#101877 (rustdoc: remove no-op CSS `.block { padding: 0 }`) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
This PR is a bit big because the first commit is a rewrite of the CSS parser to something a bit simpler which still allows to get easily access to CSS properties name.
The other two are about adding tests and adding the CSS variables check.
This check was missing because we are relying more and more on CSS variables rather than CSS selectors in themes.
r? @notriddle