-
Notifications
You must be signed in to change notification settings - Fork 185
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
Fix prepareRename support #2127
Conversation
# mypy: Signature of "is_enabled" incompatible with supertype "LspTextCommand" | ||
def is_enabled( # type: ignore | ||
self, | ||
new_name: str = "", | ||
placeholder: str = "", | ||
position: Optional[int] = None, | ||
event: Optional[dict] = None, | ||
point: Optional[int] = None | ||
) -> bool: | ||
if self.best_session("renameProvider.prepareProvider"): | ||
# The language server will tell us if the selection is on a valid token. | ||
return True | ||
return super().is_enabled(event, point) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was redundant and it's enough to use base implementation.
When this code was introduced it looked a bit differently and it made sense then.
listener = self.get_listener() | ||
if listener: | ||
listener.purge_changes_async() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added "purge" because I've noticed an issue with lsp-typescript where it requests client to apply edits and immediately triggers rename before client has triggered "didChange".
The issue with the old code was that it has shown the input overlay asking for new name before sending the
prepareRename
request and didn't use the range (or placeholder) from the response.Added big comment about how
lsp_symbol_rename
works since it behaves very differently depending on if server supports prepareRename request.Related to sublimelsp/LSP-typescript#178