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

Fix GFM tables not breaking on block-level structures #1598

Merged
merged 5 commits into from Mar 6, 2020
Merged

Fix GFM tables not breaking on block-level structures #1598

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
090d11e
Fix GFM tables not breaking on block-level structures
calculuschild Feb 5, 2020
a794959
Removed console.log
calculuschild Feb 5, 2020
30ca83b
Update src/rules.js
calculuschild Feb 10, 2020
ec38dd1
Remove + on heading regex
calculuschild Feb 11, 2020
5972e73
Update fences after tables regex
calculuschild Feb 11, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 16 additions & 2 deletions src/rules.js
View file
Open in desktop
Expand Up @@ -69,7 +69,7 @@ block.html = edit(block.html, 'i')

block.paragraph = edit(block._paragraph)
.replace('hr', block.hr)
.replace('heading', ' {0,3}#{1,6} +')
.replace('heading', ' {0,3}#{1,6} ')
.replace('|lheading', '') // setex headings don't interrupt commonmark paragraphs
.replace('blockquote', ' {0,3}>')
.replace('fences', ' {0,3}(?:`{3,}|~{3,})[^`\\n]*\\n')
Expand All @@ -94,9 +94,23 @@ block.normal = merge({}, block);

block.gfm = merge({}, block.normal, {
nptable: /^ *([^|\n ].*\|.*)\n *([-:]+ *\|[-| :]*)(?:\n((?:.*[^>\n ].*(?:\n|$))*)\n*|$)/,
table: /^ *\|(.+)\n *\|?( *[-:]+[-| :]*)(?:\n((?: *[^>\n ].*(?:\n|$))*)\n*|$)/
table: '^ *\\|(.+)\\n' // Header
+ ' *\\|?( *[-:]+[-| :]*)' // Align
+ '(?:\\n((?:(?!^|>|\\n| |hr|heading|lheading|code|fences|list|html).*(?:\\n|$))*)\\n*|$)' // Cells
});

block.gfm.table = edit(block.gfm.table)
.replace('hr', block.hr)
.replace('heading', ' {0,3}#{1,6} ')
.replace('lheading', '([^\\n]+)\\n {0,3}(=+|-+) *(?:\\n+|$)')
.replace('blockquote', ' {0,3}>')
.replace('code', ' {4}[^\\n]')
.replace('fences', ' {0,3}(?:`{3,}(?=[^`\\n]*\\n)|~{3,})[^\\n]*\\n')
.replace('list', ' {0,3}(?:[*+-]|1[.)]) ') // only lists starting from 1 can interrupt
.replace('html', '</?(?:tag)(?: +|\\n|/?>)|<(?:script|pre|style|!--)')
.replace('tag', block._tag) // pars can be interrupted by type (6) html blocks
.getRegex();

/**
* Pedantic grammar (original John Gruber's loose markdown specification)
*/
Expand Down
21 changes: 21 additions & 0 deletions test/specs/new/code_following_table.html
View file
Open in desktop
@@ -0,0 +1,21 @@
<table>
<thead>
<tr>
<th>abc</th>
<th>def</th>
</tr>
</thead>
<tbody>
<tr>
<td>bar</td>
<td>foo</td>
</tr>
<tr>
<td>baz</td>
<td>boo</td>
</tr>
</tbody>
</table>
<pre><code>a simple
*indented* code block
</code></pre>
6 changes: 6 additions & 0 deletions test/specs/new/code_following_table.md
View file
Open in desktop
@@ -0,0 +1,6 @@
| abc | def |
| --- | --- |
| bar | foo |
| baz | boo |
a simple
*indented* code block
19 changes: 19 additions & 0 deletions test/specs/new/fences_following_table.html
View file
Open in desktop
@@ -0,0 +1,19 @@
<table>
<thead>
<tr>
<th>abc</th>
<th>def</th>
</tr>
</thead>
<tbody>
<tr>
<td>bar</td>
<td>foo</td>
</tr>
<tr>
<td>baz</td>
<td>boo</td>
</tr>
</tbody>
</table>
<pre><code>foobar()</code></pre>
7 changes: 7 additions & 0 deletions test/specs/new/fences_following_table.md
View file
Open in desktop
@@ -0,0 +1,7 @@
| abc | def |
| --- | --- |
| bar | foo |
| baz | boo |
```
foobar()
```
19 changes: 19 additions & 0 deletions test/specs/new/heading_following_table.html
View file
Open in desktop
@@ -0,0 +1,19 @@
<table>
<thead>
<tr>
<th>abc</th>
<th>def</th>
</tr>
</thead>
<tbody>
<tr>
<td>bar</td>
<td>foo</td>
</tr>
<tr>
<td>baz</td>
<td>boo</td>
</tr>
</tbody>
</table>
<h1 id="title">title</h1>
5 changes: 5 additions & 0 deletions test/specs/new/heading_following_table.md
View file
Open in desktop
@@ -0,0 +1,5 @@
| abc | def |
| --- | --- |
| bar | foo |
| baz | boo |
# title
19 changes: 19 additions & 0 deletions test/specs/new/hr_following_tables.html
View file
Open in desktop
@@ -0,0 +1,19 @@
<table>
<thead>
<tr>
<th>abc</th>
<th>def</th>
</tr>
</thead>
<tbody>
<tr>
<td>bar</td>
<td>foo</td>
</tr>
<tr>
<td>baz</td>
<td>boo</td>
</tr>
</tbody>
</table>
<hr>
5 changes: 5 additions & 0 deletions test/specs/new/hr_following_tables.md
View file
Open in desktop
@@ -0,0 +1,5 @@
| abc | def |
| --- | --- |
| bar | foo |
| baz | boo |
___
19 changes: 19 additions & 0 deletions test/specs/new/html_following_table.html
View file
Open in desktop
@@ -0,0 +1,19 @@
<table>
<thead>
<tr>
<th>abc</th>
<th>def</th>
</tr>
</thead>
<tbody>
<tr>
<td>bar</td>
<td>foo</td>
</tr>
<tr>
<td>baz</td>
<td>boo</td>
</tr>
</tbody>
</table>
<div>Some HTML</div>
5 changes: 5 additions & 0 deletions test/specs/new/html_following_table.md
View file
Open in desktop
@@ -0,0 +1,5 @@
| abc | def |
| --- | --- |
| bar | foo |
| baz | boo |
<div>Some HTML</div>
22 changes: 22 additions & 0 deletions test/specs/new/inlinecode_following_tables.html
View file
Open in desktop
@@ -0,0 +1,22 @@
<table>
<thead>
<tr>
<th>abc</th>
<th>def</th>
</tr>
</thead>
<tbody>
<tr>
<td>bar</td>
<td>foo</td>
</tr>
<tr>
<td>baz</td>
<td>boo</td>
</tr>
<tr>
<td><code>hello</code></td>
<td></td>
</tr>
</tbody>
</table>
5 changes: 5 additions & 0 deletions test/specs/new/inlinecode_following_tables.md
View file
Open in desktop
@@ -0,0 +1,5 @@
| abc | def |
| --- | --- |
| bar | foo |
| baz | boo |
`hello`
19 changes: 19 additions & 0 deletions test/specs/new/lheading_following_table.html
View file
Open in desktop
@@ -0,0 +1,19 @@
<table>
<thead>
<tr>
<th>abc</th>
<th>def</th>
</tr>
</thead>
<tbody>
<tr>
<td>bar</td>
<td>foo</td>
</tr>
<tr>
<td>baz</td>
<td>boo</td>
</tr>
</tbody>
</table>
<h1 id="title">title</h1>
Copy link
Member

Choose a reason for hiding this comment

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

GitHub seems to render this one differently

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmm.... The spec says "The table is broken at the first empty line, or beginning of another block-level structure", so that would seem to break their rule, as the setext headings fall under the "Leaf Block" category.

At that point do you follow the spec, or do you follow GitHub's implementation of the spec? Either way, it's a simple change of just removing the associated lines that check for lheading.

Copy link
Member

Choose a reason for hiding this comment

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

Good point, your test seems correct. I also double checked to make sure our GFM spec tests included 201 [extension] Tables and it is passing.

Copy link
Member

@UziTech UziTech Feb 10, 2020 •
edited

Choose a reason for hiding this comment

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

from the spec:

In general, a setext heading need not be preceded or followed by a blank line. However, it cannot interrupt a paragraph, so when a setext heading comes after a paragraph, a blank line is needed between them.

Maybe since it cannot interrupt a paragraph it also shouldn't interrupt a table? I'm leaning toward following the implementation and posing this as an issue to GitHub to see what they have to say. github/cmark-gfm#179

We could leave the regex in and just comment it out with a note so we can change it easily in the future.

6 changes: 6 additions & 0 deletions test/specs/new/lheading_following_table.md
View file
Open in desktop
@@ -0,0 +1,6 @@
| abc | def |
| --- | --- |
| bar | foo |
| baz | boo |
title
=====
23 changes: 23 additions & 0 deletions test/specs/new/list_following_table.html
View file
Open in desktop
@@ -0,0 +1,23 @@
<table>
<thead>
<tr>
<th>abc</th>
<th>def</th>
</tr>
</thead>
<tbody>
<tr>
<td>bar</td>
<td>foo</td>
</tr>
<tr>
<td>baz</td>
<td>boo</td>
</tr>
</tbody>
</table>
<ul>
<li>foo</li>
<li>bar</li>
<li>baz</li>
</ul>
7 changes: 7 additions & 0 deletions test/specs/new/list_following_table.md
View file
Open in desktop
@@ -0,0 +1,7 @@
| abc | def |
| --- | --- |
| bar | foo |
| baz | boo |
- foo
- bar
- baz
22 changes: 22 additions & 0 deletions test/specs/new/strong_following_tables.html
View file
Open in desktop
@@ -0,0 +1,22 @@
<table>
<thead>
<tr>
<th>abc</th>
<th>def</th>
</tr>
</thead>
<tbody>
<tr>
<td>bar</td>
<td>foo</td>
</tr>
<tr>
<td>baz</td>
<td>boo</td>
</tr>
<tr>
<td><strong>strong</strong></td>
<td></td>
</tr>
</tbody>
</table>
5 changes: 5 additions & 0 deletions test/specs/new/strong_following_tables.md
View file
Open in desktop
@@ -0,0 +1,5 @@
| abc | def |
| --- | --- |
| bar | foo |
| baz | boo |
**strong**
22 changes: 22 additions & 0 deletions test/specs/new/text_following_tables.html
View file
Open in desktop
@@ -0,0 +1,22 @@
<table>
<thead>
<tr>
<th>abc</th>
<th>def</th>
</tr>
</thead>
<tbody>
<tr>
<td>bar</td>
<td>foo</td>
</tr>
<tr>
<td>baz</td>
<td>boo</td>
</tr>
<tr>
<td>hello</td>
<td></td>
</tr>
</tbody>
</table>
5 changes: 5 additions & 0 deletions test/specs/new/text_following_tables.md
View file
Open in desktop
@@ -0,0 +1,5 @@
| abc | def |
| --- | --- |
| bar | foo |
| baz | boo |
hello