From c92c9edc9cc5e1f645c48b2e2f008d2307ccef31 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Sun, 27 Sep 2026 15:23:23 -0400 Subject: [PATCH 1/4] ENG-2273 Scroll to and highlight the matched line when previewing a tagged result Co-Authored-By: Claude Opus 5.5 Entire-Checkpoint: 01M3J54Y422F140AV66BB5NYM8 --- .../src/components/NodeSearchModal.tsx | 71 ++++++++- apps/obsidian/src/styles/style.css | 16 ++ .../utils/__tests__/taggedLineLocator.test.ts | 141 +++++++++++++++++ apps/obsidian/src/utils/taggedLineLocator.ts | 145 ++++++++++++++++++ 4 files changed, 366 insertions(+), 7 deletions(-) create mode 100644 apps/obsidian/src/utils/__tests__/taggedLineLocator.test.ts create mode 100644 apps/obsidian/src/utils/taggedLineLocator.ts diff --git a/apps/obsidian/src/components/NodeSearchModal.tsx b/apps/obsidian/src/components/NodeSearchModal.tsx index a6e90a03d9..2104991247 100644 --- a/apps/obsidian/src/components/NodeSearchModal.tsx +++ b/apps/obsidian/src/components/NodeSearchModal.tsx @@ -58,6 +58,7 @@ import { type SortDirection, type SortKey, } from "~/utils/discourseNodeSort"; +import { findTaggedLineElement } from "~/utils/taggedLineLocator"; const MAX_VISIBLE_RESULTS = 50; const SEARCH_DEBOUNCE_MS = 250; @@ -83,6 +84,27 @@ const formatTimestamp = (epochMs: number): string => timeStyle: "short", }); +const PREVIEW_FLASH_CLASS = "dg-search-preview-flash"; + +// An image reserves no height until it loads, which would shift a line scrolled to before then. +const waitForImages = async (container: HTMLElement): Promise => { + const pending = Array.from(container.querySelectorAll("img")).filter( + (img) => !img.complete, + ); + await Promise.race([ + Promise.all( + pending.map( + (img) => + new Promise((resolve) => { + img.addEventListener("load", () => resolve(), { once: true }); + img.addEventListener("error", () => resolve(), { once: true }); + }), + ), + ), + new Promise((resolve) => window.setTimeout(resolve, 1000)), + ]); +}; + const PreviewPane = ({ app, result, @@ -98,6 +120,7 @@ const PreviewPane = ({ const [loaded, setLoaded] = useState<{ file: TFile; text: string } | null>( null, ); + const [renderedFile, setRenderedFile] = useState(null); const file = result?.file; @@ -120,21 +143,55 @@ const PreviewPane = ({ if (!container || !file || loaded?.file !== file) return; container.empty(); + setRenderedFile(null); const component = new Component(); - void MarkdownRenderer.render( - app, - loaded.text.trim() || "This note is empty.", - container, - file.path, - component, - ); + let cancelled = false; + void (async () => { + await MarkdownRenderer.render( + app, + loaded.text.trim() || "This note is empty.", + container, + file.path, + component, + ); + await waitForImages(container); + if (!cancelled) setRenderedFile(file); + })(); return () => { + cancelled = true; component.unload(); container.empty(); }; }, [app, file, loaded]); + // Separate from rendering, so moving between two lines of one note only re-scrolls. + const taggedLine = result?.tagLine?.line; + useEffect(() => { + const container = containerRef.current; + if (!container || !loaded || renderedFile !== loaded.file) return; + if (taggedLine === undefined) { + container.scrollTop = 0; + return; + } + + const target = findTaggedLineElement({ + container, + cache: app.metadataCache.getFileCache(loaded.file), + line: taggedLine, + sourceLine: loaded.text.split("\n")[taggedLine] ?? "", + }); + if (!target) return; + target.scrollIntoView({ block: "center" }); + target.addClass(PREVIEW_FLASH_CLASS); + const clearFlash = () => target.removeClass(PREVIEW_FLASH_CLASS); + target.addEventListener("animationend", clearFlash, { once: true }); + return () => { + target.removeEventListener("animationend", clearFlash); + clearFlash(); + }; + }, [app, loaded, renderedFile, taggedLine]); + if (!result || !file) { return (
diff --git a/apps/obsidian/src/styles/style.css b/apps/obsidian/src/styles/style.css index 3cf9c64ce9..6f325b7d64 100644 --- a/apps/obsidian/src/styles/style.css +++ b/apps/obsidian/src/styles/style.css @@ -3947,3 +3947,19 @@ kbd.tlui-kbd { cursor: not-allowed; opacity: 0.5; } + +/* Held at full color first, so the fade isn't mostly gone before the eye lands after the scroll. */ +.dg-node-search-modal .dg-search-preview-flash { + animation: dg-search-preview-flash 3s ease-out; + border-radius: var(--radius-s); +} + +@keyframes dg-search-preview-flash { + 0%, + 35% { + background-color: var(--text-highlight-bg); + } + 100% { + background-color: transparent; + } +} diff --git a/apps/obsidian/src/utils/__tests__/taggedLineLocator.test.ts b/apps/obsidian/src/utils/__tests__/taggedLineLocator.test.ts new file mode 100644 index 0000000000..b1ca5819ee --- /dev/null +++ b/apps/obsidian/src/utils/__tests__/taggedLineLocator.test.ts @@ -0,0 +1,141 @@ +import { describe, expect, it } from "vitest"; +import type { ListItemCache, SectionCache } from "obsidian"; +import { locateTaggedLine, renderedLineText } from "~/utils/taggedLineLocator"; + +const pos = (startLine: number, endLine: number, startOffset = startLine) => ({ + start: { line: startLine, col: 0, offset: startOffset }, + end: { line: endLine, col: 1, offset: startOffset + 1 }, +}); + +const section = ( + type: string, + startLine: number, + endLine = startLine, +): SectionCache => ({ type, position: pos(startLine, endLine) }); + +const item = (startLine: number, endLine = startLine): ListItemCache => ({ + parent: -1, + position: pos(startLine, endLine), +}); + +describe("locateTaggedLine", () => { + const sections = [ + section("yaml", 0, 2), + section("heading", 3), + section("paragraph", 4, 6), + section("footnoteDefinition", 7), + section("list", 8, 12), + section("table", 14, 17), + ]; + const listItems = [item(8, 9), item(10), item(11, 12)]; + + it("counts frontmatter as a block and skips footnote definitions", () => { + expect(locateTaggedLine({ sections, listItems, line: 3 })).toEqual({ + blockIndex: 1, + blockCount: 5, + }); + expect(locateTaggedLine({ sections, listItems, line: 8 })?.blockIndex).toBe( + 3, + ); + }); + + it("targets the whole block for a line inside a multi-line paragraph", () => { + expect(locateTaggedLine({ sections, listItems, line: 5 })).toEqual({ + blockIndex: 2, + blockCount: 5, + }); + }); + + it("skips zero-width sections at the end of the file", () => { + const eof = { line: 18, col: 0, offset: 500 }; + const withTrailing: SectionCache[] = [ + ...sections, + { type: "text", position: { start: eof, end: eof } }, + ]; + expect( + locateTaggedLine({ sections: withTrailing, listItems, line: 3 }) + ?.blockCount, + ).toBe(5); + }); + + it("picks the list item whose range covers the line, including continuations", () => { + expect(locateTaggedLine({ sections, listItems, line: 9 })?.item).toEqual({ + kind: "li", + index: 0, + count: 3, + }); + expect(locateTaggedLine({ sections, listItems, line: 12 })?.item).toEqual({ + kind: "li", + index: 2, + count: 3, + }); + }); + + it("picks the innermost item when a parent's range spans a nested one", () => { + const nested = [item(8, 12), item(10), item(11)]; + expect( + locateTaggedLine({ sections, listItems: nested, line: 10 })?.item?.index, + ).toBe(1); + }); + + it("ignores list items outside the section", () => { + const withOther = [item(0), ...listItems]; + expect( + locateTaggedLine({ sections, listItems: withOther, line: 10 })?.item, + ).toEqual({ kind: "li", index: 1, count: 3 }); + }); + + it("maps table lines to rows, skipping the separator line", () => { + const at = (line: number) => + locateTaggedLine({ sections, listItems, line })?.item; + expect(at(14)).toEqual({ kind: "tr", index: 0, count: 3 }); + expect(at(15)).toBeUndefined(); + expect(at(17)).toEqual({ kind: "tr", index: 2, count: 3 }); + }); + + it("returns null for a line in no rendered block", () => { + expect(locateTaggedLine({ sections, listItems, line: 7 })).toBeNull(); + expect(locateTaggedLine({ sections, listItems, line: 13 })).toBeNull(); + }); +}); + +describe("renderedLineText", () => { + it("drops block and list markers but keeps tags", () => { + expect(renderedLineText("## Heading #clm")).toBe("Heading #clm"); + expect(renderedLineText(" - [x] done #clm")).toBe("done #clm"); + expect(renderedLineText("> 1. quoted #clm")).toBe("quoted #clm"); + }); + + it("shows wikilinks and markdown links as their display text", () => { + expect( + renderedLineText("See [[Some note|alias text]] and [[Other]] #clm"), + ).toBe("See alias text and Other #clm"); + expect(renderedLineText("A [site](https://x.io/a_b) link")).toBe( + "A site link", + ); + }); + + it("drops embeds, footnote refs and comments", () => { + expect(renderedLineText("Pic ![[img.png]] here[^1] %%hidden%% end")).toBe( + "Pic here end", + ); + }); + + it("strips emphasis, highlight and strikethrough markers", () => { + expect( + renderedLineText("**bold** __b2__ *it* _it2_ ==hl== ~~gone~~ #clm"), + ).toBe("bold b2 it it2 hl gone #clm"); + }); + + it("keeps intraword underscores", () => { + expect(renderedLineText("use snake_case_name here")).toBe( + "use snake_case_name here", + ); + }); + + it("keeps inline code content verbatim", () => { + expect(renderedLineText("Run `a **b** [[c]]` now")).toBe( + "Run a **b** [[c]] now", + ); + }); +}); diff --git a/apps/obsidian/src/utils/taggedLineLocator.ts b/apps/obsidian/src/utils/taggedLineLocator.ts new file mode 100644 index 0000000000..1693815837 --- /dev/null +++ b/apps/obsidian/src/utils/taggedLineLocator.ts @@ -0,0 +1,145 @@ +import type { + CacheItem, + CachedMetadata, + ListItemCache, + SectionCache, +} from "obsidian"; +import { extractListPrefix } from "~/utils/taggedLine"; + +export type TaggedLineTarget = { + blockIndex: number; + blockCount: number; + item?: { kind: "li" | "tr"; index: number; count: number }; +}; + +/** + * Maps a 0-based source line to the `MarkdownRenderer.render` output, which + * has no source-line mapping but emits one top-level element per section. + */ +export const locateTaggedLine = ({ + sections, + listItems = [], + line, +}: { + sections: SectionCache[]; + listItems?: ListItemCache[]; + line: number; +}): TaggedLineTarget | null => { + // Footnote definitions render in a trailing footnotes section; empty EOF sections render nothing. + const blocks = sections.filter( + ({ type, position }) => + type !== "footnoteDefinition" && + position.start.offset !== position.end.offset, + ); + const covers = ({ position }: CacheItem): boolean => + position.start.line <= line && line <= position.end.line; + const blockIndex = blocks.findIndex(covers); + const block = blocks[blockIndex]; + if (!block) return null; + + const target = { blockIndex, blockCount: blocks.length }; + const { start, end } = block.position; + if (block.type === "list") { + const items = listItems + .filter( + ({ position }) => + start.line <= position.start.line && position.start.line <= end.line, + ) + .sort((a, b) => a.position.start.line - b.position.start.line); + // Nested items start later, so the last covering item is the innermost. + const index = items.map(covers).lastIndexOf(true); + return index === -1 + ? target + : { ...target, item: { kind: "li", index, count: items.length } }; + } + if (block.type === "table") { + const row = line - start.line; + // The separator line (row 1) renders no . + if (row === 1) return target; + return { + ...target, + item: { + kind: "tr", + index: row === 0 ? 0 : row - 1, + count: end.line - start.line, + }, + }; + } + return target; +}; + +const QUOTE_MARKERS_REGEX = /^\s*(?:>\s*)+/; +const HEADING_MARKER_REGEX = /^#{1,6}\s+/; + +const stripInlineMarkdown = (text: string): string => + text + .replace(/!\[\[[^\]]*\]\]|!\[[^\]]*\]\([^)]*\)/g, "") + .replace(/\[\[[^\]|]*\|([^\]]*)\]\]/g, "$1") + .replace(/\[\[([^\]]*)\]\]/g, "$1") + .replace(/\[([^\]]*)\]\([^)]*\)/g, "$1") + .replace(/\[\^[^\]]+\]|%%.*?%%/g, "") + .replace(/(\*\*|__|==|~~)(.+?)\1/g, "$2") + .replace(/\*(\S(?:.*?\S)?)\*/g, "$1") + // An intraword underscore (snake_case) is not emphasis. + .replace(/(^|\W)_(\S(?:.*?\S)?)_(?!\w)/g, "$1$2"); + +/** + * The text Obsidian's renderer shows for one source line, minus footnote refs. + * Tags keep their `#`, as the rendered tag link does. + */ +export const renderedLineText = (sourceLine: string): string => { + const unquoted = sourceLine.replace(QUOTE_MARKERS_REGEX, ""); + return unquoted + .slice(extractListPrefix(unquoted).length) + .replace(HEADING_MARKER_REGEX, "") + .split(/(`[^`]*`)/) + .map((part, i) => (i % 2 ? part.slice(1, -1) : stripInlineMarkdown(part))) + .join("") + .replace(/\s+/g, " ") + .trim(); +}; + +// Nested lists belong to other items, and footnote refs render as "[n]". +const ownText = (el: Element): string => { + const clone = el.cloneNode(true) as Element; + clone.querySelectorAll("ul, ol, sup.footnote-ref").forEach((n) => n.remove()); + return (clone.textContent ?? "").replace(/\s+/g, " ").trim(); +}; + +export const findTaggedLineElement = ({ + container, + cache, + line, + sourceLine, +}: { + container: HTMLElement; + cache: CachedMetadata | null; + line: number; + sourceLine: string; +}): HTMLElement | null => { + const target = cache?.sections + ? locateTaggedLine({ + sections: cache.sections, + listItems: cache.listItems, + line, + }) + : null; + const blocks = container.querySelectorAll( + ":scope > :not(section.footnotes)", + ); + if (target && blocks.length === target.blockCount) { + const block = blocks[target.blockIndex]; + const { item } = target; + if (!item) return block ?? null; + const items = block?.querySelectorAll(item.kind); + if (items?.length === item.count) return items[item.index] ?? null; + } + + // Counts disagree (e.g. a stale cache): only a unique exact text match is safe. + const text = renderedLineText(sourceLine); + if (!text) return null; + const matches = Array.from( + container.querySelectorAll("p, li, h1, h2, h3, h4, h5, h6"), + ).filter((el) => ownText(el) === text); + return matches.length === 1 ? (matches[0] ?? null) : null; +}; From 5faaddc92050ea4b4b9f6dc5c7325869d7079c7a Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Sun, 27 Sep 2026 15:27:36 -0400 Subject: [PATCH 2/4] ENG-2273 Apply adherence check fixes Co-Authored-By: Claude Opus 5.5 Entire-Checkpoint: 01M3J5CM30EQJKV9EHB765CWV0 --- .../src/components/NodeSearchModal.tsx | 2 +- .../utils/__tests__/taggedLineLocator.test.ts | 18 +++++++++++------- apps/obsidian/src/utils/taggedLineLocator.ts | 5 +++-- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/apps/obsidian/src/components/NodeSearchModal.tsx b/apps/obsidian/src/components/NodeSearchModal.tsx index 2104991247..520f8ca141 100644 --- a/apps/obsidian/src/components/NodeSearchModal.tsx +++ b/apps/obsidian/src/components/NodeSearchModal.tsx @@ -184,7 +184,7 @@ const PreviewPane = ({ if (!target) return; target.scrollIntoView({ block: "center" }); target.addClass(PREVIEW_FLASH_CLASS); - const clearFlash = () => target.removeClass(PREVIEW_FLASH_CLASS); + const clearFlash = (): void => target.removeClass(PREVIEW_FLASH_CLASS); target.addEventListener("animationend", clearFlash, { once: true }); return () => { target.removeEventListener("animationend", clearFlash); diff --git a/apps/obsidian/src/utils/__tests__/taggedLineLocator.test.ts b/apps/obsidian/src/utils/__tests__/taggedLineLocator.test.ts index b1ca5819ee..55dc1ca17f 100644 --- a/apps/obsidian/src/utils/__tests__/taggedLineLocator.test.ts +++ b/apps/obsidian/src/utils/__tests__/taggedLineLocator.test.ts @@ -1,8 +1,12 @@ import { describe, expect, it } from "vitest"; -import type { ListItemCache, SectionCache } from "obsidian"; +import type { ListItemCache, Pos, SectionCache } from "obsidian"; import { locateTaggedLine, renderedLineText } from "~/utils/taggedLineLocator"; -const pos = (startLine: number, endLine: number, startOffset = startLine) => ({ +const pos = ( + startLine: number, + endLine: number, + startOffset = startLine, +): Pos => ({ start: { line: startLine, col: 0, offset: startOffset }, end: { line: endLine, col: 1, offset: startOffset + 1 }, }); @@ -86,11 +90,11 @@ describe("locateTaggedLine", () => { }); it("maps table lines to rows, skipping the separator line", () => { - const at = (line: number) => - locateTaggedLine({ sections, listItems, line })?.item; - expect(at(14)).toEqual({ kind: "tr", index: 0, count: 3 }); - expect(at(15)).toBeUndefined(); - expect(at(17)).toEqual({ kind: "tr", index: 2, count: 3 }); + const at = (line: number): ReturnType => + locateTaggedLine({ sections, listItems, line }); + expect(at(14)?.item).toEqual({ kind: "tr", index: 0, count: 3 }); + expect(at(15)?.item).toBeUndefined(); + expect(at(17)?.item).toEqual({ kind: "tr", index: 2, count: 3 }); }); it("returns null for a line in no rendered block", () => { diff --git a/apps/obsidian/src/utils/taggedLineLocator.ts b/apps/obsidian/src/utils/taggedLineLocator.ts index 1693815837..4b97d2ba83 100644 --- a/apps/obsidian/src/utils/taggedLineLocator.ts +++ b/apps/obsidian/src/utils/taggedLineLocator.ts @@ -6,7 +6,7 @@ import type { } from "obsidian"; import { extractListPrefix } from "~/utils/taggedLine"; -export type TaggedLineTarget = { +type TaggedLineTarget = { blockIndex: number; blockCount: number; item?: { kind: "li" | "tr"; index: number; count: number }; @@ -84,7 +84,8 @@ const stripInlineMarkdown = (text: string): string => .replace(/(^|\W)_(\S(?:.*?\S)?)_(?!\w)/g, "$1$2"); /** - * The text Obsidian's renderer shows for one source line, minus footnote refs. + * The text Obsidian's renderer shows for one source line, minus footnote refs, + * embeds and comments. * Tags keep their `#`, as the rendered tag link does. */ export const renderedLineText = (sourceLine: string): string => { From 8a0e7507e20d4111d526439d8648790ff5e61af3 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Sun, 27 Sep 2026 15:31:31 -0400 Subject: [PATCH 3/4] ENG-2273 Reset preview scroll when the tagged line can't be located Co-Authored-By: Claude Opus 5.5 Entire-Checkpoint: 01M3J5KSWXQJWTYT3PWBJ2GQS2 --- .../src/components/NodeSearchModal.tsx | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/apps/obsidian/src/components/NodeSearchModal.tsx b/apps/obsidian/src/components/NodeSearchModal.tsx index 520f8ca141..0a01e73e36 100644 --- a/apps/obsidian/src/components/NodeSearchModal.tsx +++ b/apps/obsidian/src/components/NodeSearchModal.tsx @@ -170,18 +170,21 @@ const PreviewPane = ({ useEffect(() => { const container = containerRef.current; if (!container || !loaded || renderedFile !== loaded.file) return; - if (taggedLine === undefined) { + + const target = + taggedLine === undefined + ? null + : findTaggedLineElement({ + container, + cache: app.metadataCache.getFileCache(loaded.file), + line: taggedLine, + sourceLine: loaded.text.split("\n")[taggedLine] ?? "", + }); + // Same-note switches don't re-render, so an unlocated line would keep the last scroll. + if (!target) { container.scrollTop = 0; return; } - - const target = findTaggedLineElement({ - container, - cache: app.metadataCache.getFileCache(loaded.file), - line: taggedLine, - sourceLine: loaded.text.split("\n")[taggedLine] ?? "", - }); - if (!target) return; target.scrollIntoView({ block: "center" }); target.addClass(PREVIEW_FLASH_CLASS); const clearFlash = (): void => target.removeClass(PREVIEW_FLASH_CLASS); From 672317e03d794fcf75d2862f32d1bd3246b3e696 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Sun, 27 Sep 2026 15:47:52 -0400 Subject: [PATCH 4/4] ENG-2273 Address Devin review: stale cache, fallback coverage, file-switch race Co-Authored-By: Claude Opus 5.5 Entire-Checkpoint: 01M3J6HQZKQ6DYSAAWQW3F5CAH --- .../src/components/NodeSearchModal.tsx | 9 +- .../utils/__tests__/taggedLineLocator.test.ts | 60 +++++++++- apps/obsidian/src/utils/taggedLineLocator.ts | 111 +++++++++++++----- 3 files changed, 145 insertions(+), 35 deletions(-) diff --git a/apps/obsidian/src/components/NodeSearchModal.tsx b/apps/obsidian/src/components/NodeSearchModal.tsx index 0a01e73e36..db877dbd40 100644 --- a/apps/obsidian/src/components/NodeSearchModal.tsx +++ b/apps/obsidian/src/components/NodeSearchModal.tsx @@ -169,16 +169,17 @@ const PreviewPane = ({ const taggedLine = result?.tagLine?.line; useEffect(() => { const container = containerRef.current; - if (!container || !loaded || renderedFile !== loaded.file) return; + // `loaded` still holds the previous note until the new file's read finishes. + if (!container || !loaded || !file || renderedFile !== file) return; const target = taggedLine === undefined ? null : findTaggedLineElement({ container, - cache: app.metadataCache.getFileCache(loaded.file), + cache: app.metadataCache.getFileCache(file), + text: loaded.text, line: taggedLine, - sourceLine: loaded.text.split("\n")[taggedLine] ?? "", }); // Same-note switches don't re-render, so an unlocated line would keep the last scroll. if (!target) { @@ -193,7 +194,7 @@ const PreviewPane = ({ target.removeEventListener("animationend", clearFlash); clearFlash(); }; - }, [app, loaded, renderedFile, taggedLine]); + }, [app, file, loaded, renderedFile, taggedLine]); if (!result || !file) { return ( diff --git a/apps/obsidian/src/utils/__tests__/taggedLineLocator.test.ts b/apps/obsidian/src/utils/__tests__/taggedLineLocator.test.ts index 55dc1ca17f..16ce5c985d 100644 --- a/apps/obsidian/src/utils/__tests__/taggedLineLocator.test.ts +++ b/apps/obsidian/src/utils/__tests__/taggedLineLocator.test.ts @@ -1,6 +1,10 @@ import { describe, expect, it } from "vitest"; import type { ListItemCache, Pos, SectionCache } from "obsidian"; -import { locateTaggedLine, renderedLineText } from "~/utils/taggedLineLocator"; +import { + locateTaggedLine, + renderedLineText, + sectionsMatchText, +} from "~/utils/taggedLineLocator"; const pos = ( startLine: number, @@ -143,3 +147,57 @@ describe("renderedLineText", () => { ); }); }); + +describe("renderedLineText for table rows", () => { + it("joins the cells the way the rendered row reads", () => { + expect(renderedLineText("| **Alpha** | owner | #clm |")).toBe( + "Alpha owner #clm", + ); + expect(renderedLineText("| | b |")).toBe("b"); + }); +}); + +describe("sectionsMatchText", () => { + const text = "# Title\n\nfirst para\nsecond line\n"; + const at = (line: number, col: number, offset: number) => ({ + line, + col, + offset, + }); + const fresh: SectionCache[] = [ + { type: "heading", position: { start: at(0, 0, 0), end: at(0, 7, 7) } }, + { + type: "paragraph", + position: { start: at(2, 0, 9), end: at(3, 11, 31) }, + }, + ]; + + it("accepts sections whose offsets line up with the text", () => { + expect(sectionsMatchText({ sections: fresh, text })).toBe(true); + }); + + it("rejects sections from before an edit that shifted the text", () => { + expect(sectionsMatchText({ sections: fresh, text: `Intro\n${text}` })).toBe( + false, + ); + }); + + it("ignores empty EOF sections, whose offsets Obsidian reports one short", () => { + const eof = at(4, 0, 32); + const withEof: SectionCache[] = [ + ...fresh, + { type: "text", position: { start: eof, end: eof } }, + ]; + expect(sectionsMatchText({ sections: withEof, text })).toBe(true); + }); + + it("rejects sections that point past the last line", () => { + const beyond: SectionCache[] = [ + { + type: "paragraph", + position: { start: at(9, 0, 40), end: at(9, 1, 41) }, + }, + ]; + expect(sectionsMatchText({ sections: beyond, text })).toBe(false); + }); +}); diff --git a/apps/obsidian/src/utils/taggedLineLocator.ts b/apps/obsidian/src/utils/taggedLineLocator.ts index 4b97d2ba83..2881c94030 100644 --- a/apps/obsidian/src/utils/taggedLineLocator.ts +++ b/apps/obsidian/src/utils/taggedLineLocator.ts @@ -2,6 +2,7 @@ import type { CacheItem, CachedMetadata, ListItemCache, + Loc, SectionCache, } from "obsidian"; import { extractListPrefix } from "~/utils/taggedLine"; @@ -12,6 +13,11 @@ type TaggedLineTarget = { item?: { kind: "li" | "tr"; index: number; count: number }; }; +// Footnote definitions render in a trailing footnotes section; empty EOF sections render nothing. +const rendersInPlace = ({ type, position }: SectionCache): boolean => + type !== "footnoteDefinition" && + position.start.offset !== position.end.offset; + /** * Maps a 0-based source line to the `MarkdownRenderer.render` output, which * has no source-line mapping but emits one top-level element per section. @@ -25,12 +31,7 @@ export const locateTaggedLine = ({ listItems?: ListItemCache[]; line: number; }): TaggedLineTarget | null => { - // Footnote definitions render in a trailing footnotes section; empty EOF sections render nothing. - const blocks = sections.filter( - ({ type, position }) => - type !== "footnoteDefinition" && - position.start.offset !== position.end.offset, - ); + const blocks = sections.filter(rendersInPlace); const covers = ({ position }: CacheItem): boolean => position.start.line <= line && line <= position.end.line; const blockIndex = blocks.findIndex(covers); @@ -70,6 +71,7 @@ export const locateTaggedLine = ({ const QUOTE_MARKERS_REGEX = /^\s*(?:>\s*)+/; const HEADING_MARKER_REGEX = /^#{1,6}\s+/; +const TABLE_ROW_REGEX = /^\s*\|/; const stripInlineMarkdown = (text: string): string => text @@ -83,13 +85,8 @@ const stripInlineMarkdown = (text: string): string => // An intraword underscore (snake_case) is not emphasis. .replace(/(^|\W)_(\S(?:.*?\S)?)_(?!\w)/g, "$1$2"); -/** - * The text Obsidian's renderer shows for one source line, minus footnote refs, - * embeds and comments. - * Tags keep their `#`, as the rendered tag link does. - */ -export const renderedLineText = (sourceLine: string): string => { - const unquoted = sourceLine.replace(QUOTE_MARKERS_REGEX, ""); +const lineDisplayText = (line: string): string => { + const unquoted = line.replace(QUOTE_MARKERS_REGEX, ""); return unquoted .slice(extractListPrefix(unquoted).length) .replace(HEADING_MARKER_REGEX, "") @@ -100,6 +97,40 @@ export const renderedLineText = (sourceLine: string): string => { .trim(); }; +/** + * The text Obsidian's renderer shows for one source line, minus footnote refs, + * embeds and comments. Tags keep their `#`; a table row becomes its cells joined by spaces. + */ +export const renderedLineText = (sourceLine: string): string => + TABLE_ROW_REGEX.test(sourceLine) + ? sourceLine + .trim() + .replace(/^\||\|$/g, "") + .split("|") + .map(lineDisplayText) + .filter(Boolean) + .join(" ") + : lineDisplayText(sourceLine); + +/** A cache that lags an edit has section offsets that no longer line up with the text. */ +export const sectionsMatchText = ({ + sections, + text, +}: { + sections: SectionCache[]; + text: string; +}): boolean => { + const lineStarts = [0]; + for (let i = 0; i < text.length; i++) { + if (text[i] === "\n") lineStarts.push(i + 1); + } + const isAt = ({ line, col, offset }: Loc): boolean => + lineStarts[line] !== undefined && lineStarts[line] + col === offset; + return sections + .filter(rendersInPlace) + .every(({ position }) => isAt(position.start) && isAt(position.end)); +}; + // Nested lists belong to other items, and footnote refs render as "[n]". const ownText = (el: Element): string => { const clone = el.cloneNode(true) as Element; @@ -107,24 +138,50 @@ const ownText = (el: Element): string => { return (clone.textContent ?? "").replace(/\s+/g, " ").trim(); }; +// A table row's cells are separate elements, so join them as `renderedLineText` does. +const displayedText = (el: HTMLElement): string => + el.tagName === "TR" + ? Array.from(el.querySelectorAll("th, td")) + .map(ownText) + .filter(Boolean) + .join(" ") + : ownText(el); + +const findByText = ( + container: HTMLElement, + sourceLine: string, +): HTMLElement | null => { + const lineText = renderedLineText(sourceLine); + if (!lineText) return null; + const elements = Array.from( + container.querySelectorAll( + "p, li, h1, h2, h3, h4, h5, h6, tr", + ), + ).map((el) => ({ el, text: displayedText(el) })); + + // Only a unique match is safe; containment covers one line of a multi-line paragraph. + const exact = elements.filter(({ text }) => text === lineText); + if (exact.length) return exact.length === 1 ? (exact[0]?.el ?? null) : null; + const containing = elements.filter(({ text }) => text.includes(lineText)); + return containing.length === 1 ? (containing[0]?.el ?? null) : null; +}; + export const findTaggedLineElement = ({ container, cache, + text, line, - sourceLine, }: { container: HTMLElement; cache: CachedMetadata | null; + text: string; line: number; - sourceLine: string; }): HTMLElement | null => { - const target = cache?.sections - ? locateTaggedLine({ - sections: cache.sections, - listItems: cache.listItems, - line, - }) - : null; + const sections = cache?.sections; + const target = + sections && sectionsMatchText({ sections, text }) + ? locateTaggedLine({ sections, listItems: cache.listItems, line }) + : null; const blocks = container.querySelectorAll( ":scope > :not(section.footnotes)", ); @@ -135,12 +192,6 @@ export const findTaggedLineElement = ({ const items = block?.querySelectorAll(item.kind); if (items?.length === item.count) return items[item.index] ?? null; } - - // Counts disagree (e.g. a stale cache): only a unique exact text match is safe. - const text = renderedLineText(sourceLine); - if (!text) return null; - const matches = Array.from( - container.querySelectorAll("p, li, h1, h2, h3, h4, h5, h6"), - ).filter((el) => ownText(el) === text); - return matches.length === 1 ? (matches[0] ?? null) : null; + // A stale cache or an unexpected render: fall back to the line's text. + return findByText(container, text.split("\n")[line] ?? ""); };