Preserve custom regex capture whitespace#137
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough
ChangesCustomRegex whitespace preservation fix
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
| Filename | Overview |
|---|---|
| rustywind-core/src/app.rs | Adds sort_capture_classes dispatch and sort_classes_preserving_outer_whitespace helper so custom-regex captures retain their leading/trailing whitespace; also adds a regression test for the Ruby interpolation spacing bug. Logic is correct; one minor edge-case note about the unwrap_or fallback. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["sort_file_contents(file_contents)"] --> B["regex.replace_all — for each match"]
B --> C["Extract capture group\n(classes &str)"]
C --> D["sort_capture_classes(classes)"]
D --> E{FinderRegex\ntype?}
E -- CustomRegex --> F["sort_classes_preserving_outer_whitespace(classes)"]
E -- DefaultRegex --> G["sort_classes(classes)"]
F --> H{Any non-whitespace\ncharacters?}
H -- No --> I["Return original string unchanged"]
H -- Yes --> J["Find leading whitespace boundary (start)\nFind trailing whitespace boundary (end)"]
J --> K["sort_classes(class_string[start..end])"]
K --> L["Reassemble:\nleading_ws + sorted + trailing_ws"]
G --> M["split_class_tokens → sort → join(' ')"]
L --> N["caps[0].replace(classes, sorted_classes)"]
M --> N
Reviews (1): Last reviewed commit: "Preserve custom regex capture whitespace" | Re-trigger Greptile
| let end = class_string | ||
| .char_indices() | ||
| .rev() | ||
| .find(|(_, character)| !character.is_ascii_whitespace()) | ||
| .map(|(index, character)| index + character.len_utf8()) | ||
| .unwrap_or(start); |
There was a problem hiding this comment.
Unreachable fallback value may be confusing
The outer let Some((start, _)) guard guarantees at least one non-ASCII-whitespace character exists, so the reverse .find(...) will always succeed and .unwrap_or(start) is never reached. Using start as the fallback would silently produce a zero-length slice (class_string[start..start]) if it were ever hit, which would be a subtle bug. Consider replacing with .expect("at least one non-whitespace char was found by the forward scan") to make the invariant explicit.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| fn sort_classes_preserving_outer_whitespace(&self, class_string: &str) -> String { | ||
| let Some((start, _)) = class_string | ||
| .char_indices() | ||
| .find(|(_, character)| !character.is_ascii_whitespace()) | ||
| else { | ||
| return class_string.to_string(); | ||
| }; | ||
|
|
||
| let end = class_string | ||
| .char_indices() | ||
| .rev() | ||
| .find(|(_, character)| !character.is_ascii_whitespace()) | ||
| .map(|(index, character)| index + character.len_utf8()) | ||
| .unwrap_or(start); | ||
|
|
||
| format!( | ||
| "{}{}{}", | ||
| &class_string[..start], | ||
| self.sort_classes(&class_string[start..end]), | ||
| &class_string[end..] | ||
| ) | ||
| } |
There was a problem hiding this comment.
Inner whitespace is still collapsed for custom regexes
sort_classes_preserving_outer_whitespace preserves only the leading and trailing whitespace of the capture, but the call to sort_classes inside will normalise any interior whitespace (e.g. "flex p-4" → "flex p-4") because split_class_tokens splits on whitespace and rewrap_wrapped_classes rejoins with single spaces. For the primary bug (Ruby interpolation trailing space) this is not a problem, but it could silently reformat captures from custom regexes that intentionally contain multiple consecutive spaces. A short doc-comment explaining this deliberate limitation would prevent future surprises.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Fixes #124.
Summary
? "ok"is not rewritten to?"ok".Validation
cargo test -p rustywind_core test_custom_regex_preserves_ruby_interpolation_spacingcargo fmt --checkcargo test --workspacecargo test --workspace --all-featurescargo clippy --workspace -- -D warningsSummary by CodeRabbit
Bug Fixes
Tests