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

Add local node runtime to the PATH when starting the server #64

Closed
rchl opened this issue Jun 8, 2021 · 6 comments · Fixed by #67
Closed

Add local node runtime to the PATH when starting the server #64

rchl opened this issue Jun 8, 2021 · 6 comments · Fixed by #67

Comments

@rchl
Copy link
Member

rchl commented Jun 8, 2021

When using a local node runtime, some servers (LSP-typescript) expect to find node on the PATH so that they can start a node sub-process.

We should add our local node to the PATH.

The PATH can be extended using env object in the settings but that's not ideal as that overrides the original PATH. Also, this should be done in a generic way for all node-based servers.

The relevant code is at https://github.com/sublimelsp/LSP/blob/main/plugin/core/types.py#L663-L664 but I don't have immediate idea how to fix that in a nice way. By nice I mean so that:

  • we do it invisibly from lsp_utils, without adding anything to LSP-* configs
  • we append the path to existing PATH instead of replacing whole PATH
  • the user is still able to set his own env.PATH without breaking that
@rchl
Copy link
Member Author

rchl commented Jun 15, 2021

So this would be an absolute hack:

--- a/plugin/core/types.py
+++ b/plugin/core/types.py
@@ -663,7 +663,10 @@ class ClientConfig:
             command = [a.replace('{port}', str(tcp_port)) for a in command]
         env = os.environ.copy()
         for var, value in self.env.items():
-            env[var] = sublime.expand_variables(value, variables)                                                                                                                                                                     
+            if var == "extra_paths":
+                env["PATH"] += os.path.pathsep + os.path.pathsep.join(value)
+            else:
+                env[var] = sublime.expand_variables(value, variables)
         return TransportConfig(self.name, command, tcp_port, env, listener_socket)

The proper solution would be to extend ClientConfig with another argument/property like extra_path or similar.

@rwols
Copy link
Member

rwols commented Jun 15, 2021

can't you just modify the env of the ClientConfig in AbstractPlugin.on_pre_start ?

@rwols
Copy link
Member

rwols commented Jun 15, 2021

but that's not ideal as that overrides the original PATH

why wouldn't this work

>>> paths = os.environ["PATH"].split(":")
>>> paths.insert(0, "/foo/bar")
>>> os.environ["PATH"] = ":".join(paths)

@rchl
Copy link
Member Author

rchl commented Jun 16, 2021

can't you just modify the env of the ClientConfig in AbstractPlugin.on_pre_start ?

The code that creates the transport overrides the whole PATH with what user defined in env.PATH.
I could create environ copy, then append mine path and then potentially user's PATH but then the behavior would be different when using lsp_utils and when not using regarding the PATH overwriting.
But if that's fine for you then it's fine for me I guess. I don't think overwriting the PATH is generally what people expect to happen anyway.

paths = os.environ["PATH"].split(":")
paths.insert(0, "/foo/bar")
os.environ["PATH"] = ":".join(paths)

Sure but that would modify ST's environment.

@niksy
Copy link

niksy commented Jul 13, 2021

The PATH can be extended using env object in the settings but that's not ideal as that overrides the original PATH. Also, this should be done in a generic way for all node-based servers.

Is this now possible? I’m having similar problem when using TypeScript server described here: sublimelsp/LSP#1671 (comment)

Am I doing everything right?

I even tried setting node_bin in LSP Utils setting as described in comment but it doesn’t work.

@rchl
Copy link
Member Author

rchl commented Jul 13, 2021

You seem to be confused on multiple levels :)

Just check and follow https://lsp.sublimetext.io/troubleshooting/#2-lsp-cannot-find-my-language-server-no-such-file-or-directory-xyz

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 a pull request may close this issue.

3 participants