Skip to content

Misleading docs for an enum with struct variants #37500

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
mathstuf opened this issue Oct 31, 2016 · 6 comments
Closed

Misleading docs for an enum with struct variants #37500

mathstuf opened this issue Oct 31, 2016 · 6 comments
Labels
T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@mathstuf
Copy link
Contributor

Given this structure:

pub enum OldTopicRemoval {
    Obsoleted {
        old_merge: StagedTopic,
        replacement: Option<StagedTopic>,
    },
    Removed(StagedTopic),
}

I'm seeing docs rendered as:

docs-enum

Which makes it look like Removed is more closely related to the fields of Obsoleted rather than on the same level. It'd be nice if the fields for the Obsoleted variant fields were either indented somehow or collapsed with the description.

@bluss bluss added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Oct 31, 2016
@GuillaumeGomez
Copy link
Member

A tricky one. The problem just gets bigger if it has more "sub-levels". The output shouldn't be based on indentation therefore. I'm more in favor for bigger margin between the struct fields and the other variants. Also, it would be nice to replace "Fields" by something like "Fields of Obsoleted variant".

@QuietMisdreavus
Copy link
Member

I just took a moment to try out a few variations that might work for this. They all assume that the "Fields" header will include the name of the variant being described, but beyond that take a couple different directions of how it gets laid out.

Option 1: Extra margin under the list of struct fields

rustdoc struct enum variant 1

This is the simplest option, as all it does is slap some extra padding underneath the listing of struct fields via CSS. It provides a visible separation from the list of struct fields and the next variant in the listing.

Option 2: Print struct variant fields after the list of variants

rustdoc struct enum variant 2

This one's my personal favorite, as it provides a clean separation between the variant listing and the struct-field listing, without depending on more subtle style changes. On the other hand, it has the potential to put too much distance between a variant and its fields if an enum is huge enough. That could be mitigated by adding links and anchors between these sections.

Option 3: Either of the above, but with borders around the field listing

rustdoc struct enum variant 3

The list of struct fields is printed in a <table>, so the sky's the limit as far as drawing borders is concerned. It becomes a pure-CSS solution at that point, too. I admit this preview image is a little janky, but it's definitely possible to make this option look nicer with a little more thought put into the CSS.

Thoughts? Opinions?

@GuillaumeGomez
Copy link
Member

My favorite is the first, but the result isn't good enough yet. It might be interesting to be able to hide by default the sub-type content and add a "show variant field" button.

@mathstuf
Copy link
Contributor Author

This is probably too complicated, but Option 2 could have the following tweaks as well: for every subfield named the same, group them together (they're usually related anyways) and then list the variants it is a part of.

enum Animal {
    Cat { name: String, lives: u8, },
    Dog { name: String, breed: String, },
}

could render the name subfields together mentioning that it is part of both Cat and Dog while lives, and breed say they are only part of Cat and Dog, respectively. Rustdoc would probably need some directive/syntax for saying that a subfield's docstring should be shared though.

@QuietMisdreavus
Copy link
Member

@GuillaumeGomez I've changed my mind, this looks a lot nicer.

Option 4: Wrap struct variant field listings in an auto-hidden block

rustdoc struct enum variant 4 part 1

When you initially load the page, there's a line under variants with struct fields letting you know you can click to expand the listing.

rustdoc struct enum variant 4 part 2

If you click to expand, the header and table unfold into a nicely-indented listing. The indentation and table borders fell into place with its presence in this folding block, and I really like how it looks.

If you want to take a look in your own browser and screen size, I've got this version hosted on my server.

@GuillaumeGomez
Copy link
Member

screen shot 2016-11-11 at 23 25 00

bors added a commit that referenced this issue Nov 13, 2016
…meGomez

rustdoc: fold fields for enum struct variants into a docblock

Per discussion in #37500, this PR updates the enum rendering code to wrap variants with named struct fields in a `docblock` span that is hidden automatically upon load of the page. This gives struct variant fields a clean separation from other enum variants, giving a boost to the readability of such documentation. Preview output is available [on the issue page](#37500 (comment)), but for the sake of completeness I'll include the images here again.

![rustdoc struct enum variant 4 part 1](https://cloud.githubusercontent.com/assets/5217170/20231925/96160b7e-a82a-11e6-945b-bbba95c5e4bc.PNG)

When you initially load the page, there's a line under variants with struct fields letting you know you can click to expand the listing.

![rustdoc struct enum variant 4 part 2](https://cloud.githubusercontent.com/assets/3050060/20232067/1dc63266-a866-11e6-9555-8fb1c8afdcec.png)

If you click to expand, the header and table unfold into a nicely-indented listing.

If you want to take a look in your own browser and screen size, [I've got this version hosted on my server](https://shiva.icesoldier.me/doctest/doctest/enum.OldTopicRemoval.html).

Fixes #37500

r? @GuillaumeGomez
critiqjo pushed a commit to critiqjo/rustdoc that referenced this issue Dec 16, 2016
…meGomez

rustdoc: fold fields for enum struct variants into a docblock

Per discussion in #37500, this PR updates the enum rendering code to wrap variants with named struct fields in a `docblock` span that is hidden automatically upon load of the page. This gives struct variant fields a clean separation from other enum variants, giving a boost to the readability of such documentation. Preview output is available [on the issue page](rust-lang/rust#37500 (comment)), but for the sake of completeness I'll include the images here again.

![rustdoc struct enum variant 4 part 1](https://cloud.githubusercontent.com/assets/5217170/20231925/96160b7e-a82a-11e6-945b-bbba95c5e4bc.PNG)

When you initially load the page, there's a line under variants with struct fields letting you know you can click to expand the listing.

![rustdoc struct enum variant 4 part 2](https://cloud.githubusercontent.com/assets/3050060/20232067/1dc63266-a866-11e6-9555-8fb1c8afdcec.png)

If you click to expand, the header and table unfold into a nicely-indented listing.

If you want to take a look in your own browser and screen size, [I've got this version hosted on my server](https://shiva.icesoldier.me/doctest/doctest/enum.OldTopicRemoval.html).

Fixes #37500

r? @GuillaumeGomez
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants