From 19291ef2c1dc7fc20d521f32b8dbe279de413a0c Mon Sep 17 00:00:00 2001 From: Alex Dima Date: Wed, 7 Dec 2016 14:14:50 +0100 Subject: [PATCH] Fixes #16573: Ensure textEditor.options always contains all properties --- .../vscode-api-tests/src/editor.test.ts | 30 +- src/vs/workbench/api/node/extHostEditors.ts | 201 ++++++++++- src/vs/workbench/api/node/extHostTypes.ts | 5 - .../api/node/mainThreadEditorsTracker.ts | 27 +- .../test/node/api/extHostEditors.test.ts | 317 ++++++++++++++++++ 5 files changed, 544 insertions(+), 36 deletions(-) create mode 100644 src/vs/workbench/test/node/api/extHostEditors.test.ts diff --git a/extensions/vscode-api-tests/src/editor.test.ts b/extensions/vscode-api-tests/src/editor.test.ts index d2788368847..81d9565520a 100644 --- a/extensions/vscode-api-tests/src/editor.test.ts +++ b/extensions/vscode-api-tests/src/editor.test.ts @@ -6,7 +6,7 @@ 'use strict'; import * as assert from 'assert'; -import { workspace, window, Position, Range, commands, TextEditor, TextDocument } from 'vscode'; +import { workspace, window, Position, Range, commands, TextEditor, TextDocument, TextEditorCursorStyle, TextEditorLineNumbersStyle } from 'vscode'; import { createRandomFile, deleteFile, cleanUp } from './utils'; suite('editor tests', () => { @@ -98,4 +98,32 @@ suite('editor tests', () => { }); }); }); + + test('issue #16573: Extension API: insertSpaces and tabSize are undefined', () => { + return withRandomFileEditor('Hello world!\n\tHello world!', (editor, doc) => { + + assert.equal(editor.options.tabSize, 4); + assert.equal(editor.options.insertSpaces, false); + assert.equal(editor.options.cursorStyle, TextEditorCursorStyle.Line); + assert.equal(editor.options.lineNumbers, TextEditorLineNumbersStyle.On); + + editor.options = { + tabSize: 2 + }; + + assert.equal(editor.options.tabSize, 2); + assert.equal(editor.options.insertSpaces, false); + assert.equal(editor.options.cursorStyle, TextEditorCursorStyle.Line); + assert.equal(editor.options.lineNumbers, TextEditorLineNumbersStyle.On); + + editor.options.tabSize = 'invalid'; + + assert.equal(editor.options.tabSize, 2); + assert.equal(editor.options.insertSpaces, false); + assert.equal(editor.options.cursorStyle, TextEditorCursorStyle.Line); + assert.equal(editor.options.lineNumbers, TextEditorLineNumbersStyle.On); + + return Promise.resolve(); + }); + }); }); diff --git a/src/vs/workbench/api/node/extHostEditors.ts b/src/vs/workbench/api/node/extHostEditors.ts index 39e3c332ede..7556f5a818a 100644 --- a/src/vs/workbench/api/node/extHostEditors.ts +++ b/src/vs/workbench/api/node/extHostEditors.ts @@ -12,9 +12,9 @@ import Event, { Emitter } from 'vs/base/common/event'; import { TPromise } from 'vs/base/common/winjs.base'; import { IThreadService } from 'vs/workbench/services/thread/common/threadService'; import { ExtHostDocuments, ExtHostDocumentData } from 'vs/workbench/api/node/extHostDocuments'; -import { Selection, Range, Position, EditorOptions, EndOfLine, TextEditorRevealType, TextEditorSelectionChangeKind } from './extHostTypes'; -import { ISingleEditOperation } from 'vs/editor/common/editorCommon'; -import { IResolvedTextEditorConfiguration, ISelectionChangeEvent } from 'vs/workbench/api/node/mainThreadEditorsTracker'; +import { Selection, Range, Position, EndOfLine, TextEditorRevealType, TextEditorSelectionChangeKind, TextEditorLineNumbersStyle } from './extHostTypes'; +import { ISingleEditOperation, TextEditorCursorStyle } from 'vs/editor/common/editorCommon'; +import { IResolvedTextEditorConfiguration, ISelectionChangeEvent, ITextEditorConfigurationUpdate } from 'vs/workbench/api/node/mainThreadEditorsTracker'; import * as TypeConverters from './extHostTypeConverters'; import { MainContext, MainThreadEditorsShape, ExtHostEditorsShape, ITextEditorAddData, ITextEditorPositionData } from './extHost.protocol'; import * as vscode from 'vscode'; @@ -293,6 +293,180 @@ function deprecated(name: string, message: string = 'Refer to the documentation }; } +export class ExtHostTextEditorOptions implements vscode.TextEditorOptions { + + private _proxy: MainThreadEditorsShape; + private _id: string; + + private _tabSize: number; + private _insertSpaces: boolean; + private _cursorStyle: TextEditorCursorStyle; + private _lineNumbers: TextEditorLineNumbersStyle; + + constructor(proxy: MainThreadEditorsShape, id: string, source: IResolvedTextEditorConfiguration) { + this._proxy = proxy; + this._id = id; + this._accept(source); + } + + public _accept(source: IResolvedTextEditorConfiguration): void { + this._tabSize = source.tabSize; + this._insertSpaces = source.insertSpaces; + this._cursorStyle = source.cursorStyle; + this._lineNumbers = source.lineNumbers; + } + + public get tabSize(): number | string { + return this._tabSize; + } + + private _validateTabSize(value: number | string): number | 'auto' | null { + if (value === 'auto') { + return 'auto'; + } + if (typeof value === 'number') { + let r = Math.floor(value); + return (r > 0 ? r : null); + } + if (typeof value === 'string') { + let r = parseInt(value, 10); + if (isNaN(r)) { + return null; + } + return (r > 0 ? r : null); + } + return null; + } + + public set tabSize(value: number | string) { + let tabSize = this._validateTabSize(value); + if (tabSize === null) { + // ignore invalid call + return; + } + if (typeof tabSize === 'number') { + if (this._tabSize === tabSize) { + // nothing to do + return; + } + // reflect the new tabSize value immediately + this._tabSize = tabSize; + } + warnOnError(this._proxy.$trySetOptions(this._id, { + tabSize: tabSize + })); + } + + public get insertSpaces(): boolean | string { + return this._insertSpaces; + } + + private _validateInsertSpaces(value: boolean | string): boolean | 'auto' { + if (value === 'auto') { + return 'auto'; + } + return (value === 'false' ? false : Boolean(value)); + } + + public set insertSpaces(value: boolean | string) { + let insertSpaces = this._validateInsertSpaces(value); + if (typeof insertSpaces === 'boolean') { + if (this._insertSpaces === insertSpaces) { + // nothing to do + return; + } + // reflect the new insertSpaces value immediately + this._insertSpaces = insertSpaces; + } + warnOnError(this._proxy.$trySetOptions(this._id, { + insertSpaces: insertSpaces + })); + } + + public get cursorStyle(): TextEditorCursorStyle { + return this._cursorStyle; + } + + public set cursorStyle(value: TextEditorCursorStyle) { + if (this._cursorStyle === value) { + // nothing to do + return; + } + this._cursorStyle = value; + warnOnError(this._proxy.$trySetOptions(this._id, { + cursorStyle: value + })); + } + + public get lineNumbers(): TextEditorLineNumbersStyle { + return this._lineNumbers; + } + + public set lineNumbers(value: TextEditorLineNumbersStyle) { + if (this._lineNumbers === value) { + // nothing to do + return; + } + this._lineNumbers = value; + warnOnError(this._proxy.$trySetOptions(this._id, { + lineNumbers: value + })); + } + + public assign(newOptions: vscode.TextEditorOptions) { + let bulkConfigurationUpdate: ITextEditorConfigurationUpdate = {}; + let hasUpdate = false; + + if (typeof newOptions.tabSize !== 'undefined') { + let tabSize = this._validateTabSize(newOptions.tabSize); + if (tabSize === 'auto') { + hasUpdate = true; + bulkConfigurationUpdate.tabSize = tabSize; + } else if (typeof tabSize === 'number' && this._tabSize !== tabSize) { + // reflect the new tabSize value immediately + this._tabSize = tabSize; + hasUpdate = true; + bulkConfigurationUpdate.tabSize = tabSize; + } + } + + if (typeof newOptions.insertSpaces !== 'undefined') { + let insertSpaces = this._validateInsertSpaces(newOptions.insertSpaces); + if (insertSpaces === 'auto') { + hasUpdate = true; + bulkConfigurationUpdate.insertSpaces = insertSpaces; + } else if (this._insertSpaces !== insertSpaces) { + // reflect the new insertSpaces value immediately + this._insertSpaces = insertSpaces; + hasUpdate = true; + bulkConfigurationUpdate.insertSpaces = insertSpaces; + } + } + + if (typeof newOptions.cursorStyle !== 'undefined') { + if (this._cursorStyle !== newOptions.cursorStyle) { + this._cursorStyle = newOptions.cursorStyle; + hasUpdate = true; + bulkConfigurationUpdate.cursorStyle = newOptions.cursorStyle; + } + } + + if (typeof newOptions.lineNumbers !== 'undefined') { + if (this._lineNumbers !== newOptions.lineNumbers) { + this._lineNumbers = newOptions.lineNumbers; + hasUpdate = true; + bulkConfigurationUpdate.lineNumbers = newOptions.lineNumbers; + } + } + + if (hasUpdate) { + warnOnError(this._proxy.$trySetOptions(this._id, bulkConfigurationUpdate)); + } + } + + +} + class ExtHostTextEditor implements vscode.TextEditor { private _proxy: MainThreadEditorsShape; @@ -300,15 +474,15 @@ class ExtHostTextEditor implements vscode.TextEditor { private _documentData: ExtHostDocumentData; private _selections: Selection[]; - private _options: vscode.TextEditorOptions; + private _options: ExtHostTextEditorOptions; private _viewColumn: vscode.ViewColumn; - constructor(proxy: MainThreadEditorsShape, id: string, document: ExtHostDocumentData, selections: Selection[], options: EditorOptions, viewColumn: vscode.ViewColumn) { + constructor(proxy: MainThreadEditorsShape, id: string, document: ExtHostDocumentData, selections: Selection[], options: IResolvedTextEditorConfiguration, viewColumn: vscode.ViewColumn) { this._proxy = proxy; this._id = id; this._documentData = document; this._selections = selections; - this._options = options; + this._options = new ExtHostTextEditorOptions(this._proxy, this._id, options); this._viewColumn = viewColumn; } @@ -341,14 +515,11 @@ class ExtHostTextEditor implements vscode.TextEditor { } set options(value: vscode.TextEditorOptions) { - this._options = value; - this._runOnProxy(() => { - return this._proxy.$trySetOptions(this._id, this._options); - }, true); + this._options.assign(value); } - _acceptOptions(options: EditorOptions): void { - this._options = options; + _acceptOptions(options: IResolvedTextEditorConfiguration): void { + this._options._accept(options); } // ---- view column @@ -460,3 +631,9 @@ class ExtHostTextEditor implements vscode.TextEditor { }); } } + +function warnOnError(promise: TPromise): void { + promise.then(null, (err) => { + console.warn(err); + }); +} diff --git a/src/vs/workbench/api/node/extHostTypes.ts b/src/vs/workbench/api/node/extHostTypes.ts index 8a6ca5041e3..e65af684509 100644 --- a/src/vs/workbench/api/node/extHostTypes.ts +++ b/src/vs/workbench/api/node/extHostTypes.ts @@ -37,11 +37,6 @@ export class Disposable { } } -export interface EditorOptions { - tabSize: number | string; - insertSpaces: boolean | string; -} - export class Position { static Min(...positions: Position[]): Position { diff --git a/src/vs/workbench/api/node/mainThreadEditorsTracker.ts b/src/vs/workbench/api/node/mainThreadEditorsTracker.ts index 344d9d04c32..0c72506ad9f 100644 --- a/src/vs/workbench/api/node/mainThreadEditorsTracker.ts +++ b/src/vs/workbench/api/node/mainThreadEditorsTracker.ts @@ -17,8 +17,8 @@ import { Selection } from 'vs/editor/common/core/selection'; import { EndOfLine, TextEditorLineNumbersStyle } from 'vs/workbench/api/node/extHostTypes'; export interface ITextEditorConfigurationUpdate { - tabSize?: number | string; - insertSpaces?: boolean | string; + tabSize?: number | 'auto'; + insertSpaces?: boolean | 'auto'; cursorStyle?: EditorCommon.TextEditorCursorStyle; lineNumbers?: TextEditorLineNumbersStyle; } @@ -209,18 +209,12 @@ export class MainThreadTextEditor { let insertSpaces = creationOpts.insertSpaces; let tabSize = creationOpts.tabSize; - if (newConfiguration.insertSpaces !== 'auto') { - if (typeof newConfiguration.insertSpaces !== 'undefined') { - insertSpaces = (newConfiguration.insertSpaces === 'false' ? false : Boolean(newConfiguration.insertSpaces)); - } + if (newConfiguration.insertSpaces !== 'auto' && typeof newConfiguration.insertSpaces !== 'undefined') { + insertSpaces = newConfiguration.insertSpaces; } - if (newConfiguration.tabSize !== 'auto') { - if (typeof newConfiguration.tabSize !== 'undefined') { - let parsedTabSize = parseInt(newConfiguration.tabSize, 10); - if (!isNaN(parsedTabSize)) { - tabSize = parsedTabSize; - } - } + + if (newConfiguration.tabSize !== 'auto' && typeof newConfiguration.tabSize !== 'undefined') { + tabSize = newConfiguration.tabSize; } this._model.detectIndentation(insertSpaces, tabSize); @@ -229,13 +223,10 @@ export class MainThreadTextEditor { let newOpts: EditorCommon.ITextModelUpdateOptions = {}; if (typeof newConfiguration.insertSpaces !== 'undefined') { - newOpts.insertSpaces = (newConfiguration.insertSpaces === 'false' ? false : Boolean(newConfiguration.insertSpaces)); + newOpts.insertSpaces = newConfiguration.insertSpaces; } if (typeof newConfiguration.tabSize !== 'undefined') { - let parsedTabSize = parseInt(newConfiguration.tabSize, 10); - if (!isNaN(parsedTabSize)) { - newOpts.tabSize = parsedTabSize; - } + newOpts.tabSize = newConfiguration.tabSize; } this._model.updateOptions(newOpts); } diff --git a/src/vs/workbench/test/node/api/extHostEditors.test.ts b/src/vs/workbench/test/node/api/extHostEditors.test.ts new file mode 100644 index 00000000000..84de2253a89 --- /dev/null +++ b/src/vs/workbench/test/node/api/extHostEditors.test.ts @@ -0,0 +1,317 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ +'use strict'; + +import * as assert from 'assert'; +import { TPromise } from 'vs/base/common/winjs.base'; +import { TextEditorLineNumbersStyle } from 'vs/workbench/api/node/extHostTypes'; +import { TextEditorCursorStyle } from 'vs/editor/common/editorCommon'; +import { IResolvedTextEditorConfiguration, ITextEditorConfigurationUpdate } from 'vs/workbench/api/node/mainThreadEditorsTracker'; +import { MainThreadEditorsShape } from 'vs/workbench/api/node/extHost.protocol'; +import { ExtHostTextEditorOptions } from 'vs/workbench/api/node/extHostEditors'; + +suite('ExtHostTextEditorOptions', () => { + + let opts: ExtHostTextEditorOptions; + let calls: ITextEditorConfigurationUpdate[] = []; + + setup(() => { + calls = []; + let mockProxy: MainThreadEditorsShape = { + $trySetOptions: (id: string, options: ITextEditorConfigurationUpdate) => { + assert.equal(id, '1'); + calls.push(options); + return TPromise.as(void 0); + }, + $tryShowTextDocument: undefined, + $registerTextEditorDecorationType: undefined, + $removeTextEditorDecorationType: undefined, + $tryShowEditor: undefined, + $tryHideEditor: undefined, + $trySetDecorations: undefined, + $tryRevealRange: undefined, + $trySetSelections: undefined, + $tryApplyEdits: undefined, + }; + opts = new ExtHostTextEditorOptions(mockProxy, '1', { + tabSize: 4, + insertSpaces: false, + cursorStyle: TextEditorCursorStyle.Line, + lineNumbers: TextEditorLineNumbersStyle.On + }); + }); + + teardown(() => { + opts = null; + calls = null; + }); + + function assertState(opts: ExtHostTextEditorOptions, expected: IResolvedTextEditorConfiguration): void { + let actual = { + tabSize: opts.tabSize, + insertSpaces: opts.insertSpaces, + cursorStyle: opts.cursorStyle, + lineNumbers: opts.lineNumbers + }; + assert.deepEqual(actual, expected); + } + + test('can set tabSize to the same value', () => { + opts.tabSize = 4; + assertState(opts, { + tabSize: 4, + insertSpaces: false, + cursorStyle: TextEditorCursorStyle.Line, + lineNumbers: TextEditorLineNumbersStyle.On + }); + assert.deepEqual(calls, []); + }); + + test('can change tabSize to positive integer', () => { + opts.tabSize = 1; + assertState(opts, { + tabSize: 1, + insertSpaces: false, + cursorStyle: TextEditorCursorStyle.Line, + lineNumbers: TextEditorLineNumbersStyle.On + }); + assert.deepEqual(calls, [{ tabSize: 1 }]); + }); + + test('can change tabSize to positive float', () => { + opts.tabSize = 2.3; + assertState(opts, { + tabSize: 2, + insertSpaces: false, + cursorStyle: TextEditorCursorStyle.Line, + lineNumbers: TextEditorLineNumbersStyle.On + }); + assert.deepEqual(calls, [{ tabSize: 2 }]); + }); + + test('can change tabSize to a string number', () => { + opts.tabSize = '2'; + assertState(opts, { + tabSize: 2, + insertSpaces: false, + cursorStyle: TextEditorCursorStyle.Line, + lineNumbers: TextEditorLineNumbersStyle.On + }); + assert.deepEqual(calls, [{ tabSize: 2 }]); + }); + + test('tabSize can request indentation detection', () => { + opts.tabSize = 'auto'; + assertState(opts, { + tabSize: 4, + insertSpaces: false, + cursorStyle: TextEditorCursorStyle.Line, + lineNumbers: TextEditorLineNumbersStyle.On + }); + assert.deepEqual(calls, [{ tabSize: 'auto' }]); + }); + + test('ignores invalid tabSize 1', () => { + opts.tabSize = null; + assertState(opts, { + tabSize: 4, + insertSpaces: false, + cursorStyle: TextEditorCursorStyle.Line, + lineNumbers: TextEditorLineNumbersStyle.On + }); + assert.deepEqual(calls, []); + }); + + test('ignores invalid tabSize 2', () => { + opts.tabSize = -5; + assertState(opts, { + tabSize: 4, + insertSpaces: false, + cursorStyle: TextEditorCursorStyle.Line, + lineNumbers: TextEditorLineNumbersStyle.On + }); + assert.deepEqual(calls, []); + }); + + test('ignores invalid tabSize 3', () => { + opts.tabSize = 'hello'; + assertState(opts, { + tabSize: 4, + insertSpaces: false, + cursorStyle: TextEditorCursorStyle.Line, + lineNumbers: TextEditorLineNumbersStyle.On + }); + assert.deepEqual(calls, []); + }); + + test('ignores invalid tabSize 4', () => { + opts.tabSize = '-17'; + assertState(opts, { + tabSize: 4, + insertSpaces: false, + cursorStyle: TextEditorCursorStyle.Line, + lineNumbers: TextEditorLineNumbersStyle.On + }); + assert.deepEqual(calls, []); + }); + + test('can set insertSpaces to the same value', () => { + opts.insertSpaces = false; + assertState(opts, { + tabSize: 4, + insertSpaces: false, + cursorStyle: TextEditorCursorStyle.Line, + lineNumbers: TextEditorLineNumbersStyle.On + }); + assert.deepEqual(calls, []); + }); + + test('can set insertSpaces to boolean', () => { + opts.insertSpaces = true; + assertState(opts, { + tabSize: 4, + insertSpaces: true, + cursorStyle: TextEditorCursorStyle.Line, + lineNumbers: TextEditorLineNumbersStyle.On + }); + assert.deepEqual(calls, [{ insertSpaces: true }]); + }); + + test('can set insertSpaces to false string', () => { + opts.insertSpaces = 'false'; + assertState(opts, { + tabSize: 4, + insertSpaces: false, + cursorStyle: TextEditorCursorStyle.Line, + lineNumbers: TextEditorLineNumbersStyle.On + }); + assert.deepEqual(calls, []); + }); + + test('can set insertSpaces to truey', () => { + opts.insertSpaces = 'hello'; + assertState(opts, { + tabSize: 4, + insertSpaces: true, + cursorStyle: TextEditorCursorStyle.Line, + lineNumbers: TextEditorLineNumbersStyle.On + }); + assert.deepEqual(calls, [{ insertSpaces: true }]); + }); + + test('insertSpaces can request indentation detection', () => { + opts.insertSpaces = 'auto'; + assertState(opts, { + tabSize: 4, + insertSpaces: false, + cursorStyle: TextEditorCursorStyle.Line, + lineNumbers: TextEditorLineNumbersStyle.On + }); + assert.deepEqual(calls, [{ insertSpaces: 'auto' }]); + }); + + test('can set cursorStyle to same value', () => { + opts.cursorStyle = TextEditorCursorStyle.Line; + assertState(opts, { + tabSize: 4, + insertSpaces: false, + cursorStyle: TextEditorCursorStyle.Line, + lineNumbers: TextEditorLineNumbersStyle.On + }); + assert.deepEqual(calls, []); + }); + + test('can change cursorStyle', () => { + opts.cursorStyle = TextEditorCursorStyle.Block; + assertState(opts, { + tabSize: 4, + insertSpaces: false, + cursorStyle: TextEditorCursorStyle.Block, + lineNumbers: TextEditorLineNumbersStyle.On + }); + assert.deepEqual(calls, [{ cursorStyle: TextEditorCursorStyle.Block }]); + }); + + test('can set lineNumbers to same value', () => { + opts.lineNumbers = TextEditorLineNumbersStyle.On; + assertState(opts, { + tabSize: 4, + insertSpaces: false, + cursorStyle: TextEditorCursorStyle.Line, + lineNumbers: TextEditorLineNumbersStyle.On + }); + assert.deepEqual(calls, []); + }); + + test('can change lineNumbers', () => { + opts.lineNumbers = TextEditorLineNumbersStyle.Off; + assertState(opts, { + tabSize: 4, + insertSpaces: false, + cursorStyle: TextEditorCursorStyle.Line, + lineNumbers: TextEditorLineNumbersStyle.Off + }); + assert.deepEqual(calls, [{ lineNumbers: TextEditorLineNumbersStyle.Off }]); + }); + + test('can do bulk updates 0', () => { + opts.assign({ + tabSize: 4, + insertSpaces: false, + cursorStyle: TextEditorCursorStyle.Line, + lineNumbers: TextEditorLineNumbersStyle.On + }); + assertState(opts, { + tabSize: 4, + insertSpaces: false, + cursorStyle: TextEditorCursorStyle.Line, + lineNumbers: TextEditorLineNumbersStyle.On + }); + assert.deepEqual(calls, []); + }); + + test('can do bulk updates 1', () => { + opts.assign({ + tabSize: 'auto', + insertSpaces: true + }); + assertState(opts, { + tabSize: 4, + insertSpaces: true, + cursorStyle: TextEditorCursorStyle.Line, + lineNumbers: TextEditorLineNumbersStyle.On + }); + assert.deepEqual(calls, [{ tabSize: 'auto', insertSpaces: true }]); + }); + + test('can do bulk updates 2', () => { + opts.assign({ + tabSize: 3, + insertSpaces: 'auto' + }); + assertState(opts, { + tabSize: 3, + insertSpaces: false, + cursorStyle: TextEditorCursorStyle.Line, + lineNumbers: TextEditorLineNumbersStyle.On + }); + assert.deepEqual(calls, [{ tabSize: 3, insertSpaces: 'auto' }]); + }); + + test('can do bulk updates 3', () => { + opts.assign({ + cursorStyle: TextEditorCursorStyle.Block, + lineNumbers: TextEditorLineNumbersStyle.Relative + }); + assertState(opts, { + tabSize: 4, + insertSpaces: false, + cursorStyle: TextEditorCursorStyle.Block, + lineNumbers: TextEditorLineNumbersStyle.Relative + }); + assert.deepEqual(calls, [{ cursorStyle: TextEditorCursorStyle.Block, lineNumbers: TextEditorLineNumbersStyle.Relative }]); + }); + +}); -- GitLab