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

Propagate "Save As" operation to plugin host #13689

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@

<!-- ## not yet released

<a name="breaking_changes_not_yet_released">[Breaking Changes:](#breaking_changes_not_yet_released)</a> -->
<a name="breaking_changes_not_yet_released">[Breaking Changes:](#breaking_changes_not_yet_released)</a>

- [filesystem] Adjusted the "Save As" mechanism. It now assumes that `Saveable.getSnapshot()` returns a full snapshot of the editor model [#13689](https://github.com/eclipse-theia/theia/pull/13689).

-->

## 1.50.0 - 06/03/2024

Expand Down
10 changes: 9 additions & 1 deletion packages/core/src/browser/saveable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export interface Saveable {
*/
revert?(options?: Saveable.RevertOptions): Promise<void>;
/**
* Creates a snapshot of the dirty state.
* Creates a snapshot of the dirty state. See also {@link Saveable.Snapshot}.
*/
createSnapshot?(): Saveable.Snapshot;
/**
Expand Down Expand Up @@ -114,7 +114,15 @@ export namespace Saveable {
soft?: boolean
}

/**
* A snapshot of a saveable item. Contains the full content of the saveable file in a string serializable format.
*/
export type Snapshot = { value: string } | { read(): string | null };
export namespace Snapshot {
export function read(snapshot: Snapshot): string | undefined {
return 'value' in snapshot ? snapshot.value : (snapshot.read() ?? undefined);
}
}
export function isSource(arg: unknown): arg is SaveableSource {
return isObject<SaveableSource>(arg) && is(arg.saveable);
}
Expand Down
19 changes: 16 additions & 3 deletions packages/core/src/browser/shell/application-shell.ts
Original file line number Diff line number Diff line change
Expand Up @@ -960,7 +960,7 @@ export class ApplicationShell extends Widget {
}
}

getInsertionOptions(options?: Readonly<ApplicationShell.WidgetOptions>): { area: string; addOptions: DockLayout.IAddOptions; } {
getInsertionOptions(options?: Readonly<ApplicationShell.WidgetOptions>): { area: string; addOptions: TheiaDockPanel.AddOptions; } {
let ref: Widget | undefined = options?.ref;
let area: ApplicationShell.Area = options?.area || 'main';
if (!ref && (area === 'main' || area === 'bottom')) {
Expand All @@ -969,7 +969,7 @@ export class ApplicationShell extends Widget {
}
// make sure that ref belongs to area
area = ref && this.getAreaFor(ref) || area;
const addOptions: DockPanel.IAddOptions = {};
const addOptions: TheiaDockPanel.AddOptions = {};
if (ApplicationShell.isOpenToSideMode(options?.mode)) {
const areaPanel = area === 'main' ? this.mainPanel : area === 'bottom' ? this.bottomPanel : undefined;
const sideRef = areaPanel && ref && (options?.mode === 'open-to-left' ?
Expand All @@ -981,6 +981,10 @@ export class ApplicationShell extends Widget {
addOptions.ref = ref;
addOptions.mode = options?.mode === 'open-to-left' ? 'split-left' : 'split-right';
}
} else if (ApplicationShell.isReplaceMode(options?.mode)) {
addOptions.ref = options?.ref;
addOptions.closeRef = true;
addOptions.mode = 'tab-after';
} else {
addOptions.ref = ref;
addOptions.mode = options?.mode;
Expand Down Expand Up @@ -2172,6 +2176,15 @@ export namespace ApplicationShell {
return mode === 'open-to-left' || mode === 'open-to-right';
}

/**
* Whether the `ref` of the options widget should be replaced.
*/
export type ReplaceMode = 'tab-replace';

export function isReplaceMode(mode: unknown): mode is ReplaceMode {
return mode === 'tab-replace';
}

/**
* Options for adding a widget to the application shell.
*/
Expand All @@ -2185,7 +2198,7 @@ export namespace ApplicationShell {
*
* The default is `'tab-after'`.
*/
mode?: DockLayout.InsertMode | OpenToSideMode
mode?: DockLayout.InsertMode | OpenToSideMode | ReplaceMode
/**
* The reference widget for the insert location.
*
Expand Down
12 changes: 11 additions & 1 deletion packages/core/src/browser/shell/theia-dock-panel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,11 +133,14 @@ export class TheiaDockPanel extends DockPanel {
}
}

override addWidget(widget: Widget, options?: DockPanel.IAddOptions): void {
override addWidget(widget: Widget, options?: TheiaDockPanel.AddOptions): void {
if (this.mode === 'single-document' && widget.parent === this) {
return;
}
super.addWidget(widget, options);
if (options?.closeRef) {
options.ref?.close();
}
this.widgetAdded.emit(widget);
this.markActiveTabBar(widget.title);
}
Expand Down Expand Up @@ -252,4 +255,11 @@ export namespace TheiaDockPanel {
export interface Factory {
(options?: DockPanel.IOptions): TheiaDockPanel;
}

export interface AddOptions extends DockPanel.IAddOptions {
/**
* Whether to also close the widget referenced by `ref`.
*/
closeRef?: boolean
}
}
5 changes: 4 additions & 1 deletion packages/core/src/browser/widgets/react-renderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,10 @@ export class ReactRenderer implements Disposable {
}

render(): void {
this.hostRoot.render(<React.Fragment>{this.doRender()}</React.Fragment>);
// Ignore all render calls after the host element has unmounted
if (!this.toDispose.disposed) {
this.hostRoot.render(<React.Fragment>{this.doRender()}</React.Fragment>);
}
}

protected doRender(): React.ReactNode {
Expand Down
40 changes: 19 additions & 21 deletions packages/filesystem/src/browser/filesystem-saveable-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

import { environment, MessageService, nls } from '@theia/core';
import { inject, injectable } from '@theia/core/shared/inversify';
import { Navigatable, Saveable, SaveableSource, SaveOptions, Widget, open, OpenerService, ConfirmDialog, FormatType, CommonCommands } from '@theia/core/lib/browser';
import { Navigatable, Saveable, SaveableSource, SaveOptions, Widget, open, OpenerService, ConfirmDialog, CommonCommands, LabelProvider } from '@theia/core/lib/browser';
import { SaveableService } from '@theia/core/lib/browser/saveable-service';
import URI from '@theia/core/lib/common/uri';
import { FileService } from './file-service';
Expand All @@ -33,6 +33,8 @@ export class FilesystemSaveableService extends SaveableService {
protected readonly fileDialogService: FileDialogService;
@inject(OpenerService)
protected readonly openerService: OpenerService;
@inject(LabelProvider)
protected readonly labelProvider: LabelProvider;

/**
* This method ensures a few things about `widget`:
Expand Down Expand Up @@ -76,7 +78,7 @@ export class FilesystemSaveableService extends SaveableService {
return this.save(sourceWidget, options);
} else if (selected) {
try {
await this.copyAndSave(sourceWidget, selected, overwrite);
await this.saveSnapshot(sourceWidget, selected, overwrite);
return selected;
} catch (e) {
console.warn(e);
Expand All @@ -85,30 +87,26 @@ export class FilesystemSaveableService extends SaveableService {
}

/**
* Saves the current snapshot of the {@link sourceWidget} to the target file
* and replaces the widget with a new one that contains the snapshot content
*
* @param sourceWidget widget to save as `target`.
* @param target The new URI for the widget.
* @param overwrite
*/
private async copyAndSave(sourceWidget: Widget & SaveableSource & Navigatable, target: URI, overwrite: boolean): Promise<void> {
const snapshot = sourceWidget.saveable.createSnapshot!();
if (!await this.fileService.exists(target)) {
const sourceUri = sourceWidget.getResourceUri()!;
if (this.fileService.canHandleResource(sourceUri)) {
await this.fileService.copy(sourceUri, target, { overwrite });
} else {
await this.fileService.createFile(target);
}
}
const targetWidget = await open(this.openerService, target, { widgetOptions: { ref: sourceWidget } });
const targetSaveable = Saveable.get(targetWidget);
if (targetWidget && targetSaveable && targetSaveable.applySnapshot) {
targetSaveable.applySnapshot(snapshot);
await sourceWidget.saveable.revert!();
sourceWidget.close();
Saveable.save(targetWidget, { formatType: FormatType.ON });
protected async saveSnapshot(sourceWidget: Widget & SaveableSource & Navigatable, target: URI, overwrite: boolean): Promise<void> {
const saveable = sourceWidget.saveable;
const snapshot = saveable.createSnapshot!();
const content = Saveable.Snapshot.read(snapshot) ?? '';
if (await this.fileService.exists(target)) {
// Do not fire the `onDidCreate` event as the file already exists.
await this.fileService.write(target, content);
} else {
this.messageService.error(nls.localize('theia/workspace/failApply', 'Could not apply changes to new file'));
// Ensure to actually call `create` as that fires the `onDidCreate` event.
await this.fileService.create(target, content, { overwrite });
}
await saveable.revert!();
await open(this.openerService, target, { widgetOptions: { ref: sourceWidget, mode: 'tab-replace' } });
}

async confirmOverwrite(uri: URI): Promise<boolean> {
Expand All @@ -119,7 +117,7 @@ export class FilesystemSaveableService extends SaveableService {
// Prompt users for confirmation before overwriting.
const confirmed = await new ConfirmDialog({
title: nls.localizeByDefault('Overwrite'),
msg: nls.localizeByDefault('{0} already exists. Are you sure you want to overwrite it?', uri.toString())
msg: nls.localizeByDefault('{0} already exists. Are you sure you want to overwrite it?', this.labelProvider.getName(uri))
}).open();
return !!confirmed;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,9 @@ export class LocationListRenderer extends ReactRenderer {
}

override render(): void {
this.hostRoot.render(this.doRender());
if (!this.toDispose.disposed) {
this.hostRoot.render(this.doRender());
}
}

protected initResolveDirectoryCache(): void {
Expand Down
2 changes: 1 addition & 1 deletion packages/monaco/src/browser/monaco-editor-model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -651,7 +651,7 @@ export class MonacoEditorModel implements IResolvedTextEditorModel, TextEditorDo
}

applySnapshot(snapshot: Saveable.Snapshot): void {
const value = 'value' in snapshot ? snapshot.value : snapshot.read() ?? '';
const value = Saveable.Snapshot.read(snapshot) ?? '';
this.model.setValue(value);
}

Expand Down
2 changes: 1 addition & 1 deletion packages/notebook/src/browser/view-model/notebook-model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ export class NotebookModel implements Saveable, Disposable {
}

async applySnapshot(snapshot: Saveable.Snapshot): Promise<void> {
const rawData = 'read' in snapshot ? snapshot.read() : snapshot.value;
const rawData = Saveable.Snapshot.read(snapshot);
if (!rawData) {
throw new Error('could not read notebook snapshot');
}
Expand Down