From 436d64e53544098271ca5356f23b9fdbd48eb88a Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Thu, 14 May 2026 20:39:03 +0200 Subject: [PATCH 1/7] refactor: add formatter evaluator boundaries --- src/engine/MacroChoiceEngine.ts | 12 +- src/engine/SingleInlineScriptEngine.ts | 2 +- src/engine/TemplateEngine.ts | 8 +- ...captureChoiceFormatter-frontmatter.test.ts | 35 ---- ...tureChoiceFormatter-write-position.test.ts | 35 ---- ...leteFormatter.evaluator-boundaries.test.ts | 192 ++++++++++++++++++ src/formatters/completeFormatter.ts | 61 +++--- ...playFormatter.evaluator-boundaries.test.ts | 72 +++++++ src/formatters/formatDisplayFormatter.ts | 45 ++-- src/formatters/formatterEvaluators.ts | 37 ++++ src/preflight/runOnePagePreflight.ts | 7 +- src/quickAddApi.ts | 7 +- src/services/FormatterFactory.ts | 145 +++++++++++++ 13 files changed, 525 insertions(+), 133 deletions(-) create mode 100644 src/formatters/completeFormatter.evaluator-boundaries.test.ts create mode 100644 src/formatters/formatDisplayFormatter.evaluator-boundaries.test.ts create mode 100644 src/formatters/formatterEvaluators.ts create mode 100644 src/services/FormatterFactory.ts diff --git a/src/engine/MacroChoiceEngine.ts b/src/engine/MacroChoiceEngine.ts index b9ead15d..c864d3c3 100644 --- a/src/engine/MacroChoiceEngine.ts +++ b/src/engine/MacroChoiceEngine.ts @@ -35,7 +35,7 @@ import type { IAIAssistantCommand } from "src/types/macros/QuickCommands/IAIAssi import { runAIAssistant } from "src/ai/AIAssistant"; import { resolveProviderApiKey } from "src/ai/providerSecrets"; import { settingsStore } from "src/settingsStore"; -import { CompleteFormatter } from "src/formatters/completeFormatter"; +import { FormatterFactory } from "src/services/FormatterFactory"; import { getModelByName, getModelNames, @@ -580,11 +580,10 @@ export class MacroChoiceEngine extends QuickAddChoiceEngine { throw new Error(`Model ${modelName} not found with any provider.`); } - const formatter = new CompleteFormatter( + const formatter = new FormatterFactory( this.app, QuickAdd.instance, - this.choiceExecutor - ); + ).createCompleteFormatter(this.choiceExecutor); const modelProvider = getModelProvider(model.name); @@ -725,11 +724,10 @@ export class MacroChoiceEngine extends QuickAddChoiceEngine { private async executeOpenFile(command: IOpenFileCommand) { try { - const formatter = new CompleteFormatter( + const formatter = new FormatterFactory( this.app, QuickAdd.instance, - this.choiceExecutor - ); + ).createCompleteFormatter(this.choiceExecutor); const resolvedPath = await formatter.formatFileName(command.filePath, ""); const normalizedPath = resolvedPath.replace(/\\/g, "/"); diff --git a/src/engine/SingleInlineScriptEngine.ts b/src/engine/SingleInlineScriptEngine.ts index a5363e4c..3449add8 100644 --- a/src/engine/SingleInlineScriptEngine.ts +++ b/src/engine/SingleInlineScriptEngine.ts @@ -10,7 +10,7 @@ export class SingleInlineScriptEngine extends MacroChoiceEngine { app: App, plugin: QuickAdd, choiceExecutor: IChoiceExecutor, - variables: Map + variables: Map ) { //@ts-ignore super(app, plugin, null, choiceExecutor, variables); diff --git a/src/engine/TemplateEngine.ts b/src/engine/TemplateEngine.ts index c9152e6f..0dda165a 100644 --- a/src/engine/TemplateEngine.ts +++ b/src/engine/TemplateEngine.ts @@ -1,5 +1,6 @@ import { QuickAddEngine } from "./QuickAddEngine"; -import { CompleteFormatter } from "../formatters/completeFormatter"; +import type { CompleteFormatter } from "../formatters/completeFormatter"; +import { FormatterFactory } from "../services/FormatterFactory"; import type { LinkToCurrentFileBehavior } from "../formatters/formatter"; import type { App } from "obsidian"; import { Notice, TFile } from "obsidian"; @@ -85,7 +86,10 @@ export abstract class TemplateEngine extends QuickAddEngine { ) { super(app); this.templater = getTemplater(app); - this.formatter = new CompleteFormatter(app, plugin, choiceFormatter); + this.formatter = new FormatterFactory( + app, + plugin, + ).createCompleteFormatter(choiceFormatter); } public abstract run(): diff --git a/src/formatters/captureChoiceFormatter-frontmatter.test.ts b/src/formatters/captureChoiceFormatter-frontmatter.test.ts index ab3c08d0..b7747c34 100644 --- a/src/formatters/captureChoiceFormatter-frontmatter.test.ts +++ b/src/formatters/captureChoiceFormatter-frontmatter.test.ts @@ -50,41 +50,6 @@ vi.mock('../gui/MathModal', () => ({ }, })); -vi.mock('../engine/SingleInlineScriptEngine', () => ({ - __esModule: true, - SingleInlineScriptEngine: class { - public params = { variables: {} as Record }; - constructor() {} - async runAndGetOutput() { - return ''; - } - }, -})); - -vi.mock('../engine/SingleMacroEngine', () => ({ - __esModule: true, - SingleMacroEngine: class { - constructor() {} - async runAndGetOutput() { - return ''; - } - }, -})); - -vi.mock('../engine/SingleTemplateEngine', () => ({ - __esModule: true, - SingleTemplateEngine: class { - constructor() {} - async run() { - return ''; - } - getAndClearTemplatePropertyVars() { - return new Map(); - } - setLinkToCurrentFileBehavior() {} - }, -})); - vi.mock('obsidian-dataview', () => ({ __esModule: true, getAPI: vi.fn().mockReturnValue(null), diff --git a/src/formatters/captureChoiceFormatter-write-position.test.ts b/src/formatters/captureChoiceFormatter-write-position.test.ts index 18fad173..b7e8007a 100644 --- a/src/formatters/captureChoiceFormatter-write-position.test.ts +++ b/src/formatters/captureChoiceFormatter-write-position.test.ts @@ -50,41 +50,6 @@ vi.mock("../gui/MathModal", () => ({ }, })); -vi.mock("../engine/SingleInlineScriptEngine", () => ({ - __esModule: true, - SingleInlineScriptEngine: class { - public params = { variables: {} as Record }; - constructor() {} - async runAndGetOutput() { - return ""; - } - }, -})); - -vi.mock("../engine/SingleMacroEngine", () => ({ - __esModule: true, - SingleMacroEngine: class { - constructor() {} - async runAndGetOutput() { - return ""; - } - }, -})); - -vi.mock("../engine/SingleTemplateEngine", () => ({ - __esModule: true, - SingleTemplateEngine: class { - constructor() {} - async run() { - return ""; - } - getAndClearTemplatePropertyVars() { - return new Map(); - } - setLinkToCurrentFileBehavior() {} - }, -})); - vi.mock("obsidian-dataview", () => ({ __esModule: true, getAPI: vi.fn().mockReturnValue(null), diff --git a/src/formatters/completeFormatter.evaluator-boundaries.test.ts b/src/formatters/completeFormatter.evaluator-boundaries.test.ts new file mode 100644 index 00000000..ad422b3b --- /dev/null +++ b/src/formatters/completeFormatter.evaluator-boundaries.test.ts @@ -0,0 +1,192 @@ +import { describe, expect, it, vi } from "vitest"; +import { CompleteFormatter } from "./completeFormatter"; +import type { + CompleteFormatterEvaluators, + FormatterVariables, +} from "./formatterEvaluators"; +import { MacroAbortError } from "../errors/MacroAbortError"; + +vi.mock("src/gui/GenericInputPrompt/GenericInputPrompt", () => ({ + default: { PromptWithContext: vi.fn() }, +})); +vi.mock("src/gui/InputSuggester/inputSuggester", () => ({ + default: { Suggest: vi.fn() }, +})); +vi.mock("src/gui/VDateInputPrompt/VDateInputPrompt", () => ({ + default: { Prompt: vi.fn() }, +})); +vi.mock("../gui/GenericSuggester/genericSuggester", () => ({ + default: { Suggest: vi.fn() }, +})); +vi.mock("../gui/InputPrompt", () => ({ + default: class { + factory() { + return { Prompt: vi.fn(), PromptWithContext: vi.fn() }; + } + }, +})); +vi.mock("../gui/MathModal", () => ({ + MathModal: { Prompt: vi.fn() }, +})); +vi.mock("../utils/FieldValueCollector", () => ({ + collectFieldValuesProcessedDetailed: vi.fn(async () => ({ + values: [], + hasDefaultValue: false, + })), + collectFieldValuesRaw: vi.fn(async () => new Set()), + generateFieldCacheKey: vi.fn(() => "cache-key"), +})); +vi.mock("../utils/FieldValueProcessor", () => ({ + FieldValueProcessor: { getSmartDefaults: vi.fn(() => []) }, +})); + +const app = { + workspace: { + getActiveViewOfType: vi.fn(() => null), + getActiveFile: vi.fn(() => null), + }, + fileManager: { + generateMarkdownLink: vi.fn(), + }, +} as any; + +function makeExecutor(variables: FormatterVariables) { + return { + variables, + signalAbort: vi.fn(), + } as any; +} + +function makePlugin() { + return { + settings: { + choices: [], + globalVariables: { + global: "{{VALUE:fromGlobal}}", + }, + enableTemplatePropertyTypes: false, + }, + } as any; +} + +describe("CompleteFormatter evaluator boundaries", () => { + it("delegates macro, template, and inline tokens with same variables map and ordered processing", async () => { + const variables = new Map([ + ["fromGlobal", "global-value"], + ]); + const calls: string[] = []; + const observedMaps: FormatterVariables[] = []; + const evaluators: CompleteFormatterEvaluators = { + inlineJavaScript: { + evaluateInlineJavaScript: async (code, context) => { + calls.push(`inline:${code}`); + observedMaps.push(context.variables); + context.variables.set("fromInline", "inline-value"); + return "{{MACRO:Next|label:Macro label}}"; + }, + }, + macro: { + evaluateMacro: async (macroName, context) => { + calls.push(`macro:${macroName}:${context.label ?? ""}`); + observedMaps.push(context.variables); + context.variables.set("fromMacro", "macro-value"); + return "{{TEMPLATE:Templates/Example.md}}"; + }, + }, + template: { + evaluateTemplate: async (templatePath, context) => { + calls.push(`template:${templatePath}`); + observedMaps.push(context.variables); + context.variables.set("fromTemplate", "template-value"); + return [ + "{{GLOBAL_VAR:global}}", + "{{VALUE:fromInline}}", + "{{VALUE:fromMacro}}", + "{{VALUE:fromTemplate}}", + "```js quickadd\nreturn 'late';\n```", + "{{MACRO:Late}}", + ].join("|"); + }, + }, + }; + + const formatter = new CompleteFormatter( + app, + makePlugin(), + makeExecutor(variables), + undefined, + evaluators, + ); + + await expect( + formatter.formatFileContent("```js quickadd\nreturn 'first';\n```"), + ).resolves.toBe( + [ + "global-value", + "inline-value", + "macro-value", + "template-value", + "```js quickadd\nreturn 'late';\n```", + "{{MACRO:Late}}", + ].join("|"), + ); + expect(calls).toEqual([ + "inline:return 'first';", + "macro:Next:Macro label", + "template:Templates/Example.md", + ]); + expect(observedMaps).toEqual([variables, variables, variables]); + }); + + it("maps nullish macro and non-string inline results to empty strings", async () => { + const evaluators: CompleteFormatterEvaluators = { + inlineJavaScript: { + evaluateInlineJavaScript: async () => 42, + }, + macro: { + evaluateMacro: async () => null, + }, + template: { + evaluateTemplate: async () => "", + }, + }; + const formatter = new CompleteFormatter( + app, + makePlugin(), + makeExecutor(new Map()), + undefined, + evaluators, + ); + + await expect( + formatter.formatFileContent("a```js quickadd\nreturn 42;\n```b{{MACRO:None}}c"), + ).resolves.toBe("abc"); + }); + + it("propagates runtime evaluator errors and aborts", async () => { + const abort = new MacroAbortError("stop"); + const evaluators: CompleteFormatterEvaluators = { + inlineJavaScript: { + evaluateInlineJavaScript: async () => { + throw abort; + }, + }, + macro: { + evaluateMacro: async () => "", + }, + template: { + evaluateTemplate: async () => "", + }, + }; + const formatter = new CompleteFormatter( + app, + makePlugin(), + makeExecutor(new Map()), + undefined, + evaluators, + ); + + await expect(formatter.formatFileContent("```js quickadd\nthrow new Error();\n```")) + .rejects.toBe(abort); + }); +}); diff --git a/src/formatters/completeFormatter.ts b/src/formatters/completeFormatter.ts index eefb578a..7d986cc5 100644 --- a/src/formatters/completeFormatter.ts +++ b/src/formatters/completeFormatter.ts @@ -5,9 +5,6 @@ import InputSuggester from "src/gui/InputSuggester/inputSuggester"; import VDateInputPrompt from "src/gui/VDateInputPrompt/VDateInputPrompt"; import type { IChoiceExecutor } from "../IChoiceExecutor"; import { GLOBAL_VAR_REGEX, INLINE_JAVASCRIPT_REGEX } from "../constants"; -import { SingleInlineScriptEngine } from "../engine/SingleInlineScriptEngine"; -import { SingleMacroEngine } from "../engine/SingleMacroEngine"; -import { SingleTemplateEngine } from "../engine/SingleTemplateEngine"; import GenericSuggester from "../gui/GenericSuggester/genericSuggester"; import InputPrompt from "../gui/InputPrompt"; import { MathModal } from "../gui/MathModal"; @@ -27,6 +24,7 @@ import { FieldValueProcessor } from "../utils/FieldValueProcessor"; import { Formatter, type PromptContext } from "./formatter"; import { MacroAbortError } from "../errors/MacroAbortError"; import { isCancellationError } from "../utils/errorUtils"; +import type { CompleteFormatterEvaluators } from "./formatterEvaluators"; export class CompleteFormatter extends Formatter { private valueHeader: string; @@ -36,6 +34,7 @@ export class CompleteFormatter extends Formatter { private plugin: QuickAdd, protected choiceExecutor?: IChoiceExecutor, dateParser?: IDateParser, + private readonly evaluators?: CompleteFormatterEvaluators, ) { super(app); this.dateParser = dateParser || NLDParser; @@ -347,32 +346,26 @@ export class CompleteFormatter extends Formatter { macroName: string, context?: { label?: string }, ): Promise { - const macroEngine: SingleMacroEngine = new SingleMacroEngine( - this.app, - this.plugin, - this.plugin.settings.choices, - //@ts-ignore - this.choiceExecutor, - this.variables, - ); - const macroOutput = - (await macroEngine.runAndGetOutput(macroName, context)) ?? ""; - - // Copy variables from macro execution - macroEngine.getVariables().forEach((value, key) => { - this.variables.set(key, value); + if (!this.evaluators) { + throw new Error("CompleteFormatter macro evaluator is not configured."); + } + + const macroOutput = await this.evaluators.macro.evaluateMacro(macroName, { + variables: this.variables, + ...(context?.label ? { label: context.label } : {}), }); - return macroOutput; + return macroOutput == null ? "" : String(macroOutput); } protected async getTemplateContent(templatePath: string): Promise { - return await new SingleTemplateEngine( - this.app, - this.plugin, - templatePath, - this.choiceExecutor, - ).run(); + if (!this.evaluators) { + throw new Error("CompleteFormatter template evaluator is not configured."); + } + + return await this.evaluators.template.evaluateTemplate(templatePath, { + variables: this.variables, + }); } protected async getSelectedText(): Promise { @@ -403,18 +396,16 @@ export class CompleteFormatter extends Formatter { const code = match?.at(1)?.trim(); if (code) { - const executor = new SingleInlineScriptEngine( - this.app, - this.plugin, - //@ts-ignore - this.choiceExecutor, - this.variables, - ); - const outVal: unknown = await executor.runAndGetOutput(code); - - for (const key in executor.params.variables) { - this.variables.set(key, executor.params.variables[key]); + if (!this.evaluators) { + throw new Error( + "CompleteFormatter inline JavaScript evaluator is not configured.", + ); } + const outVal: unknown = + await this.evaluators.inlineJavaScript.evaluateInlineJavaScript( + code, + { variables: this.variables }, + ); output = typeof outVal === "string" diff --git a/src/formatters/formatDisplayFormatter.evaluator-boundaries.test.ts b/src/formatters/formatDisplayFormatter.evaluator-boundaries.test.ts new file mode 100644 index 00000000..6b2ec21e --- /dev/null +++ b/src/formatters/formatDisplayFormatter.evaluator-boundaries.test.ts @@ -0,0 +1,72 @@ +import { describe, expect, it, vi } from "vitest"; +import { FormatDisplayFormatter } from "./formatDisplayFormatter"; +import type { FormatDisplayFormatterEvaluators } from "./formatterEvaluators"; + +const app = { + workspace: { + getActiveFile: vi.fn(() => null), + }, +} as any; + +const plugin = { + settings: { + globalVariables: {}, + }, +} as any; + +describe("FormatDisplayFormatter evaluator boundaries", () => { + it("uses preview-safe template evaluator output without running runtime macros or inline scripts", async () => { + const evaluators: FormatDisplayFormatterEvaluators = { + template: { + evaluateTemplate: vi.fn(async () => + "{{MACRO:Runtime}} ```js quickadd\nthrow new Error('side effect');\n```", + ), + }, + }; + const formatter = new FormatDisplayFormatter( + app, + plugin, + undefined, + evaluators, + ); + + await expect( + formatter.format("Preview {{TEMPLATE:Templates/Safe}}"), + ).resolves.toBe( + "Preview {{TEMPLATE:Templates/Safe}}", + ); + await expect( + formatter.format("Preview {{TEMPLATE:Templates/Safe.md}}"), + ).resolves.toBe( + "Preview {{MACRO:Runtime}} ```js quickadd\nthrow new Error('side effect');\n```", + ); + expect(evaluators.template.evaluateTemplate).toHaveBeenLastCalledWith( + "Templates/Safe.md", + expect.objectContaining({ variables: expect.any(Map) }), + ); + }); + + it("returns safe template fallback when preview evaluation fails", async () => { + const formatter = new FormatDisplayFormatter(app, plugin, undefined, { + template: { + evaluateTemplate: async () => { + throw new Error("read failed"); + }, + }, + }); + + await expect(formatter.format("{{TEMPLATE:Missing.md}}")).resolves.toBe( + "Template (not found): Missing.md", + ); + }); + + it("returns original input for unexpected preview failures", async () => { + const formatter = new FormatDisplayFormatter(app, plugin, undefined, { + template: { + evaluateTemplate: async () => "unused", + }, + }); + + await expect(formatter.format("{{DATE:")).resolves.toBe("{{DATE:"); + }); +}); diff --git a/src/formatters/formatDisplayFormatter.ts b/src/formatters/formatDisplayFormatter.ts index bcfdb4f8..6b9d103a 100644 --- a/src/formatters/formatDisplayFormatter.ts +++ b/src/formatters/formatDisplayFormatter.ts @@ -1,7 +1,6 @@ import { Formatter, type PromptContext } from "./formatter"; import type { App } from "obsidian"; import type QuickAdd from "../main"; -import { SingleTemplateEngine } from "../engine/SingleTemplateEngine"; import { DATE_VARIABLE_REGEX, GLOBAL_VAR_REGEX } from "../constants"; import type { IDateParser } from "../parsers/IDateParser"; import { NLDParser } from "../parsers/NLDParser"; @@ -15,12 +14,14 @@ import { DateFormatPreviewGenerator } from "./helpers/previewHelpers"; import { getValueVariableBaseName } from "../utils/valueSyntax"; +import type { FormatDisplayFormatterEvaluators } from "./formatterEvaluators"; export class FormatDisplayFormatter extends Formatter { constructor( app: App, private readonly plugin: QuickAdd, dateParser?: IDateParser, + private readonly evaluators?: FormatDisplayFormatterEvaluators, ) { super(app); this.dateParser = dateParser || NLDParser; @@ -122,23 +123,43 @@ export class FormatDisplayFormatter extends Formatter { } protected async getTemplateContent(templatePath: string): Promise { - const app = this.app; - if (!app) { - return `Template (app unavailable): ${templatePath}`; - } - try { - return await new SingleTemplateEngine( - app, - this.plugin, - templatePath, - undefined, - ).run(); + return await ( + this.evaluators?.template ?? this.createDefaultTemplatePreviewEvaluator() + ).evaluateTemplate(templatePath, { + variables: this.variables, + }); } catch { return `Template (not found): ${templatePath}`; } } + private createDefaultTemplatePreviewEvaluator(): FormatDisplayFormatterEvaluators["template"] { + return { + evaluateTemplate: async (templatePath) => { + const vault = this.app?.vault; + if (!vault) { + return `Template (not found): ${templatePath}`; + } + const resolvedPath = templatePath.replace(/^\/+/, ""); + const file = vault.getAbstractFileByPath(resolvedPath); + if ( + !file || + typeof file !== "object" || + !("path" in file) || + !("extension" in file) + ) { + return `Template (not found): ${templatePath}`; + } + try { + return await vault.cachedRead(file as never); + } catch { + return `Template (not found): ${templatePath}`; + } + }, + }; + } + protected async getSelectedText(): Promise { return "selected_text"; diff --git a/src/formatters/formatterEvaluators.ts b/src/formatters/formatterEvaluators.ts new file mode 100644 index 00000000..93ecd12b --- /dev/null +++ b/src/formatters/formatterEvaluators.ts @@ -0,0 +1,37 @@ +export type FormatterVariables = Map; + +export interface FormatterEvaluatorContext { + variables: FormatterVariables; + label?: string; +} + +export interface MacroTokenEvaluator { + evaluateMacro( + macroName: string, + context: FormatterEvaluatorContext, + ): Promise; +} + +export interface TemplateTokenEvaluator { + evaluateTemplate( + templatePath: string, + context: FormatterEvaluatorContext, + ): Promise; +} + +export interface InlineJavaScriptTokenEvaluator { + evaluateInlineJavaScript( + code: string, + context: FormatterEvaluatorContext, + ): Promise; +} + +export interface CompleteFormatterEvaluators { + macro: MacroTokenEvaluator; + template: TemplateTokenEvaluator; + inlineJavaScript: InlineJavaScriptTokenEvaluator; +} + +export interface FormatDisplayFormatterEvaluators { + template: TemplateTokenEvaluator; +} diff --git a/src/preflight/runOnePagePreflight.ts b/src/preflight/runOnePagePreflight.ts index 5622c34b..366c78a0 100644 --- a/src/preflight/runOnePagePreflight.ts +++ b/src/preflight/runOnePagePreflight.ts @@ -1,7 +1,7 @@ import type { App } from "obsidian"; import type { IChoiceExecutor } from "src/IChoiceExecutor"; -import { FormatDisplayFormatter } from "src/formatters/formatDisplayFormatter"; import type QuickAdd from "src/main"; +import { FormatterFactory } from "src/services/FormatterFactory"; import type IChoice from "src/types/choices/IChoice"; import type ITemplateChoice from "src/types/choices/ITemplateChoice"; import { OnePageInputModal } from "./OnePageInputModal"; @@ -38,7 +38,10 @@ export async function runOnePagePreflight( // Optional live preview of a couple of key outputs (best-effort) const computePreview = async (values: Record) => { try { - const formatter = new FormatDisplayFormatter(app, plugin); + const formatter = new FormatterFactory( + app, + plugin, + ).createDisplayFormatter(); const out: Record = {}; // File name preview for Template if (choice.type === "Template") { diff --git a/src/quickAddApi.ts b/src/quickAddApi.ts index 1ed402ae..bbc8efdf 100644 --- a/src/quickAddApi.ts +++ b/src/quickAddApi.ts @@ -17,7 +17,7 @@ import { import type { OpenAIModelParameters } from "./ai/OpenAIModelParameters"; import type { Model } from "./ai/Provider"; import { resolveProviderApiKey } from "./ai/providerSecrets"; -import { CompleteFormatter } from "./formatters/completeFormatter"; +import { FormatterFactory } from "./services/FormatterFactory"; import GenericCheckboxPrompt from "./gui/GenericCheckboxPrompt/genericCheckboxPrompt"; import GenericInfoDialog from "./gui/GenericInfoDialog/GenericInfoDialog"; import GenericInputPrompt from "./gui/GenericInputPrompt/GenericInputPrompt"; @@ -247,11 +247,10 @@ export class QuickAddApi { }); } - const output = await new CompleteFormatter( + const output = await new FormatterFactory( app, plugin, - choiceExecutor, - ).formatFileContent(input); + ).createCompleteFormatter(choiceExecutor).formatFileContent(input); if (shouldClearVariables && snapshot) { restoreVariables(choiceExecutor.variables, snapshot); diff --git a/src/services/FormatterFactory.ts b/src/services/FormatterFactory.ts new file mode 100644 index 00000000..460eb301 --- /dev/null +++ b/src/services/FormatterFactory.ts @@ -0,0 +1,145 @@ +import type { App, TFile } from "obsidian"; +import type { IChoiceExecutor } from "../IChoiceExecutor"; +import type QuickAdd from "../main"; +import type { IDateParser } from "../parsers/IDateParser"; +import { CompleteFormatter } from "../formatters/completeFormatter"; +import { FormatDisplayFormatter } from "../formatters/formatDisplayFormatter"; +import type { + CompleteFormatterEvaluators, + FormatDisplayFormatterEvaluators, + FormatterVariables, +} from "../formatters/formatterEvaluators"; + +function normalizeTemplatePreviewPath(templatePath: string): string { + const stripped = templatePath.replace(/^\/+/, ""); + if (/\.(md|canvas|base)$/i.test(stripped)) return stripped; + return `${stripped}.md`; +} + +export class FormatterFactory { + constructor( + private readonly app: App, + private readonly plugin: QuickAdd, + ) {} + + public createCompleteFormatter( + choiceExecutor?: IChoiceExecutor, + dateParser?: IDateParser, + ): CompleteFormatter { + return new CompleteFormatter( + this.app, + this.plugin, + choiceExecutor, + dateParser, + this.createCompleteEvaluators(choiceExecutor), + ); + } + + public createDisplayFormatter(dateParser?: IDateParser): FormatDisplayFormatter { + return new FormatDisplayFormatter( + this.app, + this.plugin, + dateParser, + this.createDisplayEvaluators(), + ); + } + + private createCompleteEvaluators( + choiceExecutor?: IChoiceExecutor, + ): CompleteFormatterEvaluators { + return { + macro: { + evaluateMacro: async (macroName, context) => { + const executor = this.requireChoiceExecutor(choiceExecutor); + const { SingleMacroEngine } = await import( + "../engine/SingleMacroEngine" + ); + const engine = new SingleMacroEngine( + this.app, + this.plugin, + this.plugin.settings.choices, + executor, + context.variables, + ); + return await engine.runAndGetOutput( + macroName, + context.label ? { label: context.label } : undefined, + ); + }, + }, + template: { + evaluateTemplate: async (templatePath, context) => { + const { SingleTemplateEngine } = await import( + "../engine/SingleTemplateEngine" + ); + return await new SingleTemplateEngine( + this.app, + this.plugin, + templatePath, + this.withVariables(choiceExecutor, context.variables), + ).run(); + }, + }, + inlineJavaScript: { + evaluateInlineJavaScript: async (code, context) => { + const { SingleInlineScriptEngine } = await import( + "../engine/SingleInlineScriptEngine" + ); + const executor = new SingleInlineScriptEngine( + this.app, + this.plugin, + this.requireChoiceExecutor(choiceExecutor), + context.variables, + ); + return await executor.runAndGetOutput(code); + }, + }, + }; + } + + private createDisplayEvaluators(): FormatDisplayFormatterEvaluators { + return { + template: { + evaluateTemplate: async (templatePath) => { + const resolvedPath = normalizeTemplatePreviewPath(templatePath); + const file = this.app.vault.getAbstractFileByPath(resolvedPath); + if (!this.isReadableFile(file)) { + return `Template (not found): ${templatePath}`; + } + try { + return await this.app.vault.cachedRead(file); + } catch { + return `Template (not found): ${templatePath}`; + } + }, + }, + }; + } + + private requireChoiceExecutor( + choiceExecutor: IChoiceExecutor | undefined, + ): IChoiceExecutor { + if (!choiceExecutor) { + throw new Error("Choice executor is required for runtime evaluation."); + } + return choiceExecutor; + } + + private withVariables( + choiceExecutor: IChoiceExecutor | undefined, + variables: FormatterVariables, + ): IChoiceExecutor | undefined { + if (!choiceExecutor) return undefined; + choiceExecutor.variables = variables; + return choiceExecutor; + } + + private isReadableFile(file: unknown): file is TFile { + return Boolean( + file && + typeof file === "object" && + "extension" in file && + "path" in file, + ); + } +} From 75a4a245ff3110b1ddae283e1157ec5b42d7a8da Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Thu, 14 May 2026 20:55:04 +0200 Subject: [PATCH 2/7] refactor: extract folder template services --- src/engine/TemplateEngine.ts | 595 ++------------------- src/preflight/collectChoiceRequirements.ts | 18 +- src/services/FolderSelectionService.ts | 389 ++++++++++++++ src/services/FormatterFactory.ts | 48 +- src/services/TemplateEvaluator.test.ts | 51 ++ src/services/TemplateFileService.test.ts | 122 +++++ src/services/TemplateFileService.ts | 313 +++++++++++ 7 files changed, 943 insertions(+), 593 deletions(-) create mode 100644 src/services/FolderSelectionService.ts create mode 100644 src/services/TemplateEvaluator.test.ts create mode 100644 src/services/TemplateFileService.test.ts create mode 100644 src/services/TemplateFileService.ts diff --git a/src/engine/TemplateEngine.ts b/src/engine/TemplateEngine.ts index 0dda165a..b676609d 100644 --- a/src/engine/TemplateEngine.ts +++ b/src/engine/TemplateEngine.ts @@ -2,82 +2,24 @@ import { QuickAddEngine } from "./QuickAddEngine"; import type { CompleteFormatter } from "../formatters/completeFormatter"; import { FormatterFactory } from "../services/FormatterFactory"; import type { LinkToCurrentFileBehavior } from "../formatters/formatter"; -import type { App } from "obsidian"; -import { Notice, TFile } from "obsidian"; +import type { App, TFile } from "obsidian"; import type QuickAdd from "../main"; +import { getTemplater } from "../utilityObsidian"; +import type { IChoiceExecutor } from "../IChoiceExecutor"; import { - getTemplater, - overwriteTemplaterOnce, - templaterParseTemplate, -} from "../utilityObsidian"; -import GenericSuggester from "../gui/GenericSuggester/genericSuggester"; -import InputSuggester from "../gui/InputSuggester/inputSuggester"; -import { - BASE_FILE_EXTENSION_REGEX, - CANVAS_FILE_EXTENSION_REGEX, - MARKDOWN_FILE_EXTENSION_REGEX, -} from "../constants"; -import { reportError } from "../utils/errorUtils"; -import { basenameWithoutMdOrCanvas } from "../utils/pathUtils"; + FolderSelectionService, + type FolderChoiceOptions, +} from "../services/FolderSelectionService"; import { - INVALID_FOLDER_CHARS_REGEX, - INVALID_FOLDER_CONTROL_CHARS_REGEX, - INVALID_FOLDER_TRAILING_CHARS_REGEX, - isReservedWindowsDeviceName, -} from "../utils/pathValidation"; -import { MacroAbortError } from "../errors/MacroAbortError"; -import { isCancellationError } from "../utils/errorUtils"; -import type { IChoiceExecutor } from "../IChoiceExecutor"; -import { log } from "../logger/logManager"; - -type FolderChoiceOptions = { - allowCreate?: boolean; - placeholder?: string; - allowedRoots?: string[]; - topItems?: Array<{ path: string; label: string }>; -}; - -type FolderSelectionContext = { - items: string[]; - displayItems: string[]; - normalizedItems: string[]; - canonicalByNormalized: Map; - displayByNormalized: Map; - existingSet: Set; - allowCreate: boolean; - allowedRoots: string[]; - placeholder?: string; -}; - -type FolderSelection = { - raw: string; - normalized: string; - resolved: string; - exists: boolean; - isAllowed: boolean; - isEmpty: boolean; -}; - -class InvalidFolderPathError extends Error { - constructor(message: string) { - super(message); - this.name = "InvalidFolderPathError"; - } -} - -function isMacroAbortError(error: unknown): error is MacroAbortError { - return ( - error instanceof MacroAbortError || - (Boolean(error) && - typeof error === "object" && - "name" in (error as Record) && - (error as { name?: string }).name === "MacroAbortError") - ); -} + TemplateEvaluator, + TemplateFileService, +} from "../services/TemplateFileService"; export abstract class TemplateEngine extends QuickAddEngine { protected formatter: CompleteFormatter; protected readonly templater; + protected readonly folderSelectionService: FolderSelectionService; + protected readonly templateFileService: TemplateFileService; protected constructor( app: App, @@ -90,6 +32,15 @@ export abstract class TemplateEngine extends QuickAddEngine { app, plugin, ).createCompleteFormatter(choiceFormatter); + this.folderSelectionService = new FolderSelectionService( + app, + this.vaultFileService, + ); + this.templateFileService = new TemplateFileService( + app, + this.vaultFileService, + this.frontmatterPropertyService, + ); } public abstract run(): @@ -101,338 +52,12 @@ export abstract class TemplateEngine extends QuickAddEngine { folders: string[], options: FolderChoiceOptions = {}, ): Promise { - const context = this.buildFolderSelectionContext(folders, options); - - if (!this.shouldPromptForFolder(context)) { - return await this.handleSingleSelection(context); - } - - const selection = await this.promptUntilAllowed(context); - return selection.isEmpty ? "" : selection.resolved; - } - - private buildFolderSelectionContext( - folders: string[], - options: FolderChoiceOptions, - ): FolderSelectionContext { - const allowCreate = options.allowCreate ?? false; - const allowedRoots = - options.allowedRoots?.map((root) => this.normalizeFolderPath(root)) ?? []; - - const { - items, - displayItems, - normalizedItems, - canonicalByNormalized, - displayByNormalized, - } = this.buildFolderSuggestions( + return await this.folderSelectionService.getOrCreateFolder( folders, - options.topItems ?? [], - allowedRoots.length > 0 ? allowedRoots : undefined, - ); - - return { - items, - displayItems, - normalizedItems, - canonicalByNormalized, - displayByNormalized, - existingSet: new Set(normalizedItems), - allowCreate, - allowedRoots, - placeholder: options.placeholder, - }; - } - - private shouldPromptForFolder(context: FolderSelectionContext): boolean { - return ( - context.items.length > 1 || - (context.allowCreate && context.items.length === 0) + options, ); } - private async promptForFolder(context: FolderSelectionContext): Promise { - try { - if (context.allowCreate) { - return await InputSuggester.Suggest( - this.app, - context.displayItems, - context.items, - { - placeholder: - context.placeholder ?? "Choose a folder or type to create one", - renderItem: (item, el) => { - this.renderFolderSuggestion( - item, - el, - context.existingSet, - context.displayByNormalized, - ); - }, - }, - ); - } - - return await GenericSuggester.Suggest( - this.app, - context.displayItems, - context.items, - context.placeholder, - ); - } catch (error) { - if (isCancellationError(error)) { - throw new MacroAbortError("Input cancelled by user"); - } - throw error; - } - } - - private async resolveSelection( - raw: string, - context: FolderSelectionContext, - ): Promise { - const normalized = this.normalizeFolderPath(raw); - const isEmpty = normalized.length === 0; - const canonical = context.canonicalByNormalized.get(normalized); - const resolved = canonical ?? normalized; - - const exists = isEmpty - ? false - : canonical !== undefined || - (await this.app.vault.adapter.exists(resolved)); - - const isAllowed = - context.allowedRoots.length === 0 - ? true - : this.isPathAllowed(isEmpty ? "" : resolved, context.allowedRoots); - - return { - raw, - normalized, - resolved, - exists, - isAllowed, - isEmpty, - }; - } - - private async promptUntilAllowed( - context: FolderSelectionContext, - ): Promise { - // Keep prompting until the user provides an allowed selection or cancels. - for (;;) { - const raw = await this.promptForFolder(context); - const selection = await this.resolveSelection(raw, context); - - if (selection.isEmpty) { - if (!selection.isAllowed) { - this.showFolderNotAllowedNotice(context.allowedRoots); - continue; - } - return selection; - } - - if (!selection.isAllowed) { - this.showFolderNotAllowedNotice(context.allowedRoots); - continue; - } - - try { - this.validateFolderPath(selection.resolved); - } catch (error) { - if (error instanceof InvalidFolderPathError) { - new Notice(error.message); - continue; - } - throw error; - } - - await this.ensureFolderExists(selection); - - return selection; - } - } - - private async ensureFolderExists(selection: FolderSelection): Promise { - if (selection.isEmpty || selection.exists) return; - await this.vaultFileService.createFolder(selection.resolved); - } - - private async handleSingleSelection( - context: FolderSelectionContext, - ): Promise { - const raw = context.items[0] ?? ""; - const selection = await this.resolveSelection(raw, context); - - if (selection.isEmpty) return ""; - if (!selection.isAllowed) { - this.showFolderNotAllowedNotice(context.allowedRoots); - throw new MacroAbortError("Selected folder not allowed."); - } - - if (selection.resolved) { - try { - this.validateFolderPath(selection.resolved); - } catch (error) { - if (error instanceof InvalidFolderPathError) { - new Notice(error.message); - return ""; - } - throw error; - } - } - - await this.ensureFolderExists(selection); - return selection.resolved; - } - - private normalizeFolderPath(path: string): string { - return path.trim().replace(/^\/+/, "").replace(/\/+$/, ""); - } - - private validateFolderPath(path: string): void { - const trimmed = path.trim(); - if (!trimmed) return; - - const segments = trimmed.split("/"); - for (const segment of segments) { - this.validateFolderSegment(segment); - } - } - - private validateFolderSegment(segment: string): void { - if (!segment) { - throw new InvalidFolderPathError("Folder name cannot be empty."); - } - - if (segment === "." || segment === "..") { - throw new InvalidFolderPathError("Folder name cannot be '.' or '..'."); - } - - if (INVALID_FOLDER_CONTROL_CHARS_REGEX.test(segment)) { - throw new InvalidFolderPathError( - "Folder name cannot contain control characters.", - ); - } - - if (INVALID_FOLDER_CHARS_REGEX.test(segment)) { - throw new InvalidFolderPathError( - "Folder name cannot contain any of the following characters: \\ / : * ? \" < > |", - ); - } - - if (INVALID_FOLDER_TRAILING_CHARS_REGEX.test(segment)) { - throw new InvalidFolderPathError( - "Folder name cannot end with a space or a period.", - ); - } - - const normalized = segment.replace(/[. ]+$/u, ""); - const base = normalized.split(".")[0] ?? ""; - if (base && isReservedWindowsDeviceName(base)) { - throw new InvalidFolderPathError( - "Folder name cannot be a reserved name like CON, PRN, AUX, NUL, COM1-9, or LPT1-9.", - ); - } - } - - private isPathAllowed(path: string, roots: string[]): boolean { - const normalizedPath = this.normalizeFolderPath(path); - for (const root of roots) { - if (!root) return true; - if (normalizedPath === root) return true; - if (normalizedPath.startsWith(`${root}/`)) return true; - } - return false; - } - - private showFolderNotAllowedNotice(roots: string[]): void { - const displayRoots = roots.map((root) => (root ? root : "/")); - const list = - displayRoots.length > 3 - ? `${displayRoots.slice(0, 3).join(", ")}...` - : displayRoots.join(", "); - new Notice(`Folder must be under: ${list}`); - } - - private buildFolderSuggestions( - folders: string[], - topItems: Array<{ path: string; label: string }>, - allowedRoots?: string[], - ): { - items: string[]; - displayItems: string[]; - normalizedItems: string[]; - canonicalByNormalized: Map; - displayByNormalized: Map; - } { - const items: string[] = []; - const displayItems: string[] = []; - const normalizedItems: string[] = []; - const canonicalByNormalized = new Map(); - const displayByNormalized = new Map(); - const seen = new Set(); - - const addItem = (path: string, label?: string) => { - const normalized = this.normalizeFolderPath(path); - if (seen.has(normalized)) return; - if ( - allowedRoots && - allowedRoots.length > 0 && - !this.isPathAllowed(normalized, allowedRoots) - ) { - return; - } - seen.add(normalized); - items.push(path); - displayItems.push(label ?? path); - normalizedItems.push(normalized); - canonicalByNormalized.set(normalized, path); - if (label) displayByNormalized.set(normalized, label); - }; - - for (const item of topItems) addItem(item.path, item.label); - for (const folder of folders) addItem(folder); - - return { - items, - displayItems, - normalizedItems, - canonicalByNormalized, - displayByNormalized, - }; - } - - private renderFolderSuggestion( - item: string, - el: HTMLElement, - existingSet: Set, - displayByNormalized: Map, - ): void { - el.empty(); - el.classList.add("mod-complex"); - const normalized = this.normalizeFolderPath(item); - const display = displayByNormalized.get(normalized); - const displayPath = item || "/"; - const isExisting = existingSet.has(normalized); - let indicator = ""; - - if (display === "") { - indicator = "Current folder"; - } else if (!isExisting) { - indicator = "Create folder"; - } - - const content = el.createDiv("suggestion-content"); - const title = content.createDiv("suggestion-title"); - title.createSpan({ text: displayPath }); - - if (indicator) { - const aux = el.createDiv("suggestion-aux"); - aux.createEl("kbd", { cls: "suggestion-hotkey", text: indicator }); - } - } - protected async getFormattedFilePath( folderPath: string, format: string, @@ -446,13 +71,7 @@ export abstract class TemplateEngine extends QuickAddEngine { } protected getTemplateExtension(templatePath: string): string { - if (CANVAS_FILE_EXTENSION_REGEX.test(templatePath)) { - return ".canvas"; - } - if (BASE_FILE_EXTENSION_REGEX.test(templatePath)) { - return ".base"; - } - return ".md"; + return this.templateFileService.getTemplateExtension(templatePath); } protected normalizeTemplateFilePath( @@ -460,67 +79,23 @@ export abstract class TemplateEngine extends QuickAddEngine { fileName: string, templatePath: string ): string { - const safeFolderPath = this.vaultFileService.stripLeadingSlash(folderPath); - const actualFolderPath: string = safeFolderPath ? `${safeFolderPath}/` : ""; - const extension = this.getTemplateExtension(templatePath); - const formattedFileName: string = this.vaultFileService.stripLeadingSlash(fileName) - .replace(MARKDOWN_FILE_EXTENSION_REGEX, "") - .replace(CANVAS_FILE_EXTENSION_REGEX, "") - .replace(BASE_FILE_EXTENSION_REGEX, ""); - return `${actualFolderPath}${formattedFileName}${extension}`; + return this.templateFileService.normalizeTemplateFilePath( + folderPath, + fileName, + templatePath, + ); } protected async createFileWithTemplate( filePath: string, templatePath: string ) { - try { - const templateContent: string = await this.getTemplateContent( - templatePath - ); - - // Extract filename without extension from the full path. - const fileBasename = basenameWithoutMdOrCanvas(filePath); - this.formatter.setTitle(fileBasename); - - const formattedTemplateContent: string = - await this.formatter.withTemplatePropertyCollection(() => - this.formatter.formatFileContent(templateContent), - ); - - // Get template variables before creating the file - const templateVars = this.formatter.getAndClearTemplatePropertyVars(); - - log.logMessage(`TemplateEngine.createFileWithTemplate: Collected ${templateVars.size} template property variables for ${filePath}`); - if (templateVars.size > 0) { - log.logMessage(`Variables: ${Array.from(templateVars.keys()).join(', ')}`); - } - - const suppressTemplaterOnCreate = filePath - .toLowerCase() - .endsWith(".md"); - const createdFile: TFile = await this.vaultFileService.createFileWithInput( - filePath, - formattedTemplateContent, - { suppressTemplaterOnCreate }, - ); - - // Post-process front matter for template property types BEFORE Templater - if (this.frontmatterPropertyService.shouldPostProcessFrontMatter(createdFile, templateVars)) { - await this.frontmatterPropertyService.postProcessFrontMatter(createdFile, templateVars); - } - - // Process Templater commands for template choices - await overwriteTemplaterOnce(this.app, createdFile); - - return createdFile; - } catch (err) { - if (isMacroAbortError(err)) { - throw err; - } - reportError(err, `Could not create file with template at ${filePath}`); - return null; - } + const templateContent = await this.getTemplateContent(templatePath); + return await this.templateFileService.createFileWithTemplateContent( + filePath, + templateContent, + new TemplateEvaluator(this.formatter), + ); } public setLinkToCurrentFileBehavior(behavior: LinkToCurrentFileBehavior) { @@ -533,46 +108,12 @@ export abstract class TemplateEngine extends QuickAddEngine { file: TFile, templatePath: string ) { - try { - const templateContent: string = await this.getTemplateContent( - templatePath - ); - - // Use the existing file's basename as the title - const fileBasename = file.basename; - this.formatter.setTitle(fileBasename); - - const formattedTemplateContent: string = - await this.formatter.withTemplatePropertyCollection(() => - this.formatter.formatFileContent(templateContent), - ); - - // Get template variables before modifying the file - const templateVars = this.formatter.getAndClearTemplatePropertyVars(); - - log.logMessage(`TemplateEngine.overwriteFileWithTemplate: Collected ${templateVars.size} template property variables for ${file.path}`); - if (templateVars.size > 0) { - log.logMessage(`Variables: ${Array.from(templateVars.keys()).join(', ')}`); - } - - await this.app.vault.modify(file, formattedTemplateContent); - - // Post-process front matter for template property types BEFORE Templater - if (this.frontmatterPropertyService.shouldPostProcessFrontMatter(file, templateVars)) { - await this.frontmatterPropertyService.postProcessFrontMatter(file, templateVars); - } - - // Process Templater commands - await overwriteTemplaterOnce(this.app, file); - - return file; - } catch (err) { - if (isMacroAbortError(err)) { - throw err; - } - reportError(err, "Could not overwrite file with template"); - return null; - } + const templateContent = await this.getTemplateContent(templatePath); + return await this.templateFileService.overwriteFileWithTemplateContent( + file, + templateContent, + new TemplateEvaluator(this.formatter), + ); } protected async appendToFileWithTemplate( @@ -580,56 +121,16 @@ export abstract class TemplateEngine extends QuickAddEngine { templatePath: string, section: "top" | "bottom" ) { - try { - const templateContent: string = await this.getTemplateContent( - templatePath - ); - - // Use the existing file's basename as the title - const fileBasename = file.basename; - this.formatter.setTitle(fileBasename); - - let formattedTemplateContent: string = - await this.formatter.formatFileContent(templateContent); - if (file.extension === "md") { - formattedTemplateContent = await templaterParseTemplate( - this.app, - formattedTemplateContent, - file, - ); - } - const fileContent: string = await this.app.vault.cachedRead(file); - const newFileContent: string = - section === "top" - ? `${formattedTemplateContent}\n${fileContent}` - : `${fileContent}\n${formattedTemplateContent}`; - await this.app.vault.modify(file, newFileContent); - - return file; - } catch (err) { - if (isMacroAbortError(err)) { - throw err; - } - reportError(err, "Could not append to file with template"); - return null; - } + const templateContent = await this.getTemplateContent(templatePath); + return await this.templateFileService.appendToFileWithTemplateContent( + file, + templateContent, + section, + new TemplateEvaluator(this.formatter), + ); } protected async getTemplateContent(templatePath: string): Promise { - let correctTemplatePath: string = this.vaultFileService.stripLeadingSlash(templatePath); - if (!MARKDOWN_FILE_EXTENSION_REGEX.test(templatePath) && - !CANVAS_FILE_EXTENSION_REGEX.test(templatePath) && - !BASE_FILE_EXTENSION_REGEX.test(templatePath)) - correctTemplatePath += ".md"; - - const templateFile = - this.app.vault.getAbstractFileByPath(correctTemplatePath); - - if (!(templateFile instanceof TFile)) - throw new Error( - `Template file not found at path "${correctTemplatePath}".` - ); - - return await this.app.vault.cachedRead(templateFile); + return await this.templateFileService.readTemplateContent(templatePath); } } diff --git a/src/preflight/collectChoiceRequirements.ts b/src/preflight/collectChoiceRequirements.ts index ee2f3266..d55b94c4 100644 --- a/src/preflight/collectChoiceRequirements.ts +++ b/src/preflight/collectChoiceRequirements.ts @@ -2,9 +2,6 @@ import type { App } from "obsidian"; import { MarkdownView, TFile } from "obsidian"; import type { IChoiceExecutor } from "src/IChoiceExecutor"; import { - BASE_FILE_EXTENSION_REGEX, - CANVAS_FILE_EXTENSION_REGEX, - MARKDOWN_FILE_EXTENSION_REGEX, QA_INTERNAL_CAPTURE_TARGET_FILE_PATH, } from "src/constants"; import type QuickAdd from "src/main"; @@ -22,6 +19,7 @@ import { isFolder, } from "src/utilityObsidian"; import { log } from "src/logger/logManager"; +import { TemplateFileService } from "src/services/TemplateFileService"; import { resolveExistingVariableKey } from "src/utils/valueSyntax"; import { RequirementCollector, @@ -99,15 +97,11 @@ function getQuickAddScriptInputs(userScript: unknown): unknown[] { } async function readTemplate(app: App, path: string): Promise { - const addExt = - !MARKDOWN_FILE_EXTENSION_REGEX.test(path) && - !CANVAS_FILE_EXTENSION_REGEX.test(path) && - !BASE_FILE_EXTENSION_REGEX.test(path); - const normalized = addExt ? `${path}.md` : path; - const file = app.vault.getAbstractFileByPath(normalized); - if (file instanceof TFile) { - return await app.vault.cachedRead(file); - } + const templateFileService = new TemplateFileService(app); + const file = app.vault.getAbstractFileByPath( + templateFileService.normalizeTemplatePath(path), + ); + if (file instanceof TFile) return await app.vault.cachedRead(file); return ""; } diff --git a/src/services/FolderSelectionService.ts b/src/services/FolderSelectionService.ts new file mode 100644 index 00000000..ec37a2e0 --- /dev/null +++ b/src/services/FolderSelectionService.ts @@ -0,0 +1,389 @@ +import type { App } from "obsidian"; +import { Notice } from "obsidian"; +import GenericSuggester from "../gui/GenericSuggester/genericSuggester"; +import InputSuggester from "../gui/InputSuggester/inputSuggester"; +import { MacroAbortError } from "../errors/MacroAbortError"; +import { isCancellationError } from "../utils/errorUtils"; +import { + INVALID_FOLDER_CHARS_REGEX, + INVALID_FOLDER_CONTROL_CHARS_REGEX, + INVALID_FOLDER_TRAILING_CHARS_REGEX, + isReservedWindowsDeviceName, +} from "../utils/pathValidation"; +import { VaultFileService } from "./VaultFileService"; + +export type FolderChoiceOptions = { + allowCreate?: boolean; + placeholder?: string; + allowedRoots?: string[]; + topItems?: Array<{ path: string; label: string }>; +}; + +type FolderSelectionContext = { + items: string[]; + displayItems: string[]; + normalizedItems: string[]; + canonicalByNormalized: Map; + displayByNormalized: Map; + existingSet: Set; + allowCreate: boolean; + allowedRoots: string[]; + placeholder?: string; +}; + +type FolderSelection = { + raw: string; + normalized: string; + resolved: string; + exists: boolean; + isAllowed: boolean; + isEmpty: boolean; +}; + +class InvalidFolderPathError extends Error { + constructor(message: string) { + super(message); + this.name = "InvalidFolderPathError"; + } +} + +export class FolderSelectionService { + public constructor( + private readonly app: App, + private readonly vaultFileService = new VaultFileService(app), + ) {} + + public async getOrCreateFolder( + folders: string[], + options: FolderChoiceOptions = {}, + ): Promise { + const context = this.buildFolderSelectionContext(folders, options); + + if (!this.shouldPromptForFolder(context)) { + return await this.handleSingleSelection(context); + } + + const selection = await this.promptUntilAllowed(context); + return selection.isEmpty ? "" : selection.resolved; + } + + public normalizeFolderPath(path: string): string { + return path.trim().replace(/^\/+/, "").replace(/\/+$/, ""); + } + + private buildFolderSelectionContext( + folders: string[], + options: FolderChoiceOptions, + ): FolderSelectionContext { + const allowCreate = options.allowCreate ?? false; + const allowedRoots = + options.allowedRoots?.map((root) => this.normalizeFolderPath(root)) ?? []; + + const { + items, + displayItems, + normalizedItems, + canonicalByNormalized, + displayByNormalized, + } = this.buildFolderSuggestions( + folders, + options.topItems ?? [], + allowedRoots.length > 0 ? allowedRoots : undefined, + ); + + return { + items, + displayItems, + normalizedItems, + canonicalByNormalized, + displayByNormalized, + existingSet: new Set(normalizedItems), + allowCreate, + allowedRoots, + placeholder: options.placeholder, + }; + } + + private shouldPromptForFolder(context: FolderSelectionContext): boolean { + return ( + context.items.length > 1 || + (context.allowCreate && context.items.length === 0) + ); + } + + private async promptForFolder(context: FolderSelectionContext): Promise { + try { + if (context.allowCreate) { + return await InputSuggester.Suggest( + this.app, + context.displayItems, + context.items, + { + placeholder: + context.placeholder ?? "Choose a folder or type to create one", + renderItem: (item, el) => { + this.renderFolderSuggestion( + item, + el, + context.existingSet, + context.displayByNormalized, + ); + }, + }, + ); + } + + return await GenericSuggester.Suggest( + this.app, + context.displayItems, + context.items, + context.placeholder, + ); + } catch (error) { + if (isCancellationError(error)) { + throw new MacroAbortError("Input cancelled by user"); + } + throw error; + } + } + + private async resolveSelection( + raw: string, + context: FolderSelectionContext, + ): Promise { + const normalized = this.normalizeFolderPath(raw); + const isEmpty = normalized.length === 0; + const canonical = context.canonicalByNormalized.get(normalized); + const resolved = canonical ?? normalized; + + const exists = isEmpty + ? false + : canonical !== undefined || + (await this.app.vault.adapter.exists(resolved)); + + const isAllowed = + context.allowedRoots.length === 0 + ? true + : this.isPathAllowed(isEmpty ? "" : resolved, context.allowedRoots); + + return { + raw, + normalized, + resolved, + exists, + isAllowed, + isEmpty, + }; + } + + private async promptUntilAllowed( + context: FolderSelectionContext, + ): Promise { + for (;;) { + const raw = await this.promptForFolder(context); + const selection = await this.resolveSelection(raw, context); + + if (selection.isEmpty) { + if (!selection.isAllowed) { + this.showFolderNotAllowedNotice(context.allowedRoots); + continue; + } + return selection; + } + + if (!selection.isAllowed) { + this.showFolderNotAllowedNotice(context.allowedRoots); + continue; + } + + try { + this.validateFolderPath(selection.resolved); + } catch (error) { + if (error instanceof InvalidFolderPathError) { + new Notice(error.message); + continue; + } + throw error; + } + + await this.ensureFolderExists(selection); + return selection; + } + } + + private async ensureFolderExists(selection: FolderSelection): Promise { + if (selection.isEmpty || selection.exists) return; + await this.vaultFileService.createFolder(selection.resolved); + } + + private async handleSingleSelection( + context: FolderSelectionContext, + ): Promise { + const raw = context.items[0] ?? ""; + const selection = await this.resolveSelection(raw, context); + + if (selection.isEmpty) return ""; + if (!selection.isAllowed) { + this.showFolderNotAllowedNotice(context.allowedRoots); + throw new MacroAbortError("Selected folder not allowed."); + } + + if (selection.resolved) { + try { + this.validateFolderPath(selection.resolved); + } catch (error) { + if (error instanceof InvalidFolderPathError) { + new Notice(error.message); + return ""; + } + throw error; + } + } + + await this.ensureFolderExists(selection); + return selection.resolved; + } + + private validateFolderPath(path: string): void { + const trimmed = path.trim(); + if (!trimmed) return; + + const segments = trimmed.split("/"); + for (const segment of segments) { + this.validateFolderSegment(segment); + } + } + + private validateFolderSegment(segment: string): void { + if (!segment) { + throw new InvalidFolderPathError("Folder name cannot be empty."); + } + + if (segment === "." || segment === "..") { + throw new InvalidFolderPathError("Folder name cannot be '.' or '..'."); + } + + if (INVALID_FOLDER_CONTROL_CHARS_REGEX.test(segment)) { + throw new InvalidFolderPathError( + "Folder name cannot contain control characters.", + ); + } + + if (INVALID_FOLDER_CHARS_REGEX.test(segment)) { + throw new InvalidFolderPathError( + "Folder name cannot contain any of the following characters: \\ / : * ? \" < > |", + ); + } + + if (INVALID_FOLDER_TRAILING_CHARS_REGEX.test(segment)) { + throw new InvalidFolderPathError( + "Folder name cannot end with a space or a period.", + ); + } + + const normalized = segment.replace(/[. ]+$/u, ""); + const base = normalized.split(".")[0] ?? ""; + if (base && isReservedWindowsDeviceName(base)) { + throw new InvalidFolderPathError( + "Folder name cannot be a reserved name like CON, PRN, AUX, NUL, COM1-9, or LPT1-9.", + ); + } + } + + private isPathAllowed(path: string, roots: string[]): boolean { + const normalizedPath = this.normalizeFolderPath(path); + for (const root of roots) { + if (!root) return true; + if (normalizedPath === root) return true; + if (normalizedPath.startsWith(`${root}/`)) return true; + } + return false; + } + + private showFolderNotAllowedNotice(roots: string[]): void { + const displayRoots = roots.map((root) => (root ? root : "/")); + const list = + displayRoots.length > 3 + ? `${displayRoots.slice(0, 3).join(", ")}...` + : displayRoots.join(", "); + new Notice(`Folder must be under: ${list}`); + } + + private buildFolderSuggestions( + folders: string[], + topItems: Array<{ path: string; label: string }>, + allowedRoots?: string[], + ): { + items: string[]; + displayItems: string[]; + normalizedItems: string[]; + canonicalByNormalized: Map; + displayByNormalized: Map; + } { + const items: string[] = []; + const displayItems: string[] = []; + const normalizedItems: string[] = []; + const canonicalByNormalized = new Map(); + const displayByNormalized = new Map(); + const seen = new Set(); + + const addItem = (path: string, label?: string) => { + const normalized = this.normalizeFolderPath(path); + if (seen.has(normalized)) return; + if ( + allowedRoots && + allowedRoots.length > 0 && + !this.isPathAllowed(normalized, allowedRoots) + ) { + return; + } + seen.add(normalized); + items.push(path); + displayItems.push(label ?? path); + normalizedItems.push(normalized); + canonicalByNormalized.set(normalized, path); + if (label) displayByNormalized.set(normalized, label); + }; + + for (const item of topItems) addItem(item.path, item.label); + for (const folder of folders) addItem(folder); + + return { + items, + displayItems, + normalizedItems, + canonicalByNormalized, + displayByNormalized, + }; + } + + private renderFolderSuggestion( + item: string, + el: HTMLElement, + existingSet: Set, + displayByNormalized: Map, + ): void { + el.empty(); + el.classList.add("mod-complex"); + const normalized = this.normalizeFolderPath(item); + const display = displayByNormalized.get(normalized); + const displayPath = item || "/"; + const isExisting = existingSet.has(normalized); + let indicator = ""; + + if (display === "") { + indicator = "Current folder"; + } else if (!isExisting) { + indicator = "Create folder"; + } + + const content = el.createDiv("suggestion-content"); + const title = content.createDiv("suggestion-title"); + title.createSpan({ text: displayPath }); + + if (indicator) { + const aux = el.createDiv("suggestion-aux"); + aux.createEl("kbd", { cls: "suggestion-hotkey", text: indicator }); + } + } +} diff --git a/src/services/FormatterFactory.ts b/src/services/FormatterFactory.ts index 460eb301..acccf82c 100644 --- a/src/services/FormatterFactory.ts +++ b/src/services/FormatterFactory.ts @@ -1,4 +1,4 @@ -import type { App, TFile } from "obsidian"; +import type { App } from "obsidian"; import type { IChoiceExecutor } from "../IChoiceExecutor"; import type QuickAdd from "../main"; import type { IDateParser } from "../parsers/IDateParser"; @@ -9,12 +9,7 @@ import type { FormatDisplayFormatterEvaluators, FormatterVariables, } from "../formatters/formatterEvaluators"; - -function normalizeTemplatePreviewPath(templatePath: string): string { - const stripped = templatePath.replace(/^\/+/, ""); - if (/\.(md|canvas|base)$/i.test(stripped)) return stripped; - return `${stripped}.md`; -} +import { TemplateEvaluator, TemplateFileService } from "./TemplateFileService"; export class FormatterFactory { constructor( @@ -69,15 +64,16 @@ export class FormatterFactory { }, template: { evaluateTemplate: async (templatePath, context) => { - const { SingleTemplateEngine } = await import( - "../engine/SingleTemplateEngine" - ); - return await new SingleTemplateEngine( - this.app, - this.plugin, - templatePath, + const formatter = this.createCompleteFormatter( this.withVariables(choiceExecutor, context.variables), - ).run(); + ); + const templateFileService = new TemplateFileService(this.app); + const templateContent = + await templateFileService.readTemplateContent(templatePath); + const { content } = await new TemplateEvaluator( + formatter, + ).evaluateTemplateContent(templateContent, templatePath); + return content; }, }, inlineJavaScript: { @@ -101,16 +97,9 @@ export class FormatterFactory { return { template: { evaluateTemplate: async (templatePath) => { - const resolvedPath = normalizeTemplatePreviewPath(templatePath); - const file = this.app.vault.getAbstractFileByPath(resolvedPath); - if (!this.isReadableFile(file)) { - return `Template (not found): ${templatePath}`; - } - try { - return await this.app.vault.cachedRead(file); - } catch { - return `Template (not found): ${templatePath}`; - } + return await new TemplateFileService(this.app).previewTemplateContent( + templatePath, + ); }, }, }; @@ -133,13 +122,4 @@ export class FormatterFactory { choiceExecutor.variables = variables; return choiceExecutor; } - - private isReadableFile(file: unknown): file is TFile { - return Boolean( - file && - typeof file === "object" && - "extension" in file && - "path" in file, - ); - } } diff --git a/src/services/TemplateEvaluator.test.ts b/src/services/TemplateEvaluator.test.ts new file mode 100644 index 00000000..ddf351ed --- /dev/null +++ b/src/services/TemplateEvaluator.test.ts @@ -0,0 +1,51 @@ +import { describe, expect, it, vi } from "vitest"; +import { MacroAbortError } from "../errors/MacroAbortError"; +import type { CompleteFormatter } from "../formatters/completeFormatter"; +import { TemplateEvaluator } from "./TemplateFileService"; + +describe("TemplateEvaluator", () => { + it("sets title, collects property variables, and performs no file writes", async () => { + const vars = new Map([["project", { name: "QuickAdd" }]]); + const formatter = { + setTitle: vi.fn(), + withTemplatePropertyCollection: vi.fn(async (callback) => callback()), + formatFileContent: vi.fn(async (content: string) => + content.replace("{{title}}", "Daily"), + ), + getAndClearTemplatePropertyVars: vi.fn(() => vars), + } as unknown as CompleteFormatter; + + const result = await new TemplateEvaluator(formatter).evaluateTemplateContent( + "# {{title}}", + "Notes/Daily.md", + ); + + expect(formatter.setTitle).toHaveBeenCalledWith("Daily"); + expect(formatter.withTemplatePropertyCollection).toHaveBeenCalledTimes(1); + expect(formatter.formatFileContent).toHaveBeenCalledWith("# {{title}}"); + expect(formatter.getAndClearTemplatePropertyVars).toHaveBeenCalledTimes(1); + expect(result).toEqual({ + content: "# Daily", + templatePropertyVars: vars, + }); + }); + + it("propagates MacroAbortError unchanged", async () => { + const abort = new MacroAbortError("stop"); + const formatter = { + setTitle: vi.fn(), + withTemplatePropertyCollection: vi.fn(async () => { + throw abort; + }), + getAndClearTemplatePropertyVars: vi.fn(), + } as unknown as CompleteFormatter; + + await expect( + new TemplateEvaluator(formatter).evaluateTemplateContent( + "content", + "Notes/Daily.md", + ), + ).rejects.toBe(abort); + expect(formatter.getAndClearTemplatePropertyVars).not.toHaveBeenCalled(); + }); +}); diff --git a/src/services/TemplateFileService.test.ts b/src/services/TemplateFileService.test.ts new file mode 100644 index 00000000..3cb9dbfd --- /dev/null +++ b/src/services/TemplateFileService.test.ts @@ -0,0 +1,122 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { App } from "obsidian"; +import { TFile, TFolder } from "obsidian"; +import type { TemplateEvaluator } from "./TemplateFileService"; +import { TemplateFileService } from "./TemplateFileService"; + +vi.mock("../utilityObsidian", async (importOriginal) => { + const actual = (await importOriginal()) as Record; + return { + ...actual, + overwriteTemplaterOnce: vi.fn(async () => undefined), + templaterParseTemplate: vi.fn(async (_app, content: string) => content), + withTemplaterFileCreationSuppressed: vi.fn( + async (_app, _path, createFile: () => Promise) => createFile(), + ), + }; +}); + +function file(path: string, extension = path.split(".").pop() ?? "md"): TFile { + const result = new TFile(); + result.path = path; + result.name = path.split("/").pop() ?? path; + result.basename = result.name.replace(/\.[^.]+$/, ""); + result.extension = extension; + return result; +} + +function folder(path: string): TFolder { + const result = new TFolder(); + result.path = path; + return result; +} + +describe("TemplateFileService", () => { + let app: App; + let abstractFiles: Map; + let service: TemplateFileService; + + beforeEach(() => { + abstractFiles = new Map(); + app = { + vault: { + getAbstractFileByPath: vi.fn((path: string) => abstractFiles.get(path)), + cachedRead: vi.fn(async (template: TFile) => `content:${template.path}`), + adapter: { exists: vi.fn(async () => false) }, + createFolder: vi.fn(), + create: vi.fn(async (path: string) => file(path)), + modify: vi.fn(), + }, + fileManager: { + processFrontMatter: vi.fn(), + }, + } as unknown as App; + service = new TemplateFileService(app); + }); + + it("normalizes and reads only concrete template files", async () => { + abstractFiles.set("Templates/Daily.md", file("Templates/Daily.md")); + abstractFiles.set("Templates/Folder.md", folder("Templates/Folder.md")); + + expect(service.normalizeTemplatePath("/Templates/Daily")).toBe( + "Templates/Daily.md", + ); + await expect( + service.readTemplateContent("/Templates/Daily"), + ).resolves.toBe("content:Templates/Daily.md"); + expect(app.vault.getAbstractFileByPath).toHaveBeenCalledWith( + "Templates/Daily.md", + ); + + expect(() => service.getTemplateFile("Templates/Missing")).toThrow( + 'Template file not found at path "Templates/Missing.md".', + ); + expect(() => service.getTemplateFile("Templates/Folder")).toThrow( + 'Template file not found at path "Templates/Folder.md".', + ); + }); + + it("derives target paths from template extensions", () => { + expect( + service.normalizeTemplateFilePath("/Notes", "/Daily.md", "T.md"), + ).toBe("Notes/Daily.md"); + expect( + service.normalizeTemplateFilePath("Canvases", "Board.md", "T.canvas"), + ).toBe("Canvases/Board.canvas"); + expect( + service.normalizeTemplateFilePath("", "Data.canvas", "T.base"), + ).toBe("Data.base"); + }); + + it("creates files using evaluated content and returned property variables", async () => { + const template = file("Templates/Daily.md"); + abstractFiles.set("Templates/Daily.md", template); + const vars = new Map([["aliases", ["a"]]]); + const evaluator = { + evaluateTemplateContent: vi.fn(async () => ({ + content: "---\naliases: {{VALUE:aliases}}\n---\n", + templatePropertyVars: vars, + })), + } as unknown as TemplateEvaluator; + + const created = await service.createFileWithTemplate( + "Notes/Daily.md", + "Templates/Daily", + evaluator, + ); + + expect(created).toBeInstanceOf(TFile); + expect(evaluator.evaluateTemplateContent).toHaveBeenCalledWith( + "content:Templates/Daily.md", + "Notes/Daily.md", + ); + expect(app.vault.create).toHaveBeenCalledWith( + "Notes/Daily.md", + "---\naliases: {{VALUE:aliases}}\n---\n", + ); + expect(app.fileManager.processFrontMatter).toHaveBeenCalledWith( + created, + expect.any(Function), + ); + }); +}); diff --git a/src/services/TemplateFileService.ts b/src/services/TemplateFileService.ts new file mode 100644 index 00000000..5bcc5f8e --- /dev/null +++ b/src/services/TemplateFileService.ts @@ -0,0 +1,313 @@ +import type { App } from "obsidian"; +import { TFile } from "obsidian"; +import { + BASE_FILE_EXTENSION_REGEX, + CANVAS_FILE_EXTENSION_REGEX, + MARKDOWN_FILE_EXTENSION_REGEX, +} from "../constants"; +import { MacroAbortError } from "../errors/MacroAbortError"; +import type { CompleteFormatter } from "../formatters/completeFormatter"; +import { log } from "../logger/logManager"; +import { reportError } from "../utils/errorUtils"; +import { basenameWithoutMdOrCanvas } from "../utils/pathUtils"; +import { + overwriteTemplaterOnce, + templaterParseTemplate, +} from "../utilityObsidian"; +import { FrontmatterPropertyService } from "./FrontmatterPropertyService"; +import { VaultFileService } from "./VaultFileService"; + +export type TemplateEvaluationResult = { + content: string; + templatePropertyVars: Map; +}; + +export class TemplateEvaluator { + public constructor(private readonly formatter: CompleteFormatter) {} + + public async evaluateTemplateContent( + templateContent: string, + targetPath: string, + ): Promise { + const fileBasename = basenameWithoutMdOrCanvas(targetPath); + this.formatter.setTitle(fileBasename); + + const content = await this.formatter.withTemplatePropertyCollection(() => + this.formatter.formatFileContent(templateContent), + ); + const templatePropertyVars = + this.formatter.getAndClearTemplatePropertyVars(); + + return { content, templatePropertyVars }; + } +} + +function isMacroAbortError(error: unknown): error is MacroAbortError { + return ( + error instanceof MacroAbortError || + (Boolean(error) && + typeof error === "object" && + "name" in (error as Record) && + (error as { name?: string }).name === "MacroAbortError") + ); +} + +export class TemplateFileService { + public constructor( + private readonly app: App, + private readonly vaultFileService = new VaultFileService(app), + private readonly frontmatterPropertyService = + new FrontmatterPropertyService(app), + ) {} + + public normalizeTemplatePath(templatePath: string): string { + let correctTemplatePath = + this.vaultFileService.stripLeadingSlash(templatePath); + if ( + !MARKDOWN_FILE_EXTENSION_REGEX.test(templatePath) && + !CANVAS_FILE_EXTENSION_REGEX.test(templatePath) && + !BASE_FILE_EXTENSION_REGEX.test(templatePath) + ) { + correctTemplatePath += ".md"; + } + return correctTemplatePath; + } + + public getTemplateFile(templatePath: string): TFile { + const correctTemplatePath = this.normalizeTemplatePath(templatePath); + const templateFile = + this.app.vault.getAbstractFileByPath(correctTemplatePath); + + if (!(templateFile instanceof TFile)) { + throw new Error( + `Template file not found at path "${correctTemplatePath}".`, + ); + } + + return templateFile; + } + + public async readTemplateContent(templatePath: string): Promise { + return await this.app.vault.cachedRead(this.getTemplateFile(templatePath)); + } + + public getTemplateExtension(templatePath: string): string { + if (CANVAS_FILE_EXTENSION_REGEX.test(templatePath)) { + return ".canvas"; + } + if (BASE_FILE_EXTENSION_REGEX.test(templatePath)) { + return ".base"; + } + return ".md"; + } + + public normalizeTemplateFilePath( + folderPath: string, + fileName: string, + templatePath: string, + ): string { + const safeFolderPath = this.vaultFileService.stripLeadingSlash(folderPath); + const actualFolderPath = safeFolderPath ? `${safeFolderPath}/` : ""; + const extension = this.getTemplateExtension(templatePath); + const formattedFileName = this.vaultFileService + .stripLeadingSlash(fileName) + .replace(MARKDOWN_FILE_EXTENSION_REGEX, "") + .replace(CANVAS_FILE_EXTENSION_REGEX, "") + .replace(BASE_FILE_EXTENSION_REGEX, ""); + return `${actualFolderPath}${formattedFileName}${extension}`; + } + + public async previewTemplateContent(templatePath: string): Promise { + try { + return await this.readTemplateContent(templatePath); + } catch { + return `Template (not found): ${templatePath}`; + } + } + + public async createFileWithTemplate( + filePath: string, + templatePath: string, + evaluator: TemplateEvaluator, + ): Promise { + try { + const templateContent = await this.readTemplateContent(templatePath); + return await this.createFileWithTemplateContent( + filePath, + templateContent, + evaluator, + ); + } catch (err) { + if (isMacroAbortError(err)) throw err; + reportError(err, `Could not create file with template at ${filePath}`); + return null; + } + } + + public async createFileWithTemplateContent( + filePath: string, + templateContent: string, + evaluator: TemplateEvaluator, + ): Promise { + try { + const { content, templatePropertyVars } = + await evaluator.evaluateTemplateContent(templateContent, filePath); + + this.logCollectedVars( + "TemplateFileService.createFileWithTemplate", + filePath, + templatePropertyVars, + ); + + const suppressTemplaterOnCreate = filePath + .toLowerCase() + .endsWith(".md"); + const createdFile = await this.vaultFileService.createFileWithInput( + filePath, + content, + { suppressTemplaterOnCreate }, + ); + + if ( + this.frontmatterPropertyService.shouldPostProcessFrontMatter( + createdFile, + templatePropertyVars, + ) + ) { + await this.frontmatterPropertyService.postProcessFrontMatter( + createdFile, + templatePropertyVars, + ); + } + + await overwriteTemplaterOnce(this.app, createdFile); + + return createdFile; + } catch (err) { + if (isMacroAbortError(err)) throw err; + reportError(err, `Could not create file with template at ${filePath}`); + return null; + } + } + + public async overwriteFileWithTemplate( + file: TFile, + templatePath: string, + evaluator: TemplateEvaluator, + ): Promise { + try { + const templateContent = await this.readTemplateContent(templatePath); + return await this.overwriteFileWithTemplateContent( + file, + templateContent, + evaluator, + ); + } catch (err) { + if (isMacroAbortError(err)) throw err; + reportError(err, "Could not overwrite file with template"); + return null; + } + } + + public async overwriteFileWithTemplateContent( + file: TFile, + templateContent: string, + evaluator: TemplateEvaluator, + ): Promise { + try { + const { content, templatePropertyVars } = + await evaluator.evaluateTemplateContent(templateContent, file.path); + + this.logCollectedVars( + "TemplateFileService.overwriteFileWithTemplate", + file.path, + templatePropertyVars, + ); + + await this.app.vault.modify(file, content); + + if ( + this.frontmatterPropertyService.shouldPostProcessFrontMatter( + file, + templatePropertyVars, + ) + ) { + await this.frontmatterPropertyService.postProcessFrontMatter( + file, + templatePropertyVars, + ); + } + + await overwriteTemplaterOnce(this.app, file); + + return file; + } catch (err) { + if (isMacroAbortError(err)) throw err; + reportError(err, "Could not overwrite file with template"); + return null; + } + } + + public async appendToFileWithTemplate( + file: TFile, + templatePath: string, + section: "top" | "bottom", + evaluator: TemplateEvaluator, + ): Promise { + try { + const templateContent = await this.readTemplateContent(templatePath); + return await this.appendToFileWithTemplateContent( + file, + templateContent, + section, + evaluator, + ); + } catch (err) { + if (isMacroAbortError(err)) throw err; + reportError(err, "Could not append to file with template"); + return null; + } + } + + public async appendToFileWithTemplateContent( + file: TFile, + templateContent: string, + section: "top" | "bottom", + evaluator: TemplateEvaluator, + ): Promise { + try { + let { content } = await evaluator.evaluateTemplateContent( + templateContent, + file.path, + ); + if (file.extension === "md") { + content = await templaterParseTemplate(this.app, content, file); + } + const fileContent = await this.app.vault.cachedRead(file); + const newFileContent = + section === "top" + ? `${content}\n${fileContent}` + : `${fileContent}\n${content}`; + await this.app.vault.modify(file, newFileContent); + + return file; + } catch (err) { + if (isMacroAbortError(err)) throw err; + reportError(err, "Could not append to file with template"); + return null; + } + } + + private logCollectedVars( + scope: string, + filePath: string, + templateVars: Map, + ): void { + log.logMessage( + `${scope}: Collected ${templateVars.size} template property variables for ${filePath}`, + ); + if (templateVars.size > 0) { + log.logMessage(`Variables: ${Array.from(templateVars.keys()).join(", ")}`); + } + } +} From 056e1af0599687c984af0ce10adc7a3140b8b534 Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Thu, 14 May 2026 21:24:27 +0200 Subject: [PATCH 3/7] fix: isolate preflight preview variables --- ...playFormatter.evaluator-boundaries.test.ts | 20 +++ src/formatters/formatDisplayFormatter.ts | 7 +- ...tChoiceRequirements.runtime-safety.test.ts | 156 ++++++++++++++++++ .../runOnePagePreflight.selection.test.ts | 51 ++++++ src/preflight/runOnePagePreflight.ts | 9 +- 5 files changed, 238 insertions(+), 5 deletions(-) create mode 100644 src/preflight/collectChoiceRequirements.runtime-safety.test.ts diff --git a/src/formatters/formatDisplayFormatter.evaluator-boundaries.test.ts b/src/formatters/formatDisplayFormatter.evaluator-boundaries.test.ts index 6b2ec21e..ebf0b677 100644 --- a/src/formatters/formatDisplayFormatter.evaluator-boundaries.test.ts +++ b/src/formatters/formatDisplayFormatter.evaluator-boundaries.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it, vi } from "vitest"; import { FormatDisplayFormatter } from "./formatDisplayFormatter"; import type { FormatDisplayFormatterEvaluators } from "./formatterEvaluators"; +import { VALUE_LABEL_KEY_DELIMITER } from "../utils/valueSyntax"; const app = { workspace: { @@ -69,4 +70,23 @@ describe("FormatDisplayFormatter evaluator boundaries", () => { await expect(formatter.format("{{DATE:")).resolves.toBe("{{DATE:"); }); + + it("treats existing non-undefined preview variables as resolved", async () => { + const formatter = new FormatDisplayFormatter(app, plugin); + formatter.setPreviewVariables( + new Map([ + ["empty", ""], + ["nil", null], + ["count", 7], + ["obj", { ok: true }], + [`a,b${VALUE_LABEL_KEY_DELIMITER}mapped`, "selected-value"], + ]), + ); + + await expect( + formatter.format( + "{{VALUE:empty}}|{{VALUE:nil}}|{{VALUE:count}}|{{VALUE:obj}}|{{VALUE:a,b|label:mapped}}", + ), + ).resolves.toBe("||7|{\"ok\":true}|selected-value"); + }); }); diff --git a/src/formatters/formatDisplayFormatter.ts b/src/formatters/formatDisplayFormatter.ts index 6b9d103a..da70d636 100644 --- a/src/formatters/formatDisplayFormatter.ts +++ b/src/formatters/formatDisplayFormatter.ts @@ -15,6 +15,7 @@ import { } from "./helpers/previewHelpers"; import { getValueVariableBaseName } from "../utils/valueSyntax"; import type { FormatDisplayFormatterEvaluators } from "./formatterEvaluators"; +import { formatUnknownValue } from "../utils/conditionalHelpers"; export class FormatDisplayFormatter extends Formatter { constructor( @@ -77,11 +78,15 @@ export class FormatDisplayFormatter extends Formatter { protected getVariableValue(variableName: string): string { const stored = this.variables.get(variableName); - if (typeof stored === "string") return stored; + if (stored !== undefined) return formatUnknownValue(stored); const baseName = getValueVariableBaseName(variableName); return getVariableExample(baseName); } + public setPreviewVariables(variables: Map): void { + this.variables = variables; + } + protected getCurrentFileLink(): string | null { if (!this.app) return null; return getCurrentFileLinkPreview(this.app.workspace.getActiveFile()); diff --git a/src/preflight/collectChoiceRequirements.runtime-safety.test.ts b/src/preflight/collectChoiceRequirements.runtime-safety.test.ts new file mode 100644 index 00000000..dfb2ad18 --- /dev/null +++ b/src/preflight/collectChoiceRequirements.runtime-safety.test.ts @@ -0,0 +1,156 @@ +import { describe, expect, it, vi } from "vitest"; +import { TFile, type App } from "obsidian"; +import type { IChoiceExecutor } from "src/IChoiceExecutor"; +import type ITemplateChoice from "src/types/choices/ITemplateChoice"; +import { collectChoiceRequirements } from "./collectChoiceRequirements"; +import { runOnePagePreflight } from "./runOnePagePreflight"; + +let modalResult: Record = {}; + +vi.mock("./OnePageInputModal", () => ({ + OnePageInputModal: class { + waitForClose = Promise.resolve(modalResult); + constructor( + _app: App, + _requirements: unknown, + _variables: Map, + computePreview?: (values: Record) => Promise, + ) { + void computePreview?.({ project: "Preview Project" }); + } + }, +})); + +vi.mock("src/quickAddSettingsTab", () => ({ + QuickAddSettingsTab: class {}, +})); + +vi.mock("src/main", () => ({ + __esModule: true, + default: class QuickAddMock {}, +})); + +vi.mock("obsidian-dataview", () => ({ + __esModule: true, + getAPI: vi.fn().mockReturnValue(null), +})); + +vi.mock("src/utilityObsidian", async (importOriginal) => { + const actual = (await importOriginal()) as Record; + return { + ...actual, + getMarkdownFilesInFolder: vi.fn(() => []), + getMarkdownFilesWithTag: vi.fn(() => []), + getUserScript: vi.fn(), + isFolder: vi.fn(() => false), + }; +}); + +function createTemplateChoice(): ITemplateChoice { + return { + id: "template-choice-id", + name: "Template Choice", + type: "Template", + command: false, + templatePath: "Templates/Safe.md", + folder: { + enabled: false, + folders: [], + chooseWhenCreatingNote: false, + createInSameFolderAsActiveFile: false, + chooseFromSubfolders: false, + }, + fileNameFormat: { + enabled: true, + format: "{{VALUE:project}}", + }, + appendLink: false, + openFile: false, + fileOpening: { + location: "tab", + direction: "vertical", + mode: "default", + focus: true, + }, + fileExistsBehavior: { kind: "prompt" }, + } as ITemplateChoice; +} + +function createApp() { + const template = new TFile(); + template.path = "Templates/Safe.md"; + template.name = "Safe.md"; + template.basename = "Safe"; + template.extension = "md"; + + return { + workspace: { + getActiveViewOfType: vi.fn().mockReturnValue(null), + getActiveFile: vi.fn().mockReturnValue(null), + }, + vault: { + getAbstractFileByPath: vi.fn((path: string) => + path === "Templates/Safe.md" ? template : null, + ), + cachedRead: vi.fn( + async () => + "{{VALUE:project}} {{MACRO:ShouldNotRun}} ```js quickadd\napp.vault.create('bad.md', 'bad')\n```", + ), + create: vi.fn(), + modify: vi.fn(), + }, + commands: { + executeCommandById: vi.fn(), + }, + } as unknown as App & { + vault: { + getAbstractFileByPath: ReturnType; + cachedRead: ReturnType; + create: ReturnType; + modify: ReturnType; + }; + commands: { executeCommandById: ReturnType }; + }; +} + +describe("preflight runtime safety", () => { + it("collects and previews without runtime side effects", async () => { + modalResult = { project: "Submitted Project" }; + const app = createApp(); + const plugin = { + settings: { + inputPrompt: "single-line", + globalVariables: {}, + useSelectionAsCaptureValue: true, + }, + } as any; + const choiceExecutor: IChoiceExecutor = { + execute: vi.fn(), + variables: new Map(), + }; + const choice = createTemplateChoice(); + + const requirements = await collectChoiceRequirements( + app, + plugin, + choiceExecutor, + choice, + ); + const prompted = await runOnePagePreflight( + app, + plugin, + choiceExecutor, + choice, + ); + + expect(requirements.map((requirement) => requirement.id)).toContain( + "project", + ); + expect(prompted).toBe(true); + expect(app.vault.cachedRead).toHaveBeenCalled(); + expect(choiceExecutor.execute).not.toHaveBeenCalled(); + expect(app.commands.executeCommandById).not.toHaveBeenCalled(); + expect(app.vault.create).not.toHaveBeenCalled(); + expect(app.vault.modify).not.toHaveBeenCalled(); + }); +}); diff --git a/src/preflight/runOnePagePreflight.selection.test.ts b/src/preflight/runOnePagePreflight.selection.test.ts index f28bd899..4d11bbb2 100644 --- a/src/preflight/runOnePagePreflight.selection.test.ts +++ b/src/preflight/runOnePagePreflight.selection.test.ts @@ -10,11 +10,15 @@ const { modalOpenMock } = vi.hoisted(() => ({ })); let modalResult: Record = {}; +let modalComputePreview: + | ((values: Record) => Promise>) + | undefined; vi.mock("./OnePageInputModal", () => ({ OnePageInputModal: class { waitForClose = Promise.resolve(modalResult); constructor(...args: unknown[]) { + modalComputePreview = args[3] as typeof modalComputePreview; modalOpenMock(...args); } }, @@ -128,6 +132,7 @@ describe("runOnePagePreflight selection-as-value", () => { beforeEach(() => { modalOpenMock.mockClear(); modalResult = {}; + modalComputePreview = undefined; }); it("prefills {{VALUE}} from selection when enabled", async () => { @@ -230,4 +235,50 @@ describe("runOnePagePreflight template extension handling", () => { "Templates/Kanban.base.md", ); }); + + it("isolates live preview variables until submit and treats concrete values as resolved", async () => { + const app = { + workspace: { + getActiveViewOfType: vi.fn().mockReturnValue(null), + getActiveFile: vi.fn().mockReturnValue(null), + }, + vault: { + getAbstractFileByPath: vi.fn().mockReturnValue(null), + cachedRead: vi.fn(), + }, + } as unknown as App; + const plugin = { + settings: { + inputPrompt: "single-line", + globalVariables: {}, + useSelectionAsCaptureValue: true, + }, + } as any; + const executor = createExecutor(); + executor.variables.set("existing", ""); + executor.variables.set("maybeNull", null); + executor.variables.set("mapped", 42); + modalResult = { liveOnly: "Submitted" }; + const choice = createTemplateChoice("Templates/Preview.md"); + choice.fileNameFormat = { + enabled: true, + format: + "{{VALUE:existing}}|{{VALUE:maybeNull}}|{{VALUE:mapped}}|{{VALUE:liveOnly}}", + }; + + const result = await runOnePagePreflight(app, plugin, executor, choice); + + expect(result).toBe(true); + expect(modalComputePreview).toBeDefined(); + expect(executor.variables.has("liveOnly")).toBe(true); + expect(executor.variables.get("liveOnly")).toBe("Submitted"); + + executor.variables.delete("liveOnly"); + const beforePreview = Array.from(executor.variables.entries()); + await expect( + modalComputePreview?.({ liveOnly: "Typing" }), + ).resolves.toEqual({ fileName: "||42|Typing" }); + expect(Array.from(executor.variables.entries())).toEqual(beforePreview); + expect(executor.variables.has("liveOnly")).toBe(false); + }); }); diff --git a/src/preflight/runOnePagePreflight.ts b/src/preflight/runOnePagePreflight.ts index 366c78a0..996424e6 100644 --- a/src/preflight/runOnePagePreflight.ts +++ b/src/preflight/runOnePagePreflight.ts @@ -42,15 +42,16 @@ export async function runOnePagePreflight( app, plugin, ).createDisplayFormatter(); + const previewVariables = new Map(choiceExecutor.variables); + for (const [k, v] of Object.entries(values)) { + previewVariables.set(k, v); + } + formatter.setPreviewVariables(previewVariables); const out: Record = {}; // File name preview for Template if (choice.type === "Template") { const tmpl = choice as ITemplateChoice; if (tmpl.fileNameFormat?.enabled) { - // Seed variables map-like into formatter - for (const [k, v] of Object.entries(values)) { - formatter["variables"].set(k, v); - } out.fileName = await formatter.format(tmpl.fileNameFormat.format); } } From e0a097dadce9e2d8aee2cf564f9ec04a74007f3d Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Thu, 14 May 2026 21:29:43 +0200 Subject: [PATCH 4/7] fix: break formatter factory engine cycle --- src/IChoiceExecutor.ts | 9 +++ src/choiceExecutor.ts | 33 ++++++++++ src/services/FormatterFactory.test.ts | 90 +++++++++++++++++++++++++++ src/services/FormatterFactory.ts | 37 ++++------- 4 files changed, 145 insertions(+), 24 deletions(-) create mode 100644 src/services/FormatterFactory.test.ts diff --git a/src/IChoiceExecutor.ts b/src/IChoiceExecutor.ts index 483a1078..ec014233 100644 --- a/src/IChoiceExecutor.ts +++ b/src/IChoiceExecutor.ts @@ -1,5 +1,6 @@ import type IChoice from "./types/choices/IChoice"; import type { MacroAbortError } from "./errors/MacroAbortError"; +import type { FormatterEvaluatorContext } from "./formatters/formatterEvaluators"; export interface IChoiceExecutor { execute(choice: IChoice): Promise; @@ -15,4 +16,12 @@ export interface IChoiceExecutor { * awaiting {@link execute} to determine whether the child choice stopped early. */ consumeAbortSignal?(): MacroAbortError | null; + evaluateMacroToken?( + macroName: string, + context: FormatterEvaluatorContext, + ): Promise; + evaluateInlineJavaScriptToken?( + code: string, + context: FormatterEvaluatorContext, + ): Promise; } diff --git a/src/choiceExecutor.ts b/src/choiceExecutor.ts index 456abb56..e8175de3 100644 --- a/src/choiceExecutor.ts +++ b/src/choiceExecutor.ts @@ -7,7 +7,10 @@ import type IMacroChoice from "./types/choices/IMacroChoice"; import { TemplateChoiceEngine } from "./engine/TemplateChoiceEngine"; import { CaptureChoiceEngine } from "./engine/CaptureChoiceEngine"; import { MacroChoiceEngine } from "./engine/MacroChoiceEngine"; +import { SingleInlineScriptEngine } from "./engine/SingleInlineScriptEngine"; +import { SingleMacroEngine } from "./engine/SingleMacroEngine"; import type { IChoiceExecutor } from "./IChoiceExecutor"; +import type { FormatterEvaluatorContext } from "./formatters/formatterEvaluators"; import type IMultiChoice from "./types/choices/IMultiChoice"; import ChoiceSuggester from "./gui/suggesters/choiceSuggester"; import { settingsStore } from "./settingsStore"; @@ -32,6 +35,36 @@ export class ChoiceExecutor implements IChoiceExecutor { return abort ?? null; } + async evaluateMacroToken( + macroName: string, + context: FormatterEvaluatorContext, + ): Promise { + const engine = new SingleMacroEngine( + this.app, + this.plugin, + this.plugin.settings.choices, + this, + context.variables, + ); + return await engine.runAndGetOutput( + macroName, + context.label ? { label: context.label } : undefined, + ); + } + + async evaluateInlineJavaScriptToken( + code: string, + context: FormatterEvaluatorContext, + ): Promise { + const executor = new SingleInlineScriptEngine( + this.app, + this.plugin, + this, + context.variables, + ); + return await executor.runAndGetOutput(code); + } + async execute(choice: IChoice): Promise { this.pendingAbort = null; const originLeaf = getOpenFileOriginLeaf(this.app); diff --git a/src/services/FormatterFactory.test.ts b/src/services/FormatterFactory.test.ts new file mode 100644 index 00000000..4b45739d --- /dev/null +++ b/src/services/FormatterFactory.test.ts @@ -0,0 +1,90 @@ +import { describe, expect, it, vi } from "vitest"; +import { FormatterFactory } from "./FormatterFactory"; + +vi.mock("src/gui/GenericInputPrompt/GenericInputPrompt", () => ({ + default: { PromptWithContext: vi.fn() }, +})); +vi.mock("src/gui/InputSuggester/inputSuggester", () => ({ + default: { Suggest: vi.fn() }, +})); +vi.mock("src/gui/VDateInputPrompt/VDateInputPrompt", () => ({ + default: { Prompt: vi.fn() }, +})); +vi.mock("../gui/GenericSuggester/genericSuggester", () => ({ + default: { Suggest: vi.fn() }, +})); +vi.mock("../gui/InputPrompt", () => ({ + default: class { + factory() { + return { Prompt: vi.fn(), PromptWithContext: vi.fn() }; + } + }, +})); +vi.mock("../gui/MathModal", () => ({ + MathModal: { Prompt: vi.fn() }, +})); +vi.mock("../utils/FieldValueCollector", () => ({ + collectFieldValuesProcessedDetailed: vi.fn(async () => ({ + values: [], + hasDefaultValue: false, + })), + collectFieldValuesRaw: vi.fn(async () => new Set()), + generateFieldCacheKey: vi.fn(() => "cache-key"), +})); +vi.mock("../utils/FieldValueProcessor", () => ({ + FieldValueProcessor: { getSmartDefaults: vi.fn(() => []) }, +})); + +const app = { + workspace: { + getActiveViewOfType: vi.fn(() => null), + getActiveFile: vi.fn(() => null), + }, + fileManager: { + generateMarkdownLink: vi.fn(), + }, +} as any; + +const plugin = { + settings: { + choices: [], + globalVariables: {}, + enableTemplatePropertyTypes: false, + }, +} as any; + +describe("FormatterFactory runtime evaluator wiring", () => { + it("delegates macro and inline JavaScript tokens through the choice executor seam", async () => { + const variables = new Map(); + const evaluateMacroToken = vi.fn(async (_macroName, context) => { + expect(context.variables).toBe(variables); + context.variables.set("fromMacro", "macro-value"); + return "{{VALUE:fromInline}}"; + }); + const evaluateInlineJavaScriptToken = vi.fn(async (_code, context) => { + expect(context.variables).toBe(variables); + context.variables.set("fromInline", "inline-value"); + return "{{MACRO:Example}}"; + }); + const choiceExecutor = { + variables, + evaluateMacroToken, + evaluateInlineJavaScriptToken, + } as any; + + const formatter = new FormatterFactory( + app, + plugin, + ).createCompleteFormatter(choiceExecutor); + + await expect( + formatter.formatFileContent("```js quickadd\nreturn 'token';\n```"), + ).resolves.toBe("inline-value"); + expect(evaluateInlineJavaScriptToken).toHaveBeenCalledWith( + "return 'token';", + { variables }, + ); + expect(evaluateMacroToken).toHaveBeenCalledWith("Example", { variables }); + expect(variables.get("fromMacro")).toBe("macro-value"); + }); +}); diff --git a/src/services/FormatterFactory.ts b/src/services/FormatterFactory.ts index acccf82c..bfca8d83 100644 --- a/src/services/FormatterFactory.ts +++ b/src/services/FormatterFactory.ts @@ -46,20 +46,12 @@ export class FormatterFactory { macro: { evaluateMacro: async (macroName, context) => { const executor = this.requireChoiceExecutor(choiceExecutor); - const { SingleMacroEngine } = await import( - "../engine/SingleMacroEngine" - ); - const engine = new SingleMacroEngine( - this.app, - this.plugin, - this.plugin.settings.choices, - executor, - context.variables, - ); - return await engine.runAndGetOutput( - macroName, - context.label ? { label: context.label } : undefined, - ); + if (!executor.evaluateMacroToken) { + throw new Error( + "Choice executor cannot evaluate runtime macro tokens.", + ); + } + return await executor.evaluateMacroToken(macroName, context); }, }, template: { @@ -78,16 +70,13 @@ export class FormatterFactory { }, inlineJavaScript: { evaluateInlineJavaScript: async (code, context) => { - const { SingleInlineScriptEngine } = await import( - "../engine/SingleInlineScriptEngine" - ); - const executor = new SingleInlineScriptEngine( - this.app, - this.plugin, - this.requireChoiceExecutor(choiceExecutor), - context.variables, - ); - return await executor.runAndGetOutput(code); + const executor = this.requireChoiceExecutor(choiceExecutor); + if (!executor.evaluateInlineJavaScriptToken) { + throw new Error( + "Choice executor cannot evaluate runtime inline JavaScript tokens.", + ); + } + return await executor.evaluateInlineJavaScriptToken(code, context); }, }, }; From ed24d7a511e3b70f46b073c1e27000a2754c1eb3 Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Thu, 14 May 2026 21:32:45 +0200 Subject: [PATCH 5/7] fix: preserve template append formatting --- src/services/TemplateFileService.test.ts | 125 +++++++++++++++++++++++ src/services/TemplateFileService.ts | 12 ++- 2 files changed, 136 insertions(+), 1 deletion(-) diff --git a/src/services/TemplateFileService.test.ts b/src/services/TemplateFileService.test.ts index 3cb9dbfd..2de747cc 100644 --- a/src/services/TemplateFileService.test.ts +++ b/src/services/TemplateFileService.test.ts @@ -3,6 +3,7 @@ import type { App } from "obsidian"; import { TFile, TFolder } from "obsidian"; import type { TemplateEvaluator } from "./TemplateFileService"; import { TemplateFileService } from "./TemplateFileService"; +import { templaterParseTemplate } from "../utilityObsidian"; vi.mock("../utilityObsidian", async (importOriginal) => { const actual = (await importOriginal()) as Record; @@ -52,6 +53,10 @@ describe("TemplateFileService", () => { }, } as unknown as App; service = new TemplateFileService(app); + vi.mocked(templaterParseTemplate).mockClear(); + vi.mocked(templaterParseTemplate).mockImplementation( + async (_app, content: string) => content, + ); }); it("normalizes and reads only concrete template files", async () => { @@ -119,4 +124,124 @@ describe("TemplateFileService", () => { expect.any(Function), ); }); + + it("appends template content without collecting template property variables", async () => { + const existingFile = file("Notes/Daily.md"); + (app.vault.cachedRead as ReturnType).mockResolvedValue( + "existing", + ); + const evaluator = { + evaluateTemplateContent: vi.fn(), + evaluateTemplateContentForAppend: vi.fn(async () => "---\naliases: a, b\n---"), + } as unknown as TemplateEvaluator; + + const appended = await service.appendToFileWithTemplateContent( + existingFile, + "---\naliases: {{VALUE:aliases}}\n---", + "bottom", + evaluator, + ); + + expect(appended).toBe(existingFile); + expect(evaluator.evaluateTemplateContentForAppend).toHaveBeenCalledWith( + "---\naliases: {{VALUE:aliases}}\n---", + "Notes/Daily.md", + ); + expect(evaluator.evaluateTemplateContent).not.toHaveBeenCalled(); + expect(app.vault.modify).toHaveBeenCalledWith( + existingFile, + "existing\n---\naliases: a, b\n---", + ); + expect(app.fileManager.processFrontMatter).not.toHaveBeenCalled(); + }); + + it("parses markdown append content with Templater before modifying the file", async () => { + const existingFile = file("Notes/Daily.md"); + (app.vault.cachedRead as ReturnType).mockResolvedValue( + "existing", + ); + vi.mocked(templaterParseTemplate).mockResolvedValue("parsed"); + const evaluator = { + evaluateTemplateContentForAppend: vi.fn(async () => "raw templater"), + } as unknown as TemplateEvaluator; + + await service.appendToFileWithTemplateContent( + existingFile, + "template", + "top", + evaluator, + ); + + expect(templaterParseTemplate).toHaveBeenCalledTimes(1); + expect(templaterParseTemplate).toHaveBeenCalledWith( + app, + "raw templater", + existingFile, + ); + expect( + vi.mocked(templaterParseTemplate).mock.invocationCallOrder[0], + ).toBeLessThan( + (app.vault.cachedRead as ReturnType).mock + .invocationCallOrder[0], + ); + expect(app.vault.modify).toHaveBeenCalledWith( + existingFile, + "parsed\nexisting", + ); + }); + + it("leaves existing files unchanged when markdown append Templater parsing fails", async () => { + const existingFile = file("Notes/Daily.md"); + vi.mocked(templaterParseTemplate).mockRejectedValue(new Error("boom")); + const evaluator = { + evaluateTemplateContentForAppend: vi.fn(async () => "raw templater"), + } as unknown as TemplateEvaluator; + + const appended = await service.appendToFileWithTemplateContent( + existingFile, + "template", + "bottom", + evaluator, + ); + + expect(appended).toBeNull(); + expect(app.vault.cachedRead).not.toHaveBeenCalled(); + expect(app.vault.modify).not.toHaveBeenCalled(); + }); + + it("skips Templater when appending to canvas and base files", async () => { + const canvasFile = file("Boards/Board.canvas", "canvas"); + const baseFile = file("Bases/Data.base", "base"); + (app.vault.cachedRead as ReturnType) + .mockResolvedValueOnce("canvas existing") + .mockResolvedValueOnce("base existing"); + const evaluator = { + evaluateTemplateContentForAppend: vi.fn(async () => "content"), + } as unknown as TemplateEvaluator; + + await service.appendToFileWithTemplateContent( + canvasFile, + "template", + "bottom", + evaluator, + ); + await service.appendToFileWithTemplateContent( + baseFile, + "template", + "top", + evaluator, + ); + + expect(templaterParseTemplate).not.toHaveBeenCalled(); + expect(app.vault.modify).toHaveBeenNthCalledWith( + 1, + canvasFile, + "canvas existing\ncontent", + ); + expect(app.vault.modify).toHaveBeenNthCalledWith( + 2, + baseFile, + "content\nbase existing", + ); + }); }); diff --git a/src/services/TemplateFileService.ts b/src/services/TemplateFileService.ts index 5bcc5f8e..b97d8315 100644 --- a/src/services/TemplateFileService.ts +++ b/src/services/TemplateFileService.ts @@ -40,6 +40,16 @@ export class TemplateEvaluator { return { content, templatePropertyVars }; } + + public async evaluateTemplateContentForAppend( + templateContent: string, + targetPath: string, + ): Promise { + const fileBasename = basenameWithoutMdOrCanvas(targetPath); + this.formatter.setTitle(fileBasename); + + return await this.formatter.formatFileContent(templateContent); + } } function isMacroAbortError(error: unknown): error is MacroAbortError { @@ -276,7 +286,7 @@ export class TemplateFileService { evaluator: TemplateEvaluator, ): Promise { try { - let { content } = await evaluator.evaluateTemplateContent( + let content = await evaluator.evaluateTemplateContentForAppend( templateContent, file.path, ); From 7f94bfad618a5fb678a76e9f507434a64f0b0aa5 Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Thu, 14 May 2026 21:44:21 +0200 Subject: [PATCH 6/7] fix: wire capture formatter evaluators --- src/engine/CaptureChoiceEngine.ts | 8 +- .../captureChoiceFormatter-selection.test.ts | 77 +++++++++++++++++++ src/services/FormatterFactory.ts | 14 ++++ 3 files changed, 97 insertions(+), 2 deletions(-) diff --git a/src/engine/CaptureChoiceEngine.ts b/src/engine/CaptureChoiceEngine.ts index 844931a8..a4f8dc21 100644 --- a/src/engine/CaptureChoiceEngine.ts +++ b/src/engine/CaptureChoiceEngine.ts @@ -16,9 +16,10 @@ import { QA_INTERNAL_CAPTURE_TARGET_FILE_PATH, VALUE_SYNTAX, } from "../constants"; -import { CaptureChoiceFormatter } from "../formatters/captureChoiceFormatter"; +import type { CaptureChoiceFormatter } from "../formatters/captureChoiceFormatter"; import { log } from "../logger/logManager"; import type QuickAdd from "../main"; +import { FormatterFactory } from "../services/FormatterFactory"; import type ICaptureChoice from "../types/choices/ICaptureChoice"; import { normalizeAppendLinkOptions, type AppendLinkOptions } from "../types/linkPlacement"; import { @@ -74,7 +75,10 @@ export class CaptureChoiceEngine extends QuickAddChoiceEngine { super(app); this.choice = choice; this.plugin = plugin; - this.formatter = new CaptureChoiceFormatter(app, plugin, choiceExecutor); + this.formatter = new FormatterFactory( + app, + plugin, + ).createCaptureChoiceFormatter(choiceExecutor); } private showSuccessNotice( diff --git a/src/formatters/captureChoiceFormatter-selection.test.ts b/src/formatters/captureChoiceFormatter-selection.test.ts index 3e6efde2..0186d025 100644 --- a/src/formatters/captureChoiceFormatter-selection.test.ts +++ b/src/formatters/captureChoiceFormatter-selection.test.ts @@ -1,6 +1,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import type { App } from "obsidian"; import { CaptureChoiceFormatter } from "./captureChoiceFormatter"; +import { FormatterFactory } from "../services/FormatterFactory"; const { promptMock } = vi.hoisted(() => ({ promptMock: vi.fn().mockResolvedValue("prompted"), @@ -67,6 +68,28 @@ const createFormatter = ( return new CaptureChoiceFormatter(app, plugin, choiceExecutor); }; +const createFactoryFormatter = (choiceExecutor: any) => { + const app = { + workspace: { + getActiveViewOfType: vi.fn().mockReturnValue(null), + getActiveFile: vi.fn().mockReturnValue(null), + }, + } as unknown as App; + + const plugin = { + settings: { + inputPrompt: "single-line", + enableTemplatePropertyTypes: false, + globalVariables: {}, + useSelectionAsCaptureValue: true, + }, + } as any; + + return new FormatterFactory(app, plugin).createCaptureChoiceFormatter( + choiceExecutor, + ); +}; + describe("CaptureChoiceFormatter selection-as-value behavior", () => { beforeEach(() => { promptMock.mockClear(); @@ -119,4 +142,58 @@ describe("CaptureChoiceFormatter selection-as-value behavior", () => { expect(result).toContain("background-color: #BF616A"); expect(promptMock).not.toHaveBeenCalled(); }); + + it("evaluates macro tokens through the runtime choice executor with shared variables", async () => { + const variables = new Map([["before", "start"]]); + const choiceExecutor = { + execute: vi.fn(), + variables, + evaluateMacroToken: vi.fn().mockImplementation( + async (macroName: string, context: { variables: Map }) => { + expect(macroName).toBe("Capture Macro"); + expect(context.variables).toBe(variables); + context.variables.set("macroResult", "shared"); + return "macro-output"; + }, + ), + evaluateInlineJavaScriptToken: vi.fn(), + }; + + const formatter = createFactoryFormatter(choiceExecutor); + + const result = await formatter.formatContentOnly( + "{{MACRO:Capture Macro}} {{VALUE:macroResult}}", + ); + + expect(result).toBe("macro-output shared"); + expect(choiceExecutor.evaluateMacroToken).toHaveBeenCalledTimes(1); + expect(promptMock).not.toHaveBeenCalled(); + }); + + it("evaluates inline JavaScript tokens through the runtime choice executor with shared variables", async () => { + const variables = new Map([["input", "value"]]); + const choiceExecutor = { + execute: vi.fn(), + variables, + evaluateMacroToken: vi.fn(), + evaluateInlineJavaScriptToken: vi.fn().mockImplementation( + async (code: string, context: { variables: Map }) => { + expect(code).toBe('variables.set("inlineResult", "shared"); "inline-output";'); + expect(context.variables).toBe(variables); + context.variables.set("inlineResult", "shared"); + return "inline-output"; + }, + ), + }; + + const formatter = createFactoryFormatter(choiceExecutor); + + const result = await formatter.formatContentOnly( + '```js quickadd\nvariables.set("inlineResult", "shared"); "inline-output";\n``` {{VALUE:inlineResult}}', + ); + + expect(result).toBe("inline-output shared"); + expect(choiceExecutor.evaluateInlineJavaScriptToken).toHaveBeenCalledTimes(1); + expect(promptMock).not.toHaveBeenCalled(); + }); }); diff --git a/src/services/FormatterFactory.ts b/src/services/FormatterFactory.ts index bfca8d83..083acaaa 100644 --- a/src/services/FormatterFactory.ts +++ b/src/services/FormatterFactory.ts @@ -2,6 +2,7 @@ import type { App } from "obsidian"; import type { IChoiceExecutor } from "../IChoiceExecutor"; import type QuickAdd from "../main"; import type { IDateParser } from "../parsers/IDateParser"; +import { CaptureChoiceFormatter } from "../formatters/captureChoiceFormatter"; import { CompleteFormatter } from "../formatters/completeFormatter"; import { FormatDisplayFormatter } from "../formatters/formatDisplayFormatter"; import type { @@ -30,6 +31,19 @@ export class FormatterFactory { ); } + public createCaptureChoiceFormatter( + choiceExecutor?: IChoiceExecutor, + dateParser?: IDateParser, + ): CaptureChoiceFormatter { + return new CaptureChoiceFormatter( + this.app, + this.plugin, + choiceExecutor, + dateParser, + this.createCompleteEvaluators(choiceExecutor), + ); + } + public createDisplayFormatter(dateParser?: IDateParser): FormatDisplayFormatter { return new FormatDisplayFormatter( this.app, From 59ad913acfa78b3f3bf62f7ef43a6c3b41048ebe Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Thu, 14 May 2026 22:01:05 +0200 Subject: [PATCH 7/7] fix: remove capture single template dependency --- src/engine/CaptureChoiceEngine.ts | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/src/engine/CaptureChoiceEngine.ts b/src/engine/CaptureChoiceEngine.ts index a4f8dc21..f23eb5d7 100644 --- a/src/engine/CaptureChoiceEngine.ts +++ b/src/engine/CaptureChoiceEngine.ts @@ -20,6 +20,10 @@ import type { CaptureChoiceFormatter } from "../formatters/captureChoiceFormatte import { log } from "../logger/logManager"; import type QuickAdd from "../main"; import { FormatterFactory } from "../services/FormatterFactory"; +import { + TemplateEvaluator, + TemplateFileService, +} from "../services/TemplateFileService"; import type ICaptureChoice from "../types/choices/ICaptureChoice"; import { normalizeAppendLinkOptions, type AppendLinkOptions } from "../types/linkPlacement"; import { @@ -44,7 +48,6 @@ import { basenameWithoutMdOrCanvas } from "../utils/pathUtils"; import { QuickAddChoiceEngine } from "./QuickAddChoiceEngine"; import { ChoiceAbortError } from "../errors/ChoiceAbortError"; import { MacroAbortError } from "../errors/MacroAbortError"; -import { SingleTemplateEngine } from "./SingleTemplateEngine"; import { getCaptureAction, type CaptureAction } from "./captureAction"; import { getCanvasTextCaptureContent, @@ -62,6 +65,7 @@ export class CaptureChoiceEngine extends QuickAddChoiceEngine { choice: ICaptureChoice; private formatter: CaptureChoiceFormatter; private readonly plugin: QuickAdd; + private readonly templateFileService: TemplateFileService; private templatePropertyVars?: Map; private capturePropertyVars: Map = new Map(); @@ -79,6 +83,7 @@ export class CaptureChoiceEngine extends QuickAddChoiceEngine { app, plugin, ).createCaptureChoiceFormatter(choiceExecutor); + this.templateFileService = new TemplateFileService(app); } private showSuccessNotice( @@ -713,22 +718,19 @@ export class CaptureChoiceEngine extends QuickAddChoiceEngine { let fileContent = ""; if (this.choice.createFileIfItDoesntExist.createWithTemplate) { - const singleTemplateEngine: SingleTemplateEngine = - new SingleTemplateEngine( - this.app, - this.plugin, - this.choice.createFileIfItDoesntExist.template, - this.choiceExecutor, - ); - if (linkOptions?.enabled && !linkOptions.requireActiveFile) { - singleTemplateEngine.setLinkToCurrentFileBehavior("optional"); + this.formatter.setLinkToCurrentFileBehavior("optional"); } - fileContent = await singleTemplateEngine.run(); - - // Get template variables from the template engine's formatter - const templateVars = singleTemplateEngine.getAndClearTemplatePropertyVars(); + const templatePath = this.choice.createFileIfItDoesntExist.template; + const templateContent = + await this.templateFileService.readTemplateContent(templatePath); + const { content, templatePropertyVars: templateVars } = + await new TemplateEvaluator(this.formatter).evaluateTemplateContent( + templateContent, + filePath, + ); + fileContent = content; log.logMessage(`CaptureChoiceEngine: Collected ${templateVars.size} template property variables`); if (templateVars.size > 0) {