NoahTheDuke / coc-clojure

coc.nvim plugin for clojure-lsp
Mozilla Public License 2.0
32 stars 3 forks source link

Improve the logic around auto downloading clojure-lsp #5

Closed dharrigan closed 2 years ago

dharrigan commented 2 years ago

Hi,

Recently, coc-clojure gained the ability to automatically download a version of clojure-lsp from github, if it detects that clojure-lsp is not on the $PATH. Whilst this is a great and welcome feature to lower the barrier of entry for people to use clojure-lsp, I think it's not quite there yet in terms of robustness.

  1. I already have clojure-lsp on my path (in /bin/clojure-lsp), but upon firing up neovim with the latest coc-clojure I was asked if I wanted to download clojure-lsp, since coc-clojure could not detect clojure-lsp on my path.
  2. From time to time, I also build clojure-lsp to avail of some unreleased code changes, thus compiling it as a jar. I set the executable path to clojure-lsp, so I would hope that this would be taken into consideration to.
  3. I think it would be advantageous to have a boolean toggle that disables this feature altogether (it could default to true, i.e., download if not on path). Otherwise, since I work on various OS's, I have to then have a custom coc-settings.json for each OS to cater for the different paths of where clojure-lsp is. Since I don't need to automatically download it (it's installed as part of my development setup anyway), then I don't need to have coc-clojure check the path to see if it can find it or not.
  4. Finally, there are various versions of clojure-lsp in Linux-land, namely there is a statically linked binary and a dynamically linked binary. Some people perfer to download static, others dynamic, as is their choice to do so. However, coc-clojure only downloads the jar version without regard to the static version or dynamic version(static being a tad faster, and more resiliant to 3rd party libaries changing). A jar version will load up slower than the natively compiled version. Thus, not only should the user have the choice to download a native version, but which type of native version should also be presented. You can sorta see the sense here of just being able to disable the autodownload and let those users download/install whichever version they want (and less savvy users, with autodownload on, the jar version is downloaded).

Thank you..

-=david=-

NoahTheDuke commented 2 years ago

Thank you so much for the thoughtful issue!

I believe I have solved 1 and 2. It seems I wasn't correctly checking both that the executable existed on the PATH or that when resolved it existed.

As for 3, I'm not sure exactly what set up you have, but as you'll see from the code for version 0.0.7, coc-clojure downloads the native for all known cases (lifting this code directly from Calva) and only chooses the "standalone-jar" when faced with an unknown platform/architecture. I'm not sure how else to handle this.

I won't close this just yet as I haven't actually released the new version, but I'll be sure to comment when I do.

dharrigan commented 2 years ago

Ah,

You're right, I didn't follow the code as I should have, so yes, 4 is covered thank you. Have you considerd adding a boolean toggle to disable this altogether? Consider that every time that neovim is launched, a file system lookup will need to be performed, adding slightly to the delay before coc-clojure is ready for action. Having a toggle would just turn it off completely, thus saving an expensive IO operation.

Thank you.

NoahTheDuke commented 2 years ago

Yes, I have added a flag. Sorry, misread the question numbers and forgot to answer that one.

dharrigan commented 2 years ago

Fanastico! Thank you. Looking forward to trying it out when you release the code :-)

-=david=-

NoahTheDuke commented 2 years ago

I have published 0.0.8. It's late so I'm going to bed, but let me know if more issues arise!