Skip to content

[WIP] Improve Markdown Table formatting #4795

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

Draft
SougandhS wants to merge 1 commit into
base: master
Choose a base branch
from
+196 −84
Draft

[WIP] Improve Markdown Table formatting #4795

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
55 changes: 0 additions & 55 deletions ....tests.model/src/org/eclipse/jdt/core/tests/formatter/FormatterMarkdownCommentsTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -467,60 +467,6 @@ class m22 {
formatSource(input, expected);
}

public void testMarkdownDoNotBreakTableOutsideClass() throws JavaModelException {
setComplianceLevel(CompilerOptions.VERSION_23);
String input = """
/// | Latin | Greek |
/// |-------|-------|
/// | a | alpha |
/// | b | beta |
/// | c | gamma |
class Main {}
""";
String expected = """
/// | Latin | Greek |
/// |-------|-------|
/// | a | alpha |
/// | b | beta |
/// | c | gamma |
class Main {
}
""";
formatSource(input, expected);
}
public void testMarkdownDoNotBreakMultipleTableInsideClass() throws JavaModelException {
setComplianceLevel(CompilerOptions.VERSION_23);
String input = """
class Main {
/// | Latin | Greek |
/// |-------|-------|
/// | a | alpha |
///
/// Hello Eclipse
///
/// | Latin | Greek |
/// |-------|-------|
/// | a | alpha |
public void sample(String param1) {}}
""";
String expected = """
class Main {
/// | Latin | Greek |
/// |-------|-------|
/// | a | alpha |
///
/// Hello Eclipse
///
/// | Latin | Greek |
/// |-------|-------|
/// | a | alpha |
public void sample(String param1) {
}
}
""";
formatSource(input, expected);
}

public void testMarkdownDoNotBreakTwoListOfSameLevel() throws JavaModelException {
setComplianceLevel(CompilerOptions.VERSION_23);
String input = """
Expand Down Expand Up @@ -880,5 +826,4 @@ class Mark61 {
""";
formatSource(input, expected);
}

}
178 changes: 151 additions & 27 deletions org.eclipse.jdt.core/formatter/org/eclipse/jdt/internal/formatter/CommentsPreparator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,8 @@ public class CommentsPreparator extends ASTVisitor {
private final static Pattern MARKDOWN_HEADINGS_PATTERN_1 = Pattern.compile("(?:(?<=^)|(?<=///[ \\t]*))(#{1,6})([ \\t]+)([^\\r\\n]*)"); //$NON-NLS-1$
private final static Pattern MARKDOWN_HEADINGS_PATTERN_2 = Pattern.compile("(?:^|(?<=///[ \\t]+))[ \\t]*([=-])\\1*[ \\t]*(?=\\r?\\n|$)"); //$NON-NLS-1$
private final static Pattern MARKDOWN_FENCE_PATTERN = Pattern.compile("(`{3,}|~{3,})(.*)"); //$NON-NLS-1$
private final static Pattern MARKDOWN_TABLE_START = Pattern.compile("(?m)(?<=^[ \\t]*)\\|"); //$NON-NLS-1$
private final static Pattern MARKDOWN_TABLE_END = Pattern.compile("(?m)\\|(?!.*\\|)"); //$NON-NLS-1$
private final static Pattern MARKDOWN_TABLE_COLUMN_SEP = Pattern.compile("\\||(?<=\\|)\\s*-+\\s*(?=\\|)"); //$NON-NLS-1$
private final static Pattern MARKDOWN_TABLE_COLUMN_VALIDATOR = Pattern.compile("^[ :\\-|]+$"); //$NON-NLS-1$

// Param tags list copied from IJavaDocTagConstants in legacy formatter for compatibility.
// There were the following comments:
Expand Down Expand Up @@ -134,6 +134,7 @@ public class CommentsPreparator extends ASTVisitor {
private final ArrayList<Integer> commonAttributeAnnotations = new ArrayList<>();
private DefaultCodeFormatter preTagCodeFormatter;
private DefaultCodeFormatter snippetCodeFormatter;
private String markdownTablePipe = "|"; //$NON-NLS-1$

public CommentsPreparator(TokenManager tm, DefaultCodeFormatterOptions options, String sourceLevel) {
this.tm = tm;
Expand Down Expand Up @@ -998,39 +999,162 @@ && formatCode(codeBlockStartIndex, codeBlockEndIndex + 1, false, true)) {
}

private void handleMarkdownTable(List<ASTNode> fragments) {
int tableStartIndex = -1;
int tableLastIndex = -1;
boolean columnUnderlineFound = false;
Matcher matcher;
for (Object fragment : fragments) {
if (fragment instanceof TextElement textElement) {
ASTNode columnHeader = null;
boolean hasFormattedColumnHeader = false;
int maxRowDataLen = 0;
List<Token> columnHeaderTokens = new ArrayList<>();
List<Token> columnSeperatorTokens = new ArrayList<>();
for (int i = 0; i < fragments.size(); i++) {
if (fragments.get(i) instanceof TextElement textElement) {
String textContent = textElement.getText();
matcher = MARKDOWN_TABLE_START.matcher(textContent);
if (matcher.find()) {
if (tableStartIndex == -1) {
int startPos = matcher.start() + textElement.getStartPosition();
tableStartIndex = tokenStartingAt(startPos);
} else if (tableStartIndex != -1 && !columnUnderlineFound) {
boolean foundStart = textContent.contains("|-"); //$NON-NLS-1$
boolean foundEnd = textContent.contains("-|"); //$NON-NLS-1$
if (foundStart && foundEnd) {
columnUnderlineFound = true;
if (columnSeperatorTokens.isEmpty()) {
matcher = MARKDOWN_TABLE_COLUMN_VALIDATOR.matcher(textContent);
if (matcher.find()) {
columnHeader = fragments.get(i - 1);
// find the most lengthy cell data
for (int rowIndex = i; rowIndex < fragments.size(); rowIndex++) {
TextElement element = (TextElement) fragments.get(rowIndex);
String rowData = element.getText();
int maxRow = Arrays.stream(rowData.split("\\|")).map(e -> e.trim()) //$NON-NLS-1$
.filter(s -> !s.isEmpty()).mapToInt(String::length).max().orElse(0);
maxRowDataLen = Math.max(maxRow, maxRowDataLen);
}
String headData = ((TextElement) columnHeader).getText();
int maxHead = Arrays.stream(headData.split("\\|")).map(e -> e.trim()) //$NON-NLS-1$
.filter(s -> !s.isEmpty()).mapToInt(String::length).max().orElse(0);
maxRowDataLen = maxHead == maxRowDataLen ? maxRowDataLen + 2
: Math.max(maxHead, maxRowDataLen + 2);
matcher = MARKDOWN_TABLE_COLUMN_SEP.matcher(textContent);
while (matcher.find()) {
int startPos = matcher.start() + fragments.get(i).getStartPosition();
int tokenIndex = this.ctm.findIndex(startPos, ANY, true);
Token columnSeperatorToken = this.ctm.get(tokenIndex);
columnSeperatorToken.setWrapPolicy(WrapPolicy.DISABLE_WRAP);
columnSeperatorToken.setColumnSeparator(true);
columnSeperatorTokens.add(columnSeperatorToken);
}
if (!columnSeperatorTokens.isEmpty()) {
columnSeperatorTokens.get(columnSeperatorTokens.size() - 1).breakAfter();
}
} else if (columnUnderlineFound) {
matcher = MARKDOWN_TABLE_END.matcher(textContent);
matcher.find(); // find the last one
}
} else if (!columnSeperatorTokens.isEmpty() && !hasFormattedColumnHeader) {
String columnString = ((TextElement) columnHeader).getText();
Pattern p = Pattern.compile("\\||(?<=\\||\\s)[^|\\s]+(?=\\s|\\||$)"); //$NON-NLS-1$
matcher = p.matcher(columnString);
while (matcher.find()) {
int startPos = matcher.start() + columnHeader.getStartPosition();
int tokenIndex = this.ctm.findIndex(startPos, ANY, true);
Token columnToken = this.ctm.get(tokenIndex);
columnToken.setWrapPolicy(WrapPolicy.DISABLE_WRAP);
String content = this.ctm.toString(columnToken);
if (!content.equals(this.markdownTablePipe)) {
columnToken.setMarkdownColumnHeader(true);
}
columnHeaderTokens.add(columnToken);
}
columnHeaderTokens.get(columnSeperatorTokens.size() - 1).breakAfter();
formatMarkdownTableHeader(columnHeaderTokens, columnSeperatorTokens, maxRowDataLen);
hasFormattedColumnHeader = true;
}
if (hasFormattedColumnHeader) {
List<Token> rowTokens = new ArrayList<>();
String rowSet = textContent;
Pattern p = Pattern.compile("\\||(?<=\\||^)[^|]+(?=\\||$)"); //$NON-NLS-1$
matcher = p.matcher(rowSet);
while (matcher.find()) {
int startPos = matcher.start() + textElement.getStartPosition();
tableLastIndex = tokenStartingAt(startPos);
int tokenIndex = this.ctm.findIndex(startPos, ANY, true);
if (tokenIndex >= this.ctm.size()) {
continue;
}
Token rowToken = this.ctm.get(tokenIndex);
rowToken.setWrapPolicy(WrapPolicy.DISABLE_WRAP);
rowTokens.add(rowToken);
}
rowTokens.get(rowTokens.size() - 1).breakAfter();
formatMarkdownTableRow(rowTokens, maxRowDataLen);
}
}
}
}

private void formatMarkdownTableHeader(List<Token> columnHeaderTokens, List<Token> columnSeperatorTokens,
int maxRowDataLen) {
Token cellStart = null;
Token cellEnd = null;
List<Token> cellContent = new ArrayList<>();
int cel = 0;
for (int j = 0; j < columnHeaderTokens.size(); j++) {
Token columnToken = columnHeaderTokens.get(j);
;
if (!columnToken.isMarkdownColumnHeader() && cellStart == null) {
cellStart = columnToken;
} else if (!columnToken.isMarkdownColumnHeader() && cellEnd == null) {
cellEnd = columnToken;
} else {
cellContent.add(columnToken);
}
if (cellStart != null && cellEnd != null) {
cellStart.spaceAfter();
cellContent.get(cellContent.size() - 1).clearSpaceAfter();
cellContent.get(0).clearSpaceBefore();
int contentLength = cellContent.stream().mapToInt(t -> this.tm.toString(t).length()).sum();
contentLength = cellContent.size() > 1 ? contentLength + cellContent.size() - 1 : contentLength;
int newLen = (maxRowDataLen - contentLength);
int prev = cellStart.getAlign() == 0 ? 4 : cellStart.getAlign();
Token previous = cellContent.get(0);
if (cellContent.size() == 1) {
cellContent.get(0).setAlign(prev + (newLen / 2) + 1);
} else {
tableStartIndex = -1;
tableLastIndex = -1;
columnUnderlineFound = false;
cellContent.get(0).setAlign(prev + (newLen / 2) + 1);
if (cellContent.size() > 1) {
for (int i = 1; i < cellContent.size(); i++) {
Token tmp = cellContent.get(i);
tmp.setAlign(previous.getAlign());
previous = tmp;
cel++;
contentLength = this.tm.toString(previous).length();
}
}
}
cellEnd.setAlign(previous.getAlign() + contentLength + ((newLen / 2)));
cellStart = cellEnd;
cellEnd = null;
int toBeAdded = this.tm.toString(columnSeperatorTokens.get(j - 1 - cel)).length();
int padding = contentLength % 2 == 0 ? 0 : -1;
columnSeperatorTokens.get(j - 1 - cel).setMarkdownColumnLength(maxRowDataLen - toBeAdded + padding);
cellContent.clear();
}
}
}

private void formatMarkdownTableRow(List<Token> rowTokens, int maxRowDataLen) {
Token cellStart = null;
Token cellEnd = null;
Token cellContent = null;
for (int j = 0; j < rowTokens.size(); j++) {
Token columnToken = rowTokens.get(j);
String colVal = this.tm.toString(columnToken);
if (colVal.equals(this.markdownTablePipe) && cellStart == null) {
cellStart = columnToken;
} else if (colVal.equals(this.markdownTablePipe) && cellEnd == null) {
cellEnd = columnToken;
} else {
cellContent = columnToken;
}
if (tableStartIndex != -1 && tableLastIndex != -1) {
// TODO fix column alignment and format cells
disableFormattingExclusively(tableStartIndex, tableLastIndex);
if (cellStart != null && cellEnd != null) {
cellContent.clearSpaceAfter();
cellContent.clearSpaceBefore();
int contentLength = this.tm.toString(cellContent).length();
int newLen = (maxRowDataLen - contentLength);
cellStart.spaceAfter();
int prev = cellStart.getAlign() == 0 ? 0 : cellStart.getAlign();
int padding = contentLength == maxRowDataLen ? 0 : 1;
cellContent.setAlign(prev + (newLen / 2) + padding);
cellEnd.setAlign(cellContent.getAlign() + contentLength + ((newLen / 2)));
cellStart = cellEnd;
cellEnd = null;
}
}
}
Expand Down
19 changes: 17 additions & 2 deletions org.eclipse.jdt.core/formatter/org/eclipse/jdt/internal/formatter/TextEditsBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,14 +304,29 @@ public static void appendIndentationString(StringBuilder target, int tabChar, in
}

private boolean bufferAlign(Token token, int index) {
boolean mdColNo = false;
if(token.tokenType==TokenNameCOMMENT_MARKDOWN) {
mdColNo = token.isColumnSeparator();
int mdColLen = token.getMarkdownColumnLength();

if(mdColLen>0) {
String col = this.tm.toString(token);
if (col.contains("-")) {
for (int i = 0; i < mdColLen; i++) {
this.buffer.append("-");
}
}
}
}
int align = token.getAlign();

int alignmentChar = this.alignChar;
if (align == 0 && getLineBreaksBefore() == 0 && this.parent != null) {
align = token.getIndent();
token.setAlign(align);
alignmentChar = DefaultCodeFormatterOptions.SPACE;
}
if (align == 0)
if (align == 0 && !mdColNo)
return false;

int currentPositionInLine = 0;
Expand All @@ -322,7 +337,7 @@ private boolean bufferAlign(Token token, int index) {
currentPositionInLine = this.tm.getPositionInLine(index - 1);
currentPositionInLine += this.tm.getLength(this.tm.get(index - 1), currentPositionInLine);
}
if (currentPositionInLine >= align)
if (currentPositionInLine >= align && !mdColNo)
return false;

final int tabSize = this.options.tab_size;
Expand Down
28 changes: 28 additions & 0 deletions org.eclipse.jdt.core/formatter/org/eclipse/jdt/internal/formatter/Token.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,10 @@ public WrapPolicy(WrapMode wrapMode, int wrapParentIndex, int extraIndent) {

private List<Token> internalStructure;

private boolean columnSeparator;
private int markdownColumnLength;
private boolean isMarkdownColumnHeader;

public Token(int sourceStart, int sourceEnd, TerminalToken tokenType) {
assert sourceStart <= sourceEnd;
this.originalStart = sourceStart;
Expand Down Expand Up @@ -339,6 +343,30 @@ public int countChars() {
return this.originalEnd - this.originalStart + 1;
}

public boolean isColumnSeparator() {
return this.columnSeparator;
}

public void setColumnSeparator(boolean columnSeparator) {
this.columnSeparator = columnSeparator;
}

public int getMarkdownColumnLength() {
return this.markdownColumnLength;
}

public void setMarkdownColumnLength(int markdownColumnLength) {
this.markdownColumnLength = markdownColumnLength;
}

public boolean isMarkdownColumnHeader() {
return this.isMarkdownColumnHeader;
}

public void setMarkdownColumnHeader(boolean isMarkdownColumnHeader) {
this.isMarkdownColumnHeader = isMarkdownColumnHeader;
}

/*
* Conceptually, Token abstracts away from the source so it doesn't need to know how
* the source looks like. However, it's useful to see the actual token contents while debugging.
Expand Down
Loading