-
Notifications
You must be signed in to change notification settings - Fork 21
Support cross-repository redirects and specialized anchor scenarios #1227
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
Conversation
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.
Pull Request Overview
This PR adds support for cross‐repository redirects and enhanced anchor handling in Markdown links.
- Updated HtmxLinkInlineRenderer to differentiate between cross links and HTTP links.
- Refactored CrossLinkResolver to improve redirect resolution and error reporting.
- Enhanced URI validation in DocumentationSet and updated documentation for redirects.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
src/Elastic.Markdown/Myst/Renderers/HtmxLinkInlineRenderer.cs | Introduces new flags (isCrossLink, isHttpLink) and refines hx attribute assignment based on link types. |
src/Elastic.Markdown/Links/CrossLinks/CrossLinkResolver.cs | Refactors redirect resolution logic with clearer variable naming and improved handling of anchors and lookup paths. |
src/Elastic.Markdown/IO/DocumentationSet.cs | Adds URI validation for external redirect targets. |
docs/contribute/redirects.md | Updates documentation with examples on cross-repository redirects and complex anchor scenarios. |
'item-a': 'yy' | ||
- to: 'testing/redirects/third-page.md' | ||
anchors: | ||
'item-b': |
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.
The anchor mapping for 'item-b' is empty, which could be unintentional. Confirm whether this is the desired behavior or if an explicit null/empty value should be specified for clarity.
'item-b': | |
'item-b': '!' |
Copilot uses AI. Check for mistakes.
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 work!
A few nits about leniency. Can you also add more tests to https://github.com/elastic/docs-builder/blob/da90aa5d28612d4b9871462a4647cd41bb442494/tests/authoring/Inline/CrossLinksRedirects.fs ?
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.
A few stylistic nitpicks from me and I think the copilot review has one good suggestion.
Almost LGTM!
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.
LGTM
Closes #670, #847
This MR introduces support for cross-repository redirects, and support for more complex anchor scenarios.