-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Signature help fixes for exposed bugs and refactor after signature change #15278
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
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.
Great work! I agree with the need to change the signature, but it would be great to minimize the impact on Metals (and possible other tools)
language-server/test/dotty/tools/languageserver/SignatureHelpTest.scala
Outdated
Show resolved
Hide resolved
language-server/test/dotty/tools/languageserver/SignatureHelpTest.scala
Outdated
Show resolved
Hide resolved
language-server/test/dotty/tools/languageserver/SignatureHelpTest.scala
Outdated
Show resolved
Hide resolved
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! The changes for Metals look to be working here:
See: scala/scala3#15278 `Signatures.callInfo` was deprecated and we should use `Signatures.signatureHelp`
See: scala/scala3#15278 `Signatures.callInfo` was deprecated and we should use `Signatures.signatureHelp`
See: scala/scala3#15278 `Signatures.callInfo` was deprecated and we should use `Signatures.signatureHelp`
…ange (scala#15278) * add deleted methods as depreceted one * add type parameter inference for apply methods * change signatureHelp method signature
Fixes #15248
Fixes #15244
Fixes #15284
For all found bugs i created tests.
This pull request changes almost whole file, as in previous implementation I tried to leave function signature the way it was.
It required few hacks to achieve that but now I've hit the wall and to fix:
I had to change function signature as I discussed it with @tgodzik. As i changed signature, many places became unreadable and I had to refactor it a bit thus so many lines are changed. I could try split it into multiple pull requests but effect would be almost the same as it changed almost all file. I will pinpoint locations which caused each bug to where it was fixed.
I also wrote documentation for new functions to better explain the idea.