@b4n Thanks for having a look at it! > I'm not fond of the implementation, but it might be a good first step. I'll > have to check the plugin to get a better grasp on some design decisions > maybe, but basically I think it'd be better to unify the current code instead > of having some if (lsp) at key places.
The implementation is meant as a start of some discussion and to get something working than something that should be merged in the current form. The discussion with Thomas (and maybe I misunderstood his point) was that he suggested to get the symbols from the LSP and implement our stuff on top of that but that wouldn't bring any gains compared to the current state because the symbols themselves aren't enough e.g. for good autocompletion. So there will have to be separate calls to the LSP plugin to get the contents of the autocompletion popup, separate call for calltips, separate call for goto, etc. I'm sure many of the `if`'s can be eliminated but there will still be some key differences like the fact that LSP is asynchronous or that autocompletion is triggered by different conditions (e.g. there is no "scope" or "non-scope" autocompletion in LSP, just "trigger characters" that, when typed, should invoke the corresponding code on the LSP server side, etc.). So some form of `if`'s will still have to be there (whether explicit `if`'s or some "virtual method like" `if`'s is other question). There's of course the possibility of ctags-LSP in which case the API would be unified completely but I'm afraid I don't have time for that now (I've greatly exceeded my Geany-allocated quota already :-). We could do some half-way hybrid where TM is still inside the Geany process but uses an API similar to LSP but then we need to synchronize Scintilla buffer with some "shadow" buffer inside TM on top of which e.g. the autocompletion would happen. TM autocompletion, now, is Scintilla-assisted and we cannot easily perform it in TM alone. I was thinking that if we (ever) converted TM into a LSP server, we might still use Scintilla on the server side for the document model and then we could also use it for autocompletion assistance. The main point of the PR was to get some idea what will be needed. I initially thought we'd need many more API calls e.g. for the synchronization but since Scintilla is exposed to plugins, it's really just the "disable Geany stuff, let LSP do it" kind of API and I think functionality-wise, the API from this PR (plus the extra signal from the other LSP PR) is all that's needed. > I read your 3 points and yes this might require some non-trivial refactoring > Geany's code, but wouldn't it be cleaner in the end? Are you talking about placing LSP symbols into TM here? We could do it "somehow" (and that's really not doable in some universal way because we'd really need LSP-server specific mappings for that and I don't think it's the way to go). But let's say we do it - what would we gain of it? For instance consider C/C++ - the trouble of the current TM isn't that we would get some wrong symbols. The trouble is that from the symbols themselves we cannot get enough information for autocompletion or some smarter symbol goto for C++. In fact, for C/C++ we'd even get less symbols because we wouldn't get the symbols for local variables which are supported by the new cxx ctags parser. And as you noticed, pylsp returns only flat symbol tree without scope. Do we want to place such symbols into TM and replace the result we get from ctags? > I hear Thomas' concerns about plugins having a different representation of > document's tags, esp. if we have a LSP-filled symbol tree. Rather than not > being "good enough", it's a consistency and "privilege level" question. When you start filling LSP symbols into TM you run into another set of inconsistencies like that e.g. `clangd` parses files compiled for the current configuration of the project (e.g. GTK with only wayland and linux backend) but TM potentially parses all files. Symbol kinds are slightly different in TM and LSP (e.g. there's no "macro" symbol in LSP). The `kind` field of LSP contains a server-spacific info unlike TM arguments or `var_type`. You get local variable symbols from TM and not from LSP. So I'm afraid we cannot get any real consistency. > However, reading ahead I see that @techee suggests LSP symbol data usually > sucks (to be blunt). Sucks isn't probably the right word, "incompatible" would characterize it better. > E.g. a plugin like geanygendoc has to have info on symbols I think you need argument list for this one, right? And the `details` field of LSP symbols is specific to every server so you'd have to do server-specific things to extract it. >> and ideally I want to have this functionality for languages where LSP is the >> only option (where we have no parser). > which I basically agree: ideally a LSP for a language we don't support (well) > should be able to give us good features, not half not working (hoping the LSP > server for that language is good). At least now the situation isn't so rosy I'm afraid and I think in most cases we'll get something worse. One of the negative surprises for me was that there actually aren't so many high quality LSP servers that are easy to install. `clangd` and `gopls` are real exceptions, `pylsp` sucks a bit and that's pretty much it what's e.g. packaged with Debian. I was hoping we could use all the LSP servers that vscode has but many are written as vscode plugins only and cannot run in a standalone way. > I'm not saying we should necessarily fill doc->tm_file->tags_array, but that > if we can't provide that API with LSP, maybe we'd need to replace it with one > that does work with LSP (e.g. querying using the LSP) Querying using LSP or having a parallel set of LSP symbols is the way I'd suggest to go and let plugins decide what to do with such symbols and how to interpret and display the information. But access to symbols is only small part of what LSP servers offer and that's the difference against TM - for TM this is everything the whole logic is based on; for LSP servers it's just an exported information from its internal representation that's suitable for displaying in the symbol tree. > Are LSP and LSP servers the Holy Grail? I'd separate it in two parts - the LSP protocol and the actual server implementations. The protocol is pretty great (at least the base part of it, it appears to have grown too much in too many directions) and abstracts-away the logic of such a server from the logic of the editor in a language-independent way. For the servers themselves it depends. They may be better when there is a good LSP server. They need extra configuration. They will be slower. You typically can't just parse arbitrary independent files using them. What we have in Geany is actually great and I want Geany to keep working this project-independent way, but there may be situations where LSP is a better option. And there are situations when LSP is a terrible option and TM is much better. > Symbol lookup and quick-search FTW This UI is great and I think it should be directly in Geany so plugins don't re-implement it over and over with different keybindings showing different kinds of the same. In fact, I was thinking this UI could be used instead of the possibly giant TM symbol goto popup menu. > How comes I could build the plugin without libjsonrpc-glib-1.0-dev with > Autotools, but fails at setup with meson? Bundled but not set up for build > with meson? Because I forgot to remove it, will do :-). I had to bundle the libraries because I wanted to make sure that the plugin has this fixed: https://github.com/techee/geany-lsp/issues/16 > OK, first impression trying the plugin is… fairly terrible 🙂 Thanks, that's encouraging ;-) > Without clangd installed, I get some mixed results opening a random file: I'll have a look at it. What I should definitely do is to disable all servers in the global config file and only let users enable them manually - then it will stay out of the way when the servers aren't installed. > For me, this means that it's not fit for general use (yet?) outside a proper > project configuration, because I'd really still want to be able to open > random single files and have things (mostly) working. I understand it's > probably not gonna be fixable for languages like C; but then I think there's > a need for the plugin to seamlessly retire itself in the background when it > can know it's not gonna work right. Right. We were already talking with @elextr that there should be a config option to disable LSP for files outside the project directory. And there should also be similar config option to disable LSP if no project is open. Both options should be per-filetype because some servers might handle this situation better than others. > Very slow at start, but well, I'm waiting. Do you mean waiting until "indexing" ends or really something making the GUI stuck or unresponsive? (That shouldn't happen.) By the way, after implementing the progress messages and seeing this "indexing" I really had a slightly bad feeling. I switched to Geany from Eclipse 12 years ago because I was seeing this "indexing" all the time (on a project with a special build system that couldn't be converted to something understandable by Eclipse) with permanent near 100%CPU usage and now I'm introducing something like that to Geany. :-) > I guess maybe if I had actually build with meson it could have possibly been > faster? Nope, the indexing part is independent on how you built the project. > It's unfortunate how easily it gets distracted though. e.g. it's of no use in > ScintillaGTK.h because that header lacks a lot of includes. Yes, I've seen it too. > This was not even an hour of testing, I'll have to actually try and use this > for real things a bit to get a better impression at what it's actually good > at. But basically I kind of fell off my little cloud (does that even > translate? Not sure, but you get the point: I had blind naive hopes reality > stabbed in the back). No biggie, and I guess I should have had lowered my > expectations; and again trying it more is likely to show me the better parts > of what it can bring. I think we actually have quite a gem in terms of what TM offers in Geany and that you can throw it at arbitrary files and it works (with limitations). And I feel converting this to a LSP server would make users of other editors really happy. Having experience with other editors I kind of expected that proper project setup will be necessary and there will be some files where the typical LSP servers won't work. My biggest disappointment was the lack of good servers or at least servers not being worked on by a single guy. I expected to be able to use some vscode servers but this isn't possible. That said, I've used it for the development of the plugin itself and now I'd really miss the diagnostic messages with errors without the need of recompilation, the autocompletion, properly highlighted types, "find references" of a symbol, better behaving calltips (but the current disappearing of the calltip window could be fixed in Geany) and partly the symbol goto (I can imagine using both versions). I don't care much about the symbol tree (what I need much more is the find symbol filter popup). So while some notes above may sound gloomy, it is an improvement, but again, for certain kinds of projects only. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3571#issuecomment-1817503007 You are receiving this because you are subscribed to this thread. Message ID: <geany/geany/pull/3571/c1817503...@github.com>
