From 60e451db9fb2b31d8ca58a2b1b71639d4d26588c Mon Sep 17 00:00:00 2001 From: Andre Light <7719153+lightmotive@users.noreply.github.com> Date: Mon, 13 Apr 2026 18:13:09 -0600 Subject: [PATCH] fix: sanitize heading link targets and fix substring matching Strip # | ^ : %% [[ ]] from heading link targets to match Obsidian's autocomplete behavior. Fix compareIgnoreCase to use strict equality instead of includes(), preventing false-positive note-link simplification. --- src/linkBuilder.test.ts | 131 +++++++++++++++++++++++++++++++--------- src/linkBuilder.ts | 31 ++++++++-- 2 files changed, 128 insertions(+), 34 deletions(-) diff --git a/src/linkBuilder.test.ts b/src/linkBuilder.test.ts index 2a97504..99bfa73 100644 --- a/src/linkBuilder.test.ts +++ b/src/linkBuilder.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest'; -import { buildHeadingLink, buildBlockLink, buildFileLink, extractBlockDisplayText, encodeMarkdownLinkUrl } from './linkBuilder'; +import { buildHeadingLink, buildBlockLink, buildFileLink, extractBlockDisplayText, encodeMarkdownLinkUrl, sanitizeHeadingForLink } from './linkBuilder'; import { LinkFormat } from './type'; // --------------------------------------------------------------------------- @@ -24,6 +24,50 @@ describe('encodeMarkdownLinkUrl', () => { }); }); +// --------------------------------------------------------------------------- +// sanitizeHeadingForLink +// --------------------------------------------------------------------------- + +describe('sanitizeHeadingForLink', () => { + it('strips # | ^ : with surrounding whitespace', () => { + expect(sanitizeHeadingForLink('Test | If # This ^ Heading : Works')) + .toBe('Test If This Heading Works'); + }); + + it('strips special chars without surrounding whitespace', () => { + expect(sanitizeHeadingForLink('Test|If#This^Heading:Works, Too')) + .toBe('Test If This Heading Works, Too'); + }); + + it('strips only the special chars that are present', () => { + expect(sanitizeHeadingForLink('Test | If This ^ Heading Works (No Hash or Colon)')) + .toBe('Test If This Heading Works (No Hash or Colon)'); + }); + + it('strips %% (comment marker)', () => { + expect(sanitizeHeadingForLink('Before %% comment %% After')) + .toBe('Before comment After'); + }); + + it('strips [[ and ]]', () => { + expect(sanitizeHeadingForLink('See [[Other Note]] here')) + .toBe('See Other Note here'); + }); + + it('returns heading unchanged when no special chars', () => { + expect(sanitizeHeadingForLink('Normal Heading Text')) + .toBe('Normal Heading Text'); + }); + + it('handles empty string', () => { + expect(sanitizeHeadingForLink('')).toBe(''); + }); + + it('handles heading that is only special chars', () => { + expect(sanitizeHeadingForLink('# | ^')).toBe(''); + }); +}); + // --------------------------------------------------------------------------- // buildHeadingLink // --------------------------------------------------------------------------- @@ -190,29 +234,62 @@ describe('buildHeadingLink', () => { }); }); - // -- compareIgnoreCase `includes` behavior -------------------------------- - // BUG: compareIgnoreCase uses filename.includes(heading), which causes - // false-positive note-link simplification when the filename merely contains - // the heading as a substring. Should be fixed in a future PR (invert these - // tests when fixing). + // -- Special character sanitization in link targets ---------------------- - describe('filename-heading substring matching (known bug)', () => { - it('false-positive: simplifies when filename contains heading as substring', () => { - // BUG: "JavaScript".includes("Java") → true, incorrectly simplifies. - // Expected correct behavior: should NOT simplify (not an exact or - // derived-name match). + describe('special character sanitization', () => { + it('wiki format: strips special chars from link target, preserves display text', () => { + const result = buildHeadingLink({ + ...defaults, + heading: 'Test | If # This ^ Heading : Works', + filename: 'MyNote', + }); + expect(result.link).toBe('[[MyNote#Test If This Heading Works|Test | If # This ^ Heading : Works]]'); + }); + + it('markdown format: strips special chars from URL target', () => { + const result = buildHeadingLink({ + ...defaults, + linkFormat: LinkFormat.MDLINK, + heading: 'Test | If # This ^ Heading : Works', + filename: 'MyNote', + }); + expect(result.link).toBe('[Test | If # This ^ Heading : Works](MyNote#Test%20If%20This%20Heading%20Works)'); + }); + + it('wiki format: strips chars without surrounding whitespace', () => { + const result = buildHeadingLink({ + ...defaults, + heading: 'Test|If#This^Heading:Works, Too', + filename: 'MyNote', + }); + expect(result.link).toBe('[[MyNote#Test If This Heading Works, Too|Test|If#This^Heading:Works, Too]]'); + }); + + it('markdown format: display text preserves original heading', () => { + const result = buildHeadingLink({ + ...defaults, + linkFormat: LinkFormat.MDLINK, + heading: 'Test|If#This^Heading:Works, Too', + filename: 'MyNote', + }); + expect(result.link).toBe('[Test|If#This^Heading:Works, Too](MyNote#Test%20If%20This%20Heading%20Works,%20Too)'); + }); + }); + + // -- compareIgnoreCase: strict equality (no substring matching) ----------- + + describe('filename-heading matching (strict equality)', () => { + it('does not simplify when filename contains heading as substring', () => { const result = buildHeadingLink({ ...defaults, heading: 'Java', filename: 'JavaScript', }); - expect(result.link).toBe('[[JavaScript|Java]]'); - expect(result.isNoteLink).toBe(true); + expect(result.link).toBe('[[JavaScript#Java|Java]]'); + expect(result.isNoteLink).toBe(false); }); - it('does NOT simplify when heading contains filename as substring', () => { - // The includes check is directional: filename.includes(heading) - // "Note".includes("Notebook") → false + it('does not simplify when heading contains filename as substring', () => { const result = buildHeadingLink({ ...defaults, heading: 'Notebook', @@ -222,17 +299,14 @@ describe('buildHeadingLink', () => { expect(result.isNoteLink).toBe(false); }); - it('false-positive: simplifies on space-removed substring match', () => { - // BUG: "SomeThingElse".includes("SomeThing") → true, incorrectly - // simplifies. The intended behavior (SomeThing/Some Thing) works, - // but non-exact substring matches like this are false positives. + it('does not simplify on space-removed substring match', () => { const result = buildHeadingLink({ ...defaults, heading: 'Some Thing', filename: 'SomeThingElse', }); - expect(result.link).toBe('[[SomeThingElse|Some Thing]]'); - expect(result.isNoteLink).toBe(true); + expect(result.link).toBe('[[SomeThingElse#Some Thing|Some Thing]]'); + expect(result.isNoteLink).toBe(false); }); }); @@ -265,18 +339,17 @@ describe('buildHeadingLink', () => { expect(result.isNoteLink).toBe(true); }); - it('empty heading triggers false note-link simplification (known bug)', () => { - // BUG: ''.includes('') is always true in JS, so compareIgnoreCase - // falsely matches any filename. Should produce a heading link with - // an empty fragment instead. Fix alongside compareIgnoreCase in a - // future PR. + it('empty heading produces a heading link with empty fragment (known bug)', () => { + // Empty heading is an edge case from Obsidian's UI. The link target + // is technically malformed (MyNote#) but this matches current behavior. + // Should be addressed in a future PR. const result = buildHeadingLink({ ...defaults, heading: '', filename: 'MyNote', }); - expect(result.link).toBe('[[MyNote|]]'); - expect(result.isNoteLink).toBe(true); + expect(result.link).toBe('[[MyNote#|]]'); + expect(result.isNoteLink).toBe(false); }); it('markdown format URL-encodes spaces in link URL', () => { diff --git a/src/linkBuilder.ts b/src/linkBuilder.ts index e050031..eca6cc8 100644 --- a/src/linkBuilder.ts +++ b/src/linkBuilder.ts @@ -6,6 +6,25 @@ export function encodeMarkdownLinkUrl(url: string): string { return url.replace(/ /g, '%20'); } +// --- Heading Sanitization --- + +/** + * Sanitize heading text for use in link targets. + * + * Obsidian's [[ autocomplete surprisingly strips # | ^ : %% [[ ]] from + * heading link targets rather than URL-encoding them, replacing each + * occurrence (and any surrounding whitespace) with a single space. + * URL-encoding these characters does not work reliably in Obsidian. + * + * See: https://help.obsidian.md/Linking+notes+and+files/Internal+links + */ +export function sanitizeHeadingForLink(heading: string): string { + return heading + .replace(/\s*(?:%%|\[\[|]]|[#|^:])\s*/g, ' ') + .replace(/ {2,}/g, ' ') + .trim(); +} + // --- Heading Link --- export interface BuildHeadingLinkOptions { @@ -41,18 +60,20 @@ export function buildHeadingLink(options: BuildHeadingLinkOptions): BuildHeading displayText = `${filenameOrTitle}${separator}${selectedHeading}`; } - let linkContent = `${filename}#${selectedHeading}`; + const sanitizedHeading = sanitizeHeadingForLink(selectedHeading); + let linkContent = `${filename}#${sanitizedHeading}`; let isNoteLink = false; const compareIgnoreCase = (a: string, b: string): boolean => - a.toLowerCase() === b.toLowerCase() || a.toLowerCase().includes(b.toLowerCase()); + a.toLowerCase() === b.toLowerCase(); // 特殊情况：如果文件名包含标题，则不添加指向标题的 # 部分 // 我自己的情况——会把 SomeThing 给拆成 Some Thing 来做标题，所以也考虑空格替换的部分 if ( - filename === selectedHeading || + selectedHeading && + (filename === selectedHeading || compareIgnoreCase(filename, selectedHeading) || - compareIgnoreCase(filename, selectedHeading.replace(/\s+/g, '')) + compareIgnoreCase(filename, selectedHeading.replace(/\s+/g, ''))) ) { linkContent = filename; isNoteLink = true; @@ -77,7 +98,7 @@ export function buildHeadingLink(options: BuildHeadingLinkOptions): BuildHeading } } else { // Markdown链接格式 - link = `[${displayText}](${encodeMarkdownLinkUrl(`${filename}#${selectedHeading}`)})`; + link = `[${displayText}](${encodeMarkdownLinkUrl(`${filename}#${sanitizedHeading}`)})`; } return { link, isNoteLink };