-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
GFM compliance for tasks lists #1250
Changes from 1 commit
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 |
---|---|---|
|
@@ -195,7 +195,9 @@ Lexer.prototype.token = function(src, top) { | |
i, | ||
tag, | ||
l, | ||
isordered; | ||
isordered, | ||
istask, | ||
ischecked; | ||
|
||
while (src) { | ||
// newline | ||
|
@@ -363,10 +365,20 @@ Lexer.prototype.token = function(src, top) { | |
if (!loose) loose = next; | ||
} | ||
|
||
// Check for task list items | ||
istask = /^\[[ xX]\]/.test(item); | ||
ischecked = undefined; | ||
if (istask) { | ||
ischecked = item[1] !== ' '; | ||
item = item.replace(/^\[[ xX]\] */, ''); | ||
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. - /^\[[ xX]\] */
+ /^\[[ xX]\] +/ since at least one space is required |
||
} | ||
|
||
this.tokens.push({ | ||
type: loose | ||
? 'loose_item_start' | ||
: 'list_item_start' | ||
: 'list_item_start', | ||
task: istask, | ||
checked: ischecked | ||
}); | ||
|
||
// Recurse. | ||
|
@@ -927,6 +939,14 @@ Renderer.prototype.listitem = function(text) { | |
return '<li>' + text + '</li>\n'; | ||
}; | ||
|
||
Renderer.prototype.checkbox = function(checked) { | ||
return '<input ' | ||
+ (checked ? 'checked="" ' : '') | ||
+ 'disabled="" type="checkbox"' | ||
+ (this.options.xhtml ? ' /' : '') | ||
+ '>'; | ||
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. from the specs it looks like there should be a space after |
||
} | ||
|
||
Renderer.prototype.paragraph = function(text) { | ||
return '<p>' + text + '</p>\n'; | ||
}; | ||
|
@@ -1198,6 +1218,10 @@ Parser.prototype.tok = function() { | |
case 'list_item_start': { | ||
body = ''; | ||
|
||
if (this.token.task) { | ||
body += this.renderer.checkbox(this.token.checked); | ||
} | ||
|
||
while (this.next().type !== 'list_item_end') { | ||
body += this.token.type === 'text' | ||
? this.parseText() | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,7 +15,8 @@ Messenger.prototype.test = function(spec, section, ignore) { | |
var shouldFail = ~ignore.indexOf(spec.example); | ||
it('should ' + (shouldFail ? 'fail' : 'pass') + ' example ' + spec.example, function() { | ||
var expected = spec.html; | ||
var actual = marked(spec.markdown, { headerIds: false, xhtml: true }); | ||
var usexhtml = typeof spec.xhtml === 'boolean' ? spec.xhtml : true; | ||
var actual = marked(spec.markdown, { headerIds: false, xhtml: usexhtml }); | ||
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. It looks like just setting 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. I think CommonMark uses XHTML maybe the GFM extensions don't need XHTML. (I do think there's something to be said for the attempted solution - putting the options in the JSON - but not sure we've hit a point of actually needing it.) 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'll just turn off xhtml altogether for gfm tests, and remove this spec property. |
||
since(messenger.message(spec, expected, actual)).expect( | ||
htmlDiffer.isEqual(expected, actual) | ||
).toEqual(!shouldFail); | ||
|
@@ -42,7 +43,7 @@ describe('GFM 0.28 Tables', function() { | |
describe('GFM 0.28 Task list items', function() { | ||
var section = 'Task list items'; | ||
|
||
var shouldPassButFails = [272, 273]; | ||
var shouldPassButFails = []; | ||
|
||
var willNotBeAttemptedByCoreTeam = []; | ||
|
||
|
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.
a space seems to be required after
[ ]
other wise it is rendered like textwith space:
without space:
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'll add this space to the pattern, along with the corresponding change below.