Generate links at compile-time rather than use JS #207
Conversation
|
oh wait i bet i can do this in one pass.... |
|
|
||
| html = regex.replace_all(&html, |caps: &Captures| { | ||
| let text = &caps[1]; | ||
| let mut id = text.to_string(); |
There was a problem hiding this comment.
i am almost tempted to pull this shenanigans out into a microcrate so that rustdoc and mdbook can share it
|
regex doesn't support back-references, which is what's need to do this in one pass. so we can stick with this, or we can add a dependency on https://github.com/google/fancy-regex. I'm tempted to just stick with it. |
|
actually https://docs.rs/regex/0.2.0/regex/struct.RegexSet.html might be able to do it in one pass... |
|
Nevermind, I don't think so. This should be good to go. |
Cargo.toml
Outdated
| @@ -24,6 +24,7 @@ log = "0.3" | |||
| env_logger = "0.3" | |||
| toml = { version = "0.2", features = ["serde"] } | |||
| open = "1.1" | |||
| regex = "0.1.80" | |||
There was a problem hiding this comment.
The last version of regex seems to be 0.2.1 is there a reason for using this specific version?
There was a problem hiding this comment.
It was the version that was already in cargo.lock; I'd be happy to upgrade it too, but I was trying to not mess with dependencies 😄
There was a problem hiding this comment.
I think it's ok to go for the latest, it shouldn't cause any problems right?
There was a problem hiding this comment.
the only thing it might cause is two version of regex, given that iirc this is a dependency of something.
I could also just update all deps; https://github.com/azerupi/mdBook/pull/206 is doing some of that
happy to do whatever you want 😄
There was a problem hiding this comment.
Updating deps is good, but maybe it's better if I first accept #206 then to avoid conflicts
There was a problem hiding this comment.
#206 is merged, you can go ahead and update the rest :)
| let mut id = text.to_string(); | ||
| let repl_sub = vec!["<em>", "</em>", "<code>", "</code>", | ||
| "<strong>", "</strong>", | ||
| "<", ">", "&", "'", """]; |
There was a problem hiding this comment.
I don't think this list is exhaustive and future proof. Maybe it would be better to match against anything that could look like a tag? I am not sure how GitHub does it.
There was a problem hiding this comment.
i copied this code directly from rustdoc so that it'd have the same behavior; i agree that it's probably not exhaustive.
I am not sure how GitHub does it.
I don't know either :/
There was a problem hiding this comment.
I guess it's good enough for now :)
We can improve it later if necessary.
This project already had a transitive dependency on regex; let's use it. This isn't the most efficient solution, but it should be fine. It ends up doing five full scans of the text. There's probably an easier way but I'm mostly just trying to get this to work for now. This also implements the same algorithm that rustdoc does for generating the name for the link. Fixes #204
|
Okay! I have updated all the deps required to have one version of regex; I did not update all of the deps, but might send in a new PR later 😄 |
|
Great! Once the little lights turn green it will be good to merge 😉 |
|
@azerupi travis was taking a while, and in the meantime, I figured out how to make it one pass. Given that we know it always generates valid tags, we can just do this. LMK if you'd prefer the more explicit version with the extra passes. |
thanks @BurntSushi ❤️
|
Looks good to me, thanks! :) |
|
@azerupi any chance we can get a crates.io release for this? We need it to ship rust-lang/rust#39855 |
|
@steveklabnik Done :) |
|
❤️ |
This version of mdbook includes https://github.com/azerupi/mdBook/pull/207 , which is needed so that we can start doing linkchecker on the various books.
Update mdbook version This version of mdbook includes https://github.com/azerupi/mdBook/pull/207 , which is needed so that we can start doing linkchecker on the various books.
Fixes #204
/cc @frewsxcv