Fix #419 (check all whitelisted examples)

[strega-nil]

I didn't do anything about print.rs, this is a pure bugfix

[rust-highfive]

r? @steveklabnik

(rust_highfive has picked a reviewer for you, use r? to override)

[strega-nil]

The failure is due to rustup being down.

Also, hello again, sorry I've been away for so long 😄

[mdinger]

Nice

[steveklabnik]

kicked off a new build, let's see how it goes

[mdinger]

Could do (0..10).collect() instead of range(0, 10). I'm not sure if the parentheses will need to stay for long or not. They're needed currently though.

[strega-nil]

Honestly, I think we should pull these "whitelisted" examples out, and remake them as "what you should do", not what you shouldn't. Any thoughts?

[mdinger]

I would deal with it on a case by case basis but in general I would agree. I don't like an example failing when I first look at it. I like the error cases being labeled as such and the fixes being specified though. So commented out errors I like.

Skipped tests make out of date problems too. Kinda bad.

[strega-nil]

@mdinger I'm currently attempting to gut the whitelist; however, I've come up against a problem. examples/crates/link/executable.rs requires me to link against an external crate, but I can't think of any that are still in the prelude (don't need Cargo). You have any ideas?

[mdinger]

Kinda reminds me of rust-lang/rust-playpen#32 but it isn't about getting cargo to work in playpen. Just getting flags working.

You could do this but I don't think it's a good idea. Way out of the way for a simple example. It does work though.

// Ban `std` so we can import it manually for demonstration
#![no_std]

// Need to import `println!` also
#[macro_use (println)]
extern crate std;

// Need some tool for an example
use std::vec::Vec;

fn main() {
    let mut v = Vec::new();
    v.push(3);
    v.push(4);

    println!("{:?}", v);
}

Oh, I know how you could test this. Upgrade make test so that when non-runnable code gets tested, it gets cloned and cargoified for local testing but inserted as normal non-runnable code in the webpage. Regular code gets built as normal.

I don't know how easy that would be.

[strega-nil]

What do you mean? make test currently just rustcs every .rs file in
the examples directory. How would you change that?

On Wed, Jan 28, 2015 at 10:17 PM, mdinger notifications@github.com wrote:

Kinda reminds me of rust-lang/rust-playpen#32
rust-lang/rust-playpen#32 but it isn't about
getting cargo to work in playpen. Just getting flags working.

You could do this but I don't think it's a good idea. Way out of the way
for a simple example. It does work though.

// Ban std so we can import it manually for demonstration#![no_std]
// Need to import println! also#[macro_use (println)]extern crate std;
// Need some tool for an exampleuse std::vec::Vec;
fn main() {
let mut v = Vec::new();
v.push(3);
v.push(4);

println!("{:?}", v);

}

Oh, I know how you could test this. Upgrade make test so that when
non-runnable code gets tested, it gets cloned and cargoified for local
testing but inserted as normal non-runnable code in the webpage. Regular
code gets built as normal.

I don't know how easy that would be.

—
Reply to this email directly or view it on GitHub
#433 (comment)
.

"Calling all. This is our last cry before our eternal silence."
"The lone and level sands stretch far and away"

[mdinger]

I think you could use some shell scripting where my comment is to do it. I'd have to play with it for a while before I could get any real syntax layed down for it. The makefile test example:

test:
@$(foreach src,$(srcs),$(RUSTC_NT) $(src) || exit;)
// Do the above line again but this time only on the whitelisted files.
// This time, copy the folder structure and the cargo.toml in the folder to
// `cargotest/same/folder/structure/src` and place toml in up one directory.
// Do `cargo run` on the file to test. So, replace `$(RUSTC_NT)` with a function
// (or functionality) that does the cloning and running functionality. Call it
// on each item in the whitelist
./check-line-length.sh

[EDIT] You'd need to create a cargo.toml file for each example too.

[mdinger]

I think I have a better way than this. Testing it out now

[mdinger]

@GBGamer Look at mdinger@3a0f174

This adds one of the whitelisted examples to the Cargo.toml as an example. There, it has access to external sources and cfg and other things. Examples get run with cargo test but as they are run, and not tested, cfg(test) doesn't work on them.

This doesn't scale well for all the examples because you have to list them all explicitly. It would work well to replace some of the whitelisted examples though because there are only a few.

[mdinger]

I looked into what it would take to completely switch over to cargo test and remove the structure.json file because it'd basically be a duplicate of the Cargo.toml examples. It's a bit of effort:

• Cargo.toml's have this format:

[[example]]
name = "first"
path = "examples/more/more.rs"

[[example]]
name = "second"
path = "examples/morer/morer.rs"

This would be tedious for rbe's 100 examples. toml has had PR toml-lang/toml#235 to support more compact tables up for 6 months so no telling if/when it will land. Syntax like this would be really nice here:

examples = [
    { name = "foo", path = "examples/foo/main.rs" },
    { name = "foo", path = "examples/foo/main.rs" },
    { name = "foo", path = "examples/foo/main.rs" },
    { name = "foo", path = "examples/foo/main.rs" },
    ...
]

• Hard to see the document strucure without the more compact tables
• toml-rs works fine for decoding the examples. Seems a little awkward but that's the only issue.
• I'm not sure if it preserves the original order which would be important for the structure of the document
• I'm not sure how to write toml code to exactly match the json structure. Mapping back and forth is confusing.
• Big transition from structure.json if any major change in encoding was done. For example:
  • I thought path to the file would be easier to work with than using recursive file structure as the JSON does. This would modify a fair bit of code.
  • Least invasive would be to preserve the structure in toml
  • Stating the path and using recursive structure is pretty redundant
• Not all pages have code *.rs files and *.rs files are what cargo is interested in. Not sure if this would be a problem or not.
• Would simplify the Makefile
• Would allow all examples to be tested
• Would allow all examples access to cargo but
  • Cargo examples couldn't use playpen
• Would be a complex PR and
  • Probably wouldn't get reviewed quickly
  • Might get rejected for some reason

---

That's all I can think of currently.

[strega-nil]

@steveklabnik, your thoughts? Should we continue with this?
On Jan 29, 2015 9:14 PM, "mdinger" notifications@github.com wrote:

I looked into what it would take to completely switch over to cargo test
and remove the structure.json
https://github.com/rust-lang/rust-by-example/blob/master/examples/structure.json
file because it'd basically be a duplicate of the Cargo.toml examples.
It's a bit of effort:

• Cargo.toml's have this format:

[[example]]
name = "first"
path = "examples/more/more.rs"

[[example]]
name = "second"
path = "examples/morer/morer.rs"

This would be tedious for rbe's 100 examples. toml has had a PR to
support more compact tables for 6 months so no telling if/when it will
land. Syntax like this would be really nice here:

examples = [
{ name = "foo", path = "examples/foo/main.rs" },
{ name = "foo", path = "examples/foo/main.rs" },
{ name = "foo", path = "examples/foo/main.rs" },
{ name = "foo", path = "examples/foo/main.rs" },
...
]

• Hard to see the document strucure without the more compact tables
• toml-rs works fine for decoding the examples. Seems a little awkward
  but that's the only issue.
• I'm not sure if it preserves the original order which would be
  important for the structure of the document
• I'm not sure how to write toml code to exactly match the json
  structure. Mapping back and forth is confusing.
• Big transition from structure.json if any major change in encoding
  was done. For example:
  • I thought path to the file would be easier to work with than
    using recursive file structure as the JSON does. This would modify a fair
    bit of code.
  • Least invasive would be to preserve the structure in toml
  • Stating the path and using recursive structure is pretty redundant
    • Not all pages have code *.rs files and *.rs files are what cargo is
      interested in. Not sure if this would be a problem or not.
• Would simplify the Makefile
• Would allow all examples to be tested
• Would allow all examples access to cargo but
  • Cargo examples couldn't use playpen
    • Would be a complex PR and
  • Probably wouldn't get reviewed quickly
  • Might get rejected for some reason

---

That's all I can think of currently.

—
Reply to this email directly or view it on GitHub
#433 (comment)
.

[steveklabnik]

I just got off of like 14 hours of flights and have a talk to present about on Sunday, so I haven't been able to review at all, sorry :/

(also, technically i'm on vacation this week)

[mdinger]

If you can rewrite a few specific examples better with cargo support then I think it's a good idea to do them that way. Until toml supports the more compact tables I don't think it's a good idea to transfer entirely to cargo. The structure just isn't as clear.

So, I think global change decisions should wait until the compact syntax exists.

[mdinger]

I filed #439 to continue this discussion as it's a little off topic here. This PR should be landable fairly easily.

[steveklabnik]

Yes, I'm comfortable landing this in the meantime. Thank you.