Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds a current-folder suggestion in TemplateChoiceEngine and replaces single-path folder handling with an interactive folder selection flow in TemplateEngine that validates, optionally creates, and restricts folder choices; also adds path-validation utilities for folder names and reserved Windows names. Changes
Sequence DiagramsequenceDiagram
autonumber
participant User
participant TemplateChoiceEngine
participant TemplateEngine
participant InputSuggester
participant Vault
User->>TemplateChoiceEngine: start create-from-template
TemplateChoiceEngine->>TemplateChoiceEngine: getCurrentFolderSuggestion()
TemplateChoiceEngine->>TemplateEngine: getOrCreateFolder(folders, { topItems, allowCreate, allowedRoots })
TemplateEngine->>TemplateEngine: buildFolderSuggestions(topItems, folders, allowedRoots)
TemplateEngine->>InputSuggester: show suggestions & input
User->>InputSuggester: select suggestion or type path
InputSuggester-->>TemplateEngine: selectedOrTypedPath
alt selection empty or invalid
TemplateEngine->>InputSuggester: show notice / reprompt
else path allowed & exists
TemplateEngine->>Vault: resolve canonical path
Vault-->>TemplateEngine: canonical path
TemplateEngine-->>TemplateChoiceEngine: return path
else path allowed & missing (allowCreate)
TemplateEngine->>Vault: create missing folders
Vault-->>TemplateEngine: created path
TemplateEngine-->>TemplateChoiceEngine: return path
else user cancels
TemplateEngine-->>TemplateChoiceEngine: throw MacroAbortError
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (5)**/*.{ts,tsx,mts,mjs,js,json}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx,svelte}📄 CodeRabbit inference engine (AGENTS.md)
Files:
src/**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
src/{engine,services,utils}/**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (1)📚 Learning: 2025-12-21T07:54:34.863ZApplied to files:
🔇 Additional comments (8)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/engine/TemplateEngine.ts (3)
136-138: Clarify folder creation behavior whenallowCreateis false.When
allowCreateis false, the code still callscreateFolderon line 137. This appears to ensure folders from the suggestion list exist on disk, but it may be confusing sinceallowCreatesemantically suggests no creation should occur.Consider either:
- Adding a comment explaining this creates existing list items vs. user-typed new paths
- Renaming
allowCreatetoallowUserCreationorallowTypedFolderCreationfor clarity
183-191: Consider edge case with empty root.Line 186 returns
trueif any root is an empty string, effectively allowing all paths. This seems intentional for vault root (""), but if an empty string accidentally entersallowedRoots, it silently disables all restrictions.Consider adding a filter when constructing
normalizedAllowedRootsto exclude empty strings if vault-root should be explicitly opted into:const normalizedAllowedRoots = options.allowedRoots?.map((root) => this.normalizeFolderPath(root), -); +).filter((root) => root !== "");
111-118: Consider the "No folder selected" error path.Line 111 throws a plain
ErrorwhenfolderPathis falsy. IfInputSuggester.Suggestreturns an empty string (not by cancellation), this error propagates up as a generic error rather than aMacroAbortError.If an empty selection should abort the macro gracefully, consider throwing
MacroAbortErrordirectly:- if (!folderPath) throw new Error("No folder selected."); + if (!folderPath) throw new MacroAbortError("No folder selected");
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/engine/TemplateChoiceEngine.tssrc/engine/TemplateEngine.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,mts,mjs,js,json}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,mts,mjs,js,json}: Use tab indentation with width 2 in TypeScript and configuration files (enforced by Biome).
Follow an 80-character line guide (enforced by Biome).
Files:
src/engine/TemplateChoiceEngine.tssrc/engine/TemplateEngine.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use camelCase for variables and functions in TypeScript.
Prefer type-only imports in TypeScript.
Files:
src/engine/TemplateChoiceEngine.tssrc/engine/TemplateEngine.ts
**/*.{ts,tsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
Use PascalCase for classes and Svelte components.
Files:
src/engine/TemplateChoiceEngine.tssrc/engine/TemplateEngine.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Route logging through the
loggerutilities for consistent output.
Files:
src/engine/TemplateChoiceEngine.tssrc/engine/TemplateEngine.ts
src/{engine,services,utils}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Structure production code so Obsidian dependencies are injected behind interfaces to enable unit testing without loading real Obsidian modules.
Files:
src/engine/TemplateChoiceEngine.tssrc/engine/TemplateEngine.ts
🧠 Learnings (1)
📚 Learning: 2025-12-21T07:54:34.863Z
Learnt from: CR
Repo: chhoumann/quickadd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T07:54:34.863Z
Learning: Applies to src/**/*.ts src/**/*.tsx src/**/*.svelte src/engine/** src/services/** src/utils/** src/gui/** src/types/** src/quickAddSettingsTab.ts tests/** docs/** : 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`. Bundled artifacts `main.js` and `styles.css` stay at the repo root and should be generated, not hand-edited. Place tests and stubs in `tests/`, and keep user-facing docs in `docs/`.
Applied to files:
src/engine/TemplateChoiceEngine.ts
🔇 Additional comments (9)
src/engine/TemplateChoiceEngine.ts (3)
305-317: LGTM!Clean implementation of the current folder suggestion. The null check on
activeFileandparentis appropriate, and the display logic correctly shows "/" for root folder context.
251-252: LGTM!Good pattern for deriving
topItemsfrom the current folder suggestion, with proper null handling.
267-280: LGTM!Consistent propagation of options to
getOrCreateFolder. TheallowedRootsis correctly included for subfolder selection (enforcing configured scope) and correctly omitted forchooseWhenCreatingNote(allowing vault-wide selection).src/engine/TemplateEngine.ts (6)
22-27: LGTM!Well-structured options type with clear purpose for each property.
179-181: LGTM!Clean path normalization that handles common input variations.
193-200: LGTM!Good UX pattern with list truncation and clear user feedback via
Notice.
202-248: LGTM!Well-structured suggestion builder with proper deduplication. The
topItems-first ordering ensures current folder gets priority display. The parallel data structures are necessary for supporting distinct display labels.
250-270: LGTM!Clean rendering logic with appropriate priority for custom labels, existing folders, and new folder creation prompts.
160-176: LGTM!The single-item path handles the common case efficiently. The folder creation for pre-configured paths (even when
allowCreateis false) ensures the selection list items actually exist on disk.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/engine/TemplateEngine.ts (1)
58-177: Consider adding inline comments to clarify the allowCreate semantics.The
getOrCreateFoldermethod implements complex logic with multiple responsibilities: building suggestions, user selection, path validation, root constraint enforcement, and folder creation. TheallowCreateflag has nuanced behavior:
false: User selects from predefined folders; ensures they existtrue: User can type new folder paths; validates against roots and creates if allowedWhile the implementation appears correct, inline comments explaining these semantics and the retry loop would improve maintainability.
Optional: Consider extracting sub-methods to reduce complexity
The method could be decomposed into smaller helper methods:
promptForFolder(): Handle user selection via suggestervalidateAndCreateFolder(): Validate path against roots and create if neededhandleSingleFolderCase(): Process single-item logicThis would make the main flow easier to follow, though the current implementation is functional.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/engine/TemplateEngine.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,mts,mjs,js,json}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,mts,mjs,js,json}: Use tab indentation with width 2 in TypeScript and configuration files (enforced by Biome).
Follow an 80-character line guide (enforced by Biome).
Files:
src/engine/TemplateEngine.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use camelCase for variables and functions in TypeScript.
Prefer type-only imports in TypeScript.
Files:
src/engine/TemplateEngine.ts
**/*.{ts,tsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
Use PascalCase for classes and Svelte components.
Files:
src/engine/TemplateEngine.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Route logging through the
loggerutilities for consistent output.
Files:
src/engine/TemplateEngine.ts
src/{engine,services,utils}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Structure production code so Obsidian dependencies are injected behind interfaces to enable unit testing without loading real Obsidian modules.
Files:
src/engine/TemplateEngine.ts
🧬 Code graph analysis (1)
src/engine/TemplateEngine.ts (4)
src/gui/InputSuggester/inputSuggester.ts (1)
InputSuggester(22-195)src/gui/GenericSuggester/genericSuggester.ts (1)
GenericSuggester(8-142)src/utils/errorUtils.ts (1)
isCancellationError(69-80)src/errors/MacroAbortError.ts (1)
MacroAbortError(1-6)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/engine/TemplateChoiceEngine.ts (1)
292-295: MissingawaitbeforegetOrCreateFolder.For consistency with other branches (lines 267, 276, 298), add
awaithere. While returning a Promise from an async function works, explicitawaitensures uniform error handling and makes the code's intent clearer.🔎 Proposed fix
- return this.getOrCreateFolder([activeFile.parent.path], { + return await this.getOrCreateFolder([activeFile.parent.path], { allowCreate: true, topItems, });
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/engine/TemplateChoiceEngine.tssrc/engine/TemplateEngine.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,mts,mjs,js,json}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,mts,mjs,js,json}: Use tab indentation with width 2 in TypeScript and configuration files (enforced by Biome).
Follow an 80-character line guide (enforced by Biome).
Files:
src/engine/TemplateEngine.tssrc/engine/TemplateChoiceEngine.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use camelCase for variables and functions in TypeScript.
Prefer type-only imports in TypeScript.
Files:
src/engine/TemplateEngine.tssrc/engine/TemplateChoiceEngine.ts
**/*.{ts,tsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
Use PascalCase for classes and Svelte components.
Files:
src/engine/TemplateEngine.tssrc/engine/TemplateChoiceEngine.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Route logging through the
loggerutilities for consistent output.
Files:
src/engine/TemplateEngine.tssrc/engine/TemplateChoiceEngine.ts
src/{engine,services,utils}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Structure production code so Obsidian dependencies are injected behind interfaces to enable unit testing without loading real Obsidian modules.
Files:
src/engine/TemplateEngine.tssrc/engine/TemplateChoiceEngine.ts
🧠 Learnings (1)
📚 Learning: 2025-12-21T07:54:34.863Z
Learnt from: CR
Repo: chhoumann/quickadd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T07:54:34.863Z
Learning: Applies to src/**/*.ts src/**/*.tsx src/**/*.svelte src/engine/** src/services/** src/utils/** src/gui/** src/types/** src/quickAddSettingsTab.ts tests/** docs/** : 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`. Bundled artifacts `main.js` and `styles.css` stay at the repo root and should be generated, not hand-edited. Place tests and stubs in `tests/`, and keep user-facing docs in `docs/`.
Applied to files:
src/engine/TemplateEngine.ts
🧬 Code graph analysis (2)
src/engine/TemplateEngine.ts (5)
src/gui/InputSuggester/inputSuggester.ts (1)
InputSuggester(22-195)src/gui/GenericSuggester/genericSuggester.ts (1)
GenericSuggester(8-142)src/utils/errorUtils.ts (1)
isCancellationError(69-80)src/errors/MacroAbortError.ts (1)
MacroAbortError(1-6)tests/obsidian-stub.ts (2)
Notice(453-473)display(328-328)
src/engine/TemplateChoiceEngine.ts (1)
src/utilityObsidian.ts (1)
getAllFolderPathsInVault(736-741)
🔇 Additional comments (19)
src/engine/TemplateChoiceEngine.ts (5)
251-252: LGTM!Clean initialization of
topItemsbased on the current folder suggestion.
267-271: LGTM!Correct usage of
awaitand appropriate options for subfolder selection with root constraints.
276-279: LGTM!Vault-wide folder selection correctly omits
allowedRootsto permit any folder choice.
298-303: LGTM!Default folder flow correctly constrains selection to configured roots.
305-316: LGTM!The helper correctly derives the current folder suggestion from the active file's parent. The nullish coalescing on
parent.pathis a reasonable defensive measure.src/engine/TemplateEngine.ts (14)
5-5: LGTM!Imports are correctly added for the new folder selection functionality.
Also applies to: 13-13
22-48: LGTM!Well-designed types that clearly model the folder selection domain.
FolderSelectionnicely encapsulates all resolution aspects.
79-94: LGTM!Clean orchestration method that delegates to well-named helper methods. The flow is easy to follow.
96-127: LGTM!Context construction is thorough, with proper normalization of allowed roots and efficient set-based lookups.
129-134: LGTM!Decision logic correctly identifies when user interaction is needed.
136-170: LGTM!Appropriate suggester selection based on creation capability. Error handling correctly propagates cancellation as
MacroAbortError.
172-199: LGTM!Resolution logic correctly combines canonical lookup with filesystem check for existence, and properly validates against allowed roots.
201-224: LGTM!The prompt loop correctly enforces root constraints by re-prompting until the user provides a valid selection or cancels.
226-229: LGTM!Simple guard clause before folder creation.
247-249: LGTM!Standard path normalization handling leading/trailing slashes and whitespace.
251-259: LGTM!Path validation correctly handles root matching with proper normalization. The early return for empty root (
!root) appropriately means "no restriction."
261-268: LGTM!User-friendly notice with truncation for many roots.
270-316: LGTM!Well-structured suggestion builder with proper deduplication and root filtering. Processing
topItemsfirst ensures current folder appears at the top.
318-346: LGTM!Rich suggestion rendering with clear indicators for current folder and folder creation. Uses standard Obsidian suggestion styling.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/engine/TemplateChoiceEngine.ts (1)
292-295: The missingawaithas been resolved.This addresses the previous review comment. The
awaitkeyword is now present beforethis.getOrCreateFolder, ensuring the Promise is properly resolved.src/engine/TemplateEngine.ts (1)
272-281: MacroAbortError is now thrown for disallowed selections.This addresses the previous review comment. The method now throws
MacroAbortErrorwhen the single selection is not allowed (line 281), preventing silent fallback to root folder.
🧹 Nitpick comments (2)
src/engine/TemplateChoiceEngine.ts (1)
305-316: Well-structured helper for current folder suggestion.The
getCurrentFolderSuggestion()method correctly derives the current folder context and returns a typed object with path and label. The null-safety checks are appropriate.Optional: Redundant nullish coalescing on line 311
The
?? ""on line 311 is redundant sinceparent.pathis always a string in Obsidian's API. You can simplify to:- const path = parent.path ?? ""; + const path = parent.path;src/engine/TemplateEngine.ts (1)
300-302: Consider normalizing consecutive slashes.The method removes leading and trailing slashes but doesn't normalize consecutive slashes within the path (e.g.,
"foo//bar"→"foo/bar"). While Obsidian may handle this internally, normalizing would make path comparisons more reliable.🔎 Suggested enhancement
private normalizeFolderPath(path: string): string { - return path.trim().replace(/^\/+/, "").replace(/\/+$/, ""); + return path.trim() + .replace(/^\/+/, "") + .replace(/\/+$/, "") + .replace(/\/+/g, "/"); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/engine/TemplateChoiceEngine.tssrc/engine/TemplateEngine.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,mts,mjs,js,json}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,mts,mjs,js,json}: Use tab indentation with width 2 in TypeScript and configuration files (enforced by Biome).
Follow an 80-character line guide (enforced by Biome).
Files:
src/engine/TemplateChoiceEngine.tssrc/engine/TemplateEngine.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use camelCase for variables and functions in TypeScript.
Prefer type-only imports in TypeScript.
Files:
src/engine/TemplateChoiceEngine.tssrc/engine/TemplateEngine.ts
**/*.{ts,tsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
Use PascalCase for classes and Svelte components.
Files:
src/engine/TemplateChoiceEngine.tssrc/engine/TemplateEngine.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Route logging through the
loggerutilities for consistent output.
Files:
src/engine/TemplateChoiceEngine.tssrc/engine/TemplateEngine.ts
src/{engine,services,utils}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Structure production code so Obsidian dependencies are injected behind interfaces to enable unit testing without loading real Obsidian modules.
Files:
src/engine/TemplateChoiceEngine.tssrc/engine/TemplateEngine.ts
🧠 Learnings (1)
📚 Learning: 2025-12-21T07:54:34.863Z
Learnt from: CR
Repo: chhoumann/quickadd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T07:54:34.863Z
Learning: Applies to src/**/*.ts src/**/*.tsx src/**/*.svelte src/engine/** src/services/** src/utils/** src/gui/** src/types/** src/quickAddSettingsTab.ts tests/** docs/** : 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`. Bundled artifacts `main.js` and `styles.css` stay at the repo root and should be generated, not hand-edited. Place tests and stubs in `tests/`, and keep user-facing docs in `docs/`.
Applied to files:
src/engine/TemplateEngine.ts
🧬 Code graph analysis (1)
src/engine/TemplateChoiceEngine.ts (1)
src/utilityObsidian.ts (1)
getAllFolderPathsInVault(736-741)
🔇 Additional comments (5)
src/engine/TemplateEngine.ts (5)
22-80: Well-designed types and validation constants.The new types (
FolderChoiceOptions,FolderSelectionContext,FolderSelection) clearly document the folder selection flow. TheRESERVED_WINDOWS_DEVICE_NAMESset and customInvalidFolderPathErrorclass provide robust cross-platform validation.
111-123: Clean refactor of the folder selection entry point.The updated
getOrCreateFoldermethod has a clear flow: build context → check if prompt needed → handle appropriately → return resolved path. The option-based API provides good flexibility.
314-348: Comprehensive cross-platform folder segment validation.The validation checks cover all critical cases: empty segments, relative paths (
.and..), control characters, invalid characters, trailing whitespace/periods, and Windows reserved names. This ensures robust cross-platform folder creation.
373-419: Well-structured folder suggestion builder.The method correctly builds suggestion lists with proper deduplication via the
seenSet, filters byallowedRoots, and maintains mappings for display labels. The ordering (topItems first, then folders) ensures the current folder suggestion appears at the top.
421-449: Clear UI rendering with helpful indicators.The method provides good user experience by showing "Current folder" for the active file's parent and "Create folder" for non-existent paths. The use of Obsidian's standard suggestion classes ensures consistent UI.
|
🎉 This PR is included in version 2.10.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
1 participant
Summary
Testing
Closes #1011
Closes #1012
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
✏️ Tip: You can customize this high-level summary in your review settings.