openlawlibrary / pygls

A pythonic generic language server
https://pygls.readthedocs.io/en/latest/
Apache License 2.0
563 stars 102 forks source link

refactor: rename server Position to PositionCodec, instantiate it in Workspace #383

Closed RossBencina closed 11 months ago

RossBencina commented 11 months ago

Description

Implement the refactoring discussed in #382

Resolves #382

Code review checklist (for code reviewer to complete)

RossBencina commented 11 months ago

Fixed lint checks

RossBencina commented 11 months ago

There is a question as to whether we also want to roll back the recent changes that qualified lsprotocol symbols with types. to avoid collisions between the two Position types e.g. in https://github.com/openlawlibrary/pygls/commit/e271ba575e31a2e4efbeecf467d46b228132f7e5

If that's desirable I propose to do it in a separate PR.

RossBencina commented 11 months ago

fixed position = PositionCodec to codec = PositionCodec in test_document.py

Apologies for the forced updates. I am done now.

RossBencina commented 11 months ago

Failing CI/test-pyodide but so is the main branch. The reported problem looks unrelated to pygls.

tombh commented 11 months ago

This looks great! Thank you very much.

I think in general throughout the codebase we're trying to move towards qualifying all lsprotocol types with types.* anyway.

I'm a big fan of force pushing in PRs, so thanks for that too.

Yeah don't worry about the Pyodide test.

You can select me or @alcarney, or both, to review this.

RossBencina commented 11 months ago

^^^ force push: make all deprecation warnings consistent:

diff --git a/pygls/workspace/__init__.py b/pygls/workspace/__init__.py
index a18f9d5..4880ef4 100644
--- a/pygls/workspace/__init__.py
+++ b/pygls/workspace/__init__.py
@@ -17,9 +17,9 @@ Document = TextDocument

 def utf16_unit_offset(chars: str):
     warnings.warn(
-        "'utf16_unit_offset' has been deprecated, use "
+        "'utf16_unit_offset' has been deprecated, instead use "
         "'PositionCodec.utf16_unit_offset' via 'workspace.position_codec' "
-        "or 'text_document.position_codec' instead",
+        "or 'text_document.position_codec'",
         DeprecationWarning,
         stacklevel=2,
     )
RossBencina commented 11 months ago

You can select me or @alcarney, or both, to review this.

Thanks for the feedback! I don't believe that I have permissions to assign reviewers. I'm happy for both or either of you to review.

RossBencina commented 11 months ago

^^^ rebased to latest main

tombh commented 11 months ago

Thank you! ❤️