-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Handle input path with regards to custom css #598
Conversation
Before, when someone like the Reference set their extra css as "theme/reference.css" in their book.toml, this path would be treated as relative to the invocation of mdbook, and not respect the input path. This PR modifies these relative paths to do so. Fixes the build of rust-lang/rust#47753 which blocks updating rustc to mdbook 0.1
apparently this breaks links: https://travis-ci.org/rust-lang/rust/builds/334450214#L2554 |
Oops, I think this might have slipped in when trying to break the HTML renderer's |
I suspect that on nested pages, the relative links break. that's my guess though, I haven't dug into the output yet, and am not likely to be able to till tomorrow |
Hmm, that's interesting. I don't know how css loading interacts with the cc #597 |
I bet the linkchecker isn't taking |
Is there anything we can do to deal with that in this PR, or would it be a problem to solve by updating the linkchecker directly? |
hm, i just checked linkchecker and it does at least have code in it to handle base tags. gotta dig deeper. |
it is just generating a bad link. i was solely looking at the switcher stuff; but it turns out that
is being generated rather than
|
this happens because of this code in hbs_renderer // Add check to see if there is an additional style
if !html.additional_css.is_empty() {
let mut css = Vec::new();
for style in &html.additional_css {
match style.strip_prefix(root) {
Ok(p) => {
css.push(p.to_str().expect("Could not convert to str"))
},
Err(_) => {
css.push(style.file_name()
.expect("File has a file name")
.to_str()
.expect("Could not convert to str"))
}
}
}
data.insert("additional_css".to_owned(), json!(css));
} because the style name is Is the right thing to do to just not push the file_name? gonna give that a try. |
the style name is theme/reference.css, this results in a Err(StripPrefixError(())), which means that we push only the file_name, losing the theme bit
okay, so with this latest code, stuff seems to build |
I'd say it should push the path (like you've done), not just the Let me know when the builds are working again and I'll create another point release. |
the failure in the current build isn't related to this stuff; we should be good to |
yup, all green now! |
❤️ any chance of a release any time soon, @Michael-F-Bryan ? |
FWIW, EDIT: I think this would also mean that the |
I'm at work at the moment, but when I get home I'll make another release. Is there anything else |
Nope! as you can see, the latest commit uses mdbook master at that particular hash. 👍 ❤️ |
* Handle input path with regards to custom css Before, when someone like the Reference set their extra css as "theme/reference.css" in their book.toml, this path would be treated as relative to the invocation of mdbook, and not respect the input path. This PR modifies these relative paths to do so. Fixes the build of rust-lang/rust#47753 which blocks updating rustc to mdbook 0.1 * don't use file-name the style name is theme/reference.css, this results in a Err(StripPrefixError(())), which means that we push only the file_name, losing the theme bit
Before, when someone like the Reference set their extra css as
"theme/reference.css" in their book.toml, this path would be treated as
relative to the invocation of mdbook, and not respect the input path. This
PR modifies these relative paths to do so.
Fixes the build of rust-lang/rust#47753 which
blocks updating rustc to mdbook 0.1
It'd be nice to get a release cut with this fix so we can ship that PR!