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

[LS] Cannot use CreateFile, RenameFile, and DeleteFile inside a WorkspaceEdit #4723

Closed
kittaakos opened this issue Mar 26, 2019 · 7 comments · Fixed by #6229
Closed

[LS] Cannot use CreateFile, RenameFile, and DeleteFile inside a WorkspaceEdit #4723

kittaakos opened this issue Mar 26, 2019 · 7 comments · Fixed by #6229
Labels
bug bugs found in the application lsp language server protocol monaco issues related to monaco

Comments

@kittaakos
Copy link
Contributor

Spec: https://microsoft.github.io/language-server-protocol/specification#version_3_13_0

Currently, we use [email protected] in Theia which has the following logic to handle workspace/applyEdit:

if (change.textDocument.version && change.textDocument.version >= 0) {

Link: https://github.com/Microsoft/vscode-languageserver-node/blob/5f9c993ff38f5c369949aeb359b3e9b178172dbc/client/src/client.ts#L3019

VS. the 5.1.0 version:

if (TextDocumentEdit.is(change) && change.textDocument.version && change.textDocument.version >= 0) {

Link: https://github.com/Microsoft/vscode-languageserver-node/blob/71fc1da3fed87329080f9c2a2c32e5e07aed3128/client/src/client.ts#L3167

The older version of the language client does not support these new documentChanges and will result in an error on the server side:

org.eclipse.lsp4j.jsonrpc.ResponseErrorException: Request workspace/applyEdit failed with message: Cannot read property 'version' of undefined
	at org.eclipse.lsp4j.jsonrpc.RemoteEndpoint.handleResponse(RemoteEndpoint.java:209)
	at org.eclipse.lsp4j.jsonrpc.RemoteEndpoint.consume(RemoteEndpoint.java:193)
	at org.eclipse.lsp4j.jsonrpc.json.StreamMessageProducer.handleMessage(StreamMessageProducer.java:192)
	at org.eclipse.lsp4j.jsonrpc.json.StreamMessageProducer.listen(StreamMessageProducer.java:94)
	at org.eclipse.lsp4j.jsonrpc.json.ConcurrentMessageProcessor.run(ConcurrentMessageProcessor.java:113)
@kittaakos kittaakos added bug bugs found in the application lsp language server protocol labels Mar 26, 2019
@kittaakos
Copy link
Contributor Author

This issue should be resolved as part of #4085. See here.

@kittaakos
Copy link
Contributor Author

kittaakos commented Mar 26, 2019

Here is a workaround, if you want to patch your extension's LanguageClientFactory:

protected patch4085(client: MonacoLanguageClient): MonacoLanguageClient {
    let patchedClient = super.patch4085(client);
    patchedClient = this.patch4723(patchedClient);
    return patchedClient;
}

/**
 * https://github.com/theia-ide/theia/issues/4723
 * Patched with https://github.com/Microsoft/vscode-languageserver-node/blob/71fc1da3fed87329080f9c2a2c32e5e07aed3128/client/src/client.ts#L3158-L3180
 */
protected patch4723(client: MonacoLanguageClient): MonacoLanguageClient {
    (client as any).handleApplyWorkspaceEdit = ((params: ApplyWorkspaceEditParams): Thenable<ApplyWorkspaceEditResponse> => {
        // This is some sort of workaround since the version check should be done by VS Code in the Workspace.applyEdit.
        // However doing it here adds some safety since the server can lag more behind then an extension.
        let workspaceEdit: WorkspaceEdit = params.edit;
        let openTextDocuments: Map<string, number> = new Map<string, number>();
        this.editorManager.all.forEach(editorWidget => {
            if (editorWidget.editor instanceof MonacoEditor) {
                const model = editorWidget.editor.getControl().getModel();
                openTextDocuments.set(model.uri.toString(), model.getVersionId())
            }
        });
        let versionMismatch = false;
        if (workspaceEdit.documentChanges) {
            for (const change of workspaceEdit.documentChanges) {
                if (TextDocumentEdit.is(change) && change.textDocument.version && change.textDocument.version >= 0) {
                    let textDocumentVersion = openTextDocuments.get(change.textDocument.uri);
                    if (!!textDocumentVersion && textDocumentVersion !== change.textDocument.version) {
                        versionMismatch = true;
                        break;
                    }
                }
            }
        }
        if (versionMismatch) {
            return Promise.resolve({ applied: false });
        }
        return this.workspace.applyEdit((client as any)._p2c.asWorkspaceEdit(params.edit)).then((value) => { return { applied: value }; });
    }).bind(client);
    return client;
}

Edit: fixed the snippet.

@kittaakos
Copy link
Contributor Author

Upstream question: microsoft/monaco-editor#1396

@kittaakos
Copy link
Contributor Author

@akosyakov, was this solved with #5901? If so, I am going to verify it.

@akosyakov akosyakov added the monaco issues related to monaco label Sep 20, 2019
@akosyakov
Copy link
Member

akosyakov commented Sep 20, 2019

@kittaakos I've fixed it in monaco-languageclient: https://github.com/TypeFox/monaco-languageclient/blame/b8f6f32429111f4d96f9581e63ecebc3cd104a7d/client/src/monaco-converter.ts#L405-L429

But we have to change MonacoWorkspace.groupEdits to respect monaco.languages.ResourceFileEdit as well. Would you be able to have a look at it?

@kittaakos
Copy link
Contributor Author

But we have to change MonacoWorkspace.groupEdits to respect monaco.languages.ResourceFileEdit as well. Would you be able to have a look at it?

Thanks for your feedback. Sure; I'll take care of it.

kittaakos pushed a commit that referenced this issue Sep 20, 2019
kittaakos pushed a commit that referenced this issue Sep 20, 2019
kittaakos pushed a commit that referenced this issue Sep 20, 2019
kittaakos pushed a commit that referenced this issue Sep 20, 2019
kittaakos pushed a commit that referenced this issue Sep 25, 2019
akosyakov pushed a commit to akosyakov/theia that referenced this issue Feb 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug bugs found in the application lsp language server protocol monaco issues related to monaco
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants