Open elextr opened 9 months ago
int Hpp = 0; ++Hpp;
// ^^^ should not be highlighted
// I guess this is Scintilla (once a name is in a keyword list its always a type)
So yeah, the implementation of semantic tokens is a little dumb right now but I don't know if there's a simple way to fix it with Scintilla. The LSP server returns correctly the token types exactly with the right positions in the code that could then be colorized at those positions only. In Scintilla I found
https://www.scintilla.org/ScintillaDoc.html#Styling
but when I try to use SCI_STARTSTYLING
followed by SCI_SETSTYLING
, all the other colorization is lost. It appears to be used as an alternative to lexers when you want to colorize the whole document by yourself but it doesn't seem to be possible to colorize just some selected words in the document (but maybe I didn't do something right and I'm wrong here).
So what I did is that I check the positions of semantic tokens that LSP returns, get the strings at those positions from Scintilla, remove duplicates and pass it to Scinitlla the same way like for TM:
https://github.com/geany/geany/blob/eef67ecc200a6cf07f277c28282facf9854cd2ea/src/document.c#L2761
It's kind of backwards and we lose the information about the exact position of semantic tokens, but I'm not sure if there's anything we can do here (and before you ask, no, I currently don't plan implementing a whole lexer based on semantic tokens :-).
SCI_STARTSTYLING followed by SCI_SETSTYLING
IIUC thats for mainly for styling triggered by a SCN_STYLENEEDED notification, thats how the apps that use regex styling work with Scintila. I don't think it can safely be combined with a conventional lexer.
and before you ask, no, I currently don't plan implementing a whole lexer based on semantic tokens
I hate mind readers ;-P
Basically there needs to be a way of having syntax highlighting by the lexer (fast and synchronous) overridden by semantic stuff from the LSP (slow and asynchronous).
What do you use to trigger LSP queries? Vscode uses a slow(ish) timer (slow enough to be visible, maybe 1/4 sec Edit: actually its probably several secs, maybe I will find the setting one day) or when the cursor moves lines.
Maybe enable discussions on the repo for stuff like this?
What do you use to trigger LSP queries? Vscode uses a slow(ish) timer (slow enough to be visible, maybe 1/4 sec) or when the cursor moves lines.
Depends on what LSP queries you have in mind. Autocompletion, symbol tree, and semantic tokens are triggered by the same code like in Geany itself so it should be (if I remember correctly) the default 300ms timeout.
Diagnostic messages (underlined errors) are displayed immediately now and this isn't very efficient and could cause performance problems - there are multiple things I could do in this respect:
https://github.com/techee/geany-lsp/issues/4
It just worked OK for editing Geany itself so I didn't do anything about it yet.
Maybe enable discussions on the repo for stuff like this?
Should be enabled now (I hope).
It just worked OK for editing Geany itself so I didn't do anything about it yet.
Did you try in Scintilla (ie C++)? Remember how much slower compiling Scintilla files is than Geany files.
Did you try in Scintilla (ie C++)? Remember how much slower compiling Scintilla files is than Geany files.
I tried but didn't see anything too horrible. If you run into performance issues, would you try disabling various LSP features to check exactly which one it is? For performance, my suspects are:
diagnostics_enable=true
symbol_highlight_available=true
(well, symbol_highlight_available
should have been called symbol_highlight_enable
, I'll probably rename the settings name soon).
What do you use to trigger LSP queries? Vscode uses a slow(ish) timer (slow enough to be visible, maybe 1/4 sec) or when the cursor moves lines.
But I just noticed that I send textDocument/didChange
notification immediately which makes clangd re-parse the current file all the time. I think I should batch those (but make sure that if I need to send some other request to the server, the didChange notifications get sent before).
Well if you wait until you need it to send changes and initiate parsing you are making whatever needs the parsing wait on the parsing before it gets done, so maybe its not so good since it may make the user wait on it as well. Since the parsing is in another process having it do stuff in the background should not be a bad thing :tm:
Well if you wait until you need it to send changes and initiate parsing you are making whatever needs the parsing wait on the parsing before it gets done, so maybe its not so good since it may make the user wait on it as well.
I was thinking about batching change notifications in something like 250ms intervals unless something else might need them in which case they'd be sent earlier. From what I have seen, autocompletion for instance seems to return results before the compilation happens so it wouldn't be blocked by it. But maybe not worth the work I don't know.
I don't know how vscode behaves in this respect, it's just that while typing, the background clangd process seems to consume something like 60% CPU when editing some C++ Scintilla file (it's around 15% for plain C).
autocompletion for instance seems to return results before the compilation happens
That makes sense, if its triggering autocompletion then it is, by definition, not complete, so it possibly won't compile at that moment, so no point in waiting for a compile that might fail.
something like 60% CPU when editing some C++ Scintilla file
I tried vscode opening editor.cxx
took about 40% for a while, then editing editor.cxx
and renaming a symbol that needed it to edit several other unopened files, and it never went above 10-15% whereas just opening that file in geany-lsp has clangd at 70%, but after that it also never seems to get above 10-15%.
Edit: obviously it was a meson build of Geany to have the compile commands file, and vscode offered to work it all out itself, or read that :-)
I tried vscode opening editor.cxx took about 40% for a while, then editing editor.cxx and renaming a symbol that needed it to edit several other unopened files, and it never went above 10-15% whereas just opening that file in geany-lsp has clangd at 70%, but after that it also never seems to get above 10-15%.
Alright, if vscode (the aspirational benchmark of what Geany could become) behaves similar, it's probably alright :-).
Edit: obviously it was a meson build of Geany to have the compile commands file, and vscode offered to work it all out itself, or read that :-)
You don't even have to build it with Geany, just meson setup build
is enough and you can keep using autotools.
and you can keep using autotools.
Why? :stuck_out_tongue:
Just a new year update, have been using this intermittently and been quiet because its "just worked" for me. Congratulations, well done.
Edit: to be clear "this" means "geany-lsp" and mostly used it for answering support questions on Geany, its so nice to click a name and goto ... and have it right every time. And the hover is useful at times. C++ is still mostly Vscode due to the multi-window capability, sorry its just soooo useful.
Just a new year update, have been using this intermittently and been quiet because its "just worked" for me. Congratulations, well done.
Thanks, good to hear! I haven't had much time recently but I'd like to finish some things and have a look at #32.
Also, I've been thinking about some "non-modified-Geany" mode of the plugin in case we don't find any agreement in https://github.com/geany/geany/pull/3571. Basically, users would have to manually disable the ctags parser for the language they use the LSP plugin for in the filetype's config file using:
[settings]
tag_parser=
Then the plugin would have to detect ctrl+click and perform tag definition/declaration goto independently of Geany. There would also have to be separate keybindings for these actions. The symbol tree wouldn't work (which I wouldn't miss much myself). But then I believe I could make all the rest of the LSP functionality work fine as ctags wouldn't interfere as it would be disabled and since the Scintilla API is fully exposed to plugins.
I don't think its worth having a half baked version, all that will happen is users will ask "why doesn't it work like vscode/emacs/...".
I think we need to work through https://github.com/geany/geany/pull/3571 or some equivalent of it, provided lsp can be disabled and ctags left in its current state there is absolutely no excuse for anybody saying "I don't want it so nobody can have it". That is just unacceptable behaviour, of course they are not expected to contribute, but active blocking or attack without proposing any alternative should not happen.
Clearly we need to communicate the reasoning behind the changes, remember when it first happened I suggested using TM until I understood your reasoning of "simply replace specific Geany functions" as a less intrusive method of introducing new capabilities.
See comments in code
foo.cpp
: