eclipse / lsp4e

Language Server Protocol support in Eclipse IDE
Eclipse Public License 2.0
60 stars 53 forks source link

Refactor Preview for WorkspaceEdit with `needsConfirmation` flag #953

Closed BoykoAlex closed 4 months ago

BoykoAlex commented 4 months ago

WorkspaceEdit has a map of change annotations where any change annotation is associated with edits from the workspace edit. If any of change annotations has needsConfirmation flag on then VSCode would bring up the refactor preview with annotated changes unchecked.

This PR proposes similar (but not the same, there are limitations) behaviour into Eclipse. If there is at lease one change annotation that requires confirmation then refactor wizard is opened with the Preview page opened. All changes would be checked marked as I couldn't find how to control this piece. Another limitation is Eclipse text edits are in reverse order (they are applied in reverse order for a reason) but the preview doesn't allow to set the tree sorter on the preview tree.

@rubenporras @mickaelistria @martinlippert feedback, ideas are welcomed :-)

mickaelistria commented 4 months ago

By the way, I'd like to add that if such a wizard taking a Change as input is useful to you in other places, you could consider creating such a wizard as an API directly in Platform, that would be welcome. We could keep this code in LSP4E until Platform provides a more generic API for it and then we adopt it.

BoykoAlex commented 4 months ago

@rubenporras @mickaelistria Thank you both very much for the prompt review :-) I have added a commit to address the review comments with a few more changes:

  1. Even if the workspace change is for one file that is opened in the editor in the case of needsConfirmation flag set to true we would still bring up the refactor preview. (I think this is rather obvious change)
  2. Added runOnUIThread(Runnable) to the UI class add used it in LSPEclipseUtils
  3. ServerMessageHandler use current active shell as the parent for MessageDialog instead of creating a new one - don't think you'd ever need to create a parent shell in Eclipse for a dialog. Creation of a new shell leaves its bounds undetermined the consequence of which is dialog box moves to bottom-right every time a new one is opened.
rubenporras commented 4 months ago

thank you