csharp-mode: No highlighting after 547705d

[theothornhill]

Right now I'm using tree-sitter-hl-default-patterns inside csharp-mode to set the patterns. That seemed to work fine until latest commit, 547705d. I'm not completely sure why it stopped working, but now no highlighting shows. If I roll back one commit everything seems to be working again.

See: https://github.com/emacs-csharp/csharp-mode/blob/tree-sitter/csharp-mode.el#L335 for how the tentative, wip implementation is inside csharp-mode

[ubolonton]

When I first read this, I thought I knew what the issue was. But then I tried your code, and highlighting worked for me, so I don't know anymore 😅

Can you check the following variables, to see whether they have these expected buffer-local values:

• font-lock-mode: t
• font-lock-defaults: nil
• font-lock-set-defaults: t
• font-lock-fontify-region-function: should have tree-sitter-hl--highlight-region inside its definition.
• jit-lock-mode: t
• jit-lock-functions: (font-lock-fontify-region)
• tree-sitter-hl--query: non-nil
• tree-sitter-hl--query-cursor: non-nil

[theothornhill]

My values, where some are different:

• font-lock-mode: t
• font-lock-defaults: nil
• font-lock-set-defaults: nil (!)
• font-lock-fontify-region-function: it does
• jit-lock-mode: nil (!)
• jit-lock-functions: nil (!)
• tree-sitter-hl--query: non-nil
• tree-sitter-hl--query-cursor: non-nil

By the way, font locking in rust mode or some of the modes included here works. It is csharp-mode that isn't fontifying anymore.

I'll also add that I'm using emacs 27.1 on windows 10. My linux computer died, so cannot check there atm

[theothornhill]

And in 7875466db4fa02d0a2b8e847c7b31df2706a8935 I have the same values as you

[ubolonton]

Does setting font-lock-defaults to a non-nil value when csharp-mode is turned on fix it? Something similar to this:

(unless font-lock-defaults
  (setq font-lock-defaults '(nil)))

If that temporary fix works, I'll write down the explanation later.

[theothornhill]

This works! Explain! :D

[ubolonton]

font-lock relies on jit-lock for invalidation (on text changes and "viewport" changes).

As a "replacement" for font-lock, tree-sitter-hl should integrate with jit-lock as well. Instead of integrating with that directly, and replacing font-lock, tree-sitter-hl simply overrides font-lock functions. There are 2 reasons for this:

• I'm not yet sure how tightly coupled jit-lock and font-lock are. In theory, the former should be agnostic to the fontification code. I haven't thoroughly checked its code, though.
• More importantly, the interfaces provided by font-lock are used by many minor modes. For example, font-lock-add-keywords. We want these to keep working.

In other words, tree-sitter-hl works on top of font-lock, instead of disabling it outright.

font-lock has a "smart check" (see the end of font-lock-default-function) that, in certain situations, skips some of its initialization, including the jit-lock integration . Therefore, tree-sitter-hlused to have a hack that turns on this integration when font-lock skips it. This allowed tree-sitter-hl to work even without actually installing the corresponding major mode. However, that feature was removed because font-lock doesn't properly clean up this integration when it's explicitly disabled, causing #74.

The "certain conditions" above include checking whether any font-lock keywords were set. I have some global minor modes, so the keywords are always set for me. Without any minor modes adding font-lock keywords, one way for a major mode to trick font-lock is to specify an empty list of keywords: (setq font-lock-defaults '(nil)).

These fixes would probably be more reliable:

• tree-sitter-hl does the "empty keyword list" trick on behalf of major modes.
• tree-sitter-hl integrates with jit-lock directly when font-lock skips it.
• font-lock fixes font-lock-turn-off-thing-lock to properly clean up after font-lock-turn-on-thing-lock, so that tree-sitter-hl can use them.

[ubolonton]

Anyway, I think csharp-mode may want to initially treat tree-sitter as an optional dependency, keeping its current font-lock integration (font-lock-defaults) for a (long) while. If that's the case, this should be a non-issue for csharp-mode.

[theothornhill]

Thank you - this is useful information!

I was thinking tree sitter should be optional, considering it doesn’t seem to support *BSD etc, while cc mode does.

[jcs090218]

(unless font-lock-defaults
 (setq font-lock-defaults '(nil)))

I have tried this but it seems to be not working in the latest version of tree-sitter and csharp-mode. Any idea?

[theothornhill]

What isn't working? Right now I'm using the latest of both, and everything seems fine to me. Could you elaborate?

[jcs090218]

The highlighting isn't working. Following is my config for tree-sitter.

(use-package tree-sitter
  :config
  (require 'tree-sitter-langs)
  (add-hook 'tree-sitter-after-on-hook #'tree-sitter-hl-mode))

(global-tree-sitter-mode 1)

This works with other major mode but not csharp-mode. I have also tried emacs -q and it still not working. However, csharp-tree-sitter-mode did work.

[jcs090218]

Okay, I think the issue is there are no c-sharp highlight queries file in the directory here.

https://github.com/ubolonton/emacs-tree-sitter/tree/master/langs/queries

[theothornhill]

Okay, I think the issue is there are no c-sharp highlight queries file in the directory here.

https://github.com/ubolonton/emacs-tree-sitter/tree/master/langs/queries

That's not the case. The reason is that you are trying to apply tree-sitter to normal csharp mode, and that is absolutely not a good idea, https://github.com/emacs-csharp/csharp-mode/issues/207 being the most important point. Please rather submit you additions to csharp-tree-sitter-mode instead :)

[jcs090218]

The reason is that you are trying to apply tree-sitter to normal csharp mode, and that is absolutely not a good idea, emacs-csharp/csharp-mode#207 being the most important point.

Yes. That is why I was proposing to convert csharp-tree-sitter-mode to minor mode in the first place, see https://github.com/emacs-csharp/csharp-mode/pull/216. I do need tree-sitter runs in csharp-mode and not csharp-tree-sitter-mode because many packages use csharp-mode as an identifier.

Plus, tree-sitter is a minor mode so the user should customize it so tree-sitter does not run in csharp-mode (if they do not want tree-sitter-mode runs in csharp-mode). And there is no conflict with csharp-tree-sitter-mode since it is a major mode and NOT minor mode.

Please rather submit you additions to csharp-tree-sitter-mode instead :)

Like as said, I do need csharp-mode and not csharp-tree-sitter-mode as an identifier. I know the problem from https://github.com/emacs-csharp/csharp-mode/issues/207 but I can live with it. Furthermore, I run tree-sitter in c-mode, c++mode, java-mode, etc. They all rely on cc-mode, so basically saying I can not avoid the slowness from cc-mode. Why bother?

From my understanding, csharp-tree-sitter-mode is not going to replace csharp-mode? Then I don't see any conflicts by running tree-sitter inside csharp-mode. Especially, tree-sitter is a minor mode and it's very easy to enable/disable.

From the software architect's point of view, I see these two methods.

Contribute to upstream (my way)	Contribute to csharp-mode

Hence, I have decided to make contribution to this and not csharp-mode.

Edit: Register charp-mode to tree-sitter-langs may be unnecessary but I would still prefer architect on the left.

[theothornhill]

Yes. That is why I was proposing to convert csharp-tree-sitter-mode to minor mode in the first place, see emacs-csharp/csharp-mode#216. I do need tree-sitter runs in csharp-mode and not csharp-tree-sitter-mode because many packages use csharp-mode as an identifier.

This is a valid concern, and I have stated that at some point we should indeed make the tree-sitter version the main one. Contributing to that mode will help that goal be reached. "Forking" like this will not.

Plus, tree-sitter is a minor mode so the user should customize it so tree-sitter does not run in csharp-mode (if they do not want tree-sitter-mode runs in csharp-mode). And there is no conflict with csharp-tree-sitter-mode since it is a major mode and NOT minor mode.

I don't understand what you are saying here. It is a minor mode precisely so that major-modes should use it.

Please rather submit you additions to csharp-tree-sitter-mode instead :)

Like as said, I do need csharp-mode and not csharp-tree-sitter-mode as an identifier. I know the problem from emacs-csharp/csharp-mode#207 but I can live with it. Furthermore, I run tree-sitter in c-mode, c++mode, java-mode, etc. They all rely on cc-mode, so basically saying I can not avoid the slowness from cc-mode. Why bother?

The only modes suffering from this is pike-mode and csharp-mode. So yes, the other modes are fine. This is because of the way cc-mode handles strings. If you don't care about performance, why not use the normal csharp-mode? Your issues are solved then. Performance is not the only reason either. CC Mode uses its own font locking mechanisms that break in subtle ways when combined with other ways for emacs to fontify/propertize. I've tried to explain this several times before.

From my understanding, csharp-tree-sitter-mode is not going to replace csharp-mode? Then I don't see any conflicts by running tree-sitter inside csharp-mode. Especially, tree-sitter is a minor mode and it's very easy to enable/disable.

See above

From the software architect's point of view, I see these two methods.

Contribute to upstream (my way) Contribute to csharp-mode Hence, I have decided to make contribution to this and not csharp-mode.

Frankly, this doesn't make any sense.

Edit: Register charp-mode to tree-sitter-langs may be unnecessary but I would still prefer architect on the left.

Well, from an "architect" point of view, I'd rather fix what's broken and not completely gloss over serious issues.

[jcs090218]

This is a valid concern, and I have stated that at some point we should indeed make the tree-sitter version the main one.

Good, I will wait until that day.

Contributing to that mode will help that goal be reached. "Forking" like this will not.

I would not say this is a fork since tree-sitter-langs is package doing these kind of tasks. If you just want tree-sitter to work you probably should not use tree-sitter-langs since there is no point to use it.

I don't understand what you are saying here. It is a minor mode precisely so that major-modes should use it.

Easy, because you cannot have two major modes at a time.

Well, from an "architect" point of view, I'd rather fix what's broken and not completely gloss over serious issues.

True, so you have csharp-tree-sitter-mode and not csharp-mode (cc-mode) + tree-sitter. Good fix.

People who suffer from this issue SHOULD ALL USE csharp-tree-sitter-mode, AND NOT csharp-mode (From csharp-mode's README page, see https://github.com/emacs-csharp/csharp-mode#tree-sitter-support). I love your solution!

The only modes suffering from this is pike-mode and csharp-mode.

Okay, I was wrong. Sorry for the incorrect information.

If you don't care about performance, why not use the normal csharp-mode? Your issues are solved then.

Not solved. I am using csharp-mode but tree-sitter has a better highlighting. Maybe you can rewrite an entire highlighting from csharp-mode? I would love to use if you have a better highlighting from csharp-mode + tree-sitter.

Performance is not the only reason either. CC Mode uses its own font locking mechanisms that break in subtle ways when combined with other ways for emacs to fontify/propertize. I've tried to explain this several times before.

Well, if you can replace csharp-tree-sitter-mode to csharp-mode. Why not? I would love to use it. (Maybe don't rely on cc-mode and somehow rewrite it so csharp-mode/#207 doesn't occur)

---

Okay, so this is what I see.

What @theothornhill wants

1. Solve issue csharp/#207 (not to rely on cc-mode)

What @jcs090218 wants

1. I need csharp-mode as identifier
2. I need tree-sitter in csharp-mode (same reason from Pt. 1)

[jcs090218]

BTW, if you are going to make csharp-tree-sitter-mode officially the csharp-mode. Can you move highlight query to somewhere else (not in the code)? So we don't need to wait for melpa to update.

[theothornhill]

I think we want the sme thing. Open an issue with csharp-mode, and lets start working towards that goal. First goal should be to incorporate your additions to my code from your pr here.

There are some blockers, like M1 support missing etc, but we will get there quickly.

We will discuss the roadmap further over there. This issue is way off topic already :)

[jcs090218]

Sorry that I was being a bit rude here.

TBH, I am not good at creating major mode which I am very appreciate your work on csharp-mode! I am proposing this PR #105 because this is the easiest thing for me to do. And I personally think giving highlights.scm file is good for decoupling code (so you don't need to wait for melpa to build). I would suggest focus on solving csharp-mode/207 and rely on tree-sitter since we already added tree-sitter as charp-mode's dependency, see https://github.com/emacs-csharp/csharp-mode/pull/224 and https://github.com/emacs-csharp/csharp-mode/commit/98a179ad9c8bb7c816986cdd5b9afbe2fcc77f78.

Unless you insist to have query file inside csharp-mode's repository. Well, it's easier for me to contribute in once place (here).

We will discuss the roadmap further over there. This issue is way off topic already :)

Of course, you can ping me whenever you want to discuss this. I am willing to help out if I could. :)