Repository: palantir/python-language-server
Pull request #219 (2018-01-09 20:15:20)

Respect updates to sys path without closing/opening a document
@jamesthomp 


Patch:
diff --git a/pyls/_utils.py b/pyls/_utils.py
index 3251fdb..8141431 100644
--- a/pyls/_utils.py
+++ b/pyls/_utils.py
@@ -126,3 +126,10 @@ def format_docstring(contents):
     contents = contents.replace('  ', u'\u00A0' * 2)
     contents = contents.replace('*', '\\*')
     return contents
+
+
+def clip_column(column, lines, line_number):
+    # Normalise the position as per the LSP that accepts character positions > line length
+    # https://github.com/Microsoft/language-server-protocol/blob/master/protocol.md#position
+    max_column = len(lines[line_number]) - 1 if len(lines) > line_number else 0
+    return min(column, max_column)
diff --git a/pyls/workspace.py b/pyls/workspace.py
index a4ee406..fd9374d 100644
--- a/pyls/workspace.py
+++ b/pyls/workspace.py
@@ -112,7 +112,8 @@ class Workspace(object):
     def put_document(self, doc_uri, content, version=None):
         path = uris.to_fs_path(doc_uri)
         self._docs[doc_uri] = Document(
-            doc_uri, content, sys_path=self.syspath_for_path(path), version=version, rope=self._rope
+            doc_uri, content,
+            extra_sys_path=self.source_roots(path), version=version, rope=self._rope
         )
 
     def rm_document(self, doc_uri):
@@ -136,27 +137,15 @@ class Workspace(object):
         params = {'type': msg_type, 'message': message}
         self._lang_server.notify(self.M_SHOW_MESSAGE, params)
 
-    def syspath_for_path(self, path):
-        """Construct a sensible sys path to use for the given file path.
-
-        Since the workspace root may not be the root of the Python project we instead
-        append the closest parent directory containing a setup.py file.
-        """
-        files = _utils.find_parents(self._root_path, path, ['setup.py']) or []
-        path = [os.path.dirname(setup_py) for setup_py in files]
-
-        # Check to see if we're in a virtualenv
-        if 'VIRTUAL_ENV' in os.environ:
-            log.info("Using virtualenv %s", os.environ['VIRTUAL_ENV'])
-            path.extend(jedi.evaluate.sys_path.get_venv_path(os.environ['VIRTUAL_ENV']))
-        else:
-            path.extend(sys.path)
-        return path
+    def source_roots(self, document_path):
+        """Return the source roots for the given document."""
+        files = _utils.find_parents(self._root_path, document_path, ['setup.py']) or []
+        return [os.path.dirname(setup_py) for setup_py in files]
 
 
 class Document(object):
 
-    def __init__(self, uri, source=None, version=None, local=True, sys_path=None, rope=None):
+    def __init__(self, uri, source=None, version=None, local=True, extra_sys_path=None, rope=None):
         self.uri = uri
         self.version = version
         self.path = uris.to_fs_path(uri)
@@ -164,7 +153,7 @@ class Document(object):
 
         self._local = local
         self._source = source
-        self._sys_path = sys_path or sys.path
+        self._extra_sys_path = extra_sys_path or []
         self._rope_project = rope
 
     def __str__(self):
@@ -260,13 +249,22 @@ class Document(object):
         kwargs = {
             'source': self.source,
             'path': self.path,
-            'sys_path': self._sys_path
+            'sys_path': self.sys_path()
         }
         if position:
             kwargs['line'] = position['line'] + 1
-
-            # Normalise the position as per the LSP that accepts character positions > line length
-            # https://github.com/Microsoft/language-server-protocol/blob/master/protocol.md#position
-            line_len = len(self.lines[position['line']])
-            kwargs['column'] = min(position['character'], line_len - 1)
+            kwargs['column'] = _utils.clip_column(position['character'], self.lines, position['line'])
         return jedi.Script(**kwargs)
+
+    def sys_path(self):
+        # Copy our extra sys path
+        path = list(self._extra_sys_path)
+
+        # Check to see if we're in a virtualenv
+        if 'VIRTUAL_ENV' in os.environ:
+            log.info("Using virtualenv %s", os.environ['VIRTUAL_ENV'])
+            path.extend(jedi.evaluate.sys_path.get_venv_path(os.environ['VIRTUAL_ENV']))
+        else:
+            path.extend(sys.path)
+
+        return path


Test patch:
diff --git a/test/test_document.py b/test/test_document.py
index f53d4b2..5c6439c 100644
--- a/test/test_document.py
+++ b/test/test_document.py
@@ -1,4 +1,5 @@
 # Copyright 2017 Palantir Technologies, Inc.
+import sys
 import pytest
 from pyls import uris
 from pyls.workspace import Document
@@ -48,6 +49,14 @@ def test_word_at_position(doc):
     assert doc.word_at_position({'line': 4, 'character': 0}) == ''
 
 
+def test_document_sys_path(doc):
+    """Test the document's sys path is updated."""
+    assert 'foo' not in doc.sys_path()
+    sys.path.append('foo')
+    # Check that the new sys path is included in the doc's sys path
+    assert 'foo' in doc.sys_path()
+
+
 def test_document_empty_edit():
     doc = Document('file:///uri', u'')
     doc.apply_change({
diff --git a/test/test_utils.py b/test/test_utils.py
index f31e96a..6675374 100644
--- a/test/test_utils.py
+++ b/test/test_utils.py
@@ -49,3 +49,9 @@ def test_merge_dicts():
         {'a': True, 'b': {'x': 123, 'y': {'hello': 'world'}}},
         {'a': False, 'b': {'y': [], 'z': 987}}
     ) == {'a': False, 'b': {'x': 123, 'y': [], 'z': 987}}
+
+
+def test_clip_column():
+    assert _utils.clip_column(5, ['123'], 0) == 2
+    assert _utils.clip_column(2, ['\n', '123'], 1) == 2
+    assert _utils.clip_column(0, [], 0) == 0
diff --git a/test/test_workspace.py b/test/test_workspace.py
index 1d85ec1..8adec0a 100644
--- a/test/test_workspace.py
+++ b/test/test_workspace.py
@@ -46,4 +46,4 @@ def test_non_root_project(pyls):
     test_uri = uris.from_fs_path(os.path.join(project_root, 'hello/test.py'))
     pyls.workspace.put_document(test_uri, 'assert True')
     test_doc = pyls.workspace.get_document(test_uri)
-    assert project_root in pyls.workspace.syspath_for_path(test_doc.path)
+    assert project_root in test_doc.sys_path()