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

Previous Next

Full log

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)]

Previous Next

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