On 9 Sep 2025, at 09:06, Yuan Fu <casouri@gmail.com> wrote:
On Sep 8, 2025, at 5:59 AM, Eli Zaretskii <eliz@gnu.org> wrote:
From: Jostein Kjønigsen <jostein@secure.kjonigsen.net>
Date: Mon, 8 Sep 2025 14:50:23 +0200
Cc: 79406@debbugs.gnu.org,
Yuan Fu <casouri@gmail.com>
On 8 Sep 2025, at 13:14, Eli Zaretskii <eliz@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.