Reopen closed pane items in workspace – by uri instead of deserializing

Serializing and deserializing closed pane items was turning into a pain
with the new telepath changes that are phasing out this kind of on the
fly serialization. Reopening by uri saves memory and simplifies panes
enormously. If we want non-uri reopening we can revisit it later when
telepath models are better integrated.

spec/pane-container-spec.coffee

@@ -88,73 +88,6 @@ describe "PaneContainer", ->
       pane4.splitDown()
       expect(panes).toEqual []
 
-  describe ".reopenItem()", ->
-    describe "when there is an active pane", ->
-      it "reconstructs and shows the last-closed pane item", ->
-        expect(container.getActivePane()).toBe pane3
-        item3 = pane3.activeItem
-        item4 = new TestView('4')
-        pane3.showItem(item4)
-
-        pane3.destroyItem(item3)
-        pane3.destroyItem(item4)
-        expect(container.getActivePane()).toBe pane1
-
-        expect(container.reopenItem()).toBeTruthy()
-        expect(pane1.activeItem).toEqual item4
-
-        expect(container.reopenItem()).toBeTruthy()
-        expect(pane1.activeItem).toEqual item3
-
-        expect(container.reopenItem()).toBeFalsy()
-        expect(pane1.activeItem).toEqual item3
-
-      describe "when the last-closed pane item is an edit session", ->
-        it "reopens the edit session (regression)", ->
-          editor = atom.project.openSync('sample.js')
-          pane3.showItem(editor)
-          pane3.destroyItem(editor)
-          expect(container.reopenItem()).toBeTruthy()
-          expect(pane3.activeItem.getPath()).toBe editor.getPath()
-          expect(container.reopenItem()).toBeFalsy()
-
-    describe "when there is no active pane", ->
-      it "attaches a new pane with the reconstructed last pane item and focuses it", ->
-        container.attachToDom()
-        pane1.remove()
-        pane2.remove()
-        item3 = pane3.activeItem
-        pane3.destroyItem(item3)
-        expect(container.getActivePane()).toBeUndefined()
-
-        container.reopenItem()
-
-        expect(container.getActivePane().activeItem).toEqual item3
-        expect(container.getActivePane().activeView).toMatchSelector ':focus'
-
-    it "does not reopen an item that is already open", ->
-      item3 = pane3.activeItem
-      item4 = new TestView('4')
-      pane3.showItem(item4)
-      pane3.destroyItem(item3)
-      pane3.destroyItem(item4)
-
-      expect(container.getActivePane()).toBe pane1
-      pane1.showItem(new TestView('4'))
-
-      expect(container.reopenItem()).toBeTruthy()
-      expect(_.pluck(pane1.getItems(), 'name')).toEqual ['1', '4', '3']
-      expect(pane1.activeItem).toEqual item3
-
-      expect(container.reopenItem()).toBeFalsy()
-      expect(pane1.activeItem).toEqual item3
-
-      pane1.destroyItem(item3)
-      container.setRoot(new Pane(item3))
-      expect(container.reopenItem()).toBeFalsy()
-      expect(container.getActivePane().getItems().length).toBe 1
-      expect(container.getActivePaneItem()).toEqual item3
-
   describe ".saveAll()", ->
     it "saves all open pane items", ->
       pane1.showItem(new TestView('4'))

spec/workspace-view-spec.coffee

@@ -520,3 +520,36 @@ describe "WorkspaceView", ->
       subscription.off()
       atom.workspaceView.getActiveView().splitRight()
       expect(count).toBe 2
+
+  describe ".reopenItemSync()", ->
+    it "opens the uri associated with the last closed pane that isn't currently open", ->
+      workspace = atom.workspaceView
+      pane = workspace.getActivePane()
+      workspace.openSync('b')
+      workspace.openSync('file1')
+      workspace.openSync()
+
+      # does not reopen items with no uri
+      expect(workspace.getActivePaneItem().getUri()).toBeUndefined()
+      pane.destroyActiveItem()
+      workspace.reopenItemSync()
+      expect(workspace.getActivePaneItem().getUri()).not.toBeUndefined()
+
+      # destroy all items
+      expect(workspace.getActivePaneItem().getUri()).toBe 'a'
+      pane.destroyActiveItem()
+      expect(workspace.getActivePaneItem().getUri()).toBe 'b'
+      pane.destroyActiveItem()
+      expect(workspace.getActivePaneItem().getUri()).toBe 'file1'
+      pane.destroyActiveItem()
+
+      # reopens items with uris
+      expect(workspace.getActivePaneItem()).toBeUndefined()
+      workspace.reopenItemSync()
+      expect(workspace.getActivePaneItem().getUri()).toBe 'file1'
+
+      # does not reopen items that are already open
+      workspace.openSync('b')
+      expect(workspace.getActivePaneItem().getUri()).toBe 'b'
+      workspace.reopenItemSync()
+      expect(workspace.getActivePaneItem().getUri()).toBe 'a'

src/pane-container.coffee

@@ -19,8 +19,6 @@ class PaneContainer extends View
     @div class: 'panes'
 
   initialize: (state) ->
-    @destroyedItemStates = []
-
     if state instanceof telepath.Document
       @state = state
       @setRoot(atom.deserializers.deserialize(@state.get('root')))
@@ -86,25 +84,8 @@ class PaneContainer extends View
     nextIndex = (currentIndex + 1) % panes.length
     panes[nextIndex].makeActive()
 
-  reopenItem: ->
-    if lastItemState = @destroyedItemStates.pop()
-      if activePane = @getActivePane()
-        activePane.showItem(atom.deserializers.deserialize(lastItemState))
-        true
-      else
-        newPane = new Pane(atom.deserializers.deserialize(lastItemState))
-        @setRoot(newPane)
-        newPane.focus()
-
   itemDestroyed: (item) ->
-    if state = item.serialize?()
-      state.uri ?= item.getUri?()
-      @destroyedItemStates.push(state)
-
-  itemAdded: (item) ->
-    itemUri = item.getUri?()
-    @destroyedItemStates = @destroyedItemStates.filter (itemState) ->
-      itemState.uri isnt itemUri
+    @trigger 'item-destroyed', item
 
   getRoot: ->
     @children().first().view()
@@ -113,7 +94,6 @@ class PaneContainer extends View
     @empty()
     if root?
       @append(root)
-      @itemAdded(root.activeItem) if root.activeItem?
       root.makeActive?()
     @state.set(root: root?.getState())
 

src/pane.coffee

@@ -185,7 +185,6 @@ class Pane extends View
 
     @state.get('items').splice(index, 0, item.getState?() ? item.serialize()) if options.updateState ? true
     @items.splice(index, 0, item)
-    @getContainer()?.itemAdded(item)
     @trigger 'pane:item-added', [item, index]
     item
 

src/workspace-view.coffee

@@ -79,6 +79,10 @@ class WorkspaceView extends View
 
     @panes.replaceWith(panes)
     @panes = panes
+
+    @destroyedItemUris = []
+    @subscribe @panes, 'item-destroyed', @onPaneItemDestroyed
+
     @updateTitle()
 
     @on 'focus', (e) => @handleFocus(e)
@@ -124,8 +128,7 @@ class WorkspaceView extends View
     @command 'window:toggle-auto-indent', =>
       atom.config.toggle("editor.autoIndent")
 
-    @command 'pane:reopen-closed-item', =>
-      @panes.reopenItem()
+    @command 'pane:reopen-closed-item', => @reopenItemSync()
 
   # Private:
   serialize: ->
@@ -182,6 +185,7 @@ class WorkspaceView extends View
           activePane = new Pane(editor)
           @panes.setRoot(activePane)
 
+        @itemOpened(editor)
         activePane.showItem(editor)
         activePane.focus() if changeFocus
         @trigger "uri-opened"
@@ -216,6 +220,8 @@ class WorkspaceView extends View
       pane = new Pane(paneItem)
       @panes.setRoot(pane)
 
+    @itemOpened(paneItem)
+
     pane.focus() if changeFocus
     paneItem
 
@@ -317,3 +323,15 @@ class WorkspaceView extends View
     editorView.remove() for editorView in @getEditorViews()
     atom.project?.destroy()
     super
+
+  onPaneItemDestroyed: (e, item) =>
+    if uri = item.getUri?()
+      @destroyedItemUris.push(uri)
+
+  reopenItemSync: ->
+    if uri = @destroyedItemUris.pop()
+      @openSync(uri)
+
+  itemOpened: (item) ->
+    if uri = item.getUri?()
+      _.remove(@destroyedItemUris, uri)