superlou / lapce-python

Python LSP plugin for the Lapce editor
MIT License
22 stars 4 forks source link

Allow users to specify path to LSP #17

Closed ash-sykes closed 7 months ago

ash-sykes commented 7 months ago

After scratching my head a little I finally managed to figure out why this plugin isn't working with my lapce setup.

I make use of ASDF to manage different versions of various languages. This means the path to the binary is in a non standard location. Although that location is added to my path (see below) if I start lapce via launchpad the plugin fails to work

image image image

However, if I start lapce from the CLI everything works as expected:

image

I've managed to work around this limitation for JavaScript as the lapce plugin allows you to define the location of the LSP. Do you think you could add that (I see it was previously an option)?

superlou commented 7 months ago

I kind of thought it should, though I haven't tried it in a long time: https://github.com/superlou/lapce-python/blob/819f9b016efb136f127500e928e59ce092af51f5/src/main.rs#L40

How does the JavaScript plugin let you specify the LSP location?

ash-sykes commented 7 months ago

Is that the version that is released? Is it not the volt branch? I can configure the path via the UI:

image

The UI options (I believe) are added by updating the volt file with:

[config."volt.serverPath"]
default = ""
description = "Path to typescript-language-server"

[config."volt.serverArgs"]
default = ["--stdio"]
description = "LSP server arguments to pass when launching"

Which looks like this when configured

[lapce-typescript]
"volt.serverPath" = "/Users/ash.sykes/.asdf/shims/typescript-language-server"
"volt.serverArgs" = "--stdio"
superlou commented 7 months ago

...whoops. It's been a long time since I've looked at this. It's definitely the volt branch and it looks like I hard coded for some reason:

https://github.com/superlou/lapce-python/blob/c06b31b73d6558a41440f100dbc0cf51ec93731c/src/main.rs#L41

I have a vague memory of there being some limitation in specifying URN paths via the config file, but that doesn't sound right.

I'm happy to accept a PR to add this; otherwise, it might take me a bit to get to.

ash-sykes commented 7 months ago

I have zero rust knowledge so I'm not confident I will be able to fix this but I can give it a go...

ash-sykes commented 7 months ago

@superlou I've opened a PR that adds the ability to do this and works as expected :) you might need to touch things up as this was pretty much a copy paste to make it work

image
superlou commented 7 months ago

Merged #18 and published the plugin. It looks like it worked, but please double-check that it loads from the plugin host. Thanks!

ash-sykes commented 7 months ago

I tried it and it didn't work :( I did notice that the size of the WASM file was bigger than the one I locally built. If I replace that with the one from my local build it works fine

/Users/ash.sykes/code/lapce-python/target/wasm32-wasi/debug/lapce-python.wasm

ash-sykes commented 7 months ago

lapce-python.wasm.zip If it helps

superlou commented 7 months ago

Crud, it's because I think the file didn't copy. You're right that it's the wrong one. As soon as I can get to my computer I'll fix it.

On Thu, Mar 28, 2024, 6:06 AM Ash Sykes @.***> wrote:

lapce-python.wasm.zip https://github.com/superlou/lapce-python/files/14787578/lapce-python.wasm.zip If it helps

— Reply to this email directly, view it on GitHub https://github.com/superlou/lapce-python/issues/17#issuecomment-2024828255, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFNIP7CEWTIBUEO3E5ROSDY2PMQJAVCNFSM6AAAAABFKVWQUOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMRUHAZDQMRVGU . You are receiving this because you were mentioned.Message ID: @.***>

superlou commented 7 months ago

Try now, please. Should be published as 0.3.4 (and actually built for release).

ash-sykes commented 7 months ago

💥 that did it!

superlou commented 7 months ago

Sorry about that. Thanks for your patience and great contribution!