Skip to content

enable play button only if play.rust-lang.org supports used crates. #396

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

Closed
budziq opened this issue Aug 7, 2017 · 9 comments · Fixed by #404
Closed

enable play button only if play.rust-lang.org supports used crates. #396

budziq opened this issue Aug 7, 2017 · 9 comments · Fixed by #404

Comments

@budziq
Copy link
Contributor

budziq commented Aug 7, 2017

Hi,

rust-lang-nursery/rust-cookbook#87 Is a long standing bug in cookbook. We were forced to disabled the (awesome) play button due to its inconsistent behavior.

Firstly play.rust-lang.org did not support extern crate at all.
Now it supports the top 100 crates from crates.io which is a dynamic property.

Having the play button enabled by default is arguably not the best user experience (it's safe for std only snippets but its hit and miss with extern crate. To make it worse, the experience can degrade/upgrade dynamically day by day)

We can query the installed crate list https://play.rust-lang.org/meta/crates on each page load (if playpen snippets present) and selectively enable the button if playpen snippet:

  • is not marked no_run
  • does not use any crates or uses only the available crates

We should be able to get extern crate from snippets easily with some js regex;


Eventually the rust playground might start offering a curated crate list too rust-lang/rust-playground#168

@azerupi
Copy link
Contributor

azerupi commented Aug 7, 2017

I am not sure we should rely on extern crate on the playground 😕
When reading through their issues I noticed two properties that make it very "unstable":

  1. Every day the crate list changes, a crate that worked yesterday may not work today because it is no longer in the list
  2. Only the last version of the crate is used, if a new breaking version is released, the snippet will not compile anymore

Therefore I would recommend to not use this feature in a book. It is highly unreliable for long term documentation.

We can query the installed crate list https://play.rust-lang.org/meta/crates on each page load (if playpen snippets present) and selectively enable the button if playpen snippet

This would stil be confusing for the user, albeit somewhat less, because one day the play button might be there and then the next it could be gone.

@budziq
Copy link
Contributor Author

budziq commented Aug 7, 2017

When reading through their issues I noticed two properties that make it very "unstable":

yep the current situation is not very nice. Abut arguably enabling the play button by default might be even worse because the button will stop working without any warning.

I am not sure we should rely on extern crate on the playground 😕

maybe an opt in variant via options. I for one am able to live with play button disappearing from time to time in the cookbook as it is a very desired feature by the users.

Maybe we can make the play button disabled with tooltip ("unsupported crates" or something) if the playgraund does not support the given crate set.

Also as mentioned earlier we might hope for some stabilization with rust-lang/rust-playground#168

@budziq budziq closed this as completed Aug 7, 2017
@azerupi
Copy link
Contributor

azerupi commented Aug 7, 2017

Why did you close the issue? 😄

I have a proposal that you might like.

requirements

  • We want this to be disabled by default because we don't recommend using it (even if it is there)
  • We want to issue a warning when using it
  • The code should be fairly isolated

proposal
What I propose is to error if we find an extern crate in a playpen block. But add a new optional parameter to {{#playpen ...}} (we should rename that to playground) that allows to turn this error into a warning.

{{#playpen foo.rs extern-crates }}

The warning should state that this feature is highly unstable due to the constraints of the playground and that working snippets may break at any time.

Maybe we can make the play button disabled with tooltip ("unsupported crates" or something) if the playgraund does not support the given crate set.

I like that idea! This would only catch crates that are not in the list and not the ones with incompatible versions but it's already something.

@azerupi azerupi reopened this Aug 7, 2017
@budziq
Copy link
Contributor Author

budziq commented Aug 7, 2017

Why did you close the issue? 😄

Sorry wrong button. I'm a little preoccupied at the moment ;)

{{#playpen foo.rs extern-crates }}

please note that a major usecase for playgraund are the inline markdown rust snippets. For instance cookbook does not use the {{#playpen foo.rs extern-crates }} style playgrounds (but we could solve the problem similarly).

I guess that from book writer perspective (as cookbook mainter) I would prefer to have a global opt in option in book.toml instead of per snippet basis (this would add a lot of noise).

Also where would you prefer to for the erroring/warning to occur? In mdbook test? I would prefer not to make such inference and decision on build/serve step. I guess that such check would be on the level of link checker / spell checker (a specialized test scenario that I would prefer to make as a nice to have feature orthogonal to this one)

This would only catch crates that are not in the list and not the ones with incompatible versions but it's already something.

I certainly would not like to go into versioning rabbit hole ;). I feel that It is book authors/maintainers role to keep the examples compiling and in sync using mdbook test or skeptic and if they would like to freeze on speccific version they can always use no_run to disable play button.

@azerupi
Copy link
Contributor

azerupi commented Aug 7, 2017

please note that a major usecase for playgraund are the inline markdown rust snippets.

I am not sure I understand what you mean by that, do you mean they use it just as an include?

@budziq
Copy link
Contributor Author

budziq commented Aug 7, 2017

Sorry (as I said I'm multitasking now and the results are less than coherent 😸 ).

What meant is that the play button appears also in the markdown rust snippets

```rust
fn main(){
 println!("blah!");
}
```

So any functionality added to {{#playpen foo.rs}} style playground links would also have to be ported to markdown rust snippets.

@azerupi
Copy link
Contributor

azerupi commented Aug 7, 2017

Oh yes, you are right. I forgot that...
Now that we are in this situation, I think this was an arguably bad decision. We should not assume that all rust snippets are playable.

@budziq
Copy link
Contributor Author

budziq commented Aug 7, 2017

We should not assume that all rust snippets are playable.

Well it isn't that bad of a default. rust-book and RBE assume that all snippets are runnable and could live with some snippets being "opt out" instead of all snippets being "opt in" runnable.

I would propose to make the no_run snippets not runnable as the first order of business.

```rust, no_run
fn main(){
 println!("blah!");
}
```

no_run attribute disables running of given snippet in skeptic (it is still tested for compilation). This attribute is generally used for snippets that would otherwise use constrained resources (network, long runtime, disk quota, call processes, connect to db, etc.) which would not be usable in play.rust-lang.org anyway.

@budziq
Copy link
Contributor Author

budziq commented Sep 4, 2017

@azerupi the rust-lang/rust-playground#198 has enabled a lot of crates on rust-playground

What do you think about moving forward with #396 and #404?

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 a pull request may close this issue.

2 participants