A new broken link callback design #469
Merged
Conversation
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
Contributor
|
This looks great to me! |
jyn514
reviewed
Aug 29, 2020
Contributor
jyn514
left a comment
jyn514
left a comment
There was a problem hiding this comment.
Let me know if there's anything I can do to help with this :) I'd love to have these fixed before intra-doc links (hopefully) stabilize in 6 weeks.
marcusklaas
changed the title
marcusklaas
changed the title
jyn514
approved these changes
Aug 30, 2020
raphlinus
approved these changes
Aug 31, 2020
Collaborator
raphlinus
left a comment
raphlinus
left a comment
There was a problem hiding this comment.
Generally this looks good, suggestions for minor fixes inline. I understand this is a (minorly) breaking change and we'll want to bump semver?
marcusklaas
force-pushed
the
callback-redesign
branch
from
a726d6c to
f2c8906
Compare
Collaborator
Author
|
Sending a big thank to @jyn514, @euclio and @raphlinus for the feedback! Edit: And yes, this will require a semver breaking change bump. |
Contributor
|
This is awesome, thank you @marcusklaas! Now I just need to update |
jyn514
added a commit
to jyn514/rust
that referenced
this pull request
Sep 14, 2020
Thanks to marcusklaas' hard work in pulldown-cmark/pulldown-cmark#469, this fixes a lot of rustdoc bugs! - Get rid of unnecessary `RefCell` - Fix duplicate warnings for broken implicit reference link - Remove unnecessary copy of links
Dylan-DPC-zz
pushed a commit
to Dylan-DPC-zz/rust
that referenced
this pull request
Sep 15, 2020
…Gomez Upgrade to pulldown-cmark 0.8.0 Thanks to marcusklaas' hard work in pulldown-cmark/pulldown-cmark#469, this fixes a lot of rustdoc bugs! - Get rid of unnecessary `RefCell` - Fix duplicate warnings for broken implicit reference link - Remove unnecessary copy of links Closes rust-lang#73264, closes rust-lang#76687. r? @euclio I'm not sure if the switch away from `locate` fixes any open bugs - euclio mentioned some in pulldown-cmark/pulldown-cmark#165, but I didn't see any related issues open for rustdoc. Let me know if I missed one.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
This is a
WIPPR to address several outstanding issues with the current broken link callback design.Firstly, it provides additional data (source mapping, link type) to the callback to improve diagnostics (#373) and help disambiguate links with identical references (#165). Further, this design also prevents the callback from being called twice on the same reference (#444). And lastly, the callback now returns
CowStrs, so that it is possible to generate titles and urls without memory allocations, for example when they are static strings or derived from text in the source.Feedback is greatly appreciated. Would this cover your use-cases? Is this an improvement over the old design?
cc @euclio @jyn514 @GuillaumeGomez