-
Notifications
You must be signed in to change notification settings - Fork 408
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
Add support for interface and abstract method implementation code lens #3333
Add support for interface and abstract method implementation code lens #3333
Conversation
So this just works because the underlying code supported it all along ?! Just widening a few parameters.. Lines 120 to 124 in 806786e
|
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.
Overall, this looks good and works well. The only thing I have to consider is whether we care to be backwards compatible in the language server also 😜
Also, I found the implementation behaviour even for types (so independent of this PR) diverges from how Eclipse does it. For example, Eclipse will show as implementations, methods/types that override/extend non-interface, non-abstract classes. However, because of that isInterface
/ isAbstract
logic prior to calling getCodeLens(..)
we completely avoid rendering such classes. Might need to look back and see if there's a good reason we did this. If not, we could fix it in a later PR.
org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/CodeLensHandlerTest.java
Outdated
Show resolved
Hide resolved
I've decided it's not worth doing this on the language server side. It's probably enough if we just mention the rename in the eventual changelog. |
c72b804
to
660f67f
Compare
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.
Looks good. Can you just remove those extraneous whitespaces (tabs) on the new lines in the 3 test classes. Ext, ITest & Test ? git show
displays them.
Signed-off-by: Hope Hadfield <[email protected]>
660f67f
to
6759b15
Compare
Fixes redhat-developer/vscode-java#3813
Requires redhat-developer/vscode-java#3876
Before:
After: