Support confirmation of move instance method refactoring errors - #3862
Conversation
datho7561
left a comment
There was a problem hiding this comment.
In order to support clients other than vscode-java, one thing we do is to gate functionality that needs specific client code or protocol extensions behind a setting that the client needs to specifically send to the server. This is called "extended client capabilities". We have them stored here in the server:
I think it would be helpful to gate confirmation feature behind an extended client capability, since the client needs to implement specific code in order to get this to work properly.
For instance, if you try this PR out without redhat-developer/vscode-java#4482, you'll notice that nothing happens when the refactoring is applied. I think it makes sense to keep the old behaviour (apply the refactoring without warning/confirmation) when running with a client that doesn't support the new warning feature.
|
Oh wait hmm the move is already a protocol extension, right? Hmm. |
|
I think it'd be still nice if it's resilient to the client not knowing about the new confirmation step, since there's clients other than vscode-java that support some of the client extensions that we do that'll need to adapt to this change, and I think it'd be a good idea to keep the current behaviour for those in the mean time. |
Signed-off-by: Morgan Chang <shin19991207@gmail.com>
Signed-off-by: Morgan Chang <shin19991207@gmail.com>
3ec68de to
fb34eb5
Compare
|
@datho7561 Oh, I see! Thanks for the explanation. I've updated the change to gate the move refactoring confirmation on client capability, and tested it without redhat-developer/vscode-java#4482 to confirm that clients without supporting this new behaviour still work. Could you please take another look? |
datho7561
left a comment
There was a problem hiding this comment.
Works pretty well and seems very helpful. Thanks, Morgan!
|
I'm waiting on Jenkins for this... it seems pretty slow |
|
It's still not done??? |
|
I hope the Jenkins run taking 1h10min was an anomaly. I'll merge now. |
Related to redhat-developer/vscode-java#4460
This PR adds support for confirming non-fatal problems reported while creating a "Move Instance Method" refactoring.
JDT LS previously had no way to represent this intermediate state. A refactoring response could contain a workspace edit or an error, but it could not indicate that non-fatal problems had been found and allow the client to ask the user whether to proceed, as is supported in Eclipse IDE.
When "Move Instance Method" reports non-fatal errors, JDT LS now returns an indication that the operation can continue and an opaque confirmation token, with the reported error messages and no workspace edit. The client can display the problems and repeat the move request with the token after the user confirms the operation.
RefactoringConfirmationgenerates the token from the checked operation and its relevant inputs. On the confirmed request, JDT LS recalculates the token and returns the workspace edit only if the refactoring context still matches what the user reviewed. If the source or reported conditions have changed, JDT LS rejects the confirmation and requires the client to run the refactoring again. (The confirmation token utility was implemented with assistance from Codex)Fatal problems remain blocking and cannot be confirmed.
Tested together with redhat-developer/vscode-java#4482