On 08.12.2022 12:12, Theodor Thornhill wrote:

Jostein Kjønigsen <jostein@secure.kjonigsen.net> writes:

On 08.12.2022 11:12, Theodor Thornhill wrote:

Jostein Kjønigsen <jostein@secure.kjonigsen.net> writes:

When I use the new csharp-ts-mode, method fontification is usually accurate with only 1 exception which I have
encountered so far:

When calling methods on objects, and that method accepts a generic type-argument. You typically see this in
Startup.cs-like files in ASP.Net Core projects:

services.AddSomeExtensionWithoutTypeArguments();
services.AddSomeExtensionWithTypeArguments<MyType>();

In the above cases we see that fontification of "services" differs.

For the first line, services is fontified using font-lock-variable-name-face (correct), but in the latter case services
is fontified using font-lock-function-name-face (incorrect).

In both cases I expected services to be fontified using font-lock-variable-name-face.

Can you test this patch, Jostein, and if you're happy, please install,
Yuan :-)

I beat you by 3 minutes, but I'll be a gentleman and test none the less :D

You test mine, and we can see which one we prefer?

Sure!  Both seems to work from what I can tell :-)  I'll let you be the
judge!

Theo

Your patch solves the issue described in the bug, but does not handle another fontification error I discovered while testing my patch:

SimpleGenericMethod<Type>(params);

In the above example SimpleGenericMethod is fontified using font-lock-type-face instead of font-lock-function-name-face. My patch fixes that case as well.

As for which patch to choose:

• From an objective perspective, the way I understand the code, your patch overrides an existing fontification to apply variable-name instead.
  
• My patch however changes some selectors to be more specific selectors to avoid fontifying the variable-identifier, and also creates a new, highly-specific selector to fontify the variable-name aspect as well.
  

From a performance perspective, I would assume the latter approach is more performant, but I don't know enough tree-sitter internals to say that with 100% confidence.

Does anyone else know?

--
Jostein

Hey guys.

This patch seems to have been left out, or slightly forgotten.

Yuan, I think in this case we are going to prefer my patch over Theo's since it fixes 2 issues, instead of just 1.

Could you install this? :)

--
Jostein