Giter Site home page Giter Site logo

Comments (24)

williamboman avatar williamboman commented on September 12, 2024 1

Hey! This is definitely on my mind as well. I do want to be careful adding certain features, especially those that incur a lot of added complexity. It'd take a lot of convincing and then some more for me to consider allowing global/system-wide or even user-local installations (really, anything outside of nvim-lsp-installer's dedicated file system boundaries). But maybe I misunderstand your 2nd and 3rd points?

  • you can uninstall a package by name instead of nuking the folder

I'm not following this one, could you perhaps give an example?

  • use the package full name in the info panel instead of the abbreviated name used by lspconfig. (since you actually need npm install bash-language-server for bashls)

Yeah some of the lspconfig names are pretty opaque in terms of what's actually installed. I definitely think it makes sense to be more transparent with what'll actually end up being installed. I have an idea where this kind of metadata would quite easily be able to be inferred from each server's installation steps. For example, a server that has a npm.packages { "typescript-language-server", "typescript" } installer configured, one could easily infer that:

  1. a Node.js runtime with npm installed is required to install this server
  2. npm package typescript-language-server will be installed
  3. npm package typescript will be installed

allow an easier way to fetch metadata for some fancy LspInstallInfo per server

Yeah, I'm wondering what would be interesting to display here. I recently added a relative timestamp showing when each server was installed. I think this is the cheapest outlet to experiment with :)

All in all, there's only really two features remaining before I feel like this plugin is more or less feature complete, and that is 1) a nicer API to ensure that a set of provided servers are installed (easy to impl), and 2) functionality that allow you to upgrade servers when upgrades are available (either through some UI notification, or automatically behind the scenes). Anything else feels somewhat redundant

from nvim-lsp-installer.

williamboman avatar williamboman commented on September 12, 2024 1

That makes sense too I think! I'd probably opt for the server directory itself rather than the full command. The full command should be available via :LspInfo already, I also hope to be able to remove these cmd overrides from this plugin altogether in the future:tm:.

from nvim-lsp-installer.

williamboman avatar williamboman commented on September 12, 2024 1

Some more progress on this: https://asciinema.org/a/fmqCxtLUR4HrqLcbteNaMGQqR

from nvim-lsp-installer.

williamboman avatar williamboman commented on September 12, 2024 1

Closing this thread as we've come pretty far in terms of displaying metadata, which was the topic of this issue.

As for what was discussed for version checking of servers to allow updating outdated ones - this will be continued in #194.

from nvim-lsp-installer.

williamboman avatar williamboman commented on September 12, 2024 1

Yeah that won't be forgotten in #194! As long as the data is available, it's really easy to add to the UI

from nvim-lsp-installer.

kylo252 avatar kylo252 commented on September 12, 2024

I actually retract the idea of user-level install because you don't want to be dealing with people who didn't setup their package managers correctly. In a perfect world, the user would have configured all the correct paths for their tools, but that's sadly not the case: LunarVim/LunarVim#1468, LunarVim/LunarVim#1463 😒


Regarding the other points, let's take this example.

:LspInstall sumneko_lua pyright yamlls jedi_language_server tsserver

Here's what I got:

lsp_servers
β”œβ”€β”€ jedi_language_server
β”‚Β Β  └── venv
β”œβ”€β”€ python
β”‚Β Β  β”œβ”€β”€ node_modules
β”‚Β Β  β”œβ”€β”€ package.json
β”‚Β Β  └── package-lock.json
β”œβ”€β”€ sumneko_lua
β”‚Β Β  β”œβ”€β”€ [Content_Types].xml
β”‚Β Β  β”œβ”€β”€ extension
β”‚Β Β  └── extension.vsixmanifest
β”œβ”€β”€ tsserver
β”‚Β Β  β”œβ”€β”€ node_modules
β”‚Β Β  β”œβ”€β”€ package.json
β”‚Β Β  └── package-lock.json
└── yaml
    β”œβ”€β”€ node_modules
    β”œβ”€β”€ package.json
    └── package-lock.json

It might be good if jedi were to use the python folder as well. But the is currently not attainable, because the pyright ninstaller doesn't know how search for pyright anymore. It will resort to a more aggressive approach by removing the root folder, in this case python, completely instead of relying on the package manager to clean it up.

All in all, there's only really two features remaining before I feel like this plugin is more or less feature complete, and that is 1) a nicer API to ensure that a set of provided servers are installed (easy to impl)

This is why I feel this such an easy fix/feature. Consider this, which will be even easier internally:

fd 'package.json' --max-depth=2 | xargs dirname | xargs -I {} npm ls -C {}

# 	  results

#	  python@ /home/hatsu/.local/share/nvim/lsp_servers/python
#	  └── [email protected]
	  
#	  tsserver@ /home/hatsu/.local/share/nvim/lsp_servers/tsserver
#	  β”œβ”€β”€ [email protected]
#	  └── [email protected]
	  
#	  yaml@ /home/hatsu/.local/share/nvim/lsp_servers/yaml
#	  └── [email protected]

Do you see how it would make things just a slightly bit easier to display and validate that we're using v0.22.0 of yaml-language-server. You can now do npm rm yaml-language-server and then be free to remove the folder afterwards.

Other observations:

  • Again, this is not really specific to node modules, it just so happens that most servers are written in node, for better or for worse. And even though npm is slow, it can still give you some useful info npm view yaml-language-server. Hopefully there's some fast Rust tool that's good at dealing with nodejs mess :)
  • This will allow for less hacky ways to pin a version of language-server, use a different source/fork or any other useful options that can be passed to the package manager.

from nvim-lsp-installer.

williamboman avatar williamboman commented on September 12, 2024

It might be good if jedi were to use the python folder as well. But the is currently not attainable, because the pyright ninstaller doesn't know how search for pyright anymore. It will resort to a more aggressive approach by removing the root folder, in this case python, completely instead of relying on the package manager to clean it up.
[...]
Do you see how it would make things just a slightly bit easier to display and validate that we're using v0.22.0 of yaml-language-server. You can now do npm rm yaml-language-server and then be free to remove the folder afterwards.

I'm not following why you'd either a) want to put this kind of "semantic" state in the file system hierarchy in the first place, and b) why exactly where servers end up being installed is a concern for the (average) end user (other than the guarantee that it's inside the data stdpath [i.e., not system-wide, globally, or user-local]).

The fact that there exist a python directory name (which is allocated to pylsp iirc) is just a legacy thing. In fact soon the default behavior will be that these directories will take the same name as their corresponding lspconfig name (apart from the existing servers with non-matching names, to avoid breaking changes).

Don't you think it'd be much easier to contain this kind of state/logic in the Lua layer? Or I might still be misunderstanding.

Do you see how it would make things just a slightly bit easier to display and validate that we're using v0.22.0 of yaml-language-server. You can now do npm rm yaml-language-server and then be free to remove the folder afterwards.

Perhaps from a shell scripting perspective. I think the same can easily be implemented in a platform-agnostic manner if each server was responsible for reporting such metadata (so for npm-based installations we'd spawn subprocesses that report back a single package version). This could easily be parallelized with libuv.

Again, this is not really specific to node modules, it just so happens that most servers are written in node, for better or for worse. And even though npm is slow, it can still give you some useful info npm view yaml-language-server. Hopefully there's some fast Rust tool that's good at dealing with nodejs mess :)
[...]
This will allow for less hacky ways to pin a version of language-server, use a different source/fork or any other useful options that can be passed to the package manager.

If my quick check was correct, 23/50 servers are currently installed via npm, so it's actually not even a majority. But yeah I think the extra capabilities each server would offer (versioning, pinning, whatnot) will be entirely constrained by how it's installed in the first place. npm-based distributions can be heavily standardized whereas servers that require custom build scripts will not.

from nvim-lsp-installer.

williamboman avatar williamboman commented on September 12, 2024

I've started experimenting with this now, preview: https://asciinema.org/a/Bj6Q03SweLj6gJ7EGNqIr4beU. This is actually pretty easy to implement thanks to the state-driven UI framework that is in place. The question is what's interesting to show.. I'm thinking the following, for starterss:

For installed servers:

  • Installation date
  • Installed version
  • Latest available version (perhaps also do some diffing and highlight this if it differs from the currently installed version)
  • LSP server homepage (e.g. its GitHub repo or public website if more suitable)

For uninstalled servers:

  • Version that would be installed
  • What exactly will be installed (this one I might postpone til a later iteration)
  • LSP server homepage (e.g. its GitHub repo or public website if more suitable)

Anything else that might be interesting to display?

from nvim-lsp-installer.

wookayin avatar wookayin commented on September 12, 2024

This looks like a great feature. Although obvious, how about displaying the full installation path (the LSP server directory or the cmd executable) so that one can navigate if they'd want some troubleshooting?

from nvim-lsp-installer.

kylo252 avatar kylo252 commented on September 12, 2024

I've started experimenting with this now, preview: asciinema.org/a/Bj6Q03SweLj6gJ7EGNqIr4beU.

I'm actually working on implementing something similar just now

Demo

j0ZtZSq0UK

There was a small problem where it would try to attach the server immediately if I had the a file with the associated filetype opened.

Race condition(?)

P3enb02pVa

The solution was disabling vim.cmd [[ do User LspAttachBuffers ]].

from nvim-lsp-installer.

williamboman avatar williamboman commented on September 12, 2024

That's so cool 😎 ! However this seems to be a tighter integration between LunarVim and nvim-lsp-installer, and not so much about extending the feature set of nvim-lsp-installer itself - or am I missing something πŸ€” ?

Regarding the race condition - this might be because the .setup() function of the server is called before the server is successfully installed? vim.cmd [[ do User LspAttachBuffers ]] will essentially only trigger lspconfig to attach all existing buffers to potential server candidates that have been registered with lspconfig (i.e. through .setup())

from nvim-lsp-installer.

williamboman avatar williamboman commented on September 12, 2024

I also have plans on adding something similar to what you're doing actually! I just pushed a somewhat finished version of it to ensure-installed-servers

from nvim-lsp-installer.

kylo252 avatar kylo252 commented on September 12, 2024

Oh my bad, I misread the command name and thought it was an auto-installer!

The two features that I would like to see, and can even help with, are:

  1. allow dismissing the split quickly and also allow the use of an actual popup
  2. it has been mentioned before, but I'd like to see some kind of feedback from the installer that is not coupled to the UI state.

I feel like the last point might be tricky, but I hope not.

from nvim-lsp-installer.

williamboman avatar williamboman commented on September 12, 2024
  • allow dismissing the split quickly and also allow the use of an actual popup

Yeah now most interactions with the plugin will be centered around the UI window. Implementing a floating window instead of a regular vertical split should actually be pretty straight forward. The code is a bit so-so and not really considered stable yet, but this is the part responsible for opening the window. I'd like to avoid having options for everything so I think either a floating window or vertical split window should be supported, not both. I don't have any strong opinions so happy to receive PR changing to floating window!

2. it has been mentioned before, but I'd like to see some kind of feedback from the installer that is not coupled to the UI state.

This should be possible today. The drawback is that if you bypass the APIs that are attached to the UI window, the UI window will become out of sync with any operations done outside of it. What you're interested in would be Server:install_attached(). Here's how the UI calls that function, for reference.

edit: Everything's technically possible:tm:, so we could make sure to funnel everything through some centralized state management while allowing for 3rd party use cases (basically lift the state management from the status-win/init.lua module and spread it globally across the plugin). I'm not too compelled to make such a change at this point in time though, I'd like things to stabilize a bit more.

from nvim-lsp-installer.

kylo252 avatar kylo252 commented on September 12, 2024

edit: Everything's technically possibleℒ️, so we could make sure to funnel everything through some centralized state management while allowing for 3rd party use cases (basically lift the state management from the status-win/init.lua module and spread it globally across the plugin). I'm not too compelled to make such a change at this point in time though, I'd like things to stabilize a bit more.

Yeah that's understandable, take a look at #98 and see if it's good enough.

from nvim-lsp-installer.

kylo252 avatar kylo252 commented on September 12, 2024

Some more progress on this: asciinema.org/a/fmqCxtLUR4HrqLcbteNaMGQqR

How's the performance? I messed around a bit with yarn which seems slightly faster in fetching data, but the initial setup is way too annoying compared to npm. I can make a PR for it if you'd like to see/support it.

from nvim-lsp-installer.

williamboman avatar williamboman commented on September 12, 2024

Do you mean in terms of latency? It's perfectly fine for me, although the "latest version" is network-bound so ymmv. What happens is that when you expand a server it'll gather each individual data point separately in a subprocess via libuv, so in terms of perceived performance I don't think yarn vs npm will matter

from nvim-lsp-installer.

kylo252 avatar kylo252 commented on September 12, 2024

Do you mean in terms of latency? It's perfectly fine for me, although the "latest version" is network-bound so ymmv. What happens is that when you expand a server it'll gather each individual data point separately in a subprocess via libuv, so in terms of perceived performance I don't think yarn vs npm will matter

Good to know! I've heard conflicting tales about the superiority of yarn. However, I could not get yarn init to shut up about either the license file or the default settings. It's made to be used interactively in the first place, smh..

from nvim-lsp-installer.

williamboman avatar williamboman commented on September 12, 2024

Hehe maybe you need some NON_INTERACTIVE env set πŸ€·β€β™‚οΈ. Another reason to stick with npm is that it's generally shipped together with Node, whereas Yarn is less so (maybe this has changed). I do want to enable the possibility to run server installations via Yarn instead of npm, as npm has a tendency to cause issues when on corp VPNs/proxies. But that's a separate topic

from nvim-lsp-installer.

williamboman avatar williamboman commented on September 12, 2024

Note to self: I realized for node packages we could probably bypass npm entirely through something like:

local package_json = vim.json.decode(fs.read_file(path.concat { server.root_dir, "node_modules", package, "package.json" })
package_json.version

from nvim-lsp-installer.

williamboman avatar williamboman commented on September 12, 2024

And for fetching latest versions https://api.npms.io might be more interesting than going through npm cli, like:

local package = vim.json.decode(fetch(("https://api.npms.io/v2/package/%s"):format(package_name)))
local latest_version = package.collected.metadata.version

from nvim-lsp-installer.

kylo252 avatar kylo252 commented on September 12, 2024

And for fetching latest versions https://api.npms.io might be more interesting than going through npm cli, like:

local package = vim.json.decode(fetch(("https://api.npms.io/v2/package/%s"):format(package_name)))
local latest_version = package.collected.metadata.version

I recently learned that npm view fetches data from the online registry, which makes it slow. Is that the reason why you don't want to go through local npm calls?

from nvim-lsp-installer.

TeoDev1611 avatar TeoDev1611 commented on September 12, 2024

Note to self: I realized for node packages we could probably bypass npm entirely through something like:

local package_json = vim.json.decode(fs.read_file(path.concat { server.root_dir, "node_modules", package, "package.json" })
package_json.version

Can you check this can be help for dont use the api :)

https://stackoverflow.com/questions/10972176/find-the-version-of-an-installed-npm-package

from nvim-lsp-installer.

wookayin avatar wookayin commented on September 12, 2024

What about versions? (currently installed version as well as latest, #194) This one would be another important metadata to display.

from nvim-lsp-installer.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    πŸ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. πŸ“ŠπŸ“ˆπŸŽ‰

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❀️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.