-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Completions should prepend, not replace as it is for Scala 2 #18803
Completions should prepend, not replace as it is for Scala 2 #18803
Conversation
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 assume by exactly match you mean:
print@@ln
Could you add a test for the case were you strip the suffix (for the sake of documentation at least)?
val textEdit = range | ||
.map(lspRange => new TextEdit(lspRange, newText)) | ||
.getOrElse(new TextEdit(editRange, newText)) |
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.
Nit:
val textEdit = range | |
.map(lspRange => new TextEdit(lspRange, newText)) | |
.getOrElse(new TextEdit(editRange, newText)) | |
val textEdit = new TextEdit(range.getOrElse(editRange), newText) |
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.
LGTM!
By default, in Scala 2 completion prepend and only replace if there is exact match. This PR unifies the experience between the Scala versions. It seems more intuitive to work that way. The change will be as follows (the order is: new logic, old logic and scala 2 logic): https://github.com/lampepfl/dotty/assets/48657087/c037c322-5613-4b95-a6e5-090b4e8827b6 In the future, it should be improved by changing implementation to use `InsertReplaceEdit` instead of `TextEdit`. This will allow users to decide which behaviour they prefer, but this has to be first implemented in metals to properly handle the new logic, but for now the key is to unify behaviours. https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#insertReplaceEdit [Cherry-picked dc1e35a]
By default, in Scala 2 completion prepend and only replace if there is exact match. This PR unifies the experience between the Scala versions. It seems more intuitive to work that way.
The change will be as follows (the order is: new logic, old logic and scala 2 logic):
Screen.Recording.2023-10-31.at.15.30.31.mov
In the future, it should be improved by changing implementation to use
InsertReplaceEdit
instead ofTextEdit
. This will allow users to decide which behaviour they prefer, but this has to be first implemented in metals to properly handle the new logic, but for now the key is to unify behaviours. https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#insertReplaceEdit