Refactor code lenses #1755
Refactor code lenses #1755
Conversation
This changes the following: * Code lens state is stored in its own class * Code lenses are moved to SessionView instead of DocumentSyncListener * The code lens debounce time is lowered to 300ms from 2300ms
| def start_code_lenses_async(self) -> None: | ||
| ... | ||
|
|
||
| def resolve_visible_code_lenses_async(self) -> None: | ||
| ... | ||
|
|
||
| def get_resolved_code_lenses_for_region(self, region: sublime.Region) -> Generator[CodeLens, None, None]: | ||
| ... | ||
|
|
There was a problem hiding this comment.
I don't think these methods are required to be declared here? You should be able to remove them and mypy won't complain.
There was a problem hiding this comment.
BTW, you can run tox locally to check mypy and flake8.
There was a problem hiding this comment.
If they're not given a type error is given when you eventually access it down the chain since e.g. AbstractViewListener makes use of it and failing to meet that interface leads to a type error.
There was a problem hiding this comment.
I see. get_resolved_code_lenses_for_region is used in the text command. start_code_lenses_async and resolve_visible_code_lenses_async can go away.
| def _do_code_lenses_async(self) -> None: | ||
| session = self.session_async("codeLensProvider") | ||
| if session and session.uses_plugin(): | ||
| params = {"textDocument": text_document_identifier(self.view)} | ||
| for sv in self.session_views_async(): | ||
| if sv.session == session: | ||
| for request_id, request in sv.active_requests.items(): | ||
| if request.method == "codeAction/resolve": | ||
| session.send_notification(Notification("$/cancelRequest", {"id": request_id})) | ||
| name = session.config.name | ||
| session.send_request_async( | ||
| Request("textDocument/codeLens", params, self.view), | ||
| lambda r: self._on_code_lenses_async(name, r)) | ||
|
|
||
| def _on_code_lenses_async(self, name: str, response: Optional[List[CodeLens]]) -> None: | ||
| for i in range(0, len(self._code_lenses)): | ||
| self.view.erase_regions(self._code_lens_key(i)) | ||
| self._code_lenses.clear() | ||
| if not isinstance(response, list): | ||
| return | ||
| for index, c in enumerate(response): | ||
| region = range_to_region(Range.from_lsp(c["range"]), self.view) | ||
| self._code_lenses.append((c, region)) | ||
| if "command" in c: | ||
| # We consider a code lens that has a command to be already resolved. | ||
| self._on_resolved_code_lens_async(name, index, region, c) | ||
| else: | ||
| self._render_code_lens(name, index, region, None) | ||
| self._code_lenses = list((c, range_to_region(Range.from_lsp(c["range"]), self.view)) for c in response) | ||
| self._resolve_visible_code_lenses_async() | ||
| sv.start_code_lenses_async() |
There was a problem hiding this comment.
Because the data is now stored in SessionView, can't we now request code lenses for all sessions attached to the view?
| if session and session.uses_plugin(): | ||
| for sv in self.session_views_async(): | ||
| if sv.session == session: | ||
| sv.resolve_visible_code_lenses_async() |
There was a problem hiding this comment.
resolve for all sessions attached to the view because the data is stored per session now?
| def _region_key(self, index: int) -> str: | ||
| return '{0}.{1}'.format(self.CODE_LENS_KEY, index) |
There was a problem hiding this comment.
We're still keying on only the index, not the session. Oh well, maybe for a next step.
There was a problem hiding this comment.
Alright, let's roll with this. Couple things to do still:
- it probably doesn't work with cloned views (it didn't work with cloned views before either I think), code lens data needs to move to SessionBuffer for that. SessionView is for presentation of the data
- there's some flickering of the annotations when they're updated. Probably because they're updated from the worker thread.
- we can actually request code lenses for all sessions, not just the most suited one
| def get_resolved_code_lenses_for_region(self, region: sublime.Region) -> Generator[CodeLens, None, None]: | ||
| yield from self._code_lenses.get_resolved_code_lenses_for_region(region) |
There was a problem hiding this comment.
This code is equivalent to the old one so not a new issue but I don't really see the point of returning a generator when the caller is always gonna iterate through all the results anyway. I haven't measured it but it must be quite a bit slower than just returning a plain list or even a tuple.
This changes the following: * Code lens state is stored in its own class * Code lenses are moved to SessionView instead of DocumentSyncListener * The code lens debounce time is lowered to 300ms from 2300ms
This changes the following:
I'm aware this is a big change. The development was discussed in the Discord server in preparation of allowing to render code lenses as phantoms. As a result the PR for that one is blocked until this one is finalised since it depends on the refactoring done here.
I tried my best to separate the code that dealt with the phantom representation but some of the data changes might still linger and seem bizarre.