Skip comments in Ruby files when checking for class names #19243
Conversation
thecrypticace
force-pushed
the
fix/issue-19239
branch
from
bc1c637 to
fa2be75
Compare
Unless it’s part of `!important` at the end
| b'"' => { | ||
| cursor.advance(); | ||
|
|
||
| while cursor.pos < len { | ||
| match cursor.curr { | ||
| // Escaped character, skip ahead to the next character | ||
| b'\\' => cursor.advance_twice(), | ||
|
|
||
| // End of the string | ||
| b'"' => break, | ||
|
|
||
| // Everything else is valid | ||
| _ => cursor.advance(), | ||
| }; | ||
| } | ||
|
|
||
| cursor.advance(); | ||
| continue; | ||
| } | ||
|
|
||
| b'\'' => { | ||
| cursor.advance(); | ||
|
|
||
| while cursor.pos < len { | ||
| match cursor.curr { | ||
| // Escaped character, skip ahead to the next character | ||
| b'\\' => cursor.advance_twice(), | ||
|
|
||
| // End of the string | ||
| b'\'' => break, | ||
|
|
||
| // Everything else is valid | ||
| _ => cursor.advance(), | ||
| }; | ||
| } | ||
|
|
||
| cursor.advance(); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Can we add some tests where we handle quotes? We have to be careful with these because it could be that you start a sentence with a ' in it and never close it.
%p a quote here can't exist, or can it
-#
This won't be displayed (there is a quote here as well)
Nor will this
Nor will this.
%p a quote here can't exist, or can itThere was a problem hiding this comment.
I've added tests and updated the implementation a bit
WalkthroughThis pull request modifies the property extraction and preprocessing logic in the Oxide crate. The arbitrary property machine parser adds handling for top-level exclamation marks, introducing a new Pre-merge checks❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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
| if cursor.input[cursor.pos..].starts_with(b"!important]") { | ||
| cursor.advance_by(10); | ||
|
|
||
| return self.done(self.start_pos, cursor); | ||
| } | ||
|
|
||
| return self.restart(); | ||
| } |
There was a problem hiding this comment.
Handle !important case-insensitively.
starts_with(b"!important]") now rejects valid ASCII-insensitive variants like [color:red!IMPORTANT], which CSS accepts and the prior implementation extracted. Please keep the comparison case-insensitive so we don’t regress these inputs.
Apply this diff to make the comparison ASCII-insensitive:
- if cursor.input[cursor.pos..].starts_with(b"!important]") {
- cursor.advance_by(10);
+ const IMPORTANT: &[u8; 10] = b"!important";
+ let remaining = &cursor.input[cursor.pos..];
+
+ if remaining.len() >= IMPORTANT.len() + 1
+ && remaining[IMPORTANT.len()] == b']'
+ && remaining[..IMPORTANT.len()]
+ .iter()
+ .zip(IMPORTANT)
+ .all(|(byte, expected)| byte.to_ascii_lowercase() == *expected)
+ {
+ cursor.advance_by(IMPORTANT.len());Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In crates/oxide/src/extractor/arbitrary_property_machine.rs around lines
237-244, the code currently uses starts_with(b"!important]") which is
case-sensitive; replace it with an ASCII-insensitive check by first ensuring the
remaining slice has at least 10 bytes (use get(..10) or a bounds check), then
compare each byte lowercased (byte.to_ascii_lowercase()) against the literal
b"!important]" (or lowercased literal) — if they all match, advance by 10 and
call done(...), otherwise restart; this preserves ASCII case-insensitivity and
avoids panics on short slices.
Fixes #19239