Add warning block support in rustdoc

[GuillaumeGomez]

Fixes #79710.

You can test it here. It currently looks like this:

So a few things to note:

• Since it's a new add and it's changing the UI, we'll need to go through an FCP.
• Does the UI looks good?
• Is the way picked to add a warning block ok for everyone? The discussion on the issue seemed to be in favour of this solution but it doesn't hurt to double-check.

cc @rust-lang/rustdoc

[rustbot]

r? @notriddle

(rustbot has picked a reviewer for you, use r? to override)

[rustbot]

A change occurred in the Ayu theme.

cc @Cldfire

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez, @Folyd, @jsha

[notriddle]

This seems related to #102100, which was written to allow using the class=stab more easily. It seems reasonable to me, given that precedent.

[GuillaumeGomez]

There was another suggestion in the issue: #79710 (comment)

[Manishearth]

overall looks good. unsure about the exact styling, deferring to others

[Manishearth]

Suggested change

	If you want to make a "warning" stand out in the documentation, you can wrap it like this:
	If you want to make a warning or similar note stand out in the documentation, you can wrap it like this:

(the quotation marks don't quite make sense here)

[fmease]

I love the initiative! ❤️ Regarding the styling, what do you think about having a slightly orange background for every theme? Also, how easy would it be to add a warning symbol to the left of the vertical bar (e.g. via ::pre)?

[GuillaumeGomez]

By adding:

.warning::before {
    content: "⚠️ ";
}

It gives:

I like it too. If others like it as well, I'll make the change.

[coastalwhite]

Maybe, it is an idea to give it the same warning icon as the compile_fail of code snippets. This will render very differently on different platforms.

[GuillaumeGomez]

I don't want warning blocks to use the same symbol to differentiate it from compile fail, ignore, etc. But maybe I'm in the wrong here?

[Nemo157]

There was another suggestion in the issue: #79710 (comment)

I really like this option, there's two points I think really make it favorable:

• having it work with include_str! and github's rendering
• having it look good even in the case of markdown renderers that don't understand the syntax (a corollary to the first for when you use a different forge or a local renderer)

As an example of the second, this is what it would look like if GFM didn't support it:

Wawnying
Don't foobar the fizzbuzz

whereas this is what the current syntax would look like:

Don't foobar the fizzbuzz

---

(EDIT: there is some good counter-discussion in the tracking issue for that feature around localization, syntax and semantics, though a lot of the syntax discussion seems to discount how important it is that this looks good in other renderers while introducing new syntax doesn't)

[notriddle]

As far as other renders go:

https://talk.commonmark.org/t/github-is-beta-testing-their-own-admonition-syntax-we-should-weigh-in/4173

[Manishearth]

@GuillaumeGomez hmmm

I'd rather let people use whatever emoji they want there: people use warning blocks to talk about unstable APIs, unsafety, etc, and they may choose different emoji. Let's keep it simple, imo

[GuillaumeGomez]

@Manishearth What do you think about github's syntax btw?

[Manishearth]

#79710 (comment)

t I'd really like to avoid adding more markdown syntax where possible. This feature works fine without special syntax, I don't think we need it.

So, imo let's not.

[Manishearth]

The reason being: That's a much larger change and impacts downstream tooling as well. We should stay as close to standard markdown as possible, unless an extension really needs to be made to be able to get a feature. The more tweaks we do the more work downstream tooling needs to do: both tools that consume Rust doc comments, and authoring tools that try to understand/highlight Rust code.

[GuillaumeGomez]

It's not an add so to speak, it remains perfectly valid markdown. But it requires more code to handle it for rustdoc output indeed. Well, I'll update the PR and start the FCP so rustdoc people can tell if they accept this solution or not.

[Manishearth]

It's not an add so to speak, it remains perfectly valid markdown. But it requires more code to handle it for rustdoc output indeed.

Yes, that's still an extension. A normal markdown engine will not correctly handle this behavior and will look ugly.

Intra-doc links had a similar risk, but in that situation the extra behavior was a key component of the benefit of the feature, and it was explicitly decided to be worth it.

That's not the case here: the primary benefit of this feature is access to convenient, cross-theme standardized styling, not brevity of the syntax.

[GuillaumeGomez]

Not to continue the debate, just to have an example of the rendering without the extension:

warning this a warning
block hello hola

[Manishearth]

I think in the long run: if GitHub syntax happens, and if it grows in adoption, including outside of GitHub contexts¹, we can consider adding that syntax. For now: I do not want to deviate from standard Markdown (and yes, that includes deviations in semantics, not just deviations in syntax: I am aware all the proposals are already syntactically valid) unless there is a very strong reason, and it would absolutely need an RFC.

Footnotes

1. To give an example of this: footnote syntax is not uniformly a part of Markdown, but it is often found as an extension, in multiple communities. It didn't originate in GitHub (I think it comes from the Ruby community? It was added to GitHub a couple years ago) but it's now accepted across multiple communities. ↩

[Manishearth]

Additionally: internationalization becomes a mess for syntax like this. Do we replace the text? Do we learn to parse the word "warning" in a bajillion languages (this is a bad idea).

All of this is turning a rather simple easy-to-land proposal (I want to eventually do something similar for stab unstable!) into a much, much bigger issue

[notriddle]

As for styling, I would like something like this:

.content .docblock .warning {
	border-left: 2px solid var(--warning-border-color);
	padding: 14px;
	position: relative;
	overflow-x: visible;
	margin-bottom: 10px;
}
.content .docblock .warning:last-child {
	margin-bottom: 0;
}
.content .docblock .warning::before {
	color: var(--warning-border-color);
	content: "ⓘ";
	position: absolute;
	left: -25px;
	top: 5px;
	font-weight: bold;
	font-size: 1.25rem;
}

Which, when rendered, looks like this:

It deliberately doesn't include a background, so it won't be confused with a code block, but it does use the same padding and border and icon style, so it looks consistent. Essentially, I'm looking to create a visual language by retroactively decomposing "should_panic code blocks" into a "code block" combined with a "warning block":

	No warning	Warning
Not code
Code

[Manishearth]

Oh, I kinda like that. Should it also have a slightly yellow background? It still looks somewhat like a code block.

[notriddle]

The tricky part is that I would want to make sure that there's a difference in lightness and not just hue. Otherwise, while it might look easy-to-distinguish normally, it could become very similar to a code block if the reader has trouble discerning color (such as if they're colorblind, or viewing on a monochrome display, or running with some kind of "night vision" mode that removes all the blue tones).

[GuillaumeGomez]

Additionally: internationalization becomes a mess for syntax like this. Do we replace the text? Do we learn to parse the word "warning" in a bajillion languages (this is a bad idea).

All of this is turning a rather simple easy-to-land proposal (I want to eventually do something similar for stab unstable!) into a much, much bigger issue

All great points! This is indeed not a good idea. Thanks for the explanations!

[anden3]

Hello @GuillaumeGomez! I noticed there's some merge conflicts for this PR, what's the status of it?

[GuillaumeGomez]

Agreeing on a look.

[GuillaumeGomez]

As discussed on zulip, the team is ok with this change. I'll wait for a week before r+ing it to be sure everyone is ok with this (would be better if the people who agreed ticked their box though).

[GuillaumeGomez]

Then let's go!

@bors r=rustdoc

[bors]

📌 Commit 1d42913 has been approved by rustdoc

It is now in the queue for this repository.

[bors]

⌛ Testing commit 1d42913 with merge 5e9d3d8...

[fmease]

@rustbot label relnotes

[bors]

☀️ Test successful - checks-actions
Approved by: rustdoc
Pushing 5e9d3d8 to master...

[rust-timer]

Finished benchmarking commit (5e9d3d8): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.9%	[2.6%, 3.1%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 635.443s -> 635.148s (-0.05%)
Artifact size: 347.02 MiB -> 347.01 MiB (-0.00%)