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.

Sign up for GitHub

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

Jump to bottom

Show quick fixes for generating accessors in field declarations #2092

Merged
merged 3 commits into from
May 31, 2022
Merged

Show quick fixes for generating accessors in field declarations #2092

merged 3 commits into from
May 31, 2022

Conversation

CsCherrYY
Copy link
Contributor

@CsCherrYY CsCherrYY commented May 17, 2022 •
edited
Loading

quickfix.mp4

The quick fixes can be found at

  • variable itself
  • at line level
  • at modifier

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

@CsCherrYY CsCherrYY force-pushed the cs-getset-quickfix branch 3 times, most recently from 2a1ddb8 to cc9204c Compare May 24, 2022 02:22
@CsCherrYY
Copy link
Contributor Author

test this please

@testforstephen
Copy link
Contributor

Duplicated getter and setter quickfix, and you need to add some unit tests for new getter and setter code actions.
image

@CsCherrYY
Copy link
Contributor Author

CsCherrYY commented May 25, 2022 •
edited
Loading

Duplicated getter and setter quickfix, and you need to add some unit tests for new getter and setter code actions. image

this quick fix comes from GetterSetterCorrectionSubProcessor, and it will be triggered by an unused field. Since we will show the quickfix for those fields without accessors, can we just remove this GetterSetterCorrectionSubProcessor?

@testforstephen
Copy link
Contributor

I think it's OK to remove the old GetterSetterCorrectionSubProcessor since the new code has covered it well.

CsCherrYY
CsCherrYY commented May 27, 2022
View reviewed changes
...clipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/correction/ShowAllQuickFixTest.java
@@ -160,10 +160,10 @@ public void testShowAt() throws Exception {
List<Diagnostic> diagnostics = getDiagnostics(cu, astRoot, 4);
context.setDiagnostics(diagnostics);
codeActions = new CodeActionHandler(this.preferenceManager).getCodeActionCommands(codeActionParams, new NullProgressMonitor());
assertEquals(3, codeActions.size());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here shows the actions under CodeActionKind.QuickFix, we just removed the quick assist of generating getter and setter from fieldDeclaration to CodeActionKind.Source, so here the test case should be updated.

@CsCherrYY
Copy link
Contributor Author

test this please

@CsCherrYY CsCherrYY force-pushed the cs-getset-quickfix branch from 67b4abf to 2b16154 Compare May 30, 2022 02:51
testforstephen
testforstephen approved these changes May 31, 2022
View reviewed changes
Copy link
Contributor

@testforstephen testforstephen left a comment

Choose a reason for hiding this comment

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

The latest commit works for me.

@testforstephen testforstephen merged commit 15af562 into eclipse-jdtls:master May 31, 2022
@testforstephen testforstephen added this to the Early June 2022 milestone May 31, 2022
@testforstephen testforstephen mentioned this pull request May 31, 2022
Merged
@CsCherrYY CsCherrYY deleted the cs-getset-quickfix branch May 31, 2022 08:58
@CsCherrYY CsCherrYY mentioned this pull request Oct 14, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@testforstephen testforstephen testforstephen approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Early June 2022
Development

Successfully merging this pull request may close these issues.
2 participants
@CsCherrYY @testforstephen