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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ferris Technology #1505

Merged
merged 13 commits into from Sep 18, 2018

Conversation

@u32i64
Copy link
Contributor

u32i64 commented Aug 28, 2018

This fixes #55 馃帀

Screenshots
does_not_compile

ferris_does_not_compile

panics

Note: this is different from the already existing should_panic attribute which is used for testing the code in CI - since some code blocks should panic in CI, but don't always do that when testing locally, and thus they do not need a panicking Ferris.

ferris_panics

unsafe

Note: unsafe code is not tinted red, according to the book:

Using unsafe to take one of the four actions (superpowers) just discussed isn鈥檛 wrong or even frowned upon.

ferris_unsafe

not_desired_behavior

ferris_not_desired_behavior

Progress

  • Add Ferris Technology (js, css).
  • Add appropriate attributes to code blocks in the book.
  • Tooltips on Ferris images.
  • Explain what alarmed Ferris means in the Introduction (and make Ferrises link to the explanation).

Unresolved questions/issues

  • 1. Do code blocks which are there to show the behavior that will be implemented later in the chapter need to be annotated as not compiling (e.g. in chapters 05, 12, 17, 20)?

  • 2. Ferris positioning when there is less than 4 lines of code (sometimes it's ok, but if there is a lot of text below the code block, Ferris is covering part of it):

    Screenshot of the issue

    ferris

  • 3. Background colors for each theme. Current:

    • Light: #fff1f1
    • Rust: #fff1f1
    • Coal: #501f21
    • Navy: #501f21
    • Ayu: #501f21
  • 4. In dark themes, does_not_compile Ferris' question mark is hard to distinguish from the background.

@u32i64

This comment has been minimized.

Copy link
Contributor Author

u32i64 commented Aug 29, 2018

@carols10cents ^ any thoughts?

@aldeka Is there any chance we can get a white outline on the ? in the does_not_compile Ferris? I think it should then be easily distinguishable from the bg in any theme.

@steveklabnik

This comment has been minimized.

Copy link
Member

steveklabnik commented Sep 17, 2018

This is amazing, thank you so much! I'm sorry it's taken so long to get back to you; I had rustconf and then a vacation. Carol's been on vacation too.

Let's try to fix up these issues and ship this! I'm so excited!

  1. Do code blocks which are there to show the behavior that will be implemented later in the chapter need to be annotated as not compiling (e.g. in chapters 05, 12, 17, 20)?

I think leaving it purely as the status at that point is the right call.

  1. Ferris positioning when there is less than 4 lines of code (sometimes it's ok, but if there is a lot of text below the code block, Ferris is covering part of it):

Yeah, that's tough. Is there an easy way to maybe, just hide ferris in that case?

  1. Background colors for each theme.

I'm happy with whatever you decide looks best.

  1. In dark themes, does_not_compile Ferris' question mark is hard to distinguish from the background.

Ah, that's true. Hm. We already have some issues with this kind of thing and other images, so I don't consider it a blocker.

@u32i64 u32i64 changed the title [WIP] Ferris Technology Ferris Technology Sep 18, 2018
@u32i64

This comment has been minimized.

Copy link
Contributor Author

u32i64 commented Sep 18, 2018

Thanks for your reply!

I think I've fixed the second issue, Ferris shouldn't appear on code blocks with <= 4 lines of code anymore.
Regarding the colors, I think we can leave them as is and tweak when and if needed.

So if I understand you correctly, we are ready to merge right?

@steveklabnik

This comment has been minimized.

Copy link
Member

steveklabnik commented Sep 18, 2018

So if I understand you correctly, we are ready to merge right?

Yup! This looks fantastic! Thank you so much!

@steveklabnik steveklabnik merged commit fa91738 into rust-lang:master Sep 18, 2018
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
steveklabnik added a commit to steveklabnik/rust that referenced this pull request Sep 18, 2018
kennytm added a commit to kennytm/rust that referenced this pull request Sep 20, 2018
Update The Book to latest

Let's check out rust-lang/book#1505 on nightly.
kennytm added a commit to kennytm/rust that referenced this pull request Sep 20, 2018
Update The Book to latest

Let's check out rust-lang/book#1505 on nightly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can鈥檛 perform that action at this time.