From f381abcbad2fbfd1c7025e16eb0caa9b025d726b Mon Sep 17 00:00:00 2001 From: Ben Ogle Date: Mon, 23 Jun 2014 16:00:36 -0700 Subject: [PATCH 1/3] Re-render when a marker changes fixes #2705 --- spec/editor-component-spec.coffee | 35 ++++++++++++++++++++----------- src/display-buffer.coffee | 31 +++++++++++++++++++++++---- src/editor-component.coffee | 1 + src/editor.coffee | 1 + 4 files changed, 52 insertions(+), 16 deletions(-) diff --git a/spec/editor-component-spec.coffee b/spec/editor-component-spec.coffee index 769cd5b71..5b6ca49ae 100644 --- a/spec/editor-component-spec.coffee +++ b/spec/editor-component-spec.coffee @@ -995,18 +995,6 @@ describe "EditorComponent", -> nextTick() expect(node.querySelectorAll('.test-highlight').length).toBe 0 - it "moves rendered highlights when the marker moves", -> - regionStyle = node.querySelector('.test-highlight .region').style - originalTop = parseInt(regionStyle.top) - - editor.getBuffer().insert([0, 0], '\n') - nextTick() - - regionStyle = node.querySelector('.test-highlight .region').style - newTop = parseInt(regionStyle.top) - - expect(newTop).toBe originalTop + lineHeightInPixels - it "removes highlights when a decoration's marker is destroyed", -> marker.destroy() nextTick() @@ -1028,6 +1016,29 @@ describe "EditorComponent", -> regions = node.querySelectorAll('.test-highlight .region') expect(regions.length).toBe 2 + describe "when a decoration's marker moves", -> + it "moves rendered highlights when the buffer is changed", -> + regionStyle = node.querySelector('.test-highlight .region').style + originalTop = parseInt(regionStyle.top) + + editor.getBuffer().insert([0, 0], '\n') + nextTick() + + regionStyle = node.querySelector('.test-highlight .region').style + newTop = parseInt(regionStyle.top) + + expect(newTop).toBe originalTop + lineHeightInPixels + + it "moves rendered highlights when the marker is manually moved", -> + regionStyle = node.querySelector('.test-highlight .region').style + expect(parseInt(regionStyle.top)).toBe 2 * lineHeightInPixels + + marker.setBufferRange([[5, 8], [5, 13]]) + nextTick() + + regionStyle = node.querySelector('.test-highlight .region').style + expect(parseInt(regionStyle.top)).toBe 5 * lineHeightInPixels + describe "hidden input field", -> it "renders the hidden input field at the position of the last cursor if the cursor is on screen and the editor is focused", -> editor.setVerticalScrollMargin(0) diff --git a/src/display-buffer.coffee b/src/display-buffer.coffee index 7a24d359d..574be4c9e 100644 --- a/src/display-buffer.coffee +++ b/src/display-buffer.coffee @@ -45,7 +45,8 @@ class DisplayBuffer extends Model @markers = {} @foldsByMarkerId = {} @decorationsByMarkerId = {} - @decorationMarkerSubscriptions = {} + @decorationMarkerChangedSubscriptions = {} + @decorationMarkerDestroyedSubscriptions = {} @updateAllScreenLines() @createFoldForMarker(marker) for marker in @buffer.findMarkers(@getFoldMarkerAttributes()) @subscribe @tokenizedBuffer, 'grammar-changed', (grammar) => @emit 'grammar-changed', grammar @@ -755,7 +756,11 @@ class DisplayBuffer extends Model return marker = @getMarker(marker.id) - @decorationMarkerSubscriptions[marker.id] ?= @subscribe marker, 'destroyed', => @removeAllDecorationsForMarker(marker) + @decorationMarkerDestroyedSubscriptions[marker.id] ?= @subscribe marker, 'destroyed', => @removeAllDecorationsForMarker(marker) + + subscription = @subscribe marker, 'changed', (event) => @emit('decoration-changed', marker, decoration, event) + @decorationMarkerChangedSubscriptions[marker.id] ?= [] + @decorationMarkerChangedSubscriptions[marker.id].push {decoration, subscription} @decorationsByMarkerId[marker.id] ?= [] @decorationsByMarkerId[marker.id].push(decoration) @@ -772,6 +777,7 @@ class DisplayBuffer extends Model for i in [decorations.length - 1..0] decoration = decorations[i] if @decorationMatchesPattern(decoration, decorationPattern) + @unsubscribeDecorationFromMarker(marker, decoration) decorations.splice(i, 1) @emit 'decoration-removed', marker, decoration @@ -784,9 +790,26 @@ class DisplayBuffer extends Model @removedAllMarkerDecorations(marker) removedAllMarkerDecorations: (marker) -> - @decorationMarkerSubscriptions[marker.id].off() + @decorationMarkerDestroyedSubscriptions[marker.id].off() + + for subscriptionObject in @decorationMarkerChangedSubscriptions[marker.id] + subscriptionObject.subscription.off() + delete @decorationsByMarkerId[marker.id] - delete @decorationMarkerSubscriptions[marker.id] + delete @decorationMarkerChangedSubscriptions[marker.id] + delete @decorationMarkerDestroyedSubscriptions[marker.id] + + unsubscribeDecorationFromMarker: (marker, decoration) -> + subscriptionObjects = @decorationMarkerChangedSubscriptions[marker.id] + return unless @decorationMarkerChangedSubscriptions[marker.id] + + for i in [subscriptionObjects.length - 1..0] + subscriptionObject = subscriptionObjects[i] + if subscriptionObject.decoration == decoration + subscriptionObject.subscription.off() + subscriptionObjects.splice(i, 1) + + delete @decorationMarkerChangedSubscriptions[marker.id] if subscriptionObjects.length == 0 # Retrieves a {DisplayBufferMarker} based on its id. # diff --git a/src/editor-component.coffee b/src/editor-component.coffee index f375fed9e..cd885d0ad 100644 --- a/src/editor-component.coffee +++ b/src/editor-component.coffee @@ -291,6 +291,7 @@ EditorComponent = React.createClass @subscribe editor, 'selection-added', @onSelectionAdded @subscribe editor, 'decoration-added', @onDecorationChanged @subscribe editor, 'decoration-removed', @onDecorationChanged + @subscribe editor, 'decoration-changed', @onDecorationChanged @subscribe editor, 'character-widths-changed', @onCharacterWidthsChanged @subscribe editor.$scrollTop.changes, @onScrollTopChanged @subscribe editor.$scrollLeft.changes, @requestUpdate diff --git a/src/editor.coffee b/src/editor.coffee index f5ad5666d..f1bdfeb77 100644 --- a/src/editor.coffee +++ b/src/editor.coffee @@ -217,6 +217,7 @@ class Editor extends Model @subscribe @displayBuffer, 'soft-wrap-changed', (args...) => @emit 'soft-wrap-changed', args... @subscribe @displayBuffer, "decoration-added", (args...) => @emit 'decoration-added', args... @subscribe @displayBuffer, "decoration-removed", (args...) => @emit 'decoration-removed', args... + @subscribe @displayBuffer, "decoration-changed", (args...) => @emit 'decoration-changed', args... @subscribe @displayBuffer, "character-widths-changed", (changeCount) => @emit 'character-widths-changed', changeCount getViewClass: -> From b4f4ef8ec4c8187a4cbadf4d7a2ff54d46cbe8c1 Mon Sep 17 00:00:00 2001 From: Ben Ogle Date: Mon, 23 Jun 2014 16:56:15 -0700 Subject: [PATCH 2/3] :lipstick: Make it not suck. --- src/display-buffer.coffee | 31 +++++++++---------------------- 1 file changed, 9 insertions(+), 22 deletions(-) diff --git a/src/display-buffer.coffee b/src/display-buffer.coffee index 574be4c9e..b6e2a3931 100644 --- a/src/display-buffer.coffee +++ b/src/display-buffer.coffee @@ -756,11 +756,14 @@ class DisplayBuffer extends Model return marker = @getMarker(marker.id) - @decorationMarkerDestroyedSubscriptions[marker.id] ?= @subscribe marker, 'destroyed', => @removeAllDecorationsForMarker(marker) - subscription = @subscribe marker, 'changed', (event) => @emit('decoration-changed', marker, decoration, event) - @decorationMarkerChangedSubscriptions[marker.id] ?= [] - @decorationMarkerChangedSubscriptions[marker.id].push {decoration, subscription} + @decorationMarkerDestroyedSubscriptions[marker.id] ?= @subscribe marker, 'destroyed', => + @removeAllDecorationsForMarker(marker) + + @decorationMarkerChangedSubscriptions[marker.id] ?= @subscribe marker, 'changed', (event) => + decorations = @decorationsByMarkerId[marker.id] + for decoration in decorations + @emit 'decoration-changed', marker, decoration, event @decorationsByMarkerId[marker.id] ?= [] @decorationsByMarkerId[marker.id].push(decoration) @@ -771,13 +774,11 @@ class DisplayBuffer extends Model console.warn 'A decoration cannot be removed from a null marker' return - return unless @decorationMarkerSubscriptions[marker.id]? + return unless decorations = @decorationsByMarkerId[marker.id] - decorations = @decorationsByMarkerId[marker.id] for i in [decorations.length - 1..0] decoration = decorations[i] if @decorationMatchesPattern(decoration, decorationPattern) - @unsubscribeDecorationFromMarker(marker, decoration) decorations.splice(i, 1) @emit 'decoration-removed', marker, decoration @@ -790,27 +791,13 @@ class DisplayBuffer extends Model @removedAllMarkerDecorations(marker) removedAllMarkerDecorations: (marker) -> + @decorationMarkerChangedSubscriptions[marker.id].off() @decorationMarkerDestroyedSubscriptions[marker.id].off() - for subscriptionObject in @decorationMarkerChangedSubscriptions[marker.id] - subscriptionObject.subscription.off() - delete @decorationsByMarkerId[marker.id] delete @decorationMarkerChangedSubscriptions[marker.id] delete @decorationMarkerDestroyedSubscriptions[marker.id] - unsubscribeDecorationFromMarker: (marker, decoration) -> - subscriptionObjects = @decorationMarkerChangedSubscriptions[marker.id] - return unless @decorationMarkerChangedSubscriptions[marker.id] - - for i in [subscriptionObjects.length - 1..0] - subscriptionObject = subscriptionObjects[i] - if subscriptionObject.decoration == decoration - subscriptionObject.subscription.off() - subscriptionObjects.splice(i, 1) - - delete @decorationMarkerChangedSubscriptions[marker.id] if subscriptionObjects.length == 0 - # Retrieves a {DisplayBufferMarker} based on its id. # # id - A {Number} representing a marker id From 5ebb17c2e8d800b1191ba12e21840fc4208e2012 Mon Sep 17 00:00:00 2001 From: Ben Ogle Date: Mon, 23 Jun 2014 18:03:19 -0700 Subject: [PATCH 3/3] Be defensive when iterating through decorations --- src/display-buffer.coffee | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/display-buffer.coffee b/src/display-buffer.coffee index b6e2a3931..7d926c6a1 100644 --- a/src/display-buffer.coffee +++ b/src/display-buffer.coffee @@ -762,8 +762,12 @@ class DisplayBuffer extends Model @decorationMarkerChangedSubscriptions[marker.id] ?= @subscribe marker, 'changed', (event) => decorations = @decorationsByMarkerId[marker.id] - for decoration in decorations - @emit 'decoration-changed', marker, decoration, event + + # Why check existence? Markers may get destroyed or decorations removed + # in the change handler. Bookmarks does this. + if decorations? + for decoration in decorations + @emit 'decoration-changed', marker, decoration, event @decorationsByMarkerId[marker.id] ?= [] @decorationsByMarkerId[marker.id].push(decoration)