Templater: deterministic snippet rendering (capture + template append)#1017
Templater: deterministic snippet rendering (capture + template append)#1017
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughTemplater integration and coordination were added: new templater utilities, per-file suppression and locking, wait-for-change detection, cursor advancement, and a focus-aware workspace-open flow; engines and template handling were updated to pass suppression/focus options and to conditionally run templater actions. Changes
Sequence DiagramsequenceDiagram
autonumber
actor User
participant Engine as Capture/Template Engine
participant Vault as Vault / FileSystem
participant Workspace as Workspace / Editor
participant Templater as Templater Plugin
User->>Engine: trigger capture/create
Engine->>Vault: createFileWithInput(path, content, { suppressTemplaterOnCreate? })
alt suppress templater for MD
Engine->>Templater: withTemplaterFileCreationSuppressed(...wrap create...)
end
Vault-->>Engine: file created (TFile)
alt templater trigger-on-create enabled & template not pre-parsed
Engine->>Templater: waitForTemplaterTriggerOnCreateToComplete(file)
Templater-->>Engine: templater-trigger changes complete
end
Engine->>Workspace: openExistingFileTab(file, focus=true)
Workspace-->>Engine: WorkspaceLeaf or null
Engine->>Templater: jumpToNextTemplaterCursorIfPossible(file)
Templater-->>Engine: cursor moved / no-op
alt choice.templater.afterCapture == "wholeFile"
Engine->>Templater: overwriteTemplaterOnce(file, opts)
Templater-->>Engine: templating complete
end
Engine-->>User: operation complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/engine/QuickAddEngine.ts (1)
207-235:noteBodyIsEmptyshould probably treat whitespace-only bodies as empty.
This affects when Templater on-create is suppressed for “frontmatter-only” notes.- const noteBodyIsEmpty = body.length === 0; + const noteBodyIsEmpty = body.trim().length === 0;src/engine/CaptureChoiceEngine.ts (1)
134-167: ReorderapplyCapturePropertyVarsbeforeoverwriteTemplaterOncein the whole-file templater path.When
afterCapture === "wholeFile", the property variables captured during formatting should be applied to the file before whole-file templater processing. Currently, the order is reversed:overwriteTemplaterOnce()runs first (line 164), thenapplyCapturePropertyVars()(line 166). This means templater processes the file without seeing the updated frontmatter. Swap these calls so templater can reference the captured property variables:- await this.app.vault.modify(file, newFileContent); - if (this.choice.templater?.afterCapture === "wholeFile") { - await overwriteTemplaterOnce(this.app, file); - } - await this.applyCapturePropertyVars(file); + await this.app.vault.modify(file, newFileContent); + await this.applyCapturePropertyVars(file); + if (this.choice.templater?.afterCapture === "wholeFile") { + await overwriteTemplaterOnce(this.app, file); + }
🧹 Nitpick comments (3)
src/types/choices/CaptureChoice.ts (1)
38-40: Defaulting is good; consider normalizingafterCapturewhentemplaterexists but is partial.
Right nowLoad()only fillstemplaterwhen missing; if persisted data hastemplater: {}thenafterCaptureremainsundefined(effectively “none”, but not explicit). Optional improvement:if (!loaded.templater) { loaded.templater = { afterCapture: "none" }; +} else if (!loaded.templater.afterCapture) { + loaded.templater.afterCapture = "none"; }Also applies to: 76-78, 87-89
src/engine/TemplateEngine.ts (1)
260-268: Confirm desired failure mode iftemplaterParseTemplate()throws.
Right now a Templater exception aborts the entire append/prepend path. If you want resilience similar to “Templater not installed => no-op”, consider catching and logging inside this block (or ensuringtemplaterParseTemplatenever throws).src/utilityObsidian.ts (1)
296-299: Consider documenting the magic number forrun_mode: 4.The
run_mode: 4value is passed to Templater'sparse_templatewithout explanation. Consider adding a comment documenting what this mode represents (e.g., referencing Templater's internal enum if known) to aid future maintainers.+ // run_mode 4 = DynamicProcessor mode in Templater (parse without file creation side effects) return await parseTemplate( { target_file: targetFile, run_mode: 4 }, templateContent, );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/engine/CaptureChoiceEngine.template-property-types.test.ts(1 hunks)src/engine/CaptureChoiceEngine.ts(5 hunks)src/engine/QuickAddEngine.ts(3 hunks)src/engine/TemplateChoiceEngine.ts(2 hunks)src/engine/TemplateEngine.ts(3 hunks)src/gui/ChoiceBuilder/captureChoiceBuilder.ts(2 hunks)src/types/choices/CaptureChoice.ts(3 hunks)src/types/choices/ICaptureChoice.ts(1 hunks)src/utilityObsidian.ts(4 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/CaptureChoiceEngine.template-property-types.test.tssrc/gui/ChoiceBuilder/captureChoiceBuilder.tssrc/engine/QuickAddEngine.tssrc/engine/TemplateChoiceEngine.tssrc/types/choices/ICaptureChoice.tssrc/types/choices/CaptureChoice.tssrc/engine/CaptureChoiceEngine.tssrc/utilityObsidian.tssrc/engine/TemplateEngine.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/CaptureChoiceEngine.template-property-types.test.tssrc/gui/ChoiceBuilder/captureChoiceBuilder.tssrc/engine/QuickAddEngine.tssrc/engine/TemplateChoiceEngine.tssrc/types/choices/ICaptureChoice.tssrc/types/choices/CaptureChoice.tssrc/engine/CaptureChoiceEngine.tssrc/utilityObsidian.tssrc/engine/TemplateEngine.ts
src/**/*.{ts,tsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
Use PascalCase for classes and Svelte components
Files:
src/engine/CaptureChoiceEngine.template-property-types.test.tssrc/gui/ChoiceBuilder/captureChoiceBuilder.tssrc/engine/QuickAddEngine.tssrc/engine/TemplateChoiceEngine.tssrc/types/choices/ICaptureChoice.tssrc/types/choices/CaptureChoice.tssrc/engine/CaptureChoiceEngine.tssrc/utilityObsidian.tssrc/engine/TemplateEngine.ts
🧠 Learnings (2)
📚 Learning: 2025-12-09T21:20:52.398Z
Learnt from: CR
Repo: chhoumann/quickadd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T21:20:52.398Z
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/CaptureChoiceEngine.template-property-types.test.tssrc/utilityObsidian.ts
📚 Learning: 2025-12-09T21:20:52.398Z
Learnt from: CR
Repo: chhoumann/quickadd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T21:20:52.398Z
Learning: Applies to tests/**/*.{ts,tsx} : Add regression coverage for bug fixes
Applied to files:
src/engine/CaptureChoiceEngine.template-property-types.test.ts
🧬 Code graph analysis (4)
src/engine/QuickAddEngine.ts (2)
src/utils/yamlContext.ts (1)
findYamlFrontMatterRange(13-18)src/utilityObsidian.ts (1)
withTemplaterFileCreationSuppressed(93-138)
src/engine/TemplateChoiceEngine.ts (1)
src/utilityObsidian.ts (3)
openExistingFileTab(760-785)openFile(677-755)jumpToNextTemplaterCursorIfPossible(302-336)
src/utilityObsidian.ts (2)
tests/obsidian-stub.ts (3)
TFile(72-78)App(23-51)WorkspaceLeaf(99-104)src/utils/errorUtils.ts (1)
reportError(99-120)
src/engine/TemplateEngine.ts (2)
tests/obsidian-stub.ts (1)
TFile(72-78)src/utilityObsidian.ts (1)
templaterParseTemplate(285-300)
🔇 Additional comments (13)
src/gui/ChoiceBuilder/captureChoiceBuilder.ts (1)
66-67: Nice UX for the legacy/advanced escape hatch, and safe initialization.
The toggle correctly guardsthis.choice.templaterand maps boolean UI to the string union.Also applies to: 77-92
src/types/choices/ICaptureChoice.ts (1)
43-45: Schema extension looks consistent with the implementation + UI.
Optional nesting preserves backwards compatibility.src/engine/TemplateEngine.ts (1)
169-176:suppressTemplaterOnCreatewiring looks correct.
Delegating the “only suppress when body empty” decision toQuickAddEngine.createFileWithInput()keeps this call site simple.src/engine/TemplateChoiceEngine.ts (1)
20-25: Focus-aware “open existing tab” + cursor jump is a solid integration step.
The active-file check insidejumpToNextTemplaterCursorIfPossibleshould keep this safe whenfocus === false.Also applies to: 178-190
src/engine/CaptureChoiceEngine.ts (2)
182-190: Open+focus behavior and cursor-jump hook look correct.
Good use offocus ?? trueand guarded cursor advancement.
451-471: New-file capture sequencing (suppression + overwrite vs settle) is reasonable.
The “template => overwrite once” vs “no template + trigger-on-create => wait for settle” split matches the stated race-condition mitigation.src/utilityObsidian.ts (7)
25-45: LGTM!The
TemplaterPluginLiketype provides a well-structured interface for the Templater plugin's surface area. Using optional properties throughout enables safe optional chaining access patterns in the consuming code.
83-91: LGTM!Clean wrapper functions with proper null handling and optional chaining for safe property access.
140-195: LGTM!Well-designed file stability detection with clear distinction between grace period (no changes observed) and quiet period (after changes stop). The exponential backoff and timeout bounds are reasonable for the use case.
197-229: LGTM!The lock mechanism correctly serializes concurrent Templater operations on the same file path using promise chaining. The cleanup logic properly ensures only the final chain in the sequence removes the map entry, preventing premature cleanup.
231-283: LGTM!Solid implementation with defensive guards, proper locking, and a thoughtful rollback mechanism to prevent partial renders from corrupting files. The
skipIfNoTagsheuristic usingincludes("<%")is a reasonable performance optimization.
302-336: LGTM!Well-structured with proper guards and a thoughtful fallback mechanism. The function correctly tries the direct API first and falls back to command execution only when needed and when auto-jump is enabled in Templater's settings.
760-785: Callers already properly updated — no action needed.Both
CaptureChoiceEngine.tsandTemplateChoiceEngine.tscorrectly handle the new return type withif (!openExistingTab)null checks, so the breaking change is already accounted for throughout the codebase.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/utilityObsidian.ts (1)
822-847: Update docstring: return type no longer matches comment (“true/false”).The comment says “return false/true”, but the function now returns
WorkspaceLeaf | null. Consider updating the JSDoc to avoid misleading downstream usage.
🧹 Nitpick comments (2)
src/engine/QuickAddEngine.ts (1)
206-230: Good: opt-in Templater suppression on create, but consider path/extension canonicalization.The wrapper approach is clean and keeps default behavior unchanged. Two small hardening points:
filePath.toLowerCase().endsWith(".md")is fine, but if Obsidian ever passes non-canonical paths (case/normalization), suppression keys may miss Templater’s internalfiles_with_pending_templatesentries. Consider normalizing to Obsidian’s canonicalTFile.path(when available) before callingwithTemplaterFileCreationSuppressed.- If you already have
MARKDOWN_FILE_EXTENSION_REGEX/ extension helpers in this file, using a shared helper avoids divergence.src/utilityObsidian.ts (1)
201-256: waitForFileToStopChanging: good defaults, but ensure callers can distinguish timeout vs “settled”.Right now it always returns
void, even on timeout. If callers need to know whether it actually observed a quiet period, consider returning a boolean (true= settled,false= timed out) so higher-level flows can decide whether to proceed or add a fallback delay.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/engine/CaptureChoiceEngine.template-property-types.test.ts(1 hunks)src/engine/QuickAddEngine.ts(3 hunks)src/types/choices/CaptureChoice.ts(3 hunks)src/utilityObsidian.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/types/choices/CaptureChoice.ts
- src/engine/CaptureChoiceEngine.template-property-types.test.ts
🧰 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/QuickAddEngine.tssrc/utilityObsidian.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/QuickAddEngine.tssrc/utilityObsidian.ts
src/**/*.{ts,tsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
Use PascalCase for classes and Svelte components
Files:
src/engine/QuickAddEngine.tssrc/utilityObsidian.ts
🧠 Learnings (1)
📚 Learning: 2025-12-09T21:20:52.398Z
Learnt from: CR
Repo: chhoumann/quickadd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T21:20:52.398Z
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/utilityObsidian.ts
🧬 Code graph analysis (1)
src/engine/QuickAddEngine.ts (1)
src/utilityObsidian.ts (1)
withTemplaterFileCreationSuppressed(136-199)
🔇 Additional comments (4)
src/engine/QuickAddEngine.ts (1)
5-5: Import addition looks appropriate.
withTemplaterFileCreationSuppressedis used directly and keeps the create flow readable.src/utilityObsidian.ts (3)
105-199: Suppression coordination looks solid (active counter + teardown single-flight).The
activeTemplaterFileCreationSuppressionsguard plustemplaterSuppressionTeardownLockshould prevent premature/double teardown while concurrent suppressions are in-flight.
258-290: Per-file lock + rollback in overwriteTemplaterOnce is a big reliability win.Serializing renders per path and rolling back on render failure should reduce interleaving and partial writes substantially.
Also applies to: 292-344
346-363: Verify Templater internal API assumptions are documented or have fallbacks.This code relies on Templater internals without clear stability guarantees:
parse_template({ run_mode: 4 }, ...)assumes run_mode=4 corresponds to "DynamicProcessor"—unclear if this is a stable public API or an undocumented internal value"templater-obsidian:jump-to-next-cursor-location"command ID appears to be a hardcoded internal commandeditor_handler.jump_to_next_cursor_location(file, true)signature assumes a specific method exists with this behaviorWhile the code includes type guards (
typeof parseTemplate !== "function") and returns templateContent on fallback, Templater's undocumented internal APIs risk silent failures if the plugin updates. Add comments documenting which Templater versions these are tested against, or consider safer alternatives like delegating to Templater's public command interface rather than direct internal method calls.
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/utilityObsidian.ts (1)
822-847: Update test mocks to return correct type foropenExistingFileTab.Production callers in
TemplateChoiceEngine.tsandCaptureChoiceEngine.tscorrectly handle the newWorkspaceLeaf | nullreturn type. However, test mocks inTemplateChoiceEngine.notice.test.ts(line 83) andCaptureChoiceEngine.notice.test.ts(line 76) returnfalseinstead ofnull, violating the function's type signature. Update these mocks to returnnullto match the API. Additionally, the JSDoc comment at line 820 insrc/utilityObsidian.tsreferences the old boolean return type and should be updated.
🧹 Nitpick comments (2)
src/utilityObsidian.ts (2)
181-186: Magic number tied to Templater internals.The 350ms delay depends on Templater's internal ~300ms check interval. This coupling could break if Templater updates its timing. Consider extracting this to a named constant with documentation about its purpose and the Templater version it was tested against.
+// Templater waits ~300ms before checking files_with_pending_templates in its on-create handler. +// We hold the entry slightly longer to ensure the bypass is observed. +// Tested with templater-obsidian v2.x - may need adjustment if Templater internals change. +const TEMPLATER_PENDING_CHECK_BUFFER_MS = 350; + // ... - const minHoldMs = 350; + const minHoldMs = TEMPLATER_PENDING_CHECK_BUFFER_MS;
346-362: Consider wrapping parse_template call in try-catch.The
parseTemplatecall could throw if Templater encounters invalid syntax or other errors. Currently, this would propagate up and potentially break the calling workflow. Consider catching and logging the error while returning the original template content as a fallback.- return await parseTemplate( - // `run_mode: 4` maps to Templater's internal `RunMode.DynamicProcessor`. - { target_file: targetFile, run_mode: 4 }, - templateContent, - ); + try { + return await parseTemplate( + // `run_mode: 4` maps to Templater's internal `RunMode.DynamicProcessor`. + { target_file: targetFile, run_mode: 4 }, + templateContent, + ); + } catch (err) { + log.logWarning( + `templaterParseTemplate: failed to parse template – ${(err as Error).message}`, + ); + return templateContent; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/utilityObsidian.ts(4 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/utilityObsidian.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/utilityObsidian.ts
src/**/*.{ts,tsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
Use PascalCase for classes and Svelte components
Files:
src/utilityObsidian.ts
🧠 Learnings (1)
📚 Learning: 2025-12-09T21:20:52.398Z
Learnt from: CR
Repo: chhoumann/quickadd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T21:20:52.398Z
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/utilityObsidian.ts
🧬 Code graph analysis (1)
src/utilityObsidian.ts (3)
tests/obsidian-stub.ts (3)
TFile(72-78)App(23-51)WorkspaceLeaf(99-104)docs/static/scripts/migrateDataviewToFrontmatter.js (1)
content(88-88)src/utils/errorUtils.ts (1)
reportError(99-120)
🔇 Additional comments (6)
src/utilityObsidian.ts (6)
25-45: LGTM! Past review concern addressed.The
TemplaterPluginLiketype is now properly exported, resolving the previous concern about TypeScript declaration emit failing when an exported function returns a non-exported type.
83-91: LGTM!Clean accessor functions for Templater plugin state. The double cast pattern (
as unknown as TemplaterPluginLike) is appropriate for duck-typing external plugin APIs where the actual type isn't available.
201-256: LGTM!The file stability detection logic is well-designed with:
- Adaptive polling with exponential backoff to reduce I/O
- Separate handling for "no changes observed" (grace period) vs "changes stopped" (quiet period)
- Proper error handling with graceful fallback
258-290: LGTM!The per-file serialization lock is correctly implemented:
- Promise chaining ensures sequential execution per path
- Cleanup correctly checks identity before deletion to avoid race conditions
- Error handling prevents unhandled rejections while still propagating errors from
fn()
320-322: Heuristic tag detection may have false positives.The
original.includes("<%")check could match<%appearing in code blocks, inline code, or string literals, not just Templater syntax. This is likely acceptable as a conservative heuristic (better to run Templater unnecessarily than skip when needed), but worth noting if performance becomes a concern.
364-398: LGTM!The cursor jump implementation correctly:
- Guards against non-MD files and inactive files
- Attempts the plugin API first with error handling
- Falls back to command execution only when the setting is enabled
- Fails silently as appropriate for a best-effort "if possible" function
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/engine/CaptureChoiceEngine.notice.test.ts(1 hunks)src/engine/TemplateChoiceEngine.notice.test.ts(1 hunks)src/utilityObsidian.ts(4 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/CaptureChoiceEngine.notice.test.tssrc/engine/TemplateChoiceEngine.notice.test.tssrc/utilityObsidian.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/CaptureChoiceEngine.notice.test.tssrc/engine/TemplateChoiceEngine.notice.test.tssrc/utilityObsidian.ts
src/**/*.{ts,tsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
Use PascalCase for classes and Svelte components
Files:
src/engine/CaptureChoiceEngine.notice.test.tssrc/engine/TemplateChoiceEngine.notice.test.tssrc/utilityObsidian.ts
🧠 Learnings (2)
📚 Learning: 2025-12-09T21:20:52.398Z
Learnt from: CR
Repo: chhoumann/quickadd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T21:20:52.398Z
Learning: Applies to tests/**/*.{ts,tsx} : Add regression coverage for bug fixes
Applied to files:
src/engine/CaptureChoiceEngine.notice.test.tssrc/engine/TemplateChoiceEngine.notice.test.ts
📚 Learning: 2025-12-09T21:20:52.398Z
Learnt from: CR
Repo: chhoumann/quickadd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T21:20:52.398Z
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/CaptureChoiceEngine.notice.test.tssrc/engine/TemplateChoiceEngine.notice.test.tssrc/utilityObsidian.ts
🧬 Code graph analysis (1)
src/utilityObsidian.ts (2)
tests/obsidian-stub.ts (2)
App(23-51)WorkspaceLeaf(99-104)src/utils/errorUtils.ts (1)
reportError(99-120)
🔇 Additional comments (11)
src/engine/CaptureChoiceEngine.notice.test.ts (2)
68-81: Mock return type aligns with newopenExistingFileTabcontract.Returning
null(vsfalse) matches the updatedopenExistingFileTab(...): WorkspaceLeaf | nullsignature and avoids truthiness ambiguity in callers.
83-86:three-way-mergemock:__esModule: trueis the right interop fix.src/utilityObsidian.ts (8)
25-45: Good: explicitTemplaterPluginLikeshape keeps integration type-safe.
83-91: Good:getTemplaterPluginnull-guards +isTemplaterTriggerOnCreateEnabledare clean.
206-261:waitForFileToStopChangingpolling/backoff strategy is reasonable (grace-period vs quiet-period is a nice touch).
263-295: Per-path single-flight lock is a good fit for Templater “whole file” serialization.
297-349: Rollback-on-failure + lock + settle makesoverwriteTemplaterOncemuch safer.
824-853:openExistingFileTab(..., focus?): WorkspaceLeaf | nullis a nice API improvement (and matches the test mocks).
351-367:templaterParseTemplate:run_mode: 4correctly maps to Templater'sRunMode.DynamicProcessor.
The mapping is accurate and requires no changes.
93-204: Reliance on undocumented Templater internals requires version pinning and testing.The code depends on
files_with_pending_templates(undocumented internal Set) andfunctions_generator.teardown()(not a documented Templater API). While the"templater:all-templates-executed"event is documented, the overall suppression strategy relies on Templater internals without a stable public API contract. Pin thetemplater-obsidianversion and add integration tests to catch breaking changes in future releases.src/engine/TemplateChoiceEngine.notice.test.ts (1)
78-85: Mock updated correctly foropenExistingFileTab(...): WorkspaceLeaf | null.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
src/utilityObsidian.templater-binding.test.ts (2)
1-4: Consider moving this test totests/and usingtests/obsidian-stub.tsinstead of importingobsidian.
Importing realobsidianclasses in unit tests can be brittle outside Obsidian, and the repo learnings indicate using the stubs + keeping tests undertests/. Based on learnings, usetests/obsidian-stub.tsand relocate this file accordingly.Also applies to: 5-29, 31-58
14-21: Strengthen the assertions by verifying the arguments too (not justthis).
Right now the tests only validate the boundthiscontext; consider also asserting theoptsobject includestarget_file === fileandrun_mode === 4, and thatjump_to_next_cursor_locationreceives(file, true)to lock in the contract.Also applies to: 42-49
src/engine/CaptureChoiceEngine.ts (3)
142-160: Avoid variable shadowing ofcontentinside the editor-insert branch.
This block introduces a newconst content = ...which shadows the earlier capturecontentvariable inrun(). Renaming torenderedCaptureContent(or similar) would reduce confusion.
142-149: Decide whethertemplaterParseTemplate()failures should fall back to raw insertion.
Currently, if Templater throws while parsing the snippet, the whole capture run will error. If the intended UX is “capture still succeeds but without templater expansion,” wrap parsing intry/catchand insertcaptureContenton failure (and log/report).
181-190: Cursor-jump call is fine; consider gating it onfocusto avoid a no-op call.
Iffocus === false,jumpToNextTemplaterCursorIfPossible()will early-return because the file won’t be active; you could skip calling it in that case.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/engine/CaptureChoiceEngine.ts(5 hunks)src/utilityObsidian.templater-binding.test.ts(1 hunks)src/utilityObsidian.ts(4 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/utilityObsidian.templater-binding.test.tssrc/engine/CaptureChoiceEngine.tssrc/utilityObsidian.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/utilityObsidian.templater-binding.test.tssrc/engine/CaptureChoiceEngine.tssrc/utilityObsidian.ts
src/**/*.{ts,tsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
Use PascalCase for classes and Svelte components
Files:
src/utilityObsidian.templater-binding.test.tssrc/engine/CaptureChoiceEngine.tssrc/utilityObsidian.ts
🧠 Learnings (5)
📚 Learning: 2025-12-09T21:20:52.398Z
Learnt from: CR
Repo: chhoumann/quickadd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T21:20:52.398Z
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/utilityObsidian.templater-binding.test.tssrc/utilityObsidian.ts
📚 Learning: 2025-12-09T21:20:52.398Z
Learnt from: CR
Repo: chhoumann/quickadd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T21:20:52.398Z
Learning: Applies to tests/**/*.{ts,tsx} : Add regression coverage for bug fixes
Applied to files:
src/utilityObsidian.templater-binding.test.ts
📚 Learning: 2025-12-09T21:20:52.398Z
Learnt from: CR
Repo: chhoumann/quickadd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T21:20:52.398Z
Learning: Applies to tests/**/*.{ts,tsx} : Co-locate specs with their source or group them under `tests/feature-name`
Applied to files:
src/utilityObsidian.templater-binding.test.ts
📚 Learning: 2025-12-09T21:20:52.398Z
Learnt from: CR
Repo: chhoumann/quickadd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T21:20:52.398Z
Learning: Applies to tests/**/*.{ts,tsx,js,jsx} : Place tests and stubs in `tests/` directory
Applied to files:
src/utilityObsidian.templater-binding.test.ts
📚 Learning: 2025-12-09T21:20:52.398Z
Learnt from: CR
Repo: chhoumann/quickadd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T21:20:52.398Z
Learning: Applies to tests/**/*.test.{ts,tsx} : Use Testing Library helpers for Svelte components
Applied to files:
src/utilityObsidian.templater-binding.test.ts
🧬 Code graph analysis (3)
src/utilityObsidian.templater-binding.test.ts (2)
tests/obsidian-stub.ts (2)
App(23-51)TFile(72-78)src/utilityObsidian.ts (2)
templaterParseTemplate(389-409)jumpToNextTemplaterCursorIfPossible(411-448)
src/engine/CaptureChoiceEngine.ts (1)
src/utilityObsidian.ts (6)
overwriteTemplaterOnce(333-387)openExistingFileTab(873-898)openFile(789-867)jumpToNextTemplaterCursorIfPossible(411-448)isTemplaterTriggerOnCreateEnabled(90-92)waitForTemplaterTriggerOnCreateToComplete(94-127)
src/utilityObsidian.ts (3)
tests/obsidian-stub.ts (3)
TFile(72-78)App(23-51)WorkspaceLeaf(99-104)docs/static/scripts/migrateDataviewToFrontmatter.js (1)
content(88-88)src/utils/errorUtils.ts (1)
reportError(99-120)
🔇 Additional comments (9)
src/engine/CaptureChoiceEngine.ts (3)
134-167: Nice: snippet-only Templater parsing for editor insert actions matches the determinism goal.
ThetemplaterParseTemplate()call beforeappendToCurrentLine/insertOnNewLine*cleanly avoids unintended whole-file side effects.
161-167: Good: whole-file Templater is now explicitly opt-in viatemplater.afterCapture === "wholeFile".
This makes the side effect controllable and keeps default behavior deterministic.
451-471: Good: suppression + “wait for trigger-on-create completion” reduces on-create races.
UsingsuppressTemplaterOnCreatewhen QuickAdd is creating from a template (and otherwise waiting for trigger-on-create) fits the PR’s “avoid interleaving writes” objective.src/utilityObsidian.ts (6)
25-46:TemplaterPluginLike+getTemplaterPlugin()are a solid boundary for safe integration.
This makes the rest of the codebase much less error-prone around optional plugin presence.Also applies to: 84-92
94-127: Good fallback strategy inwaitForTemplaterTriggerOnCreateToComplete()(Set-driven when available, mtime-driven otherwise).
This is pragmatic for differing Templater internals / versions.
299-331: Per-file single-flight inwithTemplaterFileLock()is a good fit for preventing overlapping whole-file renders.
The lock chaining + cleanup looks correct for async concurrency and prevents interleavedoverwriteTemplaterOnce()calls per path.Also applies to: 333-387
411-448:jumpToNextTemplaterCursorIfPossible()behavior and binding look right (API-first with guarded fallback).
Nice use ofauto_jump_to_cursorgating +.call(editorHandler, ...)preservation.
869-898: API improvement:openExistingFileTab()returningWorkspaceLeaf | null+ optional focusing is clearer than boolean.
This should make call sites (like Capture/Template choice engines) more explicit about what they got back.
141-177: Reconsider the timing assumption: Templater checksfiles_with_pending_templateson create-file events, not on a ~300ms timer.The
TEMPLATER_PENDING_CHECK_BUFFER_MS = 350constant assumes Templater checksfiles_with_pending_templates~300ms after file creation and is held slightly longer to ensure the bypass is observed. However, Templater's actual behavior is event-driven: it checksfiles_with_pending_templatesimmediately when Obsidian emits the create-file event, not after a delay. The 350ms buffer may not be necessary and could mask the real suppression mechanism.The workspace event
templater:all-templates-executedand thefunctions_generator.teardown()call are documented and correct approaches for signaling and cleanup, but the timing strategy should be re-evaluated to ensure it aligns with Templater's event-driven architecture rather than an assumed timer-based check.Also applies to: 179-240
|
🎉 This PR is included in version 2.9.2 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
|
🚀 Release has been published: v2.9.2 |
1 participant
Summary
This PR makes QuickAdd’s Templater integration deterministic by splitting snippet rendering from whole‑file rendering, and by only doing whole‑file renders when QuickAdd truly “owns” the file content.
The two core behavior changes:
Capture into an existing file no longer triggers whole‑file Templater by default.
parse_template(...)(as it already does for capture payloads in most non-editor flows).<% %>elsewhere in the destination note (including fenced code blocks) from executing.Template append/prepend into an existing file now renders Templater on the template snippet only, not the entire file.
Additionally, this PR reduces race conditions for “trigger on file creation” workflows and makes
tp.file.cursor()jumps more reliable by invoking the jump after the file is opened and active.Motivation / root causes
Several long‑running issues stem from the same architectural problem: QuickAdd sometimes runs whole‑file Templater after a write, which:
<% %>anywhere in the destination file (even inside fenced code blocks), andKey observed patterns:
What changed
1) Capture: stop whole‑file render by default (safe‑by‑default)
CaptureChoiceEngine.run()no longer unconditionally callsoverwriteTemplaterOnce(...)aftervault.modify(...)for non-editor insertion actions.templater.afterCapture = "wholeFile" | "none"(default"none").Rationale: capture formatting already parses the capture payload with Templater where needed; running whole‑file Templater afterward was executing unrelated code elsewhere in the destination note.
2) Template append/prepend: snippet‑only render
TemplateEngine.appendToFileWithTemplate(...)now runstemplaterParseTemplate(...)on the formatted template snippet before inserting it.overwriteTemplaterOnce(...)call for append/prepend.Rationale: appending a template should not execute unrelated
<% %>that already exists in the destination file.3) New-file capture + Templater trigger-on-create: wait before anchoring
When QuickAdd creates an empty file (create-without-template) and the user has Templater’s “trigger on file creation” enabled:
Rationale: Templater’s on-create handler intentionally runs after a delay and may write folder templates/includes that create the insert-after anchor. Waiting reduces “first run fails, second run works”.
4) Cursor jump reliability
After QuickAdd opens a file (Capture/Template workflows):
auto_jump_to_cursorsetting is enabled, QuickAdd triggers a jump to the next cursor location.editor_handler.jump_to_next_cursor_location(file, true)), with a guarded command fallback.Rationale: Templater’s auto-jump returns early if the target file isn’t the active file (timing/focus gated).
5) Centralized utility helpers + locking
In
src/utilityObsidian.ts:getTemplaterPlugin) + trigger-on-create reader.waitForFileToStopChanging(...)for delayed external edits (Templater’s ~300ms on-create delay).overwriteTemplaterOnce(...)with:skipIfNoTags(default true) to avoid unnecessary runs,openExistingFileTab(app, file, focus)that respects focus and returns the leaf (ornull).Issue impact
Fixes / addresses
<% %>elsewhere won’t execute.Partial mitigation
Not addressed
<% %>(invalid JS). That needs a dedicated escaping/quoting strategy and is out of scope here.Behavior changes / compatibility
<% %>elsewhere in the destination note can re-enable legacy behavior via the new toggle.Test plan
bun run testbun run build-with-lintRecommended manual sanity checks (Obsidian):
<% %>inside a fenced code block → ensure it does NOT execute.<% %>→ ensure only inserted snippet is rendered.tp.file.cursor()+ auto-jump enabled → ensure cursor jumps when QuickAdd opens the file.Open questions / follow-ups
Refs: #715, #844, #140, #171, #608, #149, #848
Summary by CodeRabbit
New Features
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.