[parent not found: <handler.59897.B.167049179324261.ack@debbugs.gnu.org>]
* bug#59897: Acknowledgement (29.0.60; csharp-ts-mode: variable-name fontified as method when invoking method with generic type-argument.)
[not found] ` <handler.59897.B.167049179324261.ack@debbugs.gnu.org>
@ 2022-12-08 10:09 ` Jostein Kjønigsen
0 siblings, 0 replies; 17+ messages in thread
From: Jostein Kjønigsen @ 2022-12-08 10:09 UTC (permalink / raw)
To: 59897, Yuan Fu, Theodor Thornhill
[-- Attachment #1.1: Type: text/plain, Size: 819 bytes --]
And here's the patch to fix it.
*Jostein Kjønigsen*
jostein.kjønigsen.no <https://jostein.kjønigsen.no>
jostein@kjonigsen.net - jostein@gmail.com
On 08.12.2022 10:30, GNU bug Tracking System wrote:
> Thank you for filing a new bug report with debbugs.gnu.org.
>
> This is an automatically generated reply to let you know your message
> has been received.
>
> Your message is being forwarded to the package maintainers and other
> interested parties for their attention; they will reply in due course.
>
> Your message has been sent to the package maintainer(s):
> bug-gnu-emacs@gnu.org
>
> If you wish to submit further information on this problem, please
> send it to59897@debbugs.gnu.org.
>
> Please do not send mail tohelp-debbugs@gnu.org unless you wish
> to report a problem with the Bug-tracking system.
>
[-- Attachment #1.2: Type: text/html, Size: 1676 bytes --]
[-- Attachment #2: 0001-lisp-progmodes-csharp-mode.el-improve-fontification-.patch --]
[-- Type: text/x-patch, Size: 1508 bytes --]
From ab73c1ea068538e79b06012b912c02b0e961b841 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jostein=20Kj=C3=B8nigsen?= <jostein@kjonigsen.net>
Date: Thu, 8 Dec 2022 11:07:15 +0100
Subject: [PATCH] lisp/progmodes/csharp-mode.el: improve fontification in
csharp-ts-mode
Fixes highlighting of generic methods like the one below:
instance.MethodWithTypeArguments<Type>(...);
InClassMethodWithTypeArguments<Type>(...);
---
lisp/progmodes/csharp-mode.el | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/lisp/progmodes/csharp-mode.el b/lisp/progmodes/csharp-mode.el
index 82e3bc0d541..12e45272551 100644
--- a/lisp/progmodes/csharp-mode.el
+++ b/lisp/progmodes/csharp-mode.el
@@ -791,7 +791,15 @@ csharp-ts-mode--font-lock-settings
(invocation_expression
(identifier) @font-lock-function-name-face)
(invocation_expression
- (member_access_expression (identifier) @font-lock-function-name-face))
+ (member_access_expression
+ expression: (identifier) @font-lock-variable-name-face))
+ (invocation_expression
+ function: [(generic_name (identifier)) @font-lock-function-name-face
+ (generic_name (type_argument_list
+ ["<"] @font-lock-bracket-face
+ (identifier) @font-lock-type-face
+ [">"] @font-lock-bracket-face)
+ )])
(catch_declaration
((identifier) @font-lock-type-face))
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* bug#59897: 29.0.60; csharp-ts-mode: variable-name fontified as method when invoking method with generic type-argument.
2022-12-08 9:29 bug#59897: 29.0.60; csharp-ts-mode: variable-name fontified as method when invoking method with generic type-argument Jostein Kjønigsen
[not found] ` <handler.59897.B.167049179324261.ack@debbugs.gnu.org>
@ 2022-12-08 10:12 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-12-08 10:52 ` Jostein Kjønigsen
2022-12-09 21:05 ` Yuan Fu
2022-12-09 21:07 ` Yuan Fu
3 siblings, 1 reply; 17+ messages in thread
From: Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2022-12-08 10:12 UTC (permalink / raw)
To: Jostein Kjønigsen; +Cc: casouri, 59897
[-- Attachment #1: Type: text/plain, Size: 1116 bytes --]
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 :-)
BTW, I think the ruleset is getting pretty gnarly in csharp-mode, so
maybe we should consider slimming it down a little (without losing
granularity) for perf reasons down the line!
Theo
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-expression-for-generic_name-in-csharp-ts-mode.patch --]
[-- Type: text/x-diff, Size: 1441 bytes --]
From 34fa2bc67ddffca65f219644f055f75a2b073aba Mon Sep 17 00:00:00 2001
From: Theodor Thornhill <theo@thornhill.no>
Date: Thu, 8 Dec 2022 11:08:28 +0100
Subject: [PATCH] Add expression for generic_name in csharp-ts-mode
Given the below example, we want 'services' to be font-locked in
'font-lock-variable-name-face' in all cases. Previously this only
worked in the first case, and the other was font-locked as
'font-lock-function-name-face'.
namespace Foo {
void Foo() {
services.Add();
services.Add<MyType>();
}
}
* lisp/progmodes/csharp-mode.el (csharp-ts-mode--font-lock-settings):
Add new query that makes the mentioned example work.
(bug#59897)
---
lisp/progmodes/csharp-mode.el | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/lisp/progmodes/csharp-mode.el b/lisp/progmodes/csharp-mode.el
index 82e3bc0d54..bedf3f4342 100644
--- a/lisp/progmodes/csharp-mode.el
+++ b/lisp/progmodes/csharp-mode.el
@@ -784,6 +784,10 @@ csharp-ts-mode--font-lock-settings
(invocation_expression
(member_access_expression
(generic_name (identifier) @font-lock-function-name-face)))
+ (invocation_expression
+ (member_access_expression
+ expression: (identifier) @font-lock-variable-name-face
+ name: (generic_name (type_argument_list (identifier)))))
(invocation_expression
(member_access_expression
((identifier) @font-lock-variable-name-face
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* bug#59897: 29.0.60; csharp-ts-mode: variable-name fontified as method when invoking method with generic type-argument.
2022-12-08 10:12 ` bug#59897: 29.0.60; csharp-ts-mode: variable-name fontified as method when invoking method with generic type-argument Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2022-12-08 10:52 ` Jostein Kjønigsen
2022-12-08 11:12 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
0 siblings, 1 reply; 17+ messages in thread
From: Jostein Kjønigsen @ 2022-12-08 10:52 UTC (permalink / raw)
To: Theodor Thornhill, Jostein Kjønigsen; +Cc: casouri, 59897
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?
> BTW, I think the ruleset is getting pretty gnarly in csharp-mode, so
> maybe we should consider slimming it down a little (without losing
> granularity) for perf reasons down the line!
>
> Theo
It's getting somewhat intricate, agreed. It's IMO not entirely obvious
what the different rules are for or what cases they are meant to support.
In that regard, I miss our old test-suite from github's csharp-mode. It
was a nice safety-net when making changes like this.
--
Jostein
^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#59897: 29.0.60; csharp-ts-mode: variable-name fontified as method when invoking method with generic type-argument.
2022-12-08 10:52 ` Jostein Kjønigsen
@ 2022-12-08 11:12 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-12-08 11:31 ` Jostein Kjønigsen
0 siblings, 1 reply; 17+ messages in thread
From: Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2022-12-08 11:12 UTC (permalink / raw)
To: Jostein Kjønigsen, Jostein Kjønigsen; +Cc: casouri, 59897
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#59897: 29.0.60; csharp-ts-mode: variable-name fontified as method when invoking method with generic type-argument.
2022-12-08 11:12 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2022-12-08 11:31 ` Jostein Kjønigsen
2022-12-09 20:57 ` Jostein Kjønigsen
0 siblings, 1 reply; 17+ messages in thread
From: Jostein Kjønigsen @ 2022-12-08 11:31 UTC (permalink / raw)
To: Theodor Thornhill, Jostein Kjønigsen; +Cc: casouri, 59897
[-- Attachment #1: Type: text/plain, Size: 2292 bytes --]
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
[-- Attachment #2: Type: text/html, Size: 3336 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread* bug#59897: 29.0.60; csharp-ts-mode: variable-name fontified as method when invoking method with generic type-argument.
2022-12-08 11:31 ` Jostein Kjønigsen
@ 2022-12-09 20:57 ` Jostein Kjønigsen
2022-12-09 21:12 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
0 siblings, 1 reply; 17+ messages in thread
From: Jostein Kjønigsen @ 2022-12-09 20:57 UTC (permalink / raw)
To: Yuan Fu; +Cc: Theodor Thornhill, 59897
[-- Attachment #1: Type: text/plain, Size: 2650 bytes --]
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
[-- Attachment #2: Type: text/html, Size: 4055 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#59897: 29.0.60; csharp-ts-mode: variable-name fontified as method when invoking method with generic type-argument.
2022-12-09 20:57 ` Jostein Kjønigsen
@ 2022-12-09 21:12 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-12-12 7:58 ` Jostein Kjønigsen
0 siblings, 1 reply; 17+ messages in thread
From: Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2022-12-09 21:12 UTC (permalink / raw)
To: Jostein Kjønigsen, Yuan Fu; +Cc: 59897
> 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? :)
>
I concur :-)
^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#59897: 29.0.60; csharp-ts-mode: variable-name fontified as method when invoking method with generic type-argument.
2022-12-09 21:12 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2022-12-12 7:58 ` Jostein Kjønigsen
2022-12-12 22:19 ` Yuan Fu
0 siblings, 1 reply; 17+ messages in thread
From: Jostein Kjønigsen @ 2022-12-12 7:58 UTC (permalink / raw)
To: Theodor Thornhill, Yuan Fu; +Cc: 59897
On 09.12.2022 22:12, Theodor Thornhill wrote:
>> 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? :)
>>
> I concur :-)
Looking at my local emacs-29 branch it seems also this patch been
"forgotten" and not been applied.
Do you have time to get this installed, Yuan?
--
Jostein
^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#59897: 29.0.60; csharp-ts-mode: variable-name fontified as method when invoking method with generic type-argument.
2022-12-12 7:58 ` Jostein Kjønigsen
@ 2022-12-12 22:19 ` Yuan Fu
2022-12-14 18:49 ` Jostein Kjønigsen
0 siblings, 1 reply; 17+ messages in thread
From: Yuan Fu @ 2022-12-12 22:19 UTC (permalink / raw)
To: jostein; +Cc: Theodor Thornhill, 59897, 59897-done
> On Dec 11, 2022, at 11:58 PM, Jostein Kjønigsen <jostein@secure.kjonigsen.net> wrote:
>
> On 09.12.2022 22:12, Theodor Thornhill wrote:
>>> 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? :)
>>>
>> I concur :-)
>
> Looking at my local emacs-29 branch it seems also this patch been "forgotten" and not been applied.
>
> Do you have time to get this installed, Yuan?
Yes, of course, my apologies :-) Also, beautiful commit message, Jostein, I love it :-)
Yuan
^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#59897: 29.0.60; csharp-ts-mode: variable-name fontified as method when invoking method with generic type-argument.
2022-12-12 22:19 ` Yuan Fu
@ 2022-12-14 18:49 ` Jostein Kjønigsen
2022-12-14 19:43 ` Yuan Fu
0 siblings, 1 reply; 17+ messages in thread
From: Jostein Kjønigsen @ 2022-12-14 18:49 UTC (permalink / raw)
To: Yuan Fu, Eli Zaretskii; +Cc: 59897-reopen, Theodor Thornhill, 59897
On 12.12.2022 23:19, Yuan Fu wrote:
> On 09.12.2022 22:12, Theodor Thornhill wrote:
>>>> 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? :)
>>>>
>>> I concur :-)
>> Looking at my local emacs-29 branch it seems also this patch been "forgotten" and not been applied.
>>
>> Do you have time to get this installed, Yuan?
> Yes, of course, my apologies :-) Also, beautiful commit message, Jostein, I love it :-)
>
> Yuan
I see bug marked as done, but I don't see this pushed to the emacs-29
branch.
Are you sure this has been installed Yuan?
--
Jostein
^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#59897: 29.0.60; csharp-ts-mode: variable-name fontified as method when invoking method with generic type-argument.
2022-12-14 18:49 ` Jostein Kjønigsen
@ 2022-12-14 19:43 ` Yuan Fu
2022-12-14 21:45 ` Jostein Kjønigsen
0 siblings, 1 reply; 17+ messages in thread
From: Yuan Fu @ 2022-12-14 19:43 UTC (permalink / raw)
To: jostein; +Cc: 59897-reopen, Eli Zaretskii, Theodor Thornhill, 59897
> On Dec 14, 2022, at 10:49 AM, Jostein Kjønigsen <jostein@secure.kjonigsen.net> wrote:
>
> On 12.12.2022 23:19, Yuan Fu wrote:
>> On 09.12.2022 22:12, Theodor Thornhill wrote:
>>>>> 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? :)
>>>>>
>>>> I concur :-)
>>> Looking at my local emacs-29 branch it seems also this patch been "forgotten" and not been applied.
>>>
>>> Do you have time to get this installed, Yuan?
>> Yes, of course, my apologies :-) Also, beautiful commit message, Jostein, I love it :-)
>>
>> Yuan
>
> I see bug marked as done, but I don't see this pushed to the emacs-29 branch.
>
> Are you sure this has been installed Yuan?
>
> --
> Jostein
Indeed, I forgot to push my local branch. I just did that. Now you should see it, sorry for the inconvenience :-)
Yuan
^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#59897: 29.0.60; csharp-ts-mode: variable-name fontified as method when invoking method with generic type-argument.
2022-12-14 19:43 ` Yuan Fu
@ 2022-12-14 21:45 ` Jostein Kjønigsen
2022-12-14 22:34 ` Yuan Fu
0 siblings, 1 reply; 17+ messages in thread
From: Jostein Kjønigsen @ 2022-12-14 21:45 UTC (permalink / raw)
To: Yuan Fu; +Cc: 59897-reopen, Theodor Thornhill, Eli Zaretskii, jostein, 59897
[-- Attachment #1.1: Type: text/html, Size: 8632 bytes --]
[-- Attachment #1.2: favicon.ico --]
[-- Type: image/x-icon, Size: 3290 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#59897: 29.0.60; csharp-ts-mode: variable-name fontified as method when invoking method with generic type-argument.
2022-12-14 21:45 ` Jostein Kjønigsen
@ 2022-12-14 22:34 ` Yuan Fu
0 siblings, 0 replies; 17+ messages in thread
From: Yuan Fu @ 2022-12-14 22:34 UTC (permalink / raw)
To: Jostein Kjønigsen
Cc: Theodor Thornhill, Eli Zaretskii, jostein, 59897, 59897-done
> On Dec 14, 2022, at 1:45 PM, Jostein Kjønigsen <jostein@secure.kjonigsen.net> wrote:
>
> Hey Yuan.
>
> From what I can see you pushed Teos patch and not mine?
>
> https://git.savannah.gnu.org/cgit/emacs.git/commit/?h=emacs-29&id=647b6a8099f414e5a7b162efd3658e174064dfe7emacs.git - Emacs source repository
> savannah.gnu.org
> <favicon.ico>
>
>
> In the email discussion earlier we unanymously decided on that for technical reasons my patch was more appropriate for this particular bug, and it also fixed another fontification bug.
>
> What do we do now? Should we revert this commit and apply mine instead?
>
> Sorry for all the “noise”, but this gets hard to follow up for all of us when latency between input and output gets too long, and then we end up writing emails like this, instead of writing good code to improve emacs and our new major-modes.
>
> If this could get cleaned up, installed properly, confirmed -and- pushed at the same time, it would be easier for me to verify that the right patch was put in place, and then I wouldn’t nag you for weeks end for what is really just 4 lines of code ;)
Ahhhhh sorry! I reverted the old commit and applied yours, you should now see it. I’ll try to process these patches more promptly in the future.
Yuan
^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#59897: 29.0.60; csharp-ts-mode: variable-name fontified as method when invoking method with generic type-argument.
2022-12-08 9:29 bug#59897: 29.0.60; csharp-ts-mode: variable-name fontified as method when invoking method with generic type-argument Jostein Kjønigsen
[not found] ` <handler.59897.B.167049179324261.ack@debbugs.gnu.org>
2022-12-08 10:12 ` bug#59897: 29.0.60; csharp-ts-mode: variable-name fontified as method when invoking method with generic type-argument Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2022-12-09 21:05 ` Yuan Fu
2022-12-09 21:11 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-12-09 21:07 ` Yuan Fu
3 siblings, 1 reply; 17+ messages in thread
From: Yuan Fu @ 2022-12-09 21:05 UTC (permalink / raw)
To: Jostein Kjønigsen; +Cc: jostein, Theodor Thornhill, 59897
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?
>> BTW, I think the ruleset is getting pretty gnarly in csharp-mode, so
>> maybe we should consider slimming it down a little (without losing
>> granularity) for perf reasons down the line!
>>
>> Theo
>
> It's getting somewhat intricate, agreed. It's IMO not entirely obvious
> what the different rules are for or what cases they are meant to
> support.
We had a discussion on emacs-devel that aims to standardize and define
feature names, here is the list so far:
Basic tokens:
delimiter ,.; (delimit things)
operator == != || (produces a value)
bracket []{}()
misc-punctuation
constant true, false, null
number
keyword
comment (includes doc-comments)
string (includes chars and docstrings)
string-interpolation f"text {variable}"
escape-sequence "\n\t\\"
function every function identifier
variable every variable identifier
type every type identifier
property a.b <--- highlight b
key { a: b, c: d } <--- highlight a, c
error highlight parse error
Abstract features:
assignment: the LHS of an assignment (thing being assigned to), eg:
a = b <--- highlight a
a.b = c <--- highlight b
a[1] = d <--- highlight a
definition: the thing being defined, eg:
int a(int b) { <--- highlight a
return 0
}
int a; <-- highlight a
struct a { <--- highlight a
int b; <--- highlight b
}
>
> In that regard, I miss our old test-suite from github's csharp-mode.
> It was a nice safety-net when making changes like this.
A test suite is most welcome. I think we can even come up with a general
framework for testing fontification, indent, defun movement, imenu, etc.
To generate a test you would just need to open a file, verify everything
is expected, and run a command which records the current fontification
and indentation, etc, to a file. Then a test suite will just use that
file. I wonder if csharp-mode had something like this?
Yuan
^ permalink raw reply [flat|nested] 17+ messages in thread* bug#59897: 29.0.60; csharp-ts-mode: variable-name fontified as method when invoking method with generic type-argument.
2022-12-09 21:05 ` Yuan Fu
@ 2022-12-09 21:11 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
0 siblings, 0 replies; 17+ messages in thread
From: Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2022-12-09 21:11 UTC (permalink / raw)
To: Yuan Fu, Jostein Kjønigsen; +Cc: jostein, 59897
Hi!
>>
>> In that regard, I miss our old test-suite from github's csharp-mode.
>> It was a nice safety-net when making changes like this.
>
> A test suite is most welcome. I think we can even come up with a general
> framework for testing fontification, indent, defun movement, imenu, etc.
> To generate a test you would just need to open a file, verify everything
> is expected, and run a command which records the current fontification
> and indentation, etc, to a file. Then a test suite will just use that
> file. I wonder if csharp-mode had something like this?
>
Yeah we had some trickery to check the font-locking and indentation, but
it relies on external packages IIRC.
^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#59897: 29.0.60; csharp-ts-mode: variable-name fontified as method when invoking method with generic type-argument.
2022-12-08 9:29 bug#59897: 29.0.60; csharp-ts-mode: variable-name fontified as method when invoking method with generic type-argument Jostein Kjønigsen
` (2 preceding siblings ...)
2022-12-09 21:05 ` Yuan Fu
@ 2022-12-09 21:07 ` Yuan Fu
3 siblings, 0 replies; 17+ messages in thread
From: Yuan Fu @ 2022-12-09 21:07 UTC (permalink / raw)
To: Jostein Kjønigsen; +Cc: jostein, theo, 59897
> 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?
I don’t think the two approach would have significant difference. But I think
more specific pattern is definitely better in terms of code organization.
Yuan
^ permalink raw reply [flat|nested] 17+ messages in thread