-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixing links in print.html #866
Conversation
The search index is changed due to the addition of an extra chapter, which means you have to regenerate the
Unfortunately you can't just copy it from a normal book build, as it's not pretty printed. The current code basically concatenates all the files into one big html file and does it as it's rendering all the files. I.e, it renders a file, outputs it to the location it's at, then appends the html into print.html. I was going to use another project I did (https://github.com/cetra3/mdcollate) to generate a correct With mdcollate, I was going to:
Unfortunately there would need to be a few enhancements to mdcollate to support this. Namely, dealing with windows file systems (backslashes!) & converting it into a library. |
Oh wow, I just missed this, thanks :(
Yep, that's the root cause here. The strategy I'm attempting to take is to check if links are relative, and then re-base them. We'll see... |
0e01d71
to
5d93b14
Compare
Okay! While this hasn't built here, things pass locally, as well as testing in the main Rust repository (cc rust-lang/rust#57495) Can someone review this? |
I don't know the mdbook source code well enough to provide a review, but I have kicked the tires on this PR and it seems to work for me. I tossed together a quick script to convert mdbook 0.1 to 0.2: https://gist.github.com/ehuss/b2a647eb69aa8f57bc4fb397656dfac1 I ran it against the reference and I found some curious issues (unrelated to this PR) that maybe someone can answer:
|
@ehuss oh that's super cool! I have to re-read the code, but I think those are both bugs; I'd like to support them, at least. Maybe I can fix those issues quickly and make them a part of this PR as well. I'll investigate later today and report back. |
In a regex, `.` means `[0-9a-zA-Z]`, not a period character. This means that this regex worked, unless a link anchor contained `md` inside of it. This fixes the regex to match a literal period. Reported by @ehuss in #866 (comment)
@ehuss I've pushed a commit to fix the second issue, nice catch! The first issue, |
The print.html page just ends up with a few links that point to the wrong section. The headers get unique anchor names ( The issue I was specifically referring to is if a fragment-only link can elide the page name or not. It's not a big deal, just a minor convenience. The script I posted inserts the page name, so it's an easy workaround. |
I plan on merging this tomorrow and cutting a new release. |
Thanks folks! |
In a regex, `.` means `[0-9a-zA-Z]`, not a period character. This means that this regex worked, unless a link anchor contained `md` inside of it. This fixes the regex to match a literal period. Reported by @ehuss in rust-lang#866 (comment)
Fixing links in print.html
This is only a failing test, not an implementation. Opening it now for two reasons:
this fails two tests right now. @Michael-F-Bryan or someone else, how do I properly regenerate the search index json fixture? I tried to do it manually and it didn't seem to work.
does this test look reasonable?
working on a patch now cc @cetra3 if you have any advice