From 6f97438762ca321a75c660d40d54db60a6fef97f Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Mon, 30 Oct 2017 12:59:32 -0700 Subject: [PATCH 1/8] Only emit one display layer change event per buffer transaction --- spec/display-layer-spec.js | 64 +++++++++++++++++++++++++++++++++--- spec/text-buffer-spec.coffee | 16 +++++++-- src/display-layer.js | 20 ++++++++++- src/text-buffer.coffee | 22 ++++++++----- 4 files changed, 105 insertions(+), 17 deletions(-) diff --git a/spec/display-layer-spec.js b/spec/display-layer-spec.js index fa6c23fb13..42aef54fa6 100644 --- a/spec/display-layer-spec.js +++ b/spec/display-layer-spec.js @@ -2167,6 +2167,62 @@ describe('DisplayLayer', () => { }) }) + describe('.onDidChangeSync', () => { + it('calls the given callback when the display layer\'s content changes', () => { + const buffer = new TextBuffer({ + text: 'abc\ndef\nghi\njkl\nmno' + }) + + const displayLayer = buffer.addDisplayLayer({tabLength: 4}) + + const events = [] + displayLayer.onDidChangeSync((changes) => events.push(...changes)) + + displayLayer.foldBufferRange(Range(Point(1, 1), Point(2, 2))) + expect(events).toEqual([ + { + start: Point(1, 0), + oldExtent: Point(2, 0), + newExtent: Point(1, 0) + } + ]) + + events.length = 0 + displayLayer.foldBufferRange(Range(Point(3, 1), Point(4, 2))) + expect(events).toEqual([ + { + start: Point(2, 0), + oldExtent: Point(2, 0), + newExtent: Point(1, 0) + } + ]) + + events.length = 0 + displayLayer.destroyAllFolds() + expect(events).toEqual([ + { + start: Point(1, 0), + oldExtent: Point(2, 0), + newExtent: Point(4, 0) + } + ]) + + // When multiple changes occur in a transaction, the changes are combined. + events.length = 0 + buffer.transact(() => { + displayLayer.foldBufferRange(Range(Point(1, 1), Point(2, 2))) + displayLayer.foldBufferRange(Range(Point(3, 1), Point(4, 2))) + }) + expect(events).toEqual([ + { + start: Point(1, 0), + oldExtent: Point(4, 0), + newExtent: Point(2, 0) + } + ]) + }) + }) + describe('.getApproximateScreenLineCount()', () => { it('estimates the screen line count based on the currently-indexed portion of the buffer', () => { const buffer = new TextBuffer({ @@ -2458,10 +2514,10 @@ function verifyChangeEvent (displayLayer, fn) { displayLayerCopy.setTextDecorationLayer(displayLayer.getTextDecorationLayer()) const previousTokenLines = getTokens(displayLayerCopy) displayLayerCopy.destroy() - let lastChanges = null + let changes = [] - const disposable = displayLayer.onDidChangeSync(function (changes) { - lastChanges = changes + const disposable = displayLayer.onDidChangeSync((event) => { + changes.push(...event) }) fn() @@ -2469,7 +2525,7 @@ function verifyChangeEvent (displayLayer, fn) { displayLayerCopy = displayLayer.copy() displayLayerCopy.setTextDecorationLayer(displayLayer.getTextDecorationLayer()) const expectedTokenLines = getTokens(displayLayerCopy) - updateTokenLines(previousTokenLines, displayLayerCopy, lastChanges) + updateTokenLines(previousTokenLines, displayLayerCopy, changes) displayLayerCopy.destroy() expect(previousTokenLines).toEqual(expectedTokenLines) } diff --git a/spec/text-buffer-spec.coffee b/spec/text-buffer-spec.coffee index 1eff8c121d..12360c693b 100644 --- a/spec/text-buffer-spec.coffee +++ b/spec/text-buffer-spec.coffee @@ -98,6 +98,8 @@ describe "TextBuffer", -> describe "after a change", -> it "notifies, in order, decoration layers, display layers, and display layer ::onDidChangeSync observers with the relevant details", -> + buffer = new TextBuffer("hello\nworld\r\nhow are you doing?") + events = [] textDecorationLayer1 = { bufferDidChange: (e) -> events.push({source: 'decoration-layer-1', event: e}) @@ -118,11 +120,11 @@ describe "TextBuffer", -> buffer.registerTextDecorationLayer(textDecorationLayer1) # insert a duplicate decoration layer buffer.registerTextDecorationLayer(textDecorationLayer2) buffer.onDidChange (e) -> events.push({source: 'buffer', event: JSON.parse(JSON.stringify(e))}) + displayLayer1.onDidChangeSync (e) -> events.push({source: 'display-layer-event', event: e}) - disposable = displayLayer1.onDidChangeSync -> - disposable.dispose() + buffer.transact -> + buffer.setTextInRange([[0, 2], [2, 3]], "y there\r\ncat\nwhat", normalizeLineEndings: false) buffer.setTextInRange([[1, 1], [1, 2]], "abc", normalizeLineEndings: false) - buffer.setTextInRange([[0, 2], [2, 3]], "y there\r\ncat\nwhat", normalizeLineEndings: false) changeEvent1 = { oldRange: [[0, 2], [2, 3]], newRange: [[0, 2], [2, 4]] @@ -157,6 +159,14 @@ describe "TextBuffer", -> } ] } + }, + { + source: 'display-layer-event', + event: [{ + start: Point(0, 0), + oldExtent: Point(3, 0), + newExtent: Point(3, 0) + }] } ] diff --git a/src/display-layer.js b/src/display-layer.js index a4a82eedaa..b82af9b671 100644 --- a/src/display-layer.js +++ b/src/display-layer.js @@ -24,6 +24,7 @@ class DisplayLayer { this.textDecorationLayer = new EmptyDecorationLayer() this.displayMarkerLayersById = new Map() this.destroyed = false + this.deferredChangeEventPatch = new Patch() this.invisibles = params.invisibles != null ? params.invisibles : {} this.tabLength = params.tabLength != null ? params.tabLength : 4 @@ -834,7 +835,24 @@ class DisplayLayer { } emitDidChangeSyncEvent (event) { - this.emitter.emit('did-change-sync', event) + if (this.buffer.transactCallDepth === 0) { + this.emitter.emit('did-change-sync', event) + } else { + for (const change of event) { + this.deferredChangeEventPatch.splice(change.start, change.oldExtent, change.newExtent) + } + } + } + + emitDeferredChangeEvents () { + if (this.deferredChangeEventPatch.getChangeCount() > 0) { + this.emitter.emit('did-change-sync', this.deferredChangeEventPatch.getChanges().map((change) => ({ + start: change.newStart, + oldExtent: traversal(change.oldEnd, change.oldStart), + newExtent: traversal(change.newEnd, change.newStart) + }))) + this.deferredChangeEventPatch = new Patch() + } } notifyObserversIfMarkerScreenPositionsChanged () { diff --git a/src/text-buffer.coffee b/src/text-buffer.coffee index 4bd511c9ed..3f62d4856f 100644 --- a/src/text-buffer.coffee +++ b/src/text-buffer.coffee @@ -1906,15 +1906,19 @@ class TextBuffer @_emittedWillChangeEvent = true emitDidChangeTextEvent: -> - if @transactCallDepth is 0 and @changesSinceLastDidChangeTextEvent.length > 0 - compactedChanges = Object.freeze(normalizePatchChanges( - patchFromChanges(@changesSinceLastDidChangeTextEvent).getChanges() - )) - @changesSinceLastDidChangeTextEvent.length = 0 - if compactedChanges.length > 0 - @emitter.emit 'did-change-text', new ChangeEvent(this, compactedChanges) - @debouncedEmitDidStopChangingEvent() - @_emittedWillChangeEvent = false + if @transactCallDepth is 0 + if @changesSinceLastDidChangeTextEvent.length > 0 + compactedChanges = Object.freeze(normalizePatchChanges( + patchFromChanges(@changesSinceLastDidChangeTextEvent).getChanges() + )) + @changesSinceLastDidChangeTextEvent.length = 0 + if compactedChanges.length > 0 + @emitter.emit 'did-change-text', new ChangeEvent(this, compactedChanges) + @debouncedEmitDidStopChangingEvent() + @_emittedWillChangeEvent = false + for id, displayLayer of @displayLayers + displayLayer.emitDeferredChangeEvents() + return # Identifies if the buffer belongs to multiple editors. # From 9c5c8ed7f8468ac33d70806bc1f9577bf66b2298 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Mon, 30 Oct 2017 13:32:30 -0700 Subject: [PATCH 2/8] Only emit one display layer change event in undo/redo/revertToCheckpoint --- spec/display-layer-spec.js | 78 ++++++++++++++++++++++++++++++++++++++ src/text-buffer.coffee | 18 +++++++-- 2 files changed, 93 insertions(+), 3 deletions(-) diff --git a/spec/display-layer-spec.js b/spec/display-layer-spec.js index 42aef54fa6..f4bf99b842 100644 --- a/spec/display-layer-spec.js +++ b/spec/display-layer-spec.js @@ -2221,6 +2221,84 @@ describe('DisplayLayer', () => { } ]) }) + + it('calls the callback only one time per text buffer transaction', () => { + const buffer = new TextBuffer({ + text: 'abc\ndef\nghi\njkl\nmno' + }) + + const displayLayer = buffer.addDisplayLayer({tabLength: 4}) + + const events = [] + displayLayer.onDidChangeSync((changes) => events.push(changes)) + + const checkpoint = buffer.createCheckpoint() + + buffer.transact(() => { + buffer.setTextInRange([[0, 1], [0, 1]], '\n') + buffer.setTextInRange([[4, 2], [4, 2]], '\n') + buffer.setTextInRange([[4, 2], [4, 2]], '.') + buffer.setTextInRange([[4, 3], [4, 3]], '.') + buffer.setTextInRange([[4, 4], [4, 4]], '.') + }) + expect(events).toEqual([[ + { + start: Point(0, 0), + oldExtent: Point(1, 0), + newExtent: Point(2, 0) + }, + { + start: Point(4, 0), + oldExtent: Point(1, 0), + newExtent: Point(2, 0) + } + ]]) + + events.length = 0 + buffer.undo() + expect(events).toEqual([[ + { + start: Point(0, 0), + oldExtent: Point(2, 0), + newExtent: Point(1, 0) + }, + { + start: Point(3, 0), + oldExtent: Point(2, 0), + newExtent: Point(1, 0) + } + ]]) + + events.length = 0 + buffer.redo() + expect(events).toEqual([[ + { + start: Point(0, 0), + oldExtent: Point(1, 0), + newExtent: Point(2, 0) + }, + { + start: Point(4, 0), + oldExtent: Point(1, 0), + newExtent: Point(2, 0) + } + ]]) + + events.length = 0 + buffer.revertToCheckpoint(checkpoint) + expect(events).toEqual([[ + { + start: Point(0, 0), + oldExtent: Point(2, 0), + newExtent: Point(1, 0) + }, + { + start: Point(3, 0), + oldExtent: Point(2, 0), + newExtent: Point(1, 0) + } + ]]) + }) }) describe('.getApproximateScreenLineCount()', () => { diff --git a/src/text-buffer.coffee b/src/text-buffer.coffee index 3f62d4856f..6adc3c5c95 100644 --- a/src/text-buffer.coffee +++ b/src/text-buffer.coffee @@ -1126,7 +1126,11 @@ class TextBuffer undo: -> if pop = @historyProvider.undo() @emitWillChangeEvent() - @applyChange(change) for change in pop.textUpdates + @transactCallDepth++ + try + @applyChange(change) for change in pop.textUpdates + finally + @transactCallDepth-- @restoreFromMarkerSnapshot(pop.markers) @emitDidChangeTextEvent() @emitMarkerChangeEvents(pop.markers) @@ -1138,7 +1142,11 @@ class TextBuffer redo: -> if pop = @historyProvider.redo() @emitWillChangeEvent() - @applyChange(change) for change in pop.textUpdates + @transactCallDepth++ + try + @applyChange(change) for change in pop.textUpdates + finally + @transactCallDepth-- @restoreFromMarkerSnapshot(pop.markers) @emitDidChangeTextEvent() @emitMarkerChangeEvents(pop.markers) @@ -1210,7 +1218,11 @@ class TextBuffer revertToCheckpoint: (checkpoint, options) -> if truncated = @historyProvider.revertToCheckpoint(checkpoint, options) @emitWillChangeEvent() - @applyChange(change) for change in truncated.textUpdates + @transactCallDepth++ + try + @applyChange(change) for change in truncated.textUpdates + finally + @transactCallDepth-- @restoreFromMarkerSnapshot(truncated.markers) @emitDidChangeTextEvent() @emitter.emit 'did-update-markers' From 1d340bc3876ac017dc397323138e316060b3d4d2 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Mon, 30 Oct 2017 13:33:12 -0700 Subject: [PATCH 3/8] 13.8.0-display-layer-change-event-1 --- package-lock.json | 2 +- package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/package-lock.json b/package-lock.json index b11c1629f1..8099de6ecd 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "text-buffer", - "version": "13.7.1", + "version": "13.8.0-display-layer-change-event-1", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index f71d169175..b82f9ed163 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "text-buffer", - "version": "13.7.1", + "version": "13.8.0-display-layer-change-event-1", "description": "A container for large mutable strings with annotated regions", "main": "./lib/text-buffer", "scripts": { From e45e1a3db370e4d1f89f633d8a12101fc8f6a8a0 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Mon, 30 Oct 2017 14:30:43 -0700 Subject: [PATCH 4/8] Remove unused preemptDidChange method --- src/text-buffer.coffee | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/text-buffer.coffee b/src/text-buffer.coffee index 6adc3c5c95..31dd3f747a 100644 --- a/src/text-buffer.coffee +++ b/src/text-buffer.coffee @@ -369,9 +369,6 @@ class TextBuffer # Public: This is now identical to {onDidChange}. onDidChangeText: (callback) -> @onDidChange(callback) - preemptDidChange: (callback) -> - @emitter.preempt 'did-change', callback - # Public: Invoke the given callback asynchronously following one or more # changes after {::getStoppedChangingDelay} milliseconds elapse without an # additional change. From 7ee1821f3a178bedd5886b19ad9ea8d1186a86d5 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Mon, 30 Oct 2017 14:32:41 -0700 Subject: [PATCH 5/8] Simplify code for display layer change events due to buffer changes --- src/display-layer.js | 2 +- src/text-buffer.coffee | 9 +-------- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/src/display-layer.js b/src/display-layer.js index b82af9b671..5eef629edc 100644 --- a/src/display-layer.js +++ b/src/display-layer.js @@ -825,7 +825,7 @@ class DisplayLayer { combinedChanges.splice(Point(startRow, 0), extent, extent) } - return Object.freeze(combinedChanges.getChanges().map((hunk) => { + this.emitDidChangeSyncEvent(combinedChanges.getChanges().map((hunk) => { return { start: Point.fromObject(hunk.newStart), oldExtent: traversal(hunk.oldEnd, hunk.oldStart), diff --git a/src/text-buffer.coffee b/src/text-buffer.coffee index 31dd3f747a..3e65ecf000 100644 --- a/src/text-buffer.coffee +++ b/src/text-buffer.coffee @@ -881,16 +881,9 @@ class TextBuffer # Emit the change event on all the registered text decoration layers. @textDecorationLayers.forEach (textDecorationLayer) -> textDecorationLayer.bufferDidChange(changeEvent) - # Emit the change event on all the registered display layers. - changeEventsByDisplayLayer = new Map() for id, displayLayer of @displayLayers event = displayLayer.bufferDidChange(changeEvent) - changeEventsByDisplayLayer.set(displayLayer, event) - # Emit a normal `did-change` event for other subscribers too. - @emitter.emit 'did-change', changeEvent - # Emit a `did-change-sync` event from all the registered display layers. - changeEventsByDisplayLayer.forEach (event, displayLayer) -> - displayLayer.emitDidChangeSyncEvent(event) + return # Public: Delete the text in the given range. # From 43fcf81103098a3cb4ec7463f2e3e78fd224fbb1 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Mon, 30 Oct 2017 14:37:12 -0700 Subject: [PATCH 6/8] Rename DisplayLayer.onDidChangeSync -> .onDidChange --- spec/display-layer-spec.js | 10 +++++----- spec/marker-layer-spec.coffee | 2 +- spec/text-buffer-io-spec.js | 2 +- spec/text-buffer-spec.coffee | 4 ++-- src/display-layer.js | 18 +++++++++--------- 5 files changed, 18 insertions(+), 18 deletions(-) diff --git a/spec/display-layer-spec.js b/spec/display-layer-spec.js index f4bf99b842..8f448fd3d8 100644 --- a/spec/display-layer-spec.js +++ b/spec/display-layer-spec.js @@ -1989,7 +1989,7 @@ describe('DisplayLayer', () => { displayLayer.setTextDecorationLayer(decorationLayer) const allChanges = [] - displayLayer.onDidChangeSync((changes) => allChanges.push(...changes)) + displayLayer.onDidChange((changes) => allChanges.push(...changes)) decorationLayer.emitInvalidateRangeEvent([[2, 1], [3, 2]]) @@ -2167,7 +2167,7 @@ describe('DisplayLayer', () => { }) }) - describe('.onDidChangeSync', () => { + describe('.onDidChange', () => { it('calls the given callback when the display layer\'s content changes', () => { const buffer = new TextBuffer({ text: 'abc\ndef\nghi\njkl\nmno' @@ -2176,7 +2176,7 @@ describe('DisplayLayer', () => { const displayLayer = buffer.addDisplayLayer({tabLength: 4}) const events = [] - displayLayer.onDidChangeSync((changes) => events.push(...changes)) + displayLayer.onDidChange((changes) => events.push(...changes)) displayLayer.foldBufferRange(Range(Point(1, 1), Point(2, 2))) expect(events).toEqual([ @@ -2230,7 +2230,7 @@ describe('DisplayLayer', () => { const displayLayer = buffer.addDisplayLayer({tabLength: 4}) const events = [] - displayLayer.onDidChangeSync((changes) => events.push(changes)) + displayLayer.onDidChange((changes) => events.push(changes)) const checkpoint = buffer.createCheckpoint() @@ -2594,7 +2594,7 @@ function verifyChangeEvent (displayLayer, fn) { displayLayerCopy.destroy() let changes = [] - const disposable = displayLayer.onDidChangeSync((event) => { + const disposable = displayLayer.onDidChange((event) => { changes.push(...event) }) diff --git a/spec/marker-layer-spec.coffee b/spec/marker-layer-spec.coffee index 06659400cd..f8efa226af 100644 --- a/spec/marker-layer-spec.coffee +++ b/spec/marker-layer-spec.coffee @@ -219,7 +219,7 @@ describe "MarkerLayer", -> expect(updateCount).toBe(6) # update events happen after updating display layers when there is no parent transaction. - displayLayer.onDidChangeSync -> + displayLayer.onDidChange -> displayLayerDidChange = true buffer.undo() expect(updateCount).toBe(7) diff --git a/spec/text-buffer-io-spec.js b/spec/text-buffer-io-spec.js index c691e8fa84..e7b6c43962 100644 --- a/spec/text-buffer-io-spec.js +++ b/spec/text-buffer-io-spec.js @@ -165,7 +165,7 @@ describe('TextBuffer IO', () => { const events = [] const displayLayer = buffer.addDisplayLayer() - displayLayer.onDidChangeSync((event) => events.push(['display-layer', event])) + displayLayer.onDidChange((event) => events.push(['display-layer', event])) buffer.registerTextDecorationLayer({ bufferDidChange ({oldRange, newRange}) { events.push(['decoration-layer', {oldRange, newRange}]) } diff --git a/spec/text-buffer-spec.coffee b/spec/text-buffer-spec.coffee index 12360c693b..8d358c753c 100644 --- a/spec/text-buffer-spec.coffee +++ b/spec/text-buffer-spec.coffee @@ -97,7 +97,7 @@ describe "TextBuffer", -> expect(buffer.getText()).toEqual "hey\nyou're old\r\nhow are you doing?" describe "after a change", -> - it "notifies, in order, decoration layers, display layers, and display layer ::onDidChangeSync observers with the relevant details", -> + it "notifies, in order, decoration layers, display layers, and display layer ::onDidChange observers with the relevant details", -> buffer = new TextBuffer("hello\nworld\r\nhow are you doing?") events = [] @@ -120,7 +120,7 @@ describe "TextBuffer", -> buffer.registerTextDecorationLayer(textDecorationLayer1) # insert a duplicate decoration layer buffer.registerTextDecorationLayer(textDecorationLayer2) buffer.onDidChange (e) -> events.push({source: 'buffer', event: JSON.parse(JSON.stringify(e))}) - displayLayer1.onDidChangeSync (e) -> events.push({source: 'display-layer-event', event: e}) + displayLayer1.onDidChange (e) -> events.push({source: 'display-layer-event', event: e}) buffer.transact -> buffer.setTextInRange([[0, 2], [2, 3]], "y there\r\ncat\nwhat", normalizeLineEndings: false) diff --git a/src/display-layer.js b/src/display-layer.js index 5eef629edc..2d14ce2e02 100644 --- a/src/display-layer.js +++ b/src/display-layer.js @@ -154,7 +154,7 @@ class DisplayLayer { const endRow = this.translateBufferPositionWithSpatialIndex(Point(endBufferRow, 0), 'backward').row const extent = Point(endRow - startRow, 0) spliceArray(this.cachedScreenLines, startRow, extent.row, new Array(extent.row)) - this.emitDidChangeSyncEvent([{ + this.emitChangeEvent([{ start: Point(startRow, 0), oldExtent: extent, newExtent: extent @@ -186,8 +186,8 @@ class DisplayLayer { this.displayMarkerLayersById.delete(id) } - onDidChangeSync (callback) { - return this.emitter.on('did-change-sync', callback) + onDidChange (callback) { + return this.emitter.on('did-change', callback) } onDidReset (callback) { @@ -208,7 +208,7 @@ class DisplayLayer { if (containingFoldMarkers.length === 0) { const foldStartRow = bufferRange.start.row const foldEndRow = bufferRange.end.row + 1 - this.emitDidChangeSyncEvent([ + this.emitChangeEvent([ this.updateSpatialIndex(foldStartRow, foldEndRow, foldEndRow, Infinity) ]) this.notifyObserversIfMarkerScreenPositionsChanged() @@ -270,7 +270,7 @@ class DisplayLayer { foldMarker.destroy() } - this.emitDidChangeSyncEvent([this.updateSpatialIndex( + this.emitChangeEvent([this.updateSpatialIndex( combinedRangeStart.row, combinedRangeEnd.row + 1, combinedRangeEnd.row + 1, @@ -825,7 +825,7 @@ class DisplayLayer { combinedChanges.splice(Point(startRow, 0), extent, extent) } - this.emitDidChangeSyncEvent(combinedChanges.getChanges().map((hunk) => { + this.emitChangeEvent(combinedChanges.getChanges().map((hunk) => { return { start: Point.fromObject(hunk.newStart), oldExtent: traversal(hunk.oldEnd, hunk.oldStart), @@ -834,9 +834,9 @@ class DisplayLayer { })) } - emitDidChangeSyncEvent (event) { + emitChangeEvent (event) { if (this.buffer.transactCallDepth === 0) { - this.emitter.emit('did-change-sync', event) + this.emitter.emit('did-change', event) } else { for (const change of event) { this.deferredChangeEventPatch.splice(change.start, change.oldExtent, change.newExtent) @@ -846,7 +846,7 @@ class DisplayLayer { emitDeferredChangeEvents () { if (this.deferredChangeEventPatch.getChangeCount() > 0) { - this.emitter.emit('did-change-sync', this.deferredChangeEventPatch.getChanges().map((change) => ({ + this.emitter.emit('did-change', this.deferredChangeEventPatch.getChanges().map((change) => ({ start: change.newStart, oldExtent: traversal(change.oldEnd, change.oldStart), newExtent: traversal(change.newEnd, change.newStart) From 52191be9d424b001c656b56cc799aff853bd1aa7 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Mon, 30 Oct 2017 17:00:03 -0700 Subject: [PATCH 7/8] Make display layer change events more like buffer change events --- spec/display-layer-spec.js | 71 +++++++++++++++--------------------- spec/text-buffer-io-spec.js | 2 +- spec/text-buffer-spec.coffee | 5 +-- src/display-layer.js | 56 +++++++++------------------- 4 files changed, 50 insertions(+), 84 deletions(-) diff --git a/spec/display-layer-spec.js b/spec/display-layer-spec.js index 8f448fd3d8..3df091a400 100644 --- a/spec/display-layer-spec.js +++ b/spec/display-layer-spec.js @@ -1994,9 +1994,8 @@ describe('DisplayLayer', () => { decorationLayer.emitInvalidateRangeEvent([[2, 1], [3, 2]]) expect(allChanges).toEqual([{ - start: Point(1, 0), - oldExtent: Point(2, 0), - newExtent: Point(2, 0) + oldRange: Range(Point(1, 0), Point(3, 0)), + newRange: Range(Point(1, 0), Point(3, 0)) }]) }) @@ -2181,9 +2180,8 @@ describe('DisplayLayer', () => { displayLayer.foldBufferRange(Range(Point(1, 1), Point(2, 2))) expect(events).toEqual([ { - start: Point(1, 0), - oldExtent: Point(2, 0), - newExtent: Point(1, 0) + oldRange: Range(Point(1, 0), Point(3, 0)), + newRange: Range(Point(1, 0), Point(2, 0)) } ]) @@ -2191,9 +2189,8 @@ describe('DisplayLayer', () => { displayLayer.foldBufferRange(Range(Point(3, 1), Point(4, 2))) expect(events).toEqual([ { - start: Point(2, 0), - oldExtent: Point(2, 0), - newExtent: Point(1, 0) + oldRange: Range(Point(2, 0), Point(4, 0)), + newRange: Range(Point(2, 0), Point(3, 0)) } ]) @@ -2201,9 +2198,8 @@ describe('DisplayLayer', () => { displayLayer.destroyAllFolds() expect(events).toEqual([ { - start: Point(1, 0), - oldExtent: Point(2, 0), - newExtent: Point(4, 0) + oldRange: Range(Point(1, 0), Point(3, 0)), + newRange: Range(Point(1, 0), Point(5, 0)) } ]) @@ -2215,9 +2211,8 @@ describe('DisplayLayer', () => { }) expect(events).toEqual([ { - start: Point(1, 0), - oldExtent: Point(4, 0), - newExtent: Point(2, 0) + oldRange: Range(Point(1, 0), Point(5, 0)), + newRange: Range(Point(1, 0), Point(3, 0)) } ]) }) @@ -2243,14 +2238,12 @@ describe('DisplayLayer', () => { }) expect(events).toEqual([[ { - start: Point(0, 0), - oldExtent: Point(1, 0), - newExtent: Point(2, 0) + oldRange: Range(Point(0, 0), Point(1, 0)), + newRange: Range(Point(0, 0), Point(2, 0)) }, { - start: Point(4, 0), - oldExtent: Point(1, 0), - newExtent: Point(2, 0) + oldRange: Range(Point(3, 0), Point(4, 0)), + newRange: Range(Point(4, 0), Point(6, 0)) } ]]) @@ -2258,14 +2251,12 @@ describe('DisplayLayer', () => { buffer.undo() expect(events).toEqual([[ { - start: Point(0, 0), - oldExtent: Point(2, 0), - newExtent: Point(1, 0) + oldRange: Range(Point(0, 0), Point(2, 0)), + newRange: Range(Point(0, 0), Point(1, 0)) }, { - start: Point(3, 0), - oldExtent: Point(2, 0), - newExtent: Point(1, 0) + oldRange: Range(Point(4, 0), Point(6, 0)), + newRange: Range(Point(3, 0), Point(4, 0)) } ]]) @@ -2273,14 +2264,12 @@ describe('DisplayLayer', () => { buffer.redo() expect(events).toEqual([[ { - start: Point(0, 0), - oldExtent: Point(1, 0), - newExtent: Point(2, 0) + oldRange: Range(Point(0, 0), Point(1, 0)), + newRange: Range(Point(0, 0), Point(2, 0)) }, { - start: Point(4, 0), - oldExtent: Point(1, 0), - newExtent: Point(2, 0) + oldRange: Range(Point(3, 0), Point(4, 0)), + newRange: Range(Point(4, 0), Point(6, 0)) } ]]) @@ -2288,14 +2277,12 @@ describe('DisplayLayer', () => { buffer.revertToCheckpoint(checkpoint) expect(events).toEqual([[ { - start: Point(0, 0), - oldExtent: Point(2, 0), - newExtent: Point(1, 0) + oldRange: Range(Point(0, 0), Point(2, 0)), + newRange: Range(Point(0, 0), Point(1, 0)) }, { - start: Point(3, 0), - oldExtent: Point(2, 0), - newExtent: Point(1, 0) + oldRange: Range(Point(4, 0), Point(6, 0)), + newRange: Range(Point(3, 0), Point(4, 0)) } ]]) }) @@ -2834,9 +2821,9 @@ function getTokenBoundaries (displayLayer, startRow = 0, endRow = displayLayer.g } function updateTokenLines (tokenLines, displayLayer, changes) { - for (const {start, oldExtent, newExtent} of changes || []) { - const newTokenLines = getTokens(displayLayer, start.row, start.row + newExtent.row) - tokenLines.splice(start.row, oldExtent.row, ...newTokenLines) + for (const {oldRange, newRange} of changes || []) { + const newTokenLines = getTokens(displayLayer, newRange.start.row, newRange.end.row) + tokenLines.splice(newRange.start.row, oldRange.end.row - oldRange.start.row, ...newTokenLines) } } diff --git a/spec/text-buffer-io-spec.js b/spec/text-buffer-io-spec.js index e7b6c43962..a41a7f4381 100644 --- a/spec/text-buffer-io-spec.js +++ b/spec/text-buffer-io-spec.js @@ -174,7 +174,7 @@ describe('TextBuffer IO', () => { buffer.reload().then(() => { expect(events).toEqual([ ['decoration-layer', {oldRange: Range(Point(0, 7), Point(0, 7)), newRange: Range(Point(0, 7), Point(0, 11))}], - ['display-layer', [{start: Point(0, 0), oldExtent: Point(1, 0), newExtent: Point(1, 0)}]] + ['display-layer', [{oldRange: Range(Point(0, 0), Point(1, 0)), newRange: Range(Point(0, 0), Point(1, 0))}]] ]) done() }) diff --git a/spec/text-buffer-spec.coffee b/spec/text-buffer-spec.coffee index 8d358c753c..7e3888517d 100644 --- a/spec/text-buffer-spec.coffee +++ b/spec/text-buffer-spec.coffee @@ -163,9 +163,8 @@ describe "TextBuffer", -> { source: 'display-layer-event', event: [{ - start: Point(0, 0), - oldExtent: Point(3, 0), - newExtent: Point(3, 0) + oldRange: Range(Point(0, 0), Point(3, 0)), + newRange: Range(Point(0, 0), Point(3, 0)) }] } ] diff --git a/src/display-layer.js b/src/display-layer.js index 2d14ce2e02..cadc063783 100644 --- a/src/display-layer.js +++ b/src/display-layer.js @@ -24,7 +24,7 @@ class DisplayLayer { this.textDecorationLayer = new EmptyDecorationLayer() this.displayMarkerLayersById = new Map() this.destroyed = false - this.deferredChangeEventPatch = new Patch() + this.changesSinceLastEvent = new Patch() this.invisibles = params.invisibles != null ? params.invisibles : {} this.tabLength = params.tabLength != null ? params.tabLength : 4 @@ -154,11 +154,7 @@ class DisplayLayer { const endRow = this.translateBufferPositionWithSpatialIndex(Point(endBufferRow, 0), 'backward').row const extent = Point(endRow - startRow, 0) spliceArray(this.cachedScreenLines, startRow, extent.row, new Array(extent.row)) - this.emitChangeEvent([{ - start: Point(startRow, 0), - oldExtent: extent, - newExtent: extent - }]) + this.didChange({start: Point(startRow, 0), oldExtent: extent, newExtent: extent}) }) } } @@ -208,9 +204,7 @@ class DisplayLayer { if (containingFoldMarkers.length === 0) { const foldStartRow = bufferRange.start.row const foldEndRow = bufferRange.end.row + 1 - this.emitChangeEvent([ - this.updateSpatialIndex(foldStartRow, foldEndRow, foldEndRow, Infinity) - ]) + this.didChange(this.updateSpatialIndex(foldStartRow, foldEndRow, foldEndRow, Infinity)) this.notifyObserversIfMarkerScreenPositionsChanged() } return foldId @@ -270,12 +264,12 @@ class DisplayLayer { foldMarker.destroy() } - this.emitChangeEvent([this.updateSpatialIndex( + this.didChange(this.updateSpatialIndex( combinedRangeStart.row, combinedRangeEnd.row + 1, combinedRangeEnd.row + 1, Infinity - )]) + )) this.notifyObserversIfMarkerScreenPositionsChanged() return foldedRanges @@ -808,10 +802,8 @@ class DisplayLayer { } } - const combinedChanges = new Patch() this.indexedBufferRowCount += newEndRow - oldEndRow - const {start, oldExtent, newExtent} = this.updateSpatialIndex(startRow, oldEndRow + 1, newEndRow + 1, Infinity) - combinedChanges.splice(start, oldExtent, newExtent) + this.didChange(this.updateSpatialIndex(startRow, oldEndRow + 1, newEndRow + 1, Infinity)) for (let bufferRange of this.textDecorationLayer.getInvalidatedRanges()) { bufferRange = Range.fromObject(bufferRange) @@ -822,36 +814,24 @@ class DisplayLayer { const endRow = this.translateBufferPositionWithSpatialIndex(Point(endBufferRow, 0), 'backward').row const extent = Point(endRow - startRow, 0) spliceArray(this.cachedScreenLines, startRow, extent.row, new Array(extent.row)) - combinedChanges.splice(Point(startRow, 0), extent, extent) + this.didChange({start: Point(startRow, 0), oldExtent: extent, newExtent: extent}) } - - this.emitChangeEvent(combinedChanges.getChanges().map((hunk) => { - return { - start: Point.fromObject(hunk.newStart), - oldExtent: traversal(hunk.oldEnd, hunk.oldStart), - newExtent: traversal(hunk.newEnd, hunk.newStart) - } - })) } - emitChangeEvent (event) { - if (this.buffer.transactCallDepth === 0) { - this.emitter.emit('did-change', event) - } else { - for (const change of event) { - this.deferredChangeEventPatch.splice(change.start, change.oldExtent, change.newExtent) - } - } + didChange ({start, oldExtent, newExtent}) { + this.changesSinceLastEvent.splice(start, oldExtent, newExtent) + if (this.buffer.transactCallDepth === 0) this.emitDeferredChangeEvents() } emitDeferredChangeEvents () { - if (this.deferredChangeEventPatch.getChangeCount() > 0) { - this.emitter.emit('did-change', this.deferredChangeEventPatch.getChanges().map((change) => ({ - start: change.newStart, - oldExtent: traversal(change.oldEnd, change.oldStart), - newExtent: traversal(change.newEnd, change.newStart) - }))) - this.deferredChangeEventPatch = new Patch() + if (this.changesSinceLastEvent.getChangeCount() > 0) { + this.emitter.emit('did-change', this.changesSinceLastEvent.getChanges().map((change) => { + return { + oldRange: new Range(change.oldStart, change.oldEnd), + newRange: new Range(change.newStart, change.newEnd) + } + })) + this.changesSinceLastEvent = new Patch() } } From ea313af408581ca20f527686346b5301c92f2b0b Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Mon, 30 Oct 2017 17:06:18 -0700 Subject: [PATCH 8/8] 13.8.0-display-layer-change-event-2 --- package-lock.json | 2 +- package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/package-lock.json b/package-lock.json index 8099de6ecd..8867982ecc 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "text-buffer", - "version": "13.8.0-display-layer-change-event-1", + "version": "13.8.0-display-layer-change-event-2", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index 41b0e13230..34dc635409 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "text-buffer", - "version": "13.8.0-display-layer-change-event-1", + "version": "13.8.0-display-layer-change-event-2", "description": "A container for large mutable strings with annotated regions", "main": "./lib/text-buffer", "scripts": {