From ffd353c9e673bf2b47bdba61d47ea0aa654f73d3 Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Sun, 18 Oct 2026 09:27:26 +0200 Subject: [PATCH 1/2] test(template): cover a Keep existing file mode that respects Open Keep existing file always opens the note, even with Open off. These tests expect a second keep mode, Do nothing, that leaves the note closed unless Open is on, while the saved mode keeps opening it. Refs #2244 --- .../TemplateChoiceEngine.collision.test.ts | 32 +++++++++++++ .../ChoiceBuilder/TemplateChoiceForm.test.ts | 23 ++++++--- src/template/fileExistsPolicy.test.ts | 15 +----- tests/e2e/file-exists-behavior.test.ts | 47 +++++++++++++++++++ 4 files changed, 97 insertions(+), 20 deletions(-) diff --git a/src/engine/TemplateChoiceEngine.collision.test.ts b/src/engine/TemplateChoiceEngine.collision.test.ts index 6e5ffab5c..8d1de6dde 100644 --- a/src/engine/TemplateChoiceEngine.collision.test.ts +++ b/src/engine/TemplateChoiceEngine.collision.test.ts @@ -78,6 +78,11 @@ vi.mock("../formatters/completeFormatter", () => { vi.mock("../utils/templaterIntegration", () => ({ getTemplater: vi.fn(() => ({})), overwriteTemplaterOnce: vi.fn(), + jumpToNextTemplaterCursorIfPossible: vi.fn(async () => true), +})); +vi.mock("../utils/errorUtils", async (importOriginal) => ({ + ...(await importOriginal()), + reportError: vi.fn(), })); vi.mock("../utils/vaultQueries", () => ({ getAllFolderPathsInVault: vi.fn(() => []), @@ -125,6 +130,9 @@ vi.mock("./TemplateInsertEngine", () => { import { TFile, TFolder, type App } from "obsidian"; import GenericSuggester from "../gui/GenericSuggester/genericSuggester"; +import { Notice } from "../../tests/obsidian-stub"; +import { openFile as openFileInWorkspace } from "../utils/fileOpening"; +import { reportError } from "../utils/errorUtils"; import type { IChoiceExecutor } from "../IChoiceExecutor"; import { settingsStore } from "../settingsStore"; import { getPromptModes } from "../template/fileExistsPolicy"; @@ -739,4 +747,28 @@ describe("TemplateChoiceEngine collision behavior", () => { expect(GenericSuggester.Suggest).toHaveBeenCalledTimes(1); expect(engine.choice.fileExistsBehavior).toEqual({ kind: "prompt" }); }); + + it.each([ + { mode: "doNothing", openFile: false, opens: true, notice: [] }, + { mode: "leaveUnchanged", openFile: false, opens: false, notice: ["'Test Template' already exists."] }, + { mode: "leaveUnchanged", openFile: true, opens: true, notice: [] }, + ] as const)("$mode with openFile=$openFile opens the existing note: $opens", async ({ mode, openFile, opens, notice }) => { + const { app, engine } = createEngine(); + const existingFile = createExistingFile("Test Template.md"); + engine.choice.openFile = openFile; + engine.choice.fileExistsBehavior = { kind: "apply", mode }; + (app.vault.adapter.exists as ReturnType).mockResolvedValue(true); + (app.vault.getAbstractFileByPath as ReturnType).mockReturnValue(existingFile); + vi.mocked(openFileInWorkspace).mockClear(); + vi.mocked(reportError).mockClear(); + const noticesBefore = Notice.instances.length; + + await engine.run(); + + expect(vi.mocked(openFileInWorkspace).mock.calls.map(([, file]) => file)).toEqual( + opens ? [existingFile] : [], + ); + expect(Notice.instances.slice(noticesBefore).map(({ message }) => message)).toEqual(notice); + expect(reportError).not.toHaveBeenCalled(); + }); }); diff --git a/src/gui/ChoiceBuilder/TemplateChoiceForm.test.ts b/src/gui/ChoiceBuilder/TemplateChoiceForm.test.ts index 8b64d117f..0979eb8f4 100644 --- a/src/gui/ChoiceBuilder/TemplateChoiceForm.test.ts +++ b/src/gui/ChoiceBuilder/TemplateChoiceForm.test.ts @@ -244,16 +244,27 @@ describe("TemplateChoiceForm", () => { expect(settingNames(container)).toContain("File opening location"); }); - it("shows the file-exists mode row only for update/create categories", () => { + it("shows the file-exists mode row for every category except Ask every time", () => { const { container, props } = mountForm(); - expect(settingNames(container)).not.toContain("New file naming"); + const modeRows = ["Update action", "New file naming", "Keep action"]; + expect(settingNames(container).filter((name) => modeRows.includes(name))).toEqual([]); props.choice.fileExistsBehavior = { kind: "apply", mode: "increment" }; flushSync(); - const names = settingNames(container); - expect( - names.includes("New file naming") || names.includes("Update action"), - ).toBe(true); + expect(settingNames(container).filter((name) => modeRows.includes(name))).toEqual(["New file naming"]); + }); + + it("keeps a saved Keep existing file choice on Open existing file and offers Do nothing", async () => { + const { container, props } = mountForm(); + props.choice.fileExistsBehavior = { kind: "apply", mode: "doNothing" }; + flushSync(); + const select = settingItem(container, "Keep action").querySelector("select")!; + expect(select.value).toBe("doNothing"); + expect(Array.from(select.options, (option) => option.text)).toEqual(["Open existing file", "Do nothing"]); + + await fireEvent.change(select, { target: { value: "leaveUnchanged" } }); + flushSync(); + expect(props.choice.fileExistsBehavior).toEqual({ kind: "apply", mode: "leaveUnchanged" }); }); it("persists the copy-link-to-clipboard toggle", async () => { diff --git a/src/template/fileExistsPolicy.test.ts b/src/template/fileExistsPolicy.test.ts index a5a5276f4..8b4a59531 100644 --- a/src/template/fileExistsPolicy.test.ts +++ b/src/template/fileExistsPolicy.test.ts @@ -1,7 +1,6 @@ import { describe, expect, it, vi } from "vitest"; import { fileExistsBehaviorCategoryOptions, - getBehaviorCategory, getDefaultBehaviorForCategory, getFileExistsMode, getExistingNoteAction, @@ -47,22 +46,10 @@ describe("fileExistsPolicy registry", () => { ]); expect(getModesForCategory("keep").map((mode) => mode.id)).toEqual([ "doNothing", + "leaveUnchanged", ]); }); - it("derives behavior category from behavior state", () => { - expect(getBehaviorCategory({ kind: "prompt" })).toBe("prompt"); - expect( - getBehaviorCategory({ kind: "apply", mode: "appendBottom" }), - ).toBe("update"); - expect(getBehaviorCategory({ kind: "apply", mode: "increment" })).toBe( - "create", - ); - expect(getBehaviorCategory({ kind: "apply", mode: "doNothing" })).toBe( - "keep", - ); - }); - it("returns category defaults and preserves modes already in-category", () => { expect(getDefaultBehaviorForCategory("prompt")).toEqual({ kind: "prompt" }); expect(getDefaultBehaviorForCategory("update")).toEqual({ diff --git a/tests/e2e/file-exists-behavior.test.ts b/tests/e2e/file-exists-behavior.test.ts index feca3b6ba..a9f18e229 100644 --- a/tests/e2e/file-exists-behavior.test.ts +++ b/tests/e2e/file-exists-behavior.test.ts @@ -37,6 +37,7 @@ const behavior = { appendTop: { kind: "apply", mode: "appendTop" }, overwrite: { kind: "apply", mode: "overwrite" }, doNothing: { kind: "apply", mode: "doNothing" }, + leaveUnchanged: { kind: "apply", mode: "leaveUnchanged" }, prompt: { kind: "prompt" }, } as const; @@ -330,6 +331,52 @@ describe("functional: file collision behaviors", () => { }); }); +describe("with Open off, only Open existing file opens the note (#2244)", () => { + const expected = { + increment: { file: "open-off-increment1.md", effect: "created", opens: false }, + duplicateSuffix: { file: "open-off-duplicateSuffix (1).md", effect: "created", opens: false }, + appendBottom: { file: "open-off-appendBottom.md", effect: "changed", opens: false }, + appendTop: { file: "open-off-appendTop.md", effect: "changed", opens: false }, + overwrite: { file: "open-off-overwrite.md", effect: "changed", opens: false }, + doNothing: { file: "open-off-doNothing.md", effect: "unchanged", opens: true }, + leaveUnchanged: { file: "open-off-leaveUnchanged.md", effect: "unchanged", opens: false }, + } as const satisfies Record, unknown>; + const modes = Object.keys(expected) as Array; + + async function openSandboxNotes(): Promise { + return obsidian.dev.evalJson(`app.workspace.getLeavesOfType("markdown") + .map((leaf) => leaf.view.file?.path) + .filter((path) => path?.startsWith(${JSON.stringify(sandbox.path(""))}))`); + } + + beforeAll(async () => { + await qa.data().patch((data) => { + clearTestChoices(data); + data.choices.push( + ...modes.map((mode) => + templateChoice(`__qa-test-open-off-${mode}`, `${sandbox.root}/open-off-${mode}`, behavior[mode]), + ), + ); + }); + await qa.reload({ waitUntilReady: true }); + }, 15_000); + + beforeEach(async () => { + await obsidian.dev.eval(`app.workspace.getLeavesOfType("markdown") + .filter((leaf) => leaf.view.file?.path.startsWith(${JSON.stringify(sandbox.path(""))})) + .forEach((leaf) => leaf.detach())`); + }); + + it.each(modes)("%s", async (mode) => { + const { file, effect, opens } = expected[mode]; + await seedFile(`open-off-${mode}.md`, "EXISTING"); + const outcome = await obsidian.execJson("quickadd:run", { choice: `__qa-test-open-off-${mode}`, verify: true }); + expect(outcome).toMatchObject({ ok: true, effect, file: sandbox.path(file) }); + await obsidian.sleep(600); + expect(await openSandboxNotes()).toEqual(opens ? [sandbox.path(file)] : []); + }); +}); + // --------------------------------------------------------------------------- // Phase 2: Migration tests // --------------------------------------------------------------------------- From 2c36246a87f3b3ec1e7de31377abf9199674d77d Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Sun, 18 Oct 2026 09:27:26 +0200 Subject: [PATCH 2/2] feat(template): add Do nothing to Keep existing file Keep existing file opened the note even with Open off, and there was no way to keep it closed. Keep existing file now has two actions. Open existing file is the old behavior, under the id that saved choices already store. Do nothing leaves the note alone and lets Open decide. When the choice is set to Do nothing and nothing else shows the note, QuickAdd says the note already exists. The Ask every time prompt keeps its items and what they do. Its Do nothing item, which opened the note, is now labeled Open existing file. Fixes #2244 --- docs/src/content/docs/docs/Advanced/CLI.md | 6 ++-- .../docs/docs/Choices/TemplateChoice.md | 10 +++---- src/engine/TemplateChoiceEngine.ts | 21 +++++++------ src/engine/choiceOutcomeRecorder.ts | 2 +- .../ChoiceBuilder/TemplateChoiceForm.svelte | 26 +++++++--------- src/template/fileExistsPolicy.ts | 30 +++++++++++-------- src/types/ChoiceOutcome.ts | 2 +- 7 files changed, 50 insertions(+), 47 deletions(-) diff --git a/docs/src/content/docs/docs/Advanced/CLI.md b/docs/src/content/docs/docs/Advanced/CLI.md index 4f9fa52ca..0b122c1f7 100644 --- a/docs/src/content/docs/docs/Advanced/CLI.md +++ b/docs/src/content/docs/docs/Advanced/CLI.md @@ -320,9 +320,9 @@ obsidian vault=dev quickadd:run choice="Inbox" value-value=" " verify=true That run is working exactly as designed: the capture's payload was empty, so QuickAdd deliberately left `Inbox.md` alone rather than writing a blank line, and -said so in a notice. A Template set to **Do nothing** when the file already exists -reports the same. If you are counting captures, writing an idempotency marker, or -deciding whether to retry, key off `effect`, not `ok`. +said so in a notice. A Template set to **Keep existing file** when the file +already exists reports the same `effect`. If you are counting captures, writing an +idempotency marker, or deciding whether to retry, key off `effect`, not `ok`. `effect` is present on every **success** payload (`ok:true`), and `unknown` is stated rather than omitted - a missing key reads as `false` in both `jq` and JavaScript, which diff --git a/docs/src/content/docs/docs/Choices/TemplateChoice.md b/docs/src/content/docs/docs/Choices/TemplateChoice.md index cf75ace5a..387b06ebe 100644 --- a/docs/src/content/docs/docs/Choices/TemplateChoice.md +++ b/docs/src/content/docs/docs/Choices/TemplateChoice.md @@ -431,6 +431,7 @@ With **Search existing notes before creating** enabled, this setting is called existing file**, **Create another file**, or **Keep existing file**. - **Update action** - shown only when you choose **Update existing file**. - **New file naming** - shown only when you choose **Create another file**. +- **Keep action** - shown only when you choose **Keep existing file**. ### Let QuickAdd ask each time {#ask-every-time} @@ -442,7 +443,7 @@ already exists: - **Overwrite file** - **Increment trailing number** - **Append duplicate suffix** -- **Do nothing** +- **Open existing file** ### Update the existing note {#update-existing-file} @@ -479,8 +480,7 @@ These options keep the existing file untouched and create a new file instead: ### Keep the existing note {#keep-existing-file} -Selecting **Keep existing file** applies the same result as choosing **Do -nothing** from the prompt: +These options leave the existing file unchanged: -- **Do nothing** - leaves the existing file unchanged and opens it - automatically. This does not require the separate **Open** setting. +- **Open existing file** - opens the existing file, even when **Open** is off. +- **Do nothing** - opens the existing file only when **Open** is on. diff --git a/src/engine/TemplateChoiceEngine.ts b/src/engine/TemplateChoiceEngine.ts index 6d71aabbb..b1141ded3 100644 --- a/src/engine/TemplateChoiceEngine.ts +++ b/src/engine/TemplateChoiceEngine.ts @@ -155,8 +155,8 @@ export class TemplateChoiceEngine extends TemplateEngine { // What this run did to its target note (#1615). Derived from the file-exists // resolution the engine actually performed rather than from a byte compare, // which is exact for the two answers an automation acts on: "createNew" - // always writes a new note, and "reuseExisting" — the shipped "Do nothing" - // mode — writes nothing at all. Only "modifyExisting" (append/overwrite) is + // always writes a new note, and "reuseExisting" — the Keep existing file + // modes — writes nothing at all. Only "modifyExisting" (append/overwrite) is // inferred: it always writes, so `changed` can in principle over-report a // write whose bytes happened to match, which is the harmless direction. let effect: ChoiceEffect = "created"; @@ -290,14 +290,18 @@ export class TemplateChoiceEngine extends TemplateEngine { this.placeCursor(createdFile, cursorBeforeLink); } } else if ( - createdNew && + (createdNew || effect === "unchanged") && !linkOptions.enabled && !this.choice.copyLinkToClipboard ) { - // The note was created but nothing else surfaces it (not opened, no - // link appended, not copied to clipboard). Confirm the creation so - // the run isn't silent — mirroring Capture's success notice. - new Notice(`Created '${createdFile.basename}'.`); + // Nothing else surfaces the note (not opened, no link appended, not + // copied to clipboard). Say what happened so the run isn't silent - + // mirroring Capture's success notice. + new Notice( + createdNew + ? `Created '${createdFile.basename}'.` + : `'${createdFile.basename}' already exists.`, + ); } } catch (err) { if ( @@ -515,10 +519,9 @@ export class TemplateChoiceEngine extends TemplateEngine { }; } case "reuseExisting": - log.logMessage(`Opening existing file: ${existingFile!.path}`); return { createdFile: existingFile, - shouldAutoOpen: true, + shouldAutoOpen: mode.opensExistingFile, }; } } diff --git a/src/engine/choiceOutcomeRecorder.ts b/src/engine/choiceOutcomeRecorder.ts index 2e7546ec1..f1cb9e129 100644 --- a/src/engine/choiceOutcomeRecorder.ts +++ b/src/engine/choiceOutcomeRecorder.ts @@ -50,7 +50,7 @@ export class ChoiceOutcomeRecorder { * * `effect` is required rather than defaulted: every call site has to state its claim, * so a new one cannot inherit a positive "something landed" by omission. The four - * existing sites split evenly — two of them (Template's "Do nothing" mode and its + * existing sites split evenly — two of them (Template's Keep existing file modes and its * open-an-existing-note discovery path) commit nothing at all. */ success(file: TFile | undefined, effect: ChoiceEffect): void { diff --git a/src/gui/ChoiceBuilder/TemplateChoiceForm.svelte b/src/gui/ChoiceBuilder/TemplateChoiceForm.svelte index 864002f85..8fd52d9b8 100644 --- a/src/gui/ChoiceBuilder/TemplateChoiceForm.svelte +++ b/src/gui/ChoiceBuilder/TemplateChoiceForm.svelte @@ -9,7 +9,6 @@ import type { } from "../../template/fileExistsPolicy"; import { fileExistsBehaviorCategoryOptions, - getBehaviorCategory, getDefaultBehaviorForCategory, getFileExistsMode, getModesForCategory, @@ -178,18 +177,13 @@ commitPending = addPendingFolder; onDestroy(addPendingFolder); // --- File already exists ------------------------------------------------- -const behaviorCategory = $derived(getBehaviorCategory(choice.fileExistsBehavior)); -const showModeRow = $derived( - behaviorCategory !== "prompt" && behaviorCategory !== "keep", -); -const modeOptions = $derived( - getModesForCategory(behaviorCategory === "update" ? "update" : "create"), -); +const modeRowName = { update: "Update action", create: "New file naming", keep: "Keep action" }; const selectedMode = $derived( choice.fileExistsBehavior.kind === "apply" - ? choice.fileExistsBehavior.mode - : modeOptions[0].id, + ? getFileExistsMode(choice.fileExistsBehavior.mode) + : null, ); +const behaviorCategory = $derived(selectedMode?.category ?? "prompt"); function onCategoryChange(value: string) { choice.fileExistsBehavior = getDefaultBehaviorForCategory( @@ -375,15 +369,15 @@ function onModeChange(value: string) { {/snippet} - {#if showModeRow} + {#if selectedMode} {#snippet control()} ({ + value={selectedMode.id} + options={getModesForCategory(selectedMode.category).map((mode) => ({ value: mode.id, label: mode.label, }))} @@ -393,7 +387,7 @@ function onModeChange(value: string) { {/if} - + {#if choice.openFile} {/if} diff --git a/src/template/fileExistsPolicy.ts b/src/template/fileExistsPolicy.ts index c4e31400c..5d4ed5592 100644 --- a/src/template/fileExistsPolicy.ts +++ b/src/template/fileExistsPolicy.ts @@ -63,12 +63,24 @@ export const fileExistsModes = [ resolutionKind: "createNew", }, { + // Saved choices store this id from when it was the only keep mode, so it + // keeps its id and its behavior: it opens the note even with Open off. id: "doNothing", category: "keep", + label: "Open existing file", + description: "Leaves the existing file unchanged and opens it, even when Open is off.", + requiresExistingFile: true, + resolutionKind: "reuseExisting", + opensExistingFile: true, + }, + { + id: "leaveUnchanged", + category: "keep", label: "Do nothing", - description: "Leaves the file unchanged and opens the existing file.", + description: "Leaves the existing file unchanged. It opens only when Open is on.", requiresExistingFile: true, resolutionKind: "reuseExisting", + opensExistingFile: false, }, ] as const; @@ -133,6 +145,7 @@ const legacyFileExistsModeMap: Record = { increment: "increment", duplicateSuffix: "duplicateSuffix", doNothing: "doNothing", + leaveUnchanged: "leaveUnchanged", }; const createNewPathResolvers: Record< @@ -163,18 +176,11 @@ function getCategoryForMode( return getFileExistsMode(modeId).category; } +// The prompt's Do nothing item used to open the note, so its slot now reads Open +// existing file with the same behavior. Leaving the note alone from the prompt is +// what Escape already does. export function getPromptModes(): FileExistsModeDefinition[] { - return [...fileExistsModes]; -} - -export function getBehaviorCategory( - behavior: TemplateFileExistsBehavior, -): FileExistsBehaviorCategoryId { - if (behavior.kind === "prompt") { - return "prompt"; - } - - return getCategoryForMode(behavior.mode); + return fileExistsModes.filter((mode) => mode.id !== "leaveUnchanged"); } export function getDefaultBehaviorForCategory( diff --git a/src/types/ChoiceOutcome.ts b/src/types/ChoiceOutcome.ts index 881b176a9..4e43aa743 100644 --- a/src/types/ChoiceOutcome.ts +++ b/src/types/ChoiceOutcome.ts @@ -12,7 +12,7 @@ import type { TFile } from "obsidian"; * automation asks — it asks "did anything land?". Those came apart in two shipped * configurations that need no interactivity at all: a Capture whose formatted payload * is empty deliberately leaves the file untouched (inserting would add a blank line - * or, on `currentLine`, DELETE the selection), and a Template set to "Do nothing" when + * or, on `currentLine`, DELETE the selection), and a Template set to "Keep existing file" when * the file exists is, by name, a no-op. Both reported an unqualified success, so a * caller counting captures or writing an idempotency marker recorded work that never * happened (#1615).