On 08.12.2022 12:31, Jostein Kjønigsen wrote:
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
|