-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
base: master
Are you sure you want to change the base?
Conversation
Thanks for this PR! Some suggestions here.
BeforeAfterSame 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 |
Thanks, this looks pretty nice! I just cherry-picked it and added the top and bottom rows as well 😄 |
24648b4
to
96a8e62
Compare
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: My idea is to break the table up into multiple smaller ones if it's too large. For instance, by rendering it like this: |
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 |
This pr adds support for rendering markdown tables by adding the
ENABLE_TABLES
option and then handling parser events separately once we see aEvent::Start(Tag::Table(..))
up untilEvent::End(TagEnd::Table)
.Before:
After:
Closes #11175