Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Output deprecation warning when use dollar prefix in global directive #183

Merged
merged 11 commits into from
Aug 11, 2019
11 changes: 4 additions & 7 deletions src/markdown/directives/directives.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,11 @@
* Each global directive assigns to the whole slide deck. If you wrote a same
* directive many times, Marpit only recognizes the last value.
*
* You can use prefix `$` as the name of a directive for the clarity (or
* compatibility with the old version of Marp).
*
* @prop {Directive} headingDivider Specify heading divider option.
* @prop {Directive} style Specify the CSS style to apply additionally.
* @prop {Directive} theme Specify theme of the slide deck.
*/
export const globals = {
export const globals = Object.assign(Object.create(null), {
headingDivider: value => {
const headings = [1, 2, 3, 4, 5, 6]
const toInt = v =>
Expand All @@ -44,7 +41,7 @@ export const globals = {
},
style: v => ({ style: v }),
theme: (v, marpit) => (marpit.themeSet.has(v) ? { theme: v } : {}),
}
})

/**
* Local directives.
Expand All @@ -71,7 +68,7 @@ export const globals = {
* a `<header>` element to the first of each slide contents.
* @prop {Directive} paginate Show page number on the slide if you set `true`.
*/
export const locals = {
export const locals = Object.assign(Object.create(null), {
backgroundColor: v => ({ backgroundColor: v }),
backgroundImage: v => ({ backgroundImage: v }),
backgroundPosition: v => ({ backgroundPosition: v }),
Expand All @@ -82,6 +79,6 @@ export const locals = {
footer: v => (typeof v === 'string' ? { footer: v } : {}),
header: v => (typeof v === 'string' ? { header: v } : {}),
paginate: v => ({ paginate: (v || '').toLowerCase() === 'true' }),
}
})

export default [...Object.keys(globals), ...Object.keys(locals)]
69 changes: 43 additions & 26 deletions src/markdown/directives/parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,14 @@ import yaml from './yaml'
import * as directives from './directives'
import marpitPlugin from '../marpit_plugin'

const isComment = token =>
token.type === 'marpit_comment' && token.meta.marpitParsedDirectives

const markAsParsed = token => {
token.meta = token.meta || {}
token.meta.marpitCommentParsed = 'directive'
}

/**
* Parse Marpit directives and store result to the slide token meta.
*
Expand All @@ -20,6 +28,20 @@ import marpitPlugin from '../marpit_plugin'
function parse(md, opts = {}) {
const { marpit } = md

const applyBuiltinDirectives = (newProps, builtinDirectives) => {
let ret = {}

for (const prop of Object.keys(newProps)) {
if (builtinDirectives[prop]) {
ret = { ...ret, ...builtinDirectives[prop](newProps[prop], marpit) }
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When aliasing from custom directive to built-in directive, we will re-apply function of built-in directives to allow only valid value.

} else {
ret[prop] = newProps[prop]
}
}

return ret
}

// Front-matter support
const frontMatter = opts.frontMatter === undefined ? true : !!opts.frontMatter
let frontMatterObject = {}
Expand All @@ -45,25 +67,6 @@ function parse(md, opts = {}) {
})
}

const isComment = token =>
token.type === 'marpit_comment' && token.meta.marpitParsedDirectives

const markAsParsed = token => {
token.meta = token.meta || {}
token.meta.marpitCommentParsed = 'directive'
}

const filterBuiltinDirective = newProps => {
const ret = {}

for (const prop of Object.keys(newProps).filter(
p => !directives.default.includes(p)
))
ret[prop] = newProps[prop]

return ret
}

// Parse global directives
md.core.ruler.after('inline', 'marpit_directives_global_parse', state => {
if (state.inlineMode) return
Expand All @@ -73,7 +76,18 @@ function parse(md, opts = {}) {
let recognized = false

for (const key of Object.keys(obj)) {
const globalKey = key.startsWith('$') ? key.slice(1) : key
const globalKey = key.startsWith('$')
? (() => {
if (marpit.customDirectives.global[key]) return key
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use an original directive key without warning if the dollar-prefixed custom global directive was defined.


console.warn(
`Deprecation warning: Dollar prefix support for global directive "${key}" is deprecated and won't support in next major version. Just remove "$" from "${key}" to fix ("${key.slice(
1
)}").`
)
return key.slice(1)
})()
: key

if (directives.globals[globalKey]) {
recognized = true
Expand All @@ -85,8 +99,9 @@ function parse(md, opts = {}) {
recognized = true
globalDirectives = {
...globalDirectives,
...filterBuiltinDirective(
marpit.customDirectives.global[globalKey](obj[key], marpit)
...applyBuiltinDirectives(
marpit.customDirectives.global[globalKey](obj[key], marpit),
directives.globals
),
}
}
Expand Down Expand Up @@ -135,8 +150,9 @@ function parse(md, opts = {}) {
recognized = true
cursor.local = {
...cursor.local,
...filterBuiltinDirective(
marpit.customDirectives.local[key](obj[key], marpit)
...applyBuiltinDirectives(
marpit.customDirectives.local[key](obj[key], marpit),
directives.locals
),
}
}
Expand All @@ -156,8 +172,9 @@ function parse(md, opts = {}) {
recognized = true
cursor.spot = {
...cursor.spot,
...filterBuiltinDirective(
marpit.customDirectives.local[spotKey](obj[key], marpit)
...applyBuiltinDirectives(
marpit.customDirectives.local[spotKey](obj[key], marpit),
directives.locals
),
}
}
Expand Down
2 changes: 1 addition & 1 deletion test/markdown/directives/parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ describe('Marpit directives parse plugin', () => {
expect(marpitStub.lastGlobalDirectives).toStrictEqual({})
})

it('allows global directive name prefixed "$"', () => {
it('allows global directive name prefixed "$" [DEPRECATED]', () => {
md().parse('<!-- $theme: test_theme -->')
expect(marpitStub.lastGlobalDirectives).toStrictEqual(expected)
})
Expand Down
28 changes: 24 additions & 4 deletions test/marpit.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,15 +92,35 @@ describe('Marpit', () => {
expect(token.meta.marpitDirectives).toStrictEqual({ class: 'ok' })
})

it('cannot assign built-in directive as meta', () => {
it('can assign built-in directive as alias', () => {
const $theme = jest.fn(v => ({ theme: v }))
const marpit = new Marpit({ container: undefined })

marpit.themeSet.add('/* @theme foobar */')
marpit.customDirectives.global.$theme = $theme
marpit.customDirectives.local.test = v => ({ test: v, class: v })

const [first, , , second] = marpit.markdown.parse(
// Global directive (Dollar prefix)
marpit.markdown.render('<!-- $theme: foobar -->')
expect($theme).toBeCalledWith('foobar', marpit)
expect(marpit.lastGlobalDirectives.theme).toBe('foobar')

marpit.markdown.render('<!-- $theme: unknown -->')
expect($theme).toBeCalledWith('unknown', marpit)
expect(marpit.lastGlobalDirectives.theme).toBeUndefined()

// Local directive (Alias + internal meta)
const [localFirst, , , localSecond] = marpit.markdown.parse(
'<!-- test: local -->\n***\n<!-- _test: spot -->'
)
expect(first.meta.marpitDirectives).toStrictEqual({ test: 'local' })
expect(second.meta.marpitDirectives).toStrictEqual({ test: 'spot' })
expect(localFirst.meta.marpitDirectives).toStrictEqual({
test: 'local',
class: 'local',
})
expect(localSecond.meta.marpitDirectives).toStrictEqual({
test: 'spot',
class: 'spot',
})
})

context('with looseYAML option as true', () => {
Expand Down