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

Ensure didChange notification is never sent after didClose #2438

Merged
merged 15 commits into from
Apr 20, 2024
Merged
Show file tree
Hide file tree
Changes from 14 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
46 changes: 26 additions & 20 deletions plugin/session_buffer.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,9 @@ def _check_did_open(self, view: sublime.View) -> None:
self._do_document_link_async(view, version)
self.session.notify_plugin_on_session_buffer_change(self)

def _check_did_close(self) -> None:
def _check_did_close(self, view: sublime.View) -> None:
if self.opened and self.should_notify_did_close():
self.purge_changes_async(view, suppress_requests=True)
self.session.send_notification(did_close(uri=self._last_known_uri))
self.opened = False

Expand Down Expand Up @@ -202,9 +203,9 @@ def remove_session_view(self, sv: SessionViewProtocol) -> None:
self._clear_semantic_token_regions(sv.view)
self.session_views.remove(sv)
if len(self.session_views) == 0:
self._on_before_destroy()
self._on_before_destroy(sv.view)

def _on_before_destroy(self) -> None:
def _on_before_destroy(self, view: sublime.View) -> None:
self.remove_all_inlay_hints()
if self.has_capability("diagnosticProvider") and self.session.config.diagnostics_mode == "open_files":
self.session.m_textDocument_publishDiagnostics({'uri': self._last_known_uri, 'diagnostics': []})
Expand All @@ -216,7 +217,7 @@ def _on_before_destroy(self) -> None:
# in unregistering ourselves from the session.
if not self.session.exiting:
# Only send textDocument/didClose when we are the only view left (i.e. there are no other clones).
self._check_did_close()
self._check_did_close(view)
self.session.unregister_session_buffer_async(self)

def register_capability_async(
Expand Down Expand Up @@ -308,15 +309,15 @@ def on_revert_async(self, view: sublime.View) -> None:

on_reload_async = on_revert_async

def purge_changes_async(self, view: sublime.View) -> None:
def purge_changes_async(self, view: sublime.View, suppress_requests: bool = False) -> None:
predragnikolic marked this conversation as resolved.
Show resolved Hide resolved
if self._pending_changes is None:
return
sync_kind = self.text_sync_kind()
if sync_kind == TextDocumentSyncKind.None_:
return
if sync_kind == TextDocumentSyncKind.Full:
changes = None
version = view.change_count()
version = view.change_count() or self._pending_changes.version
else:
changes = self._pending_changes.changes
version = self._pending_changes.version
Expand All @@ -329,23 +330,28 @@ def purge_changes_async(self, view: sublime.View) -> None:
finally:
self._pending_changes = None
self.session.notify_plugin_on_session_buffer_change(self)
sublime.set_timeout_async(lambda: self._on_after_change_async(view, version))
sublime.set_timeout_async(lambda: self._on_after_change_async(view, version, suppress_requests))
predragnikolic marked this conversation as resolved.
Show resolved Hide resolved

def _on_after_change_async(self, view: sublime.View, version: int) -> None:
def _on_after_change_async(self, view: sublime.View, version: int, suppress_requests: bool = False) -> None:
if self._is_saving:
self._has_changed_during_save = True
return
self._do_color_boxes_async(view, version)
self.do_document_diagnostic_async(view, version)
if self.session.config.diagnostics_mode == "workspace" and \
not self.session.workspace_diagnostics_pending_response and \
self.session.has_capability('diagnosticProvider.workspaceDiagnostics'):
self._workspace_diagnostics_debouncer_async.debounce(
self.session.do_workspace_diagnostics_async, timeout_ms=WORKSPACE_DIAGNOSTICS_TIMEOUT)
self.do_semantic_tokens_async(view)
if userprefs().link_highlight_style in ("underline", "none"):
self._do_document_link_async(view, version)
self.do_inlay_hints_async(view)
if suppress_requests:
return
try:
self._do_color_boxes_async(view, version)
self.do_document_diagnostic_async(view, version)
if self.session.config.diagnostics_mode == "workspace" and \
not self.session.workspace_diagnostics_pending_response and \
self.session.has_capability('diagnosticProvider.workspaceDiagnostics'):
self._workspace_diagnostics_debouncer_async.debounce(
self.session.do_workspace_diagnostics_async, timeout_ms=WORKSPACE_DIAGNOSTICS_TIMEOUT)
self.do_semantic_tokens_async(view)
if userprefs().link_highlight_style in ("underline", "none"):
self._do_document_link_async(view, version)
self.do_inlay_hints_async(view)
except MissingUriError:
predragnikolic marked this conversation as resolved.
Show resolved Hide resolved
pass

def on_pre_save_async(self, view: sublime.View) -> None:
self._is_saving = True
Expand All @@ -357,7 +363,7 @@ def on_pre_save_async(self, view: sublime.View) -> None:
def on_post_save_async(self, view: sublime.View, new_uri: DocumentUri) -> None:
self._is_saving = False
if new_uri != self._last_known_uri:
self._check_did_close()
self._check_did_close(view)
self._last_known_uri = new_uri
self._check_did_open(view)
else:
Expand Down
73 changes: 48 additions & 25 deletions tests/test_single_document.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,31 +84,6 @@ def test_did_close(self) -> 'Generator':
self.view.close()
yield from self.await_message("textDocument/didClose")

def test_did_change(self) -> 'Generator':
assert self.view
self.maxDiff = None
self.insert_characters("A")
yield from self.await_message("textDocument/didChange")
# multiple changes are batched into one didChange notification
self.insert_characters("B\n")
self.insert_characters("🙂\n")
self.insert_characters("D")
promise = YieldPromise()
yield from self.await_message("textDocument/didChange", promise)
self.assertEqual(promise.result(), {
'contentChanges': [
{'rangeLength': 0, 'range': {'start': {'line': 0, 'character': 1}, 'end': {'line': 0, 'character': 1}}, 'text': 'B'}, # noqa
{'rangeLength': 0, 'range': {'start': {'line': 0, 'character': 2}, 'end': {'line': 0, 'character': 2}}, 'text': '\n'}, # noqa
{'rangeLength': 0, 'range': {'start': {'line': 1, 'character': 0}, 'end': {'line': 1, 'character': 0}}, 'text': '🙂'}, # noqa
# Note that this is character offset (2) is correct (UTF-16).
{'rangeLength': 0, 'range': {'start': {'line': 1, 'character': 2}, 'end': {'line': 1, 'character': 2}}, 'text': '\n'}, # noqa
{'rangeLength': 0, 'range': {'start': {'line': 2, 'character': 0}, 'end': {'line': 2, 'character': 0}}, 'text': 'D'}], # noqa
'textDocument': {
'version': self.view.change_count(),
'uri': filename_to_uri(TEST_FILE_PATH)
}
})

def test_sends_save_with_purge(self) -> 'Generator':
assert self.view
self.view.settings().set("lsp_format_on_save", False)
Expand Down Expand Up @@ -371,6 +346,54 @@ def test_progress(self) -> 'Generator':
self.assertEqual(result, {"general": "kenobi"})


class SingleDocumentTestCase2(TextDocumentTestCase):

def test_did_change(self) -> 'Generator':
assert self.view
self.maxDiff = None
self.insert_characters("A")
yield from self.await_message("textDocument/didChange")
# multiple changes are batched into one didChange notification
self.insert_characters("B\n")
self.insert_characters("🙂\n")
self.insert_characters("D")
promise = YieldPromise()
yield from self.await_message("textDocument/didChange", promise)
self.assertEqual(promise.result(), {
'contentChanges': [
{'rangeLength': 0, 'range': {'start': {'line': 0, 'character': 1}, 'end': {'line': 0, 'character': 1}}, 'text': 'B'}, # noqa
{'rangeLength': 0, 'range': {'start': {'line': 0, 'character': 2}, 'end': {'line': 0, 'character': 2}}, 'text': '\n'}, # noqa
{'rangeLength': 0, 'range': {'start': {'line': 1, 'character': 0}, 'end': {'line': 1, 'character': 0}}, 'text': '🙂'}, # noqa
# Note that this is character offset (2) is correct (UTF-16).
{'rangeLength': 0, 'range': {'start': {'line': 1, 'character': 2}, 'end': {'line': 1, 'character': 2}}, 'text': '\n'}, # noqa
{'rangeLength': 0, 'range': {'start': {'line': 2, 'character': 0}, 'end': {'line': 2, 'character': 0}}, 'text': 'D'}], # noqa
'textDocument': {
'version': self.view.change_count(),
'uri': filename_to_uri(TEST_FILE_PATH)
}
})


class SingleDocumentTestCase3(TextDocumentTestCase):

@classmethod
def get_test_name(cls) -> str:
return "testfile2"

def test_did_change_before_did_close(self) -> 'Generator':
assert self.view
self.view.window().run_command("chain", {
"commands": [
["insert", {"characters": "TEST"}],
["save", {"async": False}],
["close", {}]
]
})
yield from self.await_message('textDocument/didChange')
# yield from self.await_message('textDocument/didSave') # TODO why is this not sent?
predragnikolic marked this conversation as resolved.
Show resolved Hide resolved
yield from self.await_message('textDocument/didClose')


class WillSaveWaitUntilTestCase(TextDocumentTestCase):

@classmethod
Expand Down
Empty file added tests/testfile2.txt
Empty file.