-
Notifications
You must be signed in to change notification settings - Fork 5
[ENG-1072] Error "Folder already exists" when create new canvas #601
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
[ENG-1072] Error "Folder already exists" when create new canvas #601
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
22df988 to
543550e
Compare
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
📝 WalkthroughWalkthroughThe changes enhance path handling consistency by introducing whitespace trimming in the GeneralSettings save operation for folder paths and implementing path normalization in the file utilities module before operations are performed. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Pre-merge checks✅ Passed checks (3 passed)
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/obsidian/src/utils/file.ts (1)
3-12: Good call normalizing paths before vault operations; consider trimming defensivelyUsing
normalizePathonce and reusingnormalizedPathfor bothgetAbstractFileByPathandcreateFolderis a solid improvement and should prevent “folder already exists” errors when users enter equivalent-but-different paths (e.g. extra slashes).You could additionally guard against future callers passing whitespace-only values by trimming first, e.g.:
const trimmed = folderpath.trim(); if (!trimmed) return; const normalizedPath = normalizePath(trimmed);so these are treated like “no folder” rather than trying to create a folder named with stray spaces.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/obsidian/src/components/GeneralSettings.tsx(1 hunks)apps/obsidian/src/utils/file.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/main.mdc)
**/*.{ts,tsx}: Use Tailwind CSS for styling where possible
When refactoring inline styles, use tailwind classes
Prefertypeoverinterfacein TypeScript
Use explicit return types for functions
Avoidanytypes when possible
Prefer arrow functions over regular function declarations
Use named parameters (object destructuring) when a function has more than 2 parameters
Use PascalCase for components and types
Use camelCase for variables and functions
Use UPPERCASE for constants
Function names should describe their purpose clearly
Prefer early returns over nested conditionals for better readability
Files:
apps/obsidian/src/utils/file.tsapps/obsidian/src/components/GeneralSettings.tsx
apps/obsidian/**
📄 CodeRabbit inference engine (.cursor/rules/obsidian.mdc)
apps/obsidian/**: Prefer existing dependencies from apps/obsidian/package.json when adding dependencies to the Obsidian plugin
Follow the Obsidian style guide from help.obsidian.md/style-guide and docs.obsidian.md/Developer+policies for UI and code styling
Use Lucide and custom Obsidian icons alongside detailed elements to provide visual representation of features in platform-native UI
Files:
apps/obsidian/src/utils/file.tsapps/obsidian/src/components/GeneralSettings.tsx
The error was caused by Obsidian internally normalize the folder path when we call
vault.createFolder(). eg: if we haveDiscourse Canvas/saved, then when normalized we haveDiscourse Canvas, which already exists. I made sure to normalize the name in both Saving and Creating step -> remove the error.Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.