⚠️ 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

⚠️ 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 🙂

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 😄 (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 😉 )


Now, I'm testing the plugin

⚠️ (once more) This is me randomly testing something I had the highest expectations on. Don't jump out of your seat too fast 😉

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 🙂

clangd

Without clangd installed, I get some mixed results opening a random file:

Now, let's install clangd, because that's kind of the point.

Opening a single C file without a project (src/document.c):

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…
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, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <geany/geany/pull/3571/c1817205646@github.com>