-
Notifications
You must be signed in to change notification settings - Fork 162
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
Adds comment syntax to Markdoc #198
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,7 +7,7 @@ import { any } from 'deep-assert'; | |
|
||
describe('Markdown parser', function () { | ||
const fence = '```'; | ||
const tokenizer = new Tokenizer(); | ||
const tokenizer = new Tokenizer({ allowComments: true }); | ||
|
||
function convert(example) { | ||
const content = example.replace(/\n\s+/gm, '\n').trim(); | ||
|
@@ -636,4 +636,20 @@ describe('Markdown parser', function () { | |
`); | ||
}).not.toThrow(); | ||
}); | ||
|
||
it('parsing comments', function () { | ||
const example = convert(` | ||
this is a test | ||
|
||
<!-- foo --> | ||
`); | ||
|
||
expect(example).toDeepEqualSubset({ | ||
type: 'document', | ||
children: [ | ||
{ type: 'paragraph' }, | ||
{ type: 'comment', attributes: { content: 'foo' } }, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Neat! |
||
], | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -45,6 +45,7 @@ function handleAttrs(token: Token, type: string) { | |
} | ||
case 'text': | ||
case 'code': | ||
case 'comment': | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Love how this part of the code is working. |
||
return { content: (token.meta || {}).variable || token.content }; | ||
case 'fence': { | ||
const [language] = token.info.split(' ', 1); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,17 @@ | ||
import MarkdownIt from 'markdown-it/lib'; | ||
import annotations from './plugins/annotations'; | ||
import frontmatter from './plugins/frontmatter'; | ||
import comments from './plugins/comments'; | ||
import type Token from 'markdown-it/lib/token'; | ||
|
||
export default class Tokenizer { | ||
private parser: MarkdownIt; | ||
|
||
constructor( | ||
config: MarkdownIt.Options & { allowIndentation?: boolean } = {} | ||
config: MarkdownIt.Options & { | ||
allowIndentation?: boolean; | ||
allowComments?: boolean; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Default to |
||
} = {} | ||
) { | ||
this.parser = new MarkdownIt(config); | ||
this.parser.use(annotations, 'annotations', {}); | ||
|
@@ -17,6 +21,8 @@ export default class Tokenizer { | |
// Disable indented `code_block` support https://spec.commonmark.org/0.30/#indented-code-block | ||
'code', | ||
]); | ||
|
||
if (config.allowComments) this.parser.use(comments, 'comments', {}); | ||
} | ||
|
||
tokenize(content: string): Token[] { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,94 @@ | ||
import Tokenizer from '..'; | ||
|
||
describe('MarkdownIt Comments plugin', function () { | ||
const tokenizer = new Tokenizer({ allowComments: true }); | ||
|
||
function parse(example) { | ||
const content = example.replace(/\n\s+/gm, '\n').trim(); | ||
return tokenizer.tokenize(content); | ||
} | ||
|
||
describe('inline comments', function () { | ||
const output = [ | ||
{ type: 'paragraph_open' }, | ||
{ | ||
type: 'inline', | ||
children: [ | ||
{ type: 'text', content: 'this is a test ' }, | ||
{ type: 'comment', content: 'example comment' }, | ||
{ type: 'text', content: ' foo' }, | ||
], | ||
}, | ||
{ type: 'paragraph_close' }, | ||
]; | ||
|
||
it('simple inline comment', function () { | ||
const example = parse(` | ||
this is a test <!-- example comment --> foo | ||
`); | ||
|
||
expect(example).toDeepEqualSubset(output); | ||
}); | ||
|
||
it('inline comment with a newline', function () { | ||
const example = parse(` | ||
this is a test <!-- | ||
example comment | ||
--> foo | ||
`); | ||
|
||
expect(example).toDeepEqualSubset(output); | ||
}); | ||
}); | ||
|
||
describe('block comments', function () { | ||
const output = [ | ||
{ type: 'paragraph_open' }, | ||
{ type: 'inline' }, | ||
{ type: 'paragraph_close' }, | ||
{ type: 'comment', content: 'example comment' }, | ||
{ type: 'paragraph_open' }, | ||
{ type: 'inline', content: 'foo' }, | ||
{ type: 'paragraph_close' }, | ||
]; | ||
|
||
it('simple block comment after a paragraph', function () { | ||
const example = parse(` | ||
this is a test | ||
|
||
<!-- | ||
example comment | ||
--> | ||
|
||
foo | ||
`); | ||
|
||
expect(example).toDeepEqualSubset(output); | ||
}); | ||
|
||
it('block comment with ending on same line as content', function () { | ||
const example = parse(` | ||
this is a test | ||
|
||
<!-- | ||
example comment --> | ||
|
||
foo | ||
`); | ||
|
||
expect(example).toDeepEqualSubset(output); | ||
}); | ||
|
||
it('block comment one one line', function () { | ||
const example = parse(` | ||
this is a test | ||
|
||
<!-- example comment --> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For these last few tests, should we include the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was a good suggestion, adding it to the tests caught a bug. I'll have a push fixed shortly. |
||
|
||
foo | ||
`); | ||
|
||
expect(example).toDeepEqualSubset(output); | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
import type MarkdownIt from 'markdown-it/lib'; | ||
import type StateBlock from 'markdown-it/lib/rules_block/state_block'; | ||
import type StateInline from 'markdown-it/lib/rules_inline/state_inline'; | ||
|
||
const OPEN = '<!--'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm totally peaceful with this approach, but I wonder if we should use the same regex that markdown-it uses internally: https://github.com/markdown-it/markdown-it/blob/df4607f1d4d4be7fdc32e71c04109aea8cc373fa/lib/common/html_re.js#L18 var comment = '<!---->|<!--(?:-?[^>-])(?:-?[^-])*-->'; There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd prefer to avoid using a regex here |
||
const CLOSE = '-->'; | ||
|
||
function block( | ||
state: StateBlock, | ||
startLine: number, | ||
endLine: number, | ||
silent: boolean | ||
): boolean { | ||
const start = state.bMarks[startLine] + state.tShift[startLine]; | ||
if (!state.src.startsWith(OPEN, start)) return false; | ||
|
||
const close = state.src.indexOf(CLOSE, start); | ||
|
||
if (!close) return false; | ||
if (silent) return true; | ||
|
||
const content = state.src.slice(start + OPEN.length, close); | ||
const lines = content.split('\n').length; | ||
const token = state.push('comment', '', 0); | ||
token.content = content.trim(); | ||
token.map = [startLine, startLine + lines]; | ||
state.line += lines; | ||
|
||
return true; | ||
} | ||
|
||
function inline(state: StateInline, silent: boolean): boolean { | ||
if (!state.src.startsWith(OPEN, state.pos)) return false; | ||
|
||
const close = state.src.indexOf(CLOSE, state.pos); | ||
|
||
if (!close) return false; | ||
if (silent) return true; | ||
|
||
const content = state.src.slice(state.pos + OPEN.length, close); | ||
const token = state.push('comment', '', 0); | ||
token.content = content.trim(); | ||
state.pos = close + CLOSE.length; | ||
|
||
return true; | ||
} | ||
|
||
export default function plugin(md: MarkdownIt) { | ||
md.block.ruler.before('table', 'comment', block, { alt: ['paragraph'] }); | ||
md.inline.ruler.push('comment', inline); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feels to me like
allowComments
should betrue
by default, no?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do intend for it to eventually be on by default, but for the moment I want to keep it to off until we've had an opportunity to test it in practice more comprehensively.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh. Makes sense.