From 97228d149ebfe76b84a58e8d726303752d3e4bd4 Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Tue, 16 Jun 2026 17:02:08 +0200 Subject: [PATCH] fix(capture): abort inline insert-after on a multi-line target instead of silently missing (#468) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue #468 asked for multi-line "Insert after" matching. The default (block) path was already fixed in #1333; the inline insert-after path was the remaining gap. It skipped escape expansion, so a multi-line `\n` target silently never matched (Create line if not found off) or wrote a literal backslash-n into the note (Create line if not found on) — while the settings preview expanded `\n` and made the target look valid. - Expand `\n` escapes on the inline target before searching, symmetric with the block path, so a multi-line target trips the existing single-line guard. - That guard now throws ChoiceAbortError (parity with the block path's not-found abort) instead of logging and returning unchanged content, which previously produced a contradictory "Captured" success notice. - Warn in the Capture builder when inline mode has a target that resolves to more than one line, computed with the same FormatDisplayFormatter the live preview uses (no raw-regex drift on escaped `\\n` or token/global expansions). - Tests: new captureChoiceFormatter-468-inline-multiline.test.ts (RED without the fix) plus a backslash-semantics boundary control; update the one frontmatter test that asserted the old log-and-continue behavior. - Docs: note that Inline insertion is single-line by design. --- docs/docs/Choices/CaptureChoice.md | 2 + ...oiceFormatter-468-inline-multiline.test.ts | 312 ++++++++++++++++++ ...captureChoiceFormatter-frontmatter.test.ts | 15 +- src/formatters/captureChoiceFormatter.ts | 19 +- .../components/InsertAfterFields.svelte | 38 +++ 5 files changed, 368 insertions(+), 18 deletions(-) create mode 100644 src/formatters/captureChoiceFormatter-468-inline-multiline.test.ts diff --git a/docs/docs/Choices/CaptureChoice.md b/docs/docs/Choices/CaptureChoice.md index 16ea4779..70f6f778 100644 --- a/docs/docs/Choices/CaptureChoice.md +++ b/docs/docs/Choices/CaptureChoice.md @@ -270,6 +270,8 @@ I use this in my daily journal capture, where I insert after the heading line `# It's also possible to use `Create line if not found`, which will create the line if it doesn't exist. This is useful if you want to insert after a line that might not exist in the file you're capturing to. This setting can place the created line at the **Top** or **Bottom** of the file, at your **Cursor** position, or **Ordered** — at its sorted position among same-level headings. See [Ordered section placement](#ordered-section-placement) for reverse-chronological daily logs, changelogs, and other sorted sections. +The target may span several lines: type `\n` in the **Insert after** field to match a multi-line anchor (the preview shows it expanded). **Inline insertion** is the exception — because it inserts on the same line, immediately after the matched text, its target must be a single line; a multi-line (`\n`) target there is rejected with a notice rather than matched. Use the default placement for multi-line anchors. + ### Ordered section placement When `Create line if not found` is enabled and its location dropdown is set to **Ordered** (the full label is `Ordered (place new section among siblings)`), a missing "Insert after" heading is created at its **sorted position among same-level sibling headings** instead of at the top or bottom of the note. This is the building block for a reverse-chronological log: a new dated section is added above older ones, while a fixed title/intro stays pinned at the top. diff --git a/src/formatters/captureChoiceFormatter-468-inline-multiline.test.ts b/src/formatters/captureChoiceFormatter-468-inline-multiline.test.ts new file mode 100644 index 00000000..0e9323a8 --- /dev/null +++ b/src/formatters/captureChoiceFormatter-468-inline-multiline.test.ts @@ -0,0 +1,312 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { App, TFile } from "obsidian"; +import type ICaptureChoice from "../types/choices/ICaptureChoice"; + +// Mocks mirror captureChoiceFormatter-742-multiline-insert.test.ts so the +// formatter can run under jsdom without real Obsidian/Templater. The inline +// single-line guard now throws ChoiceAbortError (asserted via rejects.toThrow), +// so these tests don't inspect reportError (issue #468). +vi.mock("../utilityObsidian", () => ({ + templaterParseTemplate: vi.fn().mockResolvedValue(null), +})); +vi.mock("../gui/InputPrompt", () => ({ + __esModule: true, + default: class { + factory() { + return { + Prompt: vi.fn().mockResolvedValue(""), + PromptWithContext: vi.fn().mockResolvedValue(""), + } as any; + } + }, +})); +vi.mock("../gui/InputSuggester/inputSuggester", () => ({ + __esModule: true, + default: class { + constructor() {} + }, +})); +vi.mock("../gui/GenericSuggester/genericSuggester", () => ({ + __esModule: true, + default: { Suggest: vi.fn().mockResolvedValue("") }, +})); +vi.mock("../gui/VDateInputPrompt/VDateInputPrompt", () => ({ + __esModule: true, + default: { Prompt: vi.fn().mockResolvedValue("") }, +})); +vi.mock("../utils/errorUtils", () => ({ + __esModule: true, + reportError: vi.fn(), + isCancellationError: vi.fn().mockReturnValue(false), +})); +vi.mock("../gui/MathModal", () => ({ + __esModule: true, + MathModal: { Prompt: vi.fn().mockResolvedValue("") }, +})); +vi.mock("../engine/SingleInlineScriptEngine", () => ({ + __esModule: true, + SingleInlineScriptEngine: class { + public params = { variables: {} as Record }; + async runAndGetOutput() { + return ""; + } + }, +})); +vi.mock("../engine/SingleMacroEngine", () => ({ + __esModule: true, + SingleMacroEngine: class { + async runAndGetOutput() { + return ""; + } + }, +})); +vi.mock("../engine/SingleTemplateEngine", () => ({ + __esModule: true, + SingleTemplateEngine: class { + async run() { + return ""; + } + getAndClearTemplatePropertyVars() { + return new Map(); + } + setLinkToCurrentFileBehavior() {} + }, +})); +vi.mock("obsidian-dataview", () => ({ + __esModule: true, + getAPI: vi.fn().mockReturnValue(null), +})); +vi.mock("../main", () => ({ + __esModule: true, + default: class QuickAdd { + static instance = { + settings: { inputPrompt: "single-line" }, + app: { + workspace: { getActiveViewOfType: vi.fn().mockReturnValue(null) }, + }, + }; + settings = QuickAdd.instance.settings; + app = QuickAdd.instance.app; + }, +})); + +import { CaptureChoiceFormatter } from "./captureChoiceFormatter"; + +const baseInsertAfter = { + enabled: true, + after: "", + insertAtEnd: false, + considerSubsections: false, + createIfNotFound: false, + createIfNotFoundLocation: "top", + inline: true, + replaceExisting: false, + blankLineAfterMatchMode: "auto" as const, +}; + +const createChoice = ( + overrides: Partial = {}, +): ICaptureChoice => + ({ + id: "test", + name: "Test Choice", + type: "Capture", + command: false, + captureTo: "Target.md", + captureToActiveFile: false, + captureToCanvasNodeId: "", + activeFileWritePosition: "cursor", + createFileIfItDoesntExist: { + enabled: false, + createWithTemplate: false, + template: "", + }, + format: { enabled: false, format: "" }, + prepend: false, + appendLink: false, + task: false, + insertAfter: { ...baseInsertAfter }, + insertBefore: { + enabled: false, + before: "", + createIfNotFound: false, + createIfNotFoundLocation: "top", + }, + newLineCapture: { enabled: false, direction: "below" }, + openFile: false, + fileOpening: { + location: "tab", + direction: "vertical", + mode: "default", + focus: true, + }, + ...overrides, + }) as ICaptureChoice; + +const createMockApp = (): App => + ({ + workspace: { + getActiveFile: vi.fn().mockReturnValue(null), + getActiveViewOfType: vi.fn().mockReturnValue(null), + }, + metadataCache: { getFileCache: vi.fn().mockReturnValue(null) }, + fileManager: { + generateMarkdownLink: vi.fn().mockReturnValue(""), + processFrontMatter: vi.fn(), + }, + vault: { adapter: { exists: vi.fn() }, cachedRead: vi.fn() }, + }) as unknown as App; + +const createFile = (path = "Target.md"): TFile => + ({ + path, + name: path.split("/").pop() ?? path, + basename: (path.split("/").pop() ?? path).replace(/\.(md|canvas)$/i, ""), + extension: "md", + }) as unknown as TFile; + +const createFormatter = () => + new CaptureChoiceFormatter(createMockApp(), { + settings: { + inputPrompt: "single-line", + enableTemplatePropertyTypes: false, + globalVariables: {}, + useSelectionAsCaptureValue: true, + }, + } as any); + +beforeEach(() => { + (global as any).navigator = { + clipboard: { readText: vi.fn().mockResolvedValue("") }, + }; +}); + +const run = ( + choice: ICaptureChoice, + capture: string, + fileContent: string, +): Promise => + createFormatter().formatContentWithFile( + capture, + choice, + fileContent, + createFile(), + ); + +/** + * Inline insertion is single-line by definition. A `\n`-escape target renders as + * a valid multi-line block in the settings preview, but inline can never match a + * multi-line anchor. Before #468 the inline path skipped escape expansion, so it + * searched for / wrote the LITERAL two-char `\n`: the match silently never + * succeeded (createIfNotFound off) or a garbage line containing a literal + * backslash-n was written (createIfNotFound on). The fix expands escapes so the + * existing single-line guard fires and the capture aborts cleanly (a + * ChoiceAbortError, like the block path's not-found abort) with nothing written. + */ +describe("#468 — inline insert-after with a multi-line target aborts cleanly, no silent miss / garbage write", () => { + it("aborts with a clear 'single line' message when the block exists but inline is on", async () => { + const choice = createChoice({ + insertAfter: { ...baseInsertAfter, after: "**Today**\\n***" }, + }); + // The block plainly exists; the non-inline block path matches it fine. Inline + // must NOT silently miss with a misleading "target not found" — it must abort + // with the real reason (multi-line target). + const seed = "# Daily Notes\n\n**Today**\n***\n\nBelow.\n"; + await expect(run(choice, "CAPTURED", seed)).rejects.toThrow(/single line/i); + }); + + it("aborts (no literal-\\n garbage write) when createIfNotFound is on and the target is absent", async () => { + const choice = createChoice({ + insertAfter: { + ...baseInsertAfter, + after: "**Today**\\n***", + createIfNotFound: true, + createIfNotFoundLocation: "bottom", + }, + }); + const seed = "# Notes\n\nNo anchor here.\n"; + // The single-line guard fires BEFORE the create path, so the literal + // backslash-n block is never written — the run aborts instead. + await expect(run(choice, "CAPTURED", seed)).rejects.toThrow(/single line/i); + }); + + it("control: a single-line inline target still inserts on the same line (no regression)", async () => { + const choice = createChoice({ + insertAfter: { ...baseInsertAfter, after: "Status:" }, + }); + const result = await run(choice, " done", "Status: pending"); + + expect(result).toBe("Status: done pending"); + }); + + it("control: a multi-line target on the NON-inline block path still matches (issue #742 preserved)", async () => { + const choice = createChoice({ + insertAfter: { + ...baseInsertAfter, + after: "**Today**\\n***", + inline: false, + }, + }); + const seed = "# Daily Notes\n\n**Today**\n***\n\nBelow.\n"; + const result = await run(choice, "CAPTURED", seed); + + // Found and inserted after the block. + expect(result).not.toBe(seed); + expect(result).toContain("**Today**\n***"); + expect(result).toContain("CAPTURED"); + }); + + // --- Boundary: escape semantics now shared with the block path (#742). --- + // Expanding the inline target means `\n` is a newline QuickAdd-wide, while a + // doubled backslash `\\` collapses to a single literal `\`. Pin both so a + // future refactor can't silently widen/narrow the false-abort surface. + + it("control: an escaped backslash (\\\\) collapses to one literal backslash and still inserts (no false abort)", async () => { + const choice = createChoice({ + // Stored value is the two chars `\` `\` then "X" — what a user types to + // mean a literal backslash. It must NOT be read as a line break. + insertAfter: { ...baseInsertAfter, after: "Path:\\\\X" }, + }); + const result = await run(choice, "-OK", "Path:\\X here"); + + // `\\` -> `\`, so the single-line target "Path:\X" matches and inserts inline. + expect(result).toBe("Path:\\X-OK here"); + }); + + it("aborts a single-line-looking target that contains \\n (e.g. a Windows path 'C:\\notes') — \\n is a newline by convention", async () => { + const choice = createChoice({ + // `C:\notes` — the `\n` is QuickAdd's newline escape, so this expands to a + // multi-line target and is correctly rejected for inline (consistent with + // the block path, which has expanded `\n` since #742). + insertAfter: { ...baseInsertAfter, after: "C:\\notes" }, + }); + await expect(run(choice, "x", "C:\\notes\\file")).rejects.toThrow( + /single line/i, + ); + }); + + it("aborts when a token expands to a multi-line value (guard runs AFTER expansion, not just escapes)", async () => { + // A global variable whose snippet contains a real newline makes the resolved + // inline target multi-line — the guard must catch it post-expansion. + const formatter = new CaptureChoiceFormatter(createMockApp(), { + settings: { + inputPrompt: "single-line", + enableTemplatePropertyTypes: false, + globalVariables: { multi: "line-a\nline-b" }, + useSelectionAsCaptureValue: true, + }, + } as any); + const choice = createChoice({ + insertAfter: { ...baseInsertAfter, after: "{{GLOBAL_VAR:multi}}" }, + }); + + await expect( + formatter.formatContentWithFile( + "x", + choice, + "line-a\nline-b\n", + createFile(), + ), + ).rejects.toThrow(/single line/i); + }); +}); diff --git a/src/formatters/captureChoiceFormatter-frontmatter.test.ts b/src/formatters/captureChoiceFormatter-frontmatter.test.ts index 80ac9829..ffa837cf 100644 --- a/src/formatters/captureChoiceFormatter-frontmatter.test.ts +++ b/src/formatters/captureChoiceFormatter-frontmatter.test.ts @@ -103,7 +103,6 @@ vi.mock('../main', () => ({ })); import { CaptureChoiceFormatter } from './captureChoiceFormatter'; -import { reportError } from '../utils/errorUtils'; const createChoice = (overrides: Partial = {}): ICaptureChoice => ({ id: 'test', @@ -741,20 +740,14 @@ describe('CaptureChoiceFormatter insert after inline', () => { expect(result).toBe('Status: done'); }); - it('reports an error and leaves content unchanged when target contains a newline', async () => { + it('aborts with a clear message when the inline target contains a newline (must be single-line, issue #468)', async () => { const { formatter, file } = createFormatter(); const choice = createInlineChoice('Status:\n', { replaceExisting: true }); const fileContent = 'Status:\npending'; - const result = await formatter.formatContentWithFile( - 'done', - choice, - fileContent, - file, - ); - - expect(result).toBe(fileContent); - expect(reportError).toHaveBeenCalled(); + await expect( + formatter.formatContentWithFile('done', choice, fileContent, file), + ).rejects.toThrow(/single line/i); }); }); diff --git a/src/formatters/captureChoiceFormatter.ts b/src/formatters/captureChoiceFormatter.ts index 14e14b49..f8615b07 100644 --- a/src/formatters/captureChoiceFormatter.ts +++ b/src/formatters/captureChoiceFormatter.ts @@ -9,7 +9,6 @@ import { import type ICaptureChoice from "../types/choices/ICaptureChoice"; import type { BlankLineAfterMatchMode } from "../types/choices/ICaptureChoice"; import { templaterParseTemplate } from "../utilityObsidian"; -import { reportError } from "../utils/errorUtils"; import { ChoiceAbortError } from "../errors/ChoiceAbortError"; import { CompleteFormatter } from "./completeFormatter"; import getEndOfSection, { getMarkdownHeadings } from "./helpers/getEndOfSection"; @@ -499,12 +498,16 @@ export class CaptureChoiceFormatter extends CompleteFormatter { const override = this.insertAfterTargetOverride; // Inline targets are single-line by definition and use a separate - // indexOf-based path; keep their selector unexpanded (out of scope #742). + // indexOf-based path. Expand `\n` escapes here (like the block path below) + // so a multi-line target trips the single-line guard in + // insertAfterInlineHandler with a clear error — instead of silently + // searching for a literal backslash-n that never exists, or writing it + // verbatim into the note on create-if-not-found (issue #468). // The heading-picker override always wants the block (section) path, never // the same-line inline path, so the override short-circuits inline here too. if (this.choice.insertAfter?.inline && override === null) { const inlineTarget: string = await this.formatLocationString( - this.choice.insertAfter.after, + await this.expandFormatTemplateEscapes(this.choice.insertAfter.after), ); return await this.insertAfterInlineHandler(formatted, inlineTarget); } @@ -661,11 +664,13 @@ export class CaptureChoiceFormatter extends CompleteFormatter { targetString: string, ): Promise { if (this.hasInlineTargetLinebreak(targetString)) { - reportError( - new Error("Inline insert after target must be a single line."), - "Insert After Inline Error", + // Inline insertion lands mid-line after a matched substring, so a + // multi-line target can never match. Abort cleanly with a clear message + // (parity with the block path's not-found abort) instead of searching for + // a literal `\n` or writing it verbatim on create-if-not-found (issue #468). + throw new ChoiceAbortError( + "Inline insert-after target must be a single line — remove the line break (\\n) or turn off inline insertion.", ); - return this.fileContent; } const matchIndex = this.fileContent.indexOf(targetString); diff --git a/src/gui/ChoiceBuilder/components/InsertAfterFields.svelte b/src/gui/ChoiceBuilder/components/InsertAfterFields.svelte index 2e496990..cfc8c175 100644 --- a/src/gui/ChoiceBuilder/components/InsertAfterFields.svelte +++ b/src/gui/ChoiceBuilder/components/InsertAfterFields.svelte @@ -11,6 +11,7 @@ import { CREATE_IF_NOT_FOUND_TOP, } from "../../../constants"; import { FormatSyntaxSuggester } from "../../suggesters/formatSyntaxSuggester"; +import { FormatDisplayFormatter } from "../../../formatters/formatDisplayFormatter"; import SettingItem from "../../components/SettingItem.svelte"; import Toggle from "../../components/Toggle.svelte"; import Dropdown from "../../components/Dropdown.svelte"; @@ -110,6 +111,36 @@ const isOrdered = $derived( insertAfter.createIfNotFound && orderedSelected, ); +// Inline same-line insertion can only match a single line, but the live preview +// above expands `\n`, so a multi-line target looks valid while the capture +// silently never lands. Warn when inline mode is on and the target RESOLVES to +// more than one line (issue #468). We resolve with the same FormatDisplayFormatter +// the preview/runtime use — not a raw regex — so an escaped `\\n` (stays a literal +// backslash-n, single line) does not false-warn and a token/global-var that +// expands to a real newline is not missed. +const inlinePreviewFormatter = $derived(new FormatDisplayFormatter(app, plugin)); +let inlineTargetHasLinebreak = $state(false); +$effect(() => { + const raw = insertAfter.after ?? ""; + const formatter = inlinePreviewFormatter; + if (!insertAfter.inline || raw === "") { + inlineTargetHasLinebreak = false; + return; + } + let cancelled = false; + void formatter + .format(raw) + .then((resolved) => { + if (!cancelled) inlineTargetHasLinebreak = /[\n\r]/.test(resolved); + }) + .catch(() => { + if (!cancelled) inlineTargetHasLinebreak = false; + }); + return () => { + cancelled = true; + }; +}); + // Non-optional read view for the ordered sub-panel: the panel only renders when // `insertAfter.orderBy` is present, but its `{#snippet}` closures lose the // truthiness narrowing, so reads go through this guaranteed-present derived. @@ -255,6 +286,13 @@ function onPromptHeadingToggle(value: boolean) { {/snippet} + + {#if inlineTargetHasLinebreak} +
+ Inline insertion needs a single-line target. The line break (\n) can't be + matched on the same line — remove it, or turn off "Inline insertion". +
+ {/if} {/if} {#if insertAfter.inline}