Merge pull request #187 from krassowski/fix/didSaveHandler

Do not raise on missing contentChanges in didSave

Author: krassowski

CHANGELOG.md

@@ -1,5 +1,12 @@
 # CHANGELOG
 
+## `jupyter-lsp 0.7.0`
+
+- bugfixes
+  - didSave no longer causes unwanted messages in logs (
+    [#187](https://github.com/krassowski/jupyterlab-lsp/pull/187)
+    )
+
 ## `@krassowski/jupyterlab-lsp 0.7.1`
 
 - features

azure-pipelines.yml

@@ -13,7 +13,7 @@ variables:
   ATEST_RETRIES: 3
   YARN_CACHE_FOLDER: $(Pipeline.Workspace)/.yarn
 
-  PY_JLSP_VERSION: 0.7.0b0
+  PY_JLSP_VERSION: 0.7.0
   JS_JLLSP_VERSION: 0.7.1
 
   FIRST_PARTY_LABEXTENSIONS: >-

py_src/jupyter_lsp/_version.py

@@ -1,3 +1,3 @@
 """ single source of truth for jupyter_lsp version
 """
-__version__ = "0.7.0b0"
+__version__ = "0.7.0"

py_src/jupyter_lsp/tests/test_virtual_documents_shadow.py

@@ -60,6 +60,17 @@ def did_change(uri, changes: List):
     }
 
 
+def did_save_with_text(uri, text):
+    return {
+        "method": "textDocument/didSave",
+        "params": {"textDocument": {"uri": uri, "text": text}},
+    }
+
+
+def did_save_without_text(uri):
+    return {"method": "textDocument/didSave", "params": {"textDocument": {"uri": uri}}}
+
+
 @pytest.fixture
 def shadow_path(tmpdir):
     return str(tmpdir.mkdir(".virtual_documents"))
@@ -71,6 +82,7 @@ def shadow_path(tmpdir):
     [
         [did_open, "content\nof\nopened\nfile", "content\nof\nopened\nfile"],
         [did_change, [{"text": "content after change"}], "content after change"],
+        [did_save_with_text, "content at save", "content at save"],
     ],
 )
 async def test_shadow(shadow_path, message_func, content, expected_content):
@@ -89,6 +101,7 @@ async def test_shadow(shadow_path, message_func, content, expected_content):
 async def test_shadow_failures(shadow_path):
 
     shadow = setup_shadow_filesystem(Path(shadow_path).as_uri())
+    ok_file_uri = (Path(shadow_path) / "test.py").as_uri()
 
     def run_shadow(message):
         return shadow("client", message, ["python"], None)
@@ -110,3 +123,16 @@ def run_shadow(message):
     # should NOT intercept (nor shadow) files from location other than shadow_path
     result = await run_shadow(did_open("file:///other/path.py", "content"))
     assert result is None
+
+    # should fail silently on missing text in didSave
+    result = await run_shadow(did_save_without_text(ok_file_uri))
+    assert result is None
+
+    # should raise on missing changes in didChange
+    with pytest.raises(ShadowFilesystemError, match=".* is missing contentChanges"):
+        await run_shadow(
+            {
+                "method": "textDocument/didChange",
+                "params": {"textDocument": {"uri": ok_file_uri}},
+            }
+        )

py_src/jupyter_lsp/virtual_documents_shadow.py

@@ -130,13 +130,21 @@ async def shadow_virtual_documents(scope, message, languages, manager):
         text = extract_or_none(document, ["text"])
 
         if text is not None:
+            # didOpen and didSave may provide text within the document
             changes = [{"text": text}]
         else:
+            # didChange is the only one which can also provide it in params (as contentChanges)
+            if message["method"] != "textDocument/didChange":
+                return
+            if "contentChanges" not in message["params"]:
+                raise ShadowFilesystemError(
+                    "textDocument/didChange is missing contentChanges"
+                )
             changes = message["params"]["contentChanges"]
 
         if len(changes) > 1:
             manager.log.warn(  # pragma: no cover
-                "LSP warning: up to one change" " supported for textDocument/didChange"
+                "LSP warning: up to one change supported for textDocument/didChange"
             )
 
         for change in changes[:1]: