From 0bd98bf8f89b6423bc93db5455e361ec11b430f1 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Wed, 6 Feb 2013 16:54:19 -0700 Subject: [PATCH] Do a better job cleaning up after Editors and EditSessions - EditSessions destroy their Selections when they are destroyed - Editors destroy their EditSessions when they are destroyed - Editors unsubscribe from the document and window when they are removed from the DOM. - When an EditSession is destroyed via any code path, the Editor with that EditSession removes it. - Selections no longer trigger 'destroyed' events if their parent EditSession has already been destroyed. These are all really intertwined, so I'm doing them as one commit since that was the only way to keep the specs green. --- spec/app/editor-spec.coffee | 4 +- src/app/edit-session.coffee | 5 ++- src/app/editor.coffee | 77 ++++++++++++++++++++----------------- src/app/selection.coffee | 2 +- 4 files changed, 49 insertions(+), 39 deletions(-) diff --git a/spec/app/editor-spec.coffee b/spec/app/editor-spec.coffee index 454b59378..a9f020799 100644 --- a/spec/app/editor-spec.coffee +++ b/spec/app/editor-spec.coffee @@ -39,8 +39,9 @@ describe "Editor", -> rootView.remove() describe "construction", -> - it "throws an error if no editor session is given", -> + it "throws an error if no editor session is given unless deserializing", -> expect(-> new Editor).toThrow() + expect(-> new Editor(deserializing: true)).not.toThrow() describe ".copy()", -> it "builds a new editor with the same edit sessions, cursor position, and scroll position as the receiver", -> @@ -70,7 +71,6 @@ describe "Editor", -> newEditor.height(editor.height()) newEditor.width(editor.width()) - editor.remove() newEditor.attachToDom() expect(newEditor.scrollTop()).toBe editor.scrollTop() expect(newEditor.scrollView.scrollLeft()).toBe 44 diff --git a/src/app/edit-session.coffee b/src/app/edit-session.coffee index 57849e627..d0a87ff26 100644 --- a/src/app/edit-session.coffee +++ b/src/app/edit-session.coffee @@ -55,8 +55,11 @@ class EditSession @destroyed = true @unsubscribe() @buffer.release() + selection.destroy() for selection in @getSelections() @displayBuffer.destroy() - @project.removeEditSession(this) + @project?.removeEditSession(this) + @trigger 'destroyed' + @off() serialize: -> buffer: @buffer.getPath() diff --git a/src/app/editor.coffee b/src/app/editor.coffee index 4dcd2909b..89ad35924 100644 --- a/src/app/editor.coffee +++ b/src/app/editor.coffee @@ -20,6 +20,8 @@ class Editor extends View autoIndentOnPaste: false nonWordCharacters: "./\\()\"'-_:,.;<>~!@#$%^&*|+=[]{}`~?" + @nextEditorId: 1 + @content: (params) -> @div class: @classes(params), tabindex: -1, => @subview 'gutter', new Gutter @@ -54,15 +56,17 @@ class Editor extends View newSelections: null @deserialize: (state, rootView) -> + editor = new Editor(mini: state.mini, deserializing: true) editSessions = state.editSessions.map (state) -> EditSession.deserialize(state, rootView.project) - editor = new Editor(editSession: editSessions[state.activeEditSessionIndex], mini: state.mini) - editor.editSessions = editSessions + editor.pushEditSession(editSession) for editSession in editSessions + editor.setActiveEditSessionIndex(state.activeEditSessionIndex) editor.isFocused = state.isFocused editor - initialize: ({editSession, @mini} = {}) -> + initialize: ({editSession, @mini, deserializing} = {}) -> requireStylesheet 'editor.css' + @id = Editor.nextEditorId++ @lineCache = [] @configure() @bindKeys() @@ -75,19 +79,16 @@ class Editor extends View @newSelections = [] if editSession? - @editSessions.push editSession - @setActiveEditSessionIndex(0) + @edit(editSession) else if @mini - editSession = new EditSession + @edit(new EditSession buffer: new Buffer() softWrap: false tabLength: 2 softTabs: true - - @editSessions.push editSession - @setActiveEditSessionIndex(0) - else - throw new Error("Editor initialization requires an editSession") + ) + else if not deserializing + throw new Error("Editor must be constructed with an 'editSession' or 'mini: true' param") serialize: -> @saveScrollPositionForActiveEditSession() @@ -431,10 +432,10 @@ class Editor extends View @selectToScreenPosition(@screenPositionFromMouseEvent(event)) lastMoveEvent = event - $(document).on 'mousemove', moveHandler + $(document).on "mousemove.editor-#{@id}", moveHandler interval = setInterval(moveHandler, 20) - $(document).one 'mouseup', => + $(document).one "mouseup.editor-#{@id}", => clearInterval(interval) $(document).off 'mousemove', moveHandler reverse = @activeEditSession.getLastSelection().isReversed() @@ -448,7 +449,7 @@ class Editor extends View @calculateDimensions() @hiddenInput.width(@charWidth) @setSoftWrapColumn() if @activeEditSession.getSoftWrap() - @subscribe $(window), "resize", => @requestDisplayUpdate() + @subscribe $(window), "resize.editor-#{@id}", => @requestDisplayUpdate() @focus() if @isFocused @resetDisplay() @@ -457,14 +458,16 @@ class Editor extends View edit: (editSession) -> index = @editSessions.indexOf(editSession) - - if index == -1 - index = @editSessions.length - @editSessions.push(editSession) - @trigger 'editor:edit-session-added', [editSession, index] - + index = @pushEditSession(editSession) if index == -1 @setActiveEditSessionIndex(index) + pushEditSession: (editSession) -> + index = @editSessions.length + @editSessions.push(editSession) + editSession.on 'destroyed', => @editSessionDestroyed(editSession) + @trigger 'editor:edit-session-added', [editSession, index] + index + getBuffer: -> @activeEditSession.buffer destroyActiveEditSession: -> @@ -474,23 +477,18 @@ class Editor extends View return if @mini editSession = @editSessions[index] - destroySession = => - if index is @getActiveEditSessionIndex() and @editSessions.length > 1 - @loadPreviousEditSession() - _.remove(@editSessions, editSession) + destroySession = -> editSession.destroy() - @trigger 'editor:edit-session-removed', [editSession, index] - @remove() if @editSessions.length is 0 - callback(index) if callback + callback?(index) if editSession.isModified() and not editSession.hasEditors() @promptToSaveDirtySession(editSession, destroySession) else - destroySession(editSession) + destroySession() destroyInactiveEditSessions: -> destroyIndex = (index) => - index++ if @activeEditSession is @editSessions[index] + index++ if index is @getActiveEditSessionIndex() @destroyEditSessionIndex(index, destroyIndex) if @editSessions[index] destroyIndex(0) @@ -499,6 +497,13 @@ class Editor extends View @destroyEditSessionIndex(index, destroyIndex) if @editSessions[index] destroyIndex(0) + editSessionDestroyed: (editSession) -> + index = @editSessions.indexOf(editSession) + @loadPreviousEditSession() if index is @getActiveEditSessionIndex() and @editSessions.length > 1 + _.remove(@editSessions, editSession) + @trigger 'editor:edit-session-removed', [editSession, index] + @remove() if @editSessions.length is 0 + loadNextEditSession: -> nextIndex = (@getActiveEditSessionIndex() + 1) % @editSessions.length @setActiveEditSessionIndex(nextIndex) @@ -663,7 +668,7 @@ class Editor extends View if @activeEditSession.getSoftWrap() @addClass 'soft-wrap' @_setSoftWrapColumn = => @setSoftWrapColumn() - $(window).on "resize", @_setSoftWrapColumn + $(window).on "resize.editor-#{@id}", @_setSoftWrapColumn else @removeClass 'soft-wrap' $(window).off 'resize', @_setSoftWrapColumn @@ -750,15 +755,17 @@ class Editor extends View ) remove: (selector, keepData) -> - return super if keepData - + return super if keepData or @removed @trigger 'editor:will-be-removed' - - @destroyEditSessions() - if @pane() then @pane().remove() else super rootView?.focus() + afterRemove: -> + @removed = true + @destroyEditSessions() + $(window).off(".editor-#{@id}") + $(document).off(".editor-#{@id}") + getEditSessions: -> new Array(@editSessions...) diff --git a/src/app/selection.coffee b/src/app/selection.coffee index 0967056c0..81102dc0f 100644 --- a/src/app/selection.coffee +++ b/src/app/selection.coffee @@ -19,7 +19,7 @@ class Selection return if @destroyed @destroyed = true @editSession.removeSelection(this) - @trigger 'destroyed' + @trigger 'destroyed' unless @editSession.destroyed @cursor?.destroy() finalize: ->