From a3b97734a64e3b5275b4e34e8775a65687c0ef35 Mon Sep 17 00:00:00 2001 From: Alex Dima Date: Wed, 22 Feb 2017 17:07:54 +0100 Subject: [PATCH] Fixes #21073: SplitLinesCollection: attempt to access a 'newer' model --- .../editor/common/viewModel/viewModelImpl.ts | 22 +++--- .../test/common/viewModel/testViewModel.ts | 48 +++++++++++++ .../viewModel/viewModelDecorations.test.ts | 68 ++++--------------- .../common/viewModel/viewModelImpl.test.ts | 44 ++++++++++++ test/all.js | 7 +- 5 files changed, 116 insertions(+), 73 deletions(-) create mode 100644 src/vs/editor/test/common/viewModel/testViewModel.ts create mode 100644 src/vs/editor/test/common/viewModel/viewModelImpl.test.ts diff --git a/src/vs/editor/common/viewModel/viewModelImpl.ts b/src/vs/editor/common/viewModel/viewModelImpl.ts index 76776d8922f..957d48bda28 100644 --- a/src/vs/editor/common/viewModel/viewModelImpl.ts +++ b/src/vs/editor/common/viewModel/viewModelImpl.ts @@ -255,7 +255,7 @@ export class ViewModel implements IViewModel { public getCenteredRangeInViewport(): Range { if (this._centeredViewLine === -1) { - // Never got rendered + // Never got rendered or not rendered since last content change event return null; } let viewLineNumber = this._centeredViewLine; @@ -267,6 +267,7 @@ export class ViewModel implements IViewModel { const containsModelContentChangeEvent = ViewModel._containsModelContentChangeEvent(events); if (containsModelContentChangeEvent) { + this._centeredViewLine = -1; this.configuration.setMaxLineNumber(this.model.getLineCount()); } @@ -284,23 +285,18 @@ export class ViewModel implements IViewModel { previousCenteredModelRange = this.getCenteredRangeInViewport(); } - let i: number, - len: number, - e: EmitterEvent, - data: any, - modelContentChangedEvent: editorCommon.IModelContentChangedEvent, - hadOtherModelChange = false, - hadModelLineChangeThatChangedLineMapping = false, - revealPreviousCenteredModelRange = false; + let hadOtherModelChange = false; + let hadModelLineChangeThatChangedLineMapping = false; + let revealPreviousCenteredModelRange = false; - for (i = 0, len = events.length; i < len; i++) { - e = events[i]; - data = e.getData(); + for (let i = 0, len = events.length; i < len; i++) { + let e = events[i]; + let data = e.getData(); switch (e.getType()) { case editorCommon.EventType.ModelRawContentChanged: - modelContentChangedEvent = data; + let modelContentChangedEvent = data; switch (modelContentChangedEvent.changeType) { case editorCommon.EventType.ModelRawContentChangedFlush: diff --git a/src/vs/editor/test/common/viewModel/testViewModel.ts b/src/vs/editor/test/common/viewModel/testViewModel.ts new file mode 100644 index 00000000000..a44aea73467 --- /dev/null +++ b/src/vs/editor/test/common/viewModel/testViewModel.ts @@ -0,0 +1,48 @@ +/*--------------------------------------------------------------------------------------------- + * 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 { Model } from 'vs/editor/common/model/model'; +import { CharacterHardWrappingLineMapperFactory } from 'vs/editor/common/viewModel/characterHardWrappingLineMapper'; +import { MockConfiguration } from 'vs/editor/test/common/mocks/mockConfiguration'; +import { SplitLinesCollection } from 'vs/editor/common/viewModel/splitLinesCollection'; +import { ViewModel } from 'vs/editor/common/viewModel/viewModelImpl'; +import * as editorCommon from 'vs/editor/common/editorCommon'; + +export function testViewModel(text: string[], options: editorCommon.ICodeEditorWidgetCreationOptions, callback: (viewModel: ViewModel, model: Model) => void): void { + const EDITOR_ID = 1; + + let configuration = new MockConfiguration(options); + + let model = Model.createFromString(text.join('\n')); + + let factory = new CharacterHardWrappingLineMapperFactory( + configuration.editor.wrappingInfo.wordWrapBreakBeforeCharacters, + configuration.editor.wrappingInfo.wordWrapBreakAfterCharacters, + configuration.editor.wrappingInfo.wordWrapBreakObtrusiveCharacters + ); + + let linesCollection = new SplitLinesCollection( + model, + factory, + model.getOptions().tabSize, + configuration.editor.wrappingInfo.wrappingColumn, + configuration.editor.fontInfo.typicalFullwidthCharacterWidth / configuration.editor.fontInfo.typicalHalfwidthCharacterWidth, + configuration.editor.wrappingInfo.wrappingIndent + ); + + let viewModel = new ViewModel( + linesCollection, + EDITOR_ID, + configuration, + model + ); + + callback(viewModel, model); + + viewModel.dispose(); + model.dispose(); + configuration.dispose(); +} diff --git a/src/vs/editor/test/common/viewModel/viewModelDecorations.test.ts b/src/vs/editor/test/common/viewModel/viewModelDecorations.test.ts index e14fbf88ef4..0873f7b35bb 100644 --- a/src/vs/editor/test/common/viewModel/viewModelDecorations.test.ts +++ b/src/vs/editor/test/common/viewModel/viewModelDecorations.test.ts @@ -5,63 +5,18 @@ 'use strict'; import * as assert from 'assert'; -import { Model } from 'vs/editor/common/model/model'; import { Range } from 'vs/editor/common/core/range'; -import { CharacterHardWrappingLineMapperFactory } from 'vs/editor/common/viewModel/characterHardWrappingLineMapper'; -import { MockConfiguration } from 'vs/editor/test/common/mocks/mockConfiguration'; -import { SplitLinesCollection } from 'vs/editor/common/viewModel/splitLinesCollection'; -import { ViewModel } from 'vs/editor/common/viewModel/viewModelImpl'; +import { testViewModel } from 'vs/editor/test/common/viewModel/testViewModel'; suite('ViewModelDecorations', () => { - - interface ITestViewModelOpts { - wrappingColumn: number; - text: string; - } - - function withTestViewModel(opts: ITestViewModelOpts, callback: (viewModel: ViewModel, model: Model) => void): void { - const EDITOR_ID = 1; - - let configuration = new MockConfiguration({ - wrappingColumn: opts.wrappingColumn - }); - - let model = Model.createFromString(opts.text); - - let factory = new CharacterHardWrappingLineMapperFactory( - configuration.editor.wrappingInfo.wordWrapBreakBeforeCharacters, - configuration.editor.wrappingInfo.wordWrapBreakAfterCharacters, - configuration.editor.wrappingInfo.wordWrapBreakObtrusiveCharacters - ); - - let linesCollection = new SplitLinesCollection( - model, - factory, - model.getOptions().tabSize, - configuration.editor.wrappingInfo.wrappingColumn, - configuration.editor.fontInfo.typicalFullwidthCharacterWidth / configuration.editor.fontInfo.typicalHalfwidthCharacterWidth, - configuration.editor.wrappingInfo.wrappingIndent - ); - - let viewModel = new ViewModel( - linesCollection, - EDITOR_ID, - configuration, - model - ); - - callback(viewModel, model); - - viewModel.dispose(); - model.dispose(); - configuration.dispose(); - } - test('getDecorationsViewportData', () => { - withTestViewModel({ - text: 'hello world, this is a buffer that will be wrapped', + const text = [ + 'hello world, this is a buffer that will be wrapped' + ]; + const opts = { wrappingColumn: 13 - }, (viewModel, model) => { + }; + testViewModel(text, opts, (viewModel, model) => { assert.equal(viewModel.getLineContent(1), 'hello world, '); assert.equal(viewModel.getLineContent(2), 'this is a '); assert.equal(viewModel.getLineContent(3), 'buffer that '); @@ -309,10 +264,13 @@ suite('ViewModelDecorations', () => { }); test('issue #17208: Problem scrolling in 1.8.0', () => { - withTestViewModel({ - text: 'hello world, this is a buffer that will be wrapped', + const text = [ + 'hello world, this is a buffer that will be wrapped' + ]; + const opts = { wrappingColumn: 13 - }, (viewModel, model) => { + }; + testViewModel(text, opts, (viewModel, model) => { assert.equal(viewModel.getLineContent(1), 'hello world, '); assert.equal(viewModel.getLineContent(2), 'this is a '); assert.equal(viewModel.getLineContent(3), 'buffer that '); diff --git a/src/vs/editor/test/common/viewModel/viewModelImpl.test.ts b/src/vs/editor/test/common/viewModel/viewModelImpl.test.ts new file mode 100644 index 00000000000..40c071085cd --- /dev/null +++ b/src/vs/editor/test/common/viewModel/viewModelImpl.test.ts @@ -0,0 +1,44 @@ +/*--------------------------------------------------------------------------------------------- + * 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 { Range } from 'vs/editor/common/core/range'; +import { testViewModel } from 'vs/editor/test/common/viewModel/testViewModel'; + +suite('ViewModel', () => { + + test('issue #21073: SplitLinesCollection: attempt to access a \'newer\' model', () => { + const text = ['']; + const opts = { + lineNumbersMinChars: 1 + }; + testViewModel(text, opts, (viewModel, model) => { + assert.equal(viewModel.getLineCount(), 1); + + viewModel.setViewport(1, 1, 1); + + model.applyEdits([{ + identifier: null, + range: new Range(1, 1, 1, 1), + text: [ + 'line01', + 'line02', + 'line03', + 'line04', + 'line05', + 'line06', + 'line07', + 'line08', + 'line09', + 'line10', + ].join('\n'), + forceMoveMarkers: false + }]); + + assert.equal(viewModel.getLineCount(), 10); + }); + }); +}); diff --git a/test/all.js b/test/all.js index 80dd97192b0..dc91ef01e37 100644 --- a/test/all.js +++ b/test/all.js @@ -254,11 +254,8 @@ function main() { // replace the default unexpected error handler to be useful during tests loader(['vs/base/common/errors'], function(errors) { errors.setUnexpectedErrorHandler(function (err) { - try { - throw new Error('oops'); - } catch (e) { - unexpectedErrors.push((err && err.message ? err.message : err) + '\n' + e.stack); - } + let stack = (err && err.stack) || (new Error().stack); + unexpectedErrors.push((err && err.message ? err.message : err) + '\n' + stack); }); // fire up mocha