Skip to content

Update mdBook to 0.1.7 #1080

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

Closed
wants to merge 2 commits into from
Closed

Conversation

mattico
Copy link

@mattico mattico commented May 14, 2018

cc rust-lang/mdBook#693 https://github.com/ruRust/rust-by-example-ru/issues/120

I'd like to make these buttons user-configurable so you don't need to have a fork of the index.hbs. See rust-lang/mdBook#694

mattico added 2 commits May 14, 2018 11:26
@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @steveklabnik (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

Copy link
Member

@steveklabnik steveklabnik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given my comment, I'm not sure what the rest of this PR is doing, exactly; is it updating our fork of index.hbs to the latest upstream version? This doesn't do what it says you want, which is to get us off of our fork (I agree that would be nice).

@@ -14,7 +14,7 @@ If you'd like to read it locally, [install Rust], and then:
```bash
$ git clone https://github.com/rust-lang/rust-by-example
$ cd rust-by-example
$ cargo install mdbook
$ cargo install --version 0.1.7 mdbook
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since mdbook is already at 0.1.7, this doesn't change things; it actually pins it to that specific version. we already use 0.1.7 https://github.com/rust-lang/rust/blob/master/src/tools/rustbook/Cargo.toml#L11 to render the book.

@mattico
Copy link
Author

mattico commented May 14, 2018

The template used for index.hbs is very tightly coupled to the version of mdbook. If the version doesn't match you can get strange issues. Because Rust by Example uses a custom index.hbs, it may not work properly with past or future versions of mdbook, and the readme should recommend users to install the version that works.

So then why doesn't the Rust By Example hosted by rust-lang experience any strange issues? It turns out that the theme override is broken (rust-lang/mdBook#695) so the custom index.hbs isn't being used at all. If you add

[output.html]
theme = "theme"

to book.toml, the book does render incorrectly.

Given this new information, this PR doesn't actually do anything. I'll send another similar one once 0.1.8 is out with a fix for the above issue.

@mattico mattico closed this May 14, 2018
@mattico mattico deleted the mdbook-update branch May 14, 2018 19:18
@steveklabnik
Copy link
Member

steveklabnik commented May 14, 2018 via email

@mattico
Copy link
Author

mattico commented May 15, 2018

See also the edition guide, which seems to exhibit the above mentioned problems: https://rust-lang-nursery.github.io/edition-guide/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants