fix: handle bare Assets/ path in manage_shader#743
Conversation
Trim(/) was stripping the trailing slash before the StartsWith check, so Assets/ became Assets which did not match Assets/. Now handles the bare Assets case explicitly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts ManageShader path handling so a bare Class diagram for updated ManageShader path handlingclassDiagram
class ManageShader {
+static object HandleCommand(JObject params)
}
class AssetPathUtility {
+static string NormalizeSeparators(string path)
}
ManageShader ..> AssetPathUtility : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughThe change modifies shader path normalization in ManageShader.cs to explicitly handle the exact "Assets" path (case-insensitive) by treating it as the root directory, while preserving existing behavior for paths starting with "Assets/". Added explicit braces for code clarity. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Poem
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider centralizing the "Assets" and "Assets/" literals into a shared constant or helper to avoid duplication and make future path handling changes less error-prone.
- You might simplify the path logic by handling the "Assets" prefix in one place (e.g., checking StartsWith("Assets", ...) and branching based on length and following separator) instead of having separate equality and StartsWith checks.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider centralizing the "Assets" and "Assets/" literals into a shared constant or helper to avoid duplication and make future path handling changes less error-prone.
- You might simplify the path logic by handling the "Assets" prefix in one place (e.g., checking StartsWith("Assets", ...) and branching based on length and following separator) instead of having separate equality and StartsWith checks.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Trim(/) was stripping the trailing slash before the StartsWith check, so Assets/ became Assets which did not match Assets/. Now handles the bare Assets case explicitly. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Trim(/) was stripping the trailing slash before the StartsWith check, so Assets/ became Assets which did not match Assets/. Now handles the bare Assets case explicitly. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Summary
Test plan
Summary by Sourcery
Bug Fixes:
AssetsorAssets/paths so shaders are no longer created under an unintendedAssets/Assets/subdirectory.Summary by CodeRabbit