-
Notifications
You must be signed in to change notification settings - Fork 34
Add autolinks rendering #2541
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?
Add autolinks rendering #2541
Conversation
|
@elastic/docs-tech-leads This adds autolinks! |
Mpdreamz
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.
MarkDig supports .UseAutoLinks() natively, is there a reason we don't use it? (https only?)
|
@Mpdreamz I think so. Do we want to support also plain HTTP (unencrypted)? |
|
@Mpdreamz After checking with Claude: there are several reasons we implemented a custom parser instead of using Markdig's
If the requirements were simpler (all URL schemes, no diagnostics), we could have used: |
…lder into add-autolinks-rendering
🔍 Preview links for changed docs |
| [Fact] | ||
| public void GeneratesHtml() => | ||
| Html.Should().Contain( | ||
| """<a href="https://www.elastic.co/docs/deploy-manage" target="_blank" rel="noopener noreferrer">https://www.elastic.co/docs/deploy-manage</a>""" |
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.
Internal links need to have hx-* attributes, especially the hx-select-oob attribute.
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.
Autolinks are always assumed to be external, that's why they don't get the hx- attrib?
|
Temporarily moving this to draft as we assess the situation regarding bare URLs in our docs (see Slack message). |
|
|
||
| #### Hint for elastic.co/docs URLs | ||
|
|
||
| Autolinks pointing to `elastic.co/docs` trigger a hint during build, suggesting you replace them with a [cross-repository link](#cross-repository-links) or relative link for better maintainability. |
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.
Do we exempt /docs/api ? we might want to add an exlusion for .zip given we want to expose llms.zip and elasticsearch-data.zip soon.
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.
How would you suggest proceeding here? We could add a filtering system (potentially fragile, hard to maintain, etc.), launch Autolinks anyway despite some broken URLs and let teams fix this, or hold / cancel. See thread in Slack.
This follows up from a conversation in #2532 to add automatic link rendering from HTTPs URLs.
Files created:
Files modified:
Features implemented:
—-
LLM usage disclosure: I've used Claude Opus 4.5 in Cursor.