From 76a069d3e002f68c7c6f319366a87f2b60684472 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Mon, 29 Apr 2019 17:09:41 +0200 Subject: [PATCH] Fix file system providers from extensions to resolve save conflicts (#72954) * fix #72924 * add test for FileOnDiskContentProvider * make test assert more --- .../contrib/files/browser/saveErrorHandler.ts | 4 +- .../workbench/contrib/files/common/files.ts | 17 +++++--- .../test/common/fileOnDiskProvider.test.ts | 40 +++++++++++++++++++ .../workbench/test/workbenchTestServices.ts | 9 +++++ 4 files changed, 62 insertions(+), 8 deletions(-) create mode 100644 src/vs/workbench/contrib/files/test/common/fileOnDiskProvider.test.ts diff --git a/src/vs/workbench/contrib/files/browser/saveErrorHandler.ts b/src/vs/workbench/contrib/files/browser/saveErrorHandler.ts index 5b7340c997d..0e8c29a2706 100644 --- a/src/vs/workbench/contrib/files/browser/saveErrorHandler.ts +++ b/src/vs/workbench/contrib/files/browser/saveErrorHandler.ts @@ -19,7 +19,7 @@ import { ResourceMap } from 'vs/base/common/map'; import { DiffEditorInput } from 'vs/workbench/common/editor/diffEditorInput'; import { ResourceEditorInput } from 'vs/workbench/common/editor/resourceEditorInput'; import { IContextKeyService, IContextKey, RawContextKey } from 'vs/platform/contextkey/common/contextkey'; -import { FileOnDiskContentProvider } from 'vs/workbench/contrib/files/common/files'; +import { FileOnDiskContentProvider, resourceToFileOnDisk } from 'vs/workbench/contrib/files/common/files'; import { FileEditorInput } from 'vs/workbench/contrib/files/common/editors/fileEditorInput'; import { IModelService } from 'vs/editor/common/services/modelService'; import { SAVE_FILE_COMMAND_ID, REVERT_FILE_COMMAND_ID, SAVE_FILE_AS_COMMAND_ID, SAVE_FILE_AS_LABEL } from 'vs/workbench/contrib/files/browser/fileCommands'; @@ -248,7 +248,7 @@ class ResolveSaveConflictAction extends Action { return this.editorService.openEditor( { - leftResource: resource.with({ scheme: CONFLICT_RESOLUTION_SCHEME }), + leftResource: resourceToFileOnDisk(CONFLICT_RESOLUTION_SCHEME, resource), rightResource: resource, label: editorLabel, options: { pinned: true } diff --git a/src/vs/workbench/contrib/files/common/files.ts b/src/vs/workbench/contrib/files/common/files.ts index 56ce9d0b989..f0b2a14c926 100644 --- a/src/vs/workbench/contrib/files/common/files.ts +++ b/src/vs/workbench/contrib/files/common/files.ts @@ -23,8 +23,6 @@ import { createDecorator } from 'vs/platform/instantiation/common/instantiation' import { IEditorGroup } from 'vs/workbench/services/editor/common/editorGroupsService'; import { ExplorerItem } from 'vs/workbench/contrib/files/common/explorerModel'; import { once } from 'vs/base/common/functional'; -import { IWorkbenchEnvironmentService } from 'vs/workbench/services/environment/common/environmentService'; -import { toLocalResource } from 'vs/base/common/resources'; /** * Explorer viewlet id. @@ -133,6 +131,14 @@ export const SortOrderConfiguration = { export type SortOrder = 'default' | 'mixed' | 'filesFirst' | 'type' | 'modified'; +export function resourceToFileOnDisk(scheme: string, resource: URI): URI { + return resource.with({ scheme, query: JSON.stringify({ scheme: resource.scheme }) }); +} + +export function fileOnDiskToResource(resource: URI): URI { + return resource.with({ scheme: JSON.parse(resource.query)['scheme'], query: null }); +} + export class FileOnDiskContentProvider implements ITextModelContentProvider { private fileWatcherDisposable: IDisposable | undefined; @@ -140,13 +146,12 @@ export class FileOnDiskContentProvider implements ITextModelContentProvider { @ITextFileService private readonly textFileService: ITextFileService, @IFileService private readonly fileService: IFileService, @IModeService private readonly modeService: IModeService, - @IModelService private readonly modelService: IModelService, - @IWorkbenchEnvironmentService private readonly environmentService: IWorkbenchEnvironmentService + @IModelService private readonly modelService: IModelService ) { } provideTextContent(resource: URI): Promise { - const savedFileResource = toLocalResource(resource, this.environmentService.configuration.remoteAuthority); + const savedFileResource = fileOnDiskToResource(resource); // Make sure our file from disk is resolved up to date return this.resolveEditorModel(resource).then(codeEditorModel => { @@ -174,7 +179,7 @@ export class FileOnDiskContentProvider implements ITextModelContentProvider { private resolveEditorModel(resource: URI, createAsNeeded?: true): Promise; private resolveEditorModel(resource: URI, createAsNeeded?: boolean): Promise; private resolveEditorModel(resource: URI, createAsNeeded: boolean = true): Promise { - const savedFileResource = toLocalResource(resource, this.environmentService.configuration.remoteAuthority); + const savedFileResource = fileOnDiskToResource(resource); return this.textFileService.readStream(savedFileResource).then(content => { let codeEditorModel = this.modelService.getModel(resource); diff --git a/src/vs/workbench/contrib/files/test/common/fileOnDiskProvider.test.ts b/src/vs/workbench/contrib/files/test/common/fileOnDiskProvider.test.ts new file mode 100644 index 00000000000..62fcf91ca42 --- /dev/null +++ b/src/vs/workbench/contrib/files/test/common/fileOnDiskProvider.test.ts @@ -0,0 +1,40 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as assert from 'assert'; +import { URI } from 'vs/base/common/uri'; +import { workbenchInstantiationService, TestFileService } from 'vs/workbench/test/workbenchTestServices'; +import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; +import { FileOnDiskContentProvider, resourceToFileOnDisk } from 'vs/workbench/contrib/files/common/files'; +import { snapshotToString } from 'vs/workbench/services/textfile/common/textfiles'; +import { IFileService } from 'vs/platform/files/common/files'; + +class ServiceAccessor { + constructor( + @IFileService public fileService: TestFileService + ) { + } +} + +suite('Files - FileOnDiskContentProvider', () => { + + let instantiationService: IInstantiationService; + let accessor: ServiceAccessor; + + setup(() => { + instantiationService = workbenchInstantiationService(); + accessor = instantiationService.createInstance(ServiceAccessor); + }); + + test('provideTextContent', async () => { + const provider = instantiationService.createInstance(FileOnDiskContentProvider); + const uri = URI.parse('testFileOnDiskContentProvider://foo'); + + const content = await provider.provideTextContent(resourceToFileOnDisk('conflictResolution', uri)); + + assert.equal(snapshotToString(content.createSnapshot()), 'Hello Html'); + assert.equal(accessor.fileService.getLastReadFileUri().toString(), uri.toString()); + }); +}); diff --git a/src/vs/workbench/test/workbenchTestServices.ts b/src/vs/workbench/test/workbenchTestServices.ts index 9cd91b50fa6..1e00736484c 100644 --- a/src/vs/workbench/test/workbenchTestServices.ts +++ b/src/vs/workbench/test/workbenchTestServices.ts @@ -899,6 +899,7 @@ export class TestFileService implements IFileService { readonly onError: Event = Event.None; private content = 'Hello Html'; + private lastReadFileUri: URI; constructor() { this._onFileChanges = new Emitter(); @@ -913,6 +914,10 @@ export class TestFileService implements IFileService { return this.content; } + public getLastReadFileUri(): URI { + return this.lastReadFileUri; + } + public get onFileChanges(): Event { return this._onFileChanges.event; } @@ -952,6 +957,8 @@ export class TestFileService implements IFileService { } readFile(resource: URI, options?: IReadFileOptions | undefined): Promise { + this.lastReadFileUri = resource; + return Promise.resolve({ resource: resource, value: VSBuffer.fromString(this.content), @@ -964,6 +971,8 @@ export class TestFileService implements IFileService { } readFileStream(resource: URI, options?: IReadFileOptions | undefined): Promise { + this.lastReadFileUri = resource; + return Promise.resolve({ resource: resource, value: { -- GitLab