Make check for code fences more robust per spec

primer · Jul 6, 2023 · d2a6ce0 · d2a6ce0
1 parent b456e32
commit d2a6ce0
Show file tree

Hide file tree

Showing 2 changed files with 86 additions and 15 deletions.
diff --git a/src/drafts/MarkdownViewer/MarkdownViewer.test.tsx b/src/drafts/MarkdownViewer/MarkdownViewer.test.tsx
@@ -51,6 +51,32 @@ text before list
 - [x] item 1
 - [ ] item 2
 
+text after list`
+    const hierarchyBeforeTaskListNoItemsCheckedTildes = `
+text before list
+
+~~~[tasklist]
+- [ ] item A
+- [ ] item B
+\`\`\`
+~~~~~~
+
+- [ ] item 1
+- [ ] item 2
+
+text after list`
+    const hierarchyBeforeTaskListOneItemCheckedTildes = `
+text before list
+
+~~~[tasklist]
+- [ ] item A
+- [ ] item B
+\`\`\`
+~~~~~~
+
+- [x] item 1
+- [ ] item 2
+
 text after list`
 
     it('enables checklists by default', () => {
@@ -59,7 +85,7 @@ text after list`
           dangerousRenderedHTML={htmlObject}
           markdownValue={noItemsCheckedMarkdown}
           onChange={jest.fn()}
-        />,
+        />
       )
       const items = getAllByRole('checkbox')
       for (const item of items) expect(item).not.toBeDisabled()
@@ -72,7 +98,7 @@ text after list`
           markdownValue={noItemsCheckedMarkdown}
           onChange={jest.fn()}
           disabled
-        />,
+        />
       )
       const items = getAllByRole('checkbox')
       for (const item of items) expect(item).toBeDisabled()
@@ -92,7 +118,7 @@ text after list`
           markdownValue={noItemsCheckedMarkdown}
           onChange={onChangeMock}
           disabled
-        />,
+        />
       )
       const items = getAllByRole('checkbox')
       fireEvent.change(items[0])
@@ -107,13 +133,28 @@ text after list`
           markdownValue={hierarchyBeforeTaskListNoItemsChecked}
           onChange={onChangeMock}
           disabled
-        />
+        />,
       )
       const items = getAllByRole('checkbox')
       fireEvent.change(items[0])
       await waitFor(() => expect(onChangeMock).toHaveBeenCalledWith(hierarchyBeforeTaskListOneItemChecked))
     })
 
+    it('calls `onChange` with the updated Markdown when a task is checked and hierarchy is present with tildes', async () => {
+      const onChangeMock = jest.fn()
+      const {getAllByRole} = render(
+        <MarkdownViewer
+          dangerousRenderedHTML={htmlObject}
+          markdownValue={hierarchyBeforeTaskListNoItemsCheckedTildes}
+          onChange={onChangeMock}
+          disabled
+        />,
+      )
+      const items = getAllByRole('checkbox')
+      fireEvent.change(items[0])
+      await waitFor(() => expect(onChangeMock).toHaveBeenCalledWith(hierarchyBeforeTaskListOneItemCheckedTildes))
+    })
+
     it('calls `onChange` with the updated Markdown when a task is unchecked', async () => {
       const onChangeMock = jest.fn()
       const {getAllByRole} = render(
@@ -122,7 +163,22 @@ text after list`
           markdownValue={firstItemCheckedMarkdown}
           onChange={onChangeMock}
           disabled
-        />,
+        />
+      )
+      const items = getAllByRole('checkbox')
+      fireEvent.change(items[0])
+      await waitFor(() => expect(onChangeMock).toHaveBeenCalledWith(noItemsCheckedMarkdown))
+    })
+
+    it('calls `onChange` with the updated Markdown when a task is unchecked', async () => {
+      const onChangeMock = jest.fn()
+      const {getAllByRole} = render(
+        <MarkdownViewer
+          dangerousRenderedHTML={htmlObject}
+          markdownValue={firstItemCheckedMarkdown}
+          onChange={onChangeMock}
+          disabled
+        />
       )
       const items = getAllByRole('checkbox')
       fireEvent.change(items[0])
@@ -174,7 +230,7 @@ text after list`
 
       const {getByRole} = render(
         // eslint-disable-next-line github/unescaped-html-literal
-        <MarkdownViewer dangerousRenderedHTML={{__html: '<a href="https://example.com">link</a>'}} openLinksInNewTab />,
+        <MarkdownViewer dangerousRenderedHTML={{__html: '<a href="https://example.com">link</a>'}} openLinksInNewTab />
       )
       const link = getByRole('link') as HTMLAnchorElement
       await user.click(link)
@@ -190,7 +246,7 @@ text after list`
           // eslint-disable-next-line github/unescaped-html-literal
           dangerousRenderedHTML={{__html: '<a href="https://example.com">link</a>'}}
           onLinkClick={onLinkClick}
-        />,
+        />
       )
       const link = getByRole('link') as HTMLAnchorElement
       await user.click(link)
@@ -199,8 +255,8 @@ text after list`
       expect(spy).toHaveBeenCalledTimes(1)
       expect(spy).toHaveBeenCalledWith(
         expect.objectContaining({
-          message: 'Not implemented: navigation (except hash changes)',
-        }),
+          message: 'Not implemented: navigation (except hash changes)'
+        })
       )
       spy.mockRestore()
     })

diff --git a/src/drafts/MarkdownViewer/_useListInteraction.ts b/src/drafts/MarkdownViewer/_useListInteraction.ts
@@ -4,7 +4,16 @@ import {ListItem, listItemToString, parseListItem} from '../MarkdownEditor/_useL
 
 type TaskListItem = ListItem & {taskBox: '[ ]' | '[x]'}
 
-const isCodeBlockDelimiter = (line: string) => line.trimStart().startsWith('```')
+// Make check for code fences more robust per spec: https://github.github.com/gfm/#fenced-code-blocks
+const parseCodeFenceBegin = (line: string) => {
+  const match = line.match(/^ {0,3}(`{3,}|~{3,})[^`]*$/)
+  return match ? match[1] : null
+}
+
+const isCodeFenceEnd = (line: string, fence: string) => {
+  const match = line.match(new RegExp(`^ {0,3}${fence}${fence[0]}* *$`))
+  return match !== null
+}
 
 const isTaskListItem = (item: ListItem | null): item is TaskListItem => typeof item?.taskBox === 'string'
 
@@ -45,17 +54,23 @@ export const useListInteraction = ({
   const onToggleItem = useCallback(
     (toggledItemIndex: number) => () => {
       const lines = markdownRef.current.split('\n')
-      let inCodeBlock = false
+      let currentCodeFence: string | null = null
 
       for (let lineIndex = 0, taskIndex = 0; lineIndex < lines.length; lineIndex++) {
-        if (isCodeBlockDelimiter(lines[lineIndex])) {
-          inCodeBlock = !inCodeBlock
+        const line = lines[lineIndex]
+
+        if (!currentCodeFence) {
+          currentCodeFence = parseCodeFenceBegin(line)
+        } else if (isCodeFenceEnd(line, currentCodeFence)) {
+          currentCodeFence = null
           continue
         }
 
-        const parsedLine = parseListItem(lines[lineIndex])
+        if (currentCodeFence) continue
+
+        const parsedLine = parseListItem(line)
 
-        if (!isTaskListItem(parsedLine) || inCodeBlock) continue
+        if (!isTaskListItem(parsedLine)) continue
 
         if (taskIndex === toggledItemIndex) {
           const updatedLine = listItemToString(toggleTaskListItem(parsedLine))