Roam: Convert a table to a markdown table - #505
Conversation
| const binaryRegex = /https:\/\/firebasestorage(.*?)\?alt(.*?)/; | ||
|
|
||
| const blockRefRegex = /(?<=\(\()\b(.*?)\b(?=\)\))/g; | ||
| const roamTableRe = /^\{\{(\[\[)?table(\]\])?\}\}$/i; |
There was a problem hiding this comment.
The current regex has two independent optional groups:
/^\{\{(\[\[)?table(\]\])?\}\}$/i
^^^^ ^^^^
group 1 group 2
Each ? makes its group independently optional, producing 4 possible matches:
| Group 1 | Group 2 | Matches | Valid Roam? |
|---|---|---|---|
| absent | absent | {{table}} |
yes |
| present | present | {{[[table]]}} |
yes |
| present | absent | {{[[table}} |
no |
| absent | present | {{table]]}} |
no |
The proposed fix ties the brackets together as a single alternation:
/^\{\{(\[\[table\]\]|table)\}\}$/i
This only matches {{[[table]]}} or {{table}} -- brackets are always balanced.
| const roamTableRe = /^\{\{(\[\[)?table(\]\])?\}\}$/i; | |
| const roamTableRe = /^\{\{(\[\[table\]\]|table)\}\}$/i; |
| while (current) { | ||
| const scrubbed = await this.roamMarkupScrubber(graphFolder, attachmentsFolder, current.string || ''); | ||
| cells.push(scrubbed.replace(/\|/g, '\\|')); | ||
| current = current.children?.[0]; |
There was a problem hiding this comment.
If a Roam table cell has multiple children (e.g., user added sub-items), all but the first are silently discarded with no warning. This could cause data loss without any indication to the user. Consider logging a warning when current.children.length > 1.
| const prefix = json.heading ? '#'.repeat(json.heading) + ' ' : ''; | ||
| const scrubbed = await this.roamMarkupScrubber(graphFolder, attachmentsFolder, json.string); | ||
| markdown.push(`${isChild ? indent + '* ' : indent}${prefix}${scrubbed}`); | ||
| if ('string' in json && json.string && roamTableRe.test(json.string.trim()) && json.children) { |
There was a problem hiding this comment.
- children:
[](truthy) enters convertRoamTable, returns''-- table marker vanishes silently - children:
undefinedfalls to the else branch and renders raw{{[[table]]}}in output
Consider having the table-detection guard not require json.children, and letting convertRoamTable handle all cases uniformly.
| // Build pipe table | ||
| const lines: string[] = []; | ||
| // Header row | ||
| lines.push(indent + '| ' + tableData[0].join(' | ') + ' |'); |
There was a problem hiding this comment.
The formatting of the various table sections could be unified and simplified by inserting the separator row after the header:
const separator = tableData[0].map(() => '---');
tableData.splice(1, 0, separator);
const lines = tableData.map(row => indent + '| ' + row.join(' | ') + ' |');
Roam stores a table as a {{[[table]]}} marker whose children are the rows,
each row's columns being a chain of first children. Nothing read that shape,
so a table arrived as bullets - the marker on one line and the cells nested
under it, tabular structure gone.
The marker's children are now read as cells and written as a pipe table:
the first row is the header, as Roam shows it, a short row is padded, and a
pipe or a newline in a cell is escaped so it cannot end the cell or the row.
Cells go through the same markup rewrites as any other block.
The table is written at the left margin with a blank line either side,
whatever depth the marker sat at, because Obsidian does not render a pipe
table indented inside a list item - keeping the outline's indentation would
keep the bullets tidy and leave the table as rows of text.
A marker with no rows under it converts to nothing and leaves no line
behind. Timestamp accumulation moved to its own method, so cells - walked
rather than recursed into - still count towards the page's dates.
8f74dc3 to
af39677
Compare
|
Sorry for the long silence on this - thank you for the review, it was more useful than the delay suggests. Rebased onto The test harness that arrived in the meantime made this much easier to show. I have closed #506. The code-block fix that was mixed into it will come back on its own branch once this lands - and you were right about the language tags, the real problem there was Roam gluing the fences to the content, not the tags themselves. |

Roam stores a table as a
{{[[table]]}}marker whose children are the rows, each row's columns being a chain of first children. Nothing read that shape, so a table arrived as bullets: the marker on one line and the cells nested under it, tabular structure gone. Partially addresses #180.The marker's children are now read as cells and written as a pipe table. The first row becomes the header, as Roam shows it; a short row is padded; a pipe or a newline in a cell is escaped so it cannot end the cell or the row; and cells go through the same markup rewrites as any other block.
Rebased and revised
This has been rewritten against
src/formats/roam/convert.tsand now carries its recordings, so the change is visible in the diff rather than described.Each point from the review:
/^\{\{(\[\[table\]\]|table)\}\}$/i, so{{[[table}}no longer matches. Covered by a test.console.warnnaming the cell and the count, rather than a silent drop.children: []vsundefined. The detection no longer requires children. Both go through the same path and both leave nothing behind, marker included.One thing worth flagging: timestamp accumulation moved into its own method. Cells are walked rather than recursed into, so without it a page whose most recent edit lives inside a table would come out dated before its own content.
Recordings
tests/roam/expected/help-graph-excerpt/Table.mdandtests/roam/expected/small-test-graph/Theme Tester.mdare the only two that move. Nothing else in either graph changes.Eight named tests cover the shapes the recordings do not: the bare
{{table}}spelling, the unbalanced marker, a short row, a pipe in a cell, a multi-line cell, a marker with no rows, and markup inside a cell.npm test300 passing,npm run typecheckandnpm run lint:checkclean, andlint:reviewreports the same single pre-existing error on this file asmasterdoes.#506 has been closed. The code-block fix that was tangled up in it will come back on its own once this lands.