kabouzeid / nvim-lspinstall

Provides the missing :LspInstall for nvim-lspconfig
MIT License
526 stars 67 forks source link

Supporting yarn #167

Open ssbanerje opened 2 years ago

ssbanerje commented 2 years ago

Is there any way to configure the install scripts to use yarn rather than npm?

It seems like currently the install script using npm is hard-coded for almost every language server.

derektata commented 2 years ago

Yes, this can be done.

Example:

! test -f package.json && yarn init -y --scope=lspinstall || true
yarn add bash-language-server@latest

You just need to substitute install with add

ssbanerje commented 2 years ago

Well this would have to be done in every lsp install script?

derektata commented 2 years ago

AFAIK, the yaml install script was written with yarn in mind.

Maybe we could have the system check for npm or yarn prior to running the install script, and implement logic from there.

kabouzeid commented 2 years ago

what would be the benefit?

derektata commented 2 years ago

I've been thinking about it for the past few hours, and it's kind of redundant to do this. Part being that yarn is typically installed via npm on non-Unix systems. It would also make the install part of the script a little longer doing the system checks for either command.

ssbanerje commented 2 years ago

Well...

The benefit will be that there is a faster install on systems with Yarn.

Also if the systems only have Yarn and not NPM, then the whole plugin fails.

@derektata I am not sure about non-UNIX systems, but I know that it can be common to install only yarn on Linux based systems. I guess it eventually comes down to a users preference. But the whole plugin should not stop working because of that.

I have been using coc and recently moved to the builtin lsp. There it checks which pkg manager is installed and uses which ever it finds.

kabouzeid commented 2 years ago

npm is almost always installed when yarn is installed. I haven't noticed faster install times when installing a single package.

derektata commented 2 years ago

@kabouzeid, I can understand where @ssbanerje is coming from when it comes to only having one installed.

So I have an idea

  if [ ! -x $NPM ] && [ ! -x $YARN ]; then
    # if npm and yarn are not on the system
    printf "\nCan't find npm or yarn on the system\n"
  elif [ ! -x $NPM ]; then
    # if npm is not on the system
    printf "\nNpm not found..."
    # run yarn script
  elif [ ! -x $YARN ]; then
    # if yarn is not on the system
    printf "\nYarn not found..."
    # run npm script
  fi
derektata commented 2 years ago

Okay so after testing this code, I'm running into node versioning incompatibilities when using yarn, but that doesn't seem to be a problem when using npm.

recording

ssbanerje commented 2 years ago

@derektata That can be an annoying problem to fix! But it is really interesting that npm is not doing this check given its in the package file (https://github.com/angular/vscode-ng-language-service/blob/3014713e18398fbec4773686d989ad11d48f2952/server/package.json#L11-L13).

@kabouzeid Yeah. I think it is a yarn 1.x vs 2.x thing. Yarn would provide deb/apk for previous releases. I think the latest release uses NPM.

derektata commented 2 years ago

@ssbanerje I'll look into this more tomorrow, I just leveled up and enjoying a day off 🎂

derektata commented 2 years ago

@ssbanerje Good news, I found a fix for this, and I'll be submitting a pull request in a little while with the updated code.