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

Add code action to generate toString() method #736

Closed
fbricon opened this issue Jul 26, 2018 · 22 comments · Fixed by #988
Closed

Add code action to generate toString() method #736

fbricon opened this issue Jul 26, 2018 · 22 comments · Fixed by #988

Comments

@fbricon
Copy link
Contributor

fbricon commented Jul 26, 2018

No description provided.

@testforstephen
Copy link
Contributor

@fbricon Same as hashCode and equals method, the implementation class GenerateToStringOperation in org.eclipse.jdt.ui needs move to org.eclipse.jdt.core.manipulation.

@fbricon
Copy link
Contributor Author

fbricon commented Dec 13, 2018

cc @rgrunber and @jjohnstn

@jjohnstn
Copy link
Contributor

Working on it

@jjohnstn
Copy link
Contributor

Gerrit patch submitted: https://git.eclipse.org/r/134075

@jjohnstn
Copy link
Contributor

Patch has been accepted to jdt.ui/jdt.core.manipulation. This can be closed.

@fbricon
Copy link
Contributor Author

fbricon commented Jan 17, 2019

@jjohnstn we still need to make changes on the jdt.ls side, to call the new code

@jjohnstn
Copy link
Contributor

@fbricon yes, my bad.

@fbricon
Copy link
Contributor Author

fbricon commented Jan 21, 2019

@yaohaizh can you work on this one for the next release or should I assign it to someone else?

@yaohaizh
Copy link
Contributor

@fbricon Sure, I'll follow this issue on this release.

@testforstephen
Copy link
Contributor

The upstream GenerateToStringOperation.java implementation puts "generate the text edit" and "apply the text edit" steps together. But eclipse.jdt.ls only needs the first step and return a text edit to vscode client to apply.

In order to reuse this part of code, we may need more refactoring on this class. GenerateHashCodeEqualsOperation.java is a good reference.
-> Give the apply control to the external caller.
-> Expose a getter method to return the TextEdit change.

cc: @fbricon @jjohnstn

@testforstephen
Copy link
Contributor

@jjohnstn I'd like to submit a PR to the upstream code, but not familiar with the contribution process. Is there any doc to explain the PR and environment setup process for eclipse.jdt.ui code? thanks.

@jjohnstn
Copy link
Contributor

jjohnstn commented Mar 4, 2019 via email

@testforstephen
Copy link
Contributor

@jjohnstn Thanks for the information. Having a try. Will update the status if everything goes well.

@testforstephen
Copy link
Contributor

@jjohnstn i have one question about the testing. In addition to unit test, how can i manually verify my change locally?

@testforstephen
Copy link
Contributor

Tried "Debug as Eclipse Application", the new change is hit.

@testforstephen
Copy link
Contributor

Submitted a gerrit patch https://git.eclipse.org/r/#/c/138149/. But the build failed at an irrelevant test case. I tried rerun the failed case locally and it passed.

@jjohnstn Is there any way to rerun the ci job for gerrit patch?

@jjohnstn
Copy link
Contributor

jjohnstn commented Mar 6, 2019

You can't rerun unless you are a comitter (it is annoying). The ways to do this are either to wait until another patch is committed and then rebase or else make a small change (e.g. change a comment) and update the gerrit patch (keeping the same changeid).

@testforstephen
Copy link
Contributor

Got that. Roland has helped re-trigger the build. Thanks.

@rgrunber
Copy link
Contributor

rgrunber commented Mar 27, 2019

The changed was merged (targeting 4.12 M1) but should also be in the next I-build of JDT.

@testforstephen
Copy link
Contributor

@rgrunber Great, thanks. Will try to finish the change on jdt.ls side.

@fbricon
Copy link
Contributor Author

fbricon commented Apr 4, 2019

@testforstephen so I'll target this one for the next release. Let me know if we need to punt it to the next one

@fbricon fbricon added this to the Mid April 2019 milestone Apr 4, 2019
@testforstephen
Copy link
Contributor

i should have time to do it in next week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants