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

Hide inline constant commands when no reference found #1575

Merged
merged 2 commits into from
Oct 19, 2020

Conversation

CsCherrYY
Copy link
Contributor

hide meaningless refactor commands when a constant has no reference. The following gifs show the effect of this PR.
Before:
before
After:
after

@fbricon fbricon added this to the End October 2020 milestone Oct 19, 2020
@fbricon fbricon merged commit 29a74ad into eclipse-jdtls:master Oct 19, 2020
@fbricon
Copy link
Contributor

fbricon commented Oct 19, 2020

Thanks @CsCherrYY!

for (Change refactoringChange : refactoringChanges) {
referenceCount += ((CompilationUnitChange)refactoringChange).getChangeGroups().length;
}
if (referenceCount <= 1 && refactoring.isDeclarationSelected()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@CsCherrYY Below is a corner case. The change group will be 2 (1 for remove imports, 1 for remove the declaration) even though no references are using the constant field. The "Inline" menu won't be hidden.

import java.util.HashMap;
import java.util.Map;

public class Hello {
    public static final Map map = new HashMap<>();
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@testforstephen Thanks a lot for kind reminding, I would like to implement this clearer and avoid corner case like this by changing some APIs in jdt later.

@CsCherrYY CsCherrYY deleted the inlineConstantFix branch November 30, 2020 07:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants