From a1672bebb73b130f0de918f3dc0d33879d84c064 Mon Sep 17 00:00:00 2001 From: Lukas Geiger Date: Thu, 28 Dec 2017 15:47:41 +0100 Subject: [PATCH 1/2] Completion: Remove pluggy racing and disable Rope by default This allows the user to choose between Rope and Jedi for completions. It also remove the pluggy racing since Jedi's cache isn't thread safe. Closes #189 --- pyls/_utils.py | 27 --------------------------- pyls/hookspecs.py | 7 ++++++- pyls/plugins/jedi_completion.py | 2 +- pyls/plugins/rope_completion.py | 2 +- pyls/python_ls.py | 22 ++++++++-------------- test/plugins/test_completion.py | 4 ++-- vscode-client/package.json | 15 ++++++++------- 7 files changed, 26 insertions(+), 53 deletions(-) diff --git a/pyls/_utils.py b/pyls/_utils.py index 81414312..fb353c3c 100644 --- a/pyls/_utils.py +++ b/pyls/_utils.py @@ -89,33 +89,6 @@ def _merge_dicts_(a, b): return dict(_merge_dicts_(dict_a, dict_b)) -def race_hooks(hook_caller, pool, **kwargs): - """Given a pluggy hook spec, execute impls in parallel returning the first non-None result. - - Note this does not support a lot of pluggy functionality, e.g. hook wrappers. - """ - impls = hook_caller._nonwrappers + hook_caller._wrappers - log.debug("Racing hook impls for hook %s: %s", hook_caller, impls) - - if not impls: - return None - - def _apply(impl): - try: - return impl, impl.function(**kwargs) - except Exception: - log.exception("Failed to run hook %s", impl.plugin_name) - raise - - # imap unordered gives us an iterator over the items in the order they finish. - # We have to be careful to set chunksize to 1 to ensure hooks each get their own thread. - # Unfortunately, there's no way to interrupt these threads, so we just have to leave them be. - for impl, result in pool.imap_unordered(_apply, impls, chunksize=1): - if result is not None: - log.debug("Hook from plugin %s returned: %s", impl.plugin_name, result) - return result - - def format_docstring(contents): """Python doc strings come in a number of formats, but LSP wants markdown. diff --git a/pyls/hookspecs.py b/pyls/hookspecs.py index 9daa889d..b941c32d 100644 --- a/pyls/hookspecs.py +++ b/pyls/hookspecs.py @@ -23,7 +23,12 @@ def pyls_commands(config, workspace): @hookspec -def pyls_completions(config, workspace, document, position): +def pyls_jedi_completions(config, workspace, document, position): + pass + + +@hookspec +def pyls_rope_completions(config, workspace, document, position): pass diff --git a/pyls/plugins/jedi_completion.py b/pyls/plugins/jedi_completion.py index e70cb5d4..ae770fb7 100644 --- a/pyls/plugins/jedi_completion.py +++ b/pyls/plugins/jedi_completion.py @@ -7,7 +7,7 @@ @hookimpl -def pyls_completions(document, position): +def pyls_jedi_completions(document, position): log.debug('Launching Jedi') definitions = document.jedi_script(position).completions() definitions = [{ diff --git a/pyls/plugins/rope_completion.py b/pyls/plugins/rope_completion.py index e3b17dcf..c9159f43 100644 --- a/pyls/plugins/rope_completion.py +++ b/pyls/plugins/rope_completion.py @@ -9,7 +9,7 @@ @hookimpl -def pyls_completions(document, position): +def pyls_rope_completions(document, position): log.debug('Launching Rope') # Rope is a bit rubbish at completing module imports, so we'll return None diff --git a/pyls/python_ls.py b/pyls/python_ls.py index 8bceb6e5..937b643d 100644 --- a/pyls/python_ls.py +++ b/pyls/python_ls.py @@ -1,6 +1,5 @@ # Copyright 2017 Palantir Technologies, Inc. import logging -from multiprocessing import dummy as multiprocessing from . import lsp, _utils from .config import config from .language_server import LanguageServer @@ -8,7 +7,6 @@ log = logging.getLogger(__name__) -PLUGGY_RACE_POOL_SIZE = 5 LINT_DEBOUNCE_S = 0.5 # 500 ms @@ -18,14 +16,10 @@ class PythonLanguageServer(LanguageServer): workspace = None config = None - _pool = multiprocessing.Pool(PLUGGY_RACE_POOL_SIZE) - - def _hook_caller(self, hook_name): - return self.config.plugin_manager.subset_hook_caller(hook_name, self.config.disabled_plugins) - def _hook(self, hook_name, doc_uri=None, **kwargs): doc = self.workspace.get_document(doc_uri) if doc_uri else None - return self._hook_caller(hook_name)(config=self.config, workspace=self.workspace, document=doc, **kwargs) + hook = self.config.plugin_manager.subset_hook_caller(hook_name, self.config.disabled_plugins) + return hook(config=self.config, workspace=self.workspace, document=doc, **kwargs) def capabilities(self): return { @@ -65,14 +59,14 @@ def code_lens(self, doc_uri): return flatten(self._hook('pyls_code_lens', doc_uri)) def completions(self, doc_uri, position): - completions = _utils.race_hooks( - self._hook_caller('pyls_completions'), self._pool, - document=self.workspace.get_document(doc_uri) if doc_uri else None, - position=position - ) + if self.config.plugin_settings('completion').get('enabled', True) is False: + return {'isIncomplete': False, 'items': []} + + provider = self.config.plugin_settings('completion').get('provider', 'jedi') + completions = self._hook('pyls_{}_completions'.format(provider), doc_uri, position=position) return { 'isIncomplete': False, - 'items': completions or [] + 'items': flatten(completions) } def definitions(self, doc_uri, position): diff --git a/test/plugins/test_completion.py b/test/plugins/test_completion.py index 7d9b280c..bbc97d5a 100644 --- a/test/plugins/test_completion.py +++ b/test/plugins/test_completion.py @@ -4,8 +4,8 @@ from pyls import uris from pyls.workspace import Document, get_preferred_submodules -from pyls.plugins.jedi_completion import pyls_completions as pyls_jedi_completions -from pyls.plugins.rope_completion import pyls_completions as pyls_rope_completions +from pyls.plugins.jedi_completion import pyls_jedi_completions +from pyls.plugins.rope_completion import pyls_rope_completions LOCATION = os.path.realpath( os.path.join(os.getcwd(), os.path.dirname(__file__)) diff --git a/vscode-client/package.json b/vscode-client/package.json index a36da57e..587fe760 100644 --- a/vscode-client/package.json +++ b/vscode-client/package.json @@ -30,10 +30,16 @@ }, "uniqueItems": true }, - "pyls.plugins.jedi_completion.enabled": { + "pyls.plugins.completion.enabled": { "type": "boolean", "default": true, - "description": "Enable or disable the plugin." + "description": "Enable or disable completions." + }, + "pyls.plugins.completion.provider": { + "type": "string", + "default": "jedi", + "enum": ["jedi", "rope"], + "description": "Select a completion provider." }, "pyls.plugins.jedi_definition.enabled": { "type": "boolean", @@ -136,11 +142,6 @@ "default": true, "description": "Enable or disable the plugin." }, - "pyls.plugins.rope_completion.enabled": { - "type": "boolean", - "default": true, - "description": "Enable or disable the plugin." - }, "pyls.plugins.yapf.enabled": { "type": "boolean", "default": true, From f86c66e8f8b992c561f049930153ec0e1b0bfa7a Mon Sep 17 00:00:00 2001 From: Lukas Geiger Date: Tue, 2 Jan 2018 20:11:22 +0100 Subject: [PATCH 2/2] Only use single completion hook --- pyls/hookspecs.py | 7 +------ pyls/plugins/completion/__init__.py | 18 ++++++++++++++++++ .../{ => completion}/jedi_completion.py | 3 +-- .../{ => completion}/rope_completion.py | 3 +-- pyls/python_ls.py | 6 +----- setup.py | 3 +-- test/plugins/test_completion.py | 4 ++-- 7 files changed, 25 insertions(+), 19 deletions(-) create mode 100644 pyls/plugins/completion/__init__.py rename pyls/plugins/{ => completion}/jedi_completion.py (98%) rename pyls/plugins/{ => completion}/rope_completion.py (98%) diff --git a/pyls/hookspecs.py b/pyls/hookspecs.py index b941c32d..9daa889d 100644 --- a/pyls/hookspecs.py +++ b/pyls/hookspecs.py @@ -23,12 +23,7 @@ def pyls_commands(config, workspace): @hookspec -def pyls_jedi_completions(config, workspace, document, position): - pass - - -@hookspec -def pyls_rope_completions(config, workspace, document, position): +def pyls_completions(config, workspace, document, position): pass diff --git a/pyls/plugins/completion/__init__.py b/pyls/plugins/completion/__init__.py new file mode 100644 index 00000000..b1fce092 --- /dev/null +++ b/pyls/plugins/completion/__init__.py @@ -0,0 +1,18 @@ +from pyls import hookimpl +from .jedi_completion import pyls_jedi_completions +from .rope_completion import pyls_rope_completions + + +@hookimpl +def pyls_settings(): + return {'plugins': {'completion': {'provider': 'jedi'}}} + + +@hookimpl +def pyls_completions(config, document, position): + provider = config.plugin_settings('completion').get('provider', 'jedi') + if provider == 'jedi': + return pyls_jedi_completions(document, position) + elif provider == 'rope': + return pyls_rope_completions(document, position) + return [] diff --git a/pyls/plugins/jedi_completion.py b/pyls/plugins/completion/jedi_completion.py similarity index 98% rename from pyls/plugins/jedi_completion.py rename to pyls/plugins/completion/jedi_completion.py index ae770fb7..882b18fa 100644 --- a/pyls/plugins/jedi_completion.py +++ b/pyls/plugins/completion/jedi_completion.py @@ -1,12 +1,11 @@ # Copyright 2017 Palantir Technologies, Inc. import logging from pyls.lsp import CompletionItemKind -from pyls import hookimpl, _utils +from pyls import _utils log = logging.getLogger(__name__) -@hookimpl def pyls_jedi_completions(document, position): log.debug('Launching Jedi') definitions = document.jedi_script(position).completions() diff --git a/pyls/plugins/rope_completion.py b/pyls/plugins/completion/rope_completion.py similarity index 98% rename from pyls/plugins/rope_completion.py rename to pyls/plugins/completion/rope_completion.py index c9159f43..b9d234c4 100644 --- a/pyls/plugins/rope_completion.py +++ b/pyls/plugins/completion/rope_completion.py @@ -2,13 +2,12 @@ import logging from rope.contrib.codeassist import code_assist, sorted_proposals -from pyls import hookimpl, lsp +from pyls import lsp log = logging.getLogger(__name__) -@hookimpl def pyls_rope_completions(document, position): log.debug('Launching Rope') diff --git a/pyls/python_ls.py b/pyls/python_ls.py index 937b643d..45f26a23 100644 --- a/pyls/python_ls.py +++ b/pyls/python_ls.py @@ -59,11 +59,7 @@ def code_lens(self, doc_uri): return flatten(self._hook('pyls_code_lens', doc_uri)) def completions(self, doc_uri, position): - if self.config.plugin_settings('completion').get('enabled', True) is False: - return {'isIncomplete': False, 'items': []} - - provider = self.config.plugin_settings('completion').get('provider', 'jedi') - completions = self._hook('pyls_{}_completions'.format(provider), doc_uri, position=position) + completions = self._hook('pyls_completions', doc_uri, position=position) return { 'isIncomplete': False, 'items': flatten(completions) diff --git a/setup.py b/setup.py index 665e3bcc..55a821a6 100755 --- a/setup.py +++ b/setup.py @@ -61,8 +61,7 @@ 'pyls = pyls.__main__:main', ], 'pyls': [ - 'rope_completion = pyls.plugins.rope_completion', - 'jedi_completion = pyls.plugins.jedi_completion', + 'completion = pyls.plugins.completion', 'jedi_definition = pyls.plugins.definition', 'jedi_hover = pyls.plugins.hover', 'jedi_references = pyls.plugins.references', diff --git a/test/plugins/test_completion.py b/test/plugins/test_completion.py index bbc97d5a..020df482 100644 --- a/test/plugins/test_completion.py +++ b/test/plugins/test_completion.py @@ -4,8 +4,8 @@ from pyls import uris from pyls.workspace import Document, get_preferred_submodules -from pyls.plugins.jedi_completion import pyls_jedi_completions -from pyls.plugins.rope_completion import pyls_rope_completions +from pyls.plugins.completion.jedi_completion import pyls_jedi_completions +from pyls.plugins.completion.rope_completion import pyls_rope_completions LOCATION = os.path.realpath( os.path.join(os.getcwd(), os.path.dirname(__file__))