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

Make documentation of which items the prelude exports more readable. #80720

Merged
merged 2 commits into from
Mar 31, 2021

Conversation

steffahn
Copy link
Member

@steffahn steffahn commented Jan 5, 2021

I recently figured out that rustdoc allows link inside of inline code blocks as long as they’re delimited with <code> </code> instead of ` `. I think this applies nicely in the listing of prelude exports in the docs. There, currently unformatted :: and { , } is used in order to mimick import syntax while attatching links to individual identifiers.

Rendered Comparison

Currently (light)

Screenshot_20210105_155801

After this PR (light)

Screenshot_20210105_155811

Currently (dark)

Screenshot_20210105_155824

After this PR (dark)

Screenshot_20210105_155836

Currently (ayu)

Screenshot_20210105_155917

After this PR (ayu)

Screenshot_20210105_155923

Edit: I just noticed, the “current” screenshots are from stable, so there are a few more differences in the pictures than the ones from just this PR.

@rust-highfive
Copy link
Collaborator

r? @joshtriplett

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 5, 2021
@steffahn steffahn changed the title Make documentation of which items the prelude exports more readably. Make documentation of which items the prelude exports more readable. Jan 5, 2021
@steffahn
Copy link
Member Author

steffahn commented Jan 5, 2021

@rustbot modify labels: T-doc

@rustbot

This comment has been minimized.

@rustbot rustbot added the A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools label Jan 5, 2021
@steffahn
Copy link
Member Author

steffahn commented Jan 5, 2021

Another option would be to also split up the std::... prefixes, e.g. turning std::marker into std::marker, or perhaps just std::marker.

@bors
Copy link
Contributor

bors commented Jan 21, 2021

☔ The latest upstream changes (presumably #81240) made this pull request unmergeable. Please resolve the merge conflicts.

@steffahn
Copy link
Member Author

steffahn commented Jan 21, 2021

Rebased to resolve merge conflicts. I’m undoing parts of #80172 here.

The headline

IMO the new version looks worse. Both how the line “The Rust Prelude” is more indented than every other headline here (and also how there are horizontal lines above and below it and it says “prelude” twice)
Screenshot_20210121_194233
and how it’s consequently missing the period here
Screenshot_20210121_194307

Last, but not least, AFAICT it is inconsistent with every other module documentation in the standard library. Except for the top level documentation.

The colons

This is possibly just personal preference, but I do prefer commas:

Screenshot_20210121_194338
vs
Screenshot_20210121_194459

Also, the changed optics from making the preceding section a single code-block does mean that this point has to be re-debated. I find the cases where there are no braces particularly bad, e.g.

  • std::mem::drop: a convenience function for explicitly dropping a value.

vs

  • std::mem::drop, a convenience function for explicitly dropping a value.

where the different colon fonts (inside and outside the code block) are having different heights, although arguably it was actually even worse when they still matched, before this commit:

  • std::mem::drop: a convenience function for explicitly dropping a value.

Screenshot_20210121_200015
where the colon visually looks (a bit) like it becomes part of the path

finally, after #80172, the colon was missing after None and Default, anyways.

cc @camelid @steveklabnik

Note that...

...both of these things are subjective and open to debate. While I find no-headline and commas both more clean, I’m having having no hard feeling about these points, and if people ask for it, I can change it back.

@steffahn
Copy link
Member Author

@rustbot modify labels: T-libs

@rustbot rustbot added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Jan 26, 2021
@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 14, 2021
@crlf0710 crlf0710 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 5, 2021
@jyn514
Copy link
Member

jyn514 commented Mar 5, 2021

r? @steveklabnik

@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 22, 2021
@steffahn
Copy link
Member Author

Any progress here with the review?

@jyn514
Copy link
Member

jyn514 commented Mar 26, 2021

This change looks good to me, but because it undoes parts of #80172 I'd like to hear from either @camelid or @steveklabnik before merging the change to the header.

library/std/src/prelude/mod.rs Show resolved Hide resolved
library/std/src/prelude/mod.rs Outdated Show resolved Hide resolved
@camelid
Copy link
Member

camelid commented Mar 30, 2021

This change looks good to me, but because it undoes parts of #80172 I'd like to hear from either camelid or steveklabnik before merging the change to the header.

The heading change has been reverted now.

@jyn514
Copy link
Member

jyn514 commented Mar 30, 2021

@bors r=camelid,jyn514

@bors
Copy link
Contributor

bors commented Mar 30, 2021

📌 Commit 761296b has been approved by camelid,jyn514

@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 30, 2021
@jyn514
Copy link
Member

jyn514 commented Mar 30, 2021

@bors rollup

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 30, 2021
…r=camelid,jyn514

Make documentation of which items the prelude exports more readable.

I recently figured out that rustdoc allows link inside of inline code blocks as long as they’re delimited with `<code> </code>` instead of `` ` ` ``. I think this applies nicely in the listing of prelude exports [in the docs](https://doc.rust-lang.org/std/prelude/index.html). There, currently unformatted `::` and `{ , }` is used in order to mimick import syntax while attatching links to individual identifiers.

## Rendered Comparison
### Currently (light)
![Screenshot_20210105_155801](https://user-images.githubusercontent.com/3986214/103661510-1a87be80-4f6f-11eb-8360-1dfb23f732e8.png)

### After this PR (light)
![Screenshot_20210105_155811](https://user-images.githubusercontent.com/3986214/103661533-1f4c7280-4f6f-11eb-89d4-874793937824.png)

### Currently (dark)
![Screenshot_20210105_155824](https://user-images.githubusercontent.com/3986214/103661571-2a9f9e00-4f6f-11eb-95f9-e291b5570b41.png)

### After this PR (dark)
![Screenshot_20210105_155836](https://user-images.githubusercontent.com/3986214/103661592-2ffce880-4f6f-11eb-977a-82afcb07d331.png)

### Currently (ayu)
![Screenshot_20210105_155917](https://user-images.githubusercontent.com/3986214/103661619-39865080-4f6f-11eb-9ca1-9045a107cddd.png)

### After this PR (ayu)
![Screenshot_20210105_155923](https://user-images.githubusercontent.com/3986214/103661652-3db26e00-4f6f-11eb-82b7-378e38f0c41f.png)

_Edit:_ I just noticed, the “current” screenshots are from stable, so there are a few more differences in the pictures than the ones from just this PR.
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 30, 2021
Rollup of 6 pull requests

Successful merges:

 - rust-lang#80720 (Make documentation of which items the prelude exports more readable.)
 - rust-lang#83654 (Do not emit a suggestion that causes the E0632 error)
 - rust-lang#83671 (Add a regression test for issue-75801)
 - rust-lang#83678 (Fix Self keyword doc URL conflict on case insensitive file systems (until definitely fixed on rustdoc))
 - rust-lang#83680 (Update for loop desugaring docs)
 - rust-lang#83683 (bootstrap: don't complain about linkcheck if it is excluded)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 7391124 into rust-lang:master Mar 31, 2021
@rustbot rustbot added this to the 1.53.0 milestone Mar 31, 2021
@steffahn steffahn deleted the prettify_prelude_imports branch March 31, 2021 10:01
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 8, 2021
Improve links in inline code in `core::pin`.

## Context

So I recently opened rust-lang#80720. That PR uses HTML-based `<code>foo</code>` syntax in place of `` `foo` `` for some inline code. It looks like usage of `<code>` tags in doc comments is without precedent in the standard library, but the HTML-based syntax has an important advantage:

You can write something like
```
<code>[Box]<[Option]\<T>></code>
```
which becomes: <code>[Box]<[Option]\<T>></code>, whereas with ordinary backtick syntax, you cannot create links for a substring of an inline code block.

## Problem
I recalled (from my own experience) that a way to partially work around this limitation is to do something like
```
[`Box`]`<`[`Option`]`<T>>`
```
which looks like this: [`Box`]`<`[`Option`]`<T>>` _(admitted, it looks even worse on GitHub than in `rustdoc`’s CSS)_.

[Box]: https://doc.rust-lang.org/std/boxed/struct.Box.html "Box"
[`Box`]: https://doc.rust-lang.org/std/boxed/struct.Box.html "Box"
[Option]: https://doc.rust-lang.org/std/option/enum.Option.html "Option"
[`Option`]: https://doc.rust-lang.org/std/option/enum.Option.html "Option"
[Pin]: https://doc.rust-lang.org/std/pin/struct.Pin.html "Pin"
[&mut]: https://doc.rust-lang.org/std/primitive.reference.html "mutable reference"

So I searched the standard library and found that e.g. the [std::pin](https://doc.rust-lang.org/std/pin/index.html) module documentation uses this hack/workaround quite a bit, with types like <code>[Pin]<[Box]\<T>></code> or <code>[Pin]<[&mut] T>></code>. Although the way they look like in this sentence is what I would like them to look like, not what they currently look.

### Status Quo

Here’s a screenshot of what it currently looks like:
![Screenshot_20210105_202751](https://user-images.githubusercontent.com/3986214/103692608-4a978780-4f98-11eb-9451-e13622b2e3c0.png)

With a few HTML-style code blocks, we can fix all the spacing issues in the above screenshot that are due usage of this hack/workaround of putting multiple code blocks right next to each other being used.

### after d3915c5:
![Screenshot_20210105_202932](https://user-images.githubusercontent.com/3986214/103692688-6f8bfa80-4f98-11eb-9be5-9b370eaef644.png)

There’s still a problem of inconsistency. Especially in a sentence such as
> A [`Pin<P>`][Pin] where `P: Deref` should be considered as a "`P`-style pointer" to _[...]_

looks weird with the variable `P` having different colors (and `Deref` has a different color from before where it was a link, too). Or compare the difference of <code>[Pin]<[Box]\<T>></code> vs [`Box<T>`][Box] where one time the variable is part of the link and the other time it isn’t.

_Note: Color differences show even **more strongly** when the ayu theme is used, while they are a bit less prominent in the light theme than they are in the dark theme, which is the one used for these screenshots._

This is why I’ve added the next commit
### after ceaeb24
![Screenshot_20210105_203113](https://user-images.githubusercontent.com/3986214/103693496-ab738f80-4f99-11eb-942d-29dace459734.png)
pulling all the type parameters out of their links, and also the last commit with clearly visible changes
### after 87ac118
![Screenshot_20210105_203252](https://user-images.githubusercontent.com/3986214/103693625-e5dd2c80-4f99-11eb-91b7-470c37934e7e.png)
where more links are added, removing e.g. the inconsistency with `Deref`’s color in e.g. `P: Deref` that I already mentioned above.

## Discussion

I am aware that this PR may very well be overkill. If for now only the first commit (plus the fix for the `Drop` link in e65385f, the link titles 684edf7 as far as they apply, and a few of the line-break changes) are wanted, I can reduce this PR to just those changes. I personally find the rendered result with all these changes very nice though. On the other hand, all these `<code>` tags are not very nice in the source code, I’ll admit.

Perhaps alternative solutions could be preferred, such as `rustdoc` support for merging subsequent inline code blocks so that all the cases that currently use workarounds rendered as [`Box`]`<`[`Option`]`<T>>` automatically become <code>[Box]<[Option]\<T>></code> without any need for further changes. Even in this case, having a properly formatted, better looking example in the standard library docs could help motivate such a change to `rustdoc` by prodiving an example of the expected results and also the already existing alternative (i.e. using `<code>`). On the other hand, `` [`Box`]`<`[`Option`]`<T>>` `` isn’t particularly nice-looking source code either. I’m not even sure if I wouldn’t actually find the version `<code>[Box]<[Option]\<T>></code>` cleaner to read.

`@rustbot` modify labels: T-doc, T-rustdoc
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 8, 2021
Improve links in inline code in `core::pin`.

## Context

So I recently opened rust-lang#80720. That PR uses HTML-based `<code>foo</code>` syntax in place of `` `foo` `` for some inline code. It looks like usage of `<code>` tags in doc comments is without precedent in the standard library, but the HTML-based syntax has an important advantage:

You can write something like
```
<code>[Box]<[Option]\<T>></code>
```
which becomes: <code>[Box]<[Option]\<T>></code>, whereas with ordinary backtick syntax, you cannot create links for a substring of an inline code block.

## Problem
I recalled (from my own experience) that a way to partially work around this limitation is to do something like
```
[`Box`]`<`[`Option`]`<T>>`
```
which looks like this: [`Box`]`<`[`Option`]`<T>>` _(admitted, it looks even worse on GitHub than in `rustdoc`’s CSS)_.

[Box]: https://doc.rust-lang.org/std/boxed/struct.Box.html "Box"
[`Box`]: https://doc.rust-lang.org/std/boxed/struct.Box.html "Box"
[Option]: https://doc.rust-lang.org/std/option/enum.Option.html "Option"
[`Option`]: https://doc.rust-lang.org/std/option/enum.Option.html "Option"
[Pin]: https://doc.rust-lang.org/std/pin/struct.Pin.html "Pin"
[&mut]: https://doc.rust-lang.org/std/primitive.reference.html "mutable reference"

So I searched the standard library and found that e.g. the [std::pin](https://doc.rust-lang.org/std/pin/index.html) module documentation uses this hack/workaround quite a bit, with types like <code>[Pin]<[Box]\<T>></code> or <code>[Pin]<[&mut] T>></code>. Although the way they look like in this sentence is what I would like them to look like, not what they currently look.

### Status Quo

Here’s a screenshot of what it currently looks like:
![Screenshot_20210105_202751](https://user-images.githubusercontent.com/3986214/103692608-4a978780-4f98-11eb-9451-e13622b2e3c0.png)

With a few HTML-style code blocks, we can fix all the spacing issues in the above screenshot that are due usage of this hack/workaround of putting multiple code blocks right next to each other being used.

### after d3915c5:
![Screenshot_20210105_202932](https://user-images.githubusercontent.com/3986214/103692688-6f8bfa80-4f98-11eb-9be5-9b370eaef644.png)

There’s still a problem of inconsistency. Especially in a sentence such as
> A [`Pin<P>`][Pin] where `P: Deref` should be considered as a "`P`-style pointer" to _[...]_

looks weird with the variable `P` having different colors (and `Deref` has a different color from before where it was a link, too). Or compare the difference of <code>[Pin]<[Box]\<T>></code> vs [`Box<T>`][Box] where one time the variable is part of the link and the other time it isn’t.

_Note: Color differences show even **more strongly** when the ayu theme is used, while they are a bit less prominent in the light theme than they are in the dark theme, which is the one used for these screenshots._

This is why I’ve added the next commit
### after ceaeb24
![Screenshot_20210105_203113](https://user-images.githubusercontent.com/3986214/103693496-ab738f80-4f99-11eb-942d-29dace459734.png)
pulling all the type parameters out of their links, and also the last commit with clearly visible changes
### after 87ac118
![Screenshot_20210105_203252](https://user-images.githubusercontent.com/3986214/103693625-e5dd2c80-4f99-11eb-91b7-470c37934e7e.png)
where more links are added, removing e.g. the inconsistency with `Deref`’s color in e.g. `P: Deref` that I already mentioned above.

## Discussion

I am aware that this PR may very well be overkill. If for now only the first commit (plus the fix for the `Drop` link in e65385f, the link titles 684edf7 as far as they apply, and a few of the line-break changes) are wanted, I can reduce this PR to just those changes. I personally find the rendered result with all these changes very nice though. On the other hand, all these `<code>` tags are not very nice in the source code, I’ll admit.

Perhaps alternative solutions could be preferred, such as `rustdoc` support for merging subsequent inline code blocks so that all the cases that currently use workarounds rendered as [`Box`]`<`[`Option`]`<T>>` automatically become <code>[Box]<[Option]\<T>></code> without any need for further changes. Even in this case, having a properly formatted, better looking example in the standard library docs could help motivate such a change to `rustdoc` by prodiving an example of the expected results and also the already existing alternative (i.e. using `<code>`). On the other hand, `` [`Box`]`<`[`Option`]`<T>>` `` isn’t particularly nice-looking source code either. I’m not even sure if I wouldn’t actually find the version `<code>[Box]<[Option]\<T>></code>` cleaner to read.

`@rustbot` modify labels: T-doc, T-rustdoc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants