Merge pull request #173 from marp-team/loose-yaml-for-custom-directives

Allow loose YAML parsing for custom directives

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## [Unreleased]
 
+### Added
+
+- Allow loose YAML parsing for custom directives ([#173](https://github.com/marp-team/marpit/pull/173))
+
 ## v1.2.0 - 2019-06-17
 
 ### Added

src/markdown/directives/parse.js

@@ -32,7 +32,15 @@ function parse(md, opts = {}) {
     md.use(MarkdownItFrontMatter, fm => {
       frontMatterObject.text = fm
 
-      const parsed = yaml(fm, !!md.marpit.options.looseYAML)
+      const parsed = yaml(
+        fm,
+        marpit.options.looseYAML
+          ? [
+              ...Object.keys(marpit.customDirectives.global),
+              ...Object.keys(marpit.customDirectives.local),
+            ]
+          : false
+      )
       if (parsed !== false) frontMatterObject.yaml = parsed
     })
   }

src/markdown/directives/yaml.js

@@ -2,19 +2,21 @@
 import YAML, { FAILSAFE_SCHEMA } from 'js-yaml'
 import directives from './directives'
 
-/**
- * Parse text as YAML by using js-yaml's FAILSAFE_SCHEMA.
- *
- * @alias module:markdown/directives/yaml
- * @param {String} text Target text.
- * @param {boolean} allowLoose By `true`, it try to parse loose YAML in defined
- *     directives.
- * @returns {Object|false} Return parse result, or `false` when failed to parse.
- */
+const createPatterns = keys => {
+  const set = new Set()
+
+  for (const k of keys) {
+    const normalized = k.replace(/[.*+?^=!:${}()|[\]\\/]/g, '\\$&')
 
-const keyPattern = `[_$]?(?:${directives.join('|')})`
-const looseMatcher = new RegExp(`^(${keyPattern}\\s*:)(.+)$`)
-const specialChars = `["'{|>~&*`
+    set.add(normalized)
+    set.add(`"${normalized}"`)
+    set.add(`'${normalized}'`)
+  }
+
+  return [...set.values()]
+}
+
+const yamlSpecialChars = `["'{|>~&*`
 
 function parse(text) {
   try {
@@ -27,14 +29,17 @@ function parse(text) {
   }
 }
 
-function convertLoose(text) {
+function convertLoose(text, looseDirectives) {
+  const keyPattern = `[_$]?(?:${createPatterns(looseDirectives).join('|')})`
+  const looseMatcher = new RegExp(`^(${keyPattern}\\s*:)(.+)$`)
+
   let normalized = ''
 
   for (const line of text.split(/\r?\n/))
     normalized += `${line.replace(looseMatcher, (original, prop, value) => {
       const trimmed = value.trim()
 
-      if (trimmed.length === 0 || specialChars.includes(trimmed[0]))
+      if (trimmed.length === 0 || yamlSpecialChars.includes(trimmed[0]))
         return original
 
       const spaceLength = value.length - value.trimLeft().length
@@ -46,5 +51,24 @@ function convertLoose(text) {
   return normalized.trim()
 }
 
-export default (text, allowLoose) =>
-  parse(allowLoose ? convertLoose(text) : text)
+/**
+ * Parse text as YAML by using js-yaml's FAILSAFE_SCHEMA.
+ *
+ * @alias module:markdown/directives/yaml
+ * @param {String} text Target text.
+ * @param {boolean|string[]} [looseDirectives=false] By setting `true`, it try
+ *     to parse as loose YAML only in defined Marpit built-in directives. You
+ *     may also extend target keys for loose parsing by passing an array of
+ *     strings.
+ * @returns {Object|false} Return parse result, or `false` when failed to parse.
+ */
+
+export default (text, looseDirectives = false) =>
+  parse(
+    looseDirectives
+      ? convertLoose(text, [
+          ...directives,
+          ...(Array.isArray(looseDirectives) ? looseDirectives : []),
+        ])
+      : text
+  )

src/marpit.js

@@ -46,7 +46,8 @@ class Marpit {
    *     slide page at before of headings. it would apply to headings whose
    *     larger than or equal to the specified level if a number is given, or
    *     ONLY specified levels if a number array.
-   * @param {boolean} [opts.looseYAML=false] Allow loose YAML for directives.
+   * @param {boolean} [opts.looseYAML=false] Allow loose YAML parsing in
+   *     built-in directives, and custom directives defined in current instance.
    * @param {MarkdownIt|string|Object|Array} [opts.markdown] An instance of
    *     markdown-it or its constructor option(s) for wrapping. Marpit will
    *     create its instance based on CommonMark when omitted.

test/markdown/directives/yaml.js

@@ -2,14 +2,14 @@ import dedent from 'dedent'
 import yaml from '../../../src/markdown/directives/yaml'
 
 describe('Marpit directives YAML parser', () => {
-  it("ignores directive's special char with false allowLoose option", () =>
+  it("ignores directive's special char with false looseDirectives option", () =>
     expect(yaml('color: #f00', false).color).toBeNull())
 
-  context('with allowLoose option as true', () => {
+  context('with looseDirectives option as true', () => {
     it("parses directive's special char as string", () =>
       expect(yaml('color: #f00', true).color).toBe('#f00'))
 
-    it('disallows loose parsing in not defined directives', () => {
+    it('disallows loose parsing in not built-in directives', () => {
       const body = dedent`
         backgroundColor: #f00
         header: _"HELLO!"_
@@ -50,4 +50,21 @@ describe('Marpit directives YAML parser', () => {
       `)
     })
   })
+
+  context('with looseDirectives option as extra keys', () => {
+    it('allows loose parsing in not built-in directives', () => {
+      const body = dedent`
+        notDefinedDirective: # THIS IS NOT A COMMENT
+        a.c: #def
+        abc: # THIS IS A COMMENT
+      `
+      const parsed = yaml(body, ['notDefinedDirective', 'a.c'])
+
+      expect(parsed.notDefinedDirective).toBe('# THIS IS NOT A COMMENT')
+      expect(parsed['a.c']).toBe('#def')
+
+      // It would fail if you forget escape special characters for RegEx
+      expect(parsed.abc).toBeNull()
+    })
+  })
 })

test/marpit.js

@@ -102,6 +102,30 @@ describe('Marpit', () => {
         expect(first.meta.marpitDirectives).toStrictEqual({ test: 'local' })
         expect(second.meta.marpitDirectives).toStrictEqual({ test: 'spot' })
       })
+
+      context('with looseYAML option as true', () => {
+        it('allows loose YAML parsing for custom directives', () => {
+          const marpit = new Marpit({ container: undefined, looseYAML: true })
+          marpit.customDirectives.global.a = v => ({ a: v })
+          marpit.customDirectives.local.b = v => ({ b: v })
+
+          const [token] = marpit.markdown.parse('---\na: #123\nb: #abc\n---')
+          expect(token.meta.marpitDirectives.a).toBe('#123')
+          expect(token.meta.marpitDirectives.b).toBe('#abc')
+        })
+      })
+
+      context('with looseYAML option as false', () => {
+        it('disallows loose YAML parsing for custom directives', () => {
+          const marpit = new Marpit({ container: undefined, looseYAML: false })
+          marpit.customDirectives.global.a = v => ({ a: v })
+          marpit.customDirectives.local.b = v => ({ b: v })
+
+          const [token] = marpit.markdown.parse('---\na: #123\nb: #abc\n---')
+          expect(token.meta.marpitDirectives.a).toBeNull()
+          expect(token.meta.marpitDirectives.b).toBeNull()
+        })
+      })
     })
 
     it('has themeSet property', () => {
@@ -335,7 +359,7 @@ describe('Marpit', () => {
         ---
       `
 
-      it('allows loose YAML parsing when looseYAML is true', () => {
+      it('allows loose YAML parsing for built-in directives when looseYAML is true', () => {
         const rendered = instance(true).render(markdown)
         const $ = cheerio.load(rendered.html)
         const firstStyle = $('section:nth-of-type(1)').attr('style')
@@ -347,7 +371,7 @@ describe('Marpit', () => {
         expect(secondStyle).not.toContain('color:')
       })
 
-      it('disallows loose YAML parsing when looseYAML is false', () => {
+      it('disallows loose YAML parsing for built-in directives when looseYAML is false', () => {
         const rendered = instance(false).render(markdown)
         const $ = cheerio.load(rendered.html)
         const style = $('section:nth-of-type(1)').attr('style')