GNU bug report logs - #79406
[PATCH] 31.0.50; csharp-ts-mode does not fontify variable-use consistently

Previous Next

To reply to this bug, email your comments to 79406 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Jostein Kjønigsen <jostein <at> secure.kjonigsen.net>
To: bug-gnu-emacs <at> gnu.org
Cc: Yuan Fu <casouri <at> gmail.com>
Subject: [PATCH] 31.0.50; csharp-ts-mode does not fontify variable-use
 consistently
Date: Mon, 8 Sep 2025 10:34:50 +0200

[Message part 1 (text/plain, inline)]

Hey everyone.

As I'm getting more and more accustomed to font-lock level differentiation between variable-declaration and variable-use, I spot more cases which I think should be fixed in csharp-ts-mode. I've also found some areas of fontification which has been omitted entirely.

This looks inconsistent, and I'm willing to call that a bug :)

Attached is a patch which addresses the following areas of csharp-ts-mode fontification:

- hightlight reserved keywords agressively, dont allow misuse as
  variables etc (the compiler will fail!)
- highlight lambda-valued variables as funtions
- improve semantics of function/class annotations using attributes
  (maps closer to using-something than declaring it). also: rename to
  "attribute". "property" has a different, defined meaning in C#.
- better highlight variable-use in different scenarios (function-calls,
  if-statements, assignment-expressions, property-use, anonymous object
  initializers)
- better hight property-use (anonymous object initializers)

—
Kind Regards
Jostein Kjønigsen

[0001-csharp-ts-mode-fontification-fixes.patch (application/octet-stream, attachment)]

Message #8 received at 79406 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jostein Kjønigsen <jostein <at> secure.kjonigsen.net>
Cc: 79406 <at> debbugs.gnu.org, casouri <at> gmail.com
Subject: Re: bug#79406: [PATCH] 31.0.50;
 csharp-ts-mode does not fontify variable-use consistently
Date: Mon, 08 Sep 2025 14:14:32 +0300

> Cc: Yuan Fu <casouri <at> gmail.com>
> From: Jostein Kjønigsen <jostein <at> secure.kjonigsen.net>
> Date: Mon, 8 Sep 2025 10:34:50 +0200
> 
> As I'm getting more and more accustomed to font-lock level differentiation between variable-declaration and variable-use, I spot more cases which I think should be fixed in csharp-ts-mode. I've also found some areas of fontification which has been omitted entirely.
> 
> This looks inconsistent, and I'm willing to call that a bug :)
> 
> Attached is a patch which addresses the following areas of csharp-ts-mode fontification:
> 
> - hightlight reserved keywords agressively, dont allow misuse as
>   variables etc (the compiler will fail!)
> - highlight lambda-valued variables as funtions
> - improve semantics of function/class annotations using attributes
>   (maps closer to using-something than declaring it). also: rename to
>   "attribute". "property" has a different, defined meaning in C#.
> - better highlight variable-use in different scenarios (function-calls,
>   if-statements, assignment-expressions, property-use, anonymous object
>   initializers)
> - better hight property-use (anonymous object initializers)

Are all of these appropriate for the default value of
treesit-font-lock-level?  I see that
csharp-ts-mode--font-lock-settings doesn't use the level at all -- is
that on purpose?

Message #11 received at 79406 <at> debbugs.gnu.org (full text, mbox):

From: Jostein Kjønigsen <jostein <at> secure.kjonigsen.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 79406 <at> debbugs.gnu.org, Yuan Fu <casouri <at> gmail.com>
Subject: Re: bug#79406: [PATCH] 31.0.50; csharp-ts-mode does not fontify
 variable-use consistently
Date: Mon, 8 Sep 2025 14:50:23 +0200

[Message part 1 (text/plain, inline)]

> On 8 Sep 2025, at 13:14, Eli Zaretskii <eliz <at> gnu.org> wrote:
> 
> Are all of these appropriate for the default value of
> treesit-font-lock-level?  

I'll have to be honest and admit I haven't tested any major-mode in "default" fontification level in probably a decade. My settings is maximum detail, and I've based my findings on that.

It may previously have been difficult to compartementalize some of these fontifications into different features and levels (due to extensive use of override:t and rules being heavily order-dependent).

With the changes in this patch however, I can probably move some rules about if that's wanted. Anything in particular you are thinking about?

> I see that
> csharp-ts-mode--font-lock-settings doesn't use the level at all -- is
> that on purpose?


I'm not sure I'm following. We have the following LISP to configure tree-sitter for the different feature-levels?

  (setq-local treesit-font-lock-feature-list
              '(( comment definition)
                ( keyword string type directives)
                ( constant escape-sequence expression literal attribute)
                ( function bracket delimiter error)))

Can you provide a more concrete example of what you think is missing or inappropriate and I can try to address that.

—
Kind Regards
Jostein Kjønigsen

[Message part 2 (text/html, inline)]

Message #14 received at 79406 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jostein Kjønigsen <jostein <at> secure.kjonigsen.net>
Cc: 79406 <at> debbugs.gnu.org, casouri <at> gmail.com
Subject: Re: bug#79406: [PATCH] 31.0.50; csharp-ts-mode does not fontify
 variable-use consistently
Date: Mon, 08 Sep 2025 15:59:12 +0300

> From: Jostein Kjønigsen <jostein <at> secure.kjonigsen.net>
> Date: Mon, 8 Sep 2025 14:50:23 +0200
> Cc: 79406 <at> debbugs.gnu.org,
>  Yuan Fu <casouri <at> gmail.com>
> 
>  On 8 Sep 2025, at 13:14, Eli Zaretskii <eliz <at> gnu.org> wrote:
> 
>  Are all of these appropriate for the default value of
>  treesit-font-lock-level?  
> 
> I'll have to be honest and admit I haven't tested any major-mode in "default" fontification level in probably a
> decade. My settings is maximum detail, and I've based my findings on that.
> 
> It may previously have been difficult to compartementalize some of these fontifications into different features
> and levels (due to extensive use of override:t and rules being heavily order-dependent).
> 
> With the changes in this patch however, I can probably move some rules about if that's wanted. Anything in
> particular you are thinking about?

Can you correlate the syntactic elements with the guidelines in the
doc string of treesit-font-lock-level?

>  I see that
>  csharp-ts-mode--font-lock-settings doesn't use the level at all -- is
>  that on purpose?
> 
> I'm not sure I'm following. We have the following LISP to configure tree-sitter for the different feature-levels?
> 
>   (setq-local treesit-font-lock-feature-list
>               '(( comment definition)
>                 ( keyword string type directives)
>                 ( constant escape-sequence expression literal attribute)
>                 ( function bracket delimiter error)))
> 
> Can you provide a more concrete example of what you think is missing or inappropriate and I can try to
> address that.

Hmm... maybe nothing.  Yuan, could you please see if anything is
missing here?

Message #17 received at 79406 <at> debbugs.gnu.org (full text, mbox):

From: Yuan Fu <casouri <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 79406 <at> debbugs.gnu.org,
 Jostein Kjønigsen <jostein <at> secure.kjonigsen.net>
Subject: Re: bug#79406: [PATCH] 31.0.50; csharp-ts-mode does not fontify
 variable-use consistently
Date: Tue, 9 Sep 2025 00:06:30 -0700

> On Sep 8, 2025, at 5:59 AM, Eli Zaretskii <eliz <at> gnu.org> wrote:
> 
>> From: Jostein Kjønigsen <jostein <at> secure.kjonigsen.net>
>> Date: Mon, 8 Sep 2025 14:50:23 +0200
>> Cc: 79406 <at> debbugs.gnu.org,
>> Yuan Fu <casouri <at> gmail.com>
>> 
>> On 8 Sep 2025, at 13:14, Eli Zaretskii <eliz <at> gnu.org> wrote:
>> 
>> Are all of these appropriate for the default value of
>> treesit-font-lock-level?  
>> 
>> I'll have to be honest and admit I haven't tested any major-mode in "default" fontification level in probably a
>> decade. My settings is maximum detail, and I've based my findings on that.
>> 
>> It may previously have been difficult to compartementalize some of these fontifications into different features
>> and levels (due to extensive use of override:t and rules being heavily order-dependent).
>> 
>> With the changes in this patch however, I can probably move some rules about if that's wanted. Anything in
>> particular you are thinking about?
> 
> Can you correlate the syntactic elements with the guidelines in the
> doc string of treesit-font-lock-level?
> 
>> I see that
>> csharp-ts-mode--font-lock-settings doesn't use the level at all -- is
>> that on purpose?
>> 
>> I'm not sure I'm following. We have the following LISP to configure tree-sitter for the different feature-levels?
>> 
>>  (setq-local treesit-font-lock-feature-list
>>              '(( comment definition)
>>                ( keyword string type directives)
>>                ( constant escape-sequence expression literal attribute)
>>                ( function bracket delimiter error)))
>> 
>> Can you provide a more concrete example of what you think is missing or inappropriate and I can try to
>> address that.
> 
> Hmm... maybe nothing.  Yuan, could you please see if anything is
> missing here?

Csharp-ts-mode is doing the right thing. Major modes don’t interact with treesit-font-lock-level directly. They just define font-lock rules categorized into features. And provide a feature list that’s split into levels. Emacs picks the features to enable based treesit-font-lock-level.

Most of the changes are good, I didn’t look into the rules themselves since I know Jostein is much more familiar to the grammar and C# than I do. I also like that we’re using less :override.

The only thing I’m not too sure about is the change from property to attribute. It’s technically a breaking change, I don’t think it’s “wait for 10 years to change it” level, but at least we should mention it in the NEWS. Also it seems you’re highlighting the brackets as well? Is there a particular reason to do that? And IMHO since attribute is metadata, property face is more suitable than variable-use face. But that’s subjective so it’s your call.

And there’s the small things like using full sentence for comments (period at the end) and the commit message format, those I can fix when applying the patch.

Yuan

Message #20 received at 79406 <at> debbugs.gnu.org (full text, mbox):

From: Jostein Kjønigsen <jostein <at> secure.kjonigsen.net>
To: Yuan Fu <casouri <at> gmail.com>
Cc: 79406 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#79406: [PATCH] 31.0.50; csharp-ts-mode does not fontify
 variable-use consistently
Date: Tue, 9 Sep 2025 09:46:28 +0200

[Message part 1 (text/plain, inline)]

> On 9 Sep 2025, at 09:06, Yuan Fu <casouri <at> gmail.com> wrote:
> 
> 
> 
>> On Sep 8, 2025, at 5:59 AM, Eli Zaretskii <eliz <at> gnu.org> wrote:
>> 
>>> From: Jostein Kjønigsen <jostein <at> secure.kjonigsen.net>
>>> Date: Mon, 8 Sep 2025 14:50:23 +0200
>>> Cc: 79406 <at> debbugs.gnu.org,
>>> Yuan Fu <casouri <at> gmail.com>
>>> 
>>> On 8 Sep 2025, at 13:14, Eli Zaretskii <eliz <at> gnu.org> wrote:
>>> 
>>> Are all of these appropriate for the default value of
>>> treesit-font-lock-level?  
>>> 
>>> I'll have to be honest and admit I haven't tested any major-mode in "default" fontification level in probably a
>>> decade. My settings is maximum detail, and I've based my findings on that.
>>> 
>>> It may previously have been difficult to compartementalize some of these fontifications into different features
>>> and levels (due to extensive use of override:t and rules being heavily order-dependent).
>>> 
>>> With the changes in this patch however, I can probably move some rules about if that's wanted. Anything in
>>> particular you are thinking about?
>> 
>> Can you correlate the syntactic elements with the guidelines in the
>> doc string of treesit-font-lock-level?

I'll get back to this once, we're me and Yuan has reached an agreement on the things below.

>> 
>> 
>> Hmm... maybe nothing.  Yuan, could you please see if anything is
>> missing here?
> 
> Csharp-ts-mode is doing the right thing. Major modes don’t interact with treesit-font-lock-level directly. They just define font-lock rules categorized into features. And provide a feature list that’s split into levels. Emacs picks the features to enable based treesit-font-lock-level.
> 
> Most of the changes are good, I didn’t look into the rules themselves since I know Jostein is much more familiar to the grammar and C# than I do. I also like that we’re using less :override.

Yeah override makes lots of things needlessly confusing.

> 
> The only thing I’m not too sure about is the change from property to attribute. It’s technically a breaking change, I don’t think it’s “wait for 10 years to change it” level, but at least we should mention it in the NEWS.

My bad. I honestly didn't think about that. I can revert that, if it's a big deal.

> Also it seems you’re highlighting the brackets as well? Is there a particular reason to do that?

It was mostly for visual-reasons. Attributes/annotations are in many IDEs and editors "shaded out" in a mono-colour kind of way, to not distract away from the main method/class being annotated.

I think this works well and was trying to emulate the same behaviour in Emacs by making the brackets blend in with the rest of the annotation. I realize this technically speaking violates expectations some might have wrt to use of font-lock-bracket-face, but in overall I think it looks much nicer.

> And IMHO since attribute is metadata, property face is more suitable than variable-use face. But that’s subjective so it’s your call.

I agree it kinda subjective. And for me (again subjective) with my current theme I think it looks good (font-lock-variable-use is a shaded version of font-lock-varibable-name).

Now I'll be perfectly open to the fact that other people may not agree, and with other themes it might look less optimal than it does on my setup.

Maybe a way to resolve *both* these issues would be to introduce some new, csharp-ts-mode (and possible csharp-mode) specific faces?

- csharp-ts-mode-attribute-face
- csharp-ts-mode-attribute-bracket-face

The default could be font-lock-property-use-face and font-lock-bracket-face respectively, and people like me could customize them iif we want to.

Would that be a less controversial approach?

> 
> And there’s the small things like using full sentence for comments (period at the end) and the commit message format, those I can fix when applying the patch.
> 

If I'm making a new revision anyway, I can try to clean up the comments. As for the commit-message format... I thought I managed to follow that this time :-D

Oh well.

Let me know what you think, and I'll come up with a new patch.


—
Jostein

[Message part 2 (text/html, inline)]

Message #23 received at 79406 <at> debbugs.gnu.org (full text, mbox):

From: Yuan Fu <casouri <at> gmail.com>
To: Jostein Kjønigsen <jostein <at> secure.kjonigsen.net>
Cc: 79406 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#79406: [PATCH] 31.0.50; csharp-ts-mode does not fontify
 variable-use consistently
Date: Wed, 10 Sep 2025 00:07:01 -0700

> On Sep 9, 2025, at 12:46 AM, Jostein Kjønigsen <jostein <at> secure.kjonigsen.net> wrote:
> 
> 
> 
>> On 9 Sep 2025, at 09:06, Yuan Fu <casouri <at> gmail.com> wrote:
>> 
>> 
>> 
>>> On Sep 8, 2025, at 5:59 AM, Eli Zaretskii <eliz <at> gnu.org> wrote:
>>> 
>>>> From: Jostein Kjønigsen <jostein <at> secure.kjonigsen.net>
>>>> Date: Mon, 8 Sep 2025 14:50:23 +0200
>>>> Cc: 79406 <at> debbugs.gnu.org,
>>>> Yuan Fu <casouri <at> gmail.com>
>>>> 
>>>> On 8 Sep 2025, at 13:14, Eli Zaretskii <eliz <at> gnu.org> wrote:
>>>> 
>>>> Are all of these appropriate for the default value of
>>>> treesit-font-lock-level?  
>>>> 
>>>> I'll have to be honest and admit I haven't tested any major-mode in "default" fontification level in probably a
>>>> decade. My settings is maximum detail, and I've based my findings on that.
>>>> 
>>>> It may previously have been difficult to compartementalize some of these fontifications into different features
>>>> and levels (due to extensive use of override:t and rules being heavily order-dependent).
>>>> 
>>>> With the changes in this patch however, I can probably move some rules about if that's wanted. Anything in
>>>> particular you are thinking about?
>>> 
>>> Can you correlate the syntactic elements with the guidelines in the
>>> doc string of treesit-font-lock-level?
> 
> I'll get back to this once, we're me and Yuan has reached an agreement on the things below.
> 
>>> 
>>> 
>>> Hmm... maybe nothing.  Yuan, could you please see if anything is
>>> missing here?
>> 
>> Csharp-ts-mode is doing the right thing. Major modes don’t interact with treesit-font-lock-level directly. They just define font-lock rules categorized into features. And provide a feature list that’s split into levels. Emacs picks the features to enable based treesit-font-lock-level.
>> 
>> Most of the changes are good, I didn’t look into the rules themselves since I know Jostein is much more familiar to the grammar and C# than I do. I also like that we’re using less :override.
> 
> Yeah override makes lots of things needlessly confusing.
> 
>> 
>> The only thing I’m not too sure about is the change from property to attribute. It’s technically a breaking change, I don’t think it’s “wait for 10 years to change it” level, but at least we should mention it in the NEWS.
> 
> My bad. I honestly didn't think about that. I can revert that, if it's a big deal.

If you mention it in NEWS I think we’re good.

> 
>> Also it seems you’re highlighting the brackets as well? Is there a particular reason to do that? 
> 
> It was mostly for visual-reasons. Attributes/annotations are in many IDEs and editors "shaded out" in a mono-colour kind of way, to not distract away from the main method/class being annotated.
> 
> I think this works well and was trying to emulate the same behaviour in Emacs by making the brackets blend in with the rest of the annotation. I realize this technically speaking violates expectations some might have wrt to use of font-lock-bracket-face, but in overall I think it looks much nicer.

If that’s a common practice and you think it looks good, let’s do it.

> 
>> And IMHO since attribute is metadata, property face is more suitable than variable-use face. But that’s subjective so it’s your call.
> 
> I agree it kinda subjective. And for me (again subjective) with my current theme I think it looks good (font-lock-variable-use is a shaded version of font-lock-varibable-name).
> 
> Now I'll be perfectly open to the fact that other people may not agree, and with other themes it might look less optimal than it does on my setup.
> 
> Maybe a way to resolve *both* these issues would be to introduce some new, csharp-ts-mode (and possible csharp-mode) specific faces?
> 
> - csharp-ts-mode-attribute-face
> - csharp-ts-mode-attribute-bracket-face
> 
> The default could be font-lock-property-use-face and font-lock-bracket-face respectively, and people like me could customize them iif we want to.
> 
> Would that be a less controversial approach?

Yeah adding a csharp-ts-mode-attribute-face (with inherits variable-use-face) would be good. We don’t need to bother with csharp-ts-mode-attribute-bracket-face, that feels too much customization to me ;-)

Yuan

Message #26 received at 79406 <at> debbugs.gnu.org (full text, mbox):

From: Jostein Kjønigsen <jostein <at> secure.kjonigsen.net>
To: Yuan Fu <casouri <at> gmail.com>
Cc: 79406 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#79406: [PATCH] 31.0.50; csharp-ts-mode does not fontify
 variable-use consistently
Date: Thu, 11 Sep 2025 09:53:55 +0200

[Message part 1 (text/plain, inline)]

> On 10 Sep 2025, at 09:07, Yuan Fu <casouri <at> gmail.com> wrote:
> 
> 
>>>> 
>>>> Can you correlate the syntactic elements with the guidelines in the
>>>> doc string of treesit-font-lock-level?
>> 
>> I'll get back to this once, we're me and Yuan has reached an agreement on the things below.
>> 

I think this should be good now. I've moved the lambda-declaration down into the "function" section, making features semantically contained. 

You OK with this, Eli?

> 
> If you mention it in NEWS I think we’re good.

Done

> 
> If that’s a common practice and you think it looks good, let’s do it.

Done

> Yeah adding a csharp-ts-mode-attribute-face (with inherits variable-use-face) would be good. We don’t need to bother with csharp-ts-mode-attribute-bracket-face, that feels too much customization to me ;-)
> 
> Yuan

Done.

Attached is a patch which I think should address all of the above concerns. Let me know what you think!



—
Kind Regards
Jostein Kjønigsen

[Message part 2 (text/html, inline)]

[0001-csharp-ts-mode-fontification-fixes.patch (application/octet-stream, attachment)]

[Message part 4 (text/html, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.