Skip to content
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

Make attributes visible in c-tips chapter #212

Conversation

austinglaser
Copy link

Fixes #211

@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 @andre-richter (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.

@rust-highfive rust-highfive added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-resources labels Nov 11, 2019
@andre-richter
Copy link
Member

In my browser (Firefox) they are rendered:

image

@adamgreig
Copy link
Member

I also see them render OK in Chrome; what browser are you using @austinglaser ?

@austinglaser
Copy link
Author

Firefox 70.0.1, on ubuntu. I don't have any custom styles enabled.

If you build from my branch, does the chapter mis-render on your browsers?

@andre-richter
Copy link
Member

Firefox 70.0.1, on ubuntu. I don't have any custom styles enabled.

If you build from my branch, does the chapter mis-render on your browsers?

Same browser here, to the last digit. Ubuntu 64bit.

@austinglaser
Copy link
Author

If I look at the page source for one of the offending examples, I see:

<pre><pre class="playpen"><code class="language-rust">
<span class="boring">#![allow(unused_variables)]
fn main() {
</span>/// In your top-level lib.rs

<span class="boring">[cfg(feature=&quot;FIR&quot;)]
</span>pub mod fir;

<span class="boring">[cfg(feature=&quot;IIR&quot;)]
</span>pub mod iir;
<span class="boring">}
</code></pre></pre>

What do you see there on your system?

@austinglaser
Copy link
Author

I see the same behavior (and the same static html, including boring spans) when building master locally with mdbook v0.3.4. Do I need to provide any additional arguments/configuration to mdbook other than mdbook serve in the root directory?

I've tried explicitly clearing my browser's cache, and I see the same page.

I'm looking at https://rust-embedded.github.io/book/c-tips/index.html. Is there another hosting location for the book that could cause this confusion?

Are there other things I could check? Does mdbook use rustdoc internally -- could that be a source of version mismatches? The rendering you're seeing seems like it's likely to come from a newer version of the tool which is smarter about attributes -- but the fact that I see this on the public site would seem to indicate that it's unlikely to be solely an issue with tooling versions on my system.

@adamgreig
Copy link
Member

adamgreig commented Nov 11, 2019

Ah, I'm looking at https://docs.rust-embedded.org/book/c-tips/index.html which renders OK, whereas your link to rust-embedded.github.io does not render correctly (with the attributes missing).

docs.rust-embedded.org is a CNAME for rust-embedded.github.io though, so it's not immediately clear to me why they're different...

@austinglaser
Copy link
Author

There it is! Your link renders fine for me as well.

That probably means we should hold off on merging this until verifying that the changes render correctly when they eventually get pushed to the rust-embedded.org site.

Also, I've found a couple other places in the book where attributes are mistakenly getting swallowed -- I'll add them to this PR as well.

@adamgreig
Copy link
Member

adamgreig commented Nov 11, 2019

This repository is configured to serve this repository's gh-pages branch at the rust-embedded.github.io link, and the gh-pages clearly has the boring spans: https://github.com/rust-embedded/book/blob/gh-pages/c-tips/index.html

Meanwhile the docs repository's gh-pages branch is hosted at docs.rust-embedded.org and also has CI build this repository, which does not have the boring tags: https://github.com/rust-embedded/docs/blob/gh-pages/book/c-tips/index.html

I suspect the problem is that docs uses mdbook 0.2.1:

https://github.com/rust-embedded/docs/blob/master/ci/install.sh

while book uses latest:

https://github.com/rust-embedded/book/blob/master/ci/install.sh

Edit: in fact it looks like I made the commit that stuck docs on 0.2.1, and the fix for book never made it through to docs.

So it looks like we should have docs start using latest mdbook, and we need to fix all the attributes in book (and perhaps in discovery) to keep them showing up.

@austinglaser
Copy link
Author

austinglaser commented Nov 11, 2019

I've opened a PR to make the mdbook on docs the same as book. Additionally, I've fixed what I think are all the occurrences of erroneously-swallowed code in examples (at least rg boring book after a build doesn't turn up any offenders)

@austinglaser
Copy link
Author

It looks like this is fixed in mdbook v0.3.5: rust-lang/mdBook#1088

@adamgreig
Copy link
Member

Thanks for chasing this down, @austinglaser!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-resources
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Examples in intro for C developers lack attributes
5 participants