Subject: bug#79406: [PATCH] 31.0.50; csharp-ts-mode does not fontify variable-use consistently

From: Yuan Fu <[email-masked]>
Date: Wed, 10 Sep 2025 00:07:01 -0700

> On Sep 9, 2025, at 12:46 AM, Jostein Kjønigsen <[email-masked]> wrote:
> 
> 
> 
>> On 9 Sep 2025, at 09:06, Yuan Fu <[email-masked]> wrote:
>> 
>> 
>> 
>>> On Sep 8, 2025, at 5:59 AM, Eli Zaretskii <[email-masked]> wrote:
>>> 
>>>> From: Jostein Kjønigsen <[email-masked]>
>>>> Date: Mon, 8 Sep 2025 14:50:23 +0200
>>>> Cc: [email-masked],
>>>> Yuan Fu <[email-masked]>
>>>> 
>>>> On 8 Sep 2025, at 13:14, Eli Zaretskii <[email-masked]> 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