Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support rendering markdown tables in docs #12693

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

rmehri01
Copy link
Contributor

@rmehri01 rmehri01 commented Jan 27, 2025

This pr adds support for rendering markdown tables by adding the ENABLE_TABLES option and then handling parser events separately once we see a Event::Start(Tag::Table(..)) up until Event::End(TagEnd::Table).

Before:

image

After:

image

Closes #11175

@nik-rev
Copy link
Contributor

nik-rev commented Jan 27, 2025

Thanks for this PR! Some suggestions here.

  1. Let's use punctuation.special for the table separators, to be consistent with the Markdown highlights
  2. Use a different character set to render the separators, to be more visually pleasing.

Before

image

After

image

Same Markdown for reference ![image](https://github.com/user-attachments/assets/1319a425-0152-4600-9e2f-7ee6b6713d4c)

I implemented these in d00c513 which you can cherry-pick onto your PR

@rmehri01
Copy link
Contributor Author

Thanks for this PR! Some suggestions here.

1. Let's use `punctuation.special` for the table separators, to be consistent with the Markdown highlights

2. Use a different character set to render the separators, to be more visually pleasing.

Thanks, this looks pretty nice! I just cherry-picked it and added the top and bottom rows as well 😄

@nik-rev
Copy link
Contributor

nik-rev commented Jan 28, 2025

Nice, I like the idea with border on top and bottom!

One thing that I think is important and fixable: Table rendering is broken when you have a lot of columns:

With this markdown:

## Cost of Collection Operations
|                | get(i)                 | insert(i)               | remove(i)              | append(Vec(m))    | split_off(i)           | range           | append       |
|----------------|------------------------|-------------------------|------------------------|-------------------|------------------------|-----------------|--------------|
| [`Vec`]        | *O*(1)                 | *O*(*n*-*i*)*           | *O*(*n*-*i*)           | *O*(*m*)*         | *O*(*n*-*i*)           | N/A             | N/A          |
| [`VecDeque`]   | *O*(1)                 | *O*(min(*i*, *n*-*i*))* | *O*(min(*i*, *n*-*i*)) | *O*(*m*)*         | *O*(min(*i*, *n*-*i*)) | N/A             | N/A          |
| [`LinkedList`] | *O*(min(*i*, *n*-*i*)) | *O*(min(*i*, *n*-*i*))  | *O*(min(*i*, *n*-*i*)) | *O*(1)            | *O*(min(*i*, *n*-*i*)) | N/A             | N/A          |
| [`HashMap`]    | *O*(1)~                | *O*(1)~*                | *O*(1)~                | N/A               | N/A                    | N/A             | N/A          |
| [`BTreeMap`]   | *O*(log(*n*))          | *O*(log(*n*))           | *O*(log(*n*))          | N/A               | N/A                    | *O*(log(*n*))   | *O*(*n*+*m*) |

It renders as follows:

image

My idea is to break the table up into multiple smaller ones if it's too large. For instance, by rendering it like this:

image

@the-mikedavis
Copy link
Member

I don't think we can split tables like that generally. It's not always the case that the first column should be repeated between the split tables and there will be tables with cells large enough that they can't be split.

Instead I believe the right way to fix this is to avoid wrapping table text. It's not a trivial change to do that because the Markdown component renders everything in one Paragraph and that's what is wrapped. Ideally it'd be nice to reuse the Table widget from helix-tui here as well. It doesn't currently support rendering borders like this IIRC but that would be a nice improvement that should fit well into helix-tui/src/widgets/table.rs

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.

Bad markdown table format in docs preview (<space-k>)
3 participants