Highlight identifier after 'impl' keyword in Rust language definition

[RayOffiah]

Hi! 👋

Firstly, thanks for your work on this project! 🙂

Today I used patch-package to patch highlight.js@9.18.3 for the project I'm working on.

It looks as if the identifier after the impl keyword is not being highlighted. I've added an extra element to the class definition to include impl.

Here is the diff that solved my problem:

diff --git a/node_modules/highlight.js/lib/languages/rust.js b/node_modules/highlight.js/lib/languages/rust.js
index 5e2af44..590dc15 100644
--- a/node_modules/highlight.js/lib/languages/rust.js
+++ b/node_modules/highlight.js/lib/languages/rust.js
@@ -89,7 +89,7 @@ module.exports = function(hljs) {
       },
       {
         className: 'class',
-        beginKeywords: 'trait enum struct union', end: '{',
+        beginKeywords: 'trait enum struct union impl', end: '{',
         contains: [
           hljs.inherit(hljs.UNDERSCORE_TITLE_MODE, {endsParent: true})
         ],

This issue body was partially generated by patch-package.

[RunDevelopment]

impl in what context? Did you mean fn foo() -> impl Trait {}, fn foo(a: impl Trait) {}, impl Foo {}, or impl Trait for Foo {}?

Because if you intended to cover the impl Foo {} and impl Trait for Foo {} cases as well, you also have to consider generic parameters, e.g. impl<T: Clone, A: Allocator> Vec<T, A> {} and impl<T, A: Allocator> ops::Deref for Vec<T, A>.

Also, if impl (as in fn foo() -> impl Trait {} and fn foo(a: impl Trait) {}) is supported, dyn should be as well.

Also, if impl Trait for Foo is supported, then for Foo should probably be supported too. But for Foo has some syntactic ambiguity with regular for loops, so that might be a little tricky.

[RayOffiah]

Right, I see. I was just looking to cover

impl <identifier> {

}

which is just the bit we needed at the moment. I'd need to delve a bit ( a lot!) further into highlight definitions to match the others (especially the generics stuff).

[joshgoebel]

Thanks for chiming in @RunDevelopment

impl in what context?

Is there some reason these contexts should be highlighted differently?

you also have to consider generic parameters

We're happy here if someone implemented impl today and someone else looked into generics tomorrow. It's not work that all needs to be done at once - as long as each individual step is a clean patch - and the grammar overall isn't broken at any point. IE, we can't break for along the way as you mention.

I'd guess generics probably also works with classes, yet we happily ignore that at the moment.

impl Trait for Foo {} cases

I think the new multi-class matching stuff might help with this.

[RunDevelopment]

Is there some reason these contexts should be highlighted differently?

Not at all. I was asking to get at dyn and for Foo. It would be a little strange if impl Trait was supported but dyn Trait wasn't; similar for impl Trait for Foo {}.

[joshgoebel]

Isn't for just a keyword in all cases? I'm not sure any special handling would be necessary unless the desire was to color whatever came after it as a type, irregardless if it was a recognized type or not?

Until we have a language champion it's pretty common for some grammars to grow organically as people find and fix issues with them. There is no "master plan". If you'd like to step up and help us out with Rust that'd be pretty awesome. :)

[RunDevelopment]

Isn't for just a keyword in all cases?

Yes, it is. Sorry for being clear here. I meant that it would strange that in impl Trait for Foo {} only Trait would be highlighted as a class name even though Foo obviously is one as well.

[joshgoebel]

@RayAtRadix What is your involvement in Rust? It looks like they are using us for highlighting their own docs but I don't think they are using the built-in Rust grammar... I wonder if you know who we might talk to see what changes they've made and if we could possibly get those in the official library here?

[RayOffiah]

@RayAtRadix What is your involvement in Rust? It looks like they are using us for highlighting their own docs but I don't think they are using the built-in Rust grammar... I wonder if you know who we might talk to see what changes they've made and if we could possibly get those in the official library here?

Hi Josh.

I'm just experimenting with Rust at the moment, and building example pages of documentation as I'm going along, so I'm by no means a Rust expert unfortunately.

Yes, we are using the built-in Rust grammar, but I needed the impl identifier working for our documentation pages. The diff I sent is the only change I've made so far. If it helps then feel free to use it :-)

[joshgoebel]

I've made a lot of other changes a cleanups in the simpler struct PR: 9d6befc