This patch adds a simple API allowing LSP plugins to take control of certain Geany built-in features and disabling their implementation in Geany. Currently, autocompletion, calltips, and going to definition/declaration is supported.
Plugins should define the Lsp struct, fill the pointers to the functions implementing the features they override, and register themselves using the lsp_register() call. Similarly, they should unregister themselves using lsp_unregister() when they don't want to provide the Lsp features any more, or, latest, when the plugin gets unloaded.
Each of the currently supported features is implemented using two functions - one checking whether Lsp plugin will override Geany's behavior of this feature, and the other function implementing the feature. For instance for autocompletion: - autocomplete_available(doc) should return TRUE if the plugin performs autocompletion for the doc's filetype. This function is used to deactivate Geany's autocompletion when the Lsp plugin supports it. - autocomplete_perform(doc) calls the corresponding interface of the LSP server (which responds asynchronously so when autocomplete_perform() is executed, the results won't be available yet)
The Lsp struct is padded with an array large enough to allow additions of many more functions to the struct and allowing binary compatibility with older LSP plugin versions not implementing all the functions. It's just mandatory that all plugins zero-initialize the allocated Lsp struct (either by creating it on the heap or by allocating it using g_new0() or similar function). NULL pointers in this struct are handled gracefully.
The current implementation supports only a single LSP plugin. However, if in the future arises the need for more LSP plugins running in parallel (e.g. a specialized LSP plugin for certain language together with the current "generic" LSP plugin), the implementation could be extended to support a list of Lsp structs and trying to call the functions defined by them one by one. The current interface should already be sufficient to support such an implementation. You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/3571
-- Commit Summary --
* Add API for LSP plugins
-- File Changes --
M meson.build (3) M src/Makefile.am (2) M src/editor.c (32) M src/keybindings.c (11) A src/lsp.c (107) A src/lsp.h (59) M src/symbols.c (9)
-- Patch Links --
https://github.com/geany/geany/pull/3571.patch https://github.com/geany/geany/pull/3571.diff
I've created a repository containing Geany with this API + the LSP plugin I've been working on here:
https://github.com/techee/geany-lsp
For now it's easier to have Geany together with this plugin but once some form of the API is merged into Geany, I'll remove the Geany portion. I'll be really interested in any feedback - please use the repository's issue tracker so the reported problems aren't buried in some long threads and don't get forgotten.
The only missing Geany feature not implemented by the plugin yet is the symbol tree - I'm going to have a look at it soon. Otherwise all the other tag manager functionality plus some extras should be supported.
The README.md of the repository contains some information what is implemented in the plugin and how to use it. I've mostly tested it for the development of the plugin itself so mostly C code. Apart from clangd I've also tested it lightly with gopls (Go) and pylsp (Python).
I've also created a new label "lsp" which I'm going to use - however, so far the changes needed on the Geany side were quite minimalistic so hopefully it won't become a new tag manager.
@kugel- commented on this pull request.
+G_BEGIN_DECLS
+ +typedef struct { + gboolean (*autocomplete_available)(GeanyDocument *doc); + void (*autocomplete_perform)(GeanyDocument *doc); + + gboolean (*calltips_available)(GeanyDocument *doc); + void (*calltips_show)(GeanyDocument *doc); + + gboolean (*goto_available)(GeanyDocument *doc); + void (*goto_perform)(GeanyDocument *doc, gboolean definition); +} Lsp; + + +void lsp_register(Lsp *lsp); +void lsp_unregister(Lsp *lsp);
I would also pass the size of the `Lsp`. Reason: If you extend the struct for new function points you can't be sure if the plugin that's registering is already compiled against the new size, unless you also bump the ABI. To avoid having to bump the ABI the plugin can pass `sizeof(Lsp)` that it was compiled against, and Geany can pad with NULLs up to the actual size.
Furthermore, these kind of register functions should also pass the `GeanyPlugin` to give Geany some context.
I wonder why we can't have an LSP plugin shipped with the core to ensure support this kind of plugins properly.
@techee commented on this pull request.
+G_BEGIN_DECLS
+ +typedef struct { + gboolean (*autocomplete_available)(GeanyDocument *doc); + void (*autocomplete_perform)(GeanyDocument *doc); + + gboolean (*calltips_available)(GeanyDocument *doc); + void (*calltips_show)(GeanyDocument *doc); + + gboolean (*goto_available)(GeanyDocument *doc); + void (*goto_perform)(GeanyDocument *doc, gboolean definition); +} Lsp; + + +void lsp_register(Lsp *lsp); +void lsp_unregister(Lsp *lsp);
I would also pass the size of the Lsp
Oh, I somehow didn't post the right version of the patch - in the correct one I also added `gchar padding[1024]` at the end of the struct. Plugins will have to allocate this struct with `g_new0()` or on the heap so the rest of the struct is zero and those `CALL_IF_EXISTS()` macros check the NULL and fall back to some empty implementation.
Furthermore, these kind of register functions should also pass the GeanyPlugin to give Geany some context.
Could you add more detail what this would be good for?
I wonder why we can't have an LSP plugin shipped with the core to ensure support this kind of plugins properly.
What do you mean by "properly"? I think this interface adds all that's needed (the only missing piece is the symbol tree I'll start working on soon).
But of course this plugin could be added to Geany itself but it introduces a dependency on jsonrpc-glib (which itself depends on json-glib) and I'm not sure if such a dependency would be fine with everyone. But if others agree, this plugin could become part of Geany itself.
One more thing I worry about though is that as many people don't have much time for Geany, it will take ages to add new features or fix bugs - I'll be probably much more agile when maintaining the plugin by myself.
Your call. I would welcome a first class, reference plugin for LSP support, to show off how to do it right and to ensure Geany's support for LSP plugins remains fully intact. I think optional dependencies are fine, Geany still works without the plugin for the foreseeable future.
I'm not against it, let's see how others feel about it, but as I'm still planning to add more features to it, I'd suggest to have it separate at least initially while the "very active" development lasts.
By the way, I'm not sure if there will ever be "LSP plugins" and not just this single LSP plugin. I'll try to make it generic enough and configurable enough to support as many servers as possible. It would only make sense to create another LSP plugin if someone wanted to tailor it specifically to some LSP server (and a language).
@techee good on you for giving this a go.
Whilst I agree with @techee that there should only ever be one LSP plugin because LSP is itself an API intended to be extensible so there is no real need for another (so long as we guard against "all languages are C" assumptions creeping into it), for now starting this as a separate plugin is a good idea, it can be hacked and crashed and every other thing until it works reliably and supports most/all of the lsp API, eg didOpen, didChange, goto declaration/definition, colouring etc. Then it can be promoted to Geany, if @techee wants to, there are of course plugins that are not submitted to Geany, its the developers choice.
Design question, at the moment its tractable to test feature availability before calling at each place its needed throughout Geany, but its going to become messy as more features are added, which can discourage adding those features. Instead can the Geany features also be wrapped in a function like the LSP ones and put as the default in the per doc (or is it per filetype?) function pointer table, so all anything ever has to do is call the function pointed to, the table will never have null pointers.
@techee how are you proposing to handle dependency information for the LSP? It needs to know where to find include files for C++ for example so it can perform complete type tracing. Geany does not have that information available ATM, and heavyweight IDEs do it by parsing the build system, makefiles, meson files, cmake files etc.
To avoid that (at least for the moment) maybe you can review [this](https://microsoft.github.io/language-server-protocol/overviews/lsif/overview...) which seems more ctagsian and does not need includes and could maybe even be imported into tagmangler (the linked doc says "database" but probably tagmangler would do).
it can be hacked and crashed and every other thing until it works reliably and supports most/all of the lsp API, eg didOpen, didChange, goto declaration/definition
All of them work (and the plugin is stable as far as I can say) but still I want to implement more features or possibly refactor some things so it's definitely far from complete.
Design question, at the moment its tractable to test feature availability before calling at each place its needed throughout Geany, but its going to become messy as more features are added,
The only one missing is the symbol tree - otherwise the feature set overlapping with Geany is complete so there won't be much more. Those extras in LSP that don't have the Geany tag manager counterpart don't have to be handled in Geany itself at all. I already implemented "diagnostic messages" and "hover" but since the whole Scintilla is exposed to plugins, the majority of features can be implemented without Geany assistance.
Instead can the Geany features also be wrapped in a function like the LSP ones and put as the default in the per doc (or is it per filetype?) function pointer table, so all anything ever has to do is call the function pointed to, the table will never have null pointers.
There are three problems with this: 1. Geany uses synchronous calls while LSP uses asynchronous communication. Geany expects that when you call a function implementing for instance autocompletion, it will happen when the function terminates. LSP, on the other hand, only sends a request to the server and when the function finishes, the results aren't available yet. Unifying it with Geany would mean we'd probably have to modify Geany to expect asynchronous results from the current implementations of these features (the only place where I'll probably do it is the symbol tree because the amount of code related to it is quite big and complicated and I'd like to re-use as much of its implementation as possible).
2. The logic behind the Geany's implementation of various features differs significantly. For instance, for autocompletion, there are multiple modes (non-scope, scope, document wordchars, something HTML-specific too) and the logic when autocompletion is triggered differs very much from how it's done using LSP (in LSP, you just have some "trigger characters" which, when typed, should make the client send the request to the server and the server either returns something or not). The logic in Geany involves checking what has been typed before the cursor and checking tags in the tag manager and other things. So it's hard to unify both the Geany code and the LSP code behind some universal interface.
3. I wanted to make the changes on the Geany side as little intrusive as possible. The code related to the tag manager is already complicated enough and having to think about one more mode of operation would complicate things. So the current code just disables Geany-provided implementation which IMO simplifies things.
so all anything ever has to do is call the function pointed to, the table will never have null pointers.
There will probably be just two more functions for the symbol tree now - the NULL pointers are really just in case we implement something using TM in Geany and we'll want to switch between LSP and Geany's implementation.
@techee how are you proposing to handle dependency information for the LSP? It needs to know where to find include files for C++ for example so it can perform complete type tracing. Geany does not have that information available ATM, and heavyweight IDEs do it by parsing the build system, makefiles, meson files, cmake files etc.
By using an external tool, independently from Geany since Geany itself doesn't know how to build projects and relies on external tools already. I don't know if you had time to play with the plugin and if you checked
https://github.com/techee/geany-lsp/blob/master/README.md
but for clangd, it uses the `build_commands.json` generated by meson. I also tried to use the Bear tool with our autotools build and it works too.
Other languages may/may not need something similar. With the python pylsp server and the go gopls server nothing like that seemed to be necessary.
To be clear, I didn't get to look at your plugin yet, so sorry if I ask silly questions.
The only one missing is the symbol tree
But as you say, by no means the smallest :-P
But ok, if re-implementing everything in the plugin by directly accessing Scintilla (but using LSP for the smarts) is acceptable from the effort point of view then I agree with just disabling Geany functions to allow the plugin to do the lot.
With the python pylsp server and the go gopls server nothing like that seemed to be necessary.
Nice to see that modern languages have learnt from the C build debacle (autotools, jhbuild, meson, cmake, scons, ninja, make, gmake, kmake, and on and on...) and not made the same mistakes.
BTW although I was joking [here](https://github.com/geany/geany/issues/3550#issuecomment-1704453258) its almost there, see [this](https://code.visualstudio.com/docs/editor/intellisense#_enhance-completions-...).
I just thought, for colouring we can always simply turn off the Scintilla lexer (set NULL) and pass the [messages](https://www.scintilla.org/ScintillaDoc.html#SCN_STYLENEEDED) to the LSP.
I just thought, for colouring we can always simply turn off the Scintilla lexer (set NULL) and pass the [messages](https://www.scintilla.org/ScintillaDoc.html#SCN_STYLENEEDED) to the LSP.
You can't live without Scintilla coloring - ultimately it will always be Scintilla that colors the document. LSP protocol (but not all LSP server implementations) offers semantic tokens
https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17...
but they only tell you that a range in the document is say a class name but how that class is visualised is up to the client (and Scintilla in our case).
And this will be one of the last things I'm going to implement anyway :-P.
Sorry terminology error, turn off scintilla lexer setting styles, not do away with Scintilla and its rendering of the buffer.
So what I am saying is that when the LSP says that a range is a classname we can set the classname style on that range in Scintilla the same way the lexer does now.
Although MS calls it "semantic" tokens, it seems to also include simple syntax things like keywords, numbers, strings, comments etc so it can provide the information to completely replace the lexer.
And the LSP message specifies a range to return, which seems to map to Scintilla's `SCN_STYLENEEDED` message.
Of course I'm sure there will be many complicated details in that simple description :smiling_imp:
Although MS calls it "semantic" tokens, it seems to also include simple syntax things like keywords, numbers, strings, comments etc so it can provide the information to completely replace the lexer.
I think they are just trying to be generic enough for the purpose of the definition of the protocol in case some server provides such tokens, but in reality, not all of them have to be provided by the server. For instance, clangd doesn't provide keywords, numbers, or strings; pylsp doesn't support semantic tokens at all.
During the `initialize()` call the client announces its capabilities and, in return, the server sends back the features it supports. For clangd it looks this way:
``` { "capabilities" : { "astProvider" : true, "callHierarchyProvider" : true, "clangdInlayHintsProvider" : true, "codeActionProvider" : true, "compilationDatabase" : { "automaticReload" : true }, "completionProvider" : { "allCommitCharacters" : [ " ", "\t", "(", ")", "[", "]", "{", "}", "<", ">", ":", ";", ",", "+", "-", "/", "*", "%", "^", "&", "#", "?", ".", "=", """, "'", "|" ], "resolveProvider" : false, "triggerCharacters" : [ ".", "<", ">", ":", """, "/", "*" ] }, "declarationProvider" : true, "definitionProvider" : true, "documentFormattingProvider" : true, "documentHighlightProvider" : true, "documentLinkProvider" : { "resolveProvider" : false }, "documentOnTypeFormattingProvider" : { "firstTriggerCharacter" : "\n", "moreTriggerCharacter" : [] }, "documentRangeFormattingProvider" : true, "documentSymbolProvider" : true, "executeCommandProvider" : { "commands" : [ "clangd.applyFix", "clangd.applyTweak" ] }, "hoverProvider" : true, "implementationProvider" : true, "memoryUsageProvider" : true, "referencesProvider" : true, "renameProvider" : true, "selectionRangeProvider" : true, "semanticTokensProvider" : { "full" : { "delta" : true }, "legend" : { "tokenModifiers" : [ "declaration", "deprecated", "deduced", "readonly", "static", "abstract", "virtual", "dependentName", "defaultLibrary", "usedAsMutableReference", "functionScope", "classScope", "fileScope", "globalScope" ], "tokenTypes" : [ "variable", "variable", "parameter", "function", "method", "function", "property", "variable", "class", "interface", "enum", "enumMember", "type", "type", "unknown", "namespace", "typeParameter", "concept", "type", "macro", "comment" ] }, "range" : false }, "signatureHelpProvider" : { "triggerCharacters" : [ "(", ")", "{", "}", "<", ">", "," ] }, "textDocumentSync" : { "change" : 2, "openClose" : true, "save" : true }, "typeDefinitionProvider" : true, "typeHierarchyProvider" : true, "workspaceSymbolProvider" : true }, "serverInfo" : { "name" : "clangd", "version" : "Debian clangd version 14.0.6 linux+grpc aarch64-unknown-linux-gnu" } } ```
I think they are just trying to be generic enough for the purpose of the definition of the protocol in case some server provides such tokens
Probably, and/or VS server provides them.
Anyway so we are back to the issue of having semantic styling overriding lexer syntax styling, preferably without modifications to Scintilla/lexers, oh well it was a thought.
@techee pushed 6 commits.
76f093fefec295958fc212d95aedb798b650b50e Add API for LSP plugins e330c6274db1affa6d4953c90216c929565d4a32 Add padding for possible future extensions 7a83035787cad519ed8568fe6b83fe6f83683fda LSP interface for document symbol queries 50594a7694da24a5371dd7a1851718bf4339444c sidebar.c and symbols.c changes related to document symbols 31eb9006de38fed9ed3afa9939cf084e4b14d1d2 Add API for semantic token colorization 716df2f1dd71fd1925606b655c232d7f56ef18cd Support highlighting using LSP in document.c
@techee pushed 6 commits.
00d9357fe5f359a3a901cfc2dcc0f41d4536c6ef Add API for LSP plugins 74f7d0b9900b43622c1a6caa9ae86a805efcc2bb Add padding for possible future extensions 2ca122762d68cfcf1c4cc8dbe6f1404c3cbb64a8 LSP interface for document symbol queries 576e4bb153086d90f098ad9a3231c9af87a93c6e sidebar.c and symbols.c changes related to document symbols 0d8e52bb54de3982e7adac191a34c622dec177dd Add API for semantic token colorization 7ea99e1ff84d7f2a733eef48606a8992ab666707 Support highlighting using LSP in document.c
I've just re-pushed on top of current master and with additional API for the symbol tree generation and typename colorization plus related implementation changes at various places in Geany and from what I have seen, I think this is all that's need for LSP plugins. All additional features of LSP servers can be implemented independently and don't need any special API.
I'm not a huge fan of this. I use plugins that depend on tagmanager, therefore I cannot use LSP.
I think this going the wrong direction. Now, for example, when "goto definition" is requested we start some IPC or network action which can block and/or fail, and this is going to be a nightmare to debug. We don't know beforehand what will happen, like if the language server even provides any useful information.
Instead I think we should use LSP to augment tagmanager so that we have all the needed bits within our problem space when "goto definition" is triggered. Then we can reason about the result. If tagmanager data store is not suitable we should make the necessary modifications there to make LSP realistic.
Last but not least, I think we should have a unified API towards plugins so that they can work with tags and symbols regardless of LSP or tagmanager (or a combination).
I'm not a huge fan of this. I use plugins that depend on tagmanager, therefore I cannot use LSP.
That's not how it works - TM is still active and running in parallel to the LSP server. It's just the individual GUI features you see that get switched to LSP. You can even combine TM with LSP - in the plugin I made all the individual features possible to disable so you can still use the TM counterparts.
I think this going the wrong direction. Now, for example, when "goto definition" is requested we start some IPC or network action which can block and/or fail, and this is going to be a nightmare to debug. We don't know beforehand what will happen, like if the language server even provides any useful information.
Well, that's how LSP works - if you don't like it, just don't use LSP. I added the possibility to log the full JSON communication either to stdout or to a log file and also the stderr of individual servers and the whole thing is quite easy to debug. Ultimately it isn't Geany's responsibility to debug the IPC communication with the server, it's the plugin's maintainer's (mine) responsibility.
Instead I think we should use LSP to augment tagmanager so that we have all the needed bits within our problem space when "goto definition" is triggered. Then we can reason about the result. If tagmanager data store is not suitable we should make the necessary modifications there to make LSP realistic.
I have absolutely no idea what you mean by this. What do you mean by "reason about the result"? The LSP server just returns the file name and line number and I don't know what "reasoning" you want to do about it.
If you imagine that you somehow get more information about individual symbols and then you implement some advanced logic on top of that then no, this is not possible - LSP won't give you more information than we currently have. For instance for document symbols it offers less info using the (`documentSymbol` request) than we have in TM - it just gives you enough information for a single GUI feature, in this case to display the symbols in the symbol tree. The whole idea of LSP is to behave as a black box completely hiding all the details from editors behind a single interface.
Last but not least, I think we should have a unified API towards plugins so that they can work with tags and symbols regardless of LSP or tagmanager (or a combination).
Plugins can keep using the symbols from the tag manager - LSP is not meant to be used this way and symbols from LSP won't be exposed to plugins (and again, it wouldn't make sense because TM gives you more info than what you can obtain from LSP using its API).
I really suggest having a look at the LSP specification here
https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17...
to understand what it's all about and how it's supposed to be used by editors and what you can expect from it.
OK, TM is still active, but that means I may get different results when using LSP vs TM. I.e. the list of symbols on the GUI may be different to what a plugin gets from quering TM. This potential inconsistency makes me nervous.
This is especially true since LSP may have more build system insights and perhaps knows which ifdefs are active and which are not, leading to some symbols being hidden or different locations for definitions.
Instead I think we should use LSP to augment tagmanager so that we have all the needed bits within our problem space when "goto definition" is triggered. Then we can reason about the result. If TM data store is not suitable we should make the necessary modifications there to make LSP realistic.
I have absolutely no idea what you mean by this. What do you mean by "reason about the result"? The LSP server just returns the file name and line number and I don't know what "reasoning" you want to do about it.
I mean with TM we have all data required to implement the features locally available. We can do smart things before the user triggers things or after based on that. With your LSP proposal we ask the external entity only when the user triggers the action and we have to hope for the best.
I would rather see that we mix-in LSP data (even if that overrides TM) beforehand/in the background. AFAIK you can get all symbols in a document through LSP. Then we can import that to our local data storage and implement goto-definition on-top of that (and here LSP may replace TM's document parsing or we invent some smart combination).
That way other plugins can use LSP data the same way as TM data right now and there's no room for inconsistency.
Basically I'm saying: TM can use LSP data, either exclusively (instead of parsing the file itself) or as an extra data source. Rest of Geany and plugins still interface with TM (may require changes here and there if LSP provides less data and someone was depending on that missing pieces, but I hope not).
@kugel-
I think this going the wrong direction. Now, for example, when "goto definition" is requested we start some IPC or network action which can block and/or fail, and this is going to be a nightmare to debug. We don't know beforehand what will happen, like if the language server even provides any useful information.
I have been using @techee's LSP fork a bit and so far it hasn't failed at all. And on a quick glance ongoing it uses about 10-15% of my CPU (more on file open but only for a short time).
Instead I think we should use LSP to augment tagmanager so that we have all the needed bits within our problem space when "goto definition" is triggered.
That would mean significantly improving TM to contain things like scope of names which it does not handle in any form. The LSP is working with the program AST, its scoping is accurate, TM has none. Do you suggest implementing the AST for all languages in TM?
ATM all IDEs and editors are moving to LSP to leverage accurate information about the program content, including name visibility, handling templates (in C++), inferring types (C++ Python Rust Julia Go).
I think this going the wrong direction. Now, for example, when "goto definition" is requested we start some IPC or network action which can block and/or fail, and this is going to be a nightmare to debug. We don't know beforehand what will happen, like if the language server even provides any useful information.
Instead Geany runs a lower quality parser in between keystrokes and cannot improve it because if it took too much time it would be disruptive to editing.
The fact that LSP runs in a separate process means it does not interfere with the UI, and if it hangs/crashes it can simply be restarted, it won't crash Geany.
Instead I think we should use LSP to augment tagmanager so that we have all the needed bits within our problem space when "goto definition" is triggered. Then we can reason about the result. If tagmanager data store is not suitable we should make the necessary modifications there to make LSP realistic.
Thats not how LSP works, it does not export bulk information like a ctags parser, it answers individual queries, like "goto definition".
@techee
TM gives you more info than what you can obtain from LSP using its API
What extra does TM have? And what it does have is much less accurate information, no scopes, no inferred types (well thats ctags) and no template expansion.
I havn't looked at the detail of the API added, but it should be minimised, all API is a noose around our neck, it is never possible to change it since we have no process for removing it.
I mean with TM we have all data required to implement the features locally available. We can do smart things before the user triggers things or after based on that. With your LSP proposal we ask the external entity only when the user triggers the action and we have to hope for the best.
Do we really do "smart things"? The whole TM is pretty dumb - we just don't have the necessary information to do the right thing. clangd does the "smart things", not we. And we don't have the development resources or knowledge to implement something like that - have a look at how complex clang's AST is:
https://clang.llvm.org/docs/IntroductionToTheClangAST.html
We really don't want to have such a representation directly in Geany; moreover, such a representation is language to language specific and we couldn't have one for all languages.
Asking external entity is what the idea of LSP is all about - LSP servers are typically implemented by the people who develop the compiler itself and who know _much_ better what to do. And best, it will all be work-free for us, we just need to implement the protocol itself and it will work with an arbitrary language. This is like why we wanted the ctags parsers to be maintained outside of Geany - that we don't have to care about them ourselves. And similarly to LSP, if you are not satisfied with some ctags parser or a LSP server, you can submit patches upstream and the great thing is that not only Geany, but all other editors using them will benefit from the work.
I would rather see that we mix-in LSP data (even if that overrides TM) beforehand/in the background. AFAIK you can get all symbols in a document through LSP. Then we can import that to our local data storage and implement goto-definition on-top of that (and here LSP may replace TM's document parsing or we invent some smart combination).
But we won't get the information necessary to implement that - you need the AST for it which you won't get through the LSP interface. LSP just returns e.g. the symbols of the current document but there's no information about symbol types, their visibility, or anything other related. And again, why we should re-invent the wheel when the compiler people did it for us?
Basically I'm saying: TM can use LSP data, either exclusively (instead of parsing the file itself) or as an extra data source. Rest of Geany and plugins still interface with TM (may require changes here and there if LSP provides less data and someone was depending on that missing pieces, but I hope not).
Once again, you misunderstand how LSP servers work and what information they provide - while one could grab the symbols from the LSP `documentSymbols`, they provide less information than what we get from ctags parsers as they are only meant to be displayed in the GUI and aren't meant to provide the necessary information for any advanced processing. In addition, all the communication with LSP servers is asynchronous and we'd really complicate our lives having to worry if all the TM-related structures contain valid data or if there's some LSP request pending.
@techee
TM gives you more info than what you can obtain from LSP using its API What extra does TM have?
I should maybe have said "incompatible" or vaguely described. First, you can't be sure that the LSP server returns all the fields or that it even support certain feature. For instance, the `documentSymbol` call may not be supported at all. Then, some fields of the returned structures may not be returned, for instance, the documentSymbol structure looks this way: ``` export interface DocumentSymbol {
/** * The name of this symbol. Will be displayed in the user interface and * therefore must not be an empty string or a string only consisting of * white spaces. */ name: string;
/** * More detail for this symbol, e.g the signature of a function. */ detail?: string;
/** * The kind of this symbol. */ kind: SymbolKind;
/** * Tags for this document symbol. * * @since 3.16.0 */ tags?: SymbolTag[];
/** * Indicates if this symbol is deprecated. * * @deprecated Use tags instead */ deprecated?: boolean;
/** * The range enclosing this symbol not including leading/trailing whitespace * but everything else like comments. This information is typically used to * determine if the clients cursor is inside the symbol to reveal in the * symbol in the UI. */ range: Range;
/** * The range that should be selected and revealed when this symbol is being * picked, e.g. the name of a function. Must be contained by the `range`. */ selectionRange: Range;
/** * Children of this symbol, e.g. properties of a class. */ children?: DocumentSymbol[]; } ```
All the members with `?` may be omitted. Second, for instance `detail` is defined as ``` More detail for this symbol, e.g the signature of a function. ``` You can't be sure if it contains signature or not - for instance, for C it contains `void (gpointer)` for a void function returning void and taking `gpointer` parameter, for Go it's something like `func(bar int) string` and one would have to implement a LSP-server-specific parser of these fields to get the true signature out of strings like that.
Then the `SymbolKind` kind contains symbol kinds but these are different than the one Geany uses; moreover, depending on the LSP protocol version the server supports, it may contain only a subset of the values so may get different results with different versions of the same LSP server.
First, you can't be sure that the LSP server returns all the fields or that it even support certain feature.
True, but then we don't know exactly what capabilities each ctags parser has, especially regex or peg ones and we seem to get along "ok".
You can't be sure if it contains signature or not - for instance, for C it contains void (gpointer) for a void function returning void and taking gpointer parameter, for Go it's something like func(bar int) string and one would have to implement a LSP-server-specific parser of these fields to get the true signature out of strings like that.
Well, they are signatures, how do you specify a C function, exactly like that `void(gpointer)`, it is rarely used in C I admit, but its used in C++ more `std::function<void(gpointer)>` for example.
From my rather limited (10 mins) go knowledge `func (bar int) string` is the signature of a [function](https://go.dev/ref/spec#Function_types) that takes an `int` and returns a `string`. And when the programmer declared it they probably didn't even specify the `string` and it was inferred.
Probably our ctags based replication of the original declaration has preconditioned to seeing the function name in the signature, but maybe that has to change when returns or variable declarations are inferred.
Then the SymbolKind kind contains symbol kinds but these are different than the one Geany uses; moreover, depending on the LSP protocol version the server supports, it may contain only a subset of the values so may get different results with different versions of the same LSP server.
"Different" as in worse, better, or just ctags ones are more familiar? Anyway as above ctags parsers can also only support a subset.
You seem to misunderstand my point. I'm not at all against LSP, quite the opposite. I like LSP because it seems to be industry standard by now with lots of server-implementations (of varying quality) that loosens our "ctags lock-in".
I just don't like *this* proposal because it "hacks" LSP support into each relevant use case instead of augmenting the existing TM infrastructure with LSP information. It adds individual band-aids for each use case and leaves behind other uses of the TM infrastructure (in other plugins). It's OK to get familiar with LSP and have something to showcase "geany-on-lsp" but it's not something that I comfortably support going forward.
LSP servers ought to provide the symbols for files via `DocumentSymbol` or `SymbolInformation`. That should all be needed to build up a `TMTags` array attached to `TMSourceFile`s (without actually parsing via ctags) [1]. Sure, an LSP might not provide all the bits we want, but as @elextr said that may be the case with poor ctags parsers as well, or this very server instance is not worth asking anyway.
You already implement complex features like symbol tree and call tips and more using LSP so the protocol itself must be sufficiently suitable for us, right? You use `DocumentSymbol` in your reference plugin as well, after all.
[1]: The asynchronous architecture is an obstacle, that's right, but to me that's just necessary refactoring. GIO would allow us to use TM in an async fashion as well. Just need to teach code that tags are not immediately available after `document_open_file_full()` but rather after some "document-tags-ready" signal. That would probably also allow us to improve start-up and project-loading scenarios.
You seem to misunderstand my point. I'm not at all against LSP, quite the opposite. I like LSP because it seems to be industry standard by now with lots of server-implementations (of varying quality) that loosens our "ctags lock-in".
Yet you don't seem to get the point of LSP. The point is to delegate all the "smart" features to the server and let it do the hard work and then only display the results or perform the edits in the document that you obtain from the server. Basically no processing on the client side.
I just don't like this proposal because it "hacks" LSP support into each relevant use case instead of augmenting the existing TM infrastructure with LSP information. It adds individual band-aids for each use case and leaves behind other uses of the TM infrastructure (in other plugins). It's OK to get familiar with LSP and have something to showcase "geany-on-lsp" but it's not something that I comfortably support going forward.
Once again, this is how LSP works - if you want an autocomplete list, you ask the server to provide it for you using the completion request:
https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17...
And you have to do it at the point where it makes sense and disable the TM's autocompletion because you don't want two autocompletions in parallels.
Similarly, if you want go to symbol definition/declaration, you use
https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17... https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17...
and the server returns the right location.
When you want to get information for colorization of types or other symbols, you use
https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17...
For calltips you call
https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17...
For symbol tree, you call
https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17...
And so on.
LSP servers ought to provide the symbols for files via DocumentSymbol or SymbolInformation. That should all be needed to build up a TMTags array attached to TMSourceFiles (without actually parsing via ctags) [1]. Sure, an LSP might not provide all the bits we want, but as @elextr said that may be the case with poor ctags parsers as well, or this very server instance is not worth asking anyway.
You can surely do that but the question is why would you do that. You get identical (or worse) info regarding individual symbols than from TM but enough for the symbol tree (which is the purpose of this particular LSP call). But you can't derive any extra info from it for the "smart" features like goto declaration/definition or autocompletion - you don't get info about types of the symbols, their visibility etc. - this is all hidden in the implementation of the LSP server.
Further, you are not even guaranteed that the server implements the `textDocument/documentSymbol` call in which case you won't implement anything on top of that - yet the server may provide the goto declaration/definition call or autocompletion functionality.
You already implement complex features like symbol tree and call tips and more using LSP so the protocol itself must be sufficiently suitable for us, right?
Symbol tree isn't a complex feature in terms of symbol processing - you just display them (it's just all the annoyance with GtkTreeView). But if you want to implement a _good_ autocompletion or symbol goto taking into account the symbol visibility, you just have to call the corresponding LSP call - without this you'd degrade the whole LSP to the not-so-good TM-style goto or autocompletion and the LSP doesn't make sense here.
And again, the protocol itself is sufficiently suitable - you just have to call the corresponding LSP method for each of the features at the right place which this PR is all about.
---
To be clear, I don't insist the implementation has to look exactly the way I propose here. In the worst case, if this is considered too intrusive, I could probably live with some API that just disables the corresponding Geany features - Matthew did something like that in https://github.com/geany/geany/pull/592. I was just hoping for more native experience - for instance that you can use the same keybindings for symbol goto, tooltips, or autocompletion like the one Geany uses so the plugin doesn't need an extra set of its own keybindings. But I'd probably survive that.
Another thing is the symbol tree which is probably the only feature which works in a more or less same way in LSP as in TM and doesn't bring much benefit compared to TM. So this one could be completely dropped if the patch is considered too big.
The current version of the protocol is a bit overwhelming - for easier understanding what basic calls it offers, I suggest having a look at its first version:
https://github.com/microsoft/language-server-protocol/blob/main/versions/pro...
From my rather limited (10 mins) go knowledge func (bar int) string is the signature of a [function](https://go.dev/ref/spec#Function_types) that takes an int and returns a string. And when the programmer declared it they probably didn't even specify the string and it was inferred.
I was referring to @kugel- 's idea of calling `textDocument/documentSymbol` and placing the result into the TMTag structures. But here we'd have to deal with server-specific meaning of ``` /** * More detail for this symbol, e.g the signature of a function. */ detail?: string; ``` and would have to convert it to the form that the rest of TM expects and I really don't think we should have code like ``` if (clangd) { ... } else if (pylsp) { ... } else if (gopls) { ... } ``` in Geany (moreover, it may even depend on the server version used).
I just don't like this proposal because it "hacks" LSP support into each relevant use case instead of augmenting the existing TM infrastructure with LSP information. It adds individual band-aids for each use case and leaves behind other uses of the TM infrastructure (in other plugins). It's OK to get familiar with LSP and have something to showcase "geany-on-lsp" but it's not something that I comfortably support going forward.
Once again, this is how LSP works - if you want an autocomplete list, you ask the server to provide it for you using the completion request: … Similarly, if you want go to symbol definition/declaration, you use … When you want to get information for colorization of types or other symbols, you use … For calltips you call … For symbol tree, you call
https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17...
And so on.
So you're suggesting that any one editor or IDE may only implement features for that a specialized interface in the LSP exists, and no more? I.e. features that are blessed by the the LSP (=VSCode) folks?
Seems horrid. Maybe if LSP severely limits the features we may implement it's not the right tool for us. Or we use LSP in a way that allows us to implement our features using the interfaces that are available, regardless of how someone else declares "that's how LSP works".
Put another way: Assume I want to implement a "class summary" tab in the sidebar, a HTML rendering of the current class (defined by cursor position), including all members (types, signatures) and their visibility.
I don't see a special `ClassSummary` interface in the LSP spec. Does that mean I cannot implement this feature? What are the options, assuming we didn't have TM anymore.
LSP servers ought to provide the symbols for files via DocumentSymbol or SymbolInformation. That should all be needed to build up a TMTags array attached to TMSourceFiles (without actually parsing via ctags) [1]. Sure, an LSP might not provide all the bits we want, but as @elextr said that may be the case with poor ctags parsers as well, or this very server instance is not worth asking anyway.
You can surely do that but the question is why would you do that. You get identical (or worse) info regarding individual symbols than from TM but enough for the symbol tree (which is the purpose of this particular LSP call). But you can't derive any extra info from it for the "smart" features like goto declaration/definition or autocompletion - you don't get info about types of the symbols, their visibility etc. - this is all hidden in the implementation of the LSP server.
Depending on the language this might give more or more accurate (w.r.t. to build configuration). Probably you can also mix different LSP interfaces to get a more accurate picture about individual symbols.
Further, you are not even guaranteed that the server implements the `textDocument/documentSymbol` call in which case you won't implement anything on top of that - yet the server may provide the goto declaration/definition call or autocompletion functionality.
I truly think that `DocumentSymbol`/`SymbolInformation` is the very basic interface that any worthwhile LSP server implements. How else do you get to learn about any symbol in a file? You need a symbol list to pick one for advanced stuff like goto-definition don't you? So if those aren't implemented the server is useless. Do you know an example of such a limited server?
You already implement complex features like symbol tree and call tips and more using LSP so the protocol itself must be sufficiently suitable for us, right?
Symbol tree isn't a complex feature in terms of symbol processing - you just display them (it's just all the annoyance with GtkTreeView). But if you want to implement a _good_ autocompletion or symbol goto taking into account the symbol visibility, you just have to call the corresponding LSP call - without this you'd degrade the whole LSP to the not-so-good TM-style goto or autocompletion and the LSP doesn't make sense here.
Why do you think I want to take visibility into account for go-to? That would mean I cannot go to the definition of a private member function from a foreign class. Yes, that code may be illegal, but in the editor I still want to perform that jump to resolve the situation, e.g. by moving the function to the public section. Does the LSP interface even allow that or am I constrained to what LSP thinks how my implementation of goto-definition should work?
My point is: It may be nice that LSP provides specialized interface for some features. But it doesn't do for all imaginable features, and the existing interface might also not fit perfectly to our requirements. So we have to think about fallback solutions by mixing multiple existing LSP interfaces.
NB: I did not find in the LSP spec that goto-definition takes the visibility into account. What does goto-definition add over DocumentSymbol anyway? The latter includes the symbol location and goto-definition doesn't return anything else.
Actually, the LSP interface for goto-definition seems take to be a position in the text file (probably a call site) as input. That implies that this interface is not suitable when you don't come from a document context. For example, one of my plugins offers a dialog where you can enter a function name, in order to look it up and then go to it's definition. Seems the goto-definition LSP interface cannot be used for that purpose, meaning I have to look-up `DocumentSymbol`. Also you can click on a symbol in the sidebar to jump to its definition. How do you handle that?
At least for things like these we're back to square one: We do need an offline database for symbols even for things like goto-definition.
To be clear, I don't insist the implementation has to look exactly the way I propose here.
I care mainly that the TM infrastructure is still intact. And not because we still parse the file with ctags but in a way that ensures consistency with LSP data.
Assuming we fill TM using LSP data, then this consistency is given. We can then decide if we use that or use a specialized LSP interface to implement feature X or Y. But we do have to ensure that we have a solid and consistent TM infrastructure at all times.
Another thing is the symbol tree which is probably the only feature which works in a more or less same way in LSP as in TM and doesn't bring much benefit compared to TM. So this one could be completely dropped if the patch is considered too big.
That's worst. Then symbol tree and everything else is based on ctags-parsed data and actual user-interaction features are based on external LSP data. Unless both ctags parser and language server are high quality and give comparaible results this will just give the user confusing (because inconsistent) results if both worlds don't match well (e.g. if the ctags parser is too simple/limited). E.g. you may get a fine call tip for symbols that aren't visible in the sidebar or elsewhere.
@kugel- I'm really getting tired of these endless discussions that don't lead anywhere. On the one hand you say "I like LSP because it seems to be industry standard by now with lots of server-implementations", on the other hand you say "Maybe if LSP severely limits the features we may implement it's not the right tool for us". You should make your mind what you want. You are inventing some artificial reasons why it shouldn't work but the reality is that lots of editors use it and it works. My plugin works as well, try it. Period.
Let me put it very clearly: **I am only interested in implementing a LSP plugin in a way that the LSP interface is supposed to be used**. I'm not interested in any Frankenstein LSP implementation where we for some unknown reasons ignore the features LSP offers and implement it in a worse way because the only blessed interface is `textDocument/documentSymbol` which doesn't provide all we need. Please notice the **I** in the sentence - this doesn't mean that someone else cannot try to do something else, I just don't want to waste my time on that. Feel free to e.g. fork the plugin I'm developing and doing what you want.
@b4n @eht16 @elextr What's your opinion on this? This really affects whether I should keep developing the plugin. Using LSP only as an alternative source of tags doesn't make sense to me because it won't bring any better autocompletion or symbol goto so why bother. The interface as proposed in this PR doesn't have to stay this way and the scope can be reduced, but as the minimum I need to be able to disable some Geany's features to avoid e.g. two autocomplete popups, one from TM, one from LSP.
I like LSP in general. I have a problem with your narrow view ("in a way that the LSP interface is supposed to be used", whoever defines what "supposed to be used" means) that leaves the TM infrastructure behind for anyone that depends on it. TM is still active but there is no provision that TMTags is anywhere consistent with LSP data. And the user is going to interact with both backends in your current design.
What you call "Frankenstein LSP" is my attempt to envision an LSP integration that does not regress those other use case by ensuring a consistent backend.
Please answer these two questions: - How do you plan to handle other various goto-definition cases that don't come from a document context? - And for call tips (mouse over in sidebar symbols)?
Unless I'm reading the LSP spec wrong you cannot use the corresponding LSP interfaces for those. And if you still rely on current TM (ctags parsing) for those then *that's* Frankenstein.
Also, AFAICS your own ProjectOrganizer interfaces heavily with TM. What's your plan on that front? You aren't ending support for that plugin, are you?
Using LSP only as an alternative source of tags doesn't make sense to me because it won't bring any better autocompletion or symbol goto so why bother.
Sorry, I didn't want to suggest to only grab tags from LSP. My point is that we should grab tags from LSP at a minimum (and maybe use LSP in other opportunities additionally). For example, we can probably use the goto-definition LSP interface where possible (i.e. ctrl+click in the editor). I guess it has the advantage that the LSP can infer whatever foo is in `foo->func()` and then jump to the right definition, instead of presenting the user the various `func()` overloads to chose from.
But we must make sure that other goto-defintion use-cases (no document context) are also backed by LSP data and not by a completely different parser that perhaps only parses half of the document compared to the language server. Otherwise the LSP integration is going to be a source of user frustration and bug reports.
I'm not sure if I want to continue in this but one more round...
So you're suggesting that any one editor or IDE may only implement features for that a specialized interface in the LSP exists, and no more? I.e. features that are blessed by the the LSP (=VSCode) folks?
Anyone can implement anything of course but LSP servers are specialized in doing certain things (and in the case of clangd, for instance, significantly better than what we could possibly ever produce), why not use them? And of course we fall back to our TM otherwise.
I don't see a special ClassSummary interface in the LSP spec. Does that mean I cannot implement this feature? What are the options, assuming we didn't have TM anymore.
I don't know what exactly you want to see in this class summary, it sounds like you could obtain the necessary information
https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17... https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17...
But just displaying symbols is easy, you don't need anything special for that and something like our TM is mostly enough for it. What's hard is good autocompletion, symbol goto, or calltips showing only relevant info.
Depending on the language this might give more or more accurate (w.r.t. to build configuration). Probably you can also mix different LSP interfaces to get a more accurate picture about individual symbols.
But why would I mix different LSP interfaces when there are specific interfaces for the purposes we need? That's nuts.
My point is: It may be nice that LSP provides specialized interface for some features. But it doesn't do for all imaginable features, and the existing interface might also not fit perfectly to our requirements. So we have to think about fallback solutions by mixing multiple existing LSP interfaces.
I don't know what "all imaginable features" you have in mind. We definitely won't implement "all imaginable features". We want good autocompletion, we want good symbol goto, we want good calltips. All these are provided by the LSP interface. And if you want something special (and please tell me what exactly you want), sure, you can implement it using a combination of calls (but I don't know how you combine autocompletion with symbol goto together with calltips into one thing and what you expect from it).
NB: I did not find in the LSP spec that goto-definition takes the visibility into account. What does goto-definition add over DocumentSymbol anyway? The latter includes the symbol location and goto-definition doesn't return anything else.
That's not documented (it's irrelevant to the interface itself), it's just how e.g. clangd works - it goes to the right spot e.g. based on what you included in the given file. The difference between documentSymbols is that documentSymbols returns all symbols of the current file while definition gives you the location where to jump across the whole project with all the bells and whistles like taking into account class hierarchies, includes, etc.
Actually, the LSP interface for goto-definition seems take to be a position in the text file (probably a call site) as input. That implies that this interface is not suitable when you don't come from a document context. For example, one of my plugins offers a dialog where you can enter a function name, in order to look it up and then go to it's definition. Seems the goto-definition LSP interface cannot be used for that purpose, meaning I have to look-up DocumentSymbol. Also you can click on a symbol in the sidebar to jump to its definition. How do you handle that?
You could use `workspaceSymbols` if you want to do it for the whole project
https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17...
or `documentSymbols`
https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17...
and filter-out the symbols you are interested in. But this is simple, it doesn't any advanced logic like goto based on the current location.
At least for things like these we're back to square one: We do need an offline database for symbols even for things like goto-definition.
And nobody says we should drop the TM symbols, I'm just saying that if you imagine you get some better symbol information from LSP calls like the two above than what we get from TM, you are mistaken. There may always be files that get mis-parsed by the relatively simple TM parser so regarding parsing LSP will probably have the edge over TM but the information inside the symbols themselves will probably be inferior to what we have inside our TMTag structure.
As I mentioned here https://github.com/geany/geany/pull/3571#issuecomment-1793502926, you can't rely on the exact contents of the `detail` field unless you want to hard-code some rules specific to various LSP servers (and I'm _very_ against it) so without it you lose the type and the signature of variables/functions. I also think that many servers return just a flat list without `hierarchicalDocumentSymbolSupport` so you lose scope. What remains is symbol name, kind (which doesn't exactly correspond to our TMTagType and we can't even be sure which ones the server supports), line number and that's pretty much it.
I care mainly that the TM infrastructure is still intact. And not because we still parse the file with ctags but in a way that ensures consistency with LSP data. Assuming we fill TM using LSP data, then this consistency is given. We can then decide if we use that or use a specialized LSP interface to implement feature X or Y. But we do have to ensure that we have a solid and consistent TM infrastructure at all times.
You are not guaranteed to have that even in this case. When you consider pylsp:
https://github.com/python-lsp/python-lsp-server
it uses various external tools for various features and I doubt it will always behave in a consistent way.
That's worst. Then symbol tree and everything else is based on ctags-parsed data and actual user-interaction features are based on external LSP data. Unless both ctags parser and language server are high quality and give comparaible results this will just give the user confusing (because inconsistent) results if both worlds don't match well (e.g. if the ctags parser is too simple/limited). E.g. you may get a fine call tip for symbols that aren't visible in the sidebar or elsewhere.
Well, I would always prefer a good calltip over a bad calltip that is consistent with the list of symbols. And as I said above, if you want to fill the TMTag structures from LSP, you can't completely rely on what's in `detail` so you probably won't fill-in `signature` in TMTag and will lose it in the symbol tree anyway.
@kugel- as @techee said, but more bluntly, you do not seem to understand what the Language Server Protocol (LSP, not Language Server Process for removal of doubt) is designed for and are making yourself look stupid by arguing without that knowledge. Please stop being disruptive, if you have suggestions to improve the implementation please make them, but I feel your "change it all" is not helpful.
The LSP is designed for the server to hold a current version of the semantic annotated abstract syntax tree for the program, and accurately answer questions using that. There is no API (that I can see) that allows that to be exported, just a flat or hierarchical list of symbols suitable for the sidebar.
The approach of having Geany call a plugin for specific functionality, or fallback to its own, is in principle a good one. Just that Geany has never called a plugin before so its a new concept and has no design yet. Maybe we need to think about it more generally. But it allows for progressive development as individual items of functionality can be converted one at a time and features that the LSP does not support can also fallback to the existing implementation. And if it all goes pear shaped most of the code is in the plugin, not Geany, so it can be removed relatively easily or an alternate implementation plugged in. The alternative is to build LSP into Geany and be much more disruptive.
To answer your last questions:
How do you plan to handle other various goto-definition cases that don't come from a document context?
The LSP works on a project basis, in the case of clangd it uses `compile_commands.json` to specify build data in a build tool independent manner. I read your question to be what happens for files that are outside the project? Basically clangd does its best without reading the include files that are not open (since it does not know where to find them), but that is nowhere near as good as a file addressed by a `compile_commands.json`.
And for call tips (mouse over in sidebar symbols)?
The servers can return an accurate function _type_ for the function. As @techee and I discussed elsewhere, it is not just a copy of the declaration text that ctags provides, since it contains inferred information as well, so its better, but different. This would be the correct information to show but it is not implemented yet.
I guess it has the advantage that the LSP can infer whatever foo is in foo->func() and then jump to the right definition, instead of presenting the user the various func() overloads to chose from.
Yes, this is what I mean by "accurate", no need to show a go to list, "please Mr user you choose", the correct one is available and can just be switched to directly.
But we must make sure that other goto-defintion use-cases (no document context) are also backed by LSP data and not by a completely different parser that perhaps only parses half of the document compared to the language server. Otherwise the LSP integration is going to be a source of user frustration and bug reports.
As I said clangd seems to do its best for files without `compile_commands.json` and it seems to also check the current directory just in case, so flat projects like Geany will "just work". But it depends on the server, and of course for "modern" languages other mechanisms may be used.
But ultimately neither we nor any other IDE (even Vscode) control what the server writers provide, thats why servers provide capability handshakes so features not supported can fallback to whatever the IDE chooses, in which case TM or nothing is the choice.
Maybe one thing to add which I didn't mention previously because I didn't want the discussion to become too off-topic. But this is one of those PRs where the discussion got out of control so to hell with it :-).
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. I don't suggest it's something we should start working on, just one possibility to keep in mind. There would be several advantages of this: 1. ctags parsers would live outside Geany's process so they wouldn't crash Geany, Geany could just restart the server if it happens. 2. All communication would be asynchronous so ctags parsing wouldn't block Geany. 3. The ctags process would probably be scheduled on another CPU core so there could be a slight performance gain when both Geany does its stuff and the files are parsed in parallel (still, ctags isn't reentrant so no parallel parsing within the LSP process). 4. ctags LSP could be reused by any editor - basically all editors would get the basic support of many languages we offer in Geany using TM for free. 5. The interface would be just plain LSP so swapping ctags LSP for some other LSP like clangd wouldn't mean any change in the code.
The main disadvantages I see are: 1. It would break the plugin API depending on the tag manager. It could be modified so plugins can access LSP directly. 2. It would require some LSP plugin to be part of Geany 3. This would require a dependency on some JSON-RPC library for the core Geany 4. We would have to agree we want to go this way, an impossible task ;-) 5. And one small detail, someone would have to write all this, review it, etc.
Once again, I'm not proposing we should start doing that, I'm just recommending it as a thought exercise.
@techee See #3675, ideas welcome.
@techee
I don't know what exactly you want to see in this class summary, it sounds like you could obtain the necessary information
We don't have to deep dive into that. I just made up an example that doesn't have a specialized LSP interface. Your reply was expected: Use other interfaces (maybe multiple ones) that provide the necessary information. This is alright. I just want to get past the "use only LSP interfaces for their original intention" argument because it would stop you from implementing new features until a specialized LSP interface arrives.
But why would I mix different LSP interfaces when there are specific interfaces for the purposes we need? That's nuts.
Because, for example the goto-definition LSP interfaces does not work in all situations (needs a source code location as input, see below). When it's applicable then it should be preferred, that's for sure.
And nobody says we should drop the TM symbols
I think we're clear on that, nobody suggests that. We're disagreeing whether LSP data should augment the TM-ctags-based fallback. I maintain it should be done so that use cases that require the fallback don't regress or remain unavailable entirely.
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.
I think that's a fine idea and it goes much in my direction because it ensures that we can realize all existing functionality over LSP (unless you want to drop some functionality in the process?). Lets discuss that idea elsewhere, though, this PR is already becoming confusing.
@elextr
@kugel- as @techee said, but more bluntly, you do not seem to understand what the [Language Server Protocol](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17...) (LSP, not Language Server Process for removal of doubt) is designed for and are making yourself look stupid by arguing without that knowledge. Please stop being disruptive, if you have suggestions to improve the implementation please make them, but I feel your "change it all" is not helpful.
Sorry if I sound stupid but I don't really care what LSP was originally designed for. I care if we can make LSP and Geany fit together. Perhaps that may require using LSP interfaces for other purposes than their original intention. I don't want to turn Geany up-side-down just to make it fit the LSP design (at least not all at once). This is why I want to keep TM infrastructure working *and* consistent when LSP is in effect.
I'm not even too attached to the ctags parsing itself. But lots of functionality requires TM even with this PR and I don't want to lose it if LSP is in effect, and ideally I want to have this functionality for languages where LSP is the only option (where we have no parser).
The way I use Geany depends on TM but I too want to benefit from LSP. Is that hard to understand?
The LSP is designed for the server to hold a current version of the semantic annotated abstract syntax tree for the program, and accurately answer questions using that. There is no API (that I can see) that allows that to be exported, just a flat or hierarchical list of symbols suitable for the sidebar.
To my understanding, LSP just exposes interfaces that give information about the documents or projects. Whether the server keeps an AST is an implementation detail and the server may have other means to provide the interfaces.
The approach of having Geany call a plugin for specific functionality, or fallback to its own, is in principle a good one. Just that Geany has never called a plugin before so its a new concept and has no design yet. Maybe we need to think about it more generally. But it allows for progressive development as individual items of functionality can be converted one at a time and features that the LSP does not support can also fallback to the existing implementation. And if it all goes pear shaped most of the code is in the plugin, not Geany, so it can be removed relatively easily or an alternate implementation plugged in. The alternative is to build LSP into Geany and be much more disruptive.
I don't disagree. But entire point is that the "fallback to its own" is consistent with LSP data. I don't want use cases that cannot be mapped to LSP interfaces to regress when LSP is in effect. Because I think 1) LSP support is not going to be successful if it creates regressions on other areas and 2) it creates support nightmares because users may run on inconsistent data backends. The "if you use an LSP plugin you're on your own" excuse does not work always if we support a plugin API specifically for that and the issues get created anyway.
To answer your last questions:
How do you plan to handle other various goto-definition cases that don't come from a document context?
The LSP works on a project basis, in the case of clangd it uses `compile_commands.json` to specify build data in a build tool independent manner. I read your question to be what happens for files that are outside the project? Basically clangd does its best without reading the include files that are not open (since it does not know where to find them), but that is nowhere near as good as a file addressed by a `compile_commands.json`.
And for call tips (mouse over in sidebar symbols)?
The servers can return an accurate function _type_ for the function. As @techee and I discussed elsewhere, it is not just a copy of the declaration text that ctags provides, since it contains inferred information as well, so its better, but different. This would be the correct information to show but it is not implemented yet.
No, I don't mean "outside files". I mean UI interactions that are not tied to a document or an exact source code location. Both [Goto Implementation](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17...) and [Signature Help](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17...) interfaces have the current cursor position as input. I can't directly request the location or signature of function `foo()`, I do need to write the call to a source file so that the language server can read the code and infer context from that source code location.
Example: The sidebar symbols both offer goto-implementation (click on a function) and calltips (mouse-over tooltip). But in the sidebar you don't have a cursor position. I do not see how you can possibly implement the two functionalities using the LSP interfaces designed for that purposes, but using the intended interfaces - and only those - is @techee entire argument.
So to my understanding both sidebar features have to be implemented using the fallback. But the fallback, as proposed in this PR, relies entirely on TM backed by ctags parsing. Now we can have the weird situation that the sidebar lists symbols that the fallback does not know about, thus goto-implementation and calltips on the sidebar might not be available. The other way around is also plausible (although unlikely): the ctags parser may have parsed more symbols (or more modifiers) than the LSP server and the sidebar wouldn't list them even if Geany knows about them.
And this is the exact situation that I do not want to happen. I think we can achieve that if we try a little better by also mixing LSP data into the TM fallback, as a best effort solution to keep the TM fallback and primary LSP data source consistent.
I can believe that you or @techee don't care about goto-implementation and calltips enough on the sidebar but I do.
I guess it has the advantage that the LSP can infer whatever foo is in foo->func() and then jump to the right definition, instead of presenting the user the various func() overloads to chose from.
Yes, this is what I mean by "accurate", no need to show a go to list, "please Mr user you choose", the correct one is available and can just be switched to directly.
Hopefully it won't do nothing if I accidentally call a private function that is not available from the call site in question. I still want to jump to the implementation in that case.
The sidebar symbols both offer goto-implementation (click on a function) and calltips (mouse-over tooltip). But in the sidebar you don't have a cursor position. I do not see how you can possibly implement the two functionalities using the LSP interfaces designed for that purposes, but using the intended interfaces - and only those - is [@techee](https://github.com/techee) entire argument.
So to my understanding both sidebar features have to be implemented using the fallback.
I think you misunderstand the purpose of the goto-implementation, signature and documentSymbol calls: - goto-implementation: this is only meant to be used for control-click on an identifier in the editor (or using a keybinding for the symbol at the cursor position) - then, based on the identifier location in the current file, the server tries to find where that symbol is defined. clang is pretty good at that and really returns the right one, taking into account includes, type hierarchies etc. This is something that is extremely hard to do without full AST information. - signature - like goto-implementation, this is triggered e.g. by `(` and the LSP server evaluates the identifier in front of `(`, its visibility from the precise location in the editor, and returns the best possible calltip. Again, extremely hard to do without AST. - autocompletion - like above
Now documentSymbol: - returns current document's symbols, their locations, the "detail" field I mentioned several times above and kind of the symbol. These are trivial to interpret so if one wants to display "detail" as calltip, one can do it (I do it with my plugin for the sidebar). Or go to symbol for this list is also trivial to implement. There's no need for server assistance here and LSP leaves these trivial things on clients. This is no "fallback" as you call it, you can do whatever you wish with the symbols obtained using this call.
But the fallback, as proposed in this PR, relies entirely on TM backed by ctags parsing.
No, this PR uses `documentSymbol` symbols for the sidebar (shows symbol name in the tree, shows "detail" as the calltip, generates tree based on children and assigns icon based on kind). I was just suggesting that if this PR is considered too big, I could sacrifice this LSP feature and could live with the TM implementation.
I can believe that you or @techee don't care about goto-implementation and calltips enough on the sidebar but I do.
But this works, I really don't know what you are talking about. Have you actually tried the bloody plugin? It's a few minutes of your time to install and test it - would really save hours of pointless discussions.
Hopefully it won't do nothing if I accidentally call a private function that is not available from the call site in question. I still want to jump to the implementation in that case.
Try it and see how clangd behaves. I would expect it to be reasonably fuzzy. When we mention "accurate", we mean that when you have hundreds of methods called `get()` in your C++ project, the goto will jump to the right one instead of showing a list of 100 `get`s.
So to my understanding both sidebar features have to be implemented using the fallback.
I think you misunderstand the purpose of the goto-implementation, signature and documentSymbol calls:
I think understand the intention of the LSP interfaces. That's why I'm pretty clear that they cannot be used itself for implementing goto-definition and tooltip on the symbols sidebar. These functionalities in Geany have to be implemented using other LSP interfaces or the TM fallback.
I now looked at your changes in more detail and found that you accordingly don't use the goto-implementation and signature interfaces for the sidebar. My previous understanding of the PR was that these sidebar features are implemented using the TM fallback (`doc->tm_file->tags_array`) but that was a false understanding. Sorry for that!
But the fallback, as proposed in this PR, relies entirely on TM backed by ctags parsing.
No, this PR uses `documentSymbol` symbols for the sidebar (shows symbol name in the tree, shows "detail" as the calltip, generates tree based on children and assigns icon based on kind). I was just suggesting that if this PR is considered too big, I could sacrifice this LSP feature and could live with the TM implementation.
But this is essentially what I want that we do generally. I looked at your code in more detail and you're actually doing what I'm asking for all the time: you're generating `TMTag`s from the LSP data. Now can we not go one step further and not only attach these tags to the sidebar menu items but to the GeanyDocument itself?
And please don't re-iterate the "documentSymbol" support is not guaranteed. To me this is basic functionality for any language server. If that's not provided the server is worthless (we couldn't have the symbols sidebar at all!). Please show me one server that does not provide it at all.
The *quality* of the implementation is a different matter but that's no different to our ctags parser. These also vary widely, not just in terms of parser results but also performance. We are never guaranteed good support for some languages. And this is something we accept and get along with. It's always "best effort". The same applies to whatever language server we talk to.
You said that just documentSymbol might give inferior results for `doc->tm_file->tags_array`, compared to our ctags parsers. Is that always true or does it depend on the server? What about C/C++ and the `clangd` server for example? I want to get a feeling if the documentSymbol interface itself is lacking or if existing servers aren't good enough.
Speaking about superior ctags parsing: I understand your implementation is susceptible for this scenario for the sidebar use cases: If the language server does not provide `detail` in documentSymbol, then we won't have a tooltip. Similarly, if the server does not provide range/location then we won't have goto-definition. Now if we would have that information in the TM fallback because the parser recorded this information in `doc->tm_file->tags_array`, then these two use cases are effectively regressed. I don't have a feeling how realistic this is because I don't have a good picture about the quality of server implementations, but generelly speaking this is a situation that I want to avoid if possible as it would make our LSP support look poor compared to the status quo, even if it excels in other use cases.
On the other hand, if we follow-up on your other idea and use LSP even to interface with the ctags parsers then documentSymbol *ought to be* sufficient for all existing features (except maybe where we can specialized LSP interfaces like goto-implementation), right?
I can believe that you or @techee don't care about goto-implementation and calltips enough on the sidebar but I do.
But this works, I really don't know what you are talking about. Have you actually tried the bloody plugin? It's a few minutes of your time to install and test it - would really save hours of pointless discussions.
I never questioned that your plugin works, at least for C/C++ where we have a solid TM-based fallback as well. I could find the above only by code study not just by testing.
Actually testing requires me to setup an LSP server (or perhaps multiple ones) which I don't yet know how to do so I'm afraid it takes more than a few minutes of my time.
@kugel- just to be clear, silly relates to ideas or suggestions, not you yourself.
And to be clear about what might be cultural context, "silly" is used as a single word to represent the phrase "does not make sense in the current context".
However one thing that does relate to you is the approach and attitude. You come across as very aggressive and demanding. That may be a language proficiency thing, but your English is generally good so its hard not to make that interpretation. You also say a lot of "I want" "I use" ignoring the fact that many others want and use different things, I could just as easily say "All I care about is C++ symbols working accurately, who cares about TM". This approach will get us nowhere. Neither you nor I "own" the project.
And an aggressive response targeting niche or corner cases is very disheartening to those who have spent their time to propose a significant improvement of core features.
Note, @techee has made every function that uses LSP configurable, so if its not supported by the server it can fall back to TM or be disabled by the user.
This is why I want to keep TM infrastructure working and consistent when LSP is in effect.
Please propose how you will make synchronous TM work with asynchronous LSP servers. There is a fundamental divide which is why @techee handled it by having individual user facing functions use either TM or LSP, they have to adapt to the API paradigm, asynchronous LSP is not a drop in replacement of synchronous TM.
I'm not even too attached to the ctags parsing itself. But lots of functionality requires TM even with this PR and I don't want to lose it if LSP is in effect, and ideally I want to have this functionality for languages where LSP is the only option (where we have no parser).
This is one of the silly statements, to have a function you need either a ctags parser or an LSP server that supports it. If the language has an LSP server the available set of capabilities will always be better than no ctags parser.
The way I use Geany depends on TM but I too want to benefit from LSP. Is that hard to understand?
Yes it is hard to understand, please detail your use-case.
Actually testing requires me to setup an LSP server (or perhaps multiple ones) which I don't yet know how to do so I'm afraid it takes more than a few minutes of my time.
C'mon, thats a poor excuse, the readme on geany-lsp tells you `sudo apt install clangd` and the other supported servers. You have several times said to others "Why don't you just try it" in relation to your own PRs so it seems reasonable for others to expect the same from you.
@techee said: I don't know what exactly you want to see in this class summary, it sounds like you could obtain the necessary information
@kugel- reply: We don't have to deep dive into that. I just made up an example that doesn't have a specialized LSP interface.
Making up blue sky functionality that Geany does not support as a reason to attack a PR is not acceptable, it is simply disruptive behaviour that adds nothing to the discussion. Please do not do it again. If you want to _propose_ blue sky then do it on #3675.
To my understanding, LSP just exposes interfaces that give information about the documents or projects. Whether the server keeps an AST is an implementation detail and the server may have other means to provide the interfaces.
Ok, "AST or other means" although in reality thats what they do. But my point was that the server has some form of additional information that TM does not support and so provides answers to the questions with an accuracy that TM cannot. The whole point of this PR is to allow the use of LSP servers to get that accuracy, exporting lower accuracy information to TM and then expecting it to answer the question is a silly idea.
The "if you use an LSP plugin you're on your own" excuse does not work always if we support a plugin API specifically for that and the issues get created anyway.
Nobody said that, and its no different to Scintilla lexers or ctags parsers the Geany project does not maintain.
I think understand the intention of the LSP interfaces. That's why I'm pretty clear that they cannot be used itself for implementing goto-definition and tooltip on the symbols sidebar. These functionalities in Geany have to be implemented using other LSP interfaces or the TM fallback.
But this is not how this discussion started, just re-read your response here: https://github.com/geany/geany/pull/3571#issuecomment-1793149958. If it's clear now, great.
But this is essentially what I want that we do generally. I looked at your code in more detail and you're actually doing what I'm asking for all the time: you're generating TMTags from the LSP data. Now can we not go one step further and not only attach these tags to the sidebar menu items but to the GeanyDocument itself?
I understand your point but there are many related problems: 1. What do you want to take as the source of truth - tag manager, or LSP server? E.g., if one tag is present using tag manager and not when using LSP server, what is correct? Did the tag manager misparse the file and inserted an incorrect tag or did the LSP server misparse it and forgot about the tag? Or replace all tags from a file with the LSP ones (losing TMTag info which, as I understand, is not what you want). 2. You cannot be sure that the LSP server parses correctly files that are not part of the project. For instance, clangd depends on `compile_commands.json` you get from meson - other files might compile somehow but you get errors. Now tag manager has a file-based API like `tm_workspace_add_source_file()` but you cannot do something like that with LSP. So the result will be a mix of tags, some coming from TM and some from LSP in the tag manager and I'm not sure if this is what we want. 3. Now if you wanted to replace all symbols in TM and not just those in the currently edited file, you'd probably have to call https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17... and place symbols from there to the TM. But when some of the workspace files change (e.g. after doing `git pull`), you won't get any notification from the LSP server and you get outdated tags in the database. (I think this is an important point that you still don't completely get - you still want to have LSP tags cached locally but this is not how LSP works. Tags are placed in the server and you perform the queries only when you need some functionality so you always get up-to-date results.) 4. When you have a look what I did for the symbol tree (and note, I'm not proud of it and I definitely want some feedback here), I placed `detail` from LSP to `arglist` of TMTag and LSP `kind` to `type` in TMTag and process them differently in the symbol tree than TM tags. We already discussed the `detail` problem which isn't semantically equal to `arglist` in TM and would require further processing depending on the language server used. Now there's also the `kind` problem: we can't map it to arbitrary TMTagType because we have hard-coded mappings here https://github.com/geany/geany/blob/b02ee9a95a48690836f08ba10ccc9db89ee89c10... which tell us e.g. what icon to use for the given TMTagType or to which group they belong in the sidebar. This is language-specific and if we wanted to map LSP kind to TMTagType, we'd have to do it in a language-specific way which would introduce another set of tables and this is something I'd like to avoid. So instead, what I did was that I placed `kind` to `type` and introduces a fixed mapping `kind->icon` that is used for all languages with special code for this in the symbol tree. And also, disabled "group by type" when LSP is used.
Solving the above points would be really hard and actually impossible for some things. So I'm still not convinced that updating TMTags in TM is the way to go.
And please don't re-iterate the "documentSymbol" support is not guaranteed. To me this is basic functionality for any language server. If that's not provided the server is worthless (we couldn't have the symbols sidebar at all!). Please show me one server that does not provide it at all.
Just have a look here:
lots of language servers work as linters or code formatters only. That said, these don't matter to us symbol-wise. I was only mentioning that before because you seemed to insist that goto symbol and friends should be implemented only using symbols, but we are past that I hope :-),
I want to get a feeling if the documentSymbol interface itself is lacking or if existing servers aren't good enough.
I would call it more "incompatible interface" to what we do with TMTags, see point (4) above. We for instance have `var_type` and `arglist`, LSP has `detail` which is so vaguely described that one cannot be sure what we'd get from it. It's enough to show it as a tooltip in the symbol tree but we cannot be sure we derive `var_type` and `arglist` from it and this would need server-specific parsing of the field. Similar problem with the `kind` discussed above.
This again comes to the way LSP works and that its features are intended to be used - the interface is meant for you to show the symbols e.g. in a symbol tree or some popup window, filter it and allow you to jump to the specific place, but they are not meant to be used for e.g. some client-side autocompletion.
Speaking about superior ctags parsing: I understand your implementation is susceptible for this scenario for the sidebar use cases: If the language server does not provide detail in documentSymbol, then we won't have a tooltip.
Correct. But you can always disable LSP for this particular feature if you run into some shitty LSP server where our ctags parsers work better.
Similarly, if the server does not provide range/location then we won't have goto-definition.
Notice that there's no `?` at these members so these are mandatory for the LSP interface.
Now if we would have that information in the TM fallback because the parser recorded this information in doc->tm_file->tags_array, then these two use cases are effectively regressed. I don't have a feeling how realistic this is because I don't have a good picture about the quality of server implementations, but generelly speaking this is a situation that I want to avoid if possible as it would make our LSP support look poor compared to the status quo, even if it excels in other use cases.
Basically it's up to the user to evaluate the individual language servers. If they suck, they'd be probably happier with our TM implementation and could disable e.g. the `documentSymbols` feature for that particular language server and will get pure TM as the fallback. I'm really worried that if you somehow merge the information from TM with the information from the LSP, the result will be even worse, i.e. `bad_ctags_parser + bad_lsp` is worse than `bad_ctags_parser` or `bad_lsp` alone.
On the other hand, if we follow-up on your other idea and use LSP even to interface with the ctags parsers then documentSymbol ought to be sufficient for all existing features (except maybe where we can specialized LSP interfaces like goto-implementation), right?
I was thinking about it and not to lose functionality, I think we'd have to introduce some Geany-specific members like `_geany_var_type`. It sucks a bit and I haven't checked the LSP specification in detail if it explicitly prohibits something like that but I think even with extra members we should be LSP-compliant.
Now again, I'm not really suggesting we should do that and I think nobody has time to do it in the short term anyway.
Actually testing requires me to setup an LSP server (or perhaps multiple ones) which I don't yet know how to do so I'm afraid it takes more than a few minutes of my time.
See https://github.com/techee/geany-lsp and README.md at its bottom. If you are using a debian-based OS, it also tells you how to install the required packages.
However one thing that does relate to you is the approach and attitude. You come across as very aggressive and demanding.
That may be a language proficiency thing, but your English is generally good so its hard not to make that interpretation. You also say a lot of "I want" "I use" ignoring the fact that many others want and use different things, I could just as easily say "All I care about is C++ symbols working accurately, who cares about TM". This approach will get us nowhere. Neither you nor I "own" the project.
@kugel- Maybe a small note from myself. I'm also not completely happy about the way you respond. Maybe not in this particular PR but some past ones. It's clear that everyone has his own preferences but we don't live in a black and white world where one thing is clearly right and one is clearly wrong - everything has its pros and cons. I'm also for good discussion which is backed by valid arguments but in your case I have a feeling you get stuck at one position and despite how valid counter-arguments one makes, you seem to invent more and more artificial reasons to support your argument or stop responding. This is tedious, time consuming and plain discouraging.
For instance, we had this famous Windows theme discussion. Your argument was to use as native theme as possible which I completely agree with. The reality, though, was that none of the themes looked native (GTK2 was FANTASTIC in this respect, sigh) - I went through something like 15 themes, installed them on Windows but nothing seemed right. You claimed that the Windows-10 theme looked native but neither me nor Enrico agreed. When I asked you about what you were seeing and posted some screenshots here and in the subsequent post
https://github.com/geany/geany/issues/3063#issuecomment-1046806139
you didn't bother answer. I suggested the Prof-Gnome theme as a workable rather neutral theme with sufficient contrast and not so big widgets - not ideal, but I didn't find an ideal theme, neither you suggested an ideal theme. Then Enrico posted
https://github.com/geany/geany/pull/3129#issuecomment-1046331528
and you just NACKed it without providing a better alternative.
Now I found it extremely rude. I think neither you nor me are people who'd be entitled to hard-NACK someone's PRs this way - I think only Enrico as the creator of the editor and Colomban as its current maintainer are. And I've never seen them rejecting PRs in such a way - first, they'd offer a good alternative and good arguments (you simply stopped responding when we mentioned the drawbacks of the Windows-10 theme). Second, they'd express it more politely and say something like that they think it's not a good idea.
I had a similar experience in https://github.com/geany/geany/pull/2363 with pretty arrogant "arguments" like
I also don't buy that all other editors just work.
and stopping to respond eventually. As a maintainer of the macOS binaries I just apply that patch and distribute modified version of Geany on macOS but I'd wish a better solution.
So when you appear and a discussion like the current one takes place, my mental state is "Oh crap, this guy is here again, he'll NACK this PR without giving valid arguments" even though this may not be true and my responses are more aggressive than they should be (sorry for that!).
I know this is the Internet, things don't get through as they were intended, and we, guys interacting with computers, are terrible when dealing with ordinary humans. So, please, take the above as a suggestion regarding how to improve the communication here than some hard criticism. And please, tell me if you don't like something about my responses - I know I suck ;-).
Please propose how you will make synchronous TM work with asynchronous LSP servers. There is a fundamental divide which is why @techee handled it by having individual user facing functions use either TM or LSP, they have to adapt to the API paradigm, asynchronous LSP is not a drop in replacement of synchronous TM.
@elextr I think this isn't the fundamental problem - I handled it with the asynchronous `request()` calls in the interface that call the corresponding LSP asynchronous call and the synchronous `get_cached()` calls that return the last returned result. This could be done for injecting symbols to the tag manager too. If it were just that, I'd be for placing LSP tags into TM too.
But I'm more worried about the 4 points I mentioned in my response to @kugel-.
On the other hand, if we follow-up on your other idea and use LSP even to interface with the ctags parsers then documentSymbol ought to be sufficient for all existing features (except maybe where we can specialized LSP interfaces like goto-implementation), right?
I was thinking about it and not to lose functionality, I think we'd have to introduce some Geany-specific members like _geany_var_type. It sucks a bit and I haven't checked the LSP specification in detail if it explicitly prohibits something like that but I think even with extra members we should be LSP-compliant.
So I'm forgetting about how LSP works myself too :-). No, you won't need anything extra. Basically, we only need things like `var_type` or `arglist` for autocompletion or calltips, respectively, and these will be only needed on the server for the autocompletion or signature calls, but not on the client side any more. So `detail` could contain whatever we need to show in the calltip in the symbol tree.
No, @kugel- said:
the sidebar lists a symbol and you can click on it to go to the defintion (because fed by ctags) but you cannot click on the same symbol in the editor (because goto-definition in editor is implemented by LSP).
Note the _in the editor_, where LSP will indeed only goto (or show on hover) the declaration/definition only if it is valid and error it if it is invalid.
Note the in the editor, where LSP will indeed only goto (or show on hover) the declaration/definition only if it is valid and error it if it is invalid.
Ah, right, so it was me who misunderstood it :-).
But really, when someone configures the plugin to use symbols in the sidebar from the tag manager and goto from LSP, he gets exactly it and in this case it's nothing unexpected.
the sidebar lists a symbol and you can click on it to go to the defintion (because fed by ctags) but you cannot click on the same symbol in the editor (because goto-definition in editor is implemented by LSP).
Actually, in this particular case, if we wanted, we could fall back to trying TM if no goto happens using LSP (but even in this case I wouldn't do it because in bug reports we wouldn't know if it was the TM's or LSP's problem).
Actually, in this particular case, if we wanted, we could fall back to trying TM if no goto happens using LSP (but even in this case I wouldn't do it because in bug reports we wouldn't know if it was the TM's or LSP's problem).
Why would the user want to goto the wrong place? If LSP says the name is not visible going to the definition/declaration of some random thing invisible at that point in the code, but which happens to have the same name, doesn't make sense.
Why would the user want to goto the wrong place? If LSP says the name is not visible going to the definition/declaration of some random thing invisible at that point in the code, but which happens to have the same name, doesn't make sense.
Yeah, right. Too late, I start making errors. Good night :-).
For my "look up symbols in a dialog" use case: LSP's goto-implementation cannot be used for that, and other LSP interfaces are not really sutiable either, as you pictured above. So what can we do to keep such functionality?
Do I imagine correctly it shows some entry with symbols below and as you type it filters-out symbols to correspond to the entered text? I think this is exactly what vscode uses workspace/symbol for.
To avoid speculations about how exactly LSP servers behave, I have just implemented such a feature in the plugin - stealing good ideas from vscode and stealing Colomban's Commander plugin GUI code :-). In fact, I think it would be fantastic to have such a feature directly in Geany so various plugins don't have to re-implement it over and over.
<img width="722" alt="Screenshot 2023-11-11 at 11 33 05" src="https://github.com/geany/geany/assets/713965/442ad577-e483-4703-ac2b-ef92728b8299">
Basically when invoked with the `#` prefix, it performs `workspace/symbols` call and shows the result (without any further filtering or reordering, this is exactly what LSP returns). The call is re-performed every time the user types something. So this call is apparently more something like autocomplete over global symbols, it's just meant for going to the symbols based on their name, working fast in real time and showing a limited number of entries only.
Then I implemented searching in current document's symbols when prefixed with `@` - this time the filtering in the list is done by me using the code we use for filtering in the sidebar.
I also added the other vscode goto features even though they aren't directly related to the plugin - prefixed with `:` it goes to the line number and without any prefix it searches in open documents.
I also added fallbacks to use TM symbols when LSP isn't available for the given language.
Anyway, back to the question what one would have to do if interested in all symbols - `workspace/symbols` isn't the right call for it. One would probably have to query them document by document for all the project's files (and in Geany we don't even know what "project files" are) and then merge them. I really don't think this is the way to go and I think we should just leave the "intelligence" on the server.
Wait, are we renaming Geany Gscode? ;-P
No problem with having extra features, but the only use-case I can see for `workspace/symbols` is if the user does not have the symbol in the code in front of them, if its in front of them then goto definition/declaration, or if adding autocomplete will do it, maybe to lookup something unrelated to their code, doesn't seem a common use-case to me, what am I missing?
PS don't forget there is an attempt at a shared library for plugins just crying out for more content, like shared plugin GUI code ;-)
No problem with having extra features, but the only use-case I can see for workspace/symbols is if the user does not have the symbol in the code in front of them, if its in front of them then goto definition/declaration, or if adding autocomplete will do it, maybe to lookup something unrelated to their code, doesn't seem a common use-case to me, what am I missing?
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. Similarly, find in files isn't a direct replacement because it finds also uses of the symbol so for instance if I searched for `tm_tag_new()`, it would find all of its uses and not only its definition so one would have to go through the whole list in the message window and search which line is the actual definition.
Yeah, if the user had to fake-write it then it is as I said "not in their code", so its stuff thats unrelated. If what you are implementing is related then the name will most likely occur locally, and you will have the relevant file already open to click the tab or ....
Ahhhhhh, thats what split/multiple windows are for :-) I did say elsewhere that I had gotten used to them, I had even forgotten how bad life was without them. I generally run one pane with `.hpp`s in it and one pane with `.cpp`s in it, and a separate window with test files and code and miscellaneous in it.
Anyhow, as I said I'm not against Gscode having extra functions, especially if they go some way to compensate for its lack of split/multiple windows. I just didn't perceive that use-case.
@techee pushed 1 commit.
bccb605030dda0bafedb0b91e31e220c94285c58 Modify the LSP interface to return position for the symbol goto
: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.
@b4n, generally a well reasoned and fair assessment given its a first impression, it took me a while to get used to it, and I have been using Vscode for C++ for a while.
### Jumping to the conclusion
Not good without a proper project setup (expected I guess).
Yes its expected, LSPs are __compilers__ (or interpreters in Pythons case, bless its little cotton socks), they need the compiler info, for C/C++ particularly the `-I` options. But thats a problem with the language and implementations, not the tool.
But clangd isn't restricted to the tree under the Geany project, so long as it can find `compile_commands.json` or `build/compile_commands.json` in the random files parent path it will work just fine. Modern build tools (well meson and cmake that I have tried) make that file by default, so yet another reason for autotools to die.
The issues if `compile_commands.json` cannot be found may be clangd, opening random C++ files in Vscode[^1] makes less of a mess of it than clangd. I think the main reason is it turns errors off if it can't find includes. Don't know if clangd can be configured like that or we would have to somehow decide the file is outside any known build and turn errors off in the LSP plugin, or let the plugin be turned off per file[^2].
Pretty good with it, but I had disappointments of how it plays in real life I didn't expect.
Concentrate on the "pretty good" with it, damn technical people (me included) have too much of a tendency to go for the minor annoyances/its different jugular. Also remember geany-lsp is in development, consider how to improve things and raise them there, not assume any current issue is permanent.
I have been using geany-lsp for C++ a while, and sure, it still has rough edges, but they can be polished. It is so much better than TM/ctags for C++ that I could be persuaded to use Geany again if it weren't for unrelated benefits of Vscode[^3]. The improvement for C is likely to be less, it doesn't have inferred types, or overloaded names, or templates.
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.
One thing to bear in mind is that some things need to be used differently, see my previous explanations for details, but to summarise, "use the symbol in the code", not the sidebar. That way you get the benefit of clangd's knowledge of scope and visibility and inferred types and overloaded names etc.
And you should try it with C++ not C, maybe while adding the "Colomban inset:tm:"[^4] to Scintilla.
### A few specific points
Without clangd installed
Probably the choice of LSP vs TM/ctags should be improved, at the moment its pretty much all or nothing, it needs to fallback to TM/ctags more effectively per language when the server isn't available, or when it can't handle missing information, and possibly by user choice as I said above.
For me, this means that it's not fit for general use (yet?) outside a proper project configuration
Indeed it may never be fit for that (at least with C/C++ where build info is needed), but thats clangd really not the plugin, just needs better control as above. And as I said, remember how much better it is when the project is done in a suitable way.
Very slow at start, but well, I'm waiting.
Yes, clangd has to index the whole project, thats the point, there is a note on geany-lsp to show the progress bar during it. But it is only first time. After that its incremental.
not being able to get completion for a function because it's not ordered right (e.g. only declared below in the file)
Simple, don't do that, you have no compiler checking ability. And surely its not allowed with `--b4n-pedantic`.
a tad annoying when refactoring things I imagine.
Refactoring support is in development IIUC. But yes, code has to compile for LSP to help, so turn on autoclose ()[]{} too, you will get used to it :wink:.
the symbol trees after opening scintilla/gtk/ScintillaGTK*.cxx took very long to show up
Similar to the time it takes to compile that file maybe? :wink: IIUC clangd scans and indexes all files but only compiles files when queries need it (or they are a dependency of a file that is compiled).
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.
Well discuss it with [clangd](https://github.com/clangd/clangd/issues), its nothing Geany can do, unless we can use the Vscode LSP instead, Vscode opens that file just fine.
[^1]: if you can bear to read the geany-lsp repo comments you will see I use Vscode as reference, because I use it, and because it's LSP use has had lots more time to mature. But its still not perfect, so its unrealistic to expect Nirvana from an in-development tool :stuck_out_tongue:.
[^2]: using user knowledge, or wire in chatGPT :grin:
[^3]: proper working multiple windows/panes, see both the .hpp and the .cpp at the same time, or see two places in the same .cpp file (the class declaration and the member function implementation I'm writing)
[^4]: see [Geany-lsp discussion](https://github.com/techee/geany-lsp/issues/21#issue-1989722527) for details :grin:
### On the implementation
I had some time to look at it finally. There are no glaring issues I can see at first scan.
@b4n
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.
IIUC the point of this is to step reasonably lightly on the Geany code (at this stage, the full takeover is later :-). Bear in mind that LSP is asynchronous, but Geany and plugins are resolutely synchronous, it may not be a drop-in replacement. It seems to me that this achieves the "lightly" given how much functionality it adds to an application that builds in so much (that should be in per language plugins, stir).
For more intrusive changes and maybe some planning around them I made #3675 for discussion, please contribute.
### Python
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).
With pylsp 1.9.0 it seems to work for me. `pip install python-lsp-server` then edit command path in `Tools->LSP client->User Configuration->[Python]->cmd=` since I don't have `~/.local/bin` in my path.
### Misconceptions
Basically, LSP isn't entirely the holy grail I though it might have been?
And the surprise is? If you believed that then I have a bank account for you to contribute to ;-)
Nothing is perfect, its a case of getting enough to be worth it. And that may depend on the language you use, if Geany isn't to stagnate as a C IDE it has to handle language features that ctags parsers can't, inferred types, generics, overloaded names. The best current technology that can be leveraged to provide that is the LSP server. To date Geany has lived with imperfect TM/ctags, don't let the fact that LSP servers are also imperfect blind you to the significant benefits they bring to languages with complex features.
@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.
Yes its expected, LSPs are compilers (or interpreters in Pythons case, bless its little cotton socks), they need the compiler info, for C/C++ particularly the -I options. But thats a problem with the language and implementations, not the tool.
Just a small note - we are all, me including, a bit influenced by how `clangd` works but other servers may work differently. LSP is just the protocol and how exactly the server implements it is up to the server. So there may be a full parser behind it, but there may also be a set of simple regular expressions. Or the whole server may be just a code formatter and not to implement any other functionality.
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.
Servers are going to be best when they are supported by the language community like gopls. AFAIK newer languages like Rust, Julia etc provide them and generally try to keep them in sync with the language, but they may require a lot of the language infrastructure, eg rust analyser needs the whole standard library source IIUC, maybe that will change over time.
It should be possible to use the same servers as vscode, except the MS ones that use proprietary code, IIUC cpptools-srv is MIT but it isn't a compiler and uses MS stuff from Visual Studio that is proprietary and only licensed for vscode use. Also there can be intelligence in the client since each language in vscode has its own plugin that talks to the server. So a generic client may not work as well with any specific server.
We were already talking with @elextr that there should be a config option to disable LSP for files outside the project directory.
Yeah, clangd works if it can find the info it needs, but it should be possible for the user to turn LSP off for the file if they feel that it isn't helping. Again new languages with built in build systems (like go, rust/crate) should also work if they can navigate from a file to the build information they have stored for the "project" the file appears in, but I have no idea if they do.
So my preferred hierarchy is per language to automatically use TM if the server isn't running, per language settings for "LSP off" (just comment out the section in the conf file?), "in tree only"[^1], and a UI available per file setting to switch LSP or TM, save that in the project for open files.
Just a small note - we are all, me including, a bit influenced by how clangd works but other servers may work differently. LSP is just the protocol and how exactly the server implements it is up to the server. So there may be a full parser behind it, but there may also be a set of simple regular expressions. Or the whole server may be just a code formatter and not to implement any other functionality.
Indeed, and it is why TM/ctags could be packaged as an LSP, and why (as I said above) more configurability is needed to switch individual files/projects/other to/from LSP.
[^1]: "in tree" isn't necessarily the best, "in project including imported local modules" would be better, but "in tree" is easy to define and implement independent of language peculiarities.
@techee pushed 1 commit.
952c4bf5e2522aa73d810b3ae9523bf36fc8dcdf Don't call sidebar_update_tag_list() and document_highlight_tags() on startup
So my preferred hierarchy is per language to automatically use TM if the server isn't running
That should happen now.
, per language settings for "LSP off" (just comment out the section in the conf file?),
No need for such settings - just comment-out the `cmd` command
"in tree only"[1](https://github.com/geany/geany/pull/3571#user-content-fn-1-e32a32f689dd84a80...),
I called it `lsp_use_outside_project_dir` (I'll start prefixing settings affecting the whole plugin with `lsp` - I already do something similar for the individual features). ``` and a UI available per file setting to switch LSP or TM, save that in the project for open files. ^~~~~ error: I got lost starting from here ``` Anyway, I've recovered from the error and added another option `lsp_use_without_project` controlling whether to use LSP when there's no project open. Both options are set to `false` by default, but for Python, which seems to work when not using project, I set these to `true`.
and a UI available per file setting to switch LSP or TM, save that in the project for open files.
^~~~~ error: I got lost starting from here
Anyway, I've recovered from the error and added another option lsp_use_without_project controlling whether to use LSP when there's no project open. Both options are set to false by default, but for Python, which seems to work when not using project, I set these to true.
That wasn't actually what I meant, but its a good idea.
As was said before languages with built-in build systems should not need Geany to tell them where the info is, and clangd does a reasonable job of emulating that by searching for `compile_commands.json`
Now what I was trying to say was, when a per-open-file disable of LSP is added to the UI it can be stored in the project file like the session, but now we don't need a project file I guess it has to be stored in the session or the project as appropriate. (It wasn't complicated, I just wrote it that way :-)
Now what I was trying to say was, when a per-open-file disable of LSP is added to the UI it can be stored in the project file like the session, but now we don't need a project file I guess it has to be stored in the session or the project as appropriate. (It wasn't complicated, I just wrote it that way :-)
I don't know if you noticed, but under the "Project->Properties->LSP Client tab" you can set up a separate LSP config file for every project. I think this may be needed because various projects will behave differently and you may need special configuration for them. So the per-project configuration is already possible for everything.
under the "Project->Properties->LSP Client tab" you can set up a separate LSP config file for every project.
I wondered what the greyed out "Project Configuration" menu item was, but how to set it up?
So the per-project configuration is already possible for everything.
Neat, feel free to keep ahead of me ;-P
I just noticed one more thing - even symbol names obtained from LSP aren't guaranteed to be just plain names - consider for instance the Mutex source from the Go system library:
<img width="277" alt="Screenshot 2023-11-21 at 11 52 44" src="https://github.com/geany/geany/assets/713965/86d00393-2bd7-483b-bcb8-e6b6cb5e6a8c">
Here you have the functions working on top of the Mutex type prefixed with `(*Mutex)` while with our TMTags we'd have this in scope (the gopls server's representation is more idiomatic for go than having the methods scoped within the Mutex type). Similarly, `pylsp` generates what we call "local" tags (for variables inside functions) but we have no indication from the LSP server that these tags are local (which we'd need for correct autocompletion). On the other hand, `clangd` doesn't provide local symbols at all unlike our TM `cxx` parser.
So I think we just can't make any assumptions regarding what we obtain from the server - the "right" and intended way to use the interface is just to blindly display what the server returns. This also means we cannot stuff the info from the LSP into TMTags because we just don't know what the server returns.
So if we want to unify TMTags with what is returned from LSP servers, we have to do it the other way round - within TMTags introduce fields like: - `display_name` - the name that will be displayed in the symbol tree (corresponds to `name` in LSP) - `display_detail` - the name that will be shown as a calltip (corresponds to `detail` in LSP) - `display_icon` - the icon to display - here I'd just suggest avoiding Geany having to care about the symbol kind representations in LSP (there are two - one for autocompletion, one for symbol tree and workspace symbols and the values are different) and let the LSP plugin return the right icon for Geany to display - `scope` - we could keep this one for LSP symbols. What I did for the symbol tree is that I generated it from the `children` information of the symbols and it seems to work alright.
We could generate these for TM tags inside TM and the parts of Geany showing these in various UIs like the symbol tree or the autocompletion popup could just read these fields and ignore the rest of the tags.
Now my question is - should plugins really have access to these LSP symbols? I somehow can't imagine anything they could use these for apart from showing them in some different kind of UI - they'd run into the same kind of problems interpreting them as Geany would.
Back to the API - should it be specific for the LSP usage or more generic so it can be used by other plugins? I called the file `Lsp` but in reality, the current interface could serve to any other plugin that e.g. wants to provide its own autocompletion support - there's nothing LSP-specific about it. So it's more like `ExternalFunctionalityProvider` which disables Geany's handling of certain features and delegates them to an external entity.
On the other hand, the API could also be really LSP-specific where we require LSP to be present and have it somehow deeply integrated into Geany - but I don't see much use for such deep integration myself especially when we still want to keep using the TM.
Mutex type prefixed with (*Mutex) while with our TMTags we'd have this in scope
Well, bearing in mind the limitations of my Go, I think what it is telling you is that the Mutex needs to be accessed via pointer, all users of a Mutex must access the same memory, they can't be separate instances. And thats how the docs present it [here](https://pkg.go.dev/sync@go1.21.4#Mutex)
pylsp generates what we call "local" tags (for variables inside functions) but we have no indication from the LSP server that these tags are local (which we'd need for correct autocompletion).
Yes, I noticed that, Python assignment is also a variable declaration remember, so it clearly takes all assignments as declarations. Using my example `scripts/create_php_tags.py` the local `tag_list` does not appear in the ctags symbols but does in the LSP symbols.
For example type `tag_` (properly indented, its Python remember) on blank line 77:
1. The TM/ctags autocomplete is a long list but starts with `tag_list`, where it got them from I don't know. It has no scope autocomplete after accepting `tag_list`.
2. But the LSP autocomplete is 3 entries, only the local `tag_` names, including `tag_list`, and then has scope autocomplete of the methods of a list. Similarly `arg_list` has string methods autocomplete in LSP but nothing in TM/ctags.
Tthe LSP does autocompletion for variables it can trace values and types for (being dynamic Python makes that a bit hard unfortunately, therefore many variables have unknown types), "we" don't need to do autocompletion if LSP is running, and if it isn't then its TM/ctags only and no LSP symbols to care about.
On the other hand, clangd doesn't provide local symbols at all unlike our TM cxx parser.
It doesn't give locals because they are not visible outside a lexical scope from the declaration to the end of the block its in, except for class members in member functions which can be visible before the declaration and also inside member function definitions written outside the class definition, even in another file, and how do you express that? But doing anything for a symbol outside that scope is wrong, the symbol is not visible. The LSP server does the autocompletes and hovers for them, so it has the information internally. And scope autocompletion even if they are inferred types, which ctags can't do.
So if we want to unify TMTags with what is returned from LSP servers
We can't unify them, full stop. Its an either or situation I'm afraid. As I said above, the LSP knows about the lexical scope of the symbol, but has no way of passing that to TM and TM has no way of representing it anyway. Thats why LSP de-emphasises the symbol list, it cannot reasonably be right for functions like autocomplete or gotos.
Perhaps a better way would be to consider if the LSP functions can be available to plugins, but then the asynchronicity might be a problem.
Back to the API - should it be specific for the LSP usage or more generic so it can be used by other plugins? but I don't see much use for such deep integration myself especially when we still want to keep using the TM.
How do you handle multiple plugins trying to provide the same function? This is the first glimpse of plugin provided language functionality and devolution of all language specifics to plugins, like every other IDE, but while its only one plugin lets keep it simple, as I told @b4n "at this stage, the full takeover is later" ;-)
Well, bearing in mind the limitations of my Go, I think what it is telling you is that the Mutex needs to be accessed via pointer, all users of a Mutex must access the same memory, they can't be separate instances. And thats how the docs present it [here](https://pkg.go.dev/sync@go1.21.4#Mutex)
I'm a go user myself and I know what that means - and as I said, this representation is more idiomatic than what we show using our TM ctags parser.
But this wasn't my point - my point (including the python and c++ example) was that we cannot be sure what we get from the language server because `(*Mutex).Lock` isn't a "correct" name in the sense of TM where name would be just `Lock` and `Mutex` its scope (which isn't completely correct from the go language point of view because `Lock` is outside the scope of `Mutex`). Similarly some language servers will return local symbols, some not and we won't know which ones do and we won't know which tags are local. Etc. The only thing we can do is display them as they are without trying to "understand them", because we can't.
We can't unify them, full stop.
Then we are on the same boat :-).
Perhaps a better way would be to consider if the LSP functions can be available to plugins, but then the asynchronicity might be a problem.
Technically they could be, plugins would have to deal with the asynchronicity somehow (I think it wouldn't be a big deal) but the question is whether they can do anything useful with them (or in other words, if they can do something useful, whether such feature shouldn't be part of the LSP plugin itself). I for instance don't know what plugins could do with calls like autocompletion or goto tag definition. The symbol list of the current document is the only one possibly useful if they want to display the symbols differently as Thomas suggested, but even this one is a bit iffy when we can't be sure about the exact format of the symbols.
How do you handle multiple plugins trying to provide the same function?
The "first" plugin providing the functionality would win. So for instance the plugin returning `TRUE` for `autocomplete_available(doc)` would be used for the autocompletion of the given `doc`'s filetype. The "first" is in quotes because it would depend on some internal ordering depending how plugins were loaded - but basically the intended usage would be to have each plugin for a different filetype so they wouldn't overlap, otherwise the "winning" plugin would be undefined.
Anyway, this isn't something important, I just mentioned it because the current interface is of this kind already, only the implementation supports a single plugin, but that could be changed easily.
I'm a go user myself and I know what that means
Sorry, I thought you just owned a plush gopher, not that you were a gopher, have to watch out you don't start Goany. So leaving testing gopls to you. :grin:
Ahem, anyway back to LSPs.
Basically we seem to be in vociferous agreement, but from differing points of view.
Your "isn't a "correct" name in the sense of TM" is my "LSP servers don't give incorrect symbols like TM/ctags".
I for instance don't know what plugins could do with calls like autocompletion or goto tag definition.
Agreed, no point in trying to use LSP symbols to do what LSPs themselves do (and if one server doesn't do it, don't use that LSP server, use TM/ctags unLSP "server").
I just mentioned it because the current interface is of this kind already, only the implementation supports a single plugin, but that could be changed easily.
Baby steps, we mustn't let @b4n suspect we are moving all of Geany into per language plugins :wink:
The only thing we can do is display them as they are without trying to "understand them", because we can't.
The only alternative is to have per server LSP clients that understand the servers replies and can interpret them sensibly. That would be part of the per language plugin. So for now best to do nothing but show them.
On the topic, the paragraph setting is helpful, the per line one tended to cut stuff off if it happened to be a bit long. But for pylsp the paragraph setting would be best as 1 and clangd 2 AFAICT, so maybe later it can be per server.
Also you said that per project LSP settings were possible, but didn't say how to set that up.
On the topic, the paragraph setting is helpful...
I think it's rather off-topic wrt. the LSP interface, let's discuss those plugin-related things in the plugin itself. I tried to explain the configuration in https://github.com/techee/geany-lsp/issues/31
@techee pushed 1 commit.
28cd7c47860a04ce96a9c35c76e9e089213f4a11 Revert "Don't call sidebar_update_tag_list() and document_highlight_tags() on startup"
Well, as there is no input from anyone for months, and as I have been successfully using the combined branch with LSP (mostly for C, some C++, minimal Python) I am minded to commit this. Especially since it can be completely disabled, just don't load the LSP plugin if LSP is not wanted.
It may not be perfect, but we will never find out by not trying it.
@techee, is there a place where the LSP-plugin is available to be built by itself for use with this?
@elextr I don't think this PR is in the state that it should be merged. Some things (like the symbol tree LSP API) are quite ugly and should be replaced by something better. I just needed to create "something" quickly so the LSP plugin could be tested and where it's easy to see for others what Geany code it affects so the proper API can be discussed.
@techee, is there a place where the LSP-plugin is available to be built by itself for use with this?
Not yet but as I'd like to try to modify the plugin so it also builds against unmodified Geany and enables the LSP stuff that typically isn't present in Geany and doesn't interfere with anything in Geany (there will be some ifdefs in the plugin checking whether it builds against modified Geany or unmodified Geany). This plugin would then be completely standalone and I could create a PR against geany-plugins. When the LSP support is merged to Geany, the plugin would just be modified to use the new Geany API.
Some things (like the symbol tree LSP API) are quite ugly and should be replaced by something better.
Doesn't matter if it works (which it seems to) it not a beauty contest or a school homework assignment ;-). It can be beautified later if needed (but much like the build commands implementation probably never will).
Not yet but as I'd like to try to modify the plugin so it also builds against unmodified Geany and enables the LSP stuff that typically isn't present in Geany and doesn't interfere with anything in Geany (there will be some ifdefs in the plugin checking whether it builds against modified Geany or unmodified Geany).
This makes the plugin need to be distributed as two plugins (except on gentoo and others where it is compiled against the end users Geany) one against unmodified Geany and one against modified Geany. So the plugins will have to decide at runtime which is to be enabled and shown to the user. But the user will have to download both.
Doesn't matter if it works (which it seems to) it not a beauty contest or a school homework assignment ;-). It can be beautified later if needed (but much like the build commands implementation probably never will).
But this is an API thing so it shouldn't be to terrible. I think the worst thing is the symbol tree API which should really be changed. I'll see what I can do about it when I have time.
On a more general note though I think there should really be some agreement among Geany developers regarding how such an API should look like. I don't want to introduce something that's not acceptable for others.
This makes the plugin need to be distributed as two plugins (except on gentoo and others where it is compiled against the end users Geany) one against unmodified Geany and one against modified Geany. So the plugins will have to decide at runtime which is to be enabled and shown to the user. But the user will have to download both.
Not really, the modified Geany simply wouldn't be distributed until the stuff is merged to Geany.
On a more general note though I think there should really be some agreement among Geany developers regarding how such an API should look like. I don't want to introduce something that's not acceptable for others.
In the current state of Geany with most devs AWOL then its a case of what is proposed is it. You are the one who has thought about the requirements, the possibilities and chosen a design, but nobody else has actually thought about it. There will never be agreement when nobody has time to understand the problem and actually consider the pros and cons of the proposal. Thats why the only comments have been "I don't like it", not any sensible suggestion for improvement or alternatives, nobody has time to think of them. (And I don't claim any great insight, but at least I understand the reasoning behind your basic choice of approach, but I also don't have time to consider the design in intimate detail either)
@elextr please read all the comments again, mine and from @b4n . Boiling them all down to "I don't like it" is ignorant and outright rude!
You seem to be biased towards this change, perhaps because it works so well for you, and this is OK. But be honest about it and don't play down other people's voice. Maybe I didn't do a good job but I always tried to be objective and suggested alternatives or improvements.
Also accept that not even the author deems the current form as ready for merging.
You seem to be biased towards this change, perhaps because it works so well for you, and this is OK. But be honest about it and don't play down other people's voice.
You make it sound like its bad, but being excited about a feature is what open source is about, people do what they are interested and excited about, and of course that makes people positive about the change, its the lifeblood of volunteer projects, not something bad.
Maybe I didn't do a good job but I always tried to be objective and suggested alternatives or improvements.
I didn't mean to imply that you were not attempting to be helpful, but I did scan the thread before posting, and still I don't see any acceptance of the fact that the LSP works different to ctags/tagmanager. And I explicitly acknowledged that that is most probably due to a lack of time for those of us other than @techee to study the details. But that means the suggestions are inappropriate to the way LSPs work, so they are not improvements, they are trying to fit a square peg in a round hole three sizes too small.
Perhaps summarising it to "I don't like it" sounds a bit harsh, and I apologise for that, but continued arguments to do things in ways that do not fit the LSP process effectively boil down to that. Because we can't change the LSP spec so there is no point in continuing to propose suggestions that do not fit with how they work, no matter how little somebody likes them (or MS). I spent a considerable amount of time over several posts trying to explain how the LSP process works, and features of various languages that LSP supports and tagmanager doesn't, but clearly I didn't do it well enough.
I don't know what your programming language background is, but I also did not see any acknowledgement that various languages (eg C++, Rust, Go, Julia) have advanced features (eg inferred types) that the existing ctags/tagmanager infrastructure simply does not address, and cannot address without the same level of smarts as LSPs have.
Also accept that not even the author deems the current form as ready for merging.
And I added the WIP label at the time of the post above for that reason. Its unfortunate since (at least as embodied in the geany-lsp repo) it works just fine and adds considerable capability that needs to be made available to users to use and benefit from and abuse and find bugs and issues.
IIUC @techee is proposing to complicate the plugin to provide some limited capability to unmodified Geany (but didn't actually define what capability it might be able to add). To me thats just wasted redundant work because it cannot add support for those advanced features of the languages that tagmanager is incapable of supporting, and makes the plugin more complicated, and has two versions of capability to support. The plugin should simply not load if the API version is below the LSP API version.
Since most of the code is in the plugin (this PR is only a few hundred lines) and it doesn't affect anything if the plugin is not loaded, its safe to make available. Obviously if anyone finds changes to Geany behaviour with the plugin unloaded or loaded but a language for which there is no LSP they should post immediately.
I'll try to clarify my position on this PR.
I didn't really receive any valuable input regarding the implementation itself (not blaming anyone, I completely understand each of us has other duties and limited time for Geany). The feedback from @elextr and @b4n was mostly related to the functionality of the plugin which I tried to address more or less (of course, I can't do much with the behavior of LSP servers themselves).
The feedback from @kugel- was IMO based on some assumptions of how LSP works which weren't based on reality. LSP servers are black boxes that provide the given functionality and aren't meant to be used as a source of symbols that are used for some advanced logic of the client - the necessary information just isn't there. If anyone wants to go this direction, the person should first study LSP and Geany and give concrete suggestions about how to implement it (which I'm not convinced is possible).
There's also the possibility of a "hybrid" implementation so that we use the LSP API for everything but still inject the symbols into the TM but I'm really worried that the outputs of various servers will vary greatly, even among the same server's versions (e.g., while testing, @elextr ran into an issue that one clang version reported what we call "scope" but some older version didn't) and I don't believe that having to configure the behavior of LSP servers for every possible server version is the way we should go. Moreover, we can't easily get all the symbols from the whole workspace using LSP (apart from querying it individually for each file) so I'm not sure if we could really mirror what we do for the TM.
So the way I see it is to do something similar to this PR but of course there are tons of ways to implement it and I'm open to any suggestions. I needed something relatively quickly so I could create the plugin and also wanted some feedback first before doing some possible multi-thousand LOC refactoring unnecessarily. This PR is just around 500 LOCs and more or less demonstrates the things that are affected by LSP in Geany with not too many diffs involved.
This doesn't mean it couldn't be implemented the way this PR suggests - I think it's mostly OK and not too intrusive, only the symbol tree implementation should definitely be changed to something better because the current way is way too hacky. But before doing that, I'd prefer some feedback in case someone has a feeling the whole LSP integration should be implemented in some other way so I don't waste my time.
I don't want to introduce something that causes some friction among Geany developers. On the other hand, I don't want the plugin be a vaporware - it has around 7000 LOCs (the Geany part is the trivial one in comparison), it cost me a significant amount of time to develop and I just don't want to throw it to the garbage bin.
So what I'm proposing is to have some version of the plugin for the time being until (if) the Geany portion is acceptable for everyone where the plugin doesn't depend on any special support in Geany. For such a version I could create a PR against geany-plugins and it could receive a broader testing among users. The thing will be clumsy in many ways - there will have to be separate keybindings for identical things that are present in both Geany and the plugin (such as going to definition/declaration), TM will have to be disabled for the given file type manually by users and some things like the symbol tree wouldn't work at all. But there will be something usable and useful. This shouldn't be much work for me and the plugin will live at least in some form.
Closed #3571.
Closing in favor of #3849.
github-comments@lists.geany.org