Honor the GFM delimiter row's column alignment in tables - #166
Honor the GFM delimiter row's column alignment in tables#166lukebradford wants to merge 1 commit into
Conversation
The README lists `:---`, `:---:` and `---:` column alignment as supported, but every cell rendered `.leading` regardless. `Markdown.Table` exposes `columnAlignments`, and `Table+.convert` had it in hand — there was simply nowhere to put it, since `MarkdownRenderable.table` carried only headers, rows and the raw markdown. The alignment was dropped at conversion time and `TableView` never had a chance to apply it. - `MarkdownRenderable.table` gains `alignments: [MarkdownColumnAlignment]`, a small internal enum so the model keeps its current imports. - `Table+.convert` maps swift-markdown's `columnAlignments` onto it, padding short/absent declarations with `.leading`. - `TableView` applies both `multilineTextAlignment` and the cell's frame alignment per column, for the header row and both body-cell variants. A table that declares no alignment still renders `.leading` — the GFM default — so existing output is unchanged. `TableView`'s new `alignments` parameter is defaulted, so direct constructions (including the ones in `TableViewTests`) still compile untouched. This matters most for numeric columns: a currency or percentage column declared `---:` currently reads as a ragged left edge, which is what sent me looking. (Heads up for CI: `TableViewSnapshotTests.testTableCellWithOnlyCitation` and the `UnorderedListViewTests` snapshots already fail on my machine at unmodified `main` — reference mismatches under `macOS-standard-*`, seemingly recorded on different hardware — so I could only verify "no new failures" rather than green.)
|
lukebradford please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
1 similar comment
|
lukebradford please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
There was a problem hiding this comment.
Pull request overview
This PR fixes a gap in the rendering pipeline where GitHub-Flavored Markdown (GFM) table column alignment declared in the delimiter row (:---, :---:, ---:) was being dropped during conversion, causing all table columns to render as leading-aligned in SwiftUI.
Changes:
- Extend the render model (
MarkdownRenderable.table) to carry per-column alignments via an internalMarkdownColumnAlignmentenum. - Map
Markdown.Table.columnAlignmentsto the render model inTable+.convert, defaulting unspecified/short declarations to.leading. - Apply per-column alignment in
TableViewfor header and text cells (and begin applying it for attachment-backed cells).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Sources/MarkdownText/UI/TableView.swift | Accepts per-column alignments and uses them to set multilineTextAlignment / frame alignment when rendering cells. |
| Sources/MarkdownText/UI/BlockView.swift | Threads the new alignments payload from the render model into TableView. |
| Sources/MarkdownText/Models/RenderableDocument.swift | Updates .table pattern matches to account for the new alignments payload when extracting plain text / attributed strings. |
| Sources/MarkdownText/Models/MarkdownRenderable.swift | Adds MarkdownColumnAlignment and extends the .table case to persist alignments through rendering. |
| Sources/MarkdownText/Block/Table+.swift | Converts swift-markdown’s columnAlignments into the internal model enum, padding missing values with .leading. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| case .containsAttachment(let nsAttributedString): | ||
| HStack(spacing: 0) { | ||
| ParagraphView(contents: applyTypographyThemingAndGetContent(nsAttributedString)) | ||
| .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .topLeading) | ||
| .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: frameAlignment(forColumn: colIdx)) | ||
| .accessibilityValue(String.itemPositionInTable(rowIndex: rowIdx + 2, totalRow: numOfRows + 1, columnIndex: colIdx + 1, totalColumn: headings.count)) |
| let alignments = (0..<headerCells.count).map { index -> MarkdownColumnAlignment in | ||
| guard index < self.columnAlignments.count else { return .leading } | ||
| switch self.columnAlignments[index] { | ||
| case .left: return .leading | ||
| case .center: return .center |
The README lists
:---,:---:and---:column alignment as supported, but every table cell renders.leadingregardless.Markdown.TableexposescolumnAlignmentsandTable+.converthas it in hand — there was simply nowhere to put it, sinceMarkdownRenderable.tablecarried only headers, rows and the raw markdown. The alignment is dropped at conversion time, soTableViewnever gets a chance to apply it.Changes
MarkdownRenderable.tablegainsalignments: [MarkdownColumnAlignment], a small internal enum so the model keeps its current imports (noMarkdowndependency added to the model layer).Table+.convertmaps swift-markdown'scolumnAlignmentsonto it, padding absent or short declarations with.leading.TableViewapplies bothmultilineTextAlignmentand the cell frame alignment per column — for the header row and both body-cell variants (text and attachment).Compatibility
A table that declares no alignment still renders
.leading, which is the GFM default, so existing output is unchanged.TableView's newalignmentsparameter is defaulted, so direct constructions — including the ones inTableViewTests— compile untouched.Why it matters
Numeric columns. A currency or percentage column declared
---:currently reads as a ragged left edge, which is what sent me looking: we render financial tables into a chat transcript, and left-aligned money is genuinely harder to scan.Note for CI
TableViewSnapshotTests.testTableCellWithOnlyCitationand theUnorderedListViewTestssnapshots already fail for me at unmodifiedmain— reference mismatches undermacOS-standard-light/macOS-standard-dark, seemingly recorded on different hardware. So I could verify "no new failures" but not "green"; you may want to re-record those independently of this PR.Happy to add snapshot coverage for a right-aligned table if you'd like it, once the reference situation is sorted.
🤖 Generated with Claude Code