Fix template overwrite when filename casing differs#1049
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded@chhoumann has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 13 minutes and 49 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis PR fixes a bug where template choices could not overwrite existing files when the file path had different casing. It introduces case-insensitive file resolution logic that searches the vault when direct path lookup fails, and adds corresponding test coverage for this scenario. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/engine/TemplateChoiceEngine.ts (1)
206-226: Consider enhancing error details and adding documentation.The case-insensitive resolution logic correctly addresses the bug. However, two optional improvements would help maintainability:
- Add JSDoc: Document the method's behavior, especially the case-insensitive fallback and multiple-match handling.
- Improve error message: When multiple matches are found (line 220-222), consider logging the matched paths to aid debugging.
🔎 Suggested enhancements
+ /** + * Resolves an existing file by path, with case-insensitive fallback. + * On case-insensitive filesystems, adapter.exists may return true even when + * Obsidian's path index can't resolve the file due to casing differences. + * + * @param filePath - The file path to resolve + * @returns The resolved TFile, or null if not found or ambiguous + */ private findExistingFile(filePath: string): TFile | null { const direct = this.app.vault.getAbstractFileByPath(filePath); if (direct instanceof TFile) return direct; if (direct) return null; // On case-insensitive filesystems, adapter.exists can return true even when // Obsidian's case-sensitive path index can't resolve the file. const lowerPath = filePath.toLowerCase(); const matches = this.app.vault .getFiles() .filter((file) => file.path.toLowerCase() === lowerPath); if (matches.length === 1) return matches[0]; if (matches.length > 1) { + const matchedPaths = matches.map((f) => f.path).join(", "); log.logError( - `Multiple files match '${filePath}' when ignoring case.`, + `Multiple files match '${filePath}' when ignoring case: ${matchedPaths}`, ); } return null; }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/engine/TemplateChoiceEngine.notice.test.ts(4 hunks)src/engine/TemplateChoiceEngine.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Source code lives in
src/: core logic underengine/,services/, andutils/; Svelte UI insrc/gui; shared types insrc/types; settings entry insrc/quickAddSettingsTab.ts
Files:
src/engine/TemplateChoiceEngine.tssrc/engine/TemplateChoiceEngine.notice.test.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: Biome enforces tab indentation (width 2), LF endings, and an 80-character line guide; align editor settings
Use camelCase for variables and functions
Prefer type-only imports in TypeScript files
Route logging through theloggerutilities for consistent output
Structure production code so Obsidian dependencies are injected behind interfaces; unit tests target pure logic and swap in adapters ortests/obsidian-stub.ts
Files:
src/engine/TemplateChoiceEngine.tssrc/engine/TemplateChoiceEngine.notice.test.ts
src/**/*.{ts,tsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
Use PascalCase for classes and Svelte components
Files:
src/engine/TemplateChoiceEngine.tssrc/engine/TemplateChoiceEngine.notice.test.ts
🧠 Learnings (3)
📚 Learning: 2025-12-09T21:20:52.425Z
Learnt from: CR
Repo: chhoumann/quickadd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T21:20:52.425Z
Learning: Applies to src/**/*.{ts,tsx} : Structure production code so Obsidian dependencies are injected behind interfaces; unit tests target pure logic and swap in adapters or `tests/obsidian-stub.ts`
Applied to files:
src/engine/TemplateChoiceEngine.notice.test.ts
📚 Learning: 2025-12-09T21:20:52.425Z
Learnt from: CR
Repo: chhoumann/quickadd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T21:20:52.425Z
Learning: Applies to tests/**/*.{ts,tsx} : Add regression coverage for bug fixes
Applied to files:
src/engine/TemplateChoiceEngine.notice.test.ts
📚 Learning: 2025-12-09T21:20:52.425Z
Learnt from: CR
Repo: chhoumann/quickadd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T21:20:52.425Z
Learning: Applies to src/**/*.{ts,tsx,js,jsx} : Source code lives in `src/`: core logic under `engine/`, `services/`, and `utils/`; Svelte UI in `src/gui`; shared types in `src/types`; settings entry in `src/quickAddSettingsTab.ts`
Applied to files:
src/engine/TemplateChoiceEngine.notice.test.ts
🧬 Code graph analysis (2)
src/engine/TemplateChoiceEngine.ts (2)
tests/obsidian-stub.ts (1)
TFile(332-338)src/logger/logManager.ts (1)
log(59-59)
src/engine/TemplateChoiceEngine.notice.test.ts (3)
src/settingsStore.ts (1)
settingsStore(8-21)tests/obsidian-stub.ts (1)
TFile(332-338)src/constants.ts (1)
fileExistsOverwriteFile(168-168)
🔇 Additional comments (7)
src/engine/TemplateChoiceEngine.ts (2)
95-95: LGTM! Correct usage of the new helper.The switch from direct
getAbstractFileByPathtofindExistingFilecorrectly addresses the case-insensitive filesystem issue.
101-101: LGTM! Clearer error message.The updated message better explains why the operation failed (file exists but couldn't be resolved as the expected type).
src/engine/TemplateChoiceEngine.notice.test.ts (5)
101-101: LGTM! Import needed for test instantiation.The
TFileclass import (not just the type) is required to construct test instances at line 293.
108-108: LGTM! Import needed for test scenario.The
fileExistsOverwriteFileconstant is used at line 299 to configure the test case.
161-161: LGTM! Mock supports case-insensitive fallback.The
getFilesmock is needed for the new case-insensitive search logic infindExistingFile.
198-198: LGTM! Exposing app enables test configuration.Returning
appfrom the helper allows tests to configure vault mocks for specific scenarios.
279-332: Excellent regression test coverage!This test correctly exercises the exact bug scenario from issue #599:
- File exists on filesystem with one casing ("Bug report.md")
- Template choice uses different casing ("Bug Report")
- Direct path lookup fails (simulated by
getAbstractFileByPathreturning null)- Case-insensitive fallback finds the existing file
- Overwrite operation proceeds successfully
The test setup properly mocks all three conditions needed to reproduce the bug. This aligns with the learning that regression coverage should be added for bug fixes.
Based on learnings, regression coverage for bug fixes is expected.
# [2.10.0](2.9.4...2.10.0) (2026-01-28) ### Bug Fixes * add insert-after blank-line mode ([#1056](#1056)) ([231e908](231e908)) * backfill file opening defaults for legacy choices ([bfffd46](bfffd46)) * default update announcements to major ([#1042](#1042)) ([4e5659b](4e5659b)) * handle template overwrite with case-mismatched paths ([#1049](#1049)) ([0140556](0140556)) * harden suggester display items ([5460657](5460657)) * harden suggester filtering on Android ([4560f22](4560f22)), closes [#1078](#1078) * normalize leading slashes in capture/template paths ([#1050](#1050)) ([1c7def1](1c7def1)) * normalize unicode in file suggestions ([#1046](#1046)) ([10c0402](10c0402)) * pin obsidian types to 1.11.4 for SecretStorage API ([ddbf6f6](ddbf6f6)) * prefill macro rename input ([#1043](#1043)) ([06c4a25](06c4a25)) * preserve blank lines for insert after ([#1054](#1054)) ([818c036](818c036)) * preserve variables for VALUE templating ([36d43ba](36d43ba)) * reduce bundle size below sync limit ([1e1a632](1e1a632)) * restore compatibility with Templater 2.18.0 ([716f2d9](716f2d9)), closes [#1085](#1085) [#1086](#1086) ### Features * add back class to choice suggester ([#1047](#1047)) ([4c8cfe9](4c8cfe9)) * add capture selection-as-value controls ([#1055](#1055)) ([250768a](250768a)) * add inline insert-after capture mode ([b2e1ef5](b2e1ef5)) * add macro selection helper ([786b53c](786b53c)), closes [#483](#483) * add native date picker prompt ([2811c5a](2811c5a)) * add per-token multiline VALUE inputs ([98fa7db](98fa7db)), closes [#339](#339) * add versioned documentation with Docusaurus ([03c2d3e](03c2d3e)) * adopt obsidian 1.11 settings APIs ([#1041](#1041)) ([15c4b34](15c4b34)) * default multi placeholder to name ([fcd058f](fcd058f)) * enhance template folder chooser ([f1e2a9f](f1e2a9f)), closes [#1011](#1011) [#1012](#1012) * improve prompt labeling for VALUE/MACRO and multi choices ([78fd184](78fd184)) * persist input prompt drafts on cancel/escape ([#1044](#1044)) ([62a67f4](62a67f4)) * store AI provider keys in SecretStorage ([4559013](4559013)) * support short-form date aliases ([e04a3f6](e04a3f6))
|
🎉 This PR is included in version 2.10.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Summary
Testing
Fixes #599
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.