[WIP] [editor] Renaming a file should rename a corresponding editor #582
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,7 @@ import { UriSelection } from '@theia/filesystem/lib/common/filesystem-selection' | |
| import { SingleTextInputDialog, ConfirmDialog } from "@theia/core/lib/browser/dialogs"; | ||
| import { OpenerService, OpenHandler, open } from "@theia/core/lib/browser"; | ||
| import { WorkspaceService } from './workspace-service'; | ||
| import { Emitter, Event } from '@theia/core/lib/common/event'; | ||
|
|
||
| export namespace WorkspaceCommands { | ||
| export const NEW_FILE = 'file:newFile'; | ||
|
|
@@ -39,6 +40,22 @@ export namespace FileMenus { | |
| export const OPEN_GROUP = [...FILE, '2_open']; | ||
| } | ||
|
|
||
| // @injectable() | ||
| // export class UiEvent { | ||
| // public onIUriChangedEmitter = new Emitter<IUriChangedEvent>(); | ||
|
|
||
| // get onIUriChanged(): Event<IUriChangedEvent> { | ||
| // return this.onIUriChangedEmitter.event; | ||
| // } | ||
| // } | ||
|
|
||
| export interface IUriChangedEvent { | ||
| factoryId: string; | ||
| options: any; | ||
| newOptions: any; | ||
| newLabel: any; | ||
| } | ||
|
|
||
| @injectable() | ||
| export class FileMenuContribution implements MenuContribution { | ||
|
|
||
|
|
@@ -64,6 +81,15 @@ export class WorkspaceCommandContribution implements CommandContribution { | |
| @inject(OpenerService) protected readonly openerService: OpenerService | ||
| ) { } | ||
|
|
||
|
|
||
|
|
||
|
|
||
| protected readonly onIUriChangedEmitter = new Emitter<IUriChangedEvent>(); | ||
|
|
||
| get onIUriChanged(): Event<IUriChangedEvent> { | ||
|
There was a problem hiding this comment. We should move this to FileSytem and call it FileMovedEvent and it should be about what moved. There was a problem hiding this comment. Once the EditorManager receives it it can check if it has a editor for that URI and update it. |
||
| return this.onIUriChangedEmitter.event; | ||
| } | ||
|
|
||
| registerCommands(registry: CommandRegistry): void { | ||
| registry.registerCommand({ | ||
| id: WorkspaceCommands.NEW_FILE, | ||
|
|
@@ -104,9 +130,17 @@ export class WorkspaceCommandContribution implements CommandContribution { | |
| initialValue: uri.path.base, | ||
| validate: name => this.validateFileName(name, parent) | ||
| }); | ||
| dialog.open().then(name => { | ||
| this.fileSystem.move(uri.toString(), uri.parent.resolve(name).toString()).then(() => { | ||
|
There was a problem hiding this comment. I think @svenefftinge meant that you only should close an existing editor and open new: const newUri = uri.parent.resolve(name);
this.fileSystem.move(uri.toString(), newUri.toString()).then(() => {
const widget = this.editorService.getEditor(uri);
if (editor) {
const selection = widget.editor.selection;
widget.close();
open(this.openerService, newUri, {selection});
}
});Ideally, it should be done not here but on move/rename events from the filesystem in the editor extension. Making it here:
There was a problem hiding this comment. No, I did not mean that. A real rename seems better to me in general. But we have to take care of all the loose ends. There was a problem hiding this comment. Maybe we should have an generic key to reference the widget directly to avoid lose ends ? There was a problem hiding this comment. Could you elaborate on what you mean? There was a problem hiding this comment. I mean that if something has to keep a reference to an editor widget it should reference the widget object directly or we should provide a key that is not bound to any resource to refer to it. Like some abstract id. So the URI would be used as key to open a new widget or find a widget for that URI but once you get a reference to it, it should be the id or the object directly, a bit like files names and files descriptors.. you open(filename) get a file descriptor so you could getWidgetForURI() get a widget for a URI but use getWidgetId() if you already had a widget and now know only it's id. I don't see where however that would be useful.. just keeping the object seems ok since serializing that doesn't make sense as the widget-manager is in the frontend.
Why would it try to reload with an old uri ? if you updated it before you serialized the widget ? Granted if it's changed between save and restore there's nothing we can do but that is true whatever we do... |
||
| let event: IUriChangedEvent = { | ||
| factoryId: "code-editor-opener", | ||
| options: uri.parent.resolve(uri.path.base).toString(), | ||
| newOptions: uri.parent.resolve(name).toString(), | ||
| newLabel: name | ||
| }; | ||
| this.onIUriChangedEmitter.fire(event); | ||
|
There was a problem hiding this comment. Actually thinking more about this it would make more sense to have FileSytem.move() fire this event and have the event emitter there. |
||
| }); | ||
| }); | ||
| }) | ||
| })); | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need to change this