search

elixir-tools / elixir-tools.nvim

Neovim plugin for Elixir
MIT License
398 stars 29 forks source link

Should nextls cmd be expanded? #168

Closed dvic closed 10 months ago

dvic commented 10 months ago

https://github.com/elixir-tools/elixir-tools.nvim/blob/a532ade5d55156e103abec37cd70b63a940b6495/lua/elixir/nextls/init.lua#L85

I tried using mason and was expecting cmd = "nextls" to work (mason binaries are in my path). Do we want to expand the cmd property or keep it as is?

mhanberg commented 10 months ago

I think this is a bug, may have regressed at some point.

If the user supplies their own value for cmd, we shouldn't validate that it exists.

dvic commented 10 months ago

I think this is a bug, may have regressed at some point.

If the user supplies their own value for cmd, we shouldn't validate that it exists.

Sure, I'll submit a PR to remove that line. What about that port check? If it's a number we also don't want to prompt the user? What is the idea behind this check?

mhanberg commented 10 months ago

The line still needs to be there somewhere.

It checks if nextls is installed and if it isn't, it will prompt the user to confirm installing it.

Also, if the user supplies their own cmd, we shouldnt start the server with the auto update env var, as the are assumably managing it themselves.

dvic commented 10 months ago

hah okay now i’m confused, maybe i’m missing something but:

If the user supplies their own value for cmd, we shouldn't validate that it exists

vs

The line still needs to be there somewhere.

It checks if nextls is installed and if it isn't, it will prompt the user to confirm installing it.

so we should check if it exists or not? If I read the code correctly, we currently prompt if port is not a number and the cmd does not exist (or not provided), correct?

dvic commented 10 months ago

Also, if the user supplies their own cmd, we shouldnt start the server with the auto update env var, as the are assumably managing it themselves.

clear

mhanberg commented 10 months ago

If the cmd is not provided by the user, we don't need to check that it exists, we just assume that it exists.

if they do not provide it, (and use the default value), we are assuming we are using a binary managed by the plugin, so we see if it exists to know if we should download it or not.

mhanberg commented 10 months ago

https://github.com/elixir-tools/elixir-tools.nvim/blob/main/lua/elixir/init.lua#L80-L84

i think you can key off of whether or not auto update is true to determine if we need to check for the existence of the binary.

dvic commented 10 months ago

If the cmd is not provided by the user, we don't need to check that it exists, we just assume that it exists.

if they do not provide it, (and use the default value), we are assuming we are using a binary managed by the plugin, so we see if it exists to know if we should download it or not.

thanks for clarifying, now it’s clear :)