-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Implement playpen support for ```rust #222
Conversation
let text = &caps[1]; | ||
let classes = &caps[2]; | ||
|
||
if classes.contains("language-rust") && !classes.contains("ignore") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about should_panic? i can't decide whether not having a run button or having a run button that then causes a panic would be more confusing, just want to make sure we consider it :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a good question.
I'm not sure of the answer.
Here's the logic from rustdoc: pub fn maketest(s: &str, cratename: Option<&str>, dont_insert_main: bool,
opts: &TestOptions) -> String {
let (crate_attrs, everything_else) = partition_source(s);
let mut prog = String::new();
// First push any outer attributes from the example, assuming they
// are intended to be crate attributes.
prog.push_str(&crate_attrs);
// Next, any attributes for other aspects such as lints.
for attr in &opts.attrs {
prog.push_str(&format!("#![{}]\n", attr));
}
// Don't inject `extern crate std` because it's already injected by the
// compiler.
if !s.contains("extern crate") && !opts.no_crate_inject && cratename != Some("std
") {
if let Some(cratename) = cratename {
if s.contains(cratename) {
prog.push_str(&format!("extern crate {};\n", cratename));
}
}
}
if dont_insert_main || s.contains("fn main") {
prog.push_str(&everything_else);
} else {
prog.push_str("fn main() {\n");
prog.push_str(&everything_else);
prog = prog.trim().into();
prog.push_str("\n}");
}
info!("final test program: {}", prog);
prog so, doing this too shouldn't be too hard. |
I pushed up a commit to inject mains. The attribute stuff is a little harder, but not necessary. One issue... Because the older style was linking to the playpen, these warnings would just happen, and it did nothing about it. so this isn't strictly speaking a regression. There's still a lot of polish to do here but I think this gets us 99% of the way |
Discussing ⬆️ with @carols10cents , we think auto-injecting |
Done. @azerupi how are you feeling about this PR? |
It's good 👍 Obviously, we can't continue to add special cases like this forever. But until we tackle the bigger architectural changes, it's fine to handle it this way :) |
Awesome, I feel the same way. My intention is to try to look at the bigger stuff after we get all of this little stuff fixed as we go 😄 |
Oh, and do you think we could get a release? We have two or three things now; it'd be good to get this out in front of people |
Well, it's a perfect occasion to test if you can publish to crates.io too ;) |
0.0.18 published! ❤️ |
Fixes #29
So, this all works, and fixes an important regression in mdbook for the rust book.
However.
The big issue remaining is the "wrap in
main
" rules from rustdoc. In other words, something like this:will fail, as it needs to be inside of
main
.I'm not sure what the right way is to go here, exactly. I'm imagining duplicating the logic of rustdoc here makes sense.
Thoughts @azerupi @carols10cents @frewsxcv ?