Skip to content

Commit

Permalink
[preferences] fix #6845: use text models to update content
Browse files Browse the repository at this point in the history 
It resolve following issues:
- dirty editors are respected
- edits are applied in thread safe fashion

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
  • Loading branch information
@akosyakov
akosyakov committed Feb 9, 2020
1 parent 4aca394 commit 50e32d3
Show file tree
Hide file tree
Showing 7 changed files with 226 additions and 202 deletions.
198 changes: 76 additions & 122 deletions ...er/preferences/launch-preferences.spec.ts → .../api-tests/src/launch-preferences.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,50 +16,31 @@

/* eslint-disable no-unused-expressions, @typescript-eslint/no-explicit-any */

import { enableJSDOM } from '@theia/core/lib/browser/test/jsdom';
const disableJSDOM = enableJSDOM();

import * as path from 'path';
import * as fs from 'fs-extra';
import * as assert from 'assert';
import { Container } from 'inversify';
import { FileUri } from '@theia/core/lib/node/file-uri';
import { DisposableCollection, Disposable } from '@theia/core/lib/common/disposable';
import { PreferenceService, PreferenceServiceImpl, PreferenceScope } from '@theia/core/lib/browser/preferences/preference-service';
import { bindPreferenceService, bindMessageService, bindResourceProvider } from '@theia/core/lib/browser/frontend-application-bindings';
import { bindFileSystem } from '@theia/filesystem/lib/node/filesystem-backend-module';
import { bindFileResource } from '@theia/filesystem/lib/browser/filesystem-frontend-module';
import { FrontendApplicationConfigProvider } from '@theia/core/lib/browser/frontend-application-config-provider';
import { WorkspaceService } from '@theia/workspace/lib/browser/workspace-service';
import { FileSystemWatcher } from '@theia/filesystem/lib/browser/filesystem-watcher';
import { bindFileSystemPreferences } from '@theia/filesystem/lib/browser/filesystem-preferences';
import { FileShouldOverwrite } from '@theia/filesystem/lib/common/filesystem';
import { bindLogger } from '@theia/core/lib/node/logger-backend-module';
import { bindWorkspacePreferences } from '@theia/workspace/lib/browser';
import { WindowService } from '@theia/core/lib/browser/window/window-service';
import { MockWindowService } from '@theia/core/lib/browser/window/test/mock-window-service';
import { MockWorkspaceServer } from '@theia/workspace/lib/common/test/mock-workspace-server';
import { WorkspaceServer } from '@theia/workspace/lib/common/workspace-protocol';
import { bindPreferenceProviders } from '@theia/preferences/lib/browser/preference-bindings';
import { bindUserStorage } from '@theia/userstorage/lib/browser/user-storage-frontend-module';
import { FileSystemWatcherServer } from '@theia/filesystem/lib/common/filesystem-watcher-protocol';
import { MockFilesystemWatcherServer } from '@theia/filesystem/lib/common/test/mock-filesystem-watcher-server';
import { bindLaunchPreferences } from './launch-preferences';

disableJSDOM();

process.on('unhandledRejection', (reason, promise) => {
console.error(reason);
throw reason;
});
/**
* @typedef {'.vscode' | '.theia' | ['.theia', '.vscode']} ConfigMode
*/

/**
* Expectations should be tested and aligned against VS Code.
* See https://github.com/akosyakov/vscode-launch/blob/master/src/test/extension.test.ts
*/
describe('Launch Preferences', () => {
describe('Launch Preferences', function () {

const { assert } = chai;

type ConfigMode = '.vscode' | '.theia' | ['.theia', '.vscode'];
import { DisposableCollection, Disposable } from '@theia/core/lib/common/disposable';
import { PreferenceService, PreferenceScope } from '@theia/core/lib/browser/preferences/preference-service';

const Uri = require('@theia/core/lib/common/uri');
const { WorkspaceService } = require('@theia/workspace/lib/browser/workspace-service');
const { FileSystem } = require('@theia/filesystem/lib/common/filesystem');

/** @type {import('inversify').Container} */
const container = window['theia'].container;
const preferences = container.get(PreferenceService);
const workspaceService = container.get(WorkspaceService);
/** @type {import('@theia/filesystem/lib/common/filesystem').FileSystem} */
const fileSystem = container.get(FileSystem);

const defaultLaunch = {
'configurations': [],
Expand Down Expand Up @@ -107,7 +88,8 @@ describe('Launch Preferences', () => {

testSuite({
name: 'No Preferences',
expectation: defaultLaunch
expectation: defaultLaunch,
only: true
});

testLaunchAndSettingsSuite({
Expand Down Expand Up @@ -329,15 +311,20 @@ describe('Launch Preferences', () => {
}
});

/**
* @typedef {Object} LaunchAndSettingsSuiteOptions
* @property {string} name
* @property {any} expectation
* @property {any} [launch]
* @property {boolean} [only]
* @property {ConfigMode} [configMode]
*/
/**
* @type {(options: LaunchAndSettingsSuiteOptions) => void}
*/
function testLaunchAndSettingsSuite({
name, expectation, launch, only, configMode
}: {
name: string,
expectation: any,
launch?: any,
only?: boolean,
configMode?: ConfigMode
}): void {
}) {
testSuite({
name: name + ' Launch Configuration',
launch,
Expand All @@ -356,20 +343,24 @@ describe('Launch Preferences', () => {
});
}

function testSuite(options: {
name: string,
expectation: any,
inspectExpectation?: any,
launch?: any,
settings?: any,
only?: boolean,
configMode?: ConfigMode
}): void {

/**
* @typedef {Object} SuiteOptions
* @property {string} name
* @property {any} expectation
* @property {any} [inspectExpectation]
* @property {any} [launch]
* @property {any} [settings]
* @property {boolean} [only]
* @property {ConfigMode} [configMode]
*/
/**
* @type {(options: SuiteOptions) => void}
*/
function testSuite(options) {
describe(options.name, () => {

if (options.configMode) {
testConfigSuite(options as any);
testConfigSuite(options);
} else {

testConfigSuite({
Expand All @@ -394,87 +385,50 @@ describe('Launch Preferences', () => {

}

/**
* @typedef {Object} ConfigSuiteOptions
* @property {any} expectation
* @property {any} [inspectExpectation]
* @property {any} [launch]
* @property {any} [settings]
* @property {boolean} [only]
* @property {ConfigMode} [configMode]
*/
/**
* @type {(options: ConfigSuiteOptions) => void}
*/
function testConfigSuite({
configMode, expectation, inspectExpectation, settings, launch, only
}: {
configMode: ConfigMode
expectation: any,
inspectExpectation?: any,
launch?: any,
settings?: any,
only?: boolean
}): void {
}) {

describe(JSON.stringify(configMode, undefined, 2), () => {

const configPaths = Array.isArray(configMode) ? configMode : [configMode];

const rootPath = path.resolve(__dirname, '..', '..', '..', 'launch-preference-test-temp');
const rootUri = FileUri.create(rootPath).toString();

let preferences: PreferenceService;

const toTearDown = new DisposableCollection();
beforeEach(async function (): Promise<void> {
toTearDown.push(Disposable.create(enableJSDOM()));
FrontendApplicationConfigProvider.set({
'applicationName': 'test',
});

fs.removeSync(rootPath);
fs.ensureDirSync(rootPath);
toTearDown.push(Disposable.create(() => fs.removeSync(rootPath)));
beforeEach(async () => {
const rootUri = new Uri.default(workspaceService.tryGetRoots()[0].uri);

if (settings) {
for (const configPath of configPaths) {
const settingsPath = path.resolve(rootPath, configPath, 'settings.json');
fs.ensureFileSync(settingsPath);
fs.writeFileSync(settingsPath, JSON.stringify(settings), 'utf-8');
const uri = rootUri.resolve(configPath + '/settings.json');
await fileSystem.createFile(rootUri.resolve(configPath + '/settings.json'), {
content: JSON.stringify(settings),
encoding: 'utf-8'
});
this.toTearDown.push(Disposable.create(() => fileSystem.delete(uri)));
}
}
if (launch) {
for (const configPath of configPaths) {
const launchPath = path.resolve(rootPath, configPath, 'launch.json');
fs.ensureFileSync(launchPath);
fs.writeFileSync(launchPath, JSON.stringify(launch), 'utf-8');
const uri = rootUri.resolve(configPath + '/launch.json');
await fileSystem.createFile(rootUri.resolve(configPath + '/settings.json'), {
content: JSON.stringify(launch),
encoding: 'utf-8'
});
this.toTearDown.push(Disposable.create(() => fileSystem.delete(uri)));
}
}

const container = new Container();
const bind = container.bind.bind(container);
const unbind = container.unbind.bind(container);
bindLogger(bind);
bindMessageService(bind);
bindResourceProvider(bind);
bindFileResource(bind);
bindUserStorage(bind);
bindPreferenceService(bind);
bindFileSystem(bind);
bind(FileSystemWatcherServer).toConstantValue(new MockFilesystemWatcherServer());
bindFileSystemPreferences(bind);
container.bind(FileShouldOverwrite).toConstantValue(async () => true);
bind(FileSystemWatcher).toSelf().inSingletonScope();
bindPreferenceProviders(bind, unbind);
bindWorkspacePreferences(bind);
container.bind(WorkspaceService).toSelf().inSingletonScope();
container.bind(WindowService).toConstantValue(new MockWindowService());

const workspaceServer = new MockWorkspaceServer();
workspaceServer['getMostRecentlyUsedWorkspace'] = async () => rootUri;
container.bind(WorkspaceServer).toConstantValue(workspaceServer);

bindLaunchPreferences(bind);

toTearDown.push(container.get(FileSystemWatcher));

const impl = container.get(PreferenceServiceImpl);
toTearDown.push(impl);

preferences = impl;
toTearDown.push(Disposable.create(() => preferences = undefined!));

await preferences.ready;
await container.get(WorkspaceService).roots;
});

afterEach(() => toTearDown.dispose());
Expand Down Expand Up @@ -587,7 +541,7 @@ describe('Launch Preferences', () => {
await preferences.set('launch.configurations', [validConfiguration, validConfiguration2]);

const inspect = preferences.inspect('launch');
const actual = inspect && inspect.workspaceValue && (<any>inspect.workspaceValue).configurations;
const actual = inspect && inspect.workspaceValue && inspect.workspaceValue.configurations;
assert.deepStrictEqual(actual, [validConfiguration, validConfiguration2]);
});
}
Expand Down
2 changes: 1 addition & 1 deletion packages/monaco/src/browser/monaco-editor-model.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export class MonacoEditorModel implements ITextEditorModel, TextEditorDocument {
this.toDispose.push(this.onDidSaveModelEmitter);
this.toDispose.push(this.onWillSaveModelEmitter);
this.toDispose.push(this.onDirtyChangedEmitter);
this.resolveModel = resource.readContents(options).then(content => this.initialize(content));
this.resolveModel = resource.readContents(options).then(content => this.initialize(content), () => this.initialize(''));
this.defaultEncoding = options && options.encoding ? options.encoding : undefined;
}

Expand Down
31 changes: 31 additions & 0 deletions packages/monaco/src/browser/monaco-workspace.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import { WillSaveMonacoModelEvent, MonacoEditorModel, MonacoModelContentChangedE
import { MonacoEditor } from './monaco-editor';
import { MonacoConfigurations } from './monaco-configurations';
import { ProblemManager } from '@theia/markers/lib/browser';
import { MaybePromise } from '@theia/core/lib/common/types';

export interface MonacoDidChangeTextDocumentParams extends lang.DidChangeTextDocumentParams {
readonly textDocument: MonacoEditorModel;
Expand Down Expand Up @@ -276,7 +277,12 @@ export class MonacoWorkspace implements lang.Workspace {
this.onDidSaveTextDocumentEmitter.fire(model);
}

protected suppressedOpenIfDirty: MonacoEditorModel[] = [];

protected openEditorIfDirty(model: MonacoEditorModel): void {
if (this.suppressedOpenIfDirty.indexOf(model) !== -1) {
return;
}
if (model.dirty && MonacoEditor.findByDocument(this.editorManager, model).length === 0) {
// create a new reference to make sure the model is not disposed before it is
// acquired by the editor, thus losing the changes that made it dirty.
Expand All @@ -288,6 +294,18 @@ export class MonacoWorkspace implements lang.Workspace {
}
}

protected async suppressOpenIfDirty(model: MonacoEditorModel, cb: () => MaybePromise<void>): Promise<void> {
this.suppressedOpenIfDirty.push(model);
try {
await cb();
} finally {
const i = this.suppressedOpenIfDirty.indexOf(model);
if (i !== -1) {
this.suppressedOpenIfDirty.splice(i, 1);
}
}
}

createFileSystemWatcher(globPattern: string, ignoreCreateEvents?: boolean, ignoreChangeEvents?: boolean, ignoreDeleteEvents?: boolean): lang.FileSystemWatcher {
const disposables = new DisposableCollection();
const onDidCreateEmitter = new lang.Emitter<Uri>();
Expand Down Expand Up @@ -331,6 +349,19 @@ export class MonacoWorkspace implements lang.Workspace {
};
}

applyBackgroundEdit(model: MonacoEditorModel, editOperations: monaco.editor.IIdentifiedSingleEditOperation[]): Promise<void> {
return this.suppressOpenIfDirty(model, async () => {
const editor = MonacoEditor.findByDocument(this.editorManager, model)[0];
const cursorState = editor && editor.getControl().getSelections() || [];
model.textEditorModel.pushStackElement();
model.textEditorModel.pushEditOperations(cursorState, editOperations, () => cursorState);
model.textEditorModel.pushStackElement();
if (!editor) {
await model.save();
}
});
}

async applyEdit(changes: lang.WorkspaceEdit, options?: EditorOpenerOptions): Promise<boolean> {
const workspaceEdit = this.p2m.asWorkspaceEdit(changes);
await this.applyBulkEdit(workspaceEdit, options);
Expand Down
2 changes: 1 addition & 1 deletion packages/plugin-ext/src/common/plugin-api-rpc.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -810,7 +810,7 @@ export interface TextEditorsExt {
}

export interface SingleEditOperation {
range: Range;
range?: Range;
text?: string;
forceMoveMarkers?: boolean;
}
Expand Down
23 changes: 17 additions & 6 deletions packages/plugin-ext/src/main/browser/documents-main.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,13 +108,24 @@ export class DocumentsMainImpl implements DocumentsMain, Disposable {
onWillSaveModelEvent.waitUntil(new Promise<monaco.editor.IIdentifiedSingleEditOperation[]>(async (resolve, reject) => {
setTimeout(() => reject(new Error(`Aborted onWillSaveTextDocument-event after ${this.saveTimeout}ms`)), this.saveTimeout);
const edits = await this.proxy.$acceptModelWillSave(onWillSaveModelEvent.model.textEditorModel.uri, onWillSaveModelEvent.reason, this.saveTimeout);
const transformedEdits = edits.map((edit): monaco.editor.IIdentifiedSingleEditOperation =>
({
range: monaco.Range.lift(edit.range),
text: edit.text!,
const editOperations: monaco.editor.IIdentifiedSingleEditOperation[] = [];
for (const edit of edits) {
const { range, text } = edit;
if (!range && !text) {
continue;
}
if (range && range.startLineNumber === range.endLineNumber && range.startColumn === range.endColumn && !edit.text) {
continue;
}

editOperations.push({
range: range ? monaco.Range.lift(range) : onWillSaveModelEvent.model.textEditorModel.getFullModelRange(),
/* eslint-disable-next-line no-null/no-null */
text: text || null,
forceMoveMarkers: edit.forceMoveMarkers
}));
resolve(transformedEdits);
});
}
resolve(editOperations);
}));
}));
this.toDispose.push(modelService.onModelDirtyChanged(m => {
Expand Down
Loading

0 comments on commit 50e32d3

Please sign in to comment.