rustdoc: Tuple Fields of Variant section is superfluous and noisy if all fields are undocumented

[orlp]

I understand that if (at least one of) a tuple variant's fields is documented, we need a place to put that documentation. But if not this just adds superfluous noise. Especially if you have a lot of tuple enum variants with undocumented fields it becomes very noisy very quickly. As a real world example from one of my codebases:

pub enum RotationPolicy {
    /// Only rotate when [`RotatingFileWriter::rotate`] is called.
    Manual,
    /// Rotate when the active file exceeds this many bytes.
    SizeLimit(usize),
    /// Rotate periodically.
    Periodically(Period),
    /// Rotate both periodically and whenever the size limit is exceeded.
    SizeLimitAndPeriodically(usize, Period),
    /// A custom callback to determine when to rotate. Called after every write
    /// with the current active file size and age.
    Custom(Box<dyn FnMut(usize, Duration) -> bool>),
}

How it currently looks:

And with the superfluous sections removed:

I think we should get rid of any "Tuple Fields of Variant" section if that variants' fields are undocumented. It only adds noise and makes the documentation less readable in my opinion.

[camelid]

I wonder if instead we should just unconditionally get rid of the tuple fields in the header? Then it'd be consistent with struct fields. On the other hand, the header view is a bit easier to read in some ways since it matches source view.

cc @rust-lang/rustdoc: What do you think?

[jsha]

To clarify, you mean the heading of each variant, right? So e.g. SizeLimit(usize) would become SizeLimit, with a heading below that saying "Tuple Fields", and text below that saying 0: usize?

I prefer an approach where we keep SizeLimit(usize) as the heading, and say "if none of the tuple fields has a doccomment (the common case), we don't show the "Tuple Fields" section (as suggested in the top comment). Perhaps we should even deprecate comments on individual tuple fields? It seems quite uncommon to use, and rather awkward.

[GuillaumeGomez]

I agree with @jsha proposition.

[Nemo157]

Yep, in general I think if the fields need documentation people will generally be using struct-variants instead, so optimizing for doc-less tuple-variants makes sense.

[camelid]

Perhaps we should even deprecate comments on individual tuple fields? It seems quite uncommon to use, and rather awkward.

I disagree; we'd basically be removing expected behavior. I don't think we should force people to refactor their code just to add documentation. But optimizing for doc-less tuple variants (rather than enforcing it) seems potentially reasonable.

[GuillaumeGomez]

Thanks to @camelid, I realized I misunderstood @jsha's comment. So let me update my comment:

To clarify, you mean the heading of each variant, right? So e.g. SizeLimit(usize) would become SizeLimit, with a heading below that saying "Tuple Fields", and text below that saying 0: usize?

I agree with this.

I prefer an approach where we keep SizeLimit(usize) as the heading, and say "if none of the tuplie fields has a doccomment (the common case), we don't show the "Tuple Fields" section (as suggested in the top comment).

I agree with this as well.

Perhaps we should even deprecate comments on individual tuple fields? It seems quite uncommon to use, and rather awkward.

I don't agree with this.

Hopefully I didn't miss something else...

[camelid]

To clarify, you mean the heading of each variant, right? So e.g. SizeLimit(usize) would become SizeLimit, with a heading below that saying "Tuple Fields", and text below that saying 0: usize?

I agree with this.

I prefer an approach where we keep SizeLimit(usize) as the heading, and say "if none of the tuplie fields has a doccomment (the common case), we don't show the "Tuple Fields" section (as suggested in the top comment).

I agree with this as well.

I don't understand; you agree with both? The two approaches are mutually exclusive IIUC.

[GuillaumeGomez]

I don't have a preference is what I tried to say haha.

[jsha]

Since there's a PR up, I wanted to tidy up this issue thread. There was some confusion and back and forth above, but reading through it it seems like everyone who has chimed in either agrees with the original post's approach or doesn't have a preference. Since the PR (#91687) implements that approach, we're good.

[camelid]

I still think it's not great that now tuple fields and struct { ... } fields will be handled differently, but this seems like a good enough solution for now.