feat: Add support for Pandoc-style table captions - #1105
Conversation
|
Thanks! Please add more tests in a spec file like for other features. Also please note the errors in the automated workflows 🙂. |
|
Done! |
|
Nice, it looks good to me. The only thing that concerns me is that if we are adopting Pandoc spec, I would adopt the full spec allowing also captions in top of tables. What do you think, @notriddle? |
| } | ||
| line_start.scan_all_space(); | ||
| ix += line_start.bytes_scanned(); | ||
| if scan_paragraph_interrupt_no_table( |
There was a problem hiding this comment.
Can you add a test case for this feature when table captions and definition lists are enabled at the same time?
I checked, and it works, but these features both use :, which means that the interaction between them should be tested.
| ) { | ||
| return None; | ||
| } | ||
| if scan_ch(&bytes[ix..], b':') == 1 && self.options.contains(Options::ENABLE_TABLE_CAPTIONS) |
There was a problem hiding this comment.
Here's a tricky test case
```````````````````````````````` example_table_captions
Test|Table
----|-----
Test|row
: Test Caption
Test|ending
.
<table><thead><tr><th>Test</th><th>Table</th></tr></thead><tbody>
<tr><td>Test</td><td>row</td></tr>
<caption>Test Caption</caption>
<tr><td>Test</td><td>ending</td></tr>
</tbody></table>
It reflects the behavior of pandoc, except that <caption> is supposed to be the first child of the table element.
| fn parse_table_caption(&mut self, mut ix: usize) -> Option<(usize, TreeIndex)> { | ||
| let bytes = self.text.as_bytes(); | ||
| ix += scan_ch(&bytes[ix..], b':'); | ||
| ix += scan_whitespace_no_nl(&bytes[ix..]); |
There was a problem hiding this comment.
Note that there are no test cases for the zero-spaces caption:
Test|Table
----|-----
Test|row
:Test Caption
Test|ending
That seems like a good thing to test, also.
| . | ||
| <table><thead><tr><th>Test</th><th>Table</th></tr></thead><tbody> | ||
| <tr><td>Test</td><td>row</td></tr> | ||
| <caption>Test Caption</caption> |
There was a problem hiding this comment.
This isn't valid HTML.
According to the standard, you are required to put the <caption> tag as the first child of the <table> element.
I've pushed a commit that fixes this to amiroo54/pulldown-cmark@main...notriddle:pulldown-cmark:html-validation
| @@ -0,0 +1,123 @@ | |||
| Run this with `cargo test --features gen-tests suite::table_captions`. | |||
|
|
|||
| # Table Captions, inspired by Pandoc's table captions | |||
There was a problem hiding this comment.
Pandoc supports captions before tables, too. How hard would it be?
Fix invalid HTML table caption
|
A quick reminder, isn't this going to be merged? |
|
I think that we are both waiting to support for caption before tables to match Pandoc spec 🙂. |
|
I was hoping we could do without that. I'm in the middle of some exams and can't work on it for a while. It'll have to wait for the duration I guess. |
|
I submitted a port of this feature to commonmark-hs, because if we’re going to invent a new syntax, I wanted at least one other implementation of it to exist. In the past, pulldown-cmark implemented a limited version of footnotes, and we got constant complaints until we fixed it by implementing a more typical version. I don’t want to do that again. If we’re adding this, I want cross-markdown implementation consensus. |
Currently, there is no native way to add captions to tables. This PR introduces support for table captions inspired by the Pandoc table_caption specification, using the : prefix syntax immediately following a table.
Changes included:
Note on Spec Compliance:
To keep this initial implementation focused and lightweight, I scoped it to support captions after the table using the : prefix. Pandoc’s alternative syntaxes (like Table: Caption before the table) are not included in this PR, but the architecture allows for them to be added later if desired.