Add table parsing support in EntitiesHelper and TableSpan class#339
Add table parsing support in EntitiesHelper and TableSpan class#339risin42 merged 10 commits intorisin42:devfrom
Conversation
WalkthroughAdds GFM-style table detection and rendering: new TableSpan renders tables; EntitiesHelper gains table parsing and a parseMarkdown overload; MessageObject exposes getMessageDisplayText(); ChatMessageCell and MediaDataController use the new display/text flow. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Chat UI
participant MsgObj as MessageObject
participant Helper as EntitiesHelper
participant Span as TableSpan
participant Cell as ChatMessageCell
UI->>MsgObj: request layout/render
MsgObj->>MsgObj: getMessageDisplayText()
alt Neko parser selected and messageText != null
MsgObj->>Helper: parseTables(messageText)
Helper->>Helper: match TABLE_BLOCK_PATTERN, parseTableRows()
alt table parsed
Helper->>Span: create TableSpan(rows)
Helper-->>MsgObj: return CharSequence with TableSpan placeholders
else no table parsed
Helper-->>MsgObj: return original messageText
end
else parser not Neko or messageText null
MsgObj-->>UI: return original messageText
end
UI->>Cell: render using returned displayText
Cell->>Cell: extract spans/clickable links from displayText (if Spanned)
Cell->>Span: measurement & draw invoked during layout/draw
Span-->>Cell: rendered table content embedded in layout
Cell-->>UI: final rendered message
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5d969812-ca0a-4f1e-939f-f44bfc981de8
📒 Files selected for processing (5)
TMessagesProj/src/main/java/org/telegram/messenger/MessageObject.javaTMessagesProj/src/main/java/tw/nekomimi/nekogram/helpers/EntitiesHelper.javaTMessagesProj/src/main/java/tw/nekomimi/nekogram/helpers/TableSpan.javapatch_entities.patchpatch_table.py
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
TMessagesProj/src/main/java/tw/nekomimi/nekogram/helpers/EntitiesHelper.java (2)
24-26:⚠️ Potential issue | 🟠 MajorStill misses valid GFM tables without outer pipes.
Line 25 still requires every row to start and end with
|, soa | b/---|---tables never reachparseTableRows(...)even though Lines 178-179 already normalize optional outer pipes.
154-157:⚠️ Potential issue | 🟠 MajorPreserve the block newlines around the placeholder.
Line 155 collapses the whole block to one inline placeholder. If the match consumed the trailing newline, the following paragraph can render on the same logical line as the table instead of below it.
Suggested fix
- builder.replace(start, end, "\u200B"); + builder.replace(start, end, "\n\u200B\n"); TableSpan span = new TableSpan(parsed, originalMarkdown); - builder.setSpan(span, start, start + 1, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE); + builder.setSpan(span, start + 1, start + 2, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE);TMessagesProj/src/main/java/tw/nekomimi/nekogram/helpers/TableSpan.java (1)
72-80:⚠️ Potential issue | 🟠 MajorClamp computed column widths before measuring layouts.
When
colCountis large, Line 75 can makeavgWidthzero or negative, which collapses cell positions and overlaps the grid. At the same time Line 80 always reports the fullmaxTableWidth, so narrow tables reserve much more inline width than they use. Clamp each column width and derivemTableWidthfrom the actual total.Suggested fix
- int maxTableWidth = AndroidUtilities.displaySize.x - dp(100); - int availableSpace = maxTableWidth - dp(CELL_PADDING) * 2 * colCount - dp(BORDER_WIDTH) * (colCount + 1); - - int avgWidth = availableSpace / colCount; - for (int i = 0; i < colCount; i++) { - columnWidths[i] = avgWidth; - } - - mTableWidth = maxTableWidth; + int maxTableWidth = AndroidUtilities.displaySize.x - dp(100); + int padding = dp(CELL_PADDING); + int border = dp(BORDER_WIDTH); + int availableSpace = Math.max(0, maxTableWidth - padding * 2 * colCount - border * (colCount + 1)); + int avgWidth = Math.max(dp(MIN_COLUMN_WIDTH), availableSpace / Math.max(1, colCount)); + int totalWidth = border; + for (int i = 0; i < colCount; i++) { + columnWidths[i] = Math.min(dp(MAX_COLUMN_WIDTH), avgWidth); + totalWidth += columnWidths[i] + padding * 2 + border; + } + + mTableWidth = Math.min(maxTableWidth, totalWidth);
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e3ff1b8d-f7d7-4f0e-ad2f-44fe1ad2dde9
📒 Files selected for processing (2)
TMessagesProj/src/main/java/tw/nekomimi/nekogram/helpers/EntitiesHelper.javaTMessagesProj/src/main/java/tw/nekomimi/nekogram/helpers/TableSpan.java
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (2)
TMessagesProj/src/main/java/tw/nekomimi/nekogram/helpers/EntitiesHelper.java (2)
131-135:⚠️ Potential issue | 🟠 MajorSkip table detection inside mono/code spans.
The pattern matcher scans raw text without checking whether matched regions overlap with existing
TextStyleSpan.isMono()orCodeHighlighting.Spanspans. A code block containing table-like content (e.g., a markdown tutorial) will be incorrectly replaced with aTableSpan.Apply the same overlap check used in the
PATTERNS[]loop (lines 60-78) before recording table matches.Suggested approach
+ // Parse GFM tables and replace with TableSpan using placeholder char + public static CharSequence parseTables(CharSequence text) { + if (text == null) return text; + + Spannable spannable = text instanceof Spannable + ? (Spannable) text + : Spannable.Factory.getInstance().newSpannable(text); + // Collect positions manually first var positions = new ArrayList<int[]>(); var m = TABLE_BLOCK_PATTERN.matcher(text); - while (m.find()) { - positions.add(new int[]{m.start(), m.end()}); + find: + while (m.find()) { + int start = m.start(); + int end = m.end(); + + // Skip if overlapping with mono spans + var textStyleSpans = spannable.getSpans(start, end, TextStyleSpan.class); + for (var span : textStyleSpans) { + if (span.isMono()) { + int spanStart = spannable.getSpanStart(span); + int spanEnd = spannable.getSpanEnd(span); + if (spanStart <= start && spanEnd >= end) { + continue find; + } + } + } + + // Skip if overlapping with code highlighting spans + var codeSpans = spannable.getSpans(start, end, CodeHighlighting.Span.class); + for (var span : codeSpans) { + int spanStart = spannable.getSpanStart(span); + int spanEnd = spannable.getSpanEnd(span); + if (spanStart <= start && spanEnd >= end) { + continue find; + } + } + + positions.add(new int[]{start, end}); }
179-181:⚠️ Potential issue | 🟠 MajorHandle escaped pipes when splitting cells.
line.split("\\|")treats\|as a real column separator. Valid table content likea \| bwill inflate the column count, andTableSpanwill misalign or truncate that row.Parse the line character-by-character or use a regex that respects escapes.
Suggested approach using regex negative lookbehind
- String[] cells = line.split("\\|"); + // Split on | but not on escaped \| + String[] cells = line.split("(?<!\\\\)\\|"); + // Also unescape \| in each cell // Trim each cell for (int j = 0; j < cells.length; j++) { - cells[j] = cells[j].trim(); + cells[j] = cells[j].trim().replace("\\|", "|"); }
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d1922511-8ab6-416b-a478-68c935545ba7
📒 Files selected for processing (2)
TMessagesProj/src/main/java/tw/nekomimi/nekogram/helpers/EntitiesHelper.javaTMessagesProj/src/main/java/tw/nekomimi/nekogram/helpers/TableSpan.java
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 71055b8a-66f7-427d-a6d8-057646c41221
📒 Files selected for processing (2)
TMessagesProj/src/main/java/tw/nekomimi/nekogram/helpers/EntitiesHelper.javaTMessagesProj/src/main/java/tw/nekomimi/nekogram/helpers/TableSpan.java
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
TMessagesProj/src/main/java/tw/nekomimi/nekogram/helpers/EntitiesHelper.java (2)
128-131:⚠️ Potential issue | 🟠 MajorSkip table detection inside code/mono spans before recording positions.
Line 128 currently matches raw text blocks without checking overlap with
TextStyleSpan.isMono()/CodeHighlighting.Span. Code blocks containing table-like lines can still be converted intoTableSpan.
173-176:⚠️ Potential issue | 🟠 MajorPreserve escaped pipes when splitting table cells.
Line 175 uses
line.split("\\|"), so escaped\|is treated as a real delimiter. This corrupts column counts and misaligns rendering.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5a5eb634-41a2-4bae-b8aa-8c836d101f23
📒 Files selected for processing (1)
TMessagesProj/src/main/java/tw/nekomimi/nekogram/helpers/EntitiesHelper.java
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: df5e3cf4-6eef-4187-afcf-1c4f76f9be6e
📒 Files selected for processing (1)
TMessagesProj/src/main/java/tw/nekomimi/nekogram/helpers/EntitiesHelper.java
|
Thanks. The current approach of using a zero width space placeholder breaks the raw message text. There is also some unfinished/unused code and vibe leftovers that need cleaning up. If the goal is just to render Markdown tables from AI bots, sure, it visually works, but the blast radius is too large. I appreciate the effort and I don't mind AI use, but this PR needs more work before it can be merged. I need to keep the codebase maintainable but I can't take on maintenance of code in this state. |
…updating parseMarkdown method
There was a problem hiding this comment.
Actionable comments posted: 4
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1ecd778f-aa37-48b0-8c0d-9039d15d9bc4
📒 Files selected for processing (4)
TMessagesProj/src/main/java/org/telegram/messenger/MediaDataController.javaTMessagesProj/src/main/java/org/telegram/messenger/MessageObject.javaTMessagesProj/src/main/java/org/telegram/ui/Cells/ChatMessageCell.javaTMessagesProj/src/main/java/tw/nekomimi/nekogram/helpers/EntitiesHelper.java
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
TMessagesProj/src/main/java/org/telegram/ui/Cells/ChatMessageCell.java (1)
2356-2399:⚠️ Potential issue | 🔴 CriticalLinks inside table cells are not clickable and lack accessibility support.
When tables are parsed via
parseTables(), the entire table markdown is replaced with a zero-width placeholder and rendered as aTableSpan. This conversion discards all original spans (links, mentions, etc.) embedded in cell content, converting them to plain text strings.TableSpanis a pure rendering span with no touch event handling, click detection, or accessibility implementation—meaningChatMessageCell'sgetSpans()lookup cannot find interactive elements that no longer exist in the buffer, and table cells remain inaccessible to screen readers.To fix this, preserve
ClickableSpans in cell content during parsing, or add hit-testing and accessibility support toTableSpanto expose and handle cell interactions.Applies to lines 2356–2399, 27136–27270, 27676–27681.
♻️ Duplicate comments (2)
TMessagesProj/src/main/java/tw/nekomimi/nekogram/helpers/EntitiesHelper.java (1)
141-166:⚠️ Potential issue | 🟠 MajorSkip code/mono blocks on exact boundary matches too.
The current check only skips when an existing
CodeHighlighting.Span/monoTextStyleSpanextends past the candidate range. If the table-looking text is exactly the contents of a code span,skipstays false and literal code is still converted into a table. Use inclusive containment here (spanStart <= s && spanEnd >= e), or mirror the boundary logic fromparseMarkdown(...).TMessagesProj/src/main/java/tw/nekomimi/nekogram/helpers/TableSpan.java (1)
84-97:⚠️ Potential issue | 🟠 MajorRe-fit after the per-column minimum clamp.
After scaling, each column is clamped back up to
minColumnPx / 2, but the code never checks whether that pushed the sum overavailableSpaceagain. On wide tables,mTableWidthcan still exceed the intended max and overrun the bubble. Add a second fit pass or a hard fallback once the clamped widths no longer fit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8f8ac64e-d2cc-4a27-bd67-3e32735b70a8
📒 Files selected for processing (4)
TMessagesProj/src/main/java/org/telegram/messenger/MessageObject.javaTMessagesProj/src/main/java/org/telegram/ui/Cells/ChatMessageCell.javaTMessagesProj/src/main/java/tw/nekomimi/nekogram/helpers/EntitiesHelper.javaTMessagesProj/src/main/java/tw/nekomimi/nekogram/helpers/TableSpan.java
…nstants in TableSpan
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
TMessagesProj/src/main/java/tw/nekomimi/nekogram/helpers/TableSpan.java (1)
81-87:⚠️ Potential issue | 🟠 MajorPost-shrink width can still overflow available space.
At Line 85, enforcing
minColumnPx / 2after ratio shrink can push total content width back aboveavailableSpacefor wide tables (many columns). That still causes overflow/clipping risk.Proposed fix
- if (totalContentWidth > availableSpace) { + int minShrinkPx = Math.max(1, minColumnPx / 2); + if (totalContentWidth > availableSpace) { float ratio = (float) availableSpace / totalContentWidth; for (int c = 0; c < colCount; c++) { columnWidths[c] = (int) (columnWidths[c] * ratio); - columnWidths[c] = Math.max(minColumnPx / 2, columnWidths[c]); + columnWidths[c] = Math.max(minShrinkPx, columnWidths[c]); } } + + int adjustedContentWidth = 0; + for (int c = 0; c < colCount; c++) { + adjustedContentWidth += columnWidths[c]; + } + if (adjustedContentWidth > availableSpace) { + float ratio = (float) availableSpace / adjustedContentWidth; + adjustedContentWidth = 0; + for (int c = 0; c < colCount; c++) { + columnWidths[c] = Math.max(1, (int) (columnWidths[c] * ratio)); + adjustedContentWidth += columnWidths[c]; + } + } @@ - mTableWidth = decorationsWidth; - for (int w : columnWidths) { - mTableWidth += w; - } + mTableWidth = Math.min(maxTableWidth, decorationsWidth + adjustedContentWidth);Also applies to: 89-93
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0b3f0afe-d111-4e44-9b0b-1bf35298483f
📒 Files selected for processing (2)
TMessagesProj/src/main/java/org/telegram/ui/Cells/ChatMessageCell.javaTMessagesProj/src/main/java/tw/nekomimi/nekogram/helpers/TableSpan.java
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4fd2ed3e-ac3f-4adb-b2ee-b5b23af150c0
📒 Files selected for processing (1)
TMessagesProj/src/main/java/tw/nekomimi/nekogram/helpers/TableSpan.java
|
Hi, thanks for the candid review, and sorry for the churn/noise in the earlier revisions. You were right about the blast-radius concerns. I went through another cleanup pass and addressed the core maintainability/data-integrity issues you called out:
I know the earlier revisions were not at merge quality; thanks for pushing on that. |
2 participants
This pull request introduces support for rendering GitHub Flavored Markdown (GFM) tables in messages, specifically when the "Neko" markdown parser is enabled. It does this by detecting table markdown blocks, parsing them into a structured format, and rendering them as custom table spans within the message layout. The implementation includes a new
TableSpanclass for drawing the tables, as well as changes to the markdown parsing pipeline to integrate table detection and rendering. Additionally, some color and measurement logic for table rendering has been improved for better theme adaptation.Markdown Table Support
TABLE_BLOCK_PATTERN) and a newparseTablesmethod inEntitiesHelper.java. This method replaces matched table markdown blocks with a zero-width space and applies a customTableSpanfor rendering. [1] [2] [3]MARKDOWN_PARSER_NEKO).Custom Table Rendering
TableSpanclass that extendsReplacementSpanto render parsed tables as styled blocks within the message. This includes custom drawing of cell backgrounds, borders, and text, with support for header row styling and theme-aware colors.patch_table.pyscript which updates the relevant methods inTableSpan.java.Code Integration and Refactoring
These changes collectively enable rich table rendering in messages, enhancing the readability and presentation of structured data in chat.
Summary by CodeRabbit
New Features
Bug Fixes