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

Support refactoring: Convert anonymous class to nested #1178

Merged
merged 2 commits into from
Sep 17, 2019

Conversation

jdneo
Copy link
Contributor

@jdneo jdneo commented Sep 15, 2019

Apologize that I forgot to create a new issue for this feature in advance. I just filed one here: #1177

Simple case:
Kapture 2019-09-15 at 16 54 08

A more complicated one:
Kapture 2019-09-15 at 16 52 35

client side PR: redhat-developer/vscode-java#1060

Signed-off-by: Sheng Chen [email protected]

@@ -111,6 +122,26 @@ private static PositionInformation getFirstTrackedNodePosition(LinkedProposalPos
return positions[0];
}

private static PositionInformation getFirstTrackedNodePositionBySequenceRank(LinkedProposalPositionGroupCore positionGroup) {
Copy link
Contributor Author

@jdneo jdneo Sep 15, 2019

Choose a reason for hiding this comment

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

@fbricon @testforstephen

I think we should leverage the sequence rank information to get the first position in the position group.

If that is also the case in other proposals, we should avoid to get the first position by the array index since it's not reliable if any code refactor happens in the upstream side (jdt.core.manipulation)

Copy link
Contributor

Choose a reason for hiding this comment

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

@jdneo ok so can you fix it in this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fbricon How about addressing this in another PR?

Since I need some time to check if the sequence rank is also workable in the following proposals:

  • extract varaible
  • extract variable (all occurrence)
  • extract constant
  • extract method
  • extract field
  • invert variable

I can file another issue to track for this.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jdneo fine, you can open a separate issue

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's here: #1180

@fbricon fbricon merged commit 8c3859b into eclipse-jdtls:master Sep 17, 2019
@fbricon
Copy link
Contributor

fbricon commented Sep 17, 2019

Thanks @jdneo !

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.

2 participants