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

Calling WorkspaceEdit.createFile(uri, { ignoreIfExists: true }) twice discards future edits #56986

Closed
DanTup opened this issue Aug 22, 2018 · 12 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug *out-of-scope Posted issue is not in scope of VS Code workspace-edit

Comments

@DanTup
Copy link
Contributor

DanTup commented Aug 22, 2018

WorkspaceEdit's create function has an ignoreIfExists option for convenience to avoid us having to check if files exist. However, if you call it more than once, the edits made after the second call are discarded.

The following code:

changes.createFile(uri, { ignoreIfExists: true });
changes.replace(uri, new vs.Range(new vs.Position(0, 0), new vs.Position(0, 0)), "ONE");
changes.replace(uri, new vs.Range(new vs.Position(0, 0), new vs.Position(0, 0)), "TWO");
changes.createFile(uri, { ignoreIfExists: true });
changes.replace(uri, new vs.Range(new vs.Position(0, 0), new vs.Position(0, 0)), "THREE");
changes.replace(uri, new vs.Range(new vs.Position(0, 0), new vs.Position(0, 0)), "FOUR");

Results on ONETWO being inserted at the start of the document. The code executes without error (it hits the last line), the edits are just discarded.

This means we have to keep track of whether we've called it before when looping through edits from our language server - I don't think this is intended (it would be slightly more understandable if the first edits were lost, since the create call maybe was resetting the file, but this is very confusing).

@vscodebot
Copy link

vscodebot bot commented Aug 22, 2018

(Experimental duplicate detection)
Thanks for submitting this issue. Please also check if it is already covered by an existing one, like:

DanTup added a commit to Dart-Code/Dart-Code that referenced this issue Aug 22, 2018
@jrieken jrieken added workspace-edit info-needed Issue requires more information from poster labels Aug 22, 2018
@jrieken
Copy link
Member

jrieken commented Aug 22, 2018

@DanTup For me the document shows THREEFOURONETWO

@DanTup
Copy link
Contributor Author

DanTup commented Aug 22, 2018

Ugh, wha? :(

Here's my full code (I'm calling this from a command from the palette):

private async organizeImports(document: vs.TextDocument): Promise<void> {
	document = document || this.getActiveDoc();
	const uri = document.uri;
	const changes = new vs.WorkspaceEdit();
	changes.createFile(uri, { ignoreIfExists: true });
	changes.replace(uri, new vs.Range(new vs.Position(0, 0), new vs.Position(0, 0)), "ONE");
	changes.replace(uri, new vs.Range(new vs.Position(0, 0), new vs.Position(0, 0)), "TWO");
	changes.createFile(uri, { ignoreIfExists: true });
	changes.replace(uri, new vs.Range(new vs.Position(0, 0), new vs.Position(0, 0)), "THREE");
	changes.replace(uri, new vs.Range(new vs.Position(0, 0), new vs.Position(0, 0)), "FOUR");
	await vs.workspace.applyEdit(changes);
}

Here's what my doc looks like afterwards:

screen shot 2018-08-22 at 12 00 30 pm

@DanTup
Copy link
Contributor Author

DanTup commented Aug 22, 2018

Seeing same behaviour in latest Insiders too. Is your code any different to mine?

@DanTup
Copy link
Contributor Author

DanTup commented Aug 22, 2018

Oh also:

For me the document shows THREEFOURONETWO

That does not seem right either - it should be ONETWOTHREEFOUR, right? The edit is atomic and the positions were all the same? Maybe you had two separate edits somehow?

@jrieken
Copy link
Member

jrieken commented Aug 22, 2018

Below is my snippet. It matters if the file already exists the very first time or not. Also, there is an error being printed. @DanTup don't you see that error?

ERR Error: file:///Users/jrieken/Code/_samples/ConsoleAppDnx/test.fsx.txt has changed in the meantime
    at BulkEdit.<anonymous> (bulkEditService.js:409)
    at step (bulkEditService.js:50)
    at Object.next (bulkEditService.js:31)
    at fulfilled (bulkEditService.js:22)
    at <anonymous>
 let { document } = vscode.window.activeTextEditor;

        let edit = new vscode.WorkspaceEdit();
        let uri = document.uri.with({ path: document.uri.path + '.txt' });
        // let uri = document.uri;

        edit.createFile(uri, { ignoreIfExists: true });
        edit.replace(uri, new vscode.Range(new vscode.Position(0, 0), new vscode.Position(0, 0)), "ONE");
        edit.replace(uri, new vscode.Range(new vscode.Position(0, 0), new vscode.Position(0, 0)), "TWO");
        edit.createFile(uri, { ignoreIfExists: true });
        edit.replace(uri, new vscode.Range(new vscode.Position(0, 0), new vscode.Position(0, 0)), "THREE");
        edit.replace(uri, new vscode.Range(new vscode.Position(0, 0), new vscode.Position(0, 0)), "FOUR");

        vscode.workspace.applyEdit(edit);

@jrieken jrieken added bug Issue identified by VS Code Team member as probable bug and removed info-needed Issue requires more information from poster labels Aug 22, 2018
@DanTup
Copy link
Contributor Author

DanTup commented Aug 22, 2018

You're right, the behaviour differs if the file exists or not - but I think both are broken:

  1. If the file already exists, the 3rd/4th edit are ignored - we're passing ignoreIfExists which suggests the create should be a no-op, so it doesn't seem right that edits are discarded
  2. If the file does not exist, the file ends up saying THREEFOURONETWO but ONETWOTHREEFOUR is expected

I do not see the error you get, however if I try to close the unsaved file with the dev tools open, it does seem to pause on a rejected promise (though doesn't write to the console). I think that maybe normal (the ext. dev host doesn't show the save prompt).

DanTup added a commit to Dart-Code/Dart-Code that referenced this issue Aug 22, 2018
@jrieken
Copy link
Member

jrieken commented Aug 22, 2018

What are your save settings? Auto save, save on focus out?

@DanTup
Copy link
Contributor Author

DanTup commented Aug 22, 2018

Just all defaults in dev host (no auto saving).

Weirdly, I am now seeing "Error: file:///Users/dantup/Desktop/Dart%20Sample/test/main.dart has changed in the meantime" when this code runs. I tried several times earlier and didn't see it :-/ Probably that's related to the issue then.

As for the other thing (breaking on promise reject when closing the window), that seems unrelated to the creation of the file since it happens even when modifying any file in the dev host and hitting Ctrl+W to close it. It may be normal, I guess rejected promises are treated like unhandled exceptions and break by default.

Looks like this (if you think it is an issue, I can open a new issue for it to avoid confusing this one):

screen shot 2018-08-22 at 3 18 47 pm

DanTup added a commit to Dart-Code/Dart-Code that referenced this issue Aug 22, 2018
@jrieken jrieken added the *out-of-scope Posted issue is not in scope of VS Code label Jan 28, 2019
@vscodebot
Copy link

vscodebot bot commented Jan 28, 2019

This issue is being closed to keep the number of issues in our inbox on a manageable level, we are closing issues that are not going to be addressed in the foreseeable future: We look at the number of votes the issue has received and the number of duplicate issues filed. More details here. If you disagree and feel that this issue is crucial: We are happy to listen and to reconsider.

If you wonder what we are up to, please see our roadmap and issue reporting guidelines.

Thanks for your understanding and happy coding!

@vscodebot vscodebot bot closed this as completed Jan 28, 2019
@DanTup
Copy link
Contributor Author

DanTup commented Jan 28, 2019

@jrieken based on the #56986 (comment) it seems like this API doe the wrong thing whether the file exists or not. If there's no plan to fix it, should the docs at least be updated to mention this?

@taoqf
Copy link

taoqf commented Oct 15, 2019

deleteFile has the same behavior even if set ignoreIfNotExists to true,
createFile use overwrite does the same.
I am using vscode 1.39.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue identified by VS Code Team member as probable bug *out-of-scope Posted issue is not in scope of VS Code workspace-edit
Projects
None yet
Development

No branches or pull requests

3 participants