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

rustdoc: Use --crate-name with --test #16157

Closed
wants to merge 1 commit into from

Conversation

alexcrichton
Copy link
Member

This ensures that the name of the crate is set from the command line for tests
so the auto-injection of extern crate <name> in doc tests works correctly.

@@ -87,7 +88,8 @@ pub fn run(input: &str,

let mut v = RustdocVisitor::new(&*ctx, None);
v.visit(&ctx.krate);
let krate = v.clean();
let mut krate = v.clean();
krate.name = crate_name.unwrap_or(krate.name.clone());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This just seems weird. I'd rather see something like

if crate_name.is_some() { krate.name = crate_name.unwrap(); }

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or match crate_name { Some(n) => krate.name = n, None => {} } or for n in crate_name.move_iter() { krate.name = n } or crate_name.map(|n| krate.name = n).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered suggesting the match, but I figured a multi-line solution like that would not be as desired.

If we had rust-lang/rfcs#160 this would be a good candidate for if let:

if let Some(crate_name) = crate_name { krate.name = crate_name; }

This ensures that the name of the crate is set from the command line for tests
so the auto-injection of `extern crate <name>` in doc tests works correctly.
@alexcrichton
Copy link
Member Author

Updated with a match

bors added a commit that referenced this pull request Aug 1, 2014
This ensures that the name of the crate is set from the command line for tests
so the auto-injection of `extern crate <name>` in doc tests works correctly.
@bors bors closed this Aug 1, 2014
@alexcrichton alexcrichton deleted the rustdoc-tests branch August 1, 2014 16:51
alexcrichton added a commit to alexcrichton/cargo that referenced this pull request Aug 5, 2014
Whenever `cargo test` is run and a testable library target is available, the doc
tests will be run. This can be opted out of with `test = false` as usual.

This is currently not super useful due to rust-lang/rust#16157, but I expect
that to be merged soon. In the meantime examples will need to `extern crate foo`
explicitly.
bors added a commit to rust-lang/cargo that referenced this pull request Aug 6, 2014
Whenever `cargo test` is run and a testable library target is available, the doc
tests will be run. This can be opted out of with `test = false` as usual.

This is currently not super useful due to rust-lang/rust#16157, but I expect
that to be merged soon. In the meantime examples will need to `extern crate foo`
explicitly.

Closes #334
alexcrichton added a commit to alexcrichton/cargo that referenced this pull request Sep 2, 2014
Whenever `cargo test` is run and a testable library target is available, the doc
tests will be run. This can be opted out of with `test = false` as usual.

This is currently not super useful due to rust-lang/rust#16157, but I expect
that to be merged soon. In the meantime examples will need to `extern crate foo`
explicitly.
bors added a commit to alexcrichton/cargo that referenced this pull request Sep 2, 2014
Whenever `cargo test` is run and a testable library target is available, the doc
tests will be run. This can be opted out of with `test = false` as usual.

This is currently not super useful due to rust-lang/rust#16157, but I expect
that to be merged soon. In the meantime examples will need to `extern crate foo`
explicitly.

Closes rust-lang#334
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants