diff --git a/spec/display-marker-layer-spec.coffee b/spec/display-marker-layer-spec.coffee index 4a886c7689..0b741daebe 100644 --- a/spec/display-marker-layer-spec.coffee +++ b/spec/display-marker-layer-spec.coffee @@ -131,24 +131,46 @@ describe "DisplayMarkerLayer", -> marker.destroy() expect(destroyEventCount).toBe 1 - it "emits update events when markers are created, updated directly, updated indirectly, or destroyed", (done) -> - buffer = new TextBuffer(text: 'hello world') + it "emits update events when markers are created, updated directly, updated indirectly, or destroyed", -> + buffer = new TextBuffer(text: 'hello world\nhow are you?') displayLayer = buffer.addDisplayLayer(tabLength: 4) markerLayer = displayLayer.addMarkerLayer() - updateEventCount = 0 - markerLayer.onDidUpdate -> - updateEventCount++ - if updateEventCount is 1 - marker.setScreenRange([[0, 5], [1, 0]]) - else if updateEventCount is 2 - buffer.insert([0, 0], '\t') - else if updateEventCount is 3 - marker.destroy() - else if updateEventCount is 4 - done() - - marker = markerLayer.markScreenRange([[0, 4], [1, 4]]) + events = [] + markerLayer.onDidUpdate (event) -> events.push({ + created: Array.from(event.created), + updated: Array.from(event.updated), + touched: Array.from(event.touched), + destroyed: Array.from(event.destroyed) + }) + + events = [] + marker = markerLayer.markScreenRange([[0, 4], [1, 4]], {invalidate: 'inside'}) + expect(events).toEqual([ + {created: [marker.id], updated: [], touched: [], destroyed: []} + ]) + + events = [] + marker.setScreenRange([[0, 5], [1, 0]]) + expect(events).toEqual([ + {created: [], updated: [marker.id], touched: [], destroyed: []} + ]) + + events = [] + buffer.insert([0, 8], 'foo') + expect(events).toEqual([ + {created: [], updated: [], touched: [marker.id], destroyed: []} + ]) + + events = [] + buffer.insert([0, 0], '\t') + expect(events).toEqual([]) + + events = [] + marker.destroy() + expect(events).toEqual([ + {created: [], updated: [], touched: [], destroyed: [marker.id]} + ]) it "allows markers to be copied", -> buffer = new TextBuffer(text: '\ta\tbc\tdef\tg\n\th') diff --git a/spec/marker-layer-spec.coffee b/spec/marker-layer-spec.coffee index 9a2df528a9..6dc9427961 100644 --- a/spec/marker-layer-spec.coffee +++ b/spec/marker-layer-spec.coffee @@ -6,7 +6,14 @@ describe "MarkerLayer", -> beforeEach -> jasmine.addCustomEqualityTester(require("underscore-plus").isEqual) - buffer = new TextBuffer(text: "abcdefghijklmnopqrstuvwxyz") + buffer = new TextBuffer(text: """ + Lorem ipsum dolor sit amet, + consectetur adipisicing elit, + sed do eiusmod tempor incididunt + ut labore et dolore magna aliqua. + Ut enim ad minim veniam, quis + nostrud exercitation ullamco laboris. + """) layer1 = buffer.addMarkerLayer() layer2 = buffer.addMarkerLayer() @@ -68,6 +75,26 @@ describe "MarkerLayer", -> expect(createEventCount).toBe 1 expect(updateEventCount).toBe 2 + describe "::getLastMarker()", -> + it "returns the last (in terms of time) non-destroyed marker added to the layer", -> + marker1 = layer1.markRange([[0, 0], [0, 3]]) + marker2 = layer1.markRange([[0, 2], [0, 6]]) + marker3 = layer1.markRange([[0, 8], [0, 10]]) + marker4 = layer1.markRange([[1, 0], [1, 10]]) + expect(layer1.getLastMarker()).toBe(marker4) + + marker4.destroy() + expect(layer1.getLastMarker()).toBe(marker3) + + marker1.destroy() + expect(layer1.getLastMarker()).toBe(marker3) + + marker3.destroy() + expect(layer1.getLastMarker()).toBe(marker2) + + marker2.destroy() + expect(layer1.getLastMarker()).toBeFalsy() + describe "when destroyInvalidatedMarkers is enabled for the layer", -> it "destroys markers when they are invalidated via a splice", -> layer3 = buffer.addMarkerLayer(destroyInvalidatedMarkers: true) @@ -169,24 +196,94 @@ describe "MarkerLayer", -> expect(layer2.findMarkers(containsPoint: [0, 4])).toEqual [layer2Marker] describe "::onDidUpdate", -> - it "notifies observers asynchronously when markers are created, updated, or destroyed", (done) -> - updateCount = 0 - layer1.onDidUpdate -> - updateCount++ - if updateCount is 1 - marker1.setRange([[1, 2], [3, 4]]) - marker2.setRange([[4, 5], [6, 7]]) - else if updateCount is 2 - buffer.insert([0, 1], "xxx") - buffer.insert([0, 1], "yyy") - else if updateCount is 3 - marker1.destroy() - marker2.destroy() - else if updateCount is 4 - done() - - marker1 = layer1.markRange([[0, 2], [0, 4]]) - marker2 = layer1.markRange([[0, 6], [0, 8]]) + it "notifies observers synchronously or at the end of a transaction when markers are created, updated directly, updated indirectly, or destroyed", -> + layer = buffer.addMarkerLayer({maintainHistory: true}) + events = [] + [marker1, marker2, marker3] = [] + layer.onDidUpdate (event) -> events.push({ + created: Array.from(event.created), + updated: Array.from(event.updated), + touched: Array.from(event.touched), + destroyed: Array.from(event.destroyed) + }) + + buffer.transact -> + marker1 = layer.markRange([[0, 2], [0, 4]]) + marker2 = layer.markRange([[0, 6], [0, 8]]) + expect(events.length).toBe(0) + + marker3 = layer.markRange([[4, 0], [4, 5]]) + + expect(events).toEqual([ + {created: [marker1.id, marker2.id], updated: [], touched: [], destroyed: []}, + {created: [marker3.id], updated: [], touched: [], destroyed: []} + ]) + + events = [] + buffer.transact -> + marker1.setRange([[1, 2], [3, 4]]) + marker2.setRange([[3, 10], [4, 5]]) + marker3.destroy() + + expect(events).toEqual([ + {created: [], updated: [marker1.id, marker2.id], touched: [], destroyed: [marker3.id]} + ]) + + events = [] + buffer.transact -> + buffer.insert([1, 3], "xxx") + buffer.insert([2, 0], "yyy") + buffer.insert([1, 5], 'zzz') + + expect(events).toEqual([ + {created: [], updated: [], touched: [marker1.id], destroyed: []}, + {created: [], updated: [], touched: [marker1.id], destroyed: []} + ]) + + events = [] + buffer.undo() + buffer.undo() + + expect(events).toEqual([ + {created: [], updated: [], touched: [marker1.id], destroyed: []}, + {created: [], updated: [], touched: [marker1.id], destroyed: []} + ]) + + events = [] + buffer.transact -> + buffer.insert([1, 3], 'aaa') + buffer.insert([3, 11], 'bbb') + buffer.transact -> + buffer.insert([1, 9], 'ccc') + buffer.insert([1, 12], 'ddd') + buffer.insert([4, 0], 'eee') + buffer.insert([4, 3], 'fff') + + expect(events).toEqual([ + {created: [], updated: [], touched: [marker1.id, marker2.id], destroyed: []}, + {created: [], updated: [], touched: [marker2.id], destroyed: []} + ]) + + events = [] + buffer.transact -> + buffer.insert([3, 11], 'ggg') + buffer.undo() + marker1.clearTail() + marker2.clearTail() + + expect(events).toEqual([ + {created: [], updated: [], touched: [marker2.id], destroyed: []}, + {created: [], updated: [marker1.id, marker2.id], touched: [], destroyed: []} + ]) + + events = [] + buffer.transact -> + marker1.destroy() + marker2.destroy() + + expect(events).toEqual([ + {created: [], updated: [], touched: [], destroyed: [marker1.id, marker2.id]} + ]) describe "::copy", -> it "creates a new marker layer with markers in the same states", -> diff --git a/src/display-marker-layer.coffee b/src/display-marker-layer.coffee index e1dc18aba6..1597364cd0 100644 --- a/src/display-marker-layer.coffee +++ b/src/display-marker-layer.coffee @@ -34,19 +34,21 @@ class DisplayMarkerLayer onDidDestroy: (callback) -> @emitter.on('did-destroy', callback) - # Public: Subscribe to be notified asynchronously whenever markers are - # created, updated, or destroyed on this layer. *Prefer this method for - # optimal performance when interacting with layers that could contain large - # numbers of markers.* + # Public: Subscribe to be notified whenever markers are created, updated, + # touched (moved because of a textual change), or destroyed on this layer. + # *Prefer this method for optimal performance when interacting with layers + # that could contain large numbers of markers.* # # * `callback` A {Function} that will be called with no arguments when changes # occur on this layer. # - # Subscribers are notified once, asynchronously when any number of changes - # occur in a given tick of the event loop. You should re-query the layer - # to determine the state of markers in which you're interested in. It may - # be counter-intuitive, but this is much more efficient than subscribing to - # events on individual markers, which are expensive to deliver. + # Subscribers are notified once when any number of changes occur in this + # {MarkerLayer}. The notification gets scheduled either at the end of a + # transaction, or synchronously when a marker changes and no transaction is + # present. You should re-query the layer to determine the state of markers in + # which you're interested in: it may be counter-intuitive, but this is much + # more efficient than subscribing to events on individual markers, which are + # expensive to deliver. # # Returns a {Disposable}. onDidUpdate: (callback) -> @@ -234,6 +236,12 @@ class DisplayMarkerLayer getMarkers: -> @bufferMarkerLayer.getMarkers().map ({id}) => @getMarker(id) + # Public: Get the last (in terms of time) non-destroyed marker added to this layer. + # + # Returns a {DisplayMarker}. + getLastMarker: -> + @getMarker(@bufferMarkerLayer.getLastMarker()?.id) + # Public: Get the number of markers in the marker layer. # # Returns a {Number}. @@ -291,8 +299,8 @@ class DisplayMarkerLayer translateScreenRange: (screenRange, options) -> @displayLayer.translateScreenRange(screenRange, options) - emitDidUpdate: -> - @emitter.emit('did-update') + emitDidUpdate: (event) -> + @emitter.emit('did-update', event) emitDidDestroy: -> @emitter.emit('did-destroy') diff --git a/src/marker-layer.coffee b/src/marker-layer.coffee index ce07e2f787..0a79e3cf0a 100644 --- a/src/marker-layer.coffee +++ b/src/marker-layer.coffee @@ -37,8 +37,14 @@ class MarkerLayer @persistent = options?.persistent ? false @emitter = new Emitter @index = new MarkerIndex - @markersById = {} + @markersById = new Map + @markers = [] @markersIdsWithChangeSubscriptions = new Set + @createdMarkers = new Set + @destroyedMarkers = new Set + @updatedMarkers = new Set + @touchedMarkers = new Set + @setDisableDidUpdateEvent(false) @destroyed = false @emitCreateMarkerEvents = false @@ -46,7 +52,7 @@ class MarkerLayer # locations. copy: -> copy = @delegate.addMarkerLayer({@maintainHistory}) - for markerId, marker of @markersById + @markersById.forEach (marker, id) -> snapshot = marker.getSnapshot(null) copy.createMarker(marker.getRange(), marker.getSnapshot()) copy @@ -73,19 +79,27 @@ class MarkerLayer # # Returns a {Marker}. getMarker: (id) -> - @markersById[id] + @markersById.get(id) + + # Public: Get the last (in terms of time) non-destroyed marker added to this layer. + # + # Returns a {Marker}. + getLastMarker: -> + @getMarker(@markers[@markers.length - 1]) # Public: Get all existing markers on the marker layer. # # Returns an {Array} of {Marker}s. getMarkers: -> - marker for id, marker of @markersById + markers = [] + @markersById.forEach (marker) -> markers.push(marker) + markers # Public: Get the number of markers in the marker layer. # # Returns a {Number}. getMarkerCount: -> - Object.keys(@markersById).length + @markersById.size # Public: Find markers in the layer conforming to the given parameters. # @@ -124,13 +138,17 @@ class MarkerLayer continue delete params[key] - markerIds ?= new Set(Object.keys(@markersById)) - result = [] - markerIds.forEach (markerId) => - marker = @markersById[markerId] - return unless marker.matchesParams(params) - result.push(marker) + if markerIds? + markerIds.forEach (markerId) => + marker = @markersById.get(markerId) + if marker.matchesParams(params) + result.push(marker) + else + @markersById.forEach (marker) -> + if marker.matchesParams(params) + result.push(marker) + result.sort (a, b) -> a.compare(b) ### @@ -203,19 +221,21 @@ class MarkerLayer Section: Event subscription ### - # Public: Subscribe to be notified asynchronously whenever markers are - # created, updated, or destroyed on this layer. *Prefer this method for - # optimal performance when interacting with layers that could contain large - # numbers of markers.* + # Public: Subscribe to be notified whenever markers are created, updated, + # touched (moved because of a textual change), or destroyed on this layer. + # *Prefer this method for optimal performance when interacting with layers + # that could contain large numbers of markers.* # # * `callback` A {Function} that will be called with no arguments when changes # occur on this layer. # - # Subscribers are notified once, asynchronously when any number of changes - # occur in a given tick of the event loop. You should re-query the layer - # to determine the state of markers in which you're interested in. It may - # be counter-intuitive, but this is much more efficient than subscribing to - # events on individual markers, which are expensive to deliver. + # Subscribers are notified once when any number of changes occur in this + # {MarkerLayer}. The notification gets scheduled either at the end of a + # transaction, or synchronously when a marker changes and no transaction is + # present. You should re-query the layer to determine the state of markers in + # which you're interested in: it may be counter-intuitive, but this is much + # more efficient than subscribing to events on individual markers, which are + # expensive to deliver. # # Returns a {Disposable}. onDidUpdate: (callback) -> @@ -249,29 +269,30 @@ class MarkerLayer splice: (start, oldExtent, newExtent) -> invalidated = @index.splice(start, oldExtent, newExtent) invalidated.touch.forEach (id) => - marker = @markersById[id] + marker = @markersById.get(id) + @touchedMarkers.add(id) if invalidated[marker.getInvalidationStrategy()]?.has(id) if @destroyInvalidatedMarkers marker.destroy() else marker.valid = false - @scheduleUpdateEvent() + @emitDidUpdateEvent() restoreFromSnapshot: (snapshots) -> return unless snapshots? snapshotIds = Object.keys(snapshots) - existingMarkerIds = Object.keys(@markersById) + existingMarkerIds = Array.from(@markersById.keys()) for id in snapshotIds snapshot = snapshots[id] - if marker = @markersById[id] + if marker = @markersById.get(parseInt(id)) marker.update(marker.getRange(), snapshot, true) else newMarker = @createMarker(snapshot.range, snapshot) for id in existingMarkerIds - if (marker = @markersById[id]) and (not snapshots[id]?) + if (marker = @markersById.get(parseInt(id))) and (not snapshots[id]?) marker.destroy() @delegate.markersUpdated(this) @@ -279,22 +300,20 @@ class MarkerLayer createSnapshot: -> result = {} ranges = @index.dump() - for id in Object.keys(@markersById) - marker = @markersById[id] + @markersById.forEach (marker, id) -> result[id] = marker.getSnapshot(Range.fromObject(ranges[id]), false) result emitChangeEvents: (snapshot) -> @markersIdsWithChangeSubscriptions.forEach (id) => - if marker = @markersById[id] # event handlers could destroy markers + if marker = @markersById.get(id) # event handlers could destroy markers marker.emitChangeEvent(snapshot?[id]?.range, true, false) @delegate.markersUpdated(this) serialize: -> ranges = @index.dump() markersById = {} - for id in Object.keys(@markersById) - marker = @markersById[id] + @markersById.forEach (marker, id) -> markersById[id] = marker.getSnapshot(Range.fromObject(ranges[id]), false) {@id, @maintainHistory, @persistent, markersById, version: SerializationVersion} @@ -306,24 +325,28 @@ class MarkerLayer for id, markerState of state.markersById range = Range.fromObject(markerState.range) delete markerState.range - @addMarker(id, range, markerState) + @addMarker(parseInt(id), range, markerState) return ### Section: Private - Marker interface ### - markerUpdated: -> + markerUpdated: (id) -> + @updatedMarkers.add(id) + @emitDidUpdateEvent() @delegate.markersUpdated(this) - @scheduleUpdateEvent() destroyMarker: (id) -> - if @markersById.hasOwnProperty(id) - delete @markersById[id] + if @markersById.has(id) + @markersById.delete(id) + index = @indexForMarkerId(id) + @markers.splice(index, 1) if index isnt -1 @markersIdsWithChangeSubscriptions.delete(id) @index.delete(id) + @destroyedMarkers.add(id) + @emitDidUpdateEvent() @delegate.markersUpdated(this) - @scheduleUpdateEvent() getMarkerRange: (id) -> Range.fromObject(@index.getRange(id)) @@ -350,10 +373,11 @@ class MarkerLayer createMarker: (range, params) -> id = @delegate.getNextMarkerId() marker = @addMarker(id, range, params) + @createdMarkers.add(id) + @emitDidUpdateEvent() + @emitter.emit 'did-create-marker', marker if @emitCreateMarkerEvents @delegate.markerCreated(this, marker) @delegate.markersUpdated(this) - @scheduleUpdateEvent() - @emitter.emit 'did-create-marker', marker if @emitCreateMarkerEvents marker ### @@ -364,14 +388,36 @@ class MarkerLayer Point.assertValid(range.start) Point.assertValid(range.end) @index.insert(id, range.start, range.end) - @markersById[id] = new Marker(id, this, range, params) - - scheduleUpdateEvent: -> - unless @didUpdateEventScheduled - @didUpdateEventScheduled = true - process.nextTick => - @didUpdateEventScheduled = false - @emitter.emit 'did-update' + marker = new Marker(id, this, range, params) + @markersById.set(id, marker) + @markers.push(id) + marker + + indexForMarkerId: (id) -> + low = 0 + high = @markers.length - 1 + while low <= high + index = low + ((high - low) >> 1) + if id < @markers[index] + high = index - 1 + else if id is @markers[index] + return index + else + low = index + 1 + -1 + + setDisableDidUpdateEvent: (@didUpdateEventDisabled) -> + + emitDidUpdateEvent: -> + return if @didUpdateEventDisabled + + if @createdMarkers.size > 0 or @destroyedMarkers.size > 0 or @touchedMarkers.size > 0 or @updatedMarkers.size > 0 + event = {created: @createdMarkers, destroyed: @destroyedMarkers, touched: @touchedMarkers, updated: @updatedMarkers} + @createdMarkers = new Set + @destroyedMarkers = new Set + @touchedMarkers = new Set + @updatedMarkers = new Set + @emitter.emit 'did-update', event filterSet = (set1, set2) -> if set1 diff --git a/src/marker.coffee b/src/marker.coffee index 2a8dd5650a..a6f05b78c3 100644 --- a/src/marker.coffee +++ b/src/marker.coffee @@ -103,6 +103,11 @@ class Marker # Returns a {Disposable} on which `.dispose()` can be called to unsubscribe. onDidChange: (callback) -> unless @hasChangeObservers + Grim.deprecate(""" + Subscribing to marker change events is deprecated. Please, consider using + `MarkerLayer.prototype.onDidUpdate` instead. + """) + @previousEventState = @getSnapshot(@getRange()) @hasChangeObservers = true @layer.markersIdsWithChangeSubscriptions.add(@id) @@ -370,7 +375,7 @@ class Marker updated = true @emitChangeEvent(range ? oldRange, textChanged, propertiesChanged) - @layer.markerUpdated() if updated and not textChanged + @layer.markerUpdated(@id) if updated and not textChanged updated getSnapshot: (range) -> diff --git a/src/text-buffer.coffee b/src/text-buffer.coffee index 2238f82835..45e32669f8 100644 --- a/src/text-buffer.coffee +++ b/src/text-buffer.coffee @@ -108,6 +108,7 @@ class TextBuffer @markerLayers = params?.markerLayers ? {} @markerLayers[@defaultMarkerLayer.id] = @defaultMarkerLayer @nextMarkerId = params?.nextMarkerId ? 1 + @markerLayerDidUpdateEventDisabled = false @setEncoding(params?.encoding) @setPreferredLineEnding(params?.preferredLineEnding) @@ -963,9 +964,12 @@ class TextBuffer # Public: Undo the last operation. If a transaction is in progress, aborts it. undo: -> if pop = @history.popUndoStack() + markerLayerUpdateEventWasDisabled = @disableMarkerLayersDidUpdateEvent() @applyChange(change) for change in pop.patch.getChanges() @restoreFromMarkerSnapshot(pop.snapshot) + @enableMarkersLayersDidUpdateEvent() @emitMarkerChangeEvents(pop.snapshot) + @disableMarkerLayersDidUpdateEvent() if markerLayerUpdateEventWasDisabled @emitDidChangeTextEvent(pop.patch) true else @@ -974,9 +978,12 @@ class TextBuffer # Public: Redo the last operation redo: -> if pop = @history.popRedoStack() + markerLayerUpdateEventWasDisabled = @disableMarkerLayersDidUpdateEvent() @applyChange(change) for change in pop.patch.getChanges() @restoreFromMarkerSnapshot(pop.snapshot) + @enableMarkersLayersDidUpdateEvent() @emitMarkerChangeEvents(pop.snapshot) + @disableMarkerLayersDidUpdateEvent() if markerLayerUpdateEventWasDisabled @emitDidChangeTextEvent(pop.patch) true else @@ -1000,6 +1007,7 @@ class TextBuffer fn = groupingInterval groupingInterval = 0 + markerLayerUpdateEventWasDisabled = @disableMarkerLayersDidUpdateEvent() checkpointBefore = @history.createCheckpoint(@createMarkerSnapshot(), true) try @@ -1018,7 +1026,9 @@ class TextBuffer error.metadata = {history: @history.toString()} @history.applyGroupingInterval(groupingInterval) @history.enforceUndoStackSizeLimit() + @enableMarkersLayersDidUpdateEvent() @emitMarkerChangeEvents(endMarkerSnapshot) + @disableMarkerLayersDidUpdateEvent() if markerLayerUpdateEventWasDisabled @emitDidChangeTextEvent(compactedChanges) if compactedChanges result @@ -1048,9 +1058,12 @@ class TextBuffer # Returns a {Boolean} indicating whether the operation succeeded. revertToCheckpoint: (checkpoint) -> if truncated = @history.truncateUndoStack(checkpoint) + markerLayerUpdateEventWasDisabled = @disableMarkerLayersDidUpdateEvent() @applyChange(change) for change in truncated.patch.getChanges() @restoreFromMarkerSnapshot(truncated.snapshot) - @emitter.emit 'did-update-markers' + @enableMarkersLayersDidUpdateEvent() + @emitMarkerChangeEvents(truncated.snapshot) + @disableMarkerLayersDidUpdateEvent() if markerLayerUpdateEventWasDisabled @emitDidChangeTextEvent(truncated.patch) true else @@ -1542,8 +1555,21 @@ class TextBuffer emitMarkerChangeEvents: (snapshot) -> for markerLayerId, markerLayer of @markerLayers + markerLayer.emitDidUpdateEvent() markerLayer.emitChangeEvents(snapshot?[markerLayerId]) + disableMarkerLayersDidUpdateEvent: -> + wasDisabled = @markerLayerDidUpdateEventDisabled + @markerLayerDidUpdateEventDisabled = true + for id, markerLayer of @markerLayers + markerLayer.setDisableDidUpdateEvent(true) + wasDisabled + + enableMarkersLayersDidUpdateEvent: -> + @markerLayerDidUpdateEventDisabled = false + for id, markerLayer of @markerLayers + markerLayer.setDisableDidUpdateEvent(false) + emitDidChangeTextEvent: (patch) -> return if @transactCallDepth isnt 0