:warning: **Disclaimer**: Big chunk ahead! I started this taking notes reading the above discussion, and more, and I might not have re-written it that nicely, so bear with me -- but I think I should reply before the decade is over :)
First of all: I was thrilled by seeing this, LSP support seem(ed, more on that below) like what we are missing to get truly better language support in some area (esp. on completion) to me. Thanks for having implemented that @techee! --- ## Reactions to the discussion above ### Implementation in this PR :warning: I only did a *rough* pass over the code yet. 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. 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? Also, if it's not done right away (which could be just fine), maybe we should still think about what information doing so would require, so the calls could be ready. Unless we go the *tm-lsp* route, so we "just" have to make sure *everything* we use TM for is ready for LSP -- and one of my concerns below already lean towards that reviewing I guess. ### About tags source, plugins and others 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. E.g. a plugin like geanygendoc has to have info on symbols, and if the LSP could provide it it would be nice, as it could theoretically have *better* info. However, reading ahead I see that @techee suggests LSP symbol data usually sucks (to be blunt). Yet, as Thomas says > 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). 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). To see how feasible that is, we'd have to go through all consumers of the TM tags and see how that could work. ### tm-lsp > I think it would make a lot of sense to make a LSP server from the tag > manager and access it only through the LSP interface. Oh yeah, that's exactly what I thought when starting reading this :) Well, maybe not "only" if we have features LSP doesn't cover, but yes for things covered by LSP. ### Are LSP and LSP servers the Holy Grail? > I'm now using LSP for everything just to test it. But I think that for Geany > itself I'd prefer the TM+ProjectOrganizer symbol goto because in > ProjectOrganizer I can add external directories which I typically set to > directories with glib and gtk sources and get to these by ctrl+clicking gtk > or glib symbol (this way I also look at the documentation of various gtk > APIs). That's actually an interesting comment, that foreshadows what dawned on me when I actually went to trying the plugin out (see below). Basically, LSP isn't entirely the holy grail I though it might have been? And can't LSP server work nicely when throwing random files at them? This kind of changes how I gut-feel about this, because I myself want Geany to still be able to be a good *editor* where I can pop random files with decent functionalities, and not have to set up a whole project all the time. I can of course learn to create more projects to benefit from advanced LSP features, but I don't think I can learn not to open random files :slightly_smiling_face: ### Symbol lookup and quick-search FTW > You are missing this is a very common use case :-). This allows quick > navigation across the whole project based e.g. on function name so when you > are implementing something that is spread across multiple files, you can get > where you want very quickly. With normal goto you'd have to fake-write where > you want to go, control-click, go back to the original document, and undo it > afterwards. Haha, raise your hand if you never did that :smile: (yeah, yeah, @elextr, put it down) Seriously yes it's a very useful feature I think -- and no, it's not only useful because of any current limitation, it's also a useful way to navigate the code in general. I might need to e.g. work on another part of the code, and I know it's in class `ThatSomething`, or function `something_useful()` -- I might even not be sure in which file it actually is, or it might be quicker to just type ahead than navigate. I didn't write commander only to show something off, I use `f:` all the time :) (and some others as well, don't *ever* change what `tite` or `li4` does in French locale :wink: ) --- ## Now, I'm testing the plugin :warning: (once more) This is me randomly testing something I had the highest expectations on. Don't jump out of your seat too fast :wink: ### Random prelude 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? ### Actually giving this a spin OK, first impression trying the plugin is… fairly terrible :slightly_smiling_face: #### clangd Without clangd installed, I get some mixed results opening a random file: - I don't get a symbol tree *at all*. I guess that's a bug with the fallback? - I get go to definition as usual (good) Now, let's install clangd, because that's kind of the point. ##### Opening a single C file without a project (src/document.c): - I get a (degraded) symbol tree; half the contents is missing - I don't get go to definition -- well, it's there, but doesn't go anywhere useful for half the content. I guess both of these come down on clangd not understanding the input. If I `#define GEANY_API_SYMBOL /* nothing */` it improves quite a bit (symbol tree looks (almost?) complete), but still e.g. ctrl-click is erratic (e.g. `document_undo_clear_stack()` fails while `document_undo_clear_stack()` works -- I guess it has to do with unknown types in the signature. 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. ##### Setup the Geany project after meson setup… - Very slow at start, but well, I'm waiting. I guess maybe if I had actually build with meson it could have possibly been faster? - Works way better indeed :tada: - Some difference in behavior will get some getting used to, like not being able to get completion for a function because it's not ordered right (e.g. only declared below in the file)… makes sense, but can be a tad annoying when refactoring things I imagine. Nothing the implementation here is about though I guess. - note: The behavior clangd has of giving a default signature for unknown functions is a lot of thing, including understandable (that's actually valid C pre-99), unexpected, and could be very confusing without the red squiggly and the hover info. I'm basically mentioning this suggesting squigglies are kind of required for this particular completion not feeling wrong. - Loading some files felt slow, e.g. the symbol trees after opening scintilla/gtk/ScintillaGTK*.cxx took very long to show up (few seconds, but enough that I wonder if it worked at all). Maybe that's my machine, but… anyway. - 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. I don't support this style of programming, but the program itself is valid because that header is included properly… I get there is not much to be done here as standalone it's indeed invalid, and it could mean different things depending on what is included before, but Scintilla isn't the only project having "bad practices" like that. ##### General impression Not good without a proper project setup (expected I guess). Pretty good with it, but I had disappointments of how it plays in real life I didn't expect. #### pylsp Opening a single Python file (scripts/gen-api-docs.py) was even more painful than with clangd: nothing seemed to work (pylsp 1.7.1 from the headers). Somehow the Python server started working… at some point? It feels like it started working when I replaced the hashbang at the file's start to remove the `env` use, but that doesn't seem right nor reproducible… weird. Also, the symbol tree is garbage (no hierarchy). Anyway, it seems like something in the pipe makes pylsp unreliable, at least at start. Also, at least version 1.7.1, some things like the symbol tree are not on-par with CTags -- but I hope they would improve that in the future, although there might not be much incentive out there if something that seem as mondaine as the hierarchy isn't implemented yet. Once it started working, features like completion and calltips seemed to work nicely though. ### Test summary 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. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3571#issuecomment-1817205646 You are receiving this because you are subscribed to this thread. Message ID: <geany/geany/pull/3571/[email protected]>
