Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds an "insert into active note" feature for Template choices: selectable template sources (fixed path, prompt, or another Template choice), multiple insertion placements (cursor/selection/lines/top/bottom), optional YAML frontmatter merging into the active file, and supporting UI, engine, and type changes. Changes
Sequence DiagramsequenceDiagram
participant User
participant UI as UI Builder
participant Engine as TemplateChoiceEngine
participant TplEngine as TemplateEngine
participant Editor as Active Editor/File
User->>UI: Enable "Insert into active note" & choose source/placement
UI->>Engine: run(choice with insertion config)
rect rgb(245,250,255)
Engine->>Engine: Validate active Markdown file
Engine->>Engine: Resolve template path (fixed / prompt / choice)
alt Source = prompt
Engine->>User: Prompt for template path
User-->>Engine: Template path
else Source = choice
Engine->>User: Suggest template choices
User-->>Engine: Selected template path
end
Engine->>TplEngine: formatTemplateForFile(templatePath, targetFile)
TplEngine-->>Engine: formatted content + templateVars
Engine->>Engine: splitFrontmatter(content) / applyFrontmatterProperties()
end
rect rgb(235,255,235)
alt Placement = top/bottom
Engine->>Engine: insertBodyAtTop/insertBodyAtBottom()
Engine->>Editor: Replace file content (merge frontmatter/body)
else Other placements
Engine->>Engine: insertBodyIntoEditor(placement)
Engine->>Editor: Insert at cursor/selection/line
end
end
Editor-->>User: Active note updated with inserted template
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/preflight/runOnePagePreflight.ts (1)
94-107: Bug: Useschoice.templatePathinstead of computedtemplatePathvariable.After computing the correct
templatePathbased on the insertion source type (lines 66-92), thewalkfunction on line 106 incorrectly useschoice.templatePathinstead of the computedtemplatePath. This causes the preflight to scan the wrong template when insertion is enabled with a different template source.🔎 Proposed fix
if (templatePath) { const visited = new Set<string>(); const walk = async (path: string) => { if (visited.has(path)) return; visited.add(path); const content = await readTemplate(app, path); await collector.scanString(content); for (const nested of collector.templatesToScan) { if (!visited.has(nested)) await walk(nested); } collector.templatesToScan.clear(); }; - await walk(choice.templatePath); + await walk(templatePath); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
docs/docs/Choices/TemplateChoice.mdsrc/engine/TemplateChoiceEngine.tssrc/engine/TemplateEngine.tssrc/gui/ChoiceBuilder/templateChoiceBuilder.tssrc/preflight/runOnePagePreflight.tssrc/types/choices/ITemplateChoice.tssrc/types/choices/TemplateChoice.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/types/choices/ITemplateChoice.tssrc/types/choices/TemplateChoice.tssrc/engine/TemplateEngine.tssrc/gui/ChoiceBuilder/templateChoiceBuilder.tssrc/preflight/runOnePagePreflight.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/types/choices/ITemplateChoice.tssrc/types/choices/TemplateChoice.tssrc/engine/TemplateEngine.tssrc/gui/ChoiceBuilder/templateChoiceBuilder.tssrc/preflight/runOnePagePreflight.tssrc/engine/TemplateChoiceEngine.ts
**/*.{ts,tsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
Use PascalCase for classes and Svelte components.
Files:
src/types/choices/ITemplateChoice.tssrc/types/choices/TemplateChoice.tssrc/engine/TemplateEngine.tssrc/gui/ChoiceBuilder/templateChoiceBuilder.tssrc/preflight/runOnePagePreflight.tssrc/engine/TemplateChoiceEngine.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Route logging through the
loggerutilities for consistent output.
Files:
src/types/choices/ITemplateChoice.tssrc/types/choices/TemplateChoice.tssrc/engine/TemplateEngine.tssrc/gui/ChoiceBuilder/templateChoiceBuilder.tssrc/preflight/runOnePagePreflight.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
🧬 Code graph analysis (4)
src/types/choices/TemplateChoice.ts (1)
src/types/choices/ITemplateChoice.ts (2)
ITemplateChoice(49-76)DEFAULT_TEMPLATE_INSERTION(29-33)
src/engine/TemplateEngine.ts (1)
tests/obsidian-stub.ts (1)
TFile(332-338)
src/preflight/runOnePagePreflight.ts (2)
src/types/choices/ITemplateChoice.ts (2)
normalizeTemplateInsertionConfig(35-47)ITemplateChoice(49-76)src/utils/choiceUtils.ts (1)
flattenChoices(11-23)
src/engine/TemplateChoiceEngine.ts (7)
src/types/choices/ITemplateChoice.ts (4)
ITemplateChoice(49-76)normalizeTemplateInsertionConfig(35-47)TemplateInsertionConfig(23-27)TemplateInsertionPlacement(6-14)src/errors/MacroAbortError.ts (1)
MacroAbortError(1-6)src/utils/invariant.ts (1)
invariant(1-10)src/utilityObsidian.ts (1)
templaterParseTemplate(390-410)src/gui/InputSuggester/inputSuggester.ts (1)
InputSuggester(22-195)src/utils/choiceUtils.ts (1)
flattenChoices(11-23)src/types/linkPlacement.ts (1)
LinkPlacement(4-8)
🔇 Additional comments (18)
src/engine/TemplateEngine.ts (1)
546-564: LGTM!The new
formatTemplateForFilemethod cleanly encapsulates template loading and formatting logic for the insertion workflow. It follows the same pattern ascreateFileWithTemplatebut appropriately omits file creation, making it reusable for inserting into existing files.src/types/choices/ITemplateChoice.ts (2)
6-47: LGTM!The new types are well-structured and the normalization function correctly handles partial or undefined configurations with sensible defaults. The use of nullish coalescing ensures proper fallback behavior.
74-75: LGTM!The optional
insertionfield cleanly extends the interface without breaking existing configurations.src/types/choices/TemplateChoice.ts (1)
52-56: LGTM!Good use of the spread operator for
templateSourceto create a shallow copy, avoiding shared references between instances.docs/docs/Choices/TemplateChoice.md (1)
13-38: LGTM!The documentation clearly explains the new insertion feature, including placement options, template sources, and which settings are ignored when insertion is enabled. The YAML frontmatter merging behavior is well-documented.
src/preflight/runOnePagePreflight.ts (1)
50-63: LGTM!Correctly gates the file name format and folder scanning behind
insertionEnabled, since these settings are not applicable when inserting into an active note.src/gui/ChoiceBuilder/templateChoiceBuilder.ts (4)
97-100: LGTM!The
ensureInsertionConfigmethod properly normalizes the insertion config and assigns it back to the choice, ensuring consistent state.
102-137: LGTM!The insertion toggle and placement dropdown are well-implemented. The
reload()call ensures the UI updates correctly when the enabled state changes.
139-163: LGTM!The template source setting correctly clears
templateSource.valuewhen switching away from the "choice" type, preventing stale references. Thereload()call ensures the UI reflects the new state.
165-204: LGTM!Good handling of edge cases: displaying a message when no template choices are available, and preserving unknown current values in the dropdown to prevent data loss if a referenced choice was deleted.
src/engine/TemplateChoiceEngine.ts (8)
46-47: LGTM!The regex correctly handles YAML frontmatter with both
---and...closing delimiters, and supports both LF and CRLF line endings.
226-289: LGTM!The
runInsertionmethod is well-structured:
- Validates active file is a Markdown note before proceeding
- Correctly handles frontmatter merging separately from body insertion
- Uses appropriate file modification vs. editor insertion based on placement type
- Properly triggers post-processing for template variables
291-319: LGTM!The template path resolution correctly handles all three source types with proper cancellation error handling that converts to
MacroAbortError.
321-368: LGTM!The choice resolution logic correctly falls back to prompting the user when the specified choice ID/name is not found, providing a graceful degradation path.
370-406: LGTM!The frontmatter splitting and merging logic is correct:
splitFrontmatterproperly extracts the YAML content without the delimitersapplyFrontmatterInsertionpreserves existing frontmatter structure while appending new content
408-427: LGTM!
insertBodyAtTopcorrectly usesfindYamlFrontMatterRangeto insert content after any existing frontmatter, andinsertBodyAtBottomappends with proper newline handling.
462-480: LGTM!The
joinWithNewlineshelper correctly ensures single newlines between content segments without creating double newlines.
429-460: No functional issue here. TheinsertLinkWithPlacementfunction handles arbitrary content correctly—it contains no link-specific logic. All three code paths it receives (replaceSelection,afterSelection,endOfLine) use generic editor operations (replaceSelection()andreplaceRange()) with no constraints on content type. The naming is misleading, but the implementation is intentionally generic for text insertion.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/engine/TemplateChoiceEngine.ts (3)
18-23: Consider using type-only imports for type definitions.Per coding guidelines, prefer type-only imports in TypeScript.
TemplateInsertionConfig,TemplateInsertionPlacement, andLinkPlacementappear to be used only as types.🔎 Suggested change
import { normalizeTemplateInsertionConfig, - type TemplateInsertionConfig, - type TemplateInsertionPlacement, } from "../types/choices/ITemplateChoice"; -import { normalizeAppendLinkOptions, type LinkPlacement } from "../types/linkPlacement"; +import type { + TemplateInsertionConfig, + TemplateInsertionPlacement, +} from "../types/choices/ITemplateChoice"; +import { normalizeAppendLinkOptions } from "../types/linkPlacement"; +import type { LinkPlacement } from "../types/linkPlacement";
236-240: Invariant error message has unreachable condition.The ternary check
templatePath?.length === 0will never be true because iftemplatePathis falsy, the invariant fails and this message is constructed. IftemplatePathis an empty string (length 0), the invariant still fails but the optional chaining makes the condition evaluate correctly. However, iftemplatePathisundefined,templatePath?.lengthisundefined, not0, so the condition is false and no helpful message is shown.Consider simplifying:
🔎 Suggested change
invariant(templatePath, () => { - return `Invalid template path for ${this.choice.name}. ${ - templatePath?.length === 0 ? "Template path is empty." : "" - }`; + return `Invalid template path for ${this.choice.name}. Template path is empty or undefined.`; });
460-462: Consider using MacroAbortError for consistency.The default case throws a generic
Error, but other validation failures in this file throwMacroAbortError. For consistency in error handling upstream, consider using the same error type.🔎 Suggested change
default: - throw new Error(`Unknown insertion placement: ${placement}`); + throw new MacroAbortError(`Unknown insertion placement: ${placement}`);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/engine/TemplateChoiceEngine.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.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.ts
**/*.{ts,tsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
Use PascalCase for classes and Svelte components.
Files:
src/engine/TemplateChoiceEngine.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Route logging through the
loggerutilities for consistent output.
Files:
src/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/TemplateChoiceEngine.ts
🧬 Code graph analysis (1)
src/engine/TemplateChoiceEngine.ts (3)
src/types/choices/ITemplateChoice.ts (3)
ITemplateChoice(49-76)TemplateInsertionConfig(23-27)TemplateInsertionPlacement(6-14)src/utilityObsidian.ts (1)
templaterParseTemplate(390-410)src/types/linkPlacement.ts (1)
LinkPlacement(4-8)
🔇 Additional comments (10)
src/engine/TemplateChoiceEngine.ts (10)
46-47: Verify frontmatter regex allows leading whitespace intentionally.The regex permits leading whitespace (
\s*) before the opening---. Standard YAML frontmatter typically requires---at position 0 (start of file). This could match non-frontmatter content if there's accidental leading whitespace.If this is intentional for flexibility, consider adding a comment explaining the design choice.
60-66: LGTM!Clean early return pattern for insertion mode that properly short-circuits the normal template creation flow.
276-290: LGTM!Clean switch statement for resolving template path with appropriate fallback to the choice's configured template path.
306-353: LGTM!Well-structured method with proper validation, user-friendly display items that show template paths for disambiguation, and consistent error handling for cancellation.
355-368: LGTM!Clean implementation that correctly handles both cases: content with and without frontmatter.
370-409: LGTM!Robust implementation with proper YAML parsing validation and graceful error handling. Using
processFrontMatterensures proper atomic updates to the file's frontmatter.
411-422: LGTM!Correctly inserts body content after any existing frontmatter, preserving the file structure.
424-430: LGTM!Simple and correct implementation for appending content at the bottom.
465-483: LGTM!Well-designed utility that handles edge cases properly, avoiding double newlines while ensuring proper separation between content sections.
292-304: LGTM!Proper error handling pattern with cancellation detection, consistent with the rest of the codebase.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/engine/TemplateChoiceEngine.ts (1)
413-430: Array merge concatenates without deduplication.Line 418 concatenates arrays:
[...existing, ...incoming]. If a user runs insertion multiple times with the same tags/aliases, duplicates will accumulate. If this is intentional for additive behavior, it's fine; otherwise, consider deduplicating:🔎 Optional: Deduplicate primitive arrays
if (Array.isArray(existing) && Array.isArray(incoming)) { - return [...existing, ...incoming]; + const merged = [...existing, ...incoming]; + // Deduplicate primitives only + if (merged.every((v) => typeof v !== "object")) { + return [...new Set(merged)]; + } + return merged; }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/engine/TemplateChoiceEngine.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.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.ts
**/*.{ts,tsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
Use PascalCase for classes and Svelte components.
Files:
src/engine/TemplateChoiceEngine.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Route logging through the
loggerutilities for consistent output.
Files:
src/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/TemplateChoiceEngine.ts
🔇 Additional comments (6)
src/engine/TemplateChoiceEngine.ts (6)
1-41: LGTM!Imports are well-organized and appropriate for the new insertion functionality. Type imports are properly used where applicable.
46-47: Verify: Leading whitespace allowance in frontmatter regex.The
\s*at the start of the regex allows whitespace before---. Standard YAML frontmatter requires---at position 0. If templates with accidental leading whitespace are common, this is fine; otherwise, consider anchoring strictly with^---\r?\n.
60-67: LGTM!Clean separation of the insertion flow from the existing template creation flow using an early return pattern.
226-274: LGTM!The insertion flow is well-structured with proper validation, error handling using
MacroAbortError, and a good optimization to skip file modification when content is unchanged (lines 264-266).
276-353: LGTM!Template path resolution is well-implemented with:
- Clear fallback chain for path resolution
- Consistent cancellation error handling across prompts
- Proper validation that template choices exist and have paths
495-513: LGTM!The
joinWithNewlinesutility correctly ensures proper newline separation between content sections without creating double newlines.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
|
This isn't a very good implementation. Closing for now. |
1 participant
Summary
Testing
Closes #526
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.