Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add node directory to PATH correctly on Windows #111

Merged
merged 1 commit into from
Oct 28, 2023
Merged

Conversation

rchl
Copy link
Member

@rchl rchl commented Oct 27, 2023

We've added wrong node directory path to PATH on Windows thus scripts weren't able to find node during npm install.

Resolves sublimelsp/LSP-intelephense#93

@predragnikolic
Copy link
Member

CASE #1
Test Windows when I do not have a global node version installed

Cannot reproduce the issue in this case.
lsp_utils can find the local node version, even without this PR. (as long as the user pressed "Download Node.js", in the popup that says "Could not start LSP-* due to not being able to find Node.js runtime on the PATH",
and downloaded the new Node18 version)

CASE #2
Test Windows when I do have a global node version installed

I haven't tested this case.

@rchl
Copy link
Member Author

rchl commented Oct 28, 2023

I've reproduced this issue in UTM so you don't really have to but if you want then make sure that:

  • you are using the Node and not the Electron runtime
  • the LSP-intelephense is not already installed in Package Storage
  • you are trying with latest version of LSP-intelephense

@predragnikolic
Copy link
Member

Thanks, following those steps I will get an error when the server is installed for the first time.
And this PR fixes that.


I don't seem to reproduce this issue with other LSP node based servers on windows?
Did you?

@rchl
Copy link
Member Author

rchl commented Oct 28, 2023

It appears to be specific to postinstall script of some of the package so yeah, it wouldn't reproduce for every server.

@rchl rchl merged commit cb94b4f into main Oct 28, 2023
@rchl rchl deleted the fix/node_path branch October 28, 2023 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error during activation of LSP-intelephense on Windows 11 in ST4
2 participants