castwide / vscode-solargraph

A Visual Studio Code extension for Solargraph.
Other
423 stars 25 forks source link

Make the commandPath setting portable #215

Closed CleanCut closed 3 years ago

CleanCut commented 3 years ago

This pull request makes the commandPath setting reliably portable when it is a relative path by making it relative to the VS Code project root instead of relative to VS Code's current working directory (the current working directory varies depending on how and where VS Code was launched).

This makes it so that if you vendor solargraph into, e.g. bin/solargraph in a repository, and vendor your .vscode/settings.json with a "solargraph.commandPath": "bin/solargraph", into the repository as well, then that setting functions on another person's machine, no matter how they launch VS Code.

Other Stuff

CleanCut commented 3 years ago

@castwide Any tips for getting this across the finish line?

At this point, vsce package successfully produces a .vsix file and I can install it with code --install-extension solargraph-0.22.0.vsix BUT obviously I've done something wrong -- I don't get any error messages from VS Code when I load a Ruby file...but nothing works either. When I attempt to run a command via cmd-shift-p, the commands show up in the list, but when I select any of them I get an error like Command 'Solargraph: Restart Solargraph' resulted in an error (command 'solargraph.restart' not found).

I'm pretty sure it has to do with all the updating-of-stuff, but I'm not sure where to look next.

I've double-checked the package.json format, and as far as I can tell the formatting / values are all correct. πŸ€”

CleanCut commented 3 years ago

@castwide Found it! Fixed it! It's working great for me! Ready to review for merging!

I don't know why the is-relative package didn't work, but I reported that upstream and sidestepped the issue by moving the logic into this PR.

CleanCut commented 3 years ago

@castwide With advice from https://github.com/jonschlinkert/is-relative/issues/5, I was able to update the PR to use the is-relative package correctly, rather than duplicating the code manually. πŸ₯³

Looking forward to getting this reviewed, merged, and released. 😎

CleanCut commented 3 years ago

I pushed a commit to fix the treatment of bare executable names -- they were being treated as relative paths. Now they are searched for in the shell's environment (in PATH) like before.

I'd love to get a review on this!

castwide commented 3 years ago

Looks good, thanks!

CleanCut commented 3 years ago

Thank you so much! I'm looking forward to the release. πŸ˜„