Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mark deprecated rangeLength optional in TextDocumentContentChangeEvent #123

Merged
merged 3 commits into from
Jun 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,13 @@ and this project adheres to [Semantic Versioning][semver].
### Changed

- Exit server normally when `ctrl+c` is pressed in command shell.
- Mark deprecated `rangeLength` optional in `TextDocumentContentChangeEvent` ([#123])
- Optimize json-rpc message serialization ([#120])
- Fix `__init__()` constructors in several interface types ([#125])
- Fix valueSet type in `SymbolKindAbstract` ([#125])

[#125]: https://github.com/openlawlibrary/pygls/pull/125
[#123]: https://github.com/openlawlibrary/pygls/pull/123
[#120]: https://github.com/openlawlibrary/pygls/pull/120
[#117]: https://github.com/openlawlibrary/pygls/pull/117

Expand Down
5 changes: 4 additions & 1 deletion pygls/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -1198,7 +1198,10 @@ def __init__(self,


class TextDocumentContentChangeEvent:
def __init__(self, range: 'Range', range_length: NumType, text: str):
def __init__(self,
range: 'Range',
range_length: Optional[NumType] = None,
text: str = ''):
self.range = range
self.rangeLength = range_length
self.text = text
Expand Down
32 changes: 9 additions & 23 deletions pygls/workspace.py
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ def _apply_none_change(self, change: TextDocumentContentChangeEvent) -> None:

Currently does nothing, provided for consistency.
"""
pass

def apply_change(self, change: TextDocumentContentChangeEvent) -> None:
deathaxe marked this conversation as resolved.
Show resolved Hide resolved
"""Apply a text change to a document, considering TextDocumentSyncKind
Expand All @@ -232,30 +233,11 @@ def apply_change(self, change: TextDocumentContentChangeEvent) -> None:
both Range and RangeLength from their request. Consequently, the
attributes "range" and "rangeLength" will be missing from FULL
content update client requests in the pygls Python library.

Improvements:
Consider revising our treatment of TextDocumentContentChangeEvent,
and all other LSP primitive types, to set "Optional" interface
attributes from the client to "None". The "hasattr" check is
admittedly quite ugly; while it is appropriate given our current
state, there are plenty of improvements to be made. A good place to
start: require more rigorous de-serialization efforts when reading
client requests in protocol.py.
"""
if (
hasattr(change, 'range') and
hasattr(change, 'rangeLength') and
self._is_sync_kind_incremental
):
self._apply_incremental_change(change)
elif self._is_sync_kind_none:
self._apply_none_change(change)
elif not (
hasattr(change, 'range') or
hasattr(change, 'rangeLength')
):
self._apply_full_change(change)
else:
if hasattr(change, 'range'):
if self._is_sync_kind_incremental:
self._apply_incremental_change(change)
return
# Log an error, but still perform full update to preserve existing
# assumptions in test_document/test_document_full_edit. Test breaks
# otherwise, and fixing the tests would require a broader fix to
Expand All @@ -264,6 +246,10 @@ def apply_change(self, change: TextDocumentContentChangeEvent) -> None:
"Unsupported client-provided TextDocumentContentChangeEvent. "
"Please update / submit a Pull Request to your LSP client."
)

if self._is_sync_kind_none:
self._apply_none_change(change)
else:
self._apply_full_change(change)

@property
Expand Down
18 changes: 18 additions & 0 deletions tests/test_document.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,14 @@ def test_document_full_edit():
"print a, b"
]

doc = Document('file:///uri', u''.join(old), sync_kind=TextDocumentSyncKind.FULL)
change = TextDocumentContentChangeEvent(range=None, text=u'print a, b')
doc.apply_change(change)

assert doc.lines == [
"print a, b"
]


def test_document_line_edit():
doc = Document('file:///uri', u'itshelloworld')
Expand Down Expand Up @@ -96,6 +104,16 @@ def test_document_multiline_edit():
" print a, b\n"
]

doc = Document('file:///uri', u''.join(old), sync_kind=TextDocumentSyncKind.INCREMENTAL)
change = TextDocumentContentChangeEvent(
range=Range(Position(1, 4), Position(2, 11)), text=u'print a, b')
doc.apply_change(change)

assert doc.lines == [
"def hello(a, b):\n",
" print a, b\n"
]


def test_document_no_edit():
old = [
Expand Down