Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add LSP plugin #1331

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add LSP plugin #1331

wants to merge 2 commits into from

Conversation

techee
Copy link
Member

@techee techee commented Apr 22, 2024

This is a work-in-progress PR to add the LSP plugin. There are still some things missing like the documentation but in general the plugin should work. There are 3 modes of operation:

  1. Without Geany LSP support
  2. With basic LSP support using Plugin extensions (aka LSP API) geany#3849
  3. With full LSP support using Complete plugin extensions patches geany#3850

The plugin should detect with which of these modes Geany was compiled and adjust itself automatically.

For the time being I still plan continue the main development of the plugin under https://github.com/techee/geany-lsp and sync the changes here from time to time.

@techee
Copy link
Member Author

techee commented May 10, 2024

@frlan @b4n @eht16 The major things are "finished" from my perspective. If I missed something that needs to be done regarding geany-plugins integration, please let me know.

The word "finished" above means the plugin works but soft-depends on some things which are not in Geany yet, it's:

The missing signals cause some warnings when started from the command-line.

Also, having some official support for LSP in Geany like geany/geany#3849 would improve the user-friendliness and usefulness of the plugin.

I also plan to perform some more testing of the plugin and fix possible bugs but this should be independent of the string freeze if we want to make a Geany release soon.

@techee techee changed the title [WIP] Add LSP plugin Add LSP plugin May 10, 2024
@techee
Copy link
Member Author

techee commented May 26, 2024

@b4n Ping :-)

I think I'm pretty much done with the plugin and I'd like some feedback. I tried to address most of the complaints you had in geany/geany#3571 (comment):

The config file now contains these settings with these default values:

# Defines whether the plugin should be enabled automatically for new or existing
# projects (that have not yet been configured to use LSP). This configuration
# is only valid in the [all] section
enable_by_default=false
# Defines whether the server should be used when no project is open. Servers
# may not work correctly without a project because most of them need to know
# the path to the project directory which corresponds to the path defined under
# Project->Properties->Base path
use_without_project=false
# Defines whether the server should be used for files whose path is not within
# the project directory
use_outside_project_dir=false

This means that unless you explicitly enable LSP for the project, it isn't used. Similarly, files outside the project directory or plain non-project Geany use TM by default. No LSP unless you enable it.

On the other hand, I disagree there should be a fallback to TM when e.g. clangd isn't installed but LSP is enabled for the project - in this case it should be pretty clear something is wrong and falling back to TM would just hide the real problem.

I don't think clangd will ever work without a project - it really just compiles it behind the scenes - so testing it on individual files won't work and better to use TM for those.

I added

# Semicolon-separated glob patterns specifying files for which diagnostic
# messages are not shown. Useful when the server has a problem with some files
diagnostics_disable_for=*/scintilla/*/*.h

config option so diagnostic messages can be disabled for some files like the Scintilla headers (apart from diagnostic messages I believe other things work fine also for these headers).

This PR allows using the plugin without any Geany support but IMO this sucks. One has to disable TM parsing in filetype configuration and then it's all or nothing, one cannot use e.g. one feature using TM and one using LSP or one project (or just single file) using TM (without having to change the configuration). I'm still hoping we find some API that is acceptable for everyone, see geany/geany#3849 for my current proposal.

So when testing, I still recommend using the combined https://github.com/techee/geany-lsp repository.

Also, when trying the plugin, please delete ~/.config/geany/plugins/lsp/lsp.conf first - I made some changes to the config file and the previous version will be incompatible.

@elextr
Copy link
Member

elextr commented May 26, 2024

I don't think clangd will ever work without a project - it really just compiles it behind the scenes - so testing it on individual files won't work and better to use TM for those.

Well, although in theory it is possible to not have compile_commands.json but really clangd is a pain without, so not only will it not work on isolated files, it won't work on any build system that doesn't make a compile_commands.json like Meson, Cmake, or other modern ones do automatically. For others use of bear is supposedly possible.

So the restrictions of the default settings seems fine.

Just to confirm geany-lsp is Geany with #3849 and this plugin, all previous versions removed? If so I think I'll just get a new clone [end lazy git user] :-)

As I said elsewhere I am running behind in getting to be able to try these, but real "soon" now I hope.

Signed not-b4n :-)

@techee
Copy link
Member Author

techee commented May 26, 2024

Just to confirm geany-lsp is Geany with #3849 and this plugin, all previous versions removed? If so I think I'll just get a new clone [end lazy git user] :-)

It's:

  1. Geany
  2. This PR
  3. #3850, i.e. including the symbol tree patches. But you can easily simulate Plugin extensions (aka LSP API) geany#3849 by setting document_symbols_enable=false
  4. Fix emission of document-activate signal and various related GUI glitches geany#3870
  5. The patch from https://sourceforge.net/p/scintilla/bugs/2403/ which is merged upstream and we can either get it by updating Scintilla or backporting to Geany (just a few lines of code)

@elextr
Copy link
Member

elextr commented May 27, 2024

Does it include #3865?

@techee
Copy link
Member Author

techee commented May 27, 2024

Does it include #3865?

No, it doesn't, but you don't need to disable the ctags parser for geany-lsp as it contains the proper LSP interface.

@techee
Copy link
Member Author

techee commented Jun 7, 2024

@b4n I added this feature to the plugin that other LSP clients implement as well (not tested much yet):

https://github.com/techee/geany-lsp/blob/c56de66cecf4dff2c1b1a98ea37b22bd2af73f03/plugins/lsp/data/lsp.conf#L56-L68

This should essentially allow automatic opening/closing of multiple projects even if no Geany project is open and at least partially fix the problem you had when opening random files.

@techee
Copy link
Member Author

techee commented Sep 11, 2024

Alright, I've pushed the latest version here with squashed commits. Apart from bugfixes (if some bugs appear), I don't plan anything more for this release.

Please let me know what needs to be done to get the plugin merged to geany-plugins.

@elextr
Copy link
Member

elextr commented Sep 12, 2024

Please let me know what needs to be done to get the plugin merged to geany-plugins.

Good question, plugins maintainer was @frlan but he hasn't been around much so I'm not sure if he doesn't have time any more. So somebody else maybe, @b4n, @eht16 ?

@eht16
Copy link
Member

eht16 commented Sep 21, 2024

I will not do a code review but I intend to use the plugin with my work project which has a huge Python code base and I'm very curious how it will work and maybe I'm going to switch to the LSP approach completely but no promises yet :).

Though this might take a few weeks to complete as I need to get in touch with LSPs and find my way through the config.
But I already like a lot that the config file is well documented!

@techee
Copy link
Member Author

techee commented Sep 21, 2024

I will not do a code review but I intend to use the plugin with my work project which has a huge Python code base and I'm very curious how it will work and maybe I'm going to switch to the LSP approach completely but no promises yet :).

I'm really interested in your feedback - some things may just be limitations of particular servers but some things can possibly be improved in the behavior of the plugin or new config options could possibly be added to control the plugin's behavior if some servers behave differently.

I mostly tested the plugin with the clangd server and not so much with Python and other pre-configured servers which I tested only lightly. I found pylsp a little too slow so I'd recommend pyright or jedi-language-server. If you only want to use a linting language server, ruff might be a good option as it's super-fast. There has been some discussion about various Python language servers in

techee/geany-lsp#51

Also, it's always possible to disable a particular feature and fall back to Geany's default TM implementation.

Though this might take a few weeks to complete as I need to get in touch with LSPs and find my way through the config.

It's actually really simple and you don't have to study anything too much. Just install the server you want to use, check its documentation regarding how to start it and whether it needs some special command-line options and enter this to the cmd config file option. Then just follow

https://github.com/techee/geany-lsp?tab=readme-ov-file#quick-start

Some servers require the project's root directory, some don't. There are some options at the beginning of the config file you can use to configure the plugin so it runs without Geany projects if you don't want to use those.

Also, I recommend getting the plugin from https://github.com/techee/geany-lsp and not this PR as I still keep working on some stuff and sync the changes to this PR infrequently.

@techee
Copy link
Member Author

techee commented Sep 24, 2024

Alright, I got some more experience with python language servers while developing

https://github.com/techee/lsp-proxy

  1. I don't like pylsp much - seems slow and kind of buggy
  2. pyright-langserver seems to be very good but requires type annotations, otherwise it tends to report false positive errors - but I'm sure this is possible to configure by a config file where it should be possible to specify a less strict behavior.
  3. jedi-language-server also seems to be good - it doesn't do such an advanced type analysis as pyright but on the other hand it doesn't suffer from false positives because of this
  4. ruff is super fast, usable just for linting and can be a good supplemental language server to one of the three servers above - using multiple servers can be achieved by the lsp-proxy project above

The difference between 2 and 3 can be seen in code like

foo = ""
bar = foo + 1

where pyright reports an error about adding a string and a number while jedi-language-server doesn't report anything.

@Johnmcenroyy
Copy link

Johnmcenroyy commented Sep 25, 2024

@techee Also wanted to add some info about python's lsp

                   autocompletion  linter/formatter   type checker    refactoring         language
pylsp                 V(jedi)         V(ruff)          V(mypy)        V(rope,advanced)    python
jedi-language-server  V(jedi)         -                -              V                   python
ruff server           -               V(ruff)          -              -                   rust
pyright               V               -                V              V                   python/typescript
pylyzer               V               -                V              V                   rust


Type checkers are                 lsp server               
mypy    (python community)           -    seems most feature rich
pytype  (google)                     -    can work with code without annotations
pyright (microsoft)                  V    up to 3x to 5x faster than mypy
pyre    (facebook and instagram)     V
pylyzer (rust)                       V    up to 100x faster than pytype and pyright

Pylyzer is very new and in development.

I like very much jedi-language-server with ruff server (NOT ruff-lsp - this will be deprecated and written in python)
More info about ruff server: https://astral.sh/blog/ruff-v0.4.5

@elextr elextr mentioned this pull request Oct 3, 2024
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants