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

Hide type declarations by default #49412

Merged
merged 1 commit into from
Mar 30, 2018
Merged

Conversation

GuillaumeGomez
Copy link
Member

I'm not very happy for the moment about the rendering but the bases are here:

screen shot 2018-03-27 at 11 56 27

r? @QuietMisdreavus

@killercup
Copy link
Member

Kinda skeptical if we want to do this when there are public fields without doc comments, but otherwise r=me

@pietroalbini pietroalbini added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 27, 2018
@QuietMisdreavus
Copy link
Member

@killercup Fields without docs still appear in the main listing, regardless of whether they have docs on them or not. Strictly speaking, there's no true loss of information going on - we're just auto-folding the bit that takes up more room.

@killercup
Copy link
Member

@QuietMisdreavus yeah, as I've said offline, I'm fine with this change then :)

@QuietMisdreavus
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Mar 28, 2018

📌 Commit 73b97c7 has been approved by QuietMisdreavus

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 28, 2018
@bors
Copy link
Contributor

bors commented Mar 29, 2018

⌛ Testing commit 73b97c7 with merge 06b29bedd183458a59bf61c436c26d02f6fb576d...

@bors
Copy link
Contributor

bors commented Mar 29, 2018

💔 Test failed - status-appveyor

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 29, 2018
@QuietMisdreavus
Copy link
Member

@bors retry

Appveyor timeout

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 29, 2018
@bors
Copy link
Contributor

bors commented Mar 29, 2018

⌛ Testing commit 73b97c7 with merge 7a8f5cf...

bors added a commit that referenced this pull request Mar 29, 2018
Hide type declarations by default

I'm not very happy for the moment about the rendering but the bases are here:

<img width="610" alt="screen shot 2018-03-27 at 11 56 27" src="https://user-images.githubusercontent.com/3050060/37960492-0e045954-31b6-11e8-9cea-1ef8a3f980c4.png">

r? @QuietMisdreavus
@bors
Copy link
Contributor

bors commented Mar 29, 2018

💔 Test failed - status-appveyor

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 29, 2018
@kennytm
Copy link
Member

kennytm commented Mar 29, 2018

@bors retry p=15

3 hour timeout.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 29, 2018
@bors
Copy link
Contributor

bors commented Mar 30, 2018

⌛ Testing commit 73b97c7 with merge 15e8c5d...

bors added a commit that referenced this pull request Mar 30, 2018
Hide type declarations by default

I'm not very happy for the moment about the rendering but the bases are here:

<img width="610" alt="screen shot 2018-03-27 at 11 56 27" src="https://user-images.githubusercontent.com/3050060/37960492-0e045954-31b6-11e8-9cea-1ef8a3f980c4.png">

r? @QuietMisdreavus
@bors
Copy link
Contributor

bors commented Mar 30, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: QuietMisdreavus
Pushing 15e8c5d to master...

@bors bors merged commit 73b97c7 into rust-lang:master Mar 30, 2018
@GuillaumeGomez GuillaumeGomez deleted the hide-type-decl branch March 30, 2018 07:30
@squishy-clouds
Copy link

Since this change made it to the nightly docs, I have found myself constantly having to open the type declaration. To copy/paste trait function's signatures to implement them, or to see the type/lifetime parameters for the type, for example.

Usually public fields are documented, but when they're not this change makes things less convenient. As far as I can tell associated types have the same issue, which is, imo, much more important than fields because forgetting them when impling (because they're hidden away) gives you a compile error and that might be frustrating because you thought you didn't see any associated type in the docs.

It also isn't documented anywhere else whether an item is a struct, enum, trait, etc. except through the color of the name, so looking to see what kind of item a thing is means I have to open the type declaration, unless I've memorized what the colors mean. I imagine that might make things slightly more annoying for color-blind people as well.

I don't know why this change was made in the first place, I couldn't find any other issues or PRs about this, if its because of traits like Iterator whose definition takes up the whole screen, maybe could this change just be applied to longer definitions?

@GuillaumeGomez
Copy link
Member Author

Because generally, it isn't wanted by the users since you have the fields/variants listed just below with all needed information and documentation. This is "just" the rust code declaration. However, I think I'll start writing a settings menu to allow users to have a better control over a few things.

@newpavlov
Copy link
Contributor

I am also not happy with this change. Often for me type declarations are sufficient enough to remember all details which I want with a single glance, while with documented fields it's significantly less convenient/fast. So I believe this change has introduced an unpleasant speed bump for proficient users.

@GuillaumeGomez
Copy link
Member Author

You can change the corresponding settings to have like it was before this change.

@newpavlov
Copy link
Contributor

IIUC you can't do anything for docs on doc.rust-lang.org. Maybe it will make sense to add a small js+cookie logic, e.g. if you've opened type declaration is stays open and vice versa?

@GuillaumeGomez
Copy link
Member Author

It should be saved though... At least it works as expected for me. If you have a bug with settings, please open an issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants