Allow extensions to participate in file rename/move operations

[DanTup]

Many languages import files by filename:

import "myfile.dart";

If a user renames this file using explorer, the code references to it become broken and need fixing up. The Dart language server has support for returning the code edits required during a file rename to avoid this. As far as I can see there isn't an API for this yet.

I think could be useful for many languages (Dart, JS, TS, maybe even HTML's script tags/link tags!).

[jrieken]

Do you want to be part of the rename-file-command or just have an event/file-event that tells you something has been renamed?

[DanTup]

Based on the language server docs:

The refactoring must be activated before an actual file moving operation is performed.

I guess I'd need to know before it happened (I guess once the rename has happened, it may not be able to figure out the changes required). The API in theory could return errors, so it may be worthwhile allowing cancellation of it with some sort of message, though I don't know whether that's actually a possibility in the implementation (their MOVE_FILE is part of a refactor API which handles other things so the API is somewhat generic).

[jrieken]

Thanks for clarifying - I have updated the title accordingly

[mjbvz]

TS just merged an API for rename file: microsoft/TypeScript#23573

Cases we'd need events for:

• Rename file in explorer
• Drag and drop file between folders in explorer

If we want to allow extensions to react to renames (this is how TS API is designed to be used), the expected API could be something like:

onDidRenameFile: Event<{ oldResourceLocation: Uri, newResourceLocation: Uri }>

If we want extensions to be able to participate in the rename itself, we'd need a new hook (or possibly a new provider)

I'd say we should only handle renames in the explore and not try to handle renames outside of the editor or try making this API part of the FileWatcher. This should simplify the implementation while still covering most of the use cases for triggering actions based on file renames

[DanTup]

I'd say we should only handle renames in the explorer

This sounds reasonable to me; however to handle this for Dart we really need to be called before the rename occurs; onDidRenameFile will be too late.

[jrieken]

I'd say we should only handle renames in the explore and not try to handle renames outside of the editor or try making this API part of the FileWatcher.

Agreed, but going a step further to say this is shouldn't be depend on events at all. When moving a file (via whatever user-gesture) we should now that there is a move/rename participant that we need to involve in the process. And it should probably not be the default (or behind a prompt) so that you still move a file without the smarts

[mjbvz]

Related to #24846

[mjbvz]

A few thoughts on possible API designs:

Event

export interface ResourceRenamedEvent {
	readonly oldResource: Uri;
	readonly newResource: Uri;
}

export namespace workspace {
	export const onDidRenameResource: Event<ResourceRenamedEvent>;
}

Simplest. Allow an extension to be notified after the rename has occurred (thus not solving the dart requirement). Also cannot be scoped per language/filetype

Provider / Handler

export interface ResourceRenameProvider {
	provideRenameEdits(oldResource: Uri, newResource: Uri, token: CancellationToken): ProviderResult<WorkspaceEdit>;

	prepareRename?(oldResource: Uri, newResource: Uri, token: CancellationToken): ProviderResult<{ /* something */ }>;
}

export namespace workspace {
	export function registerResourceRenameProvider(selector: DocumentSelector, provider: ResourceRenameProvider): Disposable;
}

Create a standarized provider that can handle renames. This provider could start out simple and be extended with other functionality, such as intercepting renames before they happen with prepareRename.

One question is how to handle directory renames and how these would work if we also use a document selector

[DanTup]

Obviously I prefer the provider =)

I think it may be confusing to have both provideRenameEdits and prepareRename if they both fire before the rename though.

One question is how to handle directory renames and how these would work if we also use a document selector

What about just calling for each file inside? Renaming a folder is somewhat the same as creating a folder, moving all the files to it (we've been calling it rename, but move might be more appropriate, since it should be able to span folders) and then deleting the old folder.

It's also worth thinking about how these will chain together if more than one is registered (if that's allowed).

[DanTup]

Is this API usable by us in v1.24, or only internal extensions? Was it implemented in a way that supports our rename operations?

[mjbvz]

No, this api is still in proposed and does not support being triggered pre-rename

[DanTup]

Is it likely that this will be supported in future?

[jrieken]

re #43768 (comment)

Not sure if provider is a good name here... Maybe add the concept of a participant or have onWillRenameResource: Event<WillRenameEvent>. That would be much in-line with our onWillSave-event, e.g

interface WillRenameResourceEvent {
  oldUri: Uri,
  newUri: Uri,
  waitUntil(thenable: Thenable<WorkspaceEdit>);
}

const onWillRenameResource: Event<WillRenameResourceEvent>;

[jrieken]

@bpasero An onWillRenameFile-event requires some internal infrastructure (similar to onWillSave) and the question is where to put it. We have the corresponding functions in the file service interface but I also know that renaming a file is much more elaborated (reverting changes, closing editor etc). What is your recommendation esp. since debt-work is planned. Will there be an injectable, service-like rename-operation which should handle this?

[bpasero]

@jrieken I think this could go into the text file service once it implements the rename/delete logic for the text resource edits (#42640). The explorer will use that service once it is capable of these methods.

[mjbvz]

One other thing to consider: we may need a new activation event so that a language extension that handles moves/renames can be activated even if no files of that language have been opened yet. See #51298

Would be best if we could do this per language:

"activationEvents": [
     "onRenameFile:javascript"
]

Not clear what to do about directory renames here

[DanTup]

For a directory, you could just get languages for all descendants and fire the event for them?

I think we activate on workspaceContains so a new activation event probably isn't necessary (I don't know if that's common though).

[jrieken]

This is the current proposal:

vscode/src/vs/vscode.proposed.d.ts

Lines 826 to 919 in 1dc2f73

	export interface FileCreateEvent {
	/**
	* The files that got created.
	*/
	readonly files: ReadonlyArray<Uri>;
	}
	export interface FileWillCreateEvent {
	/**
	* The files that are going to be created.
	*/
	readonly files: ReadonlyArray<Uri>;
	waitUntil(thenable: Thenable<WorkspaceEdit>): void;
	waitUntil(thenable: Thenable<any>): void;
	}
	export interface FileDeleteEvent {
	/**
	* The files that got deleted.
	*/
	readonly files: ReadonlyArray<Uri>;
	}
	export interface FileWillDeleteEvent {
	/**
	* The files that are going to be deleted.
	*/
	readonly files: ReadonlyArray<Uri>;
	waitUntil(thenable: Thenable<WorkspaceEdit>): void;
	waitUntil(thenable: Thenable<any>): void;
	}
	export interface FileRenameEvent {
	/**
	* The files that got renamed.
	*/
	readonly files: ReadonlyArray<{ oldUri: Uri, newUri: Uri }>;
	}
	export interface FileWillRenameEvent {
	/**
	* The files that are going to be renamed.
	*/
	readonly files: ReadonlyArray<{ oldUri: Uri, newUri: Uri }>;
	waitUntil(thenable: Thenable<WorkspaceEdit>): void;
	waitUntil(thenable: Thenable<any>): void;
	}
	export namespace workspace {
	/**
	* An event that is emitted when files are being created.
	*
	* *Note* that this event is triggered by user gestures, like creating a file from the
	* explorer, or from the [`workspace.applyEdit`](#workspace.applyEdit)-api. This event is *not* fired when
	* files change on disk, e.g triggered by another application, or when using the
	* [`workspace.fs`](#FileSystem)-api.
	*/
	export const onWillCreateFiles: Event<FileWillCreateEvent>;
	/**
	* An event that is emitted when files are being deleted.
	*
	* *Note* that this event is triggered by user gestures, like deleting a file from the
	* explorer, or from the [`workspace.applyEdit`](#workspace.applyEdit)-api. This event is *not* fired when
	* files change on disk, e.g triggered by another application, or when using the
	* [`workspace.fs`](#FileSystem)-api.
	*/
	export const onWillDeleteFiles: Event<FileWillDeleteEvent>;
	/**
	* An event that is emitted when files are being renamed.
	*
	* *Note* that this event is triggered by user gestures, like renaming a file from the
	* explorer, and from the [`workspace.applyEdit`](#workspace.applyEdit)-api. This event is *not* fired when
	* files change on disk, e.g triggered by another application, or when using the
	* [`workspace.fs`](#FileSystem)-api.
	*/
	export const onWillRenameFiles: Event<FileWillRenameEvent>;
	export const onDidCreateFiles: Event<FileCreateEvent>;
	export const onDidDeleteFiles: Event<FileDeleteEvent>;
	export const onDidRenameFiles: Event<FileRenameEvent>;
	}

@DanTup Can you give it a try/read. Note the ways in which the event is triggered, e.g via user gestures and applyEdit but not via OS file events or the workspace.fs-api. Make sense? Surprises?

[DanTup]

@jrieken looks good to me. I think it's worth being explicit in the comments about what happens when a folder is moved though - does it send a single entry with the folder in it, or a collection of all the descendant files?

Also - if the move will conflict with existing files (eg. I drag foo.txt into bar which already has a foo.txt) how will that behave? Will VS Code prompt to overwrite first? Should extensions assume if they get a rename event and a destination already exists, VS Code is going to replace it unconditionally (and if so, would that first fire a delete event?)

[jrieken]

I think it's worth being explicit in the comments about what happens when a folder is moved though

Only the folder changes, added that to the notes

Also - if the move will conflict with existing files (eg. I drag foo.txt into bar which already has a foo.txt) how will that behave? Will VS Code prompt to overwrite first? Should extensions assume if they get a rename event and a destination already exists, VS Code is going to replace it unconditionally (and if so, would that first fire a delete event?)

There is no guarantee that the operation will succeed when will-events are created and generally no preparation has been performed yet. The latter shouldn't be too much of a problem because the WorkspaceEdit that listeners can add should generally allow you create folders etc

[DanTup]

The latter shouldn't be too much of a problem because the WorkspaceEdit that listeners can add should generally allow you create folders etc

That's true, but there are weird unfixed bugs like #56986 :(

[isidorn]

@bpasero I have added textFileService.copy api via 137bda6

@jrieken now the explorer no longer uses the fileService, only for resolving, checking if it can handle a resource and minor things like that. All operations now use the textFileService. The only exception being the creation of a folder, since I did not want to introduce notion of folders to the textFileService for now. Explorer does not use the bulk api, since the fileService could not reliebly do multiple file operations in parallel. So for now the textFileService does not need this api.

Try it out and let me know how it beahves. I just verified that the operations in the explorer work as before.

@jrieken will there be a test plan item for this? I would not mind having some people verify that the explorer actions are working as before.

Please let me know if anything else is needed from the Explorer side. Thanks

[jrieken]

@jrieken will there be a test plan item for this? I would not mind having some people verify that the explorer actions are working as before.

Yes - be aware that you'll receive the issues for not supporting N-file operation. It's not about running them in parallel but about signalling correctly to the outside world. Simply add a moveN-function that fires an event for all and then talks to the file service sequentially.

[isidorn]

@jrieken yes, makes sense. Thanks

[bpasero]

@isidorn curious what kind of issue you saw when using the file service in parallel with different operations?

[isidorn]

@bpasero that was the older issue almost a year ago. Some operations would not be executed. I can try it again and let you know if that is still the case

[bpasero]

@isidorn yes please do and file a separate issue