Skip to content
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

Source Action: Override/Implement Methods with prompt #910

Merged
merged 2 commits into from
Feb 25, 2019

Conversation

testforstephen
Copy link
Contributor

Signed-off-by: Jinbo Wang [email protected]

It implements the source action Override/Implement Methods with prompt ui.
override_implement

Closes #900

@testforstephen
Copy link
Contributor Author

Peer PR on vscode-java side: redhat-developer/vscode-java#749

Copy link
Contributor

@fbricon fbricon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's very promising! But I still need to review in more details. So far one problem I found was that the code action was returned for module-info.java and package-info.java files. It should only be returned for actual types.

return overridables;
}

public static TextEdit addOverridableMethods(IType type, OverridableMethod[] overridableMethods) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens if overridableMethods are actually already overridden (eg. call addOverridableMethods twice)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generate duplicated code. Here i just take it as an utility method, and need the client to avoid calling the same code multiple times.

@yaohaizh yaohaizh force-pushed the jinbo_overridemethods branch 2 times, most recently from fe81b6c to 51428ea Compare January 19, 2019 12:42
@yaohaizh
Copy link
Contributor

Try to resolve this PR since @testforstephen is still on leave. But there is ip validation issue, wait for @testforstephen to come back and resolve this issue.

@testforstephen
Copy link
Contributor Author

@fbricon @yaohaizh updated.

Signed-off-by: Jinbo Wang <[email protected]>
@testforstephen
Copy link
Contributor Author

@fbricon @yaohaizh Comments addressed.

@fbricon fbricon merged commit f3552fb into eclipse-jdtls:master Feb 25, 2019
@fbricon
Copy link
Contributor

fbricon commented Feb 25, 2019

Nice work @testforstephen !

@testforstephen testforstephen deleted the jinbo_overridemethods branch February 26, 2019 01:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants