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

Hiding of struct declarations means constructability isn't really documented #51860

Closed
clarfonthey opened this issue Jun 27, 2018 · 19 comments
Closed
Labels
T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@clarfonthey
Copy link
Contributor

clarfonthey commented Jun 27, 2018

This was found in review of #51854, which affects the documentation for #[non_exhaustive] and is a prerequisite for stabilisation.

Previously, the /* fields omitted */ comment would be a statement to the reader that the struct has private fields, and is thus not constructable. However, now that these declarations are hidden by default, this isn't really documented in something in the open.

In order to properly document the non-exhaustiveness of a struct, it makes sense that the constructability of a struct also be documented, as this is the primary reason for marking a struct as non-exhaustive in the first place.

My thoughts: structs which are constructable are rarer than those that aren't, so I think that a message that states so on these types would be best. As I mentioned in the linked PR, it would be something along the lines of "all of this struct's fields are public, and it can be constructed normally with the Struct { .. } syntax."

@clarfonthey
Copy link
Contributor Author

cc @QuietMisdreavus @GuillaumeGomez

I believe you two were involved in the initial hiding of these declarations, and might have more context to add to this discussion.

@GuillaumeGomez
Copy link
Member

You can change this in settings. We decided to have declarations hidden by default because they were mostly noise (especially in std lib).

@clarfonthey
Copy link
Contributor Author

Makes sense! I agree that it's probably better to hide them, although I suppose in this case this was one of the few bits of information in them not replicated elsewhere.

@GuillaumeGomez
Copy link
Member

Not sure to understand your message. But like I said, you can totally change this behavior by clicking on the cog on the right of the search bar. There are other few options in there so I'll let you take a look. Having more feedbacks is always appreciated. :)

@est31
Copy link
Member

est31 commented Jun 28, 2018

Just noting, I'm resetting all browser state each time I close my browser, as a privacy measure. Therefore, using the settings menu is not tenable for me, unfortunately.

@GuillaumeGomez
Copy link
Member

@est31: Therefore, we have two solutions:

  • using cookies (and I'm not even sure it'll work in your case if you reset your browser's state...)
  • having a login system (through oauth or other)

However, the big downside for both systems lies with the brand new laws on confidentiality (RGPD in europe for example) which would force us to add more mechanisms. Clearly, for docs, I'm not sure it's worth it.

A third option could be possible: writing a browser plugin. If enough people want to have one, I'll just add it.

@est31
Copy link
Member

est31 commented Jun 28, 2018

Yeah cookies wouldn't work, I reset them as well. As for log-ins, I try to use documentation predominantly via the file:/// protocol, either via cargo doc, or via rustup doc. Logins don't work there.

I'd prefer having the struct definition shown by default =). Just to give you feedback, as you've said you appreciate it.

@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Jun 28, 2018

It's part of the numerous "two-sides" features unfortunately. :-/

But thanks for the feedback!

@QuietMisdreavus
Copy link
Member

Would it be possible to add a note to the Fields section in the main page? Say, after all the public fields are listed (or in the place of the fields section if there are no public fields), add something like "...some fields omitted" like we have in the struct definition. Something similar could be added for non-exhaustive enums.

@kennytm
Copy link
Member

kennytm commented Jul 5, 2018

We decided to have declarations hidden by default because they were mostly noise (especially in std lib).

I disagree that it is mostly noise, the current implementation also hides away the signature which is valuable signal.

Example: https://doc.rust-lang.org/std/str/struct.RMatches.html — you can't see it is RMatches<'a, P> until expanding the declaration.

I do agree that the interior of an item e.g. https://doc.rust-lang.org/std/iter/trait.Iterator.html is mostly noise since they are repeated below, so perhaps it is better to leave the exterior intact, and collapse the interior by default:

screenshot_2018-07-06 03 52 17_uqd9du-fs8

The /* fields omitted */ comment can then be placed outside of the collapsible to convey this piece of information.

(Also, if the collapsed content is less than 2 lines then it's pointless to actually introduce a collapsible)

@kennytm kennytm added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Jul 5, 2018
@clarfonthey
Copy link
Contributor Author

One potential problem with this, @kennytm, is tuple structs. Although I've already mentioned in #38101 that we should really be handling tuple structs differently.

@fintelia
Copy link
Contributor

fintelia commented Jul 9, 2018

I personally would like to see all the relevant information extracted from the declaration so that the expand declaration feature can be completely removed. After all, there would still be the link to the source. As best I can tell, there are only a couple pieces of information left in the declaration that don't appear elsewhere:

  1. Can the struct/enum be constructed?
  2. Can it be matched on? exhaustively?
  3. What are the type/lifetime parameters? And what are the bounds on them?

Provided that rustdoc can figure out the answers to these questions, none of them seem especially hard to expose. (3) would be just a matter of adding a new section. The other two points are all yes/no questions, so they could probably be addressed with a couple of strategically placed annotations elsewhere (for instance section heading that said Fields (private fields omitted), Variants (non-exhaustive), etc.)

@dhardy
Copy link
Contributor

dhardy commented May 21, 2019

In addition to the above:

  • what bounds are required by the trait (super-traits)

I very often find I need information from the declaration (arguably it is the most important part of documentation for traits), so hiding it by default appears a strange choice to me (at least the doc settings appear to be working for now).

@GuillaumeGomez
Copy link
Member

This is fixed by allowing finer settings in #60778

@clarfonthey
Copy link
Contributor Author

I don't know if that really fixes this-- I think the constructability should be documented in addition to the definition.

@GuillaumeGomez
Copy link
Member

@clarfon Not sure to understand your point...

@dhardy
Copy link
Contributor

dhardy commented May 24, 2019

@GuillaumeGomez depends on how you view the generated documentation — it's essentially a prettier, filtered version of Rust code, excepting the declarations which are not prettified (merely filtered). The declarations are available in the docs because they contain information which is not otherwise available — this information could be presented in a prettified manner, finally making the declaration section unnecessary. For example:

  • Start all item documentation with a reduced version of the declaration covering generic parameters, e.g. enum Result<T, E>, also mentioning any bounds on parameters. For traits, this should also list super-traits, e.g. trait Error: Debug + Display.
  • For unit structs, document that this is constructible (e.g. pub struct Foo; is constructible via Foo)
  • For tuple structs, for each public field, document the index and type; also, if any fields are not public, document that this is not exhaustive (similar to non-exhaustive enums)
  • For other structs, do the same, but including field name and any available documentation
  • Non-exhaustive enums are already marked as such in the "Variants" section 👍
  • For attributes, include a new section and document each attribute where appropriate (e.g. Result has must_use).

Note that generic parameters are already mentioned via an impl<T, E> header in the "Methods" section, but not for example in the "Variants" section of std::result::Result (or the suggested sections for struct fields above).

Personally I'm not sure whether breaking all this information out into new sections is better than simply always showing the existing declaration; possibly there should be a setting to choose between the two. But what is clear is that the current system is often hiding important information unless one clicks a "+" or tweaks the settings.

@jsha
Copy link
Contributor

jsha commented Jun 26, 2021

I think since #83337 this is mostly resolved: structs contents are commonly shown now.

@camelid
Copy link
Member

camelid commented Dec 15, 2021

I'm going to close this for now since I don't think it's really actionable. As @jsha said, this should be mostly resolved.

@camelid camelid closed this as completed Dec 15, 2021
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

9 participants