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

Show "Add javadoc for" in quick assists #2133

Merged
merged 4 commits into from
Jun 28, 2022

Conversation

CsCherrYY
Copy link
Contributor

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

javadoc.mp4

@CsCherrYY CsCherrYY changed the title Show "add javadoc for" in quick assists Show "Add javadoc for" in quick assists Jun 20, 2022
Signed-off-by: Shi Chen <chenshi@microsoft.com>
@CsCherrYY CsCherrYY marked this pull request as ready for review June 20, 2022 09:01
@rgrunber rgrunber self-requested a review June 22, 2022 03:47
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 seems to be working well for me. Just some questions about placement of the new functionality.

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

Overall, the change works well. Just one thing I noticed. Not sure if it has any implications :

In this example, I set the property org.eclipse.jdt.core.compiler.problem.missingJavadocComments=warning
add-javadoc-no-quickfix-available

No quick fixes available. Of course if we trigger code action, we'll be able to fix it, but that display can be a bit confusing. I don't think many users choose to activate quick-fixes that way, but I wonder if this might affect other clients negatively.

Is there any way to preserve the quick-fix for the diagnostic while not duplicating it when triggered as a code action ?

@CsCherrYY
Copy link
Contributor Author

@rgrunber A solution is that we can separate these two kinds: if there is a problem about Missing javadoc comment (Although it's
not a default value, user can define it via .settings), we provide a QuickFix, otherwise a QuickAssist.

And we can also keep a better order under this situation: A QuickFix should have a higher priority on the client side since it will have a related diagnostic, while a QuickAssist does not. In this way, we can have a way to put a QuickAssist, which will add javadoc under other quick assists, e.g., generating accessors, the related work will be available in #2109.

quickfixandassist.mp4

Signed-off-by: Shi Chen <chenshi@microsoft.com>
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.

LGTM. Feel free to squash + rebase merge so we can include in the release.

@rgrunber rgrunber added this to the End June 2022 milestone Jun 27, 2022
@CsCherrYY CsCherrYY merged commit 7eafab9 into eclipse-jdtls:master Jun 28, 2022
@CsCherrYY CsCherrYY deleted the cs-javadoc-quickassist branch June 28, 2022 01:43
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