From afe0a6570aa7ab4bf2e69bfba8d44a9acb08dae4 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Mon, 4 Feb 2013 21:24:06 -0700 Subject: [PATCH] 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. --- spec/app/buffer-spec.coffee | 2 +- spec/app/display-buffer-spec.coffee | 188 ++++++++++++++------------- src/app/cursor.coffee | 2 +- src/app/display-buffer-marker.coffee | 101 ++++++++------ src/app/display-buffer.coffee | 6 +- src/app/edit-session.coffee | 4 +- src/app/selection.coffee | 6 +- 7 files changed, 167 insertions(+), 142 deletions(-) diff --git a/spec/app/buffer-spec.coffee b/spec/app/buffer-spec.coffee index a858a7bfc..e30fa6c77 100644 --- 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]]) diff --git a/spec/app/display-buffer-spec.coffee b/spec/app/display-buffer-spec.coffee index 78be67cb5..554a01845 100644 --- 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", -> diff --git a/src/app/cursor.coffee b/src/app/cursor.coffee index 8d219fb4c..9ae00ba63 100644 --- 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 diff --git a/src/app/display-buffer-marker.coffee b/src/app/display-buffer-marker.coffee index 95bcf1307..3ebde889e 100644 --- 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...) diff --git a/src/app/display-buffer.coffee b/src/app/display-buffer.coffee index 53778afe3..84090f052 100644 --- 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() diff --git a/src/app/edit-session.coffee b/src/app/edit-session.coffee index 92480ee43..87a00c7f4 100644 --- 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...) diff --git a/src/app/selection.coffee b/src/app/selection.coffee index 1ee467f7c..f114dca1d 100644 --- 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 = {}