Repository navigation
Refactor workspace edit handling - #2905
Conversation
| message = "Error while applying WorkspaceEdit" | ||
| if failure_reason := result.get('failureReason'): | ||
| message += f": {failure_reason}" |
There was a problem hiding this comment.
Potentially has a long error message here that would be hard to read in few seconds that it's shown.
I would opt in for adding it to the console also.
There was a problem hiding this comment.
The same message automatically gets logged to the console, but only if debug logging is enabled. I think that should be okay?
|
|
||
| def _continue(failure_reason: str | None) -> Promise[ApplyWorkspaceEditResult]: | ||
| if failure_reason: | ||
| debug(f'Error while applying WorkspaceEdit: {failure_reason}') |
There was a problem hiding this comment.
If my earlier suggestion is applied then we shouldn't need this.
I've said something to that effect before but debug seems like something only useful during feature development because nobody should want to run with it enabled normally. Which means that pretty much no one will see this during normal use.
And if it's only useful during implementation than probably also doesn't need to be included in the final code.
There was a problem hiding this comment.
Hm, but then we could also remove "log_debug" from the user settings file (i.e. make it only a "hidden" setting), if it's not supposed to be used by end users...
There was a problem hiding this comment.
It is useful sometimes to enable it temporarily. For example when wanting to see what command is used to start the server. But it's not useful to keep it enabled all the time which makes it not really suitable for cases like this one. You want to see this one when it happens without having to try to reproduce it again.
There was a problem hiding this comment.
Or maybe another idea would be to replace "log_debug" by a "log_severity": "off" | "error" | "warning" | "info" | "log" | "debug" setting, that provides more granularity about what is printed to the console. Then it could also be used for the window/showMessage and window/logMessage notifications, that we also print to the console (iirc window/logMessage only if "log_debug" is enabled, but I'm not sure right now).
And I think it would be nice to add a timestamp to the console messages too, similar to the timestamps in the LSP log panel.
There was a problem hiding this comment.
I think it's a good idea but not necessarily relevant to this case.
In this case we are showing the message to the user always (through status field) so I would say that the console message should also be logged always since its purpose is not to log something extra but just allow the user to read the details that he/she will likely not be able to read in 2 seconds.
There was a problem hiding this comment.
And since the message could potentially be long, maybe best to just show Error while applying WorkspaceEdit - see console for more details in the status field.
Co-authored-by: Rafał Chłodnicki <rchl2k@gmail.com>
Refactor
Session.apply_workspace_edit_asyncwhich may be used by code actions and the rename feature, so that the changes in the WorkspaceEdit are also applied in a sequential way (and that the WorkspaceEdit is aborted as soon as one change fails). This should make it possible for us to add support for resource operations.