Skip to content

Commit da0e0ae

Browse files
alexdimaalexdima
committed
Revert "Fixes microsoft/monaco-editor#2774: Emit content change events before cursor state change events during undo, redo, or recovery from markers"
This reverts commit 63f82f6.
1 parent ac7d722 commit da0e0ae

File tree

7 files changed

+37
-67
lines changed

7 files changed

+37
-67
lines changed

‎src/vs/editor/common/cursor/cursor.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import { Range, IRange } from 'vs/editor/common/core/range';
1616
import { ISelection, Selection, SelectionDirection } from 'vs/editor/common/core/selection';
1717
import * as editorCommon from 'vs/editor/common/editorCommon';
1818
import { ITextModel, TrackedRangeStickiness, IModelDeltaDecoration, ICursorStateComputer, IIdentifiedSingleEditOperation, IValidEditOperation } from 'vs/editor/common/model';
19-
import { RawContentChangedType, ModelInjectedTextChangedEvent, InternalModelContentChangeEvent } from 'vs/editor/common/textModelEvents';
19+
import { RawContentChangedType, ModelRawContentChangedEvent, ModelInjectedTextChangedEvent } from 'vs/editor/common/textModelEvents';
2020
import { VerticalRevealType, ViewCursorStateChangedEvent, ViewRevealRangeRequestEvent } from 'vs/editor/common/viewEvents';
2121
import { dispose, Disposable } from 'vs/base/common/lifecycle';
2222
import { ICoordinatesConverter } from 'vs/editor/common/viewModel';
@@ -216,8 +216,8 @@ export class CursorsController extends Disposable {
216216
this.revealPrimary(eventsCollector, 'restoreState', false, VerticalRevealType.Simple, true, editorCommon.ScrollType.Immediate);
217217
}
218218

219-
public onModelContentChanged(eventsCollector: ViewModelEventsCollector, event: InternalModelContentChangeEvent | ModelInjectedTextChangedEvent): void {
220-
if (event instanceof ModelInjectedTextChangedEvent) {
219+
public onModelContentChanged(eventsCollector: ViewModelEventsCollector, e: ModelRawContentChangedEvent | ModelInjectedTextChangedEvent): void {
220+
if (e instanceof ModelInjectedTextChangedEvent) {
221221
// If injected texts change, the view positions of all cursors need to be updated.
222222
if (this._isHandling) {
223223
// The view positions will be updated when handling finishes
@@ -234,7 +234,6 @@ export class CursorsController extends Disposable {
234234
this._isHandling = false;
235235
}
236236
} else {
237-
const e = event.rawContentChangedEvent;
238237
this._knownModelVersionId = e.versionId;
239238
if (this._isHandling) {
240239
return;

‎src/vs/editor/common/model.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { LineTokens } from 'vs/editor/common/tokens/lineTokens';
1111
import { IPosition, Position } from 'vs/editor/common/core/position';
1212
import { IRange, Range } from 'vs/editor/common/core/range';
1313
import { Selection } from 'vs/editor/common/core/selection';
14-
import { IModelContentChange, IModelContentChangedEvent, IModelDecorationsChangedEvent, IModelLanguageChangedEvent, IModelLanguageConfigurationChangedEvent, IModelOptionsChangedEvent, IModelTokensChangedEvent, InternalModelContentChangeEvent, ModelInjectedTextChangedEvent } from 'vs/editor/common/textModelEvents';
14+
import { IModelContentChange, IModelContentChangedEvent, IModelDecorationsChangedEvent, IModelLanguageChangedEvent, IModelLanguageConfigurationChangedEvent, IModelOptionsChangedEvent, IModelTokensChangedEvent, ModelInjectedTextChangedEvent, ModelRawContentChangedEvent } from 'vs/editor/common/textModelEvents';
1515
import { WordCharacterClassifier } from 'vs/editor/common/core/wordCharacterClassifier';
1616
import { FormattingOptions, StandardTokenType } from 'vs/editor/common/languages';
1717
import { ThemeColor } from 'vs/platform/theme/common/themeService';
@@ -1153,7 +1153,7 @@ export interface ITextModel {
11531153
* @internal
11541154
* @event
11551155
*/
1156-
readonly onDidChangeContentOrInjectedText: Event<InternalModelContentChangeEvent | ModelInjectedTextChangedEvent>;
1156+
readonly onDidChangeContentOrInjectedText: Event<ModelRawContentChangedEvent | ModelInjectedTextChangedEvent>;
11571157
/**
11581158
* An event emitted when the contents of the model have changed.
11591159
* @event

‎src/vs/editor/common/model/textModel.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -248,9 +248,9 @@ export class TextModel extends Disposable implements model.ITextModel, IDecorati
248248
public onDidChangeContent(listener: (e: IModelContentChangedEvent) => void): IDisposable {
249249
return this._eventEmitter.slowEvent((e: InternalModelContentChangeEvent) => listener(e.contentChangedEvent));
250250
}
251-
public onDidChangeContentOrInjectedText(listener: (e: InternalModelContentChangeEvent | ModelInjectedTextChangedEvent) => void): IDisposable {
251+
public onDidChangeContentOrInjectedText(listener: (e: ModelRawContentChangedEvent | ModelInjectedTextChangedEvent) => void): IDisposable {
252252
return combinedDisposable(
253-
this._eventEmitter.fastEvent(e => listener(e)),
253+
this._eventEmitter.fastEvent(e => listener(e.rawContentChangedEvent)),
254254
this._onDidChangeInjectedText.event(e => listener(e))
255255
);
256256
}

‎src/vs/editor/common/viewModel/viewModelImpl.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -274,8 +274,8 @@ export class ViewModel extends Disposable implements IViewModel {
274274
let hadOtherModelChange = false;
275275
let hadModelLineChangeThatChangedLineMapping = false;
276276

277-
const changes = (e instanceof textModelEvents.InternalModelContentChangeEvent ? e.rawContentChangedEvent.changes : e.changes);
278-
const versionId = (e instanceof textModelEvents.InternalModelContentChangeEvent ? e.rawContentChangedEvent.versionId : null);
277+
const changes = e.changes;
278+
const versionId = (e instanceof textModelEvents.ModelRawContentChangedEvent ? e.versionId : null);
279279

280280
// Do a first pass to compute line mappings, and a second pass to actually interpret them
281281
const lineBreaksComputer = this._lines.createLineBreaksComputer();
@@ -391,9 +391,6 @@ export class ViewModel extends Disposable implements IViewModel {
391391

392392
try {
393393
const eventsCollector = this._eventDispatcher.beginEmitViewEvents();
394-
if (e instanceof textModelEvents.InternalModelContentChangeEvent) {
395-
eventsCollector.emitOutgoingEvent(new ModelContentChangedEvent(e.contentChangedEvent));
396-
}
397394
this._cursor.onModelContentChanged(eventsCollector, e);
398395
} finally {
399396
this._eventDispatcher.endEmitViewEvents();
@@ -402,6 +399,10 @@ export class ViewModel extends Disposable implements IViewModel {
402399
this._tokenizeViewportSoon.schedule();
403400
}));
404401

402+
this._register(this.model.onDidChangeContent((e) => {
403+
this._eventDispatcher.emitOutgoingEvent(new ModelContentChangedEvent(e));
404+
}));
405+
405406
this._register(this.model.onDidChangeTokens((e) => {
406407
const viewRanges: { fromLineNumber: number; toLineNumber: number }[] = [];
407408
for (let j = 0, lenJ = e.ranges.length; j < lenJ; j++) {

‎src/vs/editor/test/browser/widget/codeEditorWidget.test.ts

Lines changed: 0 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -122,33 +122,4 @@ suite('CodeEditorWidget', () => {
122122
});
123123
});
124124

125-
test('monaco-editor issue #2774 - Wrong order of events onDidChangeModelContent and onDidChangeCursorSelection on redo', () => {
126-
withTestCodeEditor('', {}, (editor, viewModel) => {
127-
const disposables = new DisposableStore();
128-
129-
const calls: string[] = [];
130-
disposables.add(editor.onDidChangeModelContent((e) => {
131-
calls.push(`contentchange(${e.changes.reduce<any[]>((aggr, c) => [...aggr, c.text, c.rangeOffset, c.rangeLength], []).join(', ')})`);
132-
}));
133-
disposables.add(editor.onDidChangeCursorSelection((e) => {
134-
calls.push(`cursorchange(${e.selection.positionLineNumber}, ${e.selection.positionColumn})`);
135-
}));
136-
137-
viewModel.type('a', 'test');
138-
viewModel.model.undo();
139-
viewModel.model.redo();
140-
141-
assert.deepStrictEqual(calls, [
142-
'contentchange(a, 0, 0)',
143-
'cursorchange(1, 2)',
144-
'contentchange(, 0, 1)',
145-
'cursorchange(1, 1)',
146-
'contentchange(a, 0, 0)',
147-
'cursorchange(1, 2)'
148-
]);
149-
150-
disposables.dispose();
151-
});
152-
});
153-
154125
});

‎src/vs/editor/test/common/model/model.test.ts

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { EditOperation } from 'vs/editor/common/core/editOperation';
99
import { Position } from 'vs/editor/common/core/position';
1010
import { Range } from 'vs/editor/common/core/range';
1111
import { TextModel } from 'vs/editor/common/model/textModel';
12-
import { InternalModelContentChangeEvent, ModelRawContentChangedEvent, ModelRawFlush, ModelRawLineChanged, ModelRawLinesDeleted, ModelRawLinesInserted } from 'vs/editor/common/textModelEvents';
12+
import { ModelInjectedTextChangedEvent, ModelRawContentChangedEvent, ModelRawFlush, ModelRawLineChanged, ModelRawLinesDeleted, ModelRawLinesInserted } from 'vs/editor/common/textModelEvents';
1313
import { EncodedTokenizationResult, IState, MetadataConsts, TokenizationRegistry } from 'vs/editor/common/languages';
1414
import { LanguageConfigurationRegistry } from 'vs/editor/common/languages/languageConfigurationRegistry';
1515
import { NullState } from 'vs/editor/common/languages/nullTokenize';
@@ -103,12 +103,12 @@ suite('Editor Model - Model', () => {
103103
});
104104

105105
test('model insert text without newline eventing', () => {
106-
let e: ModelRawContentChangedEvent | null = null;
106+
let e: ModelRawContentChangedEvent | ModelInjectedTextChangedEvent | null = null;
107107
thisModel.onDidChangeContentOrInjectedText((_e) => {
108-
if (e !== null || !(_e instanceof InternalModelContentChangeEvent)) {
108+
if (e !== null) {
109109
assert.fail('Unexpected assertion error');
110110
}
111-
e = _e.rawContentChangedEvent;
111+
e = _e;
112112
});
113113
thisModel.applyEdits([EditOperation.insert(new Position(1, 1), 'foo ')]);
114114
assert.deepStrictEqual(e, new ModelRawContentChangedEvent(
@@ -122,12 +122,12 @@ suite('Editor Model - Model', () => {
122122
});
123123

124124
test('model insert text with one newline eventing', () => {
125-
let e: ModelRawContentChangedEvent | null = null;
125+
let e: ModelRawContentChangedEvent | ModelInjectedTextChangedEvent | null = null;
126126
thisModel.onDidChangeContentOrInjectedText((_e) => {
127-
if (e !== null || !(_e instanceof InternalModelContentChangeEvent)) {
127+
if (e !== null) {
128128
assert.fail('Unexpected assertion error');
129129
}
130-
e = _e.rawContentChangedEvent;
130+
e = _e;
131131
});
132132
thisModel.applyEdits([EditOperation.insert(new Position(1, 3), ' new line\nNo longer')]);
133133
assert.deepStrictEqual(e, new ModelRawContentChangedEvent(
@@ -199,12 +199,12 @@ suite('Editor Model - Model', () => {
199199
});
200200

201201
test('model delete text from one line eventing', () => {
202-
let e: ModelRawContentChangedEvent | null = null;
202+
let e: ModelRawContentChangedEvent | ModelInjectedTextChangedEvent | null = null;
203203
thisModel.onDidChangeContentOrInjectedText((_e) => {
204-
if (e !== null || !(_e instanceof InternalModelContentChangeEvent)) {
204+
if (e !== null) {
205205
assert.fail('Unexpected assertion error');
206206
}
207-
e = _e.rawContentChangedEvent;
207+
e = _e;
208208
});
209209
thisModel.applyEdits([EditOperation.delete(new Range(1, 1, 1, 2))]);
210210
assert.deepStrictEqual(e, new ModelRawContentChangedEvent(
@@ -218,12 +218,12 @@ suite('Editor Model - Model', () => {
218218
});
219219

220220
test('model delete all text from a line eventing', () => {
221-
let e: ModelRawContentChangedEvent | null = null;
221+
let e: ModelRawContentChangedEvent | ModelInjectedTextChangedEvent | null = null;
222222
thisModel.onDidChangeContentOrInjectedText((_e) => {
223-
if (e !== null || !(_e instanceof InternalModelContentChangeEvent)) {
223+
if (e !== null) {
224224
assert.fail('Unexpected assertion error');
225225
}
226-
e = _e.rawContentChangedEvent;
226+
e = _e;
227227
});
228228
thisModel.applyEdits([EditOperation.delete(new Range(1, 1, 1, 14))]);
229229
assert.deepStrictEqual(e, new ModelRawContentChangedEvent(
@@ -237,12 +237,12 @@ suite('Editor Model - Model', () => {
237237
});
238238

239239
test('model delete text from two lines eventing', () => {
240-
let e: ModelRawContentChangedEvent | null = null;
240+
let e: ModelRawContentChangedEvent | ModelInjectedTextChangedEvent | null = null;
241241
thisModel.onDidChangeContentOrInjectedText((_e) => {
242-
if (e !== null || !(_e instanceof InternalModelContentChangeEvent)) {
242+
if (e !== null) {
243243
assert.fail('Unexpected assertion error');
244244
}
245-
e = _e.rawContentChangedEvent;
245+
e = _e;
246246
});
247247
thisModel.applyEdits([EditOperation.delete(new Range(1, 4, 2, 6))]);
248248
assert.deepStrictEqual(e, new ModelRawContentChangedEvent(
@@ -257,12 +257,12 @@ suite('Editor Model - Model', () => {
257257
});
258258

259259
test('model delete text from many lines eventing', () => {
260-
let e: ModelRawContentChangedEvent | null = null;
260+
let e: ModelRawContentChangedEvent | ModelInjectedTextChangedEvent | null = null;
261261
thisModel.onDidChangeContentOrInjectedText((_e) => {
262-
if (e !== null || !(_e instanceof InternalModelContentChangeEvent)) {
262+
if (e !== null) {
263263
assert.fail('Unexpected assertion error');
264264
}
265-
e = _e.rawContentChangedEvent;
265+
e = _e;
266266
});
267267
thisModel.applyEdits([EditOperation.delete(new Range(1, 4, 3, 5))]);
268268
assert.deepStrictEqual(e, new ModelRawContentChangedEvent(
@@ -308,12 +308,12 @@ suite('Editor Model - Model', () => {
308308

309309
// --------- setValue
310310
test('setValue eventing', () => {
311-
let e: ModelRawContentChangedEvent | null = null;
311+
let e: ModelRawContentChangedEvent | ModelInjectedTextChangedEvent | null = null;
312312
thisModel.onDidChangeContentOrInjectedText((_e) => {
313-
if (e !== null || !(_e instanceof InternalModelContentChangeEvent)) {
313+
if (e !== null) {
314314
assert.fail('Unexpected assertion error');
315315
}
316-
e = _e.rawContentChangedEvent;
316+
e = _e;
317317
});
318318
thisModel.setValue('new value');
319319
assert.deepStrictEqual(e, new ModelRawContentChangedEvent(

‎src/vs/editor/test/common/model/modelInjectedText.test.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import * as assert from 'assert';
77
import { EditOperation } from 'vs/editor/common/core/editOperation';
88
import { Range } from 'vs/editor/common/core/range';
99
import { TextModel } from 'vs/editor/common/model/textModel';
10-
import { InternalModelContentChangeEvent, LineInjectedText, ModelRawChange, RawContentChangedType } from 'vs/editor/common/textModelEvents';
10+
import { LineInjectedText, ModelRawChange, RawContentChangedType } from 'vs/editor/common/textModelEvents';
1111
import { createTextModel } from 'vs/editor/test/common/testTextModel';
1212

1313
suite('Editor Model - Injected Text Events', () => {
@@ -25,8 +25,7 @@ suite('Editor Model - Injected Text Events', () => {
2525
const recordedChanges = new Array<unknown>();
2626

2727
thisModel.onDidChangeContentOrInjectedText((e) => {
28-
const changes = (e instanceof InternalModelContentChangeEvent ? e.rawContentChangedEvent.changes : e.changes);
29-
for (const change of changes) {
28+
for (const change of e.changes) {
3029
recordedChanges.push(mapChange(change));
3130
}
3231
});

0 commit comments

Comments
 (0)