Improve lazy YAML parsing to apply in defined directives only

CHANGELOG.md

@@ -3,6 +3,7 @@
 ## [Unreleased]
 
 - Remove Unicode Emoji support due to many issues on stable Chrome ([#53](https://github.com/marp-team/marpit/pull/53))
+- Improve lazy YAML parsing to apply in defined directives only ([#54](https://github.com/marp-team/marpit/pull/54))
 
 ## v0.0.11 - 2018-08-12
 

README.md

@@ -38,7 +38,7 @@ Marpit will become a core of _the next version of **[Marp](https://github.com/yh
 #### Difference from [pre-released Marp](https://github.com/yhatt/marp/)
 
 - Removed directives about slide size. [Use `width` / `height` declaration of theme CSS.](#slide-size)
-- Parse directives by YAML parser. ([js-yaml](https://github.com/nodeca/js-yaml) + [`FAILSAFE_SCHEMA`](http://www.yaml.org/spec/1.2/spec.html#id2802346), but we still support lazy YAML parser by `lazyYAML` option)
+- Parse directives by YAML parser. ([js-yaml](https://github.com/nodeca/js-yaml) + [`FAILSAFE_SCHEMA`](http://www.yaml.org/spec/1.2/spec.html#id2802346), but we still support lazy YAML for directives by `lazyYAML` option)
 - Support [Jekyll style front-matter](https://jekyllrb.com/docs/frontmatter/).
 - _[Global directives](https://github.com/yhatt/marp/blob/master/example.md#global-directives)_ is no longer requires `$` prefix. (but it still supports because of compatibility and clarity)
 - [Page directives](https://github.com/yhatt/marp/blob/master/example.md#page-directives) is renamed to _local directives_.
@@ -119,7 +119,7 @@ footer: "![image](https://example.com/image.jpg)"
 
 > :warning: Marpit uses YAML for parsing directives, so **you should wrap with (double-)quotes** when the value includes invalid chars in YAML.
 >
-> You can enable a lazy YAML parser by `lazyYAML` Marpit constructor option if you want to recognize string without quotes.
+> You can enable a lazy YAML parsing by `lazyYAML` Marpit constructor option if you want to recognize defined directive's string without quotes.
 
 > :information_source: Due to the parsing order of Markdown, you cannot use [slide background images](#slide-background) in `header` and `footer` directives.
 

src/markdown/directives/yaml.js

@@ -7,16 +7,16 @@ import directives from './directives'
  *
  * @alias module:markdown/directives/yaml
  * @param {String} text Target text.
- * @param {boolean} allowLazy By `true`, it try to parse lazy YAML at first.
+ * @param {boolean} allowLazy By `true`, it try to parse lazy YAML in defined
+ *     directives.
  * @returns {Object|false} Return parse result, or `false` when failed to parse.
  */
 
 const keyPattern = `[_$]?(?:${directives.join('|')})`
-const directivesMatcher = new RegExp(`^\\s*(${keyPattern})\\s*:(.*)$`)
+const lazyMatcher = new RegExp(`^(${keyPattern}\\s*:)(.+)$`)
 const specialChars = `["'{|>~&*`
-const whitespaceMatcher = /^\s*$/
 
-function strict(text) {
+function parse(text) {
   try {
     const obj = YAML.safeLoad(text, { schema: FAILSAFE_SCHEMA })
     if (obj === null || typeof obj !== 'object') return false
@@ -27,32 +27,25 @@ function strict(text) {
   }
 }
 
-function lazy(text) {
-  const collected = {}
-  const lines = text.split(/\r?\n/)
-
-  return lines.every(line => {
-    if (whitespaceMatcher.test(line)) return true
-
-    const matched = directivesMatcher.exec(line)
-    if (!matched) return false
-
-    const [, directive, originalValue] = matched
-    const value = originalValue.trim()
-    if (specialChars.includes(value[0])) return false
-
-    collected[directive] = value
-    return true
-  })
-    ? collected
-    : false
+function convertLazy(text) {
+  return text
+    .split(/\r?\n/)
+    .reduce(
+      (ret, line) =>
+        `${ret}${line.replace(lazyMatcher, (original, prop, value) => {
+          const trimmed = value.trim()
+
+          if (trimmed.length === 0 || specialChars.includes(trimmed[0]))
+            return original
+
+          const spaceLength = value.length - value.trimLeft().length
+          const spaces = value.substring(0, spaceLength)
+
+          return `${prop}${spaces}"${trimmed.split('"').join('\\"')}"`
+        })}\n`,
+      ''
+    )
+    .trim()
 }
 
-export default function(text, allowLazy) {
-  if (allowLazy) {
-    const lazyResult = lazy(text)
-    if (lazyResult !== false) return lazyResult
-  }
-
-  return strict(text)
-}
+export default (text, allowLazy) => parse(allowLazy ? convertLazy(text) : text)

test/markdown/directives/yaml.js

@@ -3,23 +3,32 @@ import yaml from '../../../src/markdown/directives/yaml'
 
 describe('Marpit directives YAML parser', () => {
   it("ignores directive's special char with false allowLazy option", () =>
-    expect(yaml('color: #f00', false).color).toBeFalsy())
+    expect(yaml('color: #f00', false).color).toBeNull())
 
   context('with allowLazy option as true', () => {
     it("parses directive's special char as string", () =>
       expect(yaml('color: #f00', true).color).toBe('#f00'))
 
-    it('fallbacks to regular YAML parser when passed like strict YAML', () => {
+    it('disallows lazy parsing in not defined directives', () => {
+      const body = dedent`
+        backgroundColor: #f00
+        header: _"HELLO!"_
+        notDefinedDirective: # THIS IS A COMMENT
+      `
+      const parsed = yaml(body, true)
+
+      expect(parsed.backgroundColor).toBe('#f00')
+      expect(parsed.header).toBe('_"HELLO!"_')
+      expect(parsed.notDefinedDirective).toBeNull()
+    })
+
+    it('returns result as same as regular YAML when passed like strict YAML', () => {
       const confirm = text =>
         expect(yaml(text, true)).toMatchObject(yaml(text, false))
 
       confirm('headingDivider: [3]')
       confirm('backgroundPosition: "left center"')
       confirm("backgroundSize: '100px 200px'")
-      confirm(dedent`
-        color: #f00
-        notSupported: key
-      `)
       confirm(dedent`
         class:
           - first

test/marpit.js

@@ -190,13 +190,13 @@ describe('Marpit', () => {
       const markdown = dedent`
         ---
         backgroundImage:  url('/image.jpg')
-                 _color:  #123${' \t'}
+        _color:           #123${' \t'}
         ---
 
         ---
       `
 
-      it('allows lazy YAML format when lazyYaml is true', () => {
+      it('allows lazy YAML parsing when lazyYaml is true', () => {
         const rendered = instance(true).render(markdown)
         const $ = cheerio.load(rendered.html)
         const firstStyle = $('section:nth-of-type(1)').attr('style')
@@ -208,11 +208,13 @@ describe('Marpit', () => {
         expect(secondStyle).not.toContain('color:')
       })
 
-      it('disallows lazy YAML format when lazyYaml is false', () => {
+      it('disallows lazy YAML parsing when lazyYaml is false', () => {
         const rendered = instance(false).render(markdown)
         const $ = cheerio.load(rendered.html)
+        const style = $('section:nth-of-type(1)').attr('style')
 
-        expect($('section[style]')).toHaveLength(0)
+        expect(style).toContain("background-image:url('/image.jpg')")
+        expect(style).not.toContain('color:#123;')
       })
     })
   })