Skip to content

Support "Add all missing imports"#2292

Merged
rgrunber merged 4 commits intoeclipse-jdtls:masterfrom
CsCherrYY:cs-add-missing-imports
Nov 4, 2022
Merged

Support "Add all missing imports"#2292
rgrunber merged 4 commits intoeclipse-jdtls:masterfrom
CsCherrYY:cs-add-missing-imports

Conversation

@CsCherrYY
Copy link
Copy Markdown
Contributor

@CsCherrYY CsCherrYY commented Oct 24, 2022

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

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.

@CsCherrYY CsCherrYY force-pushed the cs-add-missing-imports branch from 92a8d2a to 5c326da Compare October 31, 2022 07:34
@CsCherrYY CsCherrYY marked this pull request as ready for review October 31, 2022 08:33
Copy link
Copy Markdown
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.

@rgrunber
Copy link
Copy Markdown
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
Copy Markdown
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.

@CsCherrYY
Copy link
Copy Markdown
Contributor Author

test this please

@rgrunber
Copy link
Copy Markdown
Contributor

rgrunber commented Nov 4, 2022

re-test this please.

Copy link
Copy Markdown
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.

2 participants