feat(templates): support format syntax in the template path (#620)#1337
Merged
Conversation
Resolve QuickAdd format tokens (e.g. {{value:collectionName}}, {{date:YYYY}}) in
a template's source path so a choice can point at
"Templates/{{value:collectionName}} Template.md". The path is resolved once at
each engine's entry via a new path-safe CompleteFormatter.formatTemplateFilePath
and threaded into both target-path construction (extension/name) and the content
read, so the created file and the read template can never disagree.
- formatTemplateFilePath: path-safe token subset (value/date/time/field/global/
selected/clipboard/vdate/random/math); does not run macros, inline JS, or
{{TEMPLATE:}} inclusion; guards {{title}}; leaves {{FOLDER}} literal; trims.
- Covers all template-resolution flows: Template create/overwrite/append,
Capture create-with-template, Insert template, and apply-to-active-note
(which re-validates the resolved extension so a token resolving to
.canvas/.base is not applied to a markdown note).
- Preflight (one-page input + CLI) collects tokens in the template path via a
shared scanTemplateSource (template choices and capture create-with-template).
- Choice builder shows a neutral "resolved at run time" hint instead of a false
"Template not found" for format-syntax paths.
Closes #620
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR enables template paths to contain QuickAdd format tokens and resolves them safely at run time. It threads a memoized, path-safe resolved template source path through formatter helpers, template engines, preflight scanning, UI validators, styles, and tests so extension/lookup and content loading remain consistent. ChangesTemplate path dynamic resolution
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 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📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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 |
Deploying quickadd with
|
| Latest commit: |
00dac66
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://e9500bab.quickadd.pages.dev |
| Branch Preview URL: | https://chhoumann-620-format-syntax.quickadd.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
docs/docs/Choices/TemplateChoice.md (1)
14-20: ⚡ Quick winMissing documentation of unsupported note-relative tokens in template paths.
Both the unreleased and versioned documentation have the same gap: they don't clarify that note-relative tokens (
{{FOLDER}},{{FILENAMECURRENT}},{{LINKCURRENT}}) cannot be used in template paths and will be left literal, causing file-creation failures. Unlike{{title}}, which raises an explicit error, these tokens fail silently with cryptic "invalid filename" errors.
docs/docs/Choices/TemplateChoice.md#L14-L20: Add clarifying note after line 16 that note-relative tokens don't work in template paths.docs/versioned_docs/version-2.13.1/Choices/TemplateChoice.md#L14-L20: Apply the same clarification (keep synchronized per guidelines).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/docs/Choices/TemplateChoice.md` around lines 14 - 20, The docs fail to mention that note-relative tokens are unsupported in dynamic template paths; update both affected files to add a clarifying sentence after line 16 explaining that note-relative tokens ({{FOLDER}}, {{FILENAMECURRENT}}, {{LINKCURRENT}}) are not resolved in template paths and will be left literal (which can produce "invalid filename" failures), unlike {{title}} which raises an explicit error. Change needed in docs/docs/Choices/TemplateChoice.md (lines 14-20) — insert the clarification after line 16; make the identical insertion in docs/versioned_docs/version-2.13.1/Choices/TemplateChoice.md (lines 14-20) so the versioned docs stay synchronized.src/engine/TemplateEngine.ts (1)
700-707: ⚡ Quick winPropagate
resolvedTemplatePathnaming across local helper signatures.Now that this method explicitly requires a pre-resolved path, using
resolvedTemplatePathnaming consistently in nearby helper method parameters (createFileWithTemplate,overwriteFileWithTemplate,appendToFileWithTemplate) would reduce accidental raw-path reuse in future edits.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/engine/TemplateEngine.ts` around lines 700 - 707, The helper methods that accept a template path should use the same pre-resolved name to avoid confusion: rename the template path parameter in createFileWithTemplate, overwriteFileWithTemplate, and appendToFileWithTemplate to resolvedTemplatePath (and update all internal references and callers inside TemplateEngine to pass the already-resolved value returned/used by getTemplateContent), ensuring parameter names, JSDoc/comments, and any local variables match the new name so no callers accidentally pass or expect an unresolved/raw template path.src/utils/templatePathSyntax.ts (1)
13-15: ⚡ Quick winUse kebab-case for this utility filename to match repo convention.
The implementation is good, but the utility file name is camelCase. Please rename
src/utils/templatePathSyntax.tsto kebab-case (and update imports accordingly) to stay aligned with repository naming rules.As per coding guidelines,
**/*: “Use kebab-case for directories and utilities.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/utils/templatePathSyntax.ts` around lines 13 - 15, Rename the utility file from templatePathSyntax.ts to template-path-syntax.ts and update all imports that reference it so they point to the new filename; keep the exported function hasTemplatePathSyntax unchanged. Search the repo for "templatePathSyntax" (imports and any index/barrel exports) and update those import specifiers to "template-path-syntax" (and update any re-exports in barrels). Run a quick TypeScript build or IDE import-fix to ensure no broken references remain.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@docs/docs/Choices/TemplateChoice.md`:
- Around line 14-20: The docs fail to mention that note-relative tokens are
unsupported in dynamic template paths; update both affected files to add a
clarifying sentence after line 16 explaining that note-relative tokens
({{FOLDER}}, {{FILENAMECURRENT}}, {{LINKCURRENT}}) are not resolved in template
paths and will be left literal (which can produce "invalid filename" failures),
unlike {{title}} which raises an explicit error. Change needed in
docs/docs/Choices/TemplateChoice.md (lines 14-20) — insert the clarification
after line 16; make the identical insertion in
docs/versioned_docs/version-2.13.1/Choices/TemplateChoice.md (lines 14-20) so
the versioned docs stay synchronized.
In `@src/engine/TemplateEngine.ts`:
- Around line 700-707: The helper methods that accept a template path should use
the same pre-resolved name to avoid confusion: rename the template path
parameter in createFileWithTemplate, overwriteFileWithTemplate, and
appendToFileWithTemplate to resolvedTemplatePath (and update all internal
references and callers inside TemplateEngine to pass the already-resolved value
returned/used by getTemplateContent), ensuring parameter names, JSDoc/comments,
and any local variables match the new name so no callers accidentally pass or
expect an unresolved/raw template path.
In `@src/utils/templatePathSyntax.ts`:
- Around line 13-15: Rename the utility file from templatePathSyntax.ts to
template-path-syntax.ts and update all imports that reference it so they point
to the new filename; keep the exported function hasTemplatePathSyntax unchanged.
Search the repo for "templatePathSyntax" (imports and any index/barrel exports)
and update those import specifiers to "template-path-syntax" (and update any
re-exports in barrels). Run a quick TypeScript build or IDE import-fix to ensure
no broken references remain.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 785a23ee-c326-426e-9b07-86331127eda4
📒 Files selected for processing (21)
docs/docs/Choices/TemplateChoice.mddocs/versioned_docs/version-2.13.1/Choices/TemplateChoice.mdsrc/engine/SingleTemplateEngine.tssrc/engine/TemplateChoiceEngine.collision.test.tssrc/engine/TemplateChoiceEngine.folderSorting.test.tssrc/engine/TemplateChoiceEngine.notice.test.tssrc/engine/TemplateChoiceEngine.tssrc/engine/TemplateEngine.tssrc/engine/TemplateInsertEngine.test.tssrc/engine/TemplateInsertEngine.tssrc/engine/applyTemplateToActiveNote.test.tssrc/engine/applyTemplateToActiveNote.tssrc/formatters/completeFormatter.test.tssrc/formatters/completeFormatter.tssrc/gui/ChoiceBuilder/CaptureChoiceForm.sveltesrc/gui/ChoiceBuilder/TemplateChoiceForm.sveltesrc/gui/ChoiceBuilder/components/ValidatedInput.sveltesrc/preflight/collectChoiceRequirements.test.tssrc/preflight/collectChoiceRequirements.tssrc/utils/templatePathSyntax.test.tssrc/utils/templatePathSyntax.ts
- Neutral builder hint no longer renders in error red (new
.qa-field-hint--neutral modifier on a valid+message result).
- Move the {{title}} re-check to immediately after global-variable expansion so a
global-injected {{title}} still throws the clear error, without false-positiving
on a user value that merely contains the literal "{{title}}".
- Fix scanTemplateSource traversal: snapshot + clear the shared discovery set
before recursing, so nested/sibling literal templates aren't dropped from
preflight (previously the recursive walk mutated the set it was iterating).
- Drop the unreleased-feature note from the stable 2.13.1 docs snapshot (it ships
in the version that contains it); keep it in the unreleased docs and document
the note-relative tokens ({{FOLDER}}/{{FILENAMECURRENT}}/{{LINKCURRENT}}) left
literal in a template path.
- Add a regression test for the apply-to-note resolved-extension guard and a
neutral-hint test for ValidatedInput.
Owner
Author
Review summaryThis PR was reviewed in three passes (multi-lens ultracode workflow + opposite-model adversarial reviewers on the design, the implementation, and the final diff). No merge-blockers were found; the verified findings were addressed in
New behavior worth calling out: Capture create-with-template bodies are now pre-scanned at preflight via the shared Verified: |
…t-syntax-template-path # Conflicts: # src/preflight/collectChoiceRequirements.ts
…abbit) Rename the templatePath parameter to resolvedTemplatePath in createFileWithTemplate/overwriteFileWithTemplate/appendToFileWithTemplate to make the 'must be pre-resolved' contract explicit end-to-end and prevent accidental raw-path reuse.
Owner
Author
|
@coderabbitai thanks — addressed the nitpicks:
|
|
Tip For best results, initiate chat on the files or code changes.
The PR looks great — all findings addressed. 🎉 (◕‿◕✿) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #620
What & why
Using QuickAdd format syntax in a Template choice's Template Path (e.g.
Templates/{{value:collectionName}} Template.md) previously failed withTemplate file not found— the path was the one note-creation input never run through theformatter (file name, folder, and body already were). This makes the template path dynamic,
extending the recently-shipped
{{FOLDER}}/{{VALUE}}work.Approach
Resolve the path once at each engine's entry and thread the resolved value into both
target-path construction (extension/name) and the content read — so the file that's created
and the template that's read can never disagree (a token expanding to
.canvascreates a canvasfile, not
.md).getTemplateContenttakes a pre-resolved path and no longer formats (avoidsdouble-evaluating
{{date}}/{{random}}).New
CompleteFormatter.formatTemplateFilePathis a deliberately path-safe subset offormat():resolves value/date/time/field/global/selected/clipboard/vdate/random/math, but does not run
macros, inline JavaScript, or
{{TEMPLATE:}}inclusion (a file-path lookup shouldn't execute code).It guards
{{title}}(incl. when injected via a global var), leaves{{FOLDER}}/{{FILENAMECURRENT}}literal, and trims.
Covered flows
resolves to
.canvas/.baseis not applied to a markdown note.quickadd:check/quickadd:run) — collects tokens in thetemplate path via a shared
scanTemplateSource(template choices and capture create-with-template).false "Template not found".
Tests
pnpm test(1901 pass),pnpm lint,pnpm build, andcd docs && pnpm buildall green. Added unitcoverage for
formatTemplateFilePath(path-safe allowlist,{{title}}guard incl. global-injected,trim), the resolved-extension match in
TemplateChoiceEngine, preflight path-token collection(template + capture), and the
hasTemplatePathSyntaxhelper.Verified in the dev vault (e2e)
Real Obsidian, current build:
quickadd:checkreportscollectionName(the path token) as a required input.quickadd:run id=… value-collectionName=Gamescreates the output file from the resolvedTemplates/Games Template.md(and the body's{{value:collectionName}}reuses the same answer).Docs
Documented on the Template choice page (both
nextand the servedversion-2.13.1snapshot),including the path-safe restrictions and the known limitations below.
Known limitations (documented, intentional)
{{TEMPLATE:}}inclusion with a tokenized path isn't supported (the token regex can't capture apath containing
}}).file).
on answers still being collected); those prompts surface at run time.
{{value}}(vs named{{value:x}}) in a path may re-prompt across the capture→templateboundary.
Review
Design and implementation were each adversarially reviewed (opposite-model + multi-lens). The seam
choice (resolve-at-entry vs. formatting inside the read) and the resolved-extension validation came
directly from that review.
Summary by CodeRabbit
New Features
Bug Fixes
Validation
Documentation
Tests