Format code in doc comments

[robertbastian]

I ran cargo +nightly fmt -- --config=format_code_in_doc_comments=true. We can't make this a default flag because it's only available on nightly, but maybe we should add it to the prerelease tasks?

components/datetime/src/options/length.rs and components/plurals/src/lib.rs contain manual touchups

[zbraniecki]

This is awesome, thanks so much for finding and filing it! I was hoping this feature will one day be available :)

A couple notes:

• Overall, all the changes look like improvements except of same-line-expect/unwrap. I think unwrap/expect have lower impact on statement readability when they are in separate line from the command. I wish there was an option to "put unwrap/expect in separate line" option, but I assume there is none.
• I don't see manual changes in plurals/src/lib.rs - can you describe them?
• Can you add this step to release checklist?
• For the manual changes in datetime options lenghts - I think * ``en`` - "08:25am" (single ``) would be more readable, but up to you.

[sffc]

Praise: I'm glad that we can do this now

Thoughts:

1. I'd like to consider setting a shorter line length for the docs tests, like 40 columns, because (1) docs tests typically have less indentation, so long lines are not necessary, and (2) shorter lines work better on docs.rs across media. It might also help address Zibi's suggestions about .expect() being on a new line more often.
2. We may consider running this in CI. I would be okay with our CI fmt job running on nightly. On the other hand, it could be a big annoyance, so I'm okay with this waiting until release time.

[robertbastian]

I'd like to consider setting a shorter line length for the docs tests, like 40 columns, because (1) docs tests typically have less indentation, so long lines are not necessary, and (2) shorter lines work better on docs.rs across media. It might also help address Zibi's suggestions about .expect() being on a new line more often.

There doesn't seem to be an option for this.

We may consider running this in CI. I would be okay with our CI fmt job running on nightly. On the other hand, it could be a big annoyance, so I'm okay with this waiting until release time.

We could set it on CI, but you'd always have to remember to do cargo +nightly fmt locally because I don't see a way to set this in a config.

[robertbastian]

• Overall, all the changes look like improvements except of same-line-expect/unwrap. I think unwrap/expect have lower impact on statement readability when they are in separate line from the command. I wish there was an option to "put unwrap/expect in separate line" option, but I assume there is none.

Not that I know, but even if we set this it would apply to non-doc code as well

• I don't see manual changes in plurals/src/lib.rs - can you describe them?

Only _ => { unreachable!(); } -> _ => unreachable!(), because formatting made it three lines

• Can you add this step to release checklist?

Done

• For the manual changes in datetime options lenghts - I think ...

I think markdown is messing up what you're trying to say? I'm not code-quoting the output because they are nice strings, do you think I should?

[zbraniecki]

I'm not code-quoting the output because they are nice strings, do you think I should?

Yeah. Generally I think "foo" conveys to reader that the output is a string. foo is ambiguous. The backtick conveys that it is a special type, which en-US is. So I suggested: * LOCALE - STRING where locale is ab-CD with backticks, and string is "HH:mm" style string in quotes.
Optional, up to you whichever you prefer better :)

[sffc]

I think the main issue here is that both @zbraniecki and I would like to see the docs with various formatting settings that are different from the ones used for regular code.

@robertbastian says that there is currently no way to configure separate formatting within rustfmt for docs versus regular code.

Can we make a duckscript task that runs rustfmt with the settings we want for doc strings, and then discards changes to regular code? Maybe something that only keeps changes on lines starting with /// or //! ?

[Manishearth]

We can't make this a default flag because it's only available on nightly, but maybe we should add it to the prerelease tasks?

We can also run this as a separate optional CI job and fix it occasionally. Our CI still uses nightly in places.

[zbraniecki]

I think the main issue here is that both @zbraniecki and I would like to see the docs with various formatting settings that are different from the ones used for regular code.

My preference is very weak and I'd side with consistency over my personal preference.
My only difference from what clippy is doing is with unwrap/expect being in a separate line even when the original line is short enough. I prefer to forgo it for consistency and/or file clippy feature request for such option.

I also don't feel it's worth spending a lot of time to add it to CI now. If it's easy, great, if not, make it manual before release.

[robertbastian]

This PR will be annoying to keep conflict-free, @sffc I don't know how strong your objections are, so please either merge as is or close and we can revisit this later.

[sffc]

Discussion:

• @zbraniecki - I strongly support this PR in order to skip bikeshedding. We should file a feature request for wrapping the expect functions.
• @robertbastian - What is the width limit on docs.rs?
• @sffc - I like to keep docs in a narrow window, which sometimes also happens in IDEs, which results in horizontal scrollbars on docs examples
• @dminor - I agree with @zbraniecki
• @Manishearth - Can we keep the line length as a separate implicit setting?
• @robertbastian - The horizontal scrollbar could be fixed in rustdoc. Maybe we should file an issue with rustdoc to address that.
• @sffc - Overall I see pros and cons of this PR. I'm not super happy with formatting docs to 100 columns, but I like to have them being formatted in general.
• @nordzilla - I prefer having a standard format.
• @Manishearth - We should file a bug against rustfmt on the experimental feature.

Conclusion: Merge the PR.

[Manishearth]

Filed rust-lang/rustfmt#5345

[Manishearth]

@robertbastian @sffc so rustfmt now has a separate config feature for this on nightly, rust-lang/rustfmt#5345 (comment)

are y'all comfortable with us trying it out?

[sffc]

Yeah!

[Manishearth]

#2134