Fix rendered Markdown anchor links#10110
Conversation
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @CarlAmko on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment |
|
@cla-bot check |
|
The cla-bot has been summoned, and re-checked this pull request! |
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
|
/oz-review |
|
I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR adds same-document rendered Markdown anchor handling for notebook links, including slug normalization, heading lookup, and local scrolling.
Concerns
- Duplicate heading slug allocation can collide when a later heading's base slug is equal to an earlier generated duplicate suffix.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| continue; | ||
| } | ||
|
|
||
| let count = slug_counts.entry(base_slug.clone()).or_insert(0); |
There was a problem hiding this comment.
Goal, Goal 1, Goal, both the second and third headings generate goal-1, so #goal-2 cannot resolve.
There was a problem hiding this comment.
Overview
This PR adds rendered Markdown same-document anchor handling before generic notebook link resolution, including heading slug generation, duplicate slug handling, and tests for local jump behavior.
Concerns
- This changes user-visible rendered Markdown navigation behavior, but the PR description does not include screenshots or a video. For faster review, please upload screenshots or a video of the anchor link behavior working end to end.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
2cb84a7 to
d5994d2
Compare
|
/oz-review |
|
I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR updates rendered Markdown link handling so same-document anchors are resolved locally before generic notebook link resolution, derives anchor targets from rendered heading blocks, and adds tests for slug normalization, duplicate heading slugs, and jump behavior.
Concerns
- The PR changes user-visible rendered Markdown navigation behavior, but the description does not include screenshots or a video demonstrating anchor jumps and broken-anchor handling end to end. For faster review, please upload screenshots or a video of the feature working end to end.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
Summary
Issue
Fixes #10058
Test plan
CHANGELOG-BUG-FIX: Fixed rendered Markdown table-of-contents anchors so same-document heading links jump to the target heading.