Accept additional user-defined syntax classes in fenced code blocks by GuillaumeGomez · Pull Request #110800 · rust-lang/rust

GuillaumeGomez · 2023-04-25T14:46:57Z

Part of #79483.

This is a re-opening of #79454 after a big update/cleanup. I also converted the syntax to pandoc as suggested by @notriddle: the idea is to be as compatible as possible with the existing instead of having our own syntax.

Motivation

From the original issue: #78917

The technique used by inline-c-rs can be ported to other languages. It's just super fun to see C code inside Rust documentation that is also tested by cargo doc. I'm sure this technique can be used by other languages in the future.

Having custom CSS classes for syntax highlighting will allow tools like highlight.js to be used in order to provide highlighting for languages other than Rust while not increasing technical burden on rustdoc.

What is the feature about?

In short, this PR changes two things, both related to codeblocks in doc comments in Rust documentation:

Allow to disable generation of language-* CSS classes with the custom attribute.
Add your own CSS classes to a code block so that you can use other tools to highlight them.

The `custom` attribute

Let's start with the new custom attribute: it will disable the generation of the language-* CSS class on the generated HTML code block. For example:

/// ```custom,c
/// int main(void) {
///     return 0;
/// }
/// ```

The generated HTML code block will not have class="language-c" because the custom attribute has been set. The custom attribute becomes especially useful with the other thing added by this feature: adding your own CSS classes.

Adding your own CSS classes

The second part of this feature is to allow users to add CSS classes themselves so that they can then add a JS library which will do it (like highlight.js or prism.js), allowing to support highlighting for other languages than Rust without increasing burden on rustdoc. To disable the automatic language-* CSS class generation, you need to use the custom attribute as well.

This allow users to write the following:

/// Some code block with `{class=language-c}` as the language string.
///
/// ```custom,{class=language-c}
/// int main(void) {
///     return 0;
/// }
/// ```
fn main() {}

This will notably produce the following HTML:

<pre class="language-c">
int main(void) {
    return 0;
}</pre>

Instead of:

<pre class="rust rust-example-rendered">
<span class="ident">int</span> <span class="ident">main</span>(<span class="ident">void</span>) {
    <span class="kw">return</span> <span class="number">0</span>;
}
</pre>

To be noted, we could have written {.language-c} to achieve the same result. . and class= have the same effect.

One last syntax point: content between parens ((like this)) is now considered as comment and is not taken into account at all.

In addition to this, I added an unknown field into LangString (the parsed code block "attribute") because of cases like this:

/// ```custom,class:language-c
/// main;
/// ```
pub fn foo() {}

Without this unknown field, it would generate in the DOM: <pre class="language-class:language-c language-c">, which is quite bad. So instead, it now stores all unknown tags into the unknown field and use the first one as "language". So in this case, since there is no unknown tag, it'll simply generate <pre class="language-c">. I added tests to cover this.

Finally, I added a parser for the codeblock attributes to make it much easier to maintain. It'll be pretty easy to extend.

As to why this syntax for adding attributes was picked: it's Pandoc's syntax. Even if it seems clunkier in some cases, it's extensible, and most third-party Markdown renderers are smart enough to ignore Pandoc's brace-delimited attributes (from this comment).

Raised concerns

It's not obvious when the `language-*` attribute generation will be added or not.

It is added by default. If you want to disable it, you will need to use the custom attribute.

Why not using HTML in markdown directly then?

Code examples in most languages are likely to contain <, >, & and " characters. These characters require escaping when written inside the <pre> element. Using the ``` code blocks allows rustdoc to take care of escaping, which means doc authors can paste code samples directly without manually converting them to HTML.

cc @poliorcetics
r? @notriddle

GuillaumeGomez · 2023-04-25T15:37:07Z

Fixed the feature version errors.

notriddle · 2023-04-25T15:58:11Z

I'm surprised that it's not aligning the syntax more closely with RFC 3311.

Last time, on that RFC, I did a Babelmark run to try to design a syntax that would produce reasonable results in most Markdown parsers. The important thing is that this syntax should gracefully degrade in mdBook, docs.rs, crates.io, GitHub, and other widely-used Markdown renderers, so that people can use #[doc=include!("README.md")] and easily write documentation that works in both Rustdoc and in whatever other renderer they're using.

p.s. I still think we should align Rustdoc's handling of code strings as closely as possible with Pandoc's. Even if it seems clunkier in some cases, it's extensible, and most third-party Markdown renderers are smart enough to ignore Pandoc's brace-delimited attributes.

notriddle

Babelmark run for the proposed syntax shows that this syntax behaves badly in other markdown parsers.

GuillaumeGomez · 2023-04-25T22:07:02Z

I'm surprised that it's not aligning the syntax more closely with RFC 3311.

I wasn't aware of it. Using = sounds good to me as well.

About following pandoc, it depends on one big thing: do we want to allow code-blocks to have user-defined IDs? Also, as you mentioned, it's a bit clunkier. class= is easier to read and doesn't require to implement a new parser for it. And finally, do we want it to be extensible?

EDIT: In any case, both solutions sound good to me. But I really find the pandoc one ugly. ^^'

EDIT2: Thinking about it some more, using an existing syntax would actually be wiser so I think going for the pandoc one is the right move here. I'll update the PR for it and specifically reject id for the time being.

Note for myself: should only allow for class=something and .something. Any other attribute will be ignored.

notriddle · 2023-04-25T23:08:40Z

And finally, do we want it to be extensible?

We've already got two features that want key-value pairs here (RFC 3311, and this one). It seems pretty arbitrary to think there won't be more.

In any case, both solutions sound good to me. But I really find the pandoc one ugly.

I know their documentation always shows key=value pairs with quoted values, but you don't actually need to quote them. Here's an example where rust {class=test} does what you'd expect.

GuillaumeGomez · 2023-04-26T15:30:47Z

I switched to the pandoc syntax as suggested and created a new parser for the markdown codeblock attributes. Any attribute which is not a class (so either . or class=) will emit a warning inside "attribute block" (ie {}).

notriddle · 2023-04-26T15:22:14Z

src/librustdoc/passes/check_custom_code_classes.rs

Needs updated.

src/librustdoc/html/markdown.rs

src/doc/rustdoc/src/unstable-features.md

GuillaumeGomez · 2023-04-27T14:45:46Z

I added the suggested tests, I updated the documentation and I added the support for double quotes (which is much more permissive than pandoc's).

notriddle · 2023-04-27T17:46:07Z

I'd like to create an eBNF to describe this grammar.

There's a few aspects of it, like the way quotes and dots are handled, that make writing one difficult. Would you be okay with making this syntax a lot stricter in order to make it easier to write a spec?

GuillaumeGomez · 2023-04-27T17:56:37Z

I'm open to it. The current handling could have an eBNF as well I think. But you seem like you have an idea for a syntax so don't hesitate to suggest an eBNF. :)

notriddle · 2023-04-27T23:03:24Z

@GuillaumeGomez

Alright, I've put together an eBNF that seems to "fit the spirit of this change," by passing many of the test cases and generally acting "reasonable." It, and a combine model of it, can be found at https://gitlab.com/notriddle/fuzz-fenced-code-blocks.

notriddle · 2023-04-27T23:52:12Z

Some noticeable differences between the two of them:

It doesn't allow opening quotes in the middle of tokens. {key=v"alue"} is invalid
It doesn't allow barewords to contain most ASCII punctuation. You have to quote them. For example, .foo=bar is a particularly nasty and ambiguous parse, which is solved by rejecting it as invalid

Some notable similarities:

It allows keys in "key"=value attributes to be quoted
It allows lang tokens and dotted classes to be quoted, like "rust" {."myClass"}
It allows multiple attribute lists, and they can be reordered, like {.myClass} rust {class=myClass2}
Commas are allowed as separators

All of this stuff should be changeable, if any of it is a problem (though none of it seems too onerous to me).

bors · 2023-04-30T04:43:13Z

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

src/librustdoc/html/markdown.rs

bors · 2023-09-16T09:55:52Z

📌 Commit e39c393 has been approved by t-rustdoc

It is now in the queue for this repository.

bors · 2023-09-16T13:10:13Z

⌛ Testing commit e39c393 with merge 41bafc4...

bors · 2023-09-16T14:55:40Z

☀️ Test successful - checks-actions
Approved by: t-rustdoc
Pushing 41bafc4 to master...

rust-timer · 2023-09-16T16:18:04Z

Finished benchmarking commit (41bafc4): comparison URL.

Overall result: ❌ regressions - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.7%	[0.5%, 0.9%]	3
Regressions ❌ (secondary)	0.9%	[0.5%, 1.4%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.7%	[0.5%, 0.9%]	3

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)	-	-	0
Improvements ✅ (primary)	-0.5%	[-0.5%, -0.5%]	1
Improvements ✅ (secondary)	-1.2%	[-1.2%, -1.2%]	1
All ❌✅ (primary)	-0.5%	[-0.5%, -0.5%]	1

Cycles

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)	2.1%	[2.0%, 2.1%]	2
Regressions ❌ (secondary)	2.8%	[2.7%, 3.0%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	2.1%	[2.0%, 2.1%]	2

Binary size

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

Bootstrap: 632.782s -> 631.931s (-0.13%)
Artifact size: 318.18 MiB -> 318.17 MiB (-0.00%)

RalfJung · 2023-09-17T07:42:40Z

Is it possible that this caused a regression? We're seeing rustdoc errors about "custom classes" on nightly Miri CI since this morning. See Zulip.

GuillaumeGomez · 2023-09-17T10:20:26Z

Yes, it should emit a warning and not an error. I'll fix this right away.

…ses_in_docs-into-warning, r=Manishearth Turn custom code classes in docs into warning By habit, since it was a new feature gate, I added a check which emitted an error in case the new syntax was used. However, since rustdoc tags parser was accepting *everything*, using the "new" syntax should never ever emit errors. It now emits a warning. Follow-up of rust-lang#110800. cc `@Manishearth` r? `@notriddle`

286: Bugfix for text codeblock in documentation. r=cuviper a=robamu Fixes #285 . From what I have seen, the local output generated with `cargo +nightly doc --open` (rustc v1.74.0 nightly 203c57dbe) does not look differently than the most current one found here: https://docs.rs/num-traits/latest/num_traits/identities/trait.One.html Propably related to rust-lang/rust#110800 ? If it is, then the issue might be fixed soon and this PR should be ignored.. Co-authored-by: Robin Mueller <robin.mueller.m@gmail.com>

…_in_docs-warning, r=notriddle Custom code classes in docs warning Fixes rust-lang#115938. This PR does two things: 1. Unless the `custom_code_classes_in_docs` feature is enabled, it will use the old codeblock tag parser. 2. If there is a codeblock tag that starts with a `.`, it will emit a behaviour change warning. Hopefully this is the last missing part for this feature until stabilization. Follow-up of rust-lang#110800. r? `@notriddle`

Rollup merge of rust-lang#115947 - GuillaumeGomez:custom_code_classes_in_docs-warning, r=notriddle Custom code classes in docs warning Fixes rust-lang#115938. This PR does two things: 1. Unless the `custom_code_classes_in_docs` feature is enabled, it will use the old codeblock tag parser. 2. If there is a codeblock tag that starts with a `.`, it will emit a behaviour change warning. Hopefully this is the last missing part for this feature until stabilization. Follow-up of rust-lang#110800. r? `@notriddle`

rustbot assigned notriddle Apr 25, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Apr 25, 2023

This comment has been minimized.

Sign in to view

GuillaumeGomez force-pushed the custom_code_classes_in_docs branch from 91757a8 to 69572ff Compare April 25, 2023 14:54

This comment has been minimized.

Sign in to view

GuillaumeGomez force-pushed the custom_code_classes_in_docs branch from 69572ff to 56bd628 Compare April 25, 2023 15:02

notriddle requested changes Apr 25, 2023

View reviewed changes

GuillaumeGomez force-pushed the custom_code_classes_in_docs branch from 56bd628 to 03dd1a2 Compare April 26, 2023 14:50

notriddle reviewed Apr 26, 2023

View reviewed changes

GuillaumeGomez force-pushed the custom_code_classes_in_docs branch from 03dd1a2 to 0763193 Compare April 27, 2023 13:12

notriddle 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 28, 2023

GuillaumeGomez force-pushed the custom_code_classes_in_docs branch 2 times, most recently from 0a6a99c to ed4a959 Compare May 2, 2023 20:07

This comment has been minimized.

Sign in to view

notriddle reviewed May 2, 2023

View reviewed changes

src/librustdoc/html/markdown.rs Outdated Show resolved Hide resolved

GuillaumeGomez force-pushed the custom_code_classes_in_docs branch from ed4a959 to 6cdddd9 Compare May 2, 2023 20:54

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 Sep 16, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 16, 2023

bors merged commit 41bafc4 into rust-lang:master Sep 16, 2023

rustbot added this to the 1.74.0 milestone Sep 16, 2023

bors mentioned this pull request Sep 16, 2023

add no_compile doctest attribute #96573

Closed

GuillaumeGomez deleted the custom_code_classes_in_docs branch September 16, 2023 16:02

rustbot added the perf-regression Performance regression. label Sep 16, 2023

This was referenced Sep 17, 2023

Turn custom code classes in docs into warning #115914

Merged

Tracking Issue for user-provided classnames in fenced code blocks #79483

Closed

ctz mentioned this pull request Sep 18, 2023

rework dns_names helper, remove alloc req. rustls/webpki#178

Merged

This was referenced Sep 18, 2023

Bugfix for text codeblock in documentation. rust-num/num-traits#286

Merged

cargo doc fails if they run it with nightly Rust rust-num/num-traits#285

Closed

cpu mentioned this pull request Sep 18, 2023

Nightly warning for codeblock languages with non-alphanumeric characters #115938

Closed

GuillaumeGomez mentioned this pull request Sep 18, 2023

Custom code classes in docs warning #115947

Merged

Mark-Simulacrum added the perf-regression-triaged The performance regression has been triaged. label Sep 19, 2023

matthiaskrgr mentioned this pull request Dec 6, 2023

Rollup of 7 pull requests #118679

Merged

notriddle mentioned this pull request Apr 30, 2024

RFC: Add descriptive names to doctests rust-lang/rfcs#3311

Closed

GuillaumeGomez mentioned this pull request May 3, 2024

Stabilize custom_code_classes_in_docs feature #124577

Merged

Uh oh!

Conversation

GuillaumeGomez commented Apr 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

What is the feature about?

The custom attribute

Adding your own CSS classes

Raised concerns

It's not obvious when the language-* attribute generation will be added or not.

Why not using HTML in markdown directly then?

Uh oh!

This comment has been minimized.

This comment has been minimized.

GuillaumeGomez commented Apr 25, 2023

Uh oh!

notriddle commented Apr 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

notriddle left a comment

Choose a reason for hiding this comment

Uh oh!

GuillaumeGomez commented Apr 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

notriddle commented Apr 25, 2023

Uh oh!

GuillaumeGomez commented Apr 26, 2023

Uh oh!

notriddle Apr 26, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

GuillaumeGomez commented Apr 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

notriddle commented Apr 27, 2023

Uh oh!

GuillaumeGomez commented Apr 27, 2023

Uh oh!

notriddle commented Apr 27, 2023

Uh oh!

notriddle commented Apr 27, 2023

Uh oh!

bors commented Apr 30, 2023

Uh oh!

This comment has been minimized.

Uh oh!

bors commented Sep 16, 2023

Uh oh!

bors commented Sep 16, 2023

Uh oh!

bors commented Sep 16, 2023

Uh oh!

rust-timer commented Sep 16, 2023

Overall result: ❌ regressions - ACTION NEEDED

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Uh oh!

RalfJung commented Sep 17, 2023

Uh oh!

GuillaumeGomez commented Sep 17, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

15 participants

GuillaumeGomez commented Apr 25, 2023 •

edited

Loading

The `custom` attribute

It's not obvious when the `language-*` attribute generation will be added or not.

notriddle commented Apr 25, 2023 •

edited

Loading

GuillaumeGomez commented Apr 25, 2023 •

edited

Loading

GuillaumeGomez commented Apr 27, 2023 •

edited

Loading