WorkspaceEdit: edit followed by deleteResource does not work

[aeschli]

Testing #42334

Run the extension below.

• inserts text to the current document
• deletes the resource

The deleteResource seems to be ignore

'use strict';
import * as vscode from 'vscode';
import * as path from 'path';

export function activate(context: vscode.ExtensionContext) {
    let disposable = vscode.commands.registerCommand('extension.sayHello', () => {
        let editor = vscode.window.activeTextEditor;
        if (editor) {
            let doc = editor.document;
            let docUri = doc.uri;

            let we = new vscode.WorkspaceEdit();
            we.insert(docUri, new vscode.Position(0, 0), 'Hello');
            we.deleteResource(docUri);
            vscode.workspace.applyEdit(we);
        }
    });

    context.subscriptions.push(disposable);
}

[jrieken]

@aeschli It should show like this: '(deleted from disk)'.

@bpasero Is there a way to prevent this? Don't we know that is was us, e.g. our file service, deleting the file so that we don't keep the editor around?

[bpasero]

@jrieken it will close by default, unless workbench.editor.closeOnFileDelete is configured, which you maybe did?

[jrieken]

which you maybe did?

Nope... It also doesn't explain @aeschli's observation which is having both editors visible...

[bpasero]

@jrieken ah I see, the file is dirty and gets deleted from disk. we never ever close an editor that is dirty, no matter what happens.

[jrieken]

So, should we save it then before deleting?

[jrieken]

we never ever close an editor that is dirty, no matter what happens.

Wait. I have an editor with unsaved changes and then I delete its form the explorer and the editor doesn't stick around. @bpasero How is that controlled? I always thought that's why we have file events and file service events?

[bpasero]

@jrieken yeah, the explorer reverts the file before then deleting it. I think it would be fair if you do the same in this case.

[jrieken]

Hm... Uses a service isn't available in the common-layer (in contrast to the file service)... Would a simple save work?

[jrieken]

Would a simple save work?

I believe no

[bpasero]

@jrieken you mean ITextFileService? that one should be accessible from common, at least the interface lives in vs/workbench/services/textfile/common/textfiles.ts

[jrieken]

Sorry, imprecise wording. By 'common-layer' I have meant platform or accessible from the editor

[bpasero]

@jrieken it does not seem like ITextFileService could not live in platform, there are not a lot of workbench dependencies (just a few interfaces that could also be moved). We would have to provide some kind of bogus default implementation for the standalone editor though.

[jrieken]

We would have to provide some kind of bogus default implementation for the standalone editor though.

Yeah, I'd opt for the other direction and that we move the workspace edit up, so rename and code actions would be gone from the standalone editor

[jrieken]

@bpasero I have moved the rename/bulk-edit logic into a separate IBulkEditorService. The real implementation lives in the workbench layer and can use whatever it takes to implement rename/delete/create. Today it uses the IFileService for that which seems to be insufficient. I'll leave it up to you to correct this

[bpasero]

@jrieken I adopted new methods ITextFileService#delete and ITextFileService#move in the bulkEditService. These new methods will:

• revert a dirty file if it gets deleted
• preserve the contents of a dirty file if it gets moved

There are 3 outstanding questions:

• currently there is no prompt happening when the file is dirty and about to be deleted, would you expect one?
• currently I do not expect the URIs in these methods to be folders, just files because the text file service is typically just talking about files. would the bulk service ever pass in folder resources?
• I left the createFile call as it was going through the IFileService because I do not assume this one was causing any issues?

Edit
I guess we want to support folders at least for the scenario in #43768.

[jrieken]

Yes, folder support is needed. And, 'no' I don't expect a prompt to happen

[bpasero]

@jrieken added folder support. The explorer is now using the ITextFileService#move() method for both renaming a file/folder or when using drag and drop. This opens the door for participation in these operations from extensions. I checked that nobody else is using this functionality so far besides the explorer.