-
Notifications
You must be signed in to change notification settings - Fork 411
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 methods,inherited fields,inherited methods to Generate toString() #3055
Conversation
...lipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/GenerateToStringHandler.java
Outdated
Show resolved
Hide resolved
@rgrunber I have updated the PR. |
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, I think the code is good to merge. Just one thing I noticed.
toString
support for records doesn't really suggest the record components. Upstream does it with https://github.com/eclipse-jdt/eclipse.jdt.ui/blob/30c501df8780ac9a6f043b35ff17ad9f4211d611/org.eclipse.jdt.ui/ui/org/eclipse/jdt/ui/actions/GenerateToStringAction.java#L208-L214. Our entry point would be atLine 211 in 6c16211
nonStaticFields = hasFields(type, false);
Can this be done in this PR or is there more involved ?
Signed-off-by: Snjezana Peco <[email protected]>
@rgrunber I have updated the PR. |
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.
Note that this still doesn't fix the issue for records. It'll generate a toString
as before, with no dialog to allow the user to select members to be included. If you look at
Line 211 in 6c16211
nonStaticFields = hasFields(type, false); |
nonStaticFields
is false
and isInTypeDeclaration
is also false
. The result is just a source action being generated with an empty ( LSPVariableBinding[0]
) selection.
However, I noticed that hasFields
is also called for constructor generation, so I'm fine with addressing this in some other PR since it seems to drag in that logic also.
Fixes redhat-developer/vscode-java#2639
Requires redhat-developer/vscode-java#3495