reasonml-editor / vscode-reasonml

OCaml & Reason support for Visual Studio Code
Apache License 2.0
491 stars 62 forks source link

Prioritize project-local bsb over global bsb #203

Closed bsansouci closed 6 years ago

bsansouci commented 6 years ago

I'll start by saying that I don't use VCS and I know very little about it (really appreciate your work though!) It seems like VCS will run bsb each time you save the file (which helps merlin I'm guessing), but it'll assume global merlin. This has caused this issue. I can imagine more issues can come as people use different versions of bsb on different projects (if you ran bsb2 on a bsb3 project, you'd create subtle runtime bugs for example).

Would it be possible to check if node_modules/bs-platform/lib/bsb.exe exists, and call that instead?

Thanks!

ghost commented 6 years ago

It sounds like this is a problem related to how the current environment (i.e., PATH) is configured. Code will use whatever resolves first with regard to the bsb binary according to that. Can you test and see if when you start Code from the terminal this way if it uses the bsb you expect?

PATH=$(pwd)/node_modules/bs-platform/lib:$PATH code .

It would be possible to configure the extension to try and look for a local version of bsb anyway but I'm a little hestitant to make that change since it would mean overriding the user environment in some cases. It sounds to me like this is a configuration issue that would be better addressed when the tooling is installed if possible.

bsansouci commented 6 years ago

It's a good point that one might want to use the env to communicate paths with VSC. How about we add a way to toggle that behavior through an option in the config. Something like "resolvePathsUsingEnv"?

I don't recommend people install bsb globally actually, I tell them to use npm scripts which prioritize node_modules (and all of my example projects do that by default). I think it's safer approach, to avoid mixups.

jchavarri commented 6 years ago

I think in most cases, projects use the bsb version installed in node_modules.

It seems that VSCode doesn't support ${workspaceFolder} in package.json default values. Using ./node_modules/.bin/bsb works though, but we would have to add a configuration for each platform in the future, as indicated in https://github.com/Microsoft/vscode/issues/22829.

If you're ok with adding OSX / Linux for now, I can open a PR that changes:

Another choice could be to add these relative paths in the lang server.