Improve links in inline code in `core::pin`. #80733

steffahn · 2021-01-05T20:32:02Z

Context

So I recently opened #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: Box<Option<T>>, 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).

So I searched the standard library and found that e.g. the std::pin module documentation uses this hack/workaround quite a bit, with types like Pin<Box<T>> or Pin<&mut T>>. 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:

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`:

There’s still a problem of inconsistency. Especially in a sentence such as

A Pin<P> 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 Pin<Box<T>> vs Box<T> 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`

pulling all the type parameters out of their links, and also the last commit with clearly visible changes

after `87ac118`

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 Box<Option<T>> 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

…ks using HTML-syntax.

…ual consistency.

…the markdown link definition names being case insensitive.

rust-highfive · 2021-01-05T20:32:06Z

r? @sfackler

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

steffahn · 2021-01-05T21:06:54Z

Remarks for review:

as hinted above, the first commit is the most important to me – and the last commit is “only whitespace” – so reviewing each commit individualy could be a good approach
those two occurrences -- that I’ve removed are replaced by en-dashes – i.e. this character right here in this sentence. This doesn’t show particularly well in monospace. I don’t know if unicode punctionation characters are allowed in the standard library docs -- but two simple hyphens look super weird, IMO. There’s also the option to use em dashes—I’m not a native english speaker/writer—I find those a bit weird, especially without spaces, but perhaps those would be the most correct.
(Okay, after a grep search I can say that all four versions: en-dash, em-dash without and with spaces, and more cases of double-hyphen with spaces do exist in the std-docs already, although none is particularly common.)

sfackler · 2021-01-06T02:31:07Z

r? @rust-lang/docs

pickfire · 2021-01-06T02:44:40Z

Is it necessary to have all the links clickable? Like those Box and Pin? It maybe be better to have rustdoc autodiscover them (but it doesn't right now). But I wonder why don't we reduce the number of clickable links? Like only make one of them clickable (usually the first one or in the page)?

GuillaumeGomez · 2021-01-06T09:42:49Z

Is it necessary to have all the links clickable? Like those Box and Pin? It maybe be better to have rustdoc autodiscover them (but it doesn't right now). But I wonder why don't we reduce the number of clickable links? Like only make one of them clickable (usually the first one or in the page)?

We have made all the types clickable if possible and (imo), it makes it much more nice to use.

@steffahn I have to admit, this is a bit weird to use a code block instead of what we're doing everywhere else. For coherence, I'd say it'd be better to continue like the rest but not sure what @rust-lang/docs think about it...

steffahn · 2021-01-06T12:47:10Z

@pickfire I also like the almost source-code-formatting like nature of having things be links. Another aspect is that next to being clickable, you can also get a tooltip by hovering over the links that can include a more complete path. E.g. the drops identifying themselves as Drop::drop instead of mem::drop, i.e.: drop. Or the P::Target that you only need to hover over to find out that it comes from the Deref trait.

@GuillaumeGomez Yes, as I said <code> doesn’t seem to be used anywhere else yet, except perhaps in my own PR #80720 in case that gets merged (I’m fairly positive it will). The workaround chaining multiple code blocks next to each other is used in other places though. I would propose to eventually change these other places, too, though. (I would actually grep for them and create another PR myself with more changes like the ones in my first commit here, d3915c5, just addressing these already existing cases where the spacing is currently broken.)

So I agree that coherence has its value, but we could easily change other places, too, and coherently use a different solution. One main point of this PR is indeed that I’m not sure how far I can take this use of <code> without it becoming inacceptable. Limiting myself to just core::pin is mostly to avoid unnecessary work before getting any feedback.

Also I don’t believe we should be necessarily going all the way as I’ve done in this PR throughout the whole standard library. I think the documentation of core::pin is particularly nice and long and detailed text, so it could reasonably have particularly nice formatting, too; perhaps along with a few more similarly in-depth explanation pages (I haven’t searched too much for other good “candidates” yet, but to give a smaller example, sync::Arc has “Arc<RefCell<T>>” twice that IMO needs fixing, and since it doesn’t link to its own page, the type parameter T switches between colors all the time, too.)

GuillaumeGomez · 2021-01-06T12:55:37Z

Just to be clear: I didn't mean it in a negative way, I like the output. Just don't know what we should about it.

steveklabnik · 2021-01-11T19:09:34Z

Procedural note: the docs team no longer exists, and so this is up to @rust-lang/libs to decide, formally speaking.

KodrAus · 2021-01-11T23:18:10Z

Do we have a style guide or anything we can use to document any of these workarounds we come up with?

Without something like that I have a mild preference against using <code> and other direct HTML content in the docs. The output is nice, but it makes the source harder to read, harder to write, and opens up too many possibilities for creating inconsistencies in presentation and structure.

This is definitely an improvement over [Pin]<[Box]<T>>, but I'm not sure that's worth the hacking around inline code to create links for both Pin and Box in the first place.

m-ou-se · 2021-01-30T13:10:35Z

@jyn514 @camelid I'd like to hear your thoughts about this as well. Is this something rustdoc could/should help with in the future? (Either with automatic linking, or by merging code blocks, or in some other way.) And if not, do you think this is something we should start doing manually, like in this PR?

GuillaumeGomez · 2021-01-30T13:15:15Z

I'm not sure we should add handlings about this in rustdoc because it could be an issue in case you actually wanted it to look like that (and that'd make it impossible to revert...).

camelid · 2021-01-30T23:35:53Z

jyn514 camelid I'd like to hear your thoughts about this as well. Is this something rustdoc could/should help with in the future? (Either with automatic linking, or by merging code blocks, or in some other way.) And if not, do you think this is something we should start doing manually, like in this PR?

We've thought about automatic linking before (see #62834 (comment)),
but (a) it's likely tricky to implement, so to my knowledge it hasn't been
attempted, but also (b) in many situations people don't want nested links like
Vec<u32> and just want one Vec<u32> link. So, I ended up
implementing a simpler approach that just treats links like [`Vec<u32>`] as
if they were [`Vec<u32>`](Vec) (#76934).

However, I've thought before about merging code blocks as you suggested, and I
think this might be a win-win approach. Specifically, I think we'd do this by
adding special CSS classes that indicate that one side of inline code (or both
sides) should not have padding, margin, or border-radius, and then it would
visually look like one block of code. E.g., like this:

(Imagine that the outer corners of each group of inline code are rounded.)

The generated HTML for [`Vec`]`<`[`Option`]`<T>>` (admittedly that's
pretty hard to read, but I think it's better than using HTML — for one, it's not
restricted to web browsers) would then look like:

<a><code class="inline-code-left">Vec</code></a>
<code class="inline-code-inner">&lt;</code>
<a><code class="inline-code-inner">Option</code></a>
<code class="inline-code-right">&lt;T&gt;&gt;</code>

And things like [`Vec`] would continue to look like:

<a><code>Vec</code></a>

What do you think of that @GuillaumeGomez?

(By the way, jyn514 is on break so he may not respond to pings.)

camelid · 2021-01-30T23:41:22Z

And if not, do you think this is something we should start doing manually,
like in this PR?

Personally (not speaking on behalf of T-rustdoc), I would try to avoid using
HTML unless you really have to, because it's not cross-platform. E.g., if you
use HTML then people won't be able to render the docs to LaTeX or some other
non-HTML format. Most people probably don't do that, but in general I think it's
good to stick to pure Markdown so it's flexible.

However, it's just a formatting decision, so you can also remove all the code
tags later in favor of using ` in case you (T-libs) change your mind, so
ultimately I think it's not a big deal whether you use code tags or not :)

jyn514 · 2021-02-01T16:54:50Z

Personally (not speaking on behalf of T-rustdoc), I would try to avoid using
HTML unless you really have to, because it's not cross-platform. E.g., if you
use HTML then people won't be able to render the docs to LaTeX or some other
non-HTML format. Most people probably don't do that, but in general I think it's
good to stick to pure Markdown so it's flexible.

Markdown is a superset of HTML, anything that parses markdown has to deal with HTML already. https://rust-lang.zulipchat.com/#narrow/stream/266220-rustdoc/topic/warning.20sections.20in.20rustdoc.3A.20.2379677/near/218945328

GuillaumeGomez · 2021-02-01T18:07:15Z

@jyn514 It's not entirely true (even though it's not wrong). Let's say markdown supports HTML but it's definitely not intended to be over-using it.

camelid · 2021-02-01T21:15:15Z

Personally (not speaking on behalf of T-rustdoc), I would try to avoid using
HTML unless you really have to, because it's not cross-platform. E.g., if you
use HTML then people won't be able to render the docs to LaTeX or some other
non-HTML format. Most people probably don't do that, but in general I think it's
good to stick to pure Markdown so it's flexible.

Markdown is a superset of HTML, anything that parses markdown has to deal with HTML already. https://rust-lang.zulipchat.com/#narrow/stream/266220-rustdoc/topic/warning.20sections.20in.20rustdoc.3A.20.2379677/near/218945328

Yes, I know that technically tools have to deal with it, but if you're translating to LaTeX it's likely it will just ignore HTML. So, it's better in that case to just not use it in the first place if you can avoid it.

Dylan-DPC-zz · 2021-02-23T15:14:46Z

r? @KodrAus

library/core/src/pin.rs

jyn514 · 2021-04-08T15:37:26Z

@bors r+

Since #80720 was approved I think this should be too. I agree with @camelid we should have rustdoc do this automatically - maybe #83997 can be the tracking issue for that?

bors · 2021-04-08T15:37:28Z

📌 Commit 3e0cef7 has been approved by jyn514

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

bors · 2021-04-08T17:06:00Z

⌛ Testing commit 3e0cef7 with merge 4a284ada2697c68939384e25c77a9600a479ab66...

rust-log-analyzer · 2021-04-08T17:10:03Z

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

bors · 2021-04-08T17:11:37Z

💔 Test failed - checks-actions

steffahn · 2021-04-08T17:42:23Z

@jyn514 a step called “install MinGW” failed in CI AFAICT. You set “waiting-on-author”, but there isn’t anything I can do about this, is there?

jyn514 · 2021-04-08T18:06:05Z

Oh sorry, I didn't actually read the CI error.

@bors retry

Rollup of 6 pull requests Successful merges: - rust-lang#80733 (Improve links in inline code in `core::pin`.) - rust-lang#81764 (Stabilize `rustdoc::bare_urls` lint) - rust-lang#81938 (Stabilize `peekable_peek_mut`) - rust-lang#83980 (Fix outdated crate names in compiler docs) - rust-lang#83992 (Merge idents when generating source content) - rust-lang#84001 (Update Clippy) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

Fix spacing of links in inline code. Similar to rust-lang#80733, but the focus is different. This PR eliminates all occurrences of pieced-together inline code blocks like [`Box`]`<`[`Option`]`<T>>` and replaces them with good-looking ones (using HTML-syntax), like <code>[Box]<[Option]\<T>></code>. As far as I can tell, I should’ve found all of these in the standard library (regex search with `` r"`\]`|`\[`" ``) \[except for in `core::convert` where I’ve noticed other things in the docs that I want to fix in a separate PR]. In particular, unlike rust-lang#80733, I’ve added almost no new instance of inline code that’s broken up into multiple links (or some link and some link-free part). I also added tooltips (the stuff in quotes for the markdown link listings) in places that caught my eye, but that’s by no means systematic, just opportunistic. [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" Context: I got annoyed by repeatedly running into new misformatted inline code while reading the standard library docs. I know that once issue rust-lang#83997 (and/or related ones) are resolved, these changes become somewhat obsolete, but I fail to notice much progress on that end right now. r? `@jyn514`

steffahn added 6 commits January 5, 2021 17:08

Fix spacing in docs for core::pin by combining consequent code bloc…

d3915c5

…ks using HTML-syntax.

Exclude single type parameters from links in core::pin for more vis…

ceaeb24

…ual consistency.

Add more links to core::pin to improve visual consistency.

87ac118

Fix broken links to Drop that used to point to Drop::drop due to …

e65385f

…the markdown link definition names being case insensitive.

Add titles (tooltips) to defined links in core::pin.

684edf7

Fix overlength lines in core::pin.

3e0cef7

rustbot added A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Jan 5, 2021

rust-highfive assigned sfackler Jan 5, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 5, 2021

rust-highfive unassigned sfackler Jan 6, 2021

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 23, 2021

crlf0710 added the S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). label Apr 2, 2021

jyn514 mentioned this pull request Apr 8, 2021

Documentation: inline code with links split into multiple blocks #83997

Open

jyn514 reviewed Apr 8, 2021

View reviewed changes

library/core/src/pin.rs Show resolved Hide resolved

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-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Apr 8, 2021

Dylan-DPC-zz mentioned this pull request Apr 8, 2021

Rollup of 5 pull requests #84005

Closed

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 8, 2021

jyn514 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 8, 2021

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 8, 2021

Dylan-DPC-zz mentioned this pull request Apr 8, 2021

Rollup of 6 pull requests #84008

Merged

bors merged commit 689978c into rust-lang:master Apr 8, 2021

rustbot added this to the 1.53.0 milestone Apr 8, 2021

This was referenced Apr 9, 2021

rustdoc: Use CSS to "merge" sequential inline code spans #84043

Closed

Remove padding when code spans/code link spans are adjacent #62867

Open

steffahn deleted the prettify_pin_links branch May 20, 2021 10:26

steffahn mentioned this pull request Aug 25, 2021

Fix spacing of links in inline code. #88343

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve links in inline code in `core::pin`. #80733

Improve links in inline code in `core::pin`. #80733

steffahn commented Jan 5, 2021

rust-highfive commented Jan 5, 2021

steffahn commented Jan 5, 2021

sfackler commented Jan 6, 2021

pickfire commented Jan 6, 2021

GuillaumeGomez commented Jan 6, 2021

steffahn commented Jan 6, 2021

GuillaumeGomez commented Jan 6, 2021

steveklabnik commented Jan 11, 2021

KodrAus commented Jan 11, 2021

m-ou-se commented Jan 30, 2021

GuillaumeGomez commented Jan 30, 2021

camelid commented Jan 30, 2021

camelid commented Jan 30, 2021

jyn514 commented Feb 1, 2021

GuillaumeGomez commented Feb 1, 2021

camelid commented Feb 1, 2021

Dylan-DPC-zz commented Feb 23, 2021

jyn514 commented Apr 8, 2021

bors commented Apr 8, 2021

bors commented Apr 8, 2021

rust-log-analyzer commented Apr 8, 2021

bors commented Apr 8, 2021

steffahn commented Apr 8, 2021

jyn514 commented Apr 8, 2021

Improve links in inline code in core::pin. #80733

Improve links in inline code in core::pin. #80733

Conversation

steffahn commented Jan 5, 2021

Context

Problem

Status Quo

after d3915c5:

after ceaeb24

after 87ac118

Discussion

rust-highfive commented Jan 5, 2021

steffahn commented Jan 5, 2021

sfackler commented Jan 6, 2021

pickfire commented Jan 6, 2021

GuillaumeGomez commented Jan 6, 2021

steffahn commented Jan 6, 2021

GuillaumeGomez commented Jan 6, 2021

steveklabnik commented Jan 11, 2021

KodrAus commented Jan 11, 2021

m-ou-se commented Jan 30, 2021

GuillaumeGomez commented Jan 30, 2021

camelid commented Jan 30, 2021

camelid commented Jan 30, 2021

jyn514 commented Feb 1, 2021

GuillaumeGomez commented Feb 1, 2021

camelid commented Feb 1, 2021

Dylan-DPC-zz commented Feb 23, 2021

jyn514 commented Apr 8, 2021

bors commented Apr 8, 2021

bors commented Apr 8, 2021

rust-log-analyzer commented Apr 8, 2021

bors commented Apr 8, 2021

steffahn commented Apr 8, 2021

jyn514 commented Apr 8, 2021

Improve links in inline code in `core::pin`. #80733

Improve links in inline code in `core::pin`. #80733

after `d3915c5`:

after `ceaeb24`

after `87ac118`