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

Fix 'Save As' when saving a document with 'untitled' scheme #10608

Merged
merged 2 commits into from
Mar 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions packages/core/src/browser/common-frontend-contribution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ import { EncodingRegistry } from './encoding-registry';
import { UTF8 } from '../common/encodings';
import { EnvVariablesServer } from '../common/env-variables';
import { AuthenticationService } from './authentication-service';
import { FormatType, Saveable } from './saveable';
import { FormatType, Saveable, SaveOptions } from './saveable';
import { QuickInputService, QuickPick, QuickPickItem } from './quick-input';
import { AsyncLocalizationProvider } from '../common/i18n/localization';
import { nls } from '../common/nls';
Expand All @@ -60,6 +60,7 @@ import { WindowService } from './window/window-service';
import { FrontendApplicationConfigProvider } from './frontend-application-config-provider';
import { DecorationStyle } from './decoration-style';
import { isPinned, Title, togglePinned, Widget } from './widgets';
import { SaveResourceService } from './save-resource-service';

export namespace CommonMenus {

Expand Down Expand Up @@ -338,7 +339,8 @@ export class CommonFrontendContribution implements FrontendApplicationContributi
@inject(MessageService) protected readonly messageService: MessageService,
@inject(OpenerService) protected readonly openerService: OpenerService,
@inject(AboutDialog) protected readonly aboutDialog: AboutDialog,
@inject(AsyncLocalizationProvider) protected readonly localizationProvider: AsyncLocalizationProvider
@inject(AsyncLocalizationProvider) protected readonly localizationProvider: AsyncLocalizationProvider,
@inject(SaveResourceService) protected readonly saveResourceService: SaveResourceService,
) { }

@inject(ContextKeyService)
Expand Down Expand Up @@ -889,10 +891,10 @@ export class CommonFrontendContribution implements FrontendApplicationContributi
});

commandRegistry.registerCommand(CommonCommands.SAVE, {
execute: () => this.shell.save({ formatType: FormatType.ON })
execute: () => this.save({ formatType: FormatType.ON })
});
commandRegistry.registerCommand(CommonCommands.SAVE_WITHOUT_FORMATTING, {
execute: () => this.shell.save({ formatType: FormatType.OFF })
execute: () => this.save({ formatType: FormatType.OFF })
});
commandRegistry.registerCommand(CommonCommands.SAVE_ALL, {
execute: () => this.shell.saveAll({ formatType: FormatType.DIRTY })
Expand Down Expand Up @@ -1058,6 +1060,11 @@ export class CommonFrontendContribution implements FrontendApplicationContributi
);
}

protected async save(options?: SaveOptions): Promise<void> {
const widget = this.shell.currentWidget;
this.saveResourceService.save(widget, options);
}
Comment on lines +1063 to +1066
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: I think it would be nice from an API perspective to replace the existing ApplicationShell.save with this implementation here. Having multiple saving mechanisms in different places will probably lead to some confusion.


protected async openAbout(): Promise<void> {
this.aboutDialog.open();
}
Expand Down
3 changes: 3 additions & 0 deletions packages/core/src/browser/frontend-application-module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ import {
import { RendererHost } from './widgets';
import { TooltipService, TooltipServiceImpl } from './tooltip-service';
import { bindFrontendStopwatch, bindBackendStopwatch } from './performance';
import { SaveResourceService } from './save-resource-service';

export { bindResourceProvider, bindMessageService, bindPreferenceService };

Expand Down Expand Up @@ -395,4 +396,6 @@ export const frontendApplicationModule = new ContainerModule((bind, unbind, isBo

bindFrontendStopwatch(bind);
bindBackendStopwatch(bind);

bind(SaveResourceService).toSelf().inSingletonScope();
});
45 changes: 45 additions & 0 deletions packages/core/src/browser/save-resource-service.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/********************************************************************************
* Copyright (C) 2022 Arm and others.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0 which is available at
* http://www.eclipse.org/legal/epl-2.0.
*
* This Source Code may also be made available under the following Secondary
* Licenses when the conditions for such availability set forth in the Eclipse
* Public License v. 2.0 are satisfied: GNU General Public License, version 2
* with the GNU Classpath Exception which is available at
* https://www.gnu.org/software/classpath/license.html.
*
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/

import { injectable } from 'inversify';
import { Saveable, SaveOptions } from './saveable';
import { Widget } from './widgets';

@injectable()
export class SaveResourceService {

/**
* Indicate if the document can be saved ('Save' command should be disable if not).
*/
canSave(saveable: Saveable): boolean {
// By default, we never allow a document to be saved if it is untitled.
return Saveable.isDirty(saveable) && !Saveable.isUntitled(saveable);
}

/**
* Saves the document.
*
* This function is called only if `canSave` returns true, which means the document is not untitled
* and is thus saveable.
*/
async save(widget: Widget | undefined, options?: SaveOptions): Promise<void> {
const saveable = Saveable.get(widget);
if (saveable && this.canSave(saveable)) {
await saveable.save(options);
}
}

}
5 changes: 5 additions & 0 deletions packages/core/src/browser/saveable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { MaybePromise } from '../common/types';
import { Key } from './keyboard/keys';
import { AbstractDialog } from './dialogs';
import { waitForClosed } from './widgets';
import { URI } from 'vscode-uri';

export interface Saveable {
readonly dirty: boolean;
Expand Down Expand Up @@ -65,6 +66,10 @@ export namespace Saveable {
return !!arg && ('dirty' in arg) && ('onDirtyChanged' in arg);
}
// eslint-disable-next-line @typescript-eslint/no-explicit-any
export function isUntitled(arg: any): boolean {
return !!arg && ('uri' in arg) && URI.parse((arg as { uri: string; }).uri).scheme === 'untitled';
}
// eslint-disable-next-line @typescript-eslint/no-explicit-any
export function get(arg: any): Saveable | undefined {
if (is(arg)) {
return arg;
Expand Down
4 changes: 1 addition & 3 deletions packages/plugin-ext/src/main/browser/documents-main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import { Range } from '@theia/core/shared/vscode-languageserver-protocol';
import { OpenerService } from '@theia/core/lib/browser/opener-service';
import { Reference } from '@theia/core/lib/common/reference';
import { dispose } from '../../common/disposable-util';
import { FileResourceResolver } from '@theia/filesystem/lib/browser';

/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
Expand Down Expand Up @@ -95,7 +94,6 @@ export class DocumentsMainImpl implements DocumentsMain, Disposable {
private openerService: OpenerService,
private shell: ApplicationShell,
private untitledResourceResolver: UntitledResourceResolver,
private fileResourceResolver: FileResourceResolver
) {
this.proxy = rpc.getProxy(MAIN_RPC_CONTEXT.DOCUMENTS_EXT);

Expand Down Expand Up @@ -181,7 +179,7 @@ export class DocumentsMainImpl implements DocumentsMain, Disposable {
async $tryCreateDocument(options?: { language?: string; content?: string; }): Promise<UriComponents> {
const language = options && options.language;
const content = options && options.content;
const resource = await this.untitledResourceResolver.createUntitledResource(this.fileResourceResolver, content, language);
const resource = await this.untitledResourceResolver.createUntitledResource(content, language);
return monaco.Uri.parse(resource.uri.toString());
}

Expand Down
55 changes: 10 additions & 45 deletions packages/plugin-ext/src/main/browser/editor/untitled-resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,21 +15,16 @@
// *****************************************************************************

import { Emitter, Event } from '@theia/core/lib/common/event';
import { injectable, inject } from '@theia/core/shared/inversify';
import { Resource, ResourceResolver, ResourceVersion, ResourceSaveOptions } from '@theia/core/lib/common/resource';
import { injectable } from '@theia/core/shared/inversify';
import { Resource, ResourceResolver, ResourceVersion } from '@theia/core/lib/common/resource';
import URI from '@theia/core/lib/common/uri';
import { Schemes } from '../../../common/uri-components';
import { FileResource, FileResourceResolver } from '@theia/filesystem/lib/browser';
import { TextDocumentContentChangeEvent } from '@theia/core/shared/vscode-languageserver-protocol';

let index = 0;

@injectable()
export class UntitledResourceResolver implements ResourceResolver {

@inject(FileResourceResolver)
protected readonly fileResourceResolver: FileResourceResolver;

protected readonly resources = new Map<string, UntitledResource>();

async resolve(uri: URI): Promise<UntitledResource> {
Expand All @@ -38,14 +33,14 @@ export class UntitledResourceResolver implements ResourceResolver {
} else {
const untitledResource = this.resources.get(uri.toString());
if (!untitledResource) {
return this.createUntitledResource(this.fileResourceResolver, '', '', uri);
return this.createUntitledResource('', '', uri);
} else {
return untitledResource;
}
}
}

async createUntitledResource(fileResourceResolver: FileResourceResolver, content?: string, language?: string, uri?: URI): Promise<UntitledResource> {
async createUntitledResource(content?: string, language?: string, uri?: URI): Promise<UntitledResource> {
let extension;
if (language) {
for (const lang of monaco.languages.getLanguages()) {
Expand All @@ -58,77 +53,47 @@ export class UntitledResourceResolver implements ResourceResolver {
}
}
return new UntitledResource(this.resources, uri ? uri : new URI().withScheme(Schemes.untitled).withPath(`/Untitled-${index++}${extension ? extension : ''}`),
fileResourceResolver, content);
content);
}
}

export class UntitledResource implements Resource {

private fileResource?: FileResource;

protected readonly onDidChangeContentsEmitter = new Emitter<void>();
readonly onDidChangeContents: Event<void> = this.onDidChangeContentsEmitter.event;

constructor(private resources: Map<string, UntitledResource>, public uri: URI, private fileResourceResolver: FileResourceResolver, private content?: string) {
constructor(private resources: Map<string, UntitledResource>, public uri: URI, private content?: string) {
this.resources.set(this.uri.toString(), this);
}

dispose(): void {
this.resources.delete(this.uri.toString());
this.onDidChangeContentsEmitter.dispose();
if (this.fileResource) {
this.fileResource.dispose();
}
}

async readContents(options?: { encoding?: string | undefined; } | undefined): Promise<string> {
if (this.fileResource) {
return this.fileResource.readContents(options);
} else if (this.content) {
if (this.content) {
return this.content;
} else {
return '';
}
}

async saveContents(content: string, options?: { encoding?: string, overwriteEncoding?: boolean }): Promise<void> {
if (!this.fileResource) {
this.fileResource = await this.fileResourceResolver.resolve(new URI(this.uri.path.toString()));
if (this.fileResource.onDidChangeContents) {
this.fileResource.onDidChangeContents(() => this.fireDidChangeContents());
}
}
await this.fileResource.saveContents(content, options);
}

async saveContentChanges(changes: TextDocumentContentChangeEvent[], options?: ResourceSaveOptions): Promise<void> {
if (!this.fileResource || !this.fileResource.saveContentChanges) {
throw new Error('FileResource is not available for: ' + this.uri.path.toString());
}
await this.fileResource.saveContentChanges(changes, options);
}

async guessEncoding(): Promise<string | undefined> {
if (this.fileResource) {
return this.fileResource.guessEncoding();
}
// This function must exist to ensure readOnly is false for the Monaco editor.
// However it should not be called because saving 'untitled' is always processed as 'Save As'.
throw Error('never');
}

protected fireDidChangeContents(): void {
this.onDidChangeContentsEmitter.fire(undefined);
}

get version(): ResourceVersion | undefined {
if (this.fileResource) {
return this.fileResource.version;
}
return undefined;
}

get encoding(): string | undefined {
if (this.fileResource) {
return this.fileResource.encoding;
}
return undefined;
}
}
Expand Down
4 changes: 1 addition & 3 deletions packages/plugin-ext/src/main/browser/main-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ import { ApplicationShell } from '@theia/core/lib/browser/shell/application-shel
import { MonacoBulkEditService } from '@theia/monaco/lib/browser/monaco-bulk-edit-service';
import { MonacoEditorService } from '@theia/monaco/lib/browser/monaco-editor-service';
import { UntitledResourceResolver } from './editor/untitled-resource';
import { FileResourceResolver } from '@theia/filesystem/lib/browser';
import { MainFileSystemEventService } from './main-file-system-event-service';
import { LabelServiceMainImpl } from './label-service-main';
import { TimelineMainImpl } from './timeline-main';
Expand Down Expand Up @@ -87,8 +86,7 @@ export function setUpPluginApi(rpc: RPCProtocol, container: interfaces.Container
const openerService = container.get<OpenerService>(OpenerService);
const shell = container.get(ApplicationShell);
const untitledResourceResolver = container.get(UntitledResourceResolver);
const fileResourceResolver = container.get(FileResourceResolver);
const documentsMain = new DocumentsMainImpl(editorsAndDocuments, modelService, rpc, editorManager, openerService, shell, untitledResourceResolver, fileResourceResolver);
const documentsMain = new DocumentsMainImpl(editorsAndDocuments, modelService, rpc, editorManager, openerService, shell, untitledResourceResolver);
rpc.set(PLUGIN_RPC_CONTEXT.DOCUMENTS_MAIN, documentsMain);

const bulkEditService = container.get(MonacoBulkEditService);
Expand Down
25 changes: 17 additions & 8 deletions packages/workspace/src/browser/workspace-frontend-contribution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { CommandContribution, CommandRegistry, MenuContribution, MenuModelRegist
import { isOSX, environment, OS } from '@theia/core';
import {
open, OpenerService, CommonMenus, StorageService, LabelProvider, ConfirmDialog, KeybindingRegistry, KeybindingContribution,
CommonCommands, FrontendApplicationContribution, ApplicationShell, Saveable, SaveableSource, Widget, Navigatable, SHELL_TABBAR_CONTEXT_COPY, OnWillStopAction
CommonCommands, FrontendApplicationContribution, ApplicationShell, Saveable, SaveableSource, Widget, Navigatable, SHELL_TABBAR_CONTEXT_COPY, OnWillStopAction, FormatType
} from '@theia/core/lib/browser';
import { FileDialogService, OpenFileDialogProps, FileDialogTreeFilters } from '@theia/filesystem/lib/browser';
import { ContextKeyService } from '@theia/core/lib/browser/context-key-service';
Expand Down Expand Up @@ -479,7 +479,7 @@ export class WorkspaceFrontendContribution implements CommandContribution, Keybi
* - `widget.saveable.createSnapshot` is defined.
* - `widget.saveable.revert` is defined.
*/
protected canBeSavedAs(widget: Widget | undefined): widget is Widget & SaveableSource & Navigatable {
canBeSavedAs(widget: Widget | undefined): widget is Widget & SaveableSource & Navigatable {
return widget !== undefined
&& Saveable.isSource(widget)
&& typeof widget.saveable.createSnapshot === 'function'
Expand All @@ -491,12 +491,17 @@ export class WorkspaceFrontendContribution implements CommandContribution, Keybi
/**
* Save `sourceWidget` to a new file picked by the user.
*/
protected async saveAs(sourceWidget: Widget & SaveableSource & Navigatable): Promise<void> {
async saveAs(sourceWidget: Widget & SaveableSource & Navigatable): Promise<void> {
let exist: boolean = false;
let overwrite: boolean = false;
let selected: URI | undefined;
const uri = sourceWidget.getResourceUri()!;
const stat = await this.fileService.resolve(uri);
const uri: URI = sourceWidget.getResourceUri()!;
let stat;
if (uri.scheme === 'file') {
stat = await this.fileService.resolve(uri);
} else {
stat = this.workspaceService.workspace;
}
do {
selected = await this.fileDialogService.showSaveDialog(
{
Expand Down Expand Up @@ -530,16 +535,20 @@ export class WorkspaceFrontendContribution implements CommandContribution, Keybi
private async copyAndSave(sourceWidget: Widget & SaveableSource & Navigatable, target: URI, overwrite: boolean): Promise<void> {
const snapshot = sourceWidget.saveable.createSnapshot!();
if (!await this.fileService.exists(target)) {
await this.fileService.copy(sourceWidget.getResourceUri()!, target, { overwrite });
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);
const targetSaveable = Saveable.get(targetWidget);
if (targetWidget && targetSaveable && targetSaveable.applySnapshot) {
targetSaveable.applySnapshot(snapshot);
await sourceWidget.saveable.revert!();
sourceWidget.close();
// At this point `targetWidget` should be `applicationShell.currentWidget` for the save command to pick up:
await this.commandRegistry.executeCommand(CommonCommands.SAVE.id);
Saveable.save(targetWidget, { formatType: FormatType.ON });
} else {
this.messageService.error(nls.localize('theia/workspace/failApply', 'Could not apply changes to new file'));
}
Expand Down
4 changes: 4 additions & 0 deletions packages/workspace/src/browser/workspace-frontend-module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ import { WorkspaceBreadcrumbsContribution } from './workspace-breadcrumbs-contri
import { FilepathBreadcrumbsContribution } from '@theia/filesystem/lib/browser/breadcrumbs/filepath-breadcrumbs-contribution';
import { WorkspaceTrustService } from './workspace-trust-service';
import { bindWorkspaceTrustPreferences } from './workspace-trust-preferences';
import { SaveResourceService } from '@theia/core/lib/browser/save-resource-service';
import { WorkspaceSaveResourceService } from './workspace-save-resource-service';

export default new ContainerModule((bind: interfaces.Bind, unbind: interfaces.Unbind, isBound: interfaces.IsBound, rebind: interfaces.Rebind) => {
bindWorkspacePreferences(bind);
Expand Down Expand Up @@ -105,4 +107,6 @@ export default new ContainerModule((bind: interfaces.Bind, unbind: interfaces.Un
rebind(FilepathBreadcrumbsContribution).to(WorkspaceBreadcrumbsContribution).inSingletonScope();

bind(WorkspaceTrustService).toSelf().inSingletonScope();

rebind(SaveResourceService).to(WorkspaceSaveResourceService).inSingletonScope();
});
Loading