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.

Sign up for GitHub

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

Jump to bottom

Gfm tables #1245

Merged
merged 2 commits into from
May 1, 2018
Merged

Gfm tables #1245

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
be922ce
GFM table compliance
May 1, 2018
b78deeb
lint
May 1, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 54 additions & 43 deletions lib/marked.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,8 @@ block.gfm.paragraph = edit(block.paragraph)
*/

block.tables = merge({}, block.gfm, {
nptable: /^ *(\S.*\|.*)\n *([-:]+ *\|[-| :]*)\n((?:.*\|.*(?:\n|$))*)\n*/,
table: /^ *\|(.+)\n *\|( *[-:]+[-| :]*)\n((?: *\|.*(?:\n|$))*)\n*/
nptable: /^ *([^|\n ].*\|.*)\n *([-:]+ *\|[-| :]*)(?:\n((?:.*[^>\n ].*(?:\n|$))*)\n*|$)/,
table: /^ *\|(.+)\n *\|?( *[-:]+[-| :]*)(?:\n((?: *[^>\n ].*(?:\n|$))*)\n*|$)/
});

/**
Expand Down Expand Up @@ -245,34 +245,36 @@ Lexer.prototype.token = function(src, top) {

// table no leading pipe (gfm)
if (top && (cap = this.rules.nptable.exec(src))) {
src = src.substring(cap[0].length);

item = {
type: 'table',
header: splitCells(cap[1].replace(/^ *| *\| *$/g, '')),
align: cap[2].replace(/^ *|\| *$/g, '').split(/ *\| */),
cells: cap[3].replace(/\n$/, '').split('\n')
cells: cap[3] ? cap[3].replace(/\n$/, '').split('\n') : []
};

for (i = 0; i < item.align.length; i++) {
if (/^ *-+: *$/.test(item.align[i])) {
item.align[i] = 'right';
} else if (/^ *:-+: *$/.test(item.align[i])) {
item.align[i] = 'center';
} else if (/^ *:-+ *$/.test(item.align[i])) {
item.align[i] = 'left';
} else {
item.align[i] = null;
if (item.header.length === item.align.length) {
src = src.substring(cap[0].length);

for (i = 0; i < item.align.length; i++) {
if (/^ *-+: *$/.test(item.align[i])) {
item.align[i] = 'right';
} else if (/^ *:-+: *$/.test(item.align[i])) {
item.align[i] = 'center';
} else if (/^ *:-+ *$/.test(item.align[i])) {
item.align[i] = 'left';
} else {
item.align[i] = null;
}
}
}

for (i = 0; i < item.cells.length; i++) {
item.cells[i] = splitCells(item.cells[i]);
}
for (i = 0; i < item.cells.length; i++) {
item.cells[i] = splitCells(item.cells[i], item.header.length);
}

this.tokens.push(item);
this.tokens.push(item);

continue;
continue;
}
}

// hr
Expand Down Expand Up @@ -412,35 +414,38 @@ Lexer.prototype.token = function(src, top) {

// table (gfm)
if (top && (cap = this.rules.table.exec(src))) {
src = src.substring(cap[0].length);

item = {
type: 'table',
header: splitCells(cap[1].replace(/^ *| *\| *$/g, '')),
align: cap[2].replace(/^ *|\| *$/g, '').split(/ *\| */),
cells: cap[3].replace(/(?: *\| *)?\n$/, '').split('\n')
cells: cap[3] ? cap[3].replace(/(?: *\| *)?\n$/, '').split('\n') : []
};

for (i = 0; i < item.align.length; i++) {
if (/^ *-+: *$/.test(item.align[i])) {
item.align[i] = 'right';
} else if (/^ *:-+: *$/.test(item.align[i])) {
item.align[i] = 'center';
} else if (/^ *:-+ *$/.test(item.align[i])) {
item.align[i] = 'left';
} else {
item.align[i] = null;
if (item.header.length === item.align.length) {
src = src.substring(cap[0].length);

for (i = 0; i < item.align.length; i++) {
if (/^ *-+: *$/.test(item.align[i])) {
item.align[i] = 'right';
} else if (/^ *:-+: *$/.test(item.align[i])) {
item.align[i] = 'center';
} else if (/^ *:-+ *$/.test(item.align[i])) {
item.align[i] = 'left';
} else {
item.align[i] = null;
}
}
}

for (i = 0; i < item.cells.length; i++) {
item.cells[i] = splitCells(
item.cells[i].replace(/^ *\| *| *\| *$/g, ''));
}
for (i = 0; i < item.cells.length; i++) {
item.cells[i] = splitCells(
item.cells[i].replace(/^ *\| *| *\| *$/g, ''),
item.header.length);
}

this.tokens.push(item);
this.tokens.push(item);

continue;
continue;
}
}

// lheading
Expand Down Expand Up @@ -927,13 +932,13 @@ Renderer.prototype.paragraph = function(text) {
};

Renderer.prototype.table = function(header, body) {
if (body) body = '<tbody>' + body + '</tbody>';

return '<table>\n'
+ '<thead>\n'
+ header
+ '</thead>\n'
+ '<tbody>\n'
+ body
+ '</tbody>\n'
+ '</table>\n';
};

Expand All @@ -944,7 +949,7 @@ Renderer.prototype.tablerow = function(content) {
Renderer.prototype.tablecell = function(content, flags) {
var type = flags.header ? 'th' : 'td';
var tag = flags.align
? '<' + type + ' style="text-align:' + flags.align + '">'
? '<' + type + ' align="' + flags.align + '">'
Copy link
Contributor

@Martii Martii May 1, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Re:

Copy link
Author

Choose a reason for hiding this comment

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

This is GFM compliant, not HTML compliant. Compliance with GFM requires that we break HTML compliance.

https://github.github.com/gfm/#example-192

Copy link
Contributor

Choose a reason for hiding this comment

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

@tomtheisen

This is GFM compliant, not HTML compliant. Compliance with GFM requires that we break HTML compliance.

That's why it's in the form of a question. Good to know of a major breaking change as @styfle put it in the issue. If we continue to use this package we'll need to undo this for our sanitizer (which we do for HTML5 anyhow) as we are always striving for HTML5 compliance. Anyone using this change that has a HTML5 compliant badge will need to do this as well since it's non-standard or they'll lose their logo/rating.

Since I've been on GH just about as long as they've been around I can make an educated guess that they did this spec for older browser compatibility and also their sanitizer. So when the HTML4.x specifications drop out (presumably around 2020ish when everything hits the fan) the backward compatibility reintroduced here will not render tables correctly and it may need to be reversed. Time will tell.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe extensions can be made based on flavor in the future?? https://rawgit.com/fletcher/human-markdown-reference/master/index.html - Multimarkdown uses the inline style solution, which is not the solution used in GFM - check out the tables example and inspect element. We are definitely focused on two or three flavors to get us there.

If this gets merged now, it would be part of the 0.4, which would indicate breaking change under the zero major. If it gets merged following 0.4 then that release would be 0.5.

@styfle, @UziTech, and @davisjam Maybe a "breaking" label?? to serve as an indicator for everyone...think @styfle adding the name of 0.4.0 to the next release was a good move.

See also #1225 and #746 (We've also talked somewhere else about not making Markdown flavors but the extension.)

: '<' + type + '>';
return tag + content + '</' + type + '>\n';
};
Expand Down Expand Up @@ -1310,10 +1315,16 @@ function merge(obj) {
return obj;
}

function splitCells(tableRow) {
function splitCells(tableRow, count) {
var cells = tableRow.replace(/([^\\])\|/g, '$1 |').split(/ +\| */),
i = 0;

if (cells.length > count) {
cells.splice(count);
} else {
while (cells.length < count) cells.push('');
}

for (; i < cells.length; i++) {
cells[i] = cells[i].replace(/\\\|/g, '|');
}
Expand Down
12 changes: 6 additions & 6 deletions test/new/gfm_tables.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@
</table>
<table>
<thead>
<tr><th style="text-align:center">Header 1</th><th style="text-align:right">Header 2</th><th style="text-align:left">Header 3</th><th>Header 4</th></tr>
<tr><th align="center">Header 1</th><th align="right">Header 2</th><th align="left">Header 3</th><th>Header 4</th></tr>
</thead>
<tbody>
<tr><td style="text-align:center">Cell 1</td><td style="text-align:right">Cell 2</td><td style="text-align:left">Cell 3</td><td>Cell 4</td></tr>
<tr><td style="text-align:center">Cell 5</td><td style="text-align:right">Cell 6</td><td style="text-align:left">Cell 7</td><td>Cell 8</td></tr>
<tr><td align="center">Cell 1</td><td align="right">Cell 2</td><td align="left">Cell 3</td><td>Cell 4</td></tr>
<tr><td align="center">Cell 5</td><td align="right">Cell 6</td><td align="left">Cell 7</td><td>Cell 8</td></tr>
</tbody>
</table>
<pre><code>Test code</code></pre>
Expand All @@ -28,10 +28,10 @@
</table>
<table>
<thead>
<tr><th style="text-align:left">Header 1</th><th style="text-align:center">Header 2</th><th style="text-align:right">Header 3</th><th>Header 4</th></tr>
<tr><th align="left">Header 1</th><th align="center">Header 2</th><th align="right">Header 3</th><th>Header 4</th></tr>
</thead>
<tbody>
<tr><td style="text-align:left">Cell 1</td><td style="text-align:center">Cell 2</td><td style="text-align:right">Cell 3</td><td>Cell 4</td></tr>
<tr><td style="text-align:left"><em>Cell 5</em></td><td style="text-align:center">Cell 6</td><td style="text-align:right">Cell 7</td><td>Cell 8</td></tr>
<tr><td align="left">Cell 1</td><td align="center">Cell 2</td><td align="right">Cell 3</td><td>Cell 4</td></tr>
<tr><td align="left"><em>Cell 5</em></td><td align="center">Cell 6</td><td align="right">Cell 7</td><td>Cell 8</td></tr>
</tbody>
</table>
2 changes: 1 addition & 1 deletion test/specs/gfm/gfm-spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ var messenger = new Messenger();
describe('GFM 0.28 Tables', function() {
var section = 'Tables';

var shouldPassButFails = [192, 195, 196, 197];
var shouldPassButFails = [];

var willNotBeAttemptedByCoreTeam = [];

Expand Down
2 changes: 1 addition & 1 deletion test/specs/gfm/gfm.0.28.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
"section": "Tables",
"html": "<table>\n<thead>\n<tr>\n<th>abc</th>\n<th>def</th>\n</tr>\n</thead></table>",
"markdown": "| abc | def |\n| --- | --- |",
"example": 197
"example": 198
},
{
"section": "Task list items",
Expand Down