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 css: Put where in trait listings on a new line #36676

Merged
merged 1 commit into from
Sep 27, 2016

Conversation

bluss
Copy link
Member

@bluss bluss commented Sep 23, 2016

This is about the gray area at the top of a trait's documentation page,
that lists all methods and their signatures. A big trait page like
Iterator is very crowded without this tweak.

This is about the gray area at the top of a trait's documentation page,
that lists all methods and their signatures. A big trait page like
Iterator is very crowded without this tweak.
@rust-highfive
Copy link
Collaborator

r? @steveklabnik

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

@steveklabnik
Copy link
Member

Can you show screenshots please?

@bluss
Copy link
Member Author

bluss commented Sep 23, 2016

before after images for a trait that doesn't really need this, i.e small and simple trait like Read: http://imgur.com/a/4K5Z3

Itertools docs use this css tweak, and it would be unreadable without it: https://bluss.github.io/rust-itertools/doc/itertools/trait.Itertools.html

@steveklabnik
Copy link
Member

I like it a lot. @rust-lang/docs @rust-lang/tools ?

@bluss
Copy link
Member Author

bluss commented Sep 23, 2016

Bigger picture, do we even need the gray pre.trait box at all?

@hanna-kruppe
Copy link
Contributor

hanna-kruppe commented Sep 23, 2016

In the long term a better solution might be #36654 applied to all code-like snippets in rustdoc. (Sadly, the { ... } part of method docs currently trips up rustfmt.)

@fitzgen
Copy link
Member

fitzgen commented Sep 23, 2016

(Sadly, the { ... } part of method docs currently trips up rustfmt.)

What if instead of ... it had // ...?

@hanna-kruppe
Copy link
Contributor

That would be a problem with the closing bracket on the same line. /* ... */ would work but that style of comments is not just very rare, but apparently not idiomatic — at least, it's deliberately snubbed by the book.

@bluss
Copy link
Member Author

bluss commented Sep 23, 2016

rustdoc uses that comment style too now

pub struct Combinations<I: Iterator> { /* fields omitted */ }

@GuillaumeGomez
Copy link
Member

Oh damn! This change is absolutely awesome! I really like it!

Big 👍 for me!

@peschkaj
Copy link

👍 This is great.

@nrc
Copy link
Member

nrc commented Sep 25, 2016

  • 1 for this too

I do wonder whether we need the { ... } at all - it looks particularly weird on the where lines. I suppose the argument is that it wouldn't be valid Rust otherwise, but it is not in any case because of the ....

@steveklabnik
Copy link
Member

Given that virtually everyone is 👍 , let's :shipit:

I agree that messing with the {}s might be nice too, but let's not block this improvement on it 😄

@bors: r+ rollup

@bors
Copy link
Contributor

bors commented Sep 26, 2016

📌 Commit e82d13e has been approved by steveklabnik

sophiajt pushed a commit to sophiajt/rust that referenced this pull request Sep 26, 2016
…bnik

rustdoc css: Put `where` in trait listings on a new line

This is about the gray area at the top of a trait's documentation page,
that lists all methods and their signatures. A big trait page like
Iterator is very crowded without this tweak.
sophiajt pushed a commit to sophiajt/rust that referenced this pull request Sep 27, 2016
…bnik

rustdoc css: Put `where` in trait listings on a new line

This is about the gray area at the top of a trait's documentation page,
that lists all methods and their signatures. A big trait page like
Iterator is very crowded without this tweak.
bors added a commit that referenced this pull request Sep 27, 2016
@bors bors merged commit e82d13e into rust-lang:master Sep 27, 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.

9 participants