diff --git a/README.md b/README.md index a99bb3d..50566c5 100644 --- a/README.md +++ b/README.md @@ -10,6 +10,12 @@ suggested edits are written into the Markdown file as Collaborators' comments appear in your note as they write them. +Comments on lists, tasks, headings, and quotes preserve their Markdown prefixes. +For these blocks, select content on one source line at a time; selections across +multiple source lines within a list item are refused. Selections starting in +ordinary prose that span a later list, task, quote, or heading line are also +refused. + ![A comment arrives in a daily note, is read from the hover preview, and gets a reply](docs/comment-thread.gif) ## Suggestions diff --git a/src/critic/anchor-range.ts b/src/critic/anchor-range.ts index a14b14e..c5570f7 100644 --- a/src/critic/anchor-range.ts +++ b/src/critic/anchor-range.ts @@ -5,21 +5,28 @@ export interface AnchorRange { to: number; } -/** Length of the block syntax a line starts with: quote markers, then a - heading marker or a list marker with its optional task box. */ +/** Length of a line's quote/list containers, task box, and heading marker. */ export function structuralPrefixLength(line: string): number { - let length = 0; - let rest = line; - for (;;) { - const quote = /^[\t ]*>[\t ]?/.exec(rest); - if (!quote) break; - length += quote[0].length; - rest = rest.slice(quote[0].length); + let cursor = /^[ \t]*/.exec(line)![0].length; + let hasPrefix = false; + while (cursor < line.length) { + const quote = /^>[ \t]*/.exec(line.slice(cursor)); + const list = /^(?:[-+*]|\d{1,9}[.)])[ \t]+/.exec(line.slice(cursor)); + const marker = quote ?? list; + if (!marker) break; + if (list && /^(?:\*[ \t]*){3,}$|^(?:-[ \t]*){3,}$/.test(line.slice(cursor))) break; + cursor += marker[0].length; + hasPrefix = true; + if (list) { + cursor += /^\[[ xX]\][ \t]+/.exec(line.slice(cursor))?.[0].length ?? 0; + } + } + const heading = /^#{1,6}[ \t]+/.exec(line.slice(cursor)); + if (heading) { + cursor += heading[0].length; + hasPrefix = true; } - const block = - /^[\t ]*(?:#{1,6}[\t ]+|(?:[-+*]|\d+[.)])[\t ]+(?:\[[ xX]\][\t ]+)?)/.exec(rest); - if (block) length += block[0].length; - return length; + return hasPrefix ? cursor : 0; } /** diff --git a/src/editor/comment-draft-range.ts b/src/editor/comment-draft-range.ts new file mode 100644 index 0000000..6547380 --- /dev/null +++ b/src/editor/comment-draft-range.ts @@ -0,0 +1,35 @@ +import { structuralPrefixLength, type AnchorRange } from "../critic/anchor-range"; + +/** Null means the selection cannot safely be wrapped without moving block syntax. */ +export function commentDraftContentRange( + documentText: string, + from: number, + to: number, +): AnchorRange | null { + if (from === to) return { from, to }; + const lineFrom = from === 0 ? 0 : documentText.lastIndexOf("\n", from - 1) + 1; + const newline = documentText.indexOf("\n", from); + const lineTo = newline < 0 ? documentText.length : newline; + const line = documentText.slice(lineFrom, lineTo).replace(/\r$/, ""); + const prefixLength = structuralPrefixLength(line); + if (newline >= 0 && to > newline + 1) { + let laterFrom = newline + 1; + while (laterFrom < to) { + const laterNewline = documentText.indexOf("\n", laterFrom); + const laterTo = laterNewline < 0 ? documentText.length : laterNewline; + if (structuralPrefixLength(documentText.slice(laterFrom, laterTo)) > 0) return null; + if (laterNewline < 0) break; + laterFrom = laterNewline + 1; + } + } + if (!prefixLength) return { from, to }; + // Triple-click includes the line break. Keep review markup before it. + if (to > lineTo + (newline < 0 ? 0 : 1)) return null; + const contentFrom = Math.max(from, lineFrom + prefixLength); + let contentTo = Math.min(to, lineFrom + line.length); + if (/#{1,6}[ \t]+$/.test(line.slice(0, prefixLength))) { + const closing = /[ \t]+#+[ \t]*$/.exec(line); + if (closing) contentTo = Math.min(contentTo, lineFrom + closing.index); + } + return contentFrom < contentTo ? { from: contentFrom, to: contentTo } : null; +} diff --git a/src/main.ts b/src/main.ts index aa53207..115928d 100644 --- a/src/main.ts +++ b/src/main.ts @@ -80,6 +80,7 @@ import { getAddCommentTarget, } from "./editor/hotkeys"; import { buildCommentDraftInsertion } from "./editor/comment-draft-anchor"; +import { commentDraftContentRange } from "./editor/comment-draft-range"; import { findCanvasEditorSurface, findFocusedCanvasEditorSurface, @@ -1402,6 +1403,15 @@ export default class RelayCommentsPlugin return; } const editorText = surface?.editor.getValue(); + if (editorText !== undefined) { + const range = commentDraftContentRange(editorText, from, to); + if (!range) { + new Notice("Select text within one block line to comment on it."); + return; + } + ({ from, to } = range); + selectedText = editorText.slice(from, to); + } const marks = editorText ? parseCriticMarkup(editorText) : []; const nativeHighlights = editorText ? parseNativeHighlights(editorText, marks) @@ -1550,6 +1560,13 @@ export default class RelayCommentsPlugin draft.to, ) : null; + const range = nativeHighlight + ? null + : commentDraftContentRange(documentText, draft.from, draft.to); + if (!nativeHighlight && !range) { + new Notice("The commented text changed; select it again within one block line."); + return; + } this.commentDraft = null; this.clearCommentDraftAnchor(); if (nativeHighlight) { @@ -1569,13 +1586,13 @@ export default class RelayCommentsPlugin undefined, "relay-comments", ); - } else { - const currentText = documentText.slice(draft.from, draft.to); + } else if (range) { + const currentText = documentText.slice(range.from, range.to); const insertion = buildCommentDraftInsertion(currentText, commentMarkup); refreshedEditor.replaceRange( insertion, - refreshedEditor.offsetToPos(draft.from), - refreshedEditor.offsetToPos(draft.to), + refreshedEditor.offsetToPos(range.from), + refreshedEditor.offsetToPos(range.to), "relay-comments", ); } diff --git a/tests/unit/critic/anchor-range.test.ts b/tests/unit/critic/anchor-range.test.ts index 1cf7e7c..923ba68 100644 --- a/tests/unit/critic/anchor-range.test.ts +++ b/tests/unit/critic/anchor-range.test.ts @@ -5,6 +5,18 @@ import { } from "src/critic/anchor-range"; describe("structuralPrefixLength", () => { + it.each([ + ["- > quoted", 4], + ["> - [ ] task", 8], + ["\t\t- [ ] task", 8], + ["> > ### heading", 8], + ["* * *", 0], + ["- - -", 0], + ["1234567890. prose", 0], + ])("uses shared block syntax rules for %j", (line, length) => { + expect(structuralPrefixLength(line)).toBe(length); + }); + it("measures heading, list, task, and quote syntax", () => { expect(structuralPrefixLength("## Must fix")).toBe(3); expect(structuralPrefixLength("- item")).toBe(2); diff --git a/tests/unit/editor/comment-draft-range.test.ts b/tests/unit/editor/comment-draft-range.test.ts new file mode 100644 index 0000000..419bf97 --- /dev/null +++ b/tests/unit/editor/comment-draft-range.test.ts @@ -0,0 +1,181 @@ +import { ChangeSet, EditorSelection, EditorState } from "@codemirror/state"; +import { describe, expect, it } from "@jest/globals"; +import { parseCriticMarkup } from "src/critic/parse"; +import { narrowAnchorRange } from "src/critic/anchor-range"; +import { buildCommentDraftInsertion, mapCommentDraftAnchor } from "src/editor/comment-draft-anchor"; +import { commentDraftContentRange } from "src/editor/comment-draft-range"; +import { findNativeHighlightForSelection, parseNativeHighlights } from "src/markdown/highlights"; + +const comment = '{{authorId="reviewer" author="Reviewer">>test<<}}'; + +function insertComment(documentText: string, from: number, to: number): string { + const range = commentDraftContentRange(documentText, from, to); + if (!range) throw new Error("Unsupported comment range"); + return documentText.slice(0, range.from) + + buildCommentDraftInsertion(documentText.slice(range.from, range.to), comment) + + documentText.slice(range.to); +} + +describe("block comment content ranges", () => { + it.each([ + "1. ", "12) ", "- ", "* ", "+ ", + "- [ ] ", "- [x] ", "- [X] ", "2. [ ] ", + "# ", "## ", "###### ", "> ", ">> ", "> > ", + " - ", " - ", "\t- ", "> - [ ] ", "- > ", "> ## ", "1.\t", + ])("preserves the %j prefix and following block", (prefix) => { + const before = "Earlier text.\n"; + const line = prefix + "**Selected text.**"; + const after = "\n3. Following item.\n\nPlain paragraph."; + const text = before + line + after; + for (const trailingNewline of [0, 1]) { + const result = insertComment(text, before.length, before.length + line.length + trailingNewline); + expect(result).toBe(before + prefix + "{==**Selected text.**==}" + comment + after); + const marks = parseCriticMarkup(result); + expect(marks.map((mark) => mark.type)).toEqual(["highlight", "comment"]); + expect(marks.every((mark) => mark.valid)).toBe(true); + expect(marks[0].raw).toBe("{==**Selected text.**==}"); + expect(marks[1].raw).toBe(comment); + } + }); + + it("keeps a heading's closing hashes and line break outside the comment", () => { + const text = "## Heading ##\nNext paragraph"; + expect(insertComment(text, 0, text.indexOf("\n") + 1)).toBe( + "## {==Heading==}" + comment + " ##\nNext paragraph", + ); + }); + + it("preserves CRLF after a whole heading selection", () => { + const text = "# Heading\r\nNext paragraph"; + expect(insertComment(text, 0, text.indexOf("\n") + 1)).toBe( + "# {==Heading==}" + comment + "\r\nNext paragraph", + ); + }); + + it("adjusts forward and backward CodeMirror selections identically", () => { + const text = "# Heading\nNext paragraph"; + for (const [anchor, head] of [[0, 10], [10, 0]]) { + const state = EditorState.create({ doc: text, selection: EditorSelection.single(anchor, head) }); + const { from, to } = state.selection.main; + expect(state.doc.sliceString(from, to)).toBe("# Heading\n"); + expect(commentDraftContentRange(text, from, to)).toEqual({ from: 2, to: 9 }); + expect(state.selection.main.anchor).toBe(anchor); + } + }); + + it.each([0, 1, 2, 3, 4, 5])("moves a start inside a task prefix (%i) after the box", (from) => { + expect(insertComment("- [ ] Task", from, 10)).toBe("- [ ] {==Task==}" + comment); + }); + + it("leaves a partial content selection at its original offsets", () => { + expect(insertComment("- one two three", 6, 9)).toBe("- one {==two==}" + comment + " three"); + }); + + it.each(["plain text", "a\nparagraph", "#hashtag", "####### text", "1234567890. text", "-word", "\\# escaped", "---", "* * *"])( + "leaves ordinary or unsupported syntax unchanged: %j", (text) => { + expect(insertComment(text, 0, text.length)).toBe("{==" + text + "==}" + comment); + }, + ); + + it.each(["```md\n", "~~~\n", "````\n```\n"])("matches upstream narrowing for a code-text prefix (%j)", (fence) => { + const text = fence + "# literal code\n"; + expect(commentDraftContentRange(text, fence.length, text.length)).toEqual( + narrowAnchorRange(text, fence.length, text.length), + ); + expect(insertComment(text, fence.length, text.length)).toBe(fence + "# {==literal code==}" + comment + "\n"); + }); + + it("adjusts a block after a closed fence", () => { + const before = "```md\n# code\n```\n"; + const text = before + "# Heading"; + expect(insertComment(text, before.length, text.length)).toBe(before + "# {==Heading==}" + comment); + }); + + it.each(["1. first\n2. second", "- first\n continued", "# heading\nparagraph", "> first\n> second", "- [ ] task\n- next"])( + "refuses a prefixed selection crossing lines: %j", (text) => { + expect(commentDraftContentRange(text, 0, text.length)).toBeNull(); + }, + ); + + it.each(["- ", "# ", "> ", "- [ ] "])("refuses a prefix-only selection: %j", (text) => { + expect(commentDraftContentRange(text, 0, text.length)).toBeNull(); + }); + + it.each(["1. item", "- item", "- [ ] task", "# heading", "> quote", "> - item"])( + "refuses a selection starting before a later block: %j", (line) => { + const text = "plain text\n" + line; + expect(commentDraftContentRange(text, 0, text.length)).toBeNull(); + expect(commentDraftContentRange(text, 10, text.length)).toBeNull(); + }, + ); + + it("does not include a block starting exactly at the selection end", () => { + const text = "plain text\n- item"; + expect(commentDraftContentRange(text, 0, 11)).toEqual({ from: 0, to: 11 }); + }); + + it("refuses even a partial later prefix using the full source line", () => { + const text = "plain text\n- item"; + expect(commentDraftContentRange(text, 0, 12)).toBeNull(); + expect(commentDraftContentRange("\n- item", 0, 2)).toBeNull(); + }); + + it.each([" - ambiguous", "\t- ambiguous", " # ambiguous", "> - ambiguous", "> \t# ambiguous"])( + "keeps indentation outside the anchor without guessing the block type: %j", (text) => { + expect(commentDraftContentRange(text, 0, text.length)).toEqual({ from: text.indexOf("ambiguous"), to: text.length }); + }, + ); + + it.each(["\t- ", " - ", "\t\t- [ ] ", "> ", "> \t- "])( + "allows both whole-line and content-only nested selections: %j", (prefix) => { + const before = "- parent\n"; + const text = before + prefix + "b c"; + const contentFrom = text.indexOf("b c"); + for (const from of [before.length, contentFrom]) { + const range = commentDraftContentRange(text, from, text.length)!; + expect(range).toEqual({ from: contentFrom, to: text.length }); + expect(narrowAnchorRange(text, range.from, range.to)).toEqual(range); + expect(insertComment(text, from, text.length)).toBe(before + prefix + "{==b c==}" + comment); + } + }, + ); + + it.each([ + ["> ```\n", "> - literal code"], + ["- ```\n", " # literal code"], + [" ~~~\n", " - literal code"], + ])("keeps nested code containers outside the mark: %j", (before, line) => { + const text = before + line; + const contentFrom = text.indexOf("literal code"); + expect(commentDraftContentRange(text, before.length, text.length)).toEqual({ from: contentFrom, to: text.length }); + expect(insertComment(text, before.length, text.length)).toBe(text.slice(0, contentFrom) + "{==literal code==}" + comment); + }); + + it("adjusts a block after a closed nested fence", () => { + const before = "> ```\n> code\n> ```\n"; + const text = before + "# Heading"; + expect(insertComment(text, before.length, text.length)).toBe(before + "# {==Heading==}" + comment); + }); + + it("keeps empty selections at their surviving insertion point", () => { + expect(insertComment("# Heading", 0, 0)).toBe(comment + "# Heading"); + }); + + it("finds a complete native highlight after removing the block prefix", () => { + const text = "# ==Heading==\nNext paragraph"; + const range = commentDraftContentRange(text, 0, 14)!; + const highlight = findNativeHighlightForSelection(parseNativeHighlights(text, []), range.from, range.to); + expect(highlight?.raw).toBe("==Heading=="); + const result = text.slice(0, highlight!.to) + comment + text.slice(highlight!.to); + expect(result).toBe("# ==Heading==" + comment + "\nNext paragraph"); + expect(parseCriticMarkup(result).map((mark) => mark.type)).toEqual(["comment"]); + }); + + it("recomputes the content boundary after an entire mapped range is rewritten", () => { + const text = "old text"; + const changes = ChangeSet.of({ from: 0, to: text.length, insert: "# New heading\n" }, text.length); + const draft = mapCommentDraftAnchor({ id: 7, filePath: "note.md", from: 0, to: text.length }, changes); + const updatedText = changes.apply(EditorState.create({ doc: text }).doc).toString(); + expect(insertComment(updatedText, draft.from, draft.to)).toBe("# {==New heading==}" + comment + "\n"); + }); +});