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