diff --git a/internal/markdown/markdown_test.go b/internal/markdown/markdown_test.go index 1a26356c..a124ce62 100644 --- a/internal/markdown/markdown_test.go +++ b/internal/markdown/markdown_test.go @@ -768,6 +768,7 @@ func TestExtractTagsMemosTagV1(t *testing.T) { {name: "email inside emphasis", content: "_foo@example.com #tag_", expected: []string{"tag"}}, {name: "raw HTML syntax", content: "#text", expected: []string{"text"}}, {name: "inline math", content: "$#math$ #plain", expected: []string{"plain"}}, + {name: "currency dollars leave tags visible", content: "$20,000 #budget and $30,000", expected: []string{"budget"}}, {name: "inline math exact closing run", content: "$#one$$ #two$ #plain", expected: []string{"plain"}}, {name: "inline math does not reopen within a dollar run", content: "$$#math$ #plain", expected: []string{"math", "plain"}}, {name: "inline math after escaped dollar", content: "\\$$#math$ #plain", expected: []string{"plain"}}, diff --git a/internal/markdown/parser/math.go b/internal/markdown/parser/math.go index b13d0e58..2af3ee2c 100644 --- a/internal/markdown/parser/math.go +++ b/internal/markdown/parser/math.go @@ -2,6 +2,8 @@ package parser import ( "bytes" + "unicode" + "unicode/utf8" gast "github.com/yuin/goldmark/ast" "github.com/yuin/goldmark/parser" @@ -25,7 +27,8 @@ func (*inlineMathParser) Trigger() []byte { func (*inlineMathParser) Parse(_ gast.Node, reader text.Reader, _ parser.Context) gast.Node { line, _ := reader.PeekLine() openingLength := dollarRunLength(line) - if openingLength == 0 || hasUnescapedPrecedingDollar(reader) { + if openingLength == 0 || hasUnescapedPrecedingDollar(reader) || + (openingLength == 1 && !isValidSingleDollarOpening(line[openingLength:])) { return nil } @@ -47,6 +50,10 @@ func (*inlineMathParser) Parse(_ gast.Node, reader text.Reader, _ parser.Context } closingLength := dollarRunLength(line[pos:]) if closingLength == openingLength { + if openingLength == 1 && !isValidSingleDollarClosing(line, pos) { + reader.SetPosition(savedLine, savedPosition) + return nil + } end := pos + closingLength source = append(source, line[:end]...) reader.Advance(end) @@ -60,6 +67,30 @@ func (*inlineMathParser) Parse(_ gast.Node, reader text.Reader, _ parser.Context } } +func isValidSingleDollarOpening(source []byte) bool { + if len(source) == 0 { + return false + } + r, _ := utf8.DecodeRune(source) + return !unicode.IsSpace(r) +} + +func isValidSingleDollarClosing(line []byte, position int) bool { + if position == 0 { + return false + } + preceding, _ := utf8.DecodeLastRune(line[:position]) + if unicode.IsSpace(preceding) { + return false + } + + position++ + if position >= len(line) { + return true + } + return line[position] < '0' || line[position] > '9' +} + // hasUnescapedPrecedingDollar prevents retrying within one dollar run while // allowing a new run after an escaped dollar. func hasUnescapedPrecedingDollar(reader text.Reader) bool { diff --git a/internal/markdown/parser/math_test.go b/internal/markdown/parser/math_test.go new file mode 100644 index 00000000..63f279b0 --- /dev/null +++ b/internal/markdown/parser/math_test.go @@ -0,0 +1,80 @@ +package parser + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/yuin/goldmark" + gast "github.com/yuin/goldmark/ast" + gparser "github.com/yuin/goldmark/parser" + "github.com/yuin/goldmark/text" + "github.com/yuin/goldmark/util" + + mast "github.com/usememos/memos/internal/markdown/ast" +) + +func TestInlineMathParserDollarBoundaries(t *testing.T) { + tests := []struct { + name string + source string + expected []string + }{ + {name: "valid single dollar math", source: "$x$", expected: []string{"$x$"}}, + {name: "currency stays literal", source: "$20,000 and $30,000"}, + { + name: "multiline currency list stays literal", + source: `list of 10 houses +$140,000 max - buffer of ~10k +diy, add amount to down payment +no appliances covered +less than $1,000, client responsibilities +over $1,000 - owner responsibility +vacant is good to rent +for sale by owner +path to ownership -$50 +rent insurance - my of`, + }, + {name: "opening followed by whitespace", source: "$ x$"}, + {name: "opening followed by tab", source: "$\tx$"}, + {name: "opening followed by line ending", source: "$\nx$"}, + {name: "opening followed by Unicode whitespace", source: "$\u00a0x$"}, + {name: "opening followed by next line character", source: "$\u0085x$"}, + {name: "closing preceded by whitespace", source: "$x $"}, + {name: "closing preceded by tab", source: "$x\t$"}, + {name: "closing followed by digit", source: "$x$2"}, + {name: "retry after closer preceded by whitespace", source: "$x $ and $y$", expected: []string{"$y$"}}, + {name: "retry after closer followed by digit", source: "$x$2 and $y$", expected: []string{"$y$"}}, + {name: "retry after currency dollars", source: "$20 and $30 then $x$", expected: []string{"$x$"}}, + {name: "closing followed by Unicode digit", source: "$x$٢", expected: []string{"$x$"}}, + {name: "valid multiline math", source: "$x +\ny$", expected: []string{"$x +\ny$"}}, + {name: "multiline closing at line start", source: "$x\n$"}, + {name: "exact closing run", source: "$x$$ y$", expected: []string{"$x$$ y$"}}, + {name: "opener after escaped dollar", source: `\$$x$`, expected: []string{"$x$"}}, + {name: "multi-dollar boundaries unchanged", source: "$$ x $$2", expected: []string{"$$ x $$"}}, + } + + md := goldmark.New( + goldmark.WithParserOptions( + gparser.WithInlineParsers( + util.Prioritized(NewInlineMathParser(), 150), + ), + ), + ) + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + document := md.Parser().Parse(text.NewReader([]byte(test.source))) + var actual []string + err := gast.Walk(document, func(node gast.Node, entering bool) (gast.WalkStatus, error) { + if entering { + if mathNode, ok := node.(*mast.InlineMathNode); ok { + actual = append(actual, string(mathNode.Source)) + } + } + return gast.WalkContinue, nil + }) + require.NoError(t, err) + assert.Equal(t, test.expected, actual) + }) + } +} diff --git a/web/src/components/MemoContent/MathMarkdownRenderer.tsx b/web/src/components/MemoContent/MathMarkdownRenderer.tsx index 1afdbd23..4400a71d 100644 --- a/web/src/components/MemoContent/MathMarkdownRenderer.tsx +++ b/web/src/components/MemoContent/MathMarkdownRenderer.tsx @@ -1,12 +1,12 @@ import "katex/dist/katex.min.css"; import rehypeKatex from "rehype-katex"; -import remarkMath from "remark-math"; +import { remarkCurrencySafeMath } from "@/utils/remark-plugins/remark-currency-safe-math"; import { MemoMarkdownRendererCore, type MemoMarkdownRendererProps } from "./MemoMarkdownRenderer"; const MathMarkdownRenderer = (props: MemoMarkdownRendererProps) => ( ); diff --git a/web/src/utils/memo-markdown-extension.ts b/web/src/utils/memo-markdown-extension.ts index 484cd17a..02b214db 100644 --- a/web/src/utils/memo-markdown-extension.ts +++ b/web/src/utils/memo-markdown-extension.ts @@ -14,6 +14,14 @@ function isClosingMathFence(line: { text: string; pos: number; skipSpace(from: n return end - line.pos >= openingSize && line.skipSpace(end) === line.text.length; } +function isWhitespace(character: number): boolean { + return character < 0 || /\p{White_Space}/u.test(String.fromCharCode(character)); +} + +function isDigit(character: number): boolean { + return character >= 0x30 && character <= 0x39; +} + const mathExtension: MarkdownConfig = { defineNodes: ["InlineMath", "MathMark", { name: "BlockMath", block: true }], parseBlock: [ @@ -51,6 +59,8 @@ const mathExtension: MarkdownConfig = { let openingEnd = position + 1; while (context.char(openingEnd) === 0x24) openingEnd++; const openingSize = openingEnd - position; + if (openingSize === 1 && isWhitespace(context.char(openingEnd))) return openingEnd; + for (let cursor = openingEnd; cursor < context.end; ) { if (context.char(cursor) !== 0x24) { cursor++; @@ -58,7 +68,9 @@ const mathExtension: MarkdownConfig = { } let closingEnd = cursor + 1; while (context.char(closingEnd) === 0x24) closingEnd++; - if (closingEnd - cursor === openingSize) { + const closingSize = closingEnd - cursor; + if (closingSize === openingSize) { + if (openingSize === 1 && (isWhitespace(context.char(cursor - 1)) || isDigit(context.char(closingEnd)))) return openingEnd; return context.addElement( context.elt("InlineMath", position, closingEnd, [ context.elt("MathMark", position, openingEnd), diff --git a/web/src/utils/remark-plugins/remark-currency-safe-math.ts b/web/src/utils/remark-plugins/remark-currency-safe-math.ts new file mode 100644 index 00000000..dc165055 --- /dev/null +++ b/web/src/utils/remark-plugins/remark-currency-safe-math.ts @@ -0,0 +1,85 @@ +import remarkMath from "remark-math"; + +type MicromarkCode = number | null; +type MicromarkState = (code: MicromarkCode) => MicromarkState | undefined; + +interface MicromarkEffects { + consume: (code: number) => undefined; + [key: string]: unknown; +} + +type MathTokenizer = (this: unknown, effects: MicromarkEffects, ok: MicromarkState, nok: MicromarkState) => MicromarkState; + +interface MathTextConstruct { + tokenize: MathTokenizer; +} + +interface MathSyntaxExtension { + text?: Record; +} + +interface RemarkMathData { + micromarkExtensions?: MathSyntaxExtension[]; +} + +const DOLLAR_SIGN = 0x24; +const ASCII_ZERO = 0x30; +const ASCII_NINE = 0x39; + +const isWhitespace = (code: MicromarkCode): boolean => code !== null && (code < 0 || /\p{White_Space}/u.test(String.fromCodePoint(code))); + +const isAsciiDigit = (code: MicromarkCode): boolean => code !== null && code >= ASCII_ZERO && code <= ASCII_NINE; + +/** + * Keep remark-math's exact-run and escape behavior while applying Pandoc-style + * ambiguity guards to single-dollar inline math: no whitespace after an opener + * or before a closer, and no ASCII digit immediately after a closer. A failed + * guarded `ok` rolls the attempt back so a later dollar can open valid math. + */ +const withCurrencySafeBoundaries = (tokenize: MathTokenizer): MathTokenizer => + function currencySafeMathText(effects, ok, nok) { + let openingSize = 0; + let readingOpening = true; + let afterOpening: MicromarkCode = null; + let previousCode: MicromarkCode = null; + let beforePreviousCode: MicromarkCode = null; + const trackedEffects = Object.create(effects) as MicromarkEffects; + trackedEffects.consume = (code) => { + if (readingOpening && code === DOLLAR_SIGN) { + openingSize++; + } else if (readingOpening) { + readingOpening = false; + afterOpening = code; + } + + beforePreviousCode = previousCode; + previousCode = code; + return effects.consume(code); + }; + + const guardedOk: MicromarkState = (nextCode) => { + if (openingSize !== 1) return ok(nextCode); + + if (isWhitespace(afterOpening) || isWhitespace(beforePreviousCode) || isAsciiDigit(nextCode)) { + return nok(nextCode); + } + + return ok(nextCode); + }; + + return tokenize.call(this, trackedEffects, guardedOk, nok); + }; + +/** remark-math with currency-safe boundaries for single-dollar inline math. */ +export function remarkCurrencySafeMath(this: unknown): void { + remarkMath.call(this); + + const data = (this as { data: () => RemarkMathData }).data(); + const mathExtension = data.micromarkExtensions?.at(-1); + const mathText = mathExtension?.text?.[DOLLAR_SIGN]; + if (!mathText || Array.isArray(mathText)) { + throw new Error("remark-math did not register its inline math tokenizer"); + } + + mathText.tokenize = withCurrencySafeBoundaries(mathText.tokenize); +} diff --git a/web/tests/memo-content-lazy-renderers.test.tsx b/web/tests/memo-content-lazy-renderers.test.tsx index e44382e3..e81bf70b 100644 --- a/web/tests/memo-content-lazy-renderers.test.tsx +++ b/web/tests/memo-content-lazy-renderers.test.tsx @@ -1,9 +1,21 @@ import { render, waitFor } from "@testing-library/react"; import { describe, expect, it } from "vitest"; import { highlightCode } from "@/components/MemoContent/highlight"; +import MathMarkdownRenderer from "@/components/MemoContent/MathMarkdownRenderer"; import { MemoMarkdownRenderer } from "@/components/MemoContent/MemoMarkdownRenderer"; import { hasMathSyntax } from "@/components/MemoContent/math"; +const ISSUE_CURRENCY_CONTENT = `list of 10 houses +$140,000 max - buffer of ~10k +diy, add amount to down payment +no appliances covered +less than $1,000, client responsibilities +over $1,000 - owner responsibility +vacant is good to rent +for sale by owner +path to ownership -$50 +rent insurance - my of`; + describe("memo content lazy renderers", () => { it("detects math in prose but ignores escaped dollars and code", () => { expect(hasMathSyntax("Inline $L$ formula")).toBe(true); @@ -21,6 +33,62 @@ describe("memo content lazy renderers", () => { await waitFor(() => expect(container.querySelector(".katex")).not.toBeNull()); }); + it("keeps currency dollars literal across and within lines", () => { + const content = `${ISSUE_CURRENCY_CONTENT}\n\n$20,000 and $30,000`; + const { container } = render(); + + expect(container.querySelector(".katex")).toBeNull(); + expect(container.querySelectorAll("br")).toHaveLength(9); + for (const price of ["$140,000", "$1,000", "$50", "$20,000", "$30,000"]) { + expect(container.textContent).toContain(price); + } + }); + + it("retries later dollars after rejecting currency and preserves display math", () => { + const inline = render(); + expect(inline.container.textContent).toContain("A $5 amount and "); + expect(inline.container.querySelectorAll(".katex")).toHaveLength(1); + + const multiline = render(); + expect(multiline.container.querySelector(".katex")).not.toBeNull(); + + const display = render(); + expect(display.container.querySelector(".katex-display .katex")).not.toBeNull(); + }); + + it.each([ + "$ x$", + "$x $", + "$x$2", + "$\u00a0x$", + "$x\u00a0$", + "$\u0085x$", + "$x\u0085$", + ])("keeps invalid single-dollar boundaries literal: %s", (content) => { + const { container } = render(); + + expect(container.querySelector(".katex")).toBeNull(); + expect(container.textContent).toBe(content); + }); + + it.each([ + ["$x $ and $y$", "$x $ and "], + ["$x$2 and $y$", "$x$2 and "], + ["$20 and $30 then $x$", "$20 and $30 then "], + ])("abandons an invalid closer before parsing later math: %s", (content, literalPrefix) => { + const { container } = render(); + + expect(container.textContent).toContain(literalPrefix); + expect(container.querySelectorAll(".katex")).toHaveLength(1); + }); + + it("renders escaped currency without the Markdown escape", () => { + const { container } = render(); + + expect(container.querySelector(".katex")).toBeNull(); + expect(container.textContent).toBe("Price is $20"); + }); + it("escapes plain code and highlights common languages", async () => { expect(await highlightCode('', "")).toBe( "<script data-test="x">&</script>", diff --git a/web/tests/memo-content-security.test.tsx b/web/tests/memo-content-security.test.tsx index 455ad2ba..7fd059dc 100644 --- a/web/tests/memo-content-security.test.tsx +++ b/web/tests/memo-content-security.test.tsx @@ -5,9 +5,9 @@ import rehypeKatex from "rehype-katex"; import rehypeRaw from "rehype-raw"; import rehypeSanitize from "rehype-sanitize"; import remarkGfm from "remark-gfm"; -import remarkMath from "remark-math"; import { describe, expect, it } from "vitest"; -import { SANITIZE_SCHEMA, isTrustedIframeSrc } from "@/components/MemoContent/constants"; +import { isTrustedIframeSrc, SANITIZE_SCHEMA } from "@/components/MemoContent/constants"; +import { remarkCurrencySafeMath } from "@/utils/remark-plugins/remark-currency-safe-math"; type IframeProps = React.ComponentProps<"iframe">; @@ -21,7 +21,7 @@ const TrustedIframe = (props: IframeProps) => { const renderMemoContent = (content: string): string => renderToStaticMarkup( diff --git a/web/tests/memo-markdown-extension.test.ts b/web/tests/memo-markdown-extension.test.ts new file mode 100644 index 00000000..cae56aca --- /dev/null +++ b/web/tests/memo-markdown-extension.test.ts @@ -0,0 +1,65 @@ +import { markdown } from "@codemirror/lang-markdown"; +import { EditorState } from "@codemirror/state"; +import { parser as markdownParser } from "@lezer/markdown"; +import { describe, expect, it } from "vitest"; +import { findMarkdownTagMatches } from "@/components/MemoEditor/Editor/markdownTagRanges"; +import { memoMarkdownExtensions } from "@/utils/memo-markdown-extension"; + +const sourceParser = markdownParser.configure(memoMarkdownExtensions); + +function inlineMath(source: string): string[] { + const ranges: string[] = []; + sourceParser.parse(source).iterate({ + enter(node) { + if (node.name === "InlineMath") ranges.push(source.slice(node.from, node.to)); + }, + }); + return ranges; +} + +describe("memo Markdown math", () => { + it("keeps the issue's multiline currency list as ordinary text", () => { + const source = `list of 10 houses +$140,000 max - buffer of ~10k +diy, add amount to down payment +no appliances covered +less than $1,000, client responsibilities +over $1,000 - owner responsibility +vacant is good to rent +for sale by owner +path to ownership -$50 +rent insurance - my of`; + + expect(inlineMath(source)).toEqual([]); + }); + + it("keeps two same-line currency amounts as ordinary text", () => { + expect(inlineMath("$20,000 and $30,000")).toEqual([]); + + const source = "$20,000 #budget and $30,000"; + const state = EditorState.create({ doc: source, extensions: [markdown({ extensions: memoMarkdownExtensions })] }); + expect(findMarkdownTagMatches(state, 0, source.length).map((match) => match.value)).toEqual(["budget"]); + }); + + it("applies Pandoc-style boundaries to single-dollar inline math", () => { + expect(inlineMath("$ x$ and $x $ and $x$2")).toEqual([]); + expect(inlineMath("$\u0085x$ and $x\u0085$")).toEqual([]); + expect(inlineMath("$x$, then $y$")).toEqual(["$x$", "$y$"]); + }); + + it.each([ + ["$x $ and $y$", ["$y$"]], + ["$x$2 and $y$", ["$y$"]], + ["$20 and $30 then $x$", ["$x$"]], + ])("retries after an invalid exact-size closer in %s", (source, expected) => { + expect(inlineMath(source)).toEqual(expected); + }); + + it("keeps valid and multi-dollar math opaque", () => { + const source = "$x$ and $#hidden$ and $$#also-hidden$$, then #visible"; + const state = EditorState.create({ doc: source, extensions: [markdown({ extensions: memoMarkdownExtensions })] }); + + expect(inlineMath(source)).toEqual(["$x$", "$#hidden$", "$$#also-hidden$$"]); + expect(findMarkdownTagMatches(state, 0, source.length).map((match) => match.value)).toEqual(["visible"]); + }); +}); diff --git a/web/tests/remark-tag.test.tsx b/web/tests/remark-tag.test.tsx index 0ba09a0d..ed372d85 100644 --- a/web/tests/remark-tag.test.tsx +++ b/web/tests/remark-tag.test.tsx @@ -2,12 +2,14 @@ import { renderToStaticMarkup } from "react-dom/server"; import ReactMarkdown from "react-markdown"; import remarkBreaks from "remark-breaks"; import remarkGfm from "remark-gfm"; -import remarkMath from "remark-math"; import { describe, expect, it } from "vitest"; +import { remarkCurrencySafeMath } from "@/utils/remark-plugins/remark-currency-safe-math"; import { remarkMemoSyntax } from "@/utils/remark-plugins/remark-tag"; const renderMarkdown = (content: string): string => - renderToStaticMarkup({content}); + renderToStaticMarkup( + {content}, + ); const renderMarkdownWithoutMath = (content: string): string => renderToStaticMarkup({content}); @@ -633,6 +635,13 @@ describe("remarkMemoSyntax", () => { expect(html).toContain('data-tag="ok"'); }); + it("keeps tags between currency dollars visible", () => { + const html = renderMarkdown("$20,000 #budget and $30,000"); + + expect(html).toContain('data-tag="budget"'); + expect(html).not.toContain("math-inline"); + }); + it("keeps math opaque while the math renderer is loading", () => { const html = renderMarkdownWithoutMath("$#inline$ and $$#mismatch$ plus #ok\n\n$$\n#block\n$$");