fix(refactor): rewrite absolute links in sub-pages when moving a section#1182
Merged
Conversation
When a section with sub-pages was moved, absolute markdown links inside those sub-pages pointing to sibling pages within the same section were not updated. rewritePathChangedSubtree only ran RewriteRelativeLinksForPathChange which explicitly skips absolute links, and sub-pages are excluded from rewriteAffectedPages (which handles external pages only). Add a second pass using engine.Rewrite for absolute intra-subtree links, short-circuited via strings.Contains to avoid a redundant goldmark parse when no matching absolute links can be present. Also cache current.CalculatePath() to a single call per page. Adds three tests covering: absolute links in sub-pages to sibling sub-pages, relative links in sub-pages to external pages, and a regression guard ensuring intra-subtree relative links remain unchanged after the move.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes link rewriting for moved sections by ensuring absolute Markdown links inside sub-pages (pointing to other pages within the moved subtree) are updated when the section is moved. It extends the existing subtree rewrite logic to cover absolute intra-subtree links that were previously skipped.
Changes:
- Add a second rewrite pass in
rewritePathChangedSubtreeto rewrite absolute Markdown links within the moved subtree, with astrings.Containsshort-circuit to avoid unnecessary parsing. - Cache
current.CalculatePath()to avoid repeated path computation per page. - Add three regression tests covering absolute intra-subtree links, external relative links from sub-pages, and ensuring intra-subtree relative links remain unchanged.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
internal/wiki/pages/refactor.go |
Adds a second-pass rewrite for absolute intra-subtree Markdown links during subtree path changes, with a cheap pre-check and minor perf cleanup. |
internal/wiki/pages/pages_test.go |
Adds tests for the new absolute-link rewriting behavior and guards against unintended relative-link rewrites. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When a section with sub-pages was moved, absolute markdown links inside those sub-pages pointing to sibling pages within the same section were not updated. rewritePathChangedSubtree only ran RewriteRelativeLinksForPathChange which explicitly skips absolute links, and sub-pages are excluded from rewriteAffectedPages (which handles external pages only).
Add a second pass using engine.Rewrite for absolute intra-subtree links, short-circuited via strings.Contains to avoid a redundant goldmark parse when no matching absolute links can be present. Also cache current.CalculatePath() to a single call per page.
Adds three tests covering: absolute links in sub-pages to sibling sub-pages, relative links in sub-pages to external pages, and a regression guard ensuring intra-subtree relative links remain unchanged after the move.