Skip to content

Conversation

GuillaumeGomez
Copy link
Member

@GuillaumeGomez GuillaumeGomez commented Jan 30, 2025

Fixes #62867.
Fixes #83997.

When we have multiple <code> tags following each others, we never tried merging them into one, especially when a <code> is inside a <a>. This PR merges them into one <code>, making the generated HTML shorter. And it has the nice side-effect of also making it look nicer.

Before:

image

After:

image

You can test it here.

r? @notriddle

@GuillaumeGomez GuillaumeGomez marked this pull request as ready for review January 30, 2025 16:48
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Jan 30, 2025
@hkBst
Copy link
Member

hkBst commented Jan 30, 2025

The "A <Stream a> stuff." does not seem to get rid of extra spacing yet... for me on firefox if that matters.

  • <Stream> vs <Stream a>
  • <code>&lt;<a href="[struct.Foo.html](view-source:https://rustdoc.crud.net/imperio/merge-code-tags/foo/struct.Foo.html)" title="struct foo::Foo">Stream</a>&gt;</code> vs
  • <code>&lt;</code><a href="[struct.Foo.html](view-source:https://rustdoc.crud.net/imperio/merge-code-tags/foo/struct.Foo.html)" title="struct foo::Foo"><code>Stream</code> a</a><code>&gt;</code>

The < is still in its own code block...

@notriddle
Copy link
Contributor

notriddle commented Jan 30, 2025

I'm not convinced that this is a good solution?

Other markdown implementations don't merge consecutive code blocks, the resulting source is noisy with ` marks, and you can already fix this with <code> tags like in this example from #88343:

Additionally, the return value of this function is [`fmt::Result`] which is a
type alias of <code>[Result]<(), [std::fmt::Error]></code>. Formatting implementations

The alternative, [`Result`]`<(), `[`std::fmt::Error`]`>`, has a bunch more formatting characters inside the intended text, which seems worse than the HTML-based solution.

@GuillaumeGomez
Copy link
Member Author

The "A <Stream a> stuff." does not seem to get rid of extra spacing yet... for me on firefox if that matters.

* `<Stream>` vs `<``Stream` a`>`

* `<code>&lt;<a href="[struct.Foo.html](view-source:https://rustdoc.crud.net/imperio/merge-code-tags/foo/struct.Foo.html)" title="struct foo::Foo">Stream</a>&gt;</code>` vs

* `<code>&lt;</code><a href="[struct.Foo.html](view-source:https://rustdoc.crud.net/imperio/merge-code-tags/foo/struct.Foo.html)" title="struct foo::Foo"><code>Stream</code> a</a><code>&gt;</code>`

The < is still in its own code block...

Obviously yes. A whitespace is a character on its own and should not be merged if not part of a code. And in the case you displayed, it's even worse: it's a non-<code> in a <a>. We can improve this situation with CSS but it'll be part of a follow-up.

I'm not convinced that this is a good solution?

Other markdown implementations don't merge consecutive code blocks, the resulting source is noisy with ` marks, and you can already fix this with <code> tags like in this example from #88343:

Additionally, the return value of this function is [`fmt::Result`] which is a
type alias of **<code>[Result]<(), [std::fmt::Error]></code>**. Formatting implementations

The alternative, [`Result`]`<(), `[`std::fmt::Error`]`>`, has a bunch more formatting characters inside the intended text, which seems worse than the HTML-based solution.

It's a case common enough when intra-doc links are used and people are generally not aware that you can simply use HTML with markdown. I agree that it's less good than using <code>, but it's worth noting that even in the std, there are cases like that.

An alternative would be writing a rustdoc lint. I tend to prefer the current approach as it's invisible to the user though.

notriddle added a commit to notriddle/rust-clippy that referenced this pull request Jan 30, 2025
This is the lint described at
rust-lang/rust#136308 (comment)
that recommends using HTML to nest links inside code.
notriddle added a commit to notriddle/rust-clippy that referenced this pull request Jan 30, 2025
This is the lint described at
rust-lang/rust#136308 (comment)
that recommends using HTML to nest links inside code.
notriddle added a commit to notriddle/rust-clippy that referenced this pull request Jan 30, 2025
This is the lint described at
rust-lang/rust#136308 (comment)
that recommends using HTML to nest links inside code.
notriddle added a commit to notriddle/rust-clippy that referenced this pull request Jan 30, 2025
This is the lint described at
rust-lang/rust#136308 (comment)
that recommends using HTML to nest links inside code.
notriddle added a commit to notriddle/rust-clippy that referenced this pull request Feb 4, 2025
This is the lint described at
rust-lang/rust#136308 (comment)
that recommends using HTML to nest links inside code.
github-merge-queue bot pushed a commit to rust-lang/rust-clippy that referenced this pull request Feb 14, 2025
…14121)

This is the lint described at
rust-lang/rust#136308 (comment)
that recommends using HTML to nest links inside code.

changelog: [`doc_link_code`]: warn when a link with code and a code span
are back-to-back
@notriddle
Copy link
Contributor

We've merged the clippy lint.

Since it solves the same problem, this PR isn't a complete solution, and it's not clear if any complete solution is coming, I propose we close this PR and its linked issues as not-planed.

@rfcbot poll we should recommend HTML <code> tags to anyone who runs into this

@rfcbot
Copy link

rfcbot commented Feb 18, 2025

Team member @notriddle has asked teams: T-rustdoc, for consensus on:

we should recommend HTML <code> tags to anyone who runs into this

@GuillaumeGomez
Copy link
Member Author

Let's close it, no need to wait for the FCP to be over.

@GuillaumeGomez GuillaumeGomez deleted the merge-code-tags branch February 28, 2025 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Documentation: inline code with links split into multiple blocks Remove padding when code spans/code link spans are adjacent
5 participants