From 0d53e64402e2fa8ad99a313a14995812a863ab5a Mon Sep 17 00:00:00 2001 From: Ryan Clary <9618975+mrclary@users.noreply.github.com> Date: Mon, 5 Feb 2024 12:46:53 -0800 Subject: [PATCH 1/6] Allow extra_paths to be placed in front of sys.path. * Add pylsp.plugins.jedi.prioritize configuration key Note: when safe to break API, Document.sys_path should be removed. --- CONFIGURATION.md | 1 + pylsp/config/schema.json | 5 +++++ pylsp/workspace.py | 23 +++++++++++++---------- 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/CONFIGURATION.md b/CONFIGURATION.md index bb07fce9..feb536c9 100644 --- a/CONFIGURATION.md +++ b/CONFIGURATION.md @@ -21,6 +21,7 @@ This server can be configured using the `workspace/didChangeConfiguration` metho | `pylsp.plugins.flake8.select` | `array` of unique `string` items | List of errors and warnings to enable. | `null` | | `pylsp.plugins.jedi.auto_import_modules` | `array` of `string` items | List of module names for jedi.settings.auto_import_modules. | `["numpy"]` | | `pylsp.plugins.jedi.extra_paths` | `array` of `string` items | Define extra paths for jedi.Script. | `[]` | +| `pylsp.plugins.jedi.prioritize` | `boolean` | Whether to place extra_paths at the beginning (true) or end (false) of `sys.path` | `false` | | `pylsp.plugins.jedi.env_vars` | `object` | Define environment variables for jedi.Script and Jedi.names. | `null` | | `pylsp.plugins.jedi.environment` | `string` | Define environment for jedi.Script and Jedi.names. | `null` | | `pylsp.plugins.jedi_completion.enabled` | `boolean` | Enable or disable the plugin. | `true` | diff --git a/pylsp/config/schema.json b/pylsp/config/schema.json index 2259f1cc..78daf0aa 100644 --- a/pylsp/config/schema.json +++ b/pylsp/config/schema.json @@ -151,6 +151,11 @@ }, "description": "Define extra paths for jedi.Script." }, + "pylsp.plugins.jedi.prioritize": { + "type": "boolean", + "default": false, + "description": "Whether to place extra_paths at the beginning (true) or end (false) of `sys.path`" + }, "pylsp.plugins.jedi.env_vars": { "type": [ "object", diff --git a/pylsp/workspace.py b/pylsp/workspace.py index 139028cc..84efe6f3 100644 --- a/pylsp/workspace.py +++ b/pylsp/workspace.py @@ -521,6 +521,7 @@ def jedi_script(self, position=None, use_document_path=False): extra_paths = [] environment_path = None env_vars = None + prioritize = False if self._config: jedi_settings = self._config.plugin_settings( @@ -537,19 +538,21 @@ def jedi_script(self, position=None, use_document_path=False): extra_paths = jedi_settings.get("extra_paths") or [] env_vars = jedi_settings.get("env_vars") + prioritize = jedi_settings.get("prioritize") - # Drop PYTHONPATH from env_vars before creating the environment because that makes - # Jedi throw an error. + # Drop PYTHONPATH from env_vars before creating the environment to + # ensure that Jedi can startup properly without module name collision. if env_vars is None: env_vars = os.environ.copy() env_vars.pop("PYTHONPATH", None) - environment = ( - self.get_enviroment(environment_path, env_vars=env_vars) - if environment_path - else None - ) - sys_path = self.sys_path(environment_path, env_vars=env_vars) + extra_paths + environment = self.get_enviroment(environment_path, env_vars=env_vars) + + sys_path = list(self._extra_sys_path) + environment.get_sys_path() + if prioritize: + sys_path += extra_paths + sys_path + else: + sys_path += sys_path + extra_paths project_path = self._workspace.root_path # Extend sys_path with document's path if requested @@ -559,7 +562,7 @@ def jedi_script(self, position=None, use_document_path=False): kwargs = { "code": self.source, "path": self.path, - "environment": environment, + "environment": environment if environment_path else None, "project": jedi.Project(path=project_path, sys_path=sys_path), } @@ -585,8 +588,8 @@ def get_enviroment(self, environment_path=None, env_vars=None): return environment def sys_path(self, environment_path=None, env_vars=None): + # TODO: when safe to break API, remove this method. # Copy our extra sys path - # TODO: when safe to break API, use env_vars explicitly to pass to create_environment path = list(self._extra_sys_path) environment = self.get_enviroment( environment_path=environment_path, env_vars=env_vars From d08af9434420728a9c3d34ec3208336a46f5b82d Mon Sep 17 00:00:00 2001 From: Ryan Clary <9618975+mrclary@users.noreply.github.com> Date: Tue, 20 Feb 2024 08:59:23 -0800 Subject: [PATCH 2/6] Update tests --- test/test_workspace.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_workspace.py b/test/test_workspace.py index dabbdf86..8ceabc08 100644 --- a/test/test_workspace.py +++ b/test/test_workspace.py @@ -70,7 +70,7 @@ def test_non_root_project(pylsp, metafiles) -> None: test_uri = uris.from_fs_path(os.path.join(project_root, "hello/test.py")) pylsp.workspace.put_document(test_uri, "assert True") test_doc = pylsp.workspace.get_document(test_uri) - assert project_root in test_doc.sys_path() + assert project_root in test_doc.get_enviroment().get_sys_path() def test_root_project_with_no_setup_py(pylsp) -> None: @@ -79,7 +79,7 @@ def test_root_project_with_no_setup_py(pylsp) -> None: test_uri = uris.from_fs_path(os.path.join(workspace_root, "hello/test.py")) pylsp.workspace.put_document(test_uri, "assert True") test_doc = pylsp.workspace.get_document(test_uri) - assert workspace_root in test_doc.sys_path() + assert workspace_root in test_doc.get_enviroment().get_sys_path() def test_multiple_workspaces_from_initialize(pylsp_w_workspace_folders) -> None: From 32b1894cef7b6436fd6a5065d4a61cd8de00b396 Mon Sep 17 00:00:00 2001 From: Ryan Clary <9618975+mrclary@users.noreply.github.com> Date: Tue, 20 Feb 2024 09:19:01 -0800 Subject: [PATCH 3/6] use sys_path method --- pylsp/workspace.py | 15 ++++++++------- test/test_workspace.py | 4 ++-- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/pylsp/workspace.py b/pylsp/workspace.py index 84efe6f3..b6a7d68a 100644 --- a/pylsp/workspace.py +++ b/pylsp/workspace.py @@ -548,11 +548,8 @@ def jedi_script(self, position=None, use_document_path=False): environment = self.get_enviroment(environment_path, env_vars=env_vars) - sys_path = list(self._extra_sys_path) + environment.get_sys_path() - if prioritize: - sys_path += extra_paths + sys_path - else: - sys_path += sys_path + extra_paths + sys_path = self.sys_path(environment_path, env_vars, prioritize, extra_paths) + project_path = self._workspace.root_path # Extend sys_path with document's path if requested @@ -587,14 +584,18 @@ def get_enviroment(self, environment_path=None, env_vars=None): return environment - def sys_path(self, environment_path=None, env_vars=None): - # TODO: when safe to break API, remove this method. + def sys_path(self, environment_path=None, env_vars=None, prioritize=False, extra_paths=[]): # Copy our extra sys path path = list(self._extra_sys_path) environment = self.get_enviroment( environment_path=environment_path, env_vars=env_vars ) path.extend(environment.get_sys_path()) + if prioritize: + path += extra_paths + path + else: + path += path + extra_paths + return path diff --git a/test/test_workspace.py b/test/test_workspace.py index 8ceabc08..dabbdf86 100644 --- a/test/test_workspace.py +++ b/test/test_workspace.py @@ -70,7 +70,7 @@ def test_non_root_project(pylsp, metafiles) -> None: test_uri = uris.from_fs_path(os.path.join(project_root, "hello/test.py")) pylsp.workspace.put_document(test_uri, "assert True") test_doc = pylsp.workspace.get_document(test_uri) - assert project_root in test_doc.get_enviroment().get_sys_path() + assert project_root in test_doc.sys_path() def test_root_project_with_no_setup_py(pylsp) -> None: @@ -79,7 +79,7 @@ def test_root_project_with_no_setup_py(pylsp) -> None: test_uri = uris.from_fs_path(os.path.join(workspace_root, "hello/test.py")) pylsp.workspace.put_document(test_uri, "assert True") test_doc = pylsp.workspace.get_document(test_uri) - assert workspace_root in test_doc.get_enviroment().get_sys_path() + assert workspace_root in test_doc.sys_path() def test_multiple_workspaces_from_initialize(pylsp_w_workspace_folders) -> None: From 325c2c8d89a1d06d34195ca9319aba604b81a1e8 Mon Sep 17 00:00:00 2001 From: Ryan Clary <9618975+mrclary@users.noreply.github.com> Date: Sat, 9 Mar 2024 21:23:47 -0800 Subject: [PATCH 4/6] Ran ruff format --- pylsp/workspace.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pylsp/workspace.py b/pylsp/workspace.py index b6a7d68a..689e6ad0 100644 --- a/pylsp/workspace.py +++ b/pylsp/workspace.py @@ -584,7 +584,9 @@ def get_enviroment(self, environment_path=None, env_vars=None): return environment - def sys_path(self, environment_path=None, env_vars=None, prioritize=False, extra_paths=[]): + def sys_path( + self, environment_path=None, env_vars=None, prioritize=False, extra_paths=[] + ): # Copy our extra sys path path = list(self._extra_sys_path) environment = self.get_enviroment( From 8f1e3647d8ec1a13611df59f08bd8752ddac5beb Mon Sep 17 00:00:00 2001 From: Ryan Clary <9618975+mrclary@users.noreply.github.com> Date: Sat, 18 May 2024 15:19:43 -0700 Subject: [PATCH 5/6] Apply suggestions from code review Co-authored-by: Carlos Cordoba --- CONFIGURATION.md | 2 +- pylsp/config/schema.json | 2 +- pylsp/workspace.py | 11 +++++------ 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/CONFIGURATION.md b/CONFIGURATION.md index feb536c9..0609169b 100644 --- a/CONFIGURATION.md +++ b/CONFIGURATION.md @@ -21,7 +21,7 @@ This server can be configured using the `workspace/didChangeConfiguration` metho | `pylsp.plugins.flake8.select` | `array` of unique `string` items | List of errors and warnings to enable. | `null` | | `pylsp.plugins.jedi.auto_import_modules` | `array` of `string` items | List of module names for jedi.settings.auto_import_modules. | `["numpy"]` | | `pylsp.plugins.jedi.extra_paths` | `array` of `string` items | Define extra paths for jedi.Script. | `[]` | -| `pylsp.plugins.jedi.prioritize` | `boolean` | Whether to place extra_paths at the beginning (true) or end (false) of `sys.path` | `false` | +| `pylsp.plugins.jedi.prioritize_extra_paths` | `boolean` | Whether to place extra_paths at the beginning (true) or end (false) of `sys.path` | `false` | | `pylsp.plugins.jedi.env_vars` | `object` | Define environment variables for jedi.Script and Jedi.names. | `null` | | `pylsp.plugins.jedi.environment` | `string` | Define environment for jedi.Script and Jedi.names. | `null` | | `pylsp.plugins.jedi_completion.enabled` | `boolean` | Enable or disable the plugin. | `true` | diff --git a/pylsp/config/schema.json b/pylsp/config/schema.json index 78daf0aa..18248384 100644 --- a/pylsp/config/schema.json +++ b/pylsp/config/schema.json @@ -151,7 +151,7 @@ }, "description": "Define extra paths for jedi.Script." }, - "pylsp.plugins.jedi.prioritize": { + "pylsp.plugins.jedi.prioritize_extra_paths": { "type": "boolean", "default": false, "description": "Whether to place extra_paths at the beginning (true) or end (false) of `sys.path`" diff --git a/pylsp/workspace.py b/pylsp/workspace.py index 689e6ad0..5f588226 100644 --- a/pylsp/workspace.py +++ b/pylsp/workspace.py @@ -521,7 +521,7 @@ def jedi_script(self, position=None, use_document_path=False): extra_paths = [] environment_path = None env_vars = None - prioritize = False + prioritize_extra_paths = False if self._config: jedi_settings = self._config.plugin_settings( @@ -538,7 +538,7 @@ def jedi_script(self, position=None, use_document_path=False): extra_paths = jedi_settings.get("extra_paths") or [] env_vars = jedi_settings.get("env_vars") - prioritize = jedi_settings.get("prioritize") + prioritize_extra_paths = jedi_settings.get("prioritize_extra_paths") # Drop PYTHONPATH from env_vars before creating the environment to # ensure that Jedi can startup properly without module name collision. @@ -547,8 +547,7 @@ def jedi_script(self, position=None, use_document_path=False): env_vars.pop("PYTHONPATH", None) environment = self.get_enviroment(environment_path, env_vars=env_vars) - - sys_path = self.sys_path(environment_path, env_vars, prioritize, extra_paths) + sys_path = self.sys_path(environment_path, env_vars, prioritize_extra_paths, extra_paths) project_path = self._workspace.root_path @@ -585,7 +584,7 @@ def get_enviroment(self, environment_path=None, env_vars=None): return environment def sys_path( - self, environment_path=None, env_vars=None, prioritize=False, extra_paths=[] + self, environment_path=None, env_vars=None, prioritize_extra_paths=False, extra_paths=[] ): # Copy our extra sys path path = list(self._extra_sys_path) @@ -593,7 +592,7 @@ def sys_path( environment_path=environment_path, env_vars=env_vars ) path.extend(environment.get_sys_path()) - if prioritize: + if prioritize_extra_paths: path += extra_paths + path else: path += path + extra_paths From 6b853881218d5863450de398d91532f155b3c9c6 Mon Sep 17 00:00:00 2001 From: Ryan Clary <9618975+mrclary@users.noreply.github.com> Date: Sat, 18 May 2024 15:59:21 -0700 Subject: [PATCH 6/6] Ran ruff format --- pylsp/workspace.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/pylsp/workspace.py b/pylsp/workspace.py index 5f588226..23e815bb 100644 --- a/pylsp/workspace.py +++ b/pylsp/workspace.py @@ -547,7 +547,9 @@ def jedi_script(self, position=None, use_document_path=False): env_vars.pop("PYTHONPATH", None) environment = self.get_enviroment(environment_path, env_vars=env_vars) - sys_path = self.sys_path(environment_path, env_vars, prioritize_extra_paths, extra_paths) + sys_path = self.sys_path( + environment_path, env_vars, prioritize_extra_paths, extra_paths + ) project_path = self._workspace.root_path @@ -584,7 +586,11 @@ def get_enviroment(self, environment_path=None, env_vars=None): return environment def sys_path( - self, environment_path=None, env_vars=None, prioritize_extra_paths=False, extra_paths=[] + self, + environment_path=None, + env_vars=None, + prioritize_extra_paths=False, + extra_paths=[], ): # Copy our extra sys path path = list(self._extra_sys_path)