GNU bug report logs -
#79406
[PATCH] 31.0.50; csharp-ts-mode does not fontify variable-use consistently
Previous Next
Full log
Message #23 received at 79406 <at> debbugs.gnu.org (full text, mbox):
> 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
This bug report was last modified 2 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.