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

The api workspace.applyEdit drops the TextEdit if there is a RenameFile later #77735

Closed
testforstephen opened this issue Jul 22, 2019 · 3 comments

Comments

@testforstephen
Copy link

commented Jul 22, 2019

I have a WorkspaceEdit as below:

A simplified sample:
{
documentChanges: [
TextEdit to file1,
TextEdit to file2,
TextEdit to file1,
Move file1 to file1New
]
}

A real WorkspaceEdit for apply

{
    "changes": {},
    "documentChanges": [
        {
            "textDocument": {
                "version": 0,
                "uri": "file:///C:/Work/Debugger/lsp4xml/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/services/XMLCodeLens.java"
            },
            "edits": [
                {
                    "range": {
                        "start": {
                            "line": 16,
                            "character": 47
                        },
                        "end": {
                            "line": 17,
                            "character": 43
                        }
                    },
                    "newText": "\r\nimport org.eclipse.lsp4xml.services.CodeLensRequest;"
                }
            ]
        },
        {
            "textDocument": {
                "version": 0,
                "uri": "file:///C:/Work/Debugger/lsp4xml/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/services/XMLLanguageService.java"
            },
            "edits": [
                {
                    "range": {
                        "start": {
                            "line": 44,
                            "character": 42
                        },
                        "end": {
                            "line": 44,
                            "character": 42
                        }
                    },
                    "newText": "\r\nimport org.eclipse.lsp4xml.dom.XMLCodeLens;"
                }
            ]
        },
        {
            "textDocument": {
                "version": 0,
                "uri": "file:///C:/Work/Debugger/lsp4xml/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/services/XMLCodeLens.java"
            },
            "edits": [
                {
                    "range": {
                        "start": {
                            "line": 10,
                            "character": 8
                        },
                        "end": {
                            "line": 10,
                            "character": 36
                        }
                    },
                    "newText": "org.eclipse.lsp4xml.dom"
                }
            ]
        },
        {
            "oldUri": "file:///C:/Work/Debugger/lsp4xml/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/services/XMLCodeLens.java",
            "newUri": "file:///C:/Work/Debugger/lsp4xml/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/dom/XMLCodeLens.java",
            "kind": "rename"
        }
    ]
}

Log from the command "Developer: Open Log File... -> Window"

[2019-07-22 17:27:29.179] [renderer11] [debug] _performTextEdits

[{
		"resource": {
			"$mid": 1,
			"external": "file:///c%3A/Work/Debugger/lsp4xml/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/services/XMLCodeLens.java",
			"path": "/C:/Work/Debugger/lsp4xml/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/services/XMLCodeLens.java",
			"scheme": "file"
		},
		"edits": [{
				"text": "\r\nimport org.eclipse.lsp4xml.services.CodeLensRequest;",
				"range": {
					"startLineNumber": 17,
					"startColumn": 48,
					"endLineNumber": 18,
					"endColumn": 44
				}
			}
		]
	}, {
		"resource": {
			"$mid": 1,
			"external": "file:///c%3A/Work/Debugger/lsp4xml/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/services/XMLLanguageService.java",
			"path": "/C:/Work/Debugger/lsp4xml/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/services/XMLLanguageService.java",
			"scheme": "file"
		},
		"modelVersionId": 3,
		"edits": [{
				"text": "\r\nimport org.eclipse.lsp4xml.dom.XMLCodeLens;",
				"range": {
					"startLineNumber": 45,
					"startColumn": 43,
					"endLineNumber": 45,
					"endColumn": 43
				}
			}
		]
	}, {
		"resource": {
			"$mid": 1,
			"external": "file:///c%3A/Work/Debugger/lsp4xml/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/services/XMLCodeLens.java",
			"path": "/C:/Work/Debugger/lsp4xml/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/services/XMLCodeLens.java",
			"scheme": "file"
		},
		"edits": [{
				"text": "org.eclipse.lsp4xml.dom",
				"range": {
					"startLineNumber": 11,
					"startColumn": 9,
					"endLineNumber": 11,
					"endColumn": 37
				}
			}
		]
	}
]

[2019-07-22 17:27:29.184] [renderer11] [debug] _performFileEdits

[{
		"oldUri": {
			"$mid": 1,
			"path": "/C:/Work/Debugger/lsp4xml/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/services/XMLCodeLens.java",
			"scheme": "file"
		},
		"newUri": {
			"$mid": 1,
			"path": "/C:/Work/Debugger/lsp4xml/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/dom/XMLCodeLens.java",
			"scheme": "file"
		}
	}
]

The edits are supposed to be applied by order, the text edits for the old file should be applied first, then move to the new file.

But actually the change for file2 is always applied correctly, but sometimes the text changes for file1 don't take effect. Especially when file1 is not opened in VS Code before applying, the text changes for file1 are often dropped. Besides, if i move the same file back and forth, it can be easily reproduced.

jrieken added a commit that referenced this issue Jul 23, 2019
@jrieken

This comment has been minimized.

Copy link
Member

commented Jul 23, 2019

I have added an unit test and I cannot reproduce. Please provide extension code that demonstrates this...

@testforstephen

This comment has been minimized.

Copy link
Author

commented Jul 24, 2019

Below is my reproducing script.

  1. Create an empty file1 in your workspace root, and open it in VS Code.
  2. Run extension.helloWorld, it succeeds without errors.
  3. But run extension.helloWorld1, it doesn't. This step adds a step to close all editors before applying edit. Running 10 times, you will see it failed several times.
import * as vscode from 'vscode';
import * as path from 'path';

export function activate(context: vscode.ExtensionContext) {
	let disposable = vscode.commands.registerCommand('extension.helloWorld', async () => {
		await testApplyEdit(false);
	});

	let disposable1 = vscode.commands.registerCommand('extension.helloWorld1', async () => {
		await testApplyEdit(true);
	});

	context.subscriptions.push(disposable);
	context.subscriptions.push(disposable1);
}

async function testApplyEdit(closeEditor: boolean) {
	const result = await vscode.workspace.findFiles("file1");
	if (!result || !result.length) {
		vscode.window.showErrorMessage("Cannot find file1.");
		return;
	}

	let docUri = result[0];
	for (let i = 0; i < 10; i++) {
		if (closeEditor) {
			await vscode.commands.executeCommand("workbench.action.closeAllEditors");
		}
		let we = new vscode.WorkspaceEdit();
		let oldUri, newUri;
		let expected;
		if (i % 2 === 0) {
			oldUri = docUri;
			newUri = vscode.Uri.parse(docUri.toString().replace('file1', 'newfile1'));
			we.insert(oldUri, new vscode.Position(0, 0), 'Hello');
			expected = 'Hello';
		} else {
			oldUri = vscode.Uri.parse(docUri.toString().replace('file1', 'newfile1'));
			newUri = docUri;
			we.delete(oldUri, new vscode.Range(new vscode.Position(0, 0), new vscode.Position(0, 5)));
			expected = '';
		}

		console.log(`${i}: Dealing with ${oldUri.toString()} - ${newUri.toString()}`);
		we.renameFile(oldUri, newUri);
		await vscode.workspace.applyEdit(we);
		const document = await vscode.workspace.openTextDocument(newUri);
		await document.save();
		
		if (document.getText() !== expected) {
			vscode.window.showErrorMessage(`${i}: ${path.basename(newUri.fsPath)} failed. Expected: '${expected}', Actual: '${document.getText()}'`);
		}
		await delay(1000);
	}
}

function delay(ms: number) {
    return new Promise( resolve => setTimeout(resolve, ms) );
}

export function deactivate() {}

@jrieken jrieken assigned bpasero and unassigned jrieken Jul 24, 2019

@jrieken jrieken added file-io and removed needs more info labels Jul 24, 2019

@jrieken

This comment has been minimized.

Copy link
Member

commented Jul 24, 2019

Thanks, I can now reproduce. @bpasero It seems that textFileEditorManager knows that the file is dirty but that it doesn't get saved before moving/renaming the file. Calling save before move on my side (bulk edit) doesn't help...

@bpasero bpasero modified the milestones: July 2019, August 2019 Aug 5, 2019

@bpasero bpasero added the bug label Aug 10, 2019

bpasero added a commit that referenced this issue Aug 11, 2019

@bpasero bpasero closed this in 5099e36 Aug 12, 2019

@mjbvz mjbvz added the verified label Aug 28, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.