Add syntax extension to add warning blocks

[GuillaumeGomez]

Following #79677, we realized that rustdoc users would prefer to have a syntax extension rather than using plain HTML to have warning blocks.

@dtolnay suggested the following:

/// WARNING: it doesn't work yet.

@Darksonn suggested:

[[info]]
| Many of Tokio's types are named the same as their synchronous equivalent in
| the Rust standard library. When it makes sense, Tokio exposes the same APIs
| as `std` but using `async fn`.

What do you think?

cc @rust-lang/rustdoc

[CraftSpider]

Personal Opinions:
The first style will look fine for short forms, but I'm not sure for longer text. A possible syntax:

/// WARNING: This is a lot of text, continued
///          on multiple lines, and what if I want more formatting, or another block?
///          *this is interesting*

Which to me pushes the text too far right. Not having any spacing, but just separating with newlines, seems hard to parse at a glance what is part of the WARNING, and prevents multiple line breaks inside the warning. Also, it's not as extensible. INFO can be added, but if we ever want more extensions, they would increasingly possibly conflict with other things.
The second style makes it immediately clear what is part of the block, and is unlikely to accidentally conflict if we ever want to add more block styles. It also supports multiple lines or inner blocks trivially. A final advantage is that it may support nested blocks, in case those ever come to be useful.
Overall: I prefer the second form, both aesthetically and for future-proofing

[camelid]

I think I agree with @CraftSpider in preferring the second form. I like the first form's simplicity, but it also seems surprising that WARNING: is treated as a 'keyword' and will change the formatting of the docs.

[GuillaumeGomez]

That's why we have this debate. 😛

[Manishearth]

I personally actually prefer stabilizing a CSS class, I think we should do more of that, just because parsing can get annoying. But this is not that hard to parse anyway.

The problem with WARNING: is precisely that you can't scope it easily: what if you want to add code blocks? bullet points?

[jyn514]

I personally actually prefer stabilizing a CSS class

The problem with this is it ties rustdoc to the HTML backend; like I said on Zulip nothing rustdoc currently does guarentees that it will generate HTML from the markdown. Changing that means that to make the JSON backend have the same info, we'd have to add a full HTML parser to rustdoc just to read the HTML in the markdown, because pulldown does not parse HTML for us. I really think we should avoid making the JSON backend a second-class citizen.

[jyn514]

Also there's not much point to this if it's just a little CSS IMO, you can add that yourself with --html-after-content and it will look just as good and be more flexible since you can control the styles yourself.

[jyn514]

I'm not a giant fan of the [[info]] syntax because it looks odd when rendered by anything other than rustdoc, which I'd rather avoid, one of the major design concerns for intra-doc links was compatibility with other markdown parsers: https://github.com/rust-lang/rfcs/blob/master/text/1946-intra-rustdoc-links.md#standard-conforming-markdown

[Manishearth]

The problem with this is it ties rustdoc to the HTML backend

So, I agree, but I'm not convinced that the warning class is something that needs to be surfaced in the JSON backend as anything special.

Also, bear in mind, HTML is a part of markdown: valid HTML (well a subset of HTML) is valid markdown. Users already can <div style=foo>, and they do that in many cases.

Markdown parsers already need to be able to handle HTML. There's nothing new there. Whatever consumes the JSON backend will need to be able to handle that too.

Also there's not much point to this if it's just a little CSS IMO, you can add that yourself with --html-after-content and it will look just as good and be more flexible since you can control the styles yourself.

I mean, yes, in theory, in practice people don't do this because of the barrier to entry (you have to get your tools to use that, including docs.rs)

[Manishearth]

because pulldown does not parse HTML for us

Also, this is incorrect, pulldown absolutely parses HTML. It has to, like I said valid HTML is valid Markdown. It doesn't parse it thoroughly, I believe you can mess it up with sufficiently complex HTML, but it parses HTML.

---

What this changes is subtler: up till now you did not need any particular stylesheet to acceptably render rustdoc markdown from the JSON backend on your own (just use your favorite markdown renderer's stylesheet), but now you will need other css classes to handle rendering warnings as warnings.

[dtolnay]

The problem with WARNING: is precisely that you can't scope it easily: what if you want to add code blocks? bullet points?

I do not agree that this is a problem. I've used Phabricator's implementation of this approach happily and it is by design that warning banners are one paragraph. It is not the intention that you would put entire sections of documentation into a warning banner. They are for a brief callout. It's fine to refer to a subsequent section of the documentation from the callout if needed.

I suppose this is a matter of taste but it's one where I believe the opinionatedness of the syntax is a positive thing.

[Manishearth]

So I discussed this a bit with @jyn514 on the side, we agree that the effect on the JSON backend isn't major since HTML is already a subset of markdown and docs use HTML already (especially for images and tables).

My position is as follows:

• I strictly prefer HTML over some other scoped syntax with nuances people have to learn.
• I don't think there's as much value in unscoped syntax

which makes my position "HTML or nothing", basically. I'm not wholly against unscoped single-line syntax. We NOTE: and EXAMPLE: syntax in W3C specs and you invariably need to add more stuff to it (there's an HTML class in these specs for this reason).

Furthermore, Rust has a strong culture of wrapping text, so this means we need WARNING: to work well while wrapped, and that means we're again introducing some form of scoping mechanism (e.g. indenting the text), which editors will have to learn, as well.

[masklinn]

I don't know about admonitions specifically but there seems to be something of a community habit to use "magic tags" on fenced code blocks for this sort of features (e.g. github has a suggestion tag), maybe that could be an option? I don't know if pulldown_cmark has a hook for manipulating code blocks but…

So a warning would be

```warning
as much warning text as you want
```

or maybe the tag would be admonition:<type> to leave room for things like note/todo/danger/…

Alternatively, take a gander at how other markdown implementations extend the language for this sort of features? e.g. python-markdown seems to be using !!!<tag> for admonitions.

[camelid]

I don't know about admonitions specifically but there seems to be something of a community habit to use "magic tags" on fenced code blocks for this sort of features (e.g. github has a suggestion tag), maybe that could be an option? I don't know if pulldown_cmark has a hook for manipulating code blocks but…

Hmm, but suggestion is still for code, even if it has some extra behavior.

Aside from the design concerns, this would be feasible to implement: we just extract the language string from the code block with pulldown.

[gideongrinberg]

Could you use something that's already built into markdown? For example, rustdoc could take > :warning: Some warning here and render it as an admonition.

[GuillaumeGomez]

That's what #79710 (comment) suggested: to use markdown and make some changes on rustdoc side. I think the codeblock approach is the best personally.

[gideongrinberg]

That's what #79710 (comment) suggested: to use markdown and make some changes on rustdoc side. I think the codeblock approach is the best personally.

My bad, I didn't see that.

[jyn514]

I wonder if it makes more sense to add this as a proc-macro. Rustdoc can still provide the CSS classes and things, and you could use them with HTML directly, but you could use warning!("warning text here") or something and it would expand to the correct HTML tags and classes.

[GuillaumeGomez]

Might be better to have a doc(warning = "...") then, no?

[Manishearth]

Is that going to be a special header similar to the "unsafe"/"experimental"/etc ones, or is that going to also be used within docs in an order-sensitive way? If the latter we should probably just use a CSS class; mixing /// and #[doc] is ugly

[GuillaumeGomez]

Agreed. Among the suggested ideas, my favourite one remains the code block with "warning" as tag:

```warning
some text
```

[Manishearth]

Yeah, but triple backticks is also for preformatted text without further markup, so you won't be able to e.g. use text formatting (or other code) in there

[GuillaumeGomez]

True... Finding the right syntax is much more complicated than expected...

[masklinn]

True... Finding the right syntax is much more complicated than expected...

That's the issue of using markdown, for all that it has a pretty nice baseline syntax it really was not designed for extensibility.

[GuillaumeGomez]

We can always use <warning>: it'd allow to have text formatting inside it.

[Manishearth]

I thought I'd already left this comment, but 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.

edit: copying over the reasoning from the other thread:

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.

[coastalwhite]

@Manishearth Although, I might not agree fully, I can fully appreciate and understand the reasoning you left in the corresponding PR.

Still, I do think it might be worth to standardize some kind of syntax for some kind of warning block. This does not actually require customized markdown. Alternatively, I can suggest adding the GitHub flavor with emojis to the Rust API Guidelines documentation section. This may help in standardizing short form warnings and notes within documentation.

We could for example propose to standardize:

> ⚠️ **Warning**
> This is a warning message.

which should look like the following

⚠️ Warnlng
This is a warning message.

I do value people's opinion on this. Is this a possible compromise?

[Manishearth]

That's a valid alternative, I think the yellow color is pretty important though, and it's useful for themes to be able to make styling decisions about warnings so I do think that having some way of marking it is important.

I just don't think the way of marking it should be some magic invocation as opposed to CSS classes.

(And I think that the bar for a feature that is adding new semantics to the Rustdoc markdown parser is much, much higher than that of one stabilizing a CSS class)

[coastalwhite]

100% agree. I will look at the Rust API guidelines issue and possibly an RFC to rust to track the adoption in GitHub and possibly eventually merge.

Thank you 😄

[notriddle]

Beyond all this discussion of third-party tools, I'd really like to make sure that whatever new syntax and presentation we add to rustdoc can also be added to mdBook. That way, when the standard library starts using callouts, we can make sure the same style also gets used in The Book.

Mostly, The Book seems to just use blockquotes, often with headings like this one about ownership. mdBook doesn't do anything special with it: it's just a block quote with a header.

[GuillaumeGomez]

This is a good point. Once we have something merged here, I'll send a PR there to have an equivalent.

[notriddle]

CC @ehuss

[ehuss]

I'd be happy to add some CSS rules to mdbook if that is what you are proposing.

[notriddle]

Yeah, it is.

[toastal]

FYI, using a <blockquote> is not semantic and Microsoft GitHub is setting a bad precedence.

A block quote marker, optionally preceded by up to three spaces of
indentation, consists of (a) the character > together with a
following space of indentation, or (b) a single character > not
followed by a space of indentation.

— CommonMark Spec v0.30, https://spec.commonmark.org/0.30/#block-quotes

> denotes a <blockquote> in Markdown and is rendered as such.

The blockquote element represents a section that is quoted from
another source.

— W3C HTML spec, https://html.spec.whatwg.org/multipage/grouping-content.html#the-blockquote-element

Unless you are quoting a source, this is not the correct element to use. While I disagree with even using > altogether, at least some tools out there are at least translating the blockquote in admonition context into a <div> (div has no semantic meaning which is unfortunately the best HTML option for admonitions). It would be ill-advised to follow Microsoft GitHub’s Markdown syntax fork—especially with their disregard for semantics. Similarly using a <pre> block would also be breaking semantics as an admonition is not preformatted text.

CommonMark does have an RFC for generic directives that could be used if the <div class="warning"> solution was preferred. There’s also other reasonable options noted here that don’t break semantics.

[notriddle]

I agree that block quotes for callouts are bad, and would like to work with mdBook to roll out a common syntax (probably <div class="warning">) that will replace the ad-hoc, unsemantic markup that they use right now.

[GuillaumeGomez]

Using <div class="warning"> is what we agreed upon in https://github.com/rust-lang/rust/pull/106561/files#diff-625c2b4568f0a7b1c8233bee4e602cdd31b6d88bc06d82dbe33590bf213cd41dR265. The debate now is on the look itself.

[jhpratt]

Unfortunately .warning isn't great because markdown has a "boundary" when HTML tags are used. I wanted to add a warning indicating that users probably wanted another trait, but intra-doc links don't work (which is the correct behavior per markdown spec).

[GuillaumeGomez]

Works perfectly fine though (well, minus the UI bug I just discovered haha):

I used this code:

/// <div class="warning">
///
/// [bar]
///
/// </div>
pub fn foo() {}

pub fn bar() {}

I'll send a PR to improve documentation.

[jhpratt]

Ah. Well I suppose the issue wasn't with intra-doc links, but with inline code blocks. What I tried was that but with [`bar`], and it failed.

[GuillaumeGomez]

Works too:

[GuillaumeGomez]

I opened #119245 which adds extra explanation on how to use markdown inside html tags. That should solve your issue.

[jhpratt]

That note is exactly what I was just looking in to. I had it inline, given my familiarity with HTML. TIL that matters.

[GuillaumeGomez]

It doesn't hurt to have it and if it can helps users, well, why not doing it. :)