elisp-tree-sitter
elisp-tree-sitter copied to clipboard
csharp-mode: No highlighting after 547705d
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
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 havetree-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
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
And in 7875466db4fa02d0a2b8e847c7b31df2706a8935 I have the same values as you
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.
This works! Explain! :D
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
andfont-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-hl
used 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 withjit-lock
directly whenfont-lock
skips it. -
font-lock
fixesfont-lock-turn-off-thing-lock
to properly clean up afterfont-lock-turn-on-thing-lock
, so thattree-sitter-hl
can use them.
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
.
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.
(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?
What isn't working? Right now I'm using the latest of both, and everything seems fine to me. Could you elaborate?
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.
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
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 :)
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.
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 needtree-sitter
runs incsharp-mode
and notcsharp-tree-sitter-mode
because many packages usecsharp-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 sotree-sitter
does not run incsharp-mode
(if they do not wanttree-sitter-mode
runs incsharp-mode
). And there is no conflict withcsharp-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 notcsharp-tree-sitter-mode
as an identifier. I know the problem from emacs-csharp/csharp-mode#207 but I can live with it. Furthermore, I runtree-sitter
inc-mode
,c++mode
,java-mode
, etc. They all rely oncc-mode
, so basically saying I can not avoid the slowness fromcc-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 replacecsharp-mode
? Then I don't see any conflicts by runningtree-sitter
insidecsharp-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
totree-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.
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
- Solve issue csharp/#207 (not to rely on
cc-mode
)
What @jcs090218 wants
- I need
csharp-mode
as identifier - I need
tree-sitter
incsharp-mode
(same reason from Pt. 1)
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.
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 :)
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. :)