Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Alternative highlighting for Rust code block in rustdoc #78917

Open
Hywan opened this issue Nov 10, 2020 · 25 comments
Open

Alternative highlighting for Rust code block in rustdoc #78917

Hywan opened this issue Nov 10, 2020 · 25 comments
Assignees
Labels
A-markdown-parsing Area: Markdown parsing for doc-comments C-feature-request Category: A feature request, i.e: not implemented / a PR. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@Hywan
Copy link
Contributor

Hywan commented Nov 10, 2020

Context https://twitter.com/imperioworld_/status/1325916530257833984 cc @GuillaumeGomez

Description

I'm the author of https://github.com/Hywan/inline-c-rs/. It allows to write C code inside Rust. It's a super basic proc_macro that transforms the code into a string (with some extra manipulations), which is saved inside a temp file that is compiled by a C/C++ compiler, and finally run. That's useful for testing C API of a Rust program inside Rust with the Rust tooling (like cargo test). It is super useful for us at https://github.com/wasmerio/wasmer.

Recently, we pushed the concept further: Using inline-c-rs inside documentation, like this:

/// Blah blah blah.
///
/// # Examples
///
/// ```rust
/// # use inline_c::assert_c;
/// # fn main() {
/// #    (assert_c! {
/// # #include "tests/wasmer_wasm.h"
/// #
/// int main() {
///     wasm_engine_t* engine = wasm_engine_new();
///     wasm_store_t* store = wasm_store_new(engine);
///
///     // etc.
///    
///     return 0;
/// }
/// #    })
/// #    .success();
/// # }
/// ```
#[no_mangle]
pub unsafe extern "C" fn wasm_module_new(/* skipped */) -> /* skipped */ {
    // skipped
}

This documentation is:

  • ✅ written in Rust, and contains C,
  • ✅ is tested by cargo test --doc because the code block is tagged with ```rust … so we can test C with cargo test 🤪!
  • ✅ is compiled to HTML by rustdoc.

The result of cargo doc looks like this:

Screenshot 2020-11-06 at 11 40 44

That's excellent! All the “Rust decoration” is “removed” (thanks to # …), and only the C code stays. That's insane I know, but it works and that's super fun.

One problem though:

  • ❌ The highlighting is incorrect.

Because it's a ```rust code block, there is a special handler for that I assume. I tried to write rust,language-c but the `language-c` part is ignored and is not present in the generated HTML code, as it can be seen here:

<pre class="rust rust-example-rendered">

Expectation

I would expect ```rust to be a special keyword that unfolds to rust + `language-rust`:

  • The former describes that it's a Rust code block, and consequently is a candidate for a test,
  • The second describes the syntax highlighting ruleset to apply for the HTML documentation.

This keyword could have a different meaning in the presence of language-<lang_id> which “cancels” language-rust.

In other words, writing rust,language-c would keep the actual behavior of rust but it will disable the default highlighting to allow the user to define another one.

How does it sound?

Motivation

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.

@GuillaumeGomez GuillaumeGomez added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Nov 10, 2020
@GuillaumeGomez
Copy link
Member

So I thought a lot about this after our discussion, and I reached the following conclusion: people might want to give special classes to some code blocks without following a pattern. Therefore, I had the following idea: adding a new code "tag" (like rust, ignore, etc) which would look like class-some-class-name and that would generate the block with it (but without the prepending class-). So it would give <code class="some-class-name">.

What do you think? Like that, you can support highlight.js using class-language-C or any other syntax highlighter (or for some other use as well).

@Hywan
Copy link
Contributor Author

Hywan commented Nov 10, 2020

That's a good idea yes!

We should also disable the Rust “HTML highlighting” (lexing + creating the HTML spans around the tokens). Note that this last part is optional: If we have a @class attribute, we can run Javascript to collect the text only, remove all the HTML tags, and do something with that.

As I understand the workflow, there is one step to remove the # <line>s, and one step that tokenize + create the HTML spans, is that correct? Then the first step must be kept, and the second should be skipped.
How to take that decision? I don't know for the moment. Maybe it's not a good idea to go that path.

@GuillaumeGomez
Copy link
Member

I just assumed that if the class-whatever tag was used, we were disabling default syntax highlighting, but you did well to bring it up, at least it makes it clear this way.

As for removing the # <line>, we still want to "keep it" (understanding, remove the lines from the display).

As for the final decision, I guess we just need someone else from the @rust-lang/rustdoc team to approve what the final feature should looks like. And at first, it'll only be available on nightly version for a while until we're sure the feature is good to be stable.

@Hywan
Copy link
Contributor Author

Hywan commented Nov 10, 2020

Of course! That's fine for me to get it on nightly for a moment. Feel free to ping me if you need anyhelp.

@Manishearth
Copy link
Member

custom class names seems fine. I don't want this feature to get too complicated though

@jyn514
Copy link
Member

jyn514 commented Nov 10, 2020

Instead of adding a new syntax, could we treat rust,c as 'rust for the purposes of tests, but with C highlighting'?

@jyn514 jyn514 added C-enhancement Category: An issue proposing an enhancement or a PR with one. C-feature-request Category: A feature request, i.e: not implemented / a PR. and removed C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Nov 10, 2020
@camelid
Copy link
Member

camelid commented Nov 10, 2020

rust,c seems unclear to me. What about rust,syntax-c or something like that, to make it clear that it's just for syntax highlighting?

@GuillaumeGomez
Copy link
Member

That's why I suggested what I did which allows to add whatever class you want to the code block. Like that, it's not necessarily about syntax highlighting but about whatever you want and it's pretty simple to handle for rustdoc.

Instead of adding a new syntax, could we treat rust,c as 'rust for the purposes of tests, but with C highlighting'?

I don't think this is a good idea because it'll mean adding support C syntax. And no one wants that. Also, what happens if other people wants more languages? The simplest way is to just allow to generate classes and let people do whatever they want with it I think.

@camelid
Copy link
Member

camelid commented Nov 10, 2020

it'll mean adding support C syntax. And no one wants that.

I wouldn't say that :)

I'm sure there are a lot of people that would like to have C syntax highlighting in their docs for their FFI code.

@GuillaumeGomez
Copy link
Member

I didn't mean it this way. More like "no one wants to add the C syntax highlighting in rustdoc directly". Even myself wants to have C support. :p

@estebank
Copy link
Contributor

So I thought a lot about this after our discussion, and I reached the following conclusion: people might want to give special classes to some code blocks without following a pattern. Therefore, I had the following idea: adding a new code "tag" (like rust, ignore, etc) which would look like class-some-class-name and that would generate the block with it (but without the prepending class-). So it would give <code class="some-class-name">.

@GuillaumeGomez I like this idea as it's something that can have multiple uses. I was thinking we could try and do something slightly less magical than plain prefixes: we could have a colon syntax that would look like rust,class:clang and would also allow us to eventually add other things to the html code node (id, data- attrs, etc) for further customization.

Removing the rust syntax highlighting seems like it might be more challenging to do in a neat way, but maybe we can just have another modifier like rust,plain,class:clang for that.

@GuillaumeGomez
Copy link
Member

Oh, I like the syntax with the ":"!

As for removing the rust syntax highlighting, I just assumed that if you used "class:*", we wouldn't run it.

@Hywan
Copy link
Contributor Author

Hywan commented Nov 11, 2020

Actually, using plain makes things less magical, which is good. It's a little bit more verbose though.

On the other side, one may argue that everytime a user will add a class, it's very likely to customize the CSS style, so… it may imply plain.

Anyway, if plain is not a feature, i.e. if class:<class-names> disables the Rust highlighting, a new feature must be provided to re-enable the Rust default highlighting in case that's not the expected behavior for the user.

@GuillaumeGomez
Copy link
Member

I wonder: in which cases one could want to add a custom class but still want the rust syntax highlighting?

@Hywan
Copy link
Contributor Author

Hywan commented Nov 11, 2020

I've no idea, but if there is one way to disable a feature, there should be a way to re-enable it.

Also, I like the <attr>:<value> notation, for instance to add lines highlighting with prism.js, such as data-line:1-2 -> @data-line="1-2".

Be sure to allow : in the value part too.

@GuillaumeGomez
Copy link
Member

Well, we don't split on ":" so the goal is to keep everything following "class:". As for re-enabling, I don't see the point for the moment so unless someone brings up a need for it (that's why we don't want to make it stable right away!), I think it'd be better not to add it.

@jyn514 jyn514 added the A-markdown-parsing Area: Markdown parsing for doc-comments label Nov 12, 2020
@Hywan
Copy link
Contributor Author

Hywan commented Nov 12, 2020

Both options are good to me :-).

What about extending the idea of class:<class-names> to <attr-name>:<attr-value> in general?

@GuillaumeGomez
Copy link
Member

Let's say I'm "open" to the idea. But as always, I need a case for it to be useful otherwise I think it's better to focus on one thing only. After all, it's only about code blocks.

@Hywan
Copy link
Contributor Author

Hywan commented Nov 12, 2020

I've presented a usecase in #78917 (comment) with prism.js.

However, I agree we should focus on one feature at a time.

@GuillaumeGomez
Copy link
Member

It can be included as a class (or multiple ones hehe). 😛

@Hywan
Copy link
Contributor Author

Hywan commented Nov 12, 2020

It isn't the API of Prism.js, but let's keep class:<class-names> for the moment :-)! It'll be a really awesome new feature :-).

@Manishearth
Copy link
Member

Instead of adding a new syntax, could we treat rust,c as 'rust for the purposes of tests, but with C highlighting'?

There are tons of PLs out there, and we shouldn't lock ourselves out of future modifiers.

I think class:classname is good.

@poliorcetics
Copy link
Contributor

poliorcetics commented Nov 26, 2020

@rustbot claim

Edit: I have something working locally, I'll clean it up a little and make a PR soon. 😄

@Hywan
Copy link
Contributor Author

Hywan commented Jul 6, 2021

Hey :-). Any news on that front? #79454 has been closed due to inactivity. Anyone willing to take it? Can I take it?

@jyn514
Copy link
Member

jyn514 commented Jul 6, 2021

@Hywan sure thing, feel free to open a PR :)

bors added a commit to rust-lang-ci/rust that referenced this issue Sep 15, 2023
…n_docs, r=t-rustdoc

Accept additional user-defined syntax classes in fenced code blocks

Part of rust-lang#79483.

This is a re-opening of rust-lang#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: rust-lang#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:

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

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

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

Instead of:

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

```rust
/// ```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](https://pandoc.org/MANUAL.html#extension-fenced_code_attributes). 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](rust-lang#110800 (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](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/pre) 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`
bors added a commit to rust-lang-ci/rust that referenced this issue Sep 16, 2023
…n_docs, r=t-rustdoc

Accept additional user-defined syntax classes in fenced code blocks

Part of rust-lang#79483.

This is a re-opening of rust-lang#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: rust-lang#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:

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

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

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

Instead of:

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

```rust
/// ```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](https://pandoc.org/MANUAL.html#extension-fenced_code_attributes). 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](rust-lang#110800 (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](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/pre) 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`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-markdown-parsing Area: Markdown parsing for doc-comments C-feature-request Category: A feature request, i.e: not implemented / a PR. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants