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

Jump to bottom

Java editor LS Quick Assist #111

Merged
merged 3 commits into from May 3, 2022
Merged

Java editor LS Quick Assist #111

merged 3 commits into from May 3, 2022

Conversation

BoykoAlex
Copy link
Contributor

@BoykoAlex BoykoAlex commented Apr 29, 2022 •
edited

  1. Support codeAction/resolve for edit property
  2. LS based Quick Assist processor for Java editor
  3. Single undo operation for WorkspaceEdit (UndoManager was missing from the apply operaion)
  4. Bugs in LSPCodeActionQuickAssistProcessor
  • Can't cache range. Each subsequent invocation of QA in the same doc different location brings up the same cached proposals
  • Don't think caching proposals is a good idea. Computation method can be reentrant but not recursive. Problem is old proposals are never cleared. I might be misunderstanding this bit though...

@BoykoAlex BoykoAlex force-pushed the java-quick-assist branch 2 times, most recently from 2249ec3 to ea27e8e Compare April 29, 2022 20:12
mickaelistria
mickaelistria reviewed May 2, 2022
View reviewed changes
Copy link
Contributor

@mickaelistria mickaelistria left a comment

Choose a reason for hiding this comment

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

This looks good. Some minor comments inline.

org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/rename/LSPRenameProcessor.java Outdated Show resolved Hide resolved
...se.lsp4e/src/org/eclipse/lsp4e/operations/codeactions/LSPCodeActionQuickAssistProcessor.java Outdated Show resolved Hide resolved
...se.lsp4e/src/org/eclipse/lsp4e/operations/codeactions/LSPCodeActionQuickAssistProcessor.java Show resolved Hide resolved
@BoykoAlex
Copy link
Contributor Author

@mickaelistria The first 2 comments - adjusted the PR. The third about recursion needs a bit of discussion because i might be missing something...

@BoykoAlex
Copy link
Contributor Author

@mickaelistria should be good to go now hopefully

@mickaelistria mickaelistria merged commit f88a552 into eclipse:master May 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mickaelistria mickaelistria mickaelistria left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@BoykoAlex @mickaelistria