From 42b203d502f8bc41d4d0aedfd58fdbbe2de881fe Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Wed, 18 Dec 2013 17:02:40 -0700 Subject: [PATCH] :non-potable_water: Fix mini editor leak Previously, we were overriding Editor::destroy, which is now provided by telepath. Since the real destroy wasn't being called, we were failing to remove editors associated with mini editor views. --- package.json | 2 +- spec/editor-view-spec.coffee | 2 +- spec/pane-spec.coffee | 22 +++++++++++----------- src/editor-view.coffee | 2 +- src/editor.coffee | 4 +--- src/pane.coffee | 6 +++--- src/selection.coffee | 2 +- 7 files changed, 19 insertions(+), 21 deletions(-) diff --git a/package.json b/package.json index 7dfb043cf..0905fd28a 100644 --- a/package.json +++ b/package.json @@ -44,7 +44,7 @@ "season": "0.14.0", "semver": "1.1.4", "space-pen": "2.0.2", - "telepath": "0.80.0", + "telepath": "0.81.0", "temp": "0.5.0", "underscore-plus": "0.6.1" }, diff --git a/spec/editor-view-spec.coffee b/spec/editor-view-spec.coffee index bd4f71dcf..96d10e98c 100644 --- a/spec/editor-view-spec.coffee +++ b/spec/editor-view-spec.coffee @@ -123,7 +123,7 @@ describe "EditorView", -> describe ".remove()", -> it "destroys the edit session", -> editorView.remove() - expect(editorView.editor.destroyed).toBeTruthy() + expect(editorView.editor.isDestroyed()).toBe true describe ".edit(editor)", -> [newEditor, newBuffer] = [] diff --git a/spec/pane-spec.coffee b/spec/pane-spec.coffee index 356013b4b..540603bf5 100644 --- a/spec/pane-spec.coffee +++ b/spec/pane-spec.coffee @@ -143,7 +143,7 @@ describe "Pane", -> it "removes the item and tries to call destroy on it", -> pane.destroyItem(editor2) expect(pane.getItems().indexOf(editor2)).toBe -1 - expect(editor2.destroyed).toBeTruthy() + expect(editor2.isDestroyed()).toBe true describe "if the item is modified", -> beforeEach -> @@ -162,7 +162,7 @@ describe "Pane", -> expect(editor2.save).toHaveBeenCalled() expect(pane.getItems().indexOf(editor2)).toBe -1 - expect(editor2.destroyed).toBeTruthy() + expect(editor2.isDestroyed()).toBe true describe "when the item has no uri", -> it "presents a save-as dialog, then saves the item with the given uri before removing and destroying it", -> @@ -176,7 +176,7 @@ describe "Pane", -> expect(editor2.saveAs).toHaveBeenCalledWith("/selected/path") expect(pane.getItems().indexOf(editor2)).toBe -1 - expect(editor2.destroyed).toBeTruthy() + expect(editor2.isDestroyed()).toBe true describe "if the [Don't Save] option is selected", -> it "removes and destroys the item without saving it", -> @@ -185,7 +185,7 @@ describe "Pane", -> expect(editor2.save).not.toHaveBeenCalled() expect(pane.getItems().indexOf(editor2)).toBe -1 - expect(editor2.destroyed).toBeTruthy() + expect(editor2.isDestroyed()).toBe true describe "if the [Cancel] option is selected", -> it "does not save, remove, or destroy the item", -> @@ -194,7 +194,7 @@ describe "Pane", -> expect(editor2.save).not.toHaveBeenCalled() expect(pane.getItems().indexOf(editor2)).not.toBe -1 - expect(editor2.destroyed).toBeFalsy() + expect(editor2.isDestroyed()).toBe false describe "::removeItem(item)", -> it "removes the item from the items list and shows the next item if it was showing", -> @@ -290,7 +290,7 @@ describe "Pane", -> expect(pane.hasParent()).toBeFalsy() expect(pane2.getItems()).toEqual [view3, editor1] - expect(editor1.destroyed).toBeFalsy() + expect(editor1.isDestroyed()).toBe false describe "when the item is a jQuery object", -> it "preserves data by detaching instead of removing", -> @@ -304,14 +304,14 @@ describe "Pane", -> pane.showItem(editor1) pane.trigger 'pane:close' expect(pane.hasParent()).toBeFalsy() - expect(editor2.destroyed).toBeTruthy() - expect(editor1.destroyed).toBeTruthy() + expect(editor2.isDestroyed()).toBe true + expect(editor1.isDestroyed()).toBe true describe "pane:close-other-items", -> it "destroys all items except the current", -> pane.showItem(editor1) pane.trigger 'pane:close-other-items' - expect(editor2.destroyed).toBeTruthy() + expect(editor2.isDestroyed()).toBe true expect(pane.getItems()).toEqual [editor1] describe "::saveActiveItem()", -> @@ -423,8 +423,8 @@ describe "Pane", -> describe "::remove()", -> it "destroys all the pane's items", -> pane.remove() - expect(editor1.destroyed).toBeTruthy() - expect(editor2.destroyed).toBeTruthy() + expect(editor1.isDestroyed()).toBe true + expect(editor2.isDestroyed()).toBe true it "triggers a 'pane:removed' event with the pane", -> removedHandler = jasmine.createSpy("removedHandler") diff --git a/src/editor-view.coffee b/src/editor-view.coffee index 67a5d3ce7..daf326146 100644 --- a/src/editor-view.coffee +++ b/src/editor-view.coffee @@ -1229,7 +1229,7 @@ class EditorView extends View updateDisplay: (options={}) -> return unless @attached and @editor - return if @editor.destroyed + return if @editor.isDestroyed() unless @isOnDom() and @isVisible() @redrawOnReattach = true return diff --git a/src/editor.coffee b/src/editor.coffee index f0ec6dcf5..1dfd071d2 100644 --- a/src/editor.coffee +++ b/src/editor.coffee @@ -126,9 +126,7 @@ class Editor extends Model require './editor-view' # Private: - destroy: -> - return if @destroyed - @destroyed = true + destroyed: -> @unsubscribe() selection.destroy() for selection in @getSelections() @buffer.release() diff --git a/src/pane.coffee b/src/pane.coffee index 0133b134a..3e2fb8643 100644 --- a/src/pane.coffee +++ b/src/pane.coffee @@ -192,7 +192,7 @@ class Pane extends View handleItemEvents: (item) -> if _.isFunction(item.on) @subscribe item, 'destroyed', => - @destroyItem(item) if @state.isAlive() + @destroyItem(item, updateState: false) if @state.isAlive() # Public: Remove the currently active item. destroyActiveItem: => @@ -200,13 +200,13 @@ class Pane extends View false # Public: Remove the specified item. - destroyItem: (item) -> + destroyItem: (item, options) -> @unsubscribe(item) if _.isFunction(item.off) @trigger 'pane:before-item-destroyed', [item] if @promptToSaveItem(item) @getContainer()?.itemDestroyed(item) - @removeItem(item) + @removeItem(item, options) item.destroy?() true else diff --git a/src/selection.coffee b/src/selection.coffee index 984d29849..4427f17cc 100644 --- a/src/selection.coffee +++ b/src/selection.coffee @@ -21,7 +21,7 @@ class Selection @marker.on 'destroyed', => @destroyed = true @editor.removeSelection(this) - @emit 'destroyed' unless @editor.destroyed + @emit 'destroyed' unless @editor.isDestroyed() # Private: destroy: ->