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: get back missing crate-name when --playground-url is used #37911

Merged
merged 3 commits into from
Dec 1, 2016

Conversation

liigo
Copy link
Contributor

@liigo liigo commented Nov 21, 2016

follow up PR #37763
r? @alexcrichton (since you r+ed to #37763 )


Edit: When #![doc(html_playground_url="")] is used, the current crate name is saved to PLAYGROUND, so rustdoc may generate extern crate NAME; into code snips automatically. But when --playground-url was introduced in PR #37763, I forgot saving crate name to PLAYGROUND. This PR fix that.


Update:

  • add test
  • unstable --playground-url

@alexcrichton
Copy link
Member

Thanks for the PR, but could you describe what the bug is and how this is fixing it?

@liigo
Copy link
Contributor Author

liigo commented Nov 22, 2016

@alexcrichton I've updated the description of the PR. Thank you!

@ollie27
Copy link
Member

ollie27 commented Nov 23, 2016

My concerns from the previous PR still apply #37763 (review). At the very least this feature needs tests.

@liigo
Copy link
Contributor Author

liigo commented Nov 23, 2016

This pr simply fixes a bug found in previous pr.
I've tested by manual, and ensure it works well as before. But writing test harness code is difficult in this case IMHO.

@alexcrichton
Copy link
Member

Yeah I do agree that if we're fixing bugs it's best if we have some tests for this. Could some tests be added to src/test/rustdoc?

@liigo
Copy link
Contributor Author

liigo commented Nov 25, 2016

Ok, I'll try to add some tests. Maybe need a little more time.

@ollie27
Copy link
Member

ollie27 commented Nov 25, 2016

@liigo you should be able to do something like https://github.com/rust-lang/rust/blob/master/src/test/rustdoc/playground.rs but with // compile-flags: --playground-url=https://www.example.com/.

@alexcrichton shouldn't new command line options for rustdoc at least start out as unstable?

@liigo
Copy link
Contributor Author

liigo commented Nov 25, 2016

Thank you!

@alexcrichton
Copy link
Member

Ah yes actually I forgot that we had unstable options in rustdoc now. Can we make this an unstable option at the same time?

@liigo
Copy link
Contributor Author

liigo commented Nov 28, 2016

@alexcrichton Yes, Will do.

@liigo
Copy link
Contributor Author

liigo commented Nov 28, 2016

Done. r? @alexcrichton

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Nov 28, 2016

📌 Commit ae94007 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Nov 29, 2016

⌛ Testing commit ae94007 with merge 80ec92e...

@bors
Copy link
Contributor

bors commented Nov 29, 2016

💔 Test failed - auto-mac-32-opt

@liigo
Copy link
Contributor Author

liigo commented Nov 30, 2016

updated to pass the tests

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Dec 1, 2016

📌 Commit d5785a3 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Dec 1, 2016

⌛ Testing commit d5785a3 with merge 827eba4...

bors added a commit that referenced this pull request Dec 1, 2016
rustdoc: get back missing crate-name when --playground-url is used

follow up PR #37763
r? @alexcrichton (since you r+ed to #37763 )

----

Edit: When `#![doc(html_playground_url="")]` is used, the current crate name is saved to `PLAYGROUND`, so rustdoc may generate `extern crate NAME;` into code snips automatically. But when `--playground-url` was introduced in PR #37763, I forgot saving crate name to `PLAYGROUND`. This PR fix that.

----

Update:
- add test
- unstable `--playground-url`
@bors bors merged commit d5785a3 into rust-lang:master Dec 1, 2016
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