Skip comments in Ruby files when checking for class names#19243
Skip comments in Ruby files when checking for class names#19243thecrypticace merged 13 commits intomainfrom
Conversation
bc1c637 to
fa2be75
Compare
WalkthroughAdds an Pre-merge checks✅ Passed checks (5 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
crates/oxide/src/extractor/arbitrary_property_machine.rs(3 hunks)crates/oxide/src/extractor/pre_processors/ruby.rs(5 hunks)crates/oxide/src/extractor/pre_processors/test-fixtures/haml/dst-17051.haml(0 hunks)
💤 Files with no reviewable changes (1)
- crates/oxide/src/extractor/pre_processors/test-fixtures/haml/dst-17051.haml
🧰 Additional context used
🧬 Code graph analysis (1)
crates/oxide/src/extractor/pre_processors/ruby.rs (2)
crates/oxide/src/extractor/candidate_machine.rs (1)
next(29-169)crates/oxide/src/extractor/string_machine.rs (1)
next(32-68)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Linux
- GitHub Check: Linux / postcss
- GitHub Check: Linux / vite
- GitHub Check: Linux / upgrade
RobinMalfait
left a comment
There was a problem hiding this comment.
Looks good, but not sure about the !important.
Can you also add a changelog entry?
crates/oxide/src/extractor/pre_processors/test-fixtures/haml/dst-17051.haml
Show resolved
Hide resolved
|
Yeah I was not gonna change that part. It's unrelated anyway. Will get the change log updated. |
Unless it’s part of `!important` at the end
42f48be to
f381118
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
CHANGELOG.md(1 hunks)crates/oxide/src/extractor/arbitrary_property_machine.rs(3 hunks)crates/oxide/src/extractor/pre_processors/ruby.rs(5 hunks)crates/oxide/src/extractor/pre_processors/test-fixtures/haml/dst-17051.haml(0 hunks)
💤 Files with no reviewable changes (1)
- crates/oxide/src/extractor/pre_processors/test-fixtures/haml/dst-17051.haml
🧰 Additional context used
🧬 Code graph analysis (1)
crates/oxide/src/extractor/pre_processors/ruby.rs (2)
crates/oxide/src/extractor/candidate_machine.rs (1)
next(29-169)crates/oxide/src/extractor/string_machine.rs (1)
next(32-68)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Linux
- GitHub Check: Linux / cli
- GitHub Check: Linux / upgrade
- GitHub Check: Linux / postcss
- GitHub Check: Linux / vite
- GitHub Check: Linux / webpack
This PR contains the following updates: | Package | Change | Age | Confidence | |---|---|---|---| | [@tailwindcss/postcss](https://tailwindcss.com) ([source](https://github.com/tailwindlabs/tailwindcss/tree/HEAD/packages/@tailwindcss-postcss)) | [`4.1.17` -> `4.1.18`](https://renovatebot.com/diffs/npm/@tailwindcss%2fpostcss/4.1.17/4.1.18) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>tailwindlabs/tailwindcss (@​tailwindcss/postcss)</summary> ### [`v4.1.18`](https://github.com/tailwindlabs/tailwindcss/blob/HEAD/CHANGELOG.md#4118---2025-12-11) [Compare Source](tailwindlabs/tailwindcss@v4.1.17...v4.1.18) ##### Fixed - Ensure validation of `source(…)` happens relative to the file it is in ([#​19274](tailwindlabs/tailwindcss#19274)) - Include filename and line numbers in CSS parse errors ([#​19282](tailwindlabs/tailwindcss#19282)) - Skip comments in Ruby files when checking for class names ([#​19243](tailwindlabs/tailwindcss#19243)) - Skip over arbitrary property utilities with a top-level `!` in the value ([#​19243](tailwindlabs/tailwindcss#19243)) - Support environment API in `@tailwindcss/vite` ([#​18970](tailwindlabs/tailwindcss#18970)) - Preserve case of theme keys from JS configs and plugins ([#​19337](tailwindlabs/tailwindcss#19337)) - Write source maps correctly on the CLI when using `--watch` ([#​19373](tailwindlabs/tailwindcss#19373)) - Handle special defaults (like `ringColor.DEFAULT`) in JS configs ([#​19348](tailwindlabs/tailwindcss#19348)) - Improve backwards compatibility for `content` theme key from JS configs ([#​19381](tailwindlabs/tailwindcss#19381)) - Upgrade: Handle `future` and `experimental` config keys ([#​19344](tailwindlabs/tailwindcss#19344)) - Try to canonicalize any arbitrary utility to a bare value ([#​19379](tailwindlabs/tailwindcss#19379)) - Validate candidates similarly to Oxide ([#​19397](tailwindlabs/tailwindcss#19397)) - Canonicalization: combine `text-*` and `leading-*` classes ([#​19396](tailwindlabs/tailwindcss#19396)) - Correctly handle duplicate CLI arguments ([#​19416](tailwindlabs/tailwindcss#19416)) - Don’t emit color-mix fallback rules inside `@keyframes` ([#​19419](tailwindlabs/tailwindcss#19419)) - CLI: Don't hang when output is `/dev/stdout` ([#​19421](tailwindlabs/tailwindcss#19421)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0Mi4xNC4yIiwidXBkYXRlZEluVmVyIjoiNDIuMTQuMiIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOltdfQ==--> Reviewed-on: https://git.csmpro.ru/csmpro/mapban/pulls/74 Co-authored-by: Renovate Bot <renovate@csmpro.ru> Co-committed-by: Renovate Bot <renovate@csmpro.ru>
Fixes: #19481 This PR improves the Ruby extractor to better handle strict locals. We recently introduced skipping comments in the Ruby extractor (PR #19243 for #19239) by ignoring comments that start with `#` until the end of the line. Strict locals are implemented like this: ```ruby <%# locals: (css: "text-amber-600") %> ``` Notice the `#` after the `<%`, we considered this a comment and ignored it. This PR changes that behavior slightly where we skip comments that are preceded by `%`. This means that `<%# anything here _will_ be scanned %>`. This should solve the strict locals case, and normal comments will still be skipped. We can be more strict in the future if needed, but I think that this should be a good solution for both scenarios. ### Test plan 1. Added a test to ensure we extract candidates in strict locals 2. Added a regression test for issue #19239 where we introduced skipping comments in the Ruby extractor 3. Other existing tests are still passing We can also verify the extracted candidates: (it's subtle, but you can see that the class is being extracted now) <img width="1187" height="1376" alt="image" src="https://github.com/user-attachments/assets/74bbfd79-9db4-4a5b-bd8d-25f1565c6bfd" />
Fixes #19239