Incorrect use of args passing in plugin.toml leads to logfile not working

[nheuillet]

Hello @superlou ,

I have taken a look at your try of the pylsp server. I noticed the reason you consider --logfile to not be passed to the plugin is actually because it is not implemented on master. On my PR there is a way, using binary_args. Here is an updated plugin.toml that will make the logfile working:

name = "lapce-python"
display-name = "Python"
description = "Python for Lapce: powered by https://github.com/python-lsp/python-lsp-server"
version = "0.1.0"
author = "dholth"
repository = "dholth/lapce-python"
wasm = "lapce-python.wasm"

[configuration]
# language_id may not matter to lapce yet
language_id = "python"
executable = "/Users/dholth/.local/bin/pylsp"
[configuration.options.binary]
binary_args= ["--log-file", "pylsp-logs.txt"]

the pylsp-logs file will be created where lapce lies (for me, as I ran lapce using cargo run --bin lapce, the file is created on my fork's folder). here is the output on my side:

2022-06-22 03:47:02,493 CEST - WARNING - pylsp.config.config - Failed to load pylsp entry point 'mccabe': No module named 'mccabe'
2022-06-22 03:47:02,495 CEST - WARNING - pylsp.config.config - Failed to load pylsp entry point 'pydocstyle': No module named 'pydocstyle'
2022-06-22 03:47:02,496 CEST - WARNING - pylsp.config.config - Failed to load pylsp entry point 'pyflakes': No module named 'pyflakes'
2022-06-22 03:47:02,497 CEST - WARNING - pylsp.config.config - Failed to load pylsp entry point 'pylint': No module named 'pylint'
2022-06-22 03:47:02,498 CEST - WARNING - pylsp.config.config - Failed to load pylsp entry point 'rope_completion': No module named 'rope'
2022-06-22 03:47:02,499 CEST - WARNING - pylsp.config.config - Failed to load pylsp entry point 'rope_rename': No module named 'rope'
2022-06-22 03:47:02,499 CEST - WARNING - pylsp.config.config - Failed to load pylsp entry point 'yapf': No module named 'yapf'
2022-06-22 03:47:25,796 CEST - WARNING - pylsp.config.config - Failed to load pylsp entry point 'mccabe': No module named 'mccabe'
2022-06-22 03:47:25,798 CEST - WARNING - pylsp.config.config - Failed to load pylsp entry point 'pydocstyle': No module named 'pydocstyle'
2022-06-22 03:47:25,799 CEST - WARNING - pylsp.config.config - Failed to load pylsp entry point 'pyflakes': No module named 'pyflakes'
2022-06-22 03:47:25,799 CEST - WARNING - pylsp.config.config - Failed to load pylsp entry point 'pylint': No module named 'pylint'
2022-06-22 03:47:25,800 CEST - WARNING - pylsp.config.config - Failed to load pylsp entry point 'rope_completion': No module named 'rope'
2022-06-22 03:47:25,801 CEST - WARNING - pylsp.config.config - Failed to load pylsp entry point 'rope_rename': No module named 'rope'
2022-06-22 03:47:25,802 CEST - WARNING - pylsp.config.config - Failed to load pylsp entry point 'yapf': No module named 'yapf'

My theory is that it's because of the way the sandbox is created: as there is no python env, the plugin can't find any modules as it doesn't know where to look at. This is in theory pretty simple to solve, and I'm gonna try to look into it asap.

using pip install "python-lsp-server[all]" solves this problem, and patching the Golang PR with this PR makes it run!

[dholth]

I added a wrapper script passing the desired arguments to pylsp, and pipx inject python-lsp-server autopep8 flake8 mccabe pycodestyle pydocstyle pyflakes pylint rope yapf whatthepatch since I installed pylsp with pipx. I tried https://github.com/lapce/lapce/pull/665 and I'm using the golang branch, but it hasn't clicked yet

[nheuillet]

Yeah, using pip install "python-lsp-server[all]" did the trick for the log above. No logs so far on my side either, so I'm gonna look into which step fails on lapce's side, but it looks like the initialize handshake is not complete, which is unexpected because that was the first fix I did for the golang lsp server

[nheuillet]

Nevermind I got it working! Turns out I was missing the language id from your pr @dholth

[nheuillet]

Everything looks to be indeed working out of the box:

I believe with these informations you guys should be able to make a fully functioning plugin. I'm glad to know my PR is working as intended though, and it confirms that it also opens the door to many other lsp servers :)

[dholth]

Mind blown! Now I just have to get it running myself, trying cargo clean on the main editor build...

[dholth]

And we have to figure out how to compile the entire lsp in wasi :)

[dholth]

executable = "/Users/dholth/.local/bin/pylsp"
[configuration.options.binary]
binary_args= ["--log-file", "pylsp-logs.txt"]

Suggest renaming to executable_args or just args

[nheuillet]

No need for the lsp compilation in wasi! What is done for rust-analyze, and gopls is simply checks to make sure the necessary lsp server is installed. If not, a more robust plugin could warn the user to install it, or better yet, to install it for the user, just like vscode would do. as of new, I know my golang plugin stops and warns the user gopls was not found, and rust-analyzer downloads it's own lsp binary (which is not the best as it doesn't check for a local one, but that's another issue).

I might as well rename it yes, args seems good as binary is already in the section name above ([configuration.options.binary])

[dholth]

In other words the name of the binary path could match the name of the options executable / executable or binary / binary

executable = "/path/to/server"
[configuration.options.executable]
args = []

[dholth]

I got it working over here! Very neat. lapce makes me feel like I'm using a fast computer, because I am using a fast computer. Then it crashes but you have to take what you can get :)

[nheuillet]

Just pushed an update where I renamed it to args! I prefer binary instead of executable but that's a purely subjective choice :) What crash are you referring to? I barely made sure it's running, and did not try anything remotely complex with it

[dholth]

It's not the plugin, just the whole editor likes to crash on OSX when you do things like "save a file" or whatever. Not all the time, just often enough to be exciting.

I suppose we think a Python script, a text file with the +x bit, is just executable and not binary. It doesn't exactly matter but it should be called the same thing in both places. Or put both path, args in [configuration.options.executable/binary]

[nheuillet]

Very true! I can nest a check on either binary or executable in the options, but I suggest not bothering because the whole plugin.toml file will probably change once a plugin api draft is created

[superlou]

... better yet, to install it for the user, just like vscode would do.

I think that's what lapce-rust is doing here: https://github.com/lapce/lapce-rust/blob/9003eb1d29b95cad64a4f25ded4a9c89dab5dcf5/src/main.rs#L53-L57

A warning if pyls isn't found and instructions to pip install 'python-language-server[all]' would probably be fine though.

[nheuillet]

Yes lapce-rust does install rust analyzer. The caveat being that it doesn't check if rust-analyzer is already installed, so you end up with a copy. The end goal to me would be to check and invoke directly all necessary commands if the user confirms some prompt . But I think an actual draft of the plugin api is more important than taking care of these problems for now!

[superlou]

Agreed, and thanks for everything you've been doing!

[nheuillet]

No problem! I'm glad my PR ended up making pylsp work. Now that it's merged, it should help more lsp servers to work out of the box in the future! If you have any questions, feel free to ping me wherever you want, but I'm more active on the discord :)