Skip to content

RST writer: Add col spans for simple tables#11173

Merged
jgm merged 1 commit intojgm:mainfrom
TuongNM:rst-writer-simple-table-col-spans
Sep 29, 2025
Merged

RST writer: Add col spans for simple tables#11173
jgm merged 1 commit intojgm:mainfrom
TuongNM:rst-writer-simple-table-col-spans

Conversation

@TuongNM
Copy link
Contributor

@TuongNM TuongNM commented Sep 28, 2025

Together with the reader part from #11115 this writer part fixes #10127

Comment on lines +938 to +933
let hdr = if all null headers
let hdr = if null headers
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the change from all null to null? This could make a difference.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The headers were previously a List of Lists of Block [[Block]] from toLegacyTable. Now the function is operating directly on the TableHead headers which is just a List of Rows [Row].

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still, maybe we need to also check for the case where the Rows aren't empty but they contain only empty cells? That was the case we were catching before.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not absolutely sure it's necessary, but I worry that it will be a regression.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha. I'm gonna try to adjust my code to ensure that this check will still hold.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I just added the helper functions isEmptyRow and isEmptyCell to be able to check for empty Rows and Cells.

in (doc, Just col) : replicate (colSpan - 1) emptyDoc
| otherwise = [(doc, Just col)]

mapToRow colWidths docsWithColSpans = vcat $ concatMap (toRow colWidths) docsWithColSpans
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be written point free: omit docsWithColSpans and change $ to ..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I changed it.

@TuongNM TuongNM force-pushed the rst-writer-simple-table-col-spans branch from 65f434b to ad449df Compare September 28, 2025 14:00
@TuongNM TuongNM force-pushed the rst-writer-simple-table-col-spans branch from ad449df to 1a44e9e Compare September 28, 2025 15:17
@jgm jgm merged commit ca0fc7d into jgm:main Sep 29, 2025
11 of 14 checks passed
@jgm
Copy link
Owner

jgm commented Sep 29, 2025

thanks!

@TuongNM TuongNM deleted the rst-writer-simple-table-col-spans branch September 29, 2025 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RST simple tables: support col spans

2 participants