Allow renaming/creating/deleting files in a workspaced edit #41552
Conversation
mjbvz
changed the title
Fixes #10659 Allows workspace edits to also change files in the workspace
mjbvz
force-pushed
the
rename-workspaceedit-proto
branch
from
1145f32
to
e932b8a
Compare
src/vs/editor/common/modes.ts
Outdated
|
|
||
| export interface IResourceCreate { | ||
| readonly uri: URI; | ||
| readonly contents: string; |
There was a problem hiding this comment.
Is contents field really necessary? WorkspaceEdit also has edits that can be applied to 0,0 location.
There was a problem hiding this comment.
Yeah, tempting... We could then just have one type, a rename is from: Uri, to: Uri, a create would be from: null, to: Uri, and a delete would be from: Uri, to: null... Similar to how we map inserted, deleted, and changed text
| import { IResourceRename, IResourceCreate } from 'vs/editor/common/modes'; | ||
|
|
||
| export interface IResourceFileEdit { | ||
| readonly renamedResources: { from: URI, to }[]; |
src/vs/editor/common/modes.ts
Outdated
|
|
||
| export interface IResourceCreate { | ||
| readonly uri: URI; | ||
| readonly contents: string; |
There was a problem hiding this comment.
Yeah, tempting... We could then just have one type, a rename is from: Uri, to: Uri, a create would be from: null, to: Uri, and a delete would be from: Uri, to: null... Similar to how we map inserted, deleted, and changed text
|
@gorkem Just wondering in what order JDT is generating edit when renaming a class? Is it first a file rename and then an edit in the new file or the other way around, text edit the old and then rename? Or would expect to have both supported (which our proposal doesn't) |
|
@jrieken At the framework level any mixed order is supported. Everything is seen as a |
|
Makes sense @gorkem. I will give it a shot and see what I can do there... Today (without resources changes) we group all changes by file and performs per file... I'll try to preserve that but with interleaving resource changes... |
|
@mjbvz I have pushed quite a monster commit to support more interleaving between text and file changes. I say 'more' because some are extra tricky because of how the API is designed, the workspace edit automatically groups text edits by resource. That means the following
will be executed in this order: |
There was a problem hiding this comment.
I scanned over the changes but am not familiar enough with the editor logic to give change around bulkEdit a proper review.
The compromise around edit ordering you suggest sounds reasonable, although I'm not so sure if is something we should support. How much complexity does supporting out of order edits add to the implementation? Could the first iteration of this API have extra checks that prevent constructing an out of order workspaceEdit and then we could relax this in the future if the restriction is causing problems for ext developers?
| * is represented as `[from, to]`, a delete-operation as `[from, null]`, | ||
| * and a create-operation as `[null, to]`; | ||
| */ | ||
| resourceEdits(): [Uri, Uri][]; |
There was a problem hiding this comment.
I know we already use tuples in this API but they can be pretty annoying to work with. Can we use interfaces / object literal types instead? This would also better document what each component represents
| private _index = new Map<string, number>(); | ||
| private _clock: number = 0; | ||
|
|
||
| private _resourceEdits: [number/*time*/, URI, URI][] = []; |
There was a problem hiding this comment.
Same comment about avoid tuples in our internals as well. They are weird type system wise (typeof([1, 2]) !== type [number, number]) and it easy to confuse tuple fields when writing or refactoring code
There was a problem hiding this comment.
Yeah, can make that change internally.. For the API I'd go for consistency even tho I share the concern. Having said that, I don't expect heavy usage of those reading APIs
Fixes #10659
Allows workspace edits to also change files in the workspace