Skip to content
This repository has been archived by the owner on Mar 15, 2024. It is now read-only.

Commit

Permalink
Fix GFM table parser to handle dangling pipe correctly (fixes commonm…
Browse files Browse the repository at this point in the history
  • Loading branch information
robinst committed Oct 18, 2022
1 parent 0bc0b2d commit 7584f00
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 19 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
This project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html),
with the exception that 0.x versions can break between minor versions.

## [Unreleased]
### Fixed
- A single pipe (optional whitespace) now ends a table instead of crashing or
being treated as an empty row, for consistency with GitHub (#255).

## [0.19.0] - 2022-06-02
### Added
- YAML front matter extension: Limited support for single and double
Expand Down Expand Up @@ -362,6 +367,7 @@ Initial release of commonmark-java, a port of commonmark.js with extensions
for autolinking URLs, GitHub flavored strikethrough and tables.
[Unreleased]: https://github.com/commonmark/commonmark-java/compare/commonmark-parent-0.19.0...HEAD
[0.19.0]: https://github.com/commonmark/commonmark-java/compare/commonmark-parent-0.18.2...commonmark-parent-0.19.0
[0.18.2]: https://github.com/commonmark/commonmark-java/compare/commonmark-parent-0.18.1...commonmark-parent-0.18.2
[0.18.1]: https://github.com/commonmark/commonmark-java/compare/commonmark-parent-0.18.0...commonmark-parent-0.18.1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,16 @@ public class TableBlockParser extends AbstractBlockParser {
private final List<SourceLine> rowLines = new ArrayList<>();
private final List<TableCell.Alignment> columns;

private boolean canHaveLazyContinuationLines = true;

private TableBlockParser(List<TableCell.Alignment> columns, SourceLine headerLine) {
this.columns = columns;
this.rowLines.add(headerLine);
}

@Override
public boolean canHaveLazyContinuationLines() {
return true;
return canHaveLazyContinuationLines;
}

@Override
Expand All @@ -36,7 +38,17 @@ public Block getBlock() {

@Override
public BlockContinue tryContinue(ParserState state) {
if (Parsing.find('|', state.getLine().getContent(), 0) != -1) {
CharSequence content = state.getLine().getContent();
int pipe = Parsing.find('|', content, state.getNextNonSpaceIndex());
if (pipe != -1) {
if (pipe == state.getNextNonSpaceIndex()) {
// If we *only* have a pipe character (and whitespace), that is not a valid table row and ends the table.
if (Parsing.skipSpaceTab(content, pipe + 1, content.length()) == content.length()) {
// We also don't want the pipe to be added via lazy continuation.
canHaveLazyContinuationLines = false;
return BlockContinue.none();
}
}
return BlockContinue.atIndex(state.getIndex());
} else {
return BlockContinue.none();
Expand Down Expand Up @@ -128,7 +140,7 @@ private static List<SourceLine> split(SourceLine line) {
// This row has leading/trailing pipes - skip the leading pipe
cellStart = nonSpace + 1;
// Strip whitespace from the end but not the pipe or we could miss an empty ("||") cell
int nonSpaceEnd = Parsing.skipSpaceTabBackwards(row, row.length() - 1, cellStart + 1);
int nonSpaceEnd = Parsing.skipSpaceTabBackwards(row, row.length() - 1, cellStart);
cellEnd = nonSpaceEnd + 1;
}
List<SourceLine> cells = new ArrayList<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -261,21 +261,21 @@ public void pipesOnOutsideZeroLengthHeaders() {
"-|-------------|-\n" +
"1| 2 |3",
"<table>\n" +
"<thead>\n" +
"<tr>\n" +
"<th></th>\n" +
"<th>center header</th>\n" +
"<th></th>\n" +
"</tr>\n" +
"</thead>\n" +
"<tbody>\n" +
"<tr>\n" +
"<td>1</td>\n" +
"<td>2</td>\n" +
"<td>3</td>\n" +
"</tr>\n" +
"</tbody>\n" +
"</table>\n");
"<thead>\n" +
"<tr>\n" +
"<th></th>\n" +
"<th>center header</th>\n" +
"<th></th>\n" +
"</tr>\n" +
"</thead>\n" +
"<tbody>\n" +
"<tr>\n" +
"<td>1</td>\n" +
"<td>2</td>\n" +
"<td>3</td>\n" +
"</tr>\n" +
"</tbody>\n" +
"</table>\n");
}

@Test
Expand Down Expand Up @@ -664,6 +664,47 @@ public void issue142() {
"</table>\n");
}

@Test
public void danglingPipe() {
assertRendering("Abc|Def\n" +
"---|---\n" +
"1|2\n" +
"|", "<table>\n" +
"<thead>\n" +
"<tr>\n" +
"<th>Abc</th>\n" +
"<th>Def</th>\n" +
"</tr>\n" +
"</thead>\n" +
"<tbody>\n" +
"<tr>\n" +
"<td>1</td>\n" +
"<td>2</td>\n" +
"</tr>\n" +
"</tbody>\n" +
"</table>\n" +
"<p>|</p>\n");

assertRendering("Abc|Def\n" +
"---|---\n" +
"1|2\n" +
" | ", "<table>\n" +
"<thead>\n" +
"<tr>\n" +
"<th>Abc</th>\n" +
"<th>Def</th>\n" +
"</tr>\n" +
"</thead>\n" +
"<tbody>\n" +
"<tr>\n" +
"<td>1</td>\n" +
"<td>2</td>\n" +
"</tr>\n" +
"</tbody>\n" +
"</table>\n" +
"<p>|</p>\n");
}

@Test
public void attributeProviderIsApplied() {
AttributeProviderFactory factory = new AttributeProviderFactory() {
Expand Down Expand Up @@ -718,7 +759,7 @@ public void sourceSpans() {

TableBlock block = (TableBlock) document.getFirstChild();
assertEquals(Arrays.asList(SourceSpan.of(0, 0, 7), SourceSpan.of(1, 0, 7),
SourceSpan.of(2, 0, 4), SourceSpan.of(3, 0, 8), SourceSpan.of(4, 0, 3)),
SourceSpan.of(2, 0, 4), SourceSpan.of(3, 0, 8), SourceSpan.of(4, 0, 3)),
block.getSourceSpans());

TableHead head = (TableHead) block.getFirstChild();
Expand Down

0 comments on commit 7584f00

Please sign in to comment.