Reduce observation to single observeMarker method on display buffer

It will call the observer callbacks with the buffer and screen positions of the head and tell each time any of these values is changed.
2026-01-23 22:08:08 -05:00 · 2013-02-04 21:24:06 -07:00
parent 58450d6a65
commit afe0a6570a
7 changed files with 167 additions and 142 deletions
--- a/spec/app/buffer-spec.coffee
+++ b/spec/app/buffer-spec.coffee
@@ -666,7 +666,7 @@ describe 'Buffer', ->
      expect(buffer.positionForCharacterIndex(61)).toEqual [2, 0]
      expect(buffer.positionForCharacterIndex(408)).toEqual [12, 2]

-  describe "markers", ->
+  fdescribe "markers", ->
    describe "marker creation", ->
      it "allows markers to be created with ranges and positions", ->
        marker1 = buffer.markRange([[4, 20], [4, 23]])
--- a/spec/app/display-buffer-spec.coffee
+++ b/spec/app/display-buffer-spec.coffee
@@ -590,7 +590,7 @@ describe "DisplayBuffer", ->
    it "returns the length of the longest screen line", ->
      expect(displayBuffer.maxLineLength()).toBe 65

-  describe "markers", ->
+  fdescribe "markers", ->
    beforeEach ->
      displayBuffer.foldBufferRow(4)

@@ -613,107 +613,113 @@ describe "DisplayBuffer", ->
        expect(displayBuffer.isMarkerReversed(marker)).toBeTruthy()
        expect(displayBuffer.getMarkerBufferRange(marker)).toEqual [[5, 4], [8, 4]]

-    describe "marker observation", ->
-      observeHandler = null
+    describe ".observeMarker(marker, callback)", ->
+      [observeHandler, marker, subscription] = []

      beforeEach ->
        observeHandler = jasmine.createSpy("observeHandler")
+        marker = displayBuffer.markScreenRange([[5, 4], [5, 10]])
+        subscription = displayBuffer.observeMarker(marker, observeHandler)

-      describe ".observeMarkerHeadPosition(marker, callback)", ->
-        it "calls the callback whenever the markers head's screen position changes with the new position and whether it was precipitated by a buffer change", ->
-          marker = displayBuffer.markScreenRange([[5, 4], [5, 10]])
-          displayBuffer.observeMarkerHeadPosition(marker, observeHandler)
-          displayBuffer.setMarkerHeadScreenPosition(marker, [8, 20])
-          expect(observeHandler).toHaveBeenCalled()
-          expect(observeHandler.argsForCall[0][0]).toEqual {
-            oldScreenPosition: [5, 10]
-            newScreenPosition: [8, 20]
-            oldBufferPosition: [8, 10]
-            newBufferPosition: [11, 20]
-            bufferChanged: false
-          }
-          observeHandler.reset()
+      it "calls the callback whenever the markers head's screen position changes in the buffer or on screen", ->
+        displayBuffer.setMarkerHeadScreenPosition(marker, [8, 20])
+        expect(observeHandler).toHaveBeenCalled()
+        expect(observeHandler.argsForCall[0][0]).toEqual {
+          oldHeadScreenPosition: [5, 10]
+          oldHeadBufferPosition: [8, 10]
+          newHeadScreenPosition: [8, 20]
+          newHeadBufferPosition: [11, 20]
+          oldTailScreenPosition: [5, 4]
+          oldTailBufferPosition: [8, 4]
+          newTailScreenPosition: [5, 4]
+          newTailBufferPosition: [8, 4]
+          bufferChanged: false
+        }
+        observeHandler.reset()

-          buffer.insert([11, 0], '...')
-          expect(observeHandler).toHaveBeenCalled()
-          expect(observeHandler.argsForCall[0][0]).toEqual {
-            oldScreenPosition: [8, 20]
-            newScreenPosition: [8, 23]
-            oldBufferPosition: [11, 20]
-            newBufferPosition: [11, 23]
-            bufferChanged: true
-          }
-          observeHandler.reset()
+        buffer.insert([11, 0], '...')
+        expect(observeHandler).toHaveBeenCalled()
+        expect(observeHandler.argsForCall[0][0]).toEqual {
+          oldHeadScreenPosition: [8, 20]
+          oldHeadBufferPosition: [11, 20]
+          newHeadScreenPosition: [8, 23]
+          newHeadBufferPosition: [11, 23]
+          oldTailScreenPosition: [5, 4]
+          oldTailBufferPosition: [8, 4]
+          newTailScreenPosition: [5, 4]
+          newTailBufferPosition: [8, 4]
+          bufferChanged: true
+        }
+        observeHandler.reset()

-          displayBuffer.unfoldBufferRow(4)
-          expect(observeHandler).toHaveBeenCalled()
-          expect(observeHandler.argsForCall[0][0]).toEqual {
-            oldScreenPosition: [8, 23]
-            newScreenPosition: [11, 23]
-            oldBufferPosition: [11, 23]
-            newBufferPosition: [11, 23]
-            bufferChanged: false
-          }
-          observeHandler.reset()
+        displayBuffer.unfoldBufferRow(4)
+        expect(observeHandler).toHaveBeenCalled()
+        expect(observeHandler.argsForCall[0][0]).toEqual {
+          oldHeadScreenPosition: [8, 23]
+          oldHeadBufferPosition: [11, 23]
+          newHeadScreenPosition: [11, 23]
+          newHeadBufferPosition: [11, 23]
+          oldTailScreenPosition: [5, 4]
+          oldTailBufferPosition: [8, 4]
+          newTailScreenPosition: [8, 4]
+          newTailBufferPosition: [8, 4]
+          bufferChanged: false
+        }
+        observeHandler.reset()

-          displayBuffer.foldBufferRow(4)
-          expect(observeHandler).toHaveBeenCalled()
-          expect(observeHandler.argsForCall[0][0]).toEqual {
-            oldScreenPosition: [11, 23]
-            newScreenPosition: [8, 23]
-            oldBufferPosition: [11, 23]
-            newBufferPosition: [11, 23]
-            bufferChanged: false
-          }
+        displayBuffer.foldBufferRow(4)
+        expect(observeHandler).toHaveBeenCalled()
+        expect(observeHandler.argsForCall[0][0]).toEqual {
+          oldHeadScreenPosition: [11, 23]
+          oldHeadBufferPosition: [11, 23]
+          newHeadScreenPosition: [8, 23]
+          newHeadBufferPosition: [11, 23]
+          oldTailScreenPosition: [8, 4]
+          oldTailBufferPosition: [8, 4]
+          newTailScreenPosition: [5, 4]
+          newTailBufferPosition: [8, 4]
+          bufferChanged: false
+        }

-        it "does not call the callback for screen changes that don't change the position of the marker", ->
-          marker = displayBuffer.markScreenPosition([3, 4])
-          displayBuffer.observeMarkerHeadPosition(marker, observeHandler)
+      it "calls the callback whenever the marker tail's position changes in the buffer or on screen", ->
+        displayBuffer.setMarkerTailScreenPosition(marker, [8, 20])
+        expect(observeHandler).toHaveBeenCalled()
+        expect(observeHandler.argsForCall[0][0]).toEqual {
+          oldHeadScreenPosition: [5, 10]
+          oldHeadBufferPosition: [8, 10]
+          newHeadScreenPosition: [5, 10]
+          newHeadBufferPosition: [8, 10]
+          oldTailScreenPosition: [5, 4]
+          oldTailBufferPosition: [8, 4]
+          newTailScreenPosition: [8, 20]
+          newTailBufferPosition: [11, 20]
+          bufferChanged: false
+        }
+        observeHandler.reset()

-          buffer.insert([3, 0], '...')
-          expect(observeHandler).toHaveBeenCalled()
-          expect(observeHandler.argsForCall[0][0]).toEqual {
-            oldScreenPosition: [3, 4]
-            newScreenPosition: [3, 7]
-            oldBufferPosition: [3, 4]
-            newBufferPosition: [3, 7]
-            bufferChanged: true
-          }
-          observeHandler.reset()
+        buffer.insert([11, 0], '...')
+        expect(observeHandler).toHaveBeenCalled()
+        expect(observeHandler.argsForCall[0][0]).toEqual {
+          oldHeadScreenPosition: [5, 10]
+          oldHeadBufferPosition: [8, 10]
+          newHeadScreenPosition: [5, 10]
+          newHeadBufferPosition: [8, 10]
+          oldTailScreenPosition: [8, 20]
+          oldTailBufferPosition: [11, 20]
+          newTailScreenPosition: [8, 23]
+          newTailBufferPosition: [11, 23]
+          bufferChanged: true
+        }

-          displayBuffer.unfoldBufferRow(4)
-          expect(observeHandler).not.toHaveBeenCalled()
+      it "does not call the callback for screen changes that don't change the position of the marker", ->
+        displayBuffer.createFold(10, 11)
+        expect(observeHandler).not.toHaveBeenCalled()

-          fold = displayBuffer.createFold(0, 2)
-          expect(observeHandler).toHaveBeenCalled()
-          expect(observeHandler.argsForCall[0][0]).toEqual {
-            oldScreenPosition: [3, 7]
-            newScreenPosition: [1, 7]
-            oldBufferPosition: [3, 7]
-            newBufferPosition: [3, 7]
-            bufferChanged: false
-          }
-          observeHandler.reset()
-
-          fold.destroy()
-          expect(observeHandler).toHaveBeenCalled()
-          expect(observeHandler.argsForCall[0][0]).toEqual {
-            oldScreenPosition: [1, 7]
-            newScreenPosition: [3, 7]
-            oldBufferPosition: [3, 7]
-            newBufferPosition: [3, 7]
-            bufferChanged: false
-          }
-
-        it "allows observation subscriptions to be cancelled", ->
-          marker = displayBuffer.markScreenRange([[5, 4], [5, 10]])
-          subscription = displayBuffer.observeMarkerHeadPosition(marker, observeHandler)
-          subscription.cancel()
-          buffer.insert([11, 0], '...')
-          expect(observeHandler).not.toHaveBeenCalled()
-
-          displayBuffer.unfoldBufferRow(4)
-          expect(observeHandler).not.toHaveBeenCalled()
+      it "allows observation subscriptions to be cancelled", ->
+        subscription.cancel()
+        displayBuffer.setMarkerHeadScreenPosition(marker, [8, 20])
+        displayBuffer.unfoldBufferRow(4)
+        expect(observeHandler).not.toHaveBeenCalled()

    describe "marker destruction", ->
      it "allows markers to be destroyed", ->
--- a/src/app/cursor.coffee
+++ b/src/app/cursor.coffee
@@ -12,7 +12,7 @@ class Cursor
  needsAutoscroll: null

  constructor: ({@editSession, @marker}) ->
-    @editSession.observeMarkerHeadPosition @marker, (e) =>
+    @editSession.observeMarker @marker, (e) =>
      @needsAutoscroll ?= @isLastCursor() and !e.bufferChanged
      @trigger 'moved', e
      @editSession.trigger 'cursor-moved', e
--- a/src/app/display-buffer-marker.coffee
+++ b/src/app/display-buffer-marker.coffee
@@ -1,7 +1,13 @@
+Range = require 'range'
 _ = require 'underscore'

 module.exports =
 class DisplayBufferMarker
+  observers: null
+  bufferMarkerSubscription: null
+  previousHeadScreenPosition: null
+  previousTailScreenPosition: null
+
  constructor: ({@id, @displayBuffer}) ->
    @buffer = @displayBuffer.buffer

@@ -18,7 +24,7 @@ class DisplayBufferMarker
    @buffer.setMarkerRange(@id, bufferRange, options)

  getHeadScreenPosition: ->
-    @headScreenPosition ?= @displayBuffer.screenPositionForBufferPosition(@getHeadBufferPosition(), wrapAtSoftNewlines: true)
+    @displayBuffer.screenPositionForBufferPosition(@getHeadBufferPosition(), wrapAtSoftNewlines: true)

  setHeadScreenPosition: (screenPosition, options) ->
    screenPosition = @displayBuffer.clipScreenPosition(screenPosition, options)
@@ -31,7 +37,8 @@ class DisplayBufferMarker
    @buffer.setMarkerHeadPosition(@id, bufferPosition)

  getTailScreenPosition: ->
-    @getMarker(@id).getTailScreenPosition()
+    if tailBufferPosition = @getTailBufferPosition()
+      @displayBuffer.screenPositionForBufferPosition(tailBufferPosition, wrapAtSoftNewlines: true)

  setTailScreenPosition: (screenPosition, options) ->
    screenPosition = @displayBuffer.clipScreenPosition(screenPosition, options)
@@ -49,44 +56,60 @@ class DisplayBufferMarker
  clearTail: ->
    @buffer.clearMarkerTail(@id)

-  observeHeadPosition: (callback) ->
-    unless @headObservers
-      @observeBufferMarkerHeadPosition()
-      @displayBuffer.markers[@id] = this
-      @headObservers = []
-    @headObservers.push(callback)
-    cancel: => @unobserveHeadPosition(callback)
+  observe: (callback) ->
+    @observeBufferMarkerIfNeeded()
+    @observers.push(callback)
+    cancel: => @unobserve(callback)

-  unobserveHeadPosition: (callback) ->
-    _.remove(@headObservers, callback)
-    @unsubscribe() unless @headObservers.length
+  unobserve: (callback) ->
+    _.remove(@observers, callback)
+    @unobserveBufferMarkerIfNeeded()

-  observeBufferMarkerHeadPosition: ->
-    @getHeadScreenPosition()
-    @bufferMarkerHeadSubscription =
-      @buffer.observeMarkerHeadPosition @id, (e) =>
-        bufferChanged = e.bufferChanged
-        oldBufferPosition = e.oldPosition
-        newBufferPosition = e.newPosition
-        @refreshHeadScreenPosition({bufferChanged, oldBufferPosition, newBufferPosition})
+  observeBufferMarkerIfNeeded: ->
+    return if @observers
+    @observers = []
+    @previousHeadScreenPosition = @getHeadScreenPosition()
+    @previousTailScreenPosition = @getTailScreenPosition()
+    @bufferMarkerSubscription =
+      @buffer.observeMarker @id, ({oldHeadPosition, newHeadPosition, oldTailPosition, newTailPosition, bufferChanged}) =>
+        @notifyObservers
+          oldHeadBufferPosition: oldHeadPosition
+          newHeadBufferPosition: newHeadPosition
+          oldTailBufferPosition: oldTailPosition
+          newTailBufferPosition: newTailPosition
+          bufferChanged: bufferChanged
+    @displayBuffer.markers[@id] = this

-  refreshHeadScreenPosition: ({bufferChanged, oldBufferPosition, newBufferPosition}={}) ->
-    unless bufferChanged
-      oldBufferPosition ?= @getHeadBufferPosition()
-      newBufferPosition ?= oldBufferPosition
-    oldScreenPosition = @getHeadScreenPosition()
-    @headScreenPosition = null
-    newScreenPosition = @getHeadScreenPosition()
-
-    unless newScreenPosition.isEqual(oldScreenPosition)
-      @notifyHeadObservers({ oldBufferPosition, newBufferPosition, oldScreenPosition, newScreenPosition, bufferChanged })
-
-  notifyHeadObservers: (event) ->
-    observer(event) for observer in @getHeadObservers()
-
-  getHeadObservers: ->
-    new Array(@headObservers...)
-
-  unsubscribe: ->
-    @bufferMarkerHeadSubscription.cancel()
+  unobserveBufferMarkerIfNeeded: ->
+    return if @observers.length
+    @observers = null
+    @bufferMarkerSubscription.cancel()
    delete @displayBuffer.markers[@id]
+
+  notifyObservers: ({oldHeadBufferPosition, oldTailBufferPosition, bufferChanged}) ->
+    oldHeadScreenPosition = @previousHeadScreenPosition
+    newHeadScreenPosition = @getHeadScreenPosition()
+    @previousHeadScreenPosition = newHeadScreenPosition
+
+    oldTailScreenPosition = @previousTailScreenPosition
+    newTailScreenPosition = @getTailScreenPosition()
+    @previousTailScreenPosition = newTailScreenPosition
+
+    return if _.isEqual(newHeadScreenPosition, oldHeadScreenPosition) and _.isEqual(newTailScreenPosition, oldTailScreenPosition)
+
+    oldHeadBufferPosition ?= @getHeadBufferPosition()
+    newHeadBufferPosition = @getHeadBufferPosition()
+    oldTailBufferPosition ?= @getTailBufferPosition()
+    newTailBufferPosition = @getTailBufferPosition()
+
+    for observer in @getObservers()
+      observer({
+        oldHeadScreenPosition, newHeadScreenPosition,
+        oldTailScreenPosition, newTailScreenPosition,
+        oldHeadBufferPosition, newHeadBufferPosition,
+        oldTailBufferPosition, newTailBufferPosition,
+        bufferChanged
+      })
+
+  getObservers: ->
+    new Array(@observers...)
--- a/src/app/display-buffer.coffee
+++ b/src/app/display-buffer.coffee
@@ -369,12 +369,12 @@ class DisplayBuffer
  isMarkerReversed: (id) ->
    @buffer.isMarkerReversed(id)

-  observeMarkerHeadPosition: (id, callback) ->
-    @getMarker(id).observeHeadPosition(callback)
+  observeMarker: (id, callback) ->
+    @getMarker(id).observe(callback)

  refreshMarkerScreenPositions: ->
    for marker in @getMarkers()
-      marker.refreshHeadScreenPosition(bufferChanged: false)
+      marker.notifyObservers(bufferChanged: false)

  destroy: ->
    @tokenizedBuffer.destroy()
--- a/src/app/edit-session.coffee
+++ b/src/app/edit-session.coffee
@@ -489,8 +489,8 @@ class EditSession
  setMarkerTailBufferPosition: (args...) ->
    @displayBuffer.setMarkerTailBufferPosition(args...)

-  observeMarkerHeadPosition: (args...) ->
-    @displayBuffer.observeMarkerHeadPosition(args...)
+  observeMarker: (args...) ->
+    @displayBuffer.observeMarker(args...)

  placeMarkerTail: (args...) ->
    @displayBuffer.placeMarkerTail(args...)
--- a/src/app/selection.coffee
+++ b/src/app/selection.coffee
@@ -10,10 +10,7 @@ class Selection

  constructor: ({@cursor, @marker, @editSession}) ->
    @cursor.selection = this
-
-    @cursor.on 'moved.selection', ({bufferChange}) =>
-      @screenRangeChanged() unless bufferChange
-
+    @editSession.observeMarker @marker, => @screenRangeChanged()
    @cursor.on 'destroyed.selection', =>
      @cursor = null
      @destroy()
@@ -78,7 +75,6 @@ class Selection

  clear: ->
    @editSession.clearMarkerTail(@marker)
-    @screenRangeChanged()

  selectWord: ->
    options = {}