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

Explain alarmed ferris convention in chapter 1 for code that doesn't compile on purpose #55

Closed
diptanu opened this issue Jan 3, 2016 · 24 comments · Fixed by #1505
Labels
Milestone

Comments

@diptanu
Copy link

@diptanu diptanu commented Jan 3, 2016

This example from Learn Rust doesn't work -

extern crate rand;

use std::io;
use std::cmp::Ordering;
use rand::Rng;

fn main() {
    println!("Guess the number!");

    let secret_number = rand::thread_rng().gen_range(1, 101);

    println!("The secret number is: {}", secret_number);

    println!("Please input your guess.");

    let mut guess = String::new();

    io::stdin().read_line(&mut guess)
        .ok()
        .expect("failed to read line");

    println!("You guessed: {}", guess);

    match guess.cmp(&secret_number) {
        Ordering::Less    => println!("Too small!"),
        Ordering::Greater => println!("Too big!"),
        Ordering::Equal   => println!("You win!"),
    }
}

I am using Rust 1.5 on OSX and the example doesn't work on Rust playground either.

@diptanu diptanu closed this Jan 3, 2016
@diptanu

This comment has been minimized.

Copy link
Author

@diptanu diptanu commented Jan 3, 2016

Ha Ha! Just realized that the example isn't suppose to work :) Sorry for jumping the gun here!

@jingweno

This comment has been minimized.

Copy link

@jingweno jingweno commented Jun 13, 2016

I met the same issue. Why isn't the example supposed to work? There should be an update on the latest rust. Here's how I got it work with 1.9 on OSX:

extern crate rand;

use std::io;
use std::cmp::Ordering;
use rand::Rng;

fn main() {
    println!("Guess the number!");

    let secret_number = rand::thread_rng().gen_range(1, 101);

    println!("The secret number is: {}", secret_number);

    println!("Please input your guess.");

    let mut guess = String::new();

    io::stdin()
        .read_line(&mut guess)
        .ok()
        .expect("failed to read line");

    println!("You guessed: {}", guess);

    let input: u32 = guess.trim().parse().ok().expect("wanted a number");
    match input.cmp(&secret_number) {
        Ordering::Less => println!("Too small!"),
        Ordering::Greater => println!("Too big!"),
        Ordering::Equal => println!("You win!"),
    }
}
@jingweno

This comment has been minimized.

Copy link

@jingweno jingweno commented Jun 13, 2016

Yeah...saw the next couple lines to explain why it wasn't compiled and gave the right answer...I guess this is common mistakes made by the readers though

@synthmeat

This comment has been minimized.

Copy link

@synthmeat synthmeat commented Aug 22, 2016

I'd consider reopening this one, since I've stumbled upon this one too.

Potential fix is to immediately after the example mention that it doesn't compile. Now, the mention of it just too far away from the sample code.

@carols10cents carols10cents reopened this Aug 22, 2016
@carols10cents

This comment has been minimized.

Copy link
Member

@carols10cents carols10cents commented Aug 22, 2016

So I was looking at some other programming books for inspiration-- and I found this in a few places in Land of Lisp. Code examples that have problems start with a comment that says Warning! Contains bugs!:

I kind of like this-- I would put a comment that says Warning! Doesn't compile! for examples that don't compile, and possibly also Warning! Contains bugs! if the code compiles but doesn't do quite what we want it to do.

Would that be obvious enough, do yinz think?

@carols10cents carols10cents changed the title Example from Learn Rust doesn't work Make it clearer when an example is not supposed to compile Aug 22, 2016
@carols10cents carols10cents added this to the all milestone Aug 24, 2016
@carols10cents

This comment has been minimized.

Copy link
Member

@carols10cents carols10cents commented Feb 20, 2017

Ooooooh just had a fun idea! I have this BASIC programming book that was one of my first programming books, and it has these little aliens that appear in the margins:

2017-02-20 13 16 07

What if we had Ferris in the margin looking kind of alarmed D: instead of his usual :D face when a code example intentionally doesn't compile??? I think that would be cute :)

@jugglerchris

This comment has been minimized.

Copy link

@jugglerchris jugglerchris commented Mar 27, 2017

This is only a "me too" comment, but my 10 year old started going through the book online and had exactly the same experience trying to work out why this example wasn't compiling before moving on (to the text which explained that it wasn't supposed to). I like the idea of Ferris in the margin!

@budziq

This comment has been minimized.

Copy link

@budziq budziq commented Jun 5, 2017

Hi @carols10cents!

Are you still interested in this feature? I have some working knowledge of MDBook internals and implementing it would be quite straightforward. MDBook passes all rust playpen attributes (no_run, ignore, etc.) as html class attributes so these can be used as css selectors.

Basically one would have to annotate the examples expected not to compile with

```rust,no_run,super_duper_expect_error_flag

instead of standard

```rust,no_run

Then we could add a nice angry ferris icon (and possibly tint the code background a little redish but this would actually need to provide a version for each theme)

Here is how my POC looks like in case of rust-cookbook:
angry

This would require either pulling and modifying book.js from MDbook or minor changes upstream (I would need to discuss with @azerupi If he would be willing to merge such code)

Please note that I have zero web dev experience (C++ to Rust convert here ;)) so any output of my work might need to be rewritten by someone with actual competence :)

@carols10cents

This comment has been minimized.

Copy link
Member

@carols10cents carols10cents commented Jun 5, 2017

@budziq yes!! I would be interested in this!! I love the angry ferris :)

@budziq

This comment has been minimized.

Copy link

@budziq budziq commented Jun 5, 2017

Thanks 😄. I've gotten the ferris from some old reddit https://www.reddit.com/r/rust/comments/6c1y80/i_made_a_rust_emoji_for_our_slack_group/ so for now it's just a stub (I've no Idea what are the IP rights for it) so someone would have to provide an image that would be both fun and unencumbered.

When required changes to MDBook are merged I'll create a PR.

@carols10cents

This comment has been minimized.

Copy link
Member

@carols10cents carols10cents commented Jun 5, 2017

@steveklabnik do you think whoisaldeka would make us an alarmed ferris? :3

@steveklabnik

This comment has been minimized.

Copy link
Member

@steveklabnik steveklabnik commented Jun 5, 2017

Maybe, let's ask her. @aldeka, what do you think of the above?

@aldeka

This comment has been minimized.

Copy link

@aldeka aldeka commented Jun 10, 2017

Just saw this--sure!

But sure from context--are you thinking of having just one less-than-happy ferrises, or two (won't compile versus has bugs)?

@budziq

This comment has been minimized.

Copy link

@budziq budziq commented Jun 10, 2017

On somewhat related note, there are some code examples that are not really meant to be compiled or run like these.

Running them provides no real output. Showing compiler version at best and dead code warnings at worst which might be nearly as confusing as hard errors. Identical annotations could be used to hide the play/run button for these.

@setharnold

This comment has been minimized.

Copy link

@setharnold setharnold commented Jun 15, 2017

Please also consider cases beyond "not meant to compile" to include "not idiomatic" and "not secure".

Thanks

@budziq

This comment has been minimized.

Copy link

@budziq budziq commented Jun 17, 2017

Also examples that are marked should_panic like this one should have a visual cue.

Which gives us a list of:

  • does not compile
  • should / will panic
  • unsafe / unsecure
  • not idiomatic - I don't know if the book does / should contain these

Imho single graphical representation for all of these could suffice as long as the warning ferris icon would provide an informative poppup and / or a hyperlink to a description (preferably at the begining of the book describing visual ques and icon meanings)

@budziq

This comment has been minimized.

Copy link

@budziq budziq commented Jun 17, 2017

@carols10cents we've merged the required changes to mdBook (although the next release will be a big one and still needs some polish before it's available on crates.io). As long as I get a general wishlist and preliminary graphics for this feature I can start working on some preliminary versions for testing.

@aldeka

This comment has been minimized.

Copy link

@aldeka aldeka commented Jun 18, 2017

gesture

does-not-compute

panic

unsafe

@aldeka

This comment has been minimized.

Copy link

@aldeka aldeka commented Jun 18, 2017

^ some ferrises, can provide SVGs of same.

Does the book require grayscale or b&w images?

@budziq

This comment has been minimized.

Copy link

@budziq budziq commented Jun 18, 2017

@aldeka Absolutely lovely 😍

Svg sources would be great so not to bother you with minor details (like question mark positioning etc.)
Also is there a chance for variant with exclamation point? Or just an svg of the exclamation piont in the same font. Most likely we will need light and dark theme variants anyway so svg's would be best.

@carols10cents

This comment has been minimized.

Copy link
Member

@carols10cents carols10cents commented Jun 18, 2017

eeeeee so cute!!!!!!!!!!!!

Does the book require grayscale or b&w images?

I'm not sure yet, I will ask. I'm not even sure if we can do this in print yet... there are... things.... going on right now that have prevented me from bringing it up. I will do so soon though and let you know!!

@steveklabnik

This comment has been minimized.

Copy link
Member

@steveklabnik steveklabnik commented Jun 19, 2017

ahhh these are awesome

@aldeka

This comment has been minimized.

Copy link

@aldeka aldeka commented Jun 19, 2017

Hmm, I don't seem to be able to attach an SVG here.

@aldeka

This comment has been minimized.

Copy link

@aldeka aldeka commented Jun 19, 2017

Here's the SVGs: https://github.com/aldeka/rustacean.net/tree/master/site/more-crabby-things/rustbook-emotes

Also I modified the unsafe rustacean a little to hopefully look more like a sea urchin.
unsafe

@carols10cents carols10cents modified the milestones: all, ch1 Oct 25, 2017
@carols10cents carols10cents changed the title Make it clearer when an example is not supposed to compile Explain alarmed ferris convention in chapter 1 for code that doesn't compile on purpose Oct 25, 2017
@u32i64 u32i64 referenced this issue Aug 28, 2018
8 of 8 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
9 participants
You can’t perform that action at this time.