Merge pull request #240 from marp-team/directive-parser-crlf

Fix wrong highlight in CRLF
marp-team · May 18, 2021 · 0e0e0ba · 0e0e0ba
2 parents d42d9c2 + d935fa2
commit 0e0e0ba
Show file tree

Hide file tree

Showing 10 changed files with 160 additions and 45 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -83,20 +83,6 @@ jobs:
     steps:
       - audit
 
-  unit-electron9:
-    executor:
-      name: node
-      version: '12.14.1'
-    steps:
-      - test
-
-  unit-electron11:
-    executor:
-      name: node
-      version: '12.18.3'
-    steps:
-      - test
-
   unit-electron12:
     executor:
       name: node
@@ -108,12 +94,6 @@ workflows:
   test:
     jobs:
       - audit
-      - unit-electron9:
-          requires:
-            - audit
-      - unit-electron11:
-          requires:
-            - audit
       - unit-electron12:
           requires:
             - audit
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,10 @@
 
 ## [Unreleased]
 
+### Fixed
+
+- Wrong highlight in CRLF ([#239](https://github.com/marp-team/marp-vscode/issues/239), [#240](https://github.com/marp-team/marp-vscode/pull/240))
+
 ## v1.0.0 - 2021-05-17
 
 ### Breaking

diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -273,7 +273,7 @@
     "unist-util-visit": "^3.1.0",
     "utf-8-validate": "^5.0.5",
     "vsce": "^1.88.0",
-    "yaml": "^1.10.2"
+    "yaml": "^2.0.0-5"
   },
   "dependencies": {
     "@marp-team/marp-cli": "^1.1.1",

diff --git a/src/diagnostics/deprecated-dollar-prefix.ts b/src/diagnostics/deprecated-dollar-prefix.ts
@@ -30,7 +30,7 @@ export function register(
   diagnostics: Diagnostic[]
 ) {
   directiveParser.on('directive', ({ item, offset }) => {
-    if (warnDirectives.includes(item.key.value)) {
+    if (warnDirectives.includes(item.key.value) && item.key.range) {
       const name = item.key.value.slice(1)
       const [start, end] = item.key.range
 

diff --git a/src/diagnostics/overloading-global-directive.ts b/src/diagnostics/overloading-global-directive.ts
@@ -1,6 +1,6 @@
 import { Diagnostic, DiagnosticSeverity, Range, TextDocument } from 'vscode'
-import { Pair } from 'yaml/types'
 import { DirectiveParser, DirectiveType } from '../directives/parser'
+import type { Pair } from 'yaml'
 
 interface ParsedGlobalDirective {
   item: Pair
@@ -21,7 +21,7 @@ export function register(
   })
 
   directiveParser.on('directive', ({ item, offset, info }) => {
-    if (info?.type === DirectiveType.Global) {
+    if (info?.type === DirectiveType.Global && item.key.range) {
       const [start] = item.key.range
       const [, end] = item.value?.range ?? item.key.range
 

diff --git a/src/diagnostics/unknown-theme.ts b/src/diagnostics/unknown-theme.ts
@@ -21,7 +21,7 @@ export function register(
   })
 
   directiveParser.on('directive', ({ item, offset, info }) => {
-    if (info?.name === 'theme' && item.value) {
+    if (info?.name === 'theme' && item.value?.range) {
       const [start, end] = item.value.range
 
       parsed = {

diff --git a/src/directives/parser.ts b/src/directives/parser.ts
@@ -5,8 +5,7 @@ import TypedEmitter from 'typed-emitter'
 import unified from 'unified'
 import { visit } from 'unist-util-visit'
 import { Range, TextDocument } from 'vscode'
-import yaml from 'yaml'
-import { Pair, YAMLMap } from 'yaml/types'
+import yaml, { Pair, Scalar, YAMLMap } from 'yaml'
 import { frontMatterRegex } from '../utils'
 import {
   builtinDirectives,
@@ -30,7 +29,7 @@ const parseYaml = (yamlBody: string) =>
 
 export interface DirectiveEventHandler {
   info?: DirectiveInfo
-  item: Pair
+  item: Pair<Scalar<string>, Scalar<any>>
   offset: number
 }
 
@@ -63,11 +62,12 @@ export class DirectiveParser extends (EventEmitter as new () => TypedEmitter<Dir
       offset: number,
       definedIn = DirectiveDefinedIn.Comment
     ) => {
-      const { contents, errors } = parseYaml(text)
+      const { contents, errors } = parseYaml(text.replace(/\r$/, ''))
 
       if (errors.length === 0 && contents?.['items']) {
-        for (const item of (contents as YAMLMap).items) {
-          if (item.type === 'PAIR') {
+        for (const item of (contents as YAMLMap<Scalar<string>, Scalar<any>>)
+          .items) {
+          if (item.key && item.value) {
             let scoped: boolean | undefined = undefined
 
             const directiveInfo = this.directives.find((d) => {
@@ -116,23 +116,30 @@ export class DirectiveParser extends (EventEmitter as new () => TypedEmitter<Dir
     // HTML comments
     visit(parseMd(markdown), 'html', (n: any) => {
       visit(parseHtml(n.value), 'comment', (c: any) => {
+        const rawBody = n.value.slice(
+          c.position.start.offset,
+          c.position.end.offset
+        )
+
         this.emit('comment', {
-          body: `<!--${c.value}-->`,
+          body: rawBody,
           range: new Range(
             doc.positionAt(index + n.position.start.offset),
             doc.positionAt(index + n.position.end.offset)
           ),
         })
 
-        const trimmedLeft = c.value.replace(/^-*\s*/, '')
+        // c.value should not use because it has normalized CRLF to LF
+        const value = rawBody.slice(4, -3)
+        const trimmedLeft = value.replace(/^-*\s*/, '')
 
         detectDirectives(
           trimmedLeft.replace(/\s*-*$/, ''),
           index +
             n.position.start.offset +
             c.position.start.offset +
             4 +
-            (c.value.length - trimmedLeft.length)
+            (value.length - trimmedLeft.length)
         )
       })
     })

diff --git a/src/language/parser.test.ts b/src/language/parser.test.ts
@@ -68,10 +68,17 @@ describe('Language parser', () => {
           test: test
           ---
 
-          <!-- paginate: true -->
+          <!--
+          paginate: true
+          -->
           <!-- _paginate: false -->
         `),
-      positionAt: (idx: number) => new Position(0, idx),
+      positionAt(offset: number) {
+        const lines = this.getText().slice(0, offset).split('\n')
+
+        // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+        return new Position(lines.length - 1, lines.pop()!.length)
+      },
     }
     window.activeTextEditor = { document } as any
 
@@ -82,6 +89,106 @@ describe('Language parser', () => {
     expect(data?.commentRanges).toHaveLength(2)
     expect(data?.frontMatterRange).toBeInstanceOf(Range)
     expect(data?.directvies).toHaveLength(4)
+
+    // Range data
+    expect(data?.directvies[0].range.start).toMatchObject({
+      line: 1,
+      character: 0,
+    })
+    expect(data?.directvies[0].range.end).toMatchObject({
+      line: 1,
+      character: 10,
+    })
+    expect(data?.directvies[1].range.start).toMatchObject({
+      line: 2,
+      character: 0,
+    })
+    expect(data?.directvies[1].range.end).toMatchObject({
+      line: 2,
+      character: 14,
+    })
+    expect(data?.directvies[2].range.start).toMatchObject({
+      line: 7,
+      character: 0,
+    })
+    expect(data?.directvies[2].range.end).toMatchObject({
+      line: 7,
+      character: 14,
+    })
+    expect(data?.directvies[3].range.start).toMatchObject({
+      line: 9,
+      character: 5,
+    })
+    expect(data?.directvies[3].range.end).toMatchObject({
+      line: 9,
+      character: 21,
+    })
+  })
+
+  it('has correct source map even if used CR+LF newline', async () => {
+    const document = {
+      languageId: 'markdown',
+      getText: () => {
+        const baseDoc = dedent(`
+          ---
+          marp: true
+          theme: default
+          test: test
+          ---
+
+          <!--
+          paginate: true
+          -->
+          <!-- _paginate: false -->
+        `)
+
+        return baseDoc.split('\n').join('\r\n')
+      },
+      positionAt(offset: number) {
+        const lines = this.getText().slice(0, offset).split('\n')
+
+        // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+        return new Position(lines.length - 1, lines.pop()!.length)
+      },
+    }
+    window.activeTextEditor = { document } as any
+
+    const parser = new LanguageParser([])
+    const data = await parser.getParseData(document as any)
+
+    expect(data).toBeTruthy()
+    expect(data?.directvies[0].range.start).toMatchObject({
+      line: 1,
+      character: 0,
+    })
+    expect(data?.directvies[0].range.end).toMatchObject({
+      line: 1,
+      character: 10,
+    })
+    expect(data?.directvies[1].range.start).toMatchObject({
+      line: 2,
+      character: 0,
+    })
+    expect(data?.directvies[1].range.end).toMatchObject({
+      line: 2,
+      character: 14,
+    })
+    expect(data?.directvies[2].range.start).toMatchObject({
+      line: 7,
+      character: 0,
+    })
+    expect(data?.directvies[2].range.end).toMatchObject({
+      line: 7,
+      character: 14,
+    })
+    expect(data?.directvies[3].range.start).toMatchObject({
+      line: 9,
+      character: 5,
+    })
+    expect(data?.directvies[3].range.end).toMatchObject({
+      line: 9,
+      character: 21,
+    })
   })
 
   it('does not parse Markdown when it is not Marp Markdown', async () => {

diff --git a/src/language/parser.ts b/src/language/parser.ts
@@ -113,7 +113,7 @@ export class LanguageParser
       })
       .on('comment', ({ range }) => commentRanges.push(range))
       .on('directive', ({ item, info, offset }) => {
-        if (info) {
+        if (info && item.key.range) {
           const [start, end] = item.key.range
           const [, vEnd] = item.value?.range ?? item.key.range