-
Notifications
You must be signed in to change notification settings - Fork 34
Add Markdown support to the CSV directive #2532
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
base: main
Are you sure you want to change the base?
Conversation
🔍 Preview links for changed docs |
shainaraskas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice nice nice this is awesome
|
I am not opposed to adding autolink. We should poll all docs tech leads. |
I wish we had it because mostly not having it introduces bugs for us (people putting in plain links without us noticing) |
|
Is it working? https://docs-v3-preview.elastic.dev/elastic/docs-builder/pull/2532/syntax/csv-include.md The output shows an empty tag <csv-include>
</csv-include>// edit: Ah, I just noticed.. this is about rendering markdown from CSV cells. I guess I found a bug. // edit 2: |
|
Autolinks were the default in asciidoc and it was handy so people are used to it and I'm all for it, we'd just need to make sure it's deactivated where needed (code snippets & co) |
|
I'm removing autolinks as an exception from this PR to avoid mixing two features and will open a separate PR to add and test autolinks where it makes sense, if that's OK. |
|
@elastic/docs-engineering Moved the autolinks bits to #2541 With that, I think this one is ready for prime time! |
This adds Markdown rendering support to the CSV include directive.
Some notes:
NormalizeCsvCellMarkdown + IsPlainUrl: These exist because the mainMarkdownParser.Pipelinedoesn't includeUseAutoLinks(). If we added.UseAutoLinks()to the pipeline, bare URLs would automatically become links and these two methods could be deleted. However, that's a global change that might affect other content unexpectedly.<p>tags inside table cells. Without it,**Bold**would render as<td><p><strong>Bold</strong></p></td>instead of<td><strong>Bold</strong></td>.The most impactful simplification would be adding UseAutoLinks() to the main pipeline, which would eliminate NormalizeCsvCellMarkdown and IsPlainUrl entirely. The inline rendering optimization is CSV/table-specific.
Update
Removing autolinks as an exception from the PR so I can open a separate PR just for autolinks.
——
LLM disclosure: I've used GPT Codex 5.2 and Claude Opus 4.5 in Cursor.