Return a WorkspaceEdit even if an exception is thrown #1952
Conversation
The error is logged but the method still returns a (null) value. This allows the server to return a CodeAction to the client, that this can inspect and discard (because no editAction is set). Otherwise the client will wait for the server and show it like that on the UI, which does not create a good user experience.
| @@ -131,7 +134,8 @@ public WorkspaceEdit apply() { | |||
| } | |||
| return edit; | |||
| } catch (Exception exc) { | |||
| throw new WrappedException(exc); | |||
| log.error("Creation of WorkspaceEdit failed.", exc); | |||
There was a problem hiding this comment.
@heinrichWeichert are you ok with this change?
There was a problem hiding this comment.
Could you we return an empty Edit instead that does nothing?
There was a problem hiding this comment.
@heinrichWeichert : Yes, we could return new WorkspaceEdit(), LSP defines the workspaceEdit as a nullable field. I checked LSP4E and it always checks for the field to be != null before accessing it, I also checked what it would do with an empty edit, and it will have the same result, as it will contain no changes. I think returning null is simpler, but if you think returning a new WorkspaceEdit() is better, I would do that, I do not have a strong opinion. May I ask why you think that returning an empty Edit is preferable?
There was a problem hiding this comment.
@szarnekow what do you think?
There was a problem hiding this comment.
@rubenporras OK, i didn't check that. If it is indeed nullsafe and expected it's fine with me.
One thing about the logger @cdietrich is there are more suitable logger that logs xtext framework errors? In log4j, i would have to configure this one differently, and it seems more like a debug/trace level for me? I would be OK if this is not that relevant....
There was a problem hiding this comment.
I've a preference towards returning null, too. An empty edit suggests that there is something the client could apply which might mess with its undo queue or whatsoever.
There was a problem hiding this comment.
i am not sure if and how tracing works in standalone mode.
|
thx @rubenporras |
The error is logged but the method still returns a (null) value. This allows the server to return a CodeAction to the client, that this can inspect and discard (because no editAction is set). Otherwise the client will wait for the server and show it like that on the UI, which does not create a good user experience.