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 "Add all missing imports" #2292

Merged
merged 4 commits into from
Nov 4, 2022

Conversation

CsCherrYY
Copy link
Contributor

@CsCherrYY CsCherrYY commented Oct 24, 2022

Signed-off-by: Shi Chen chenshi@microsoft.com

The quickfix will be available when there is an unresolved type, and the source action will be available if the current file has unresolved type errors, the actions will only add missing imports and do not affect current import order.

addallmissingimports.mp4

see redhat-developer/vscode-java#2748
related to: redhat-developer/vscode-java#2753

@CsCherrYY

This comment was marked as resolved.

Signed-off-by: Shi Chen <chenshi@microsoft.com>
@CsCherrYY CsCherrYY marked this pull request as ready for review October 31, 2022 08:33
Copy link
Contributor

@rgrunber rgrunber left a comment

Choose a reason for hiding this comment

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

Overall, this looks good. Just some small things to address/clarify.

Signed-off-by: Shi Chen <chenshi@microsoft.com>
Signed-off-by: Shi Chen <chenshi@microsoft.com>
@rgrunber
Copy link
Contributor

rgrunber commented Nov 3, 2022

Looks like the failures are likely due to something that the change is regressing (relating to statics?). Once it's fixed I think this is definitely good to merge.

@CsCherrYY
Copy link
Contributor Author

CsCherrYY commented Nov 4, 2022

the root cause is that we always return proposal (formerly code action for some cases), which will be resolved directly if a client doesn't support resolving the code action. The related logic has an extra static part here. Let me fix it.

Signed-off-by: Shi Chen <chenshi@microsoft.com>
@CsCherrYY
Copy link
Contributor Author

test this please

@rgrunber
Copy link
Contributor

rgrunber commented Nov 4, 2022

re-test this please.

Copy link
Contributor

@rgrunber rgrunber left a comment

Choose a reason for hiding this comment

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

Change looks good to me.

@rgrunber rgrunber merged commit f07c1a5 into eclipse-jdtls:master Nov 4, 2022
@CsCherrYY CsCherrYY deleted the cs-add-missing-imports branch November 7, 2022 02:37
@CsCherrYY CsCherrYY restored the cs-add-missing-imports branch November 8, 2022 02:26
@CsCherrYY CsCherrYY deleted the cs-add-missing-imports branch November 8, 2022 02:26
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.

None yet

2 participants