Provide ability to provide a callback to be used in case of broken link references #123
Conversation
|
r? @raphlinus |
raphlinus
left a comment
There was a problem hiding this comment.
Overall looks good, just a couple of points. Thanks for doing the work!
| containers: Vec<Container>, | ||
| last_line_was_empty: bool, | ||
|
|
||
| /// In case we have a broken link/image reference, we can call this callback |
There was a problem hiding this comment.
I'd prefer the naming "link resolver"
| /// the provided callback will be called with the reference name, | ||
| /// and the returned pair will be used as the link name and title if not | ||
| /// None. | ||
| pub fn new_with_broken_link_callback(text: &'a str, mut opts: Options, |
There was a problem hiding this comment.
I'm curious whether the callback might be a reference rather than a Box, as it's only used for the lifetime of the parser (similar to text).
There was a problem hiding this comment.
It could, I didn't want to introduce another lifetime param. But reusing the same one may work.
|
ping @raphlinus |
|
Sorry, I was waiting for my initial review to be addressed, and kinda dropped the ball. Do you think this is ready for merge? |
|
I think so |
|
Another thing we need is to make the HTML escaping stuff public, but I'll open a separate PR for that. |
|
Ok, I'll merge as is then. Thanks! |
|
@raphlinus can we get a version bump for this? rust-lang/rust#48274 recently landed so I'd like to incorporate this into rustc |
|
@Manishearth Done. I can do another release when the other stuff has landed. |
|
Sounds good. I'll have to wait for that since the feature as is can't be used. Might want to actually do it in a single version bump (or yank)? The other PR is technically a breaking change, but only if there's a version between these two PRs. |
fixes #121