-
Notifications
You must be signed in to change notification settings - Fork 73
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: parse_path failed under windows #1571
Conversation
Walkthrough此次更改对 Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- crates/mako/src/ast/file.rs (3 hunks)
Additional comments not posted (2)
crates/mako/src/ast/file.rs (2)
335-360
: 审查parse_path
函数的更改此函数现在根据操作系统调整路径格式,以支持 Windows 路径。使用
Url
来解析查询字符串是一个合理的选择,可以简化代码逻辑。建议在处理路径和查询字符串之前添加更多的注释,以提高代码的可读性。
388-396
: 审查test_parse_path_support_windows
测试用例此测试用例旨在验证
parse_path
函数对 Windows 路径的处理。测试结构良好,检查了路径、搜索、参数和片段的正确性。建议添加更多边缘情况测试,例如包含多个查询参数或特殊字符的路径。
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1571 +/- ##
==========================================
+ Coverage 61.92% 62.00% +0.08%
==========================================
Files 127 127
Lines 15311 15344 +33
==========================================
+ Hits 9481 9514 +33
Misses 5830 5830 ☔ View full report in Codecov by Sentry. |
crates/mako/src/ast/file.rs
Outdated
// so we need to decode it, e.g. "a%20b" -> "a b" | ||
let path = percent_decode_str(&path).decode_utf8()?; | ||
Ok((path.to_string(), search, query_vec, fragment)) | ||
let path = if env::consts::OS == "windows" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
不要用运行时判断吧,用 cfg 条件编译
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- crates/mako/src/ast/file.rs (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- crates/mako/src/ast/file.rs
CI 的 E2E 也加上 windows-latest 吧 |
should be done in seperate pr. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- crates/mako/src/ast/file.rs (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- crates/mako/src/ast/file.rs
e.g. builld with-antd
Summary by CodeRabbit
新功能
测试