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

Improved rustdoc rendering for unstable features #38843

Merged
merged 1 commit into from
Jan 10, 2017

Conversation

Manishearth
Copy link
Member

@Manishearth Manishearth commented Jan 5, 2017

This replaces "unstable" with "this is an experimental API", and uses a <details> tag to expand to the reason.

The <details> tag renders as a regular div (with the details show) on browsers which don't support it, On browsers which do support it, it shows only the summary line with an expandy-arrow next to it, and on clicking it the details will turn up below it.

This is somewhat a strawman proposal. The main issue is that we need to improve our messaging around unstable APIs. Since they turn up in the docs, we should be clearer that they are experimental (and perhaps add something about nightly-only). I'm making this PR to kickstart discussion on this.

Example rendering: http://manishearth.github.io/rust-internals-docs/std/io/trait.Read.html#method.chars

screen shot 2017-01-04 at 10 15 37 pm

expands to

screen shot 2017-01-04 at 10 15 43 pm

cc @steveklabnik @jdub

@rust-highfive
Copy link
Collaborator

r? @steveklabnik

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

@Manishearth
Copy link
Member Author

Manishearth commented Jan 5, 2017

"This is a nightly-only experimental API" might be better.

Changing the text (or even the fact that it is displayed) based on the version of the docs being built is also interesting.

We should also be greying-out unstable methods everywhere, not just in the module list.

@nagisa
Copy link
Member

nagisa commented Jan 5, 2017

"This is a nightly-only experimental API" might be better.

“Unstable API”?


I like the use of <summary>+<detail>. We could use that for actual documentation as well instead of using the javascript junk to emulate expandable stuff as an improvement later on as well.

@Manishearth
Copy link
Member Author

Part of the point was to remove the word "unstable" as it can cause confusion.

@jdub
Copy link
Contributor

jdub commented Jan 5, 2017

Nice! I just saw your suggestion to open an RFC… this seems more straightforward. 😄

It's getting a bit wordy, but perhaps: "⚠️ This is an experimental API, available only on Rust nightly builds". It may seem superfluous to say "Rust", but it helps clarify the "nightly" jargon.

@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Jan 5, 2017

"unstable API" seems fine for me, but otherwise I don't have much preference in here.

Big 👍 for me. This is a huge improvement and I'm really looking forward to it.

@steveklabnik
Copy link
Member

/cc @rust-lang/core , and specifically, @aturon and @wycats

@aturon
Copy link
Member

aturon commented Jan 5, 2017

@Manishearth This is a great change. I agree that moving away from the stable/unstable terminology will reduce confusion. Adding "nightly-only" is probably a good idea, for maximum clarity.

cc @brson

@nikomatsakis
Copy link
Contributor

I too prefer experimental to unstable. It makes us sound like scientists. I would prefer to add some mention of nightly as well.

/me wants a lab-coat

@Manishearth
Copy link
Member Author

I'm disappointed that there is no Erlenmeyer flask emoji. Microscope (🔬) exists, and it is supposed to combine with person emojis to create scientist emojis, but that doesn't work on many fonts right now.

So something like:

  • "⚠️ This is an experimental API, available only on Rust nightly builds"
  • "🔬 This is a nightly-only experimental API"

is probably what we're settling on.

@hanna-kruppe
Copy link
Contributor

FWIW, the ⚠️ emoji is already used to mark unsafe functions.

@Manishearth
Copy link
Member Author

I don't see that being used in the docs anywhere?

I noticed that not all APIs have extended unstable reasons, so I should probably fix up this PR to handle that (e.g. http://manishearth.github.io/rust-internals-docs/syntax/parse/fn.raw_str_lit.html)

@steveklabnik
Copy link
Member

steveklabnik commented Jan 5, 2017

#37250

But it's not actually appearing.... hmmmm

@jdub
Copy link
Contributor

jdub commented Jan 6, 2017

@Manishearth Microscope is way cooler. 😄

@liigo
Copy link
Contributor

liigo commented Jan 6, 2017

🍼 Baby Bottle (U+1F37C)

🍼 This is an experimental API, available only on Rust nightly builds.
or just the icon 🍼 with a popup tooltip.

@Manishearth
Copy link
Member Author

screen shot 2017-01-06 at 10 16 32 am

@nikomatsakis
Copy link
Contributor

@Manishearth I can't really make out that it is a microscope. :( I sort of like baby bottle. =)

@Manishearth
Copy link
Member Author

Not really better with the baby bottle

screen shot 2017-01-06 at 5 18 42 pm

Note that emoji will render differently on different platforms. What we should do here is just make the font size a bit larger.

@Manishearth
Copy link
Member Author

How does a font size of 1.5em sound?

screen shot 2017-01-06 at 5 20 10 pm

@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Jan 7, 2017

@nikomatsakis: And I prefer the microscope. Should we vote? :p

@Manishearth: I prefer with 1.5em, indeed (but it seems a little bit too big).

@Manishearth
Copy link
Member Author

Manishearth commented Jan 8, 2017

(I'll need to update the tests too, so don't directly r+ this, just mention "r=me" or whatever. I won't update them now since we have to yet settle on the exact way to display this)

@liigo
Copy link
Contributor

liigo commented Jan 9, 2017

Note that emoji will render differently on different platforms.

There is EmojiOne. image image

@nikomatsakis
Copy link
Contributor

Microscope is fine with me. The bigger size looks mildly better.

@Manishearth
Copy link
Member Author

Are we still in the bikeshed phase for this PR or should I start cleaning up the tests so that we can land this?

@GuillaumeGomez
Copy link
Member

From my point of view, we're in the details now. Text shouldn't change and it seems microscope is the preferred icon. Maybe wait confirmation from others and then clean up?

@aturon
Copy link
Member

aturon commented Jan 9, 2017

@Manishearth let's ship! Agreed on larger-sized microscope.

@Manishearth
Copy link
Member Author

Updated tests.

@GuillaumeGomez
Copy link
Member

Then it's all good. Thanks!

@bors: r+

@bors
Copy link
Contributor

bors commented Jan 10, 2017

📌 Commit 0a1c9ae has been approved by GuillaumeGomez

@bors
Copy link
Contributor

bors commented Jan 10, 2017

⌛ Testing commit 0a1c9ae with merge 8a4f77e...

@bors
Copy link
Contributor

bors commented Jan 10, 2017

💔 Test failed - status-appveyor

@alexcrichton
Copy link
Member

alexcrichton commented Jan 10, 2017 via email

@bors
Copy link
Contributor

bors commented Jan 10, 2017

⌛ Testing commit 0a1c9ae with merge d6b927d...

bors added a commit that referenced this pull request Jan 10, 2017
Improved rustdoc rendering for unstable features

This replaces "unstable" with "this is an experimental API", and uses a `<details>` tag to expand to the reason.

The `<details>` tag renders as a regular div (with the details show) on browsers which don't support it, On browsers which do support it, it shows only the summary line with an expandy-arrow next to it, and on clicking it the details will turn up below it.

This is somewhat a strawman proposal. The main issue is that we need to improve our messaging around unstable APIs. Since they turn up in the docs, we should be clearer that they are experimental (and perhaps add something about nightly-only). I'm making this PR to kickstart discussion on this.

Example rendering: http://manishearth.github.io/rust-internals-docs/std/io/trait.Read.html#method.chars

<img width="375" alt="screen shot 2017-01-04 at 10 15 37 pm" src="https://cloud.githubusercontent.com/assets/1617736/21670712/5a96c7de-d2cb-11e6-86a6-87f70818d634.png">

expands to

<img width="799" alt="screen shot 2017-01-04 at 10 15 43 pm" src="https://cloud.githubusercontent.com/assets/1617736/21670714/5db88bb4-d2cb-11e6-8fcc-5cf11d198b75.png">

cc @steveklabnik @jdub
@bors
Copy link
Contributor

bors commented Jan 10, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: GuillaumeGomez
Pushing d6b927d to master...

@bors bors merged commit 0a1c9ae into rust-lang:master Jan 10, 2017
@Manishearth Manishearth deleted the proposed branch January 14, 2017 03:50
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.