Merge pull request #10959 from atom/ku-move-pending-state-from-item-to-pane

(WIP) Refactor pending state to live in pane instead of items
This commit is contained in:
Michelle Tilley
2016-02-26 14:19:23 -08:00
committed by Max Brunsfeld
parent 6a0c541cfb
commit 4cd9c08bc1
6 changed files with 145 additions and 114 deletions

View File

@@ -18,8 +18,8 @@ describe "Pane", ->
onDidDestroy: (fn) -> @emitter.on('did-destroy', fn)
destroy: -> @destroyed = true; @emitter.emit('did-destroy')
isDestroyed: -> @destroyed
isPending: -> @pending
pending: false
onDidTerminatePendingState: (callback) -> @emitter.on 'terminate-pending-state', callback
terminatePendingState: -> @emitter.emit 'terminate-pending-state'
beforeEach ->
confirm = spyOn(atom.applicationDelegate, 'confirm')
@@ -136,10 +136,8 @@ describe "Pane", ->
pane = new Pane(paneParams(items: []))
itemA = new Item("A")
itemB = new Item("B")
itemA.pending = true
itemB.pending = true
pane.addItem(itemA)
pane.addItem(itemB)
pane.addItem(itemA, undefined, false, true)
pane.addItem(itemB, undefined, false, true)
expect(itemA.isDestroyed()).toBe true
describe "::activateItem(item)", ->
@@ -172,19 +170,17 @@ describe "Pane", ->
beforeEach ->
itemC = new Item("C")
itemD = new Item("D")
itemC.pending = true
itemD.pending = true
it "replaces the active item if it is pending", ->
pane.activateItem(itemC)
pane.activateItem(itemC, true)
expect(pane.getItems().map (item) -> item.name).toEqual ['A', 'C', 'B']
pane.activateItem(itemD)
pane.activateItem(itemD, true)
expect(pane.getItems().map (item) -> item.name).toEqual ['A', 'D', 'B']
it "adds the item after the active item if it is not pending", ->
pane.activateItem(itemC)
pane.activateItem(itemC, true)
pane.activateItemAtIndex(2)
pane.activateItem(itemD)
pane.activateItem(itemD, true)
expect(pane.getItems().map (item) -> item.name).toEqual ['A', 'B', 'D']
describe "::activateNextItem() and ::activatePreviousItem()", ->
@@ -806,6 +802,67 @@ describe "Pane", ->
pane2.destroy()
expect(container.root).toBe pane1
describe "pending state", ->
editor1 = null
pane = null
eventCount = null
beforeEach ->
waitsForPromise ->
atom.workspace.open('sample.txt', pending: true).then (o) ->
editor1 = o
pane = atom.workspace.getActivePane()
runs ->
eventCount = 0
editor1.onDidTerminatePendingState -> eventCount++
it "does not open file in pending state by default", ->
waitsForPromise ->
atom.workspace.open('sample.js').then (o) ->
editor1 = o
pane = atom.workspace.getActivePane()
runs ->
expect(pane.getPendingItem()).toBeNull()
it "opens file in pending state if 'pending' option is true", ->
expect(pane.getPendingItem()).toEqual editor1
it "terminates pending state if ::terminatePendingState is invoked", ->
editor1.terminatePendingState()
expect(pane.getPendingItem()).toBeNull()
expect(eventCount).toBe 1
it "terminates pending state when buffer is changed", ->
editor1.insertText('I\'ll be back!')
advanceClock(editor1.getBuffer().stoppedChangingDelay)
expect(pane.getPendingItem()).toBeNull()
expect(eventCount).toBe 1
it "only calls terminate handler once when text is modified twice", ->
editor1.insertText('Some text')
advanceClock(editor1.getBuffer().stoppedChangingDelay)
editor1.save()
editor1.insertText('More text')
advanceClock(editor1.getBuffer().stoppedChangingDelay)
expect(pane.getPendingItem()).toBeNull()
expect(eventCount).toBe 1
it "only calls clearPendingItem if there is a pending item to clear", ->
spyOn(pane, "clearPendingItem").andCallThrough()
editor1.terminatePendingState()
editor1.terminatePendingState()
expect(pane.getPendingItem()).toBeNull()
expect(pane.clearPendingItem.callCount).toBe 1
describe "serialization", ->
pane = null

View File

@@ -55,16 +55,6 @@ describe "TextEditor", ->
expect(editor.tokenizedLineForScreenRow(0).invisibles.eol).toBe '?'
it "restores pending tabs in pending state", ->
expect(editor.isPending()).toBe false
editor2 = TextEditor.deserialize(editor.serialize(), atom)
expect(editor2.isPending()).toBe false
pendingEditor = atom.workspace.buildTextEditor(pending: true)
expect(pendingEditor.isPending()).toBe true
editor3 = TextEditor.deserialize(pendingEditor.serialize(), atom)
expect(editor3.isPending()).toBe true
describe "when the editor is constructed with the largeFileMode option set to true", ->
it "loads the editor but doesn't tokenize", ->
editor = null
@@ -5818,53 +5808,3 @@ describe "TextEditor", ->
screenRange: marker1.getRange(),
rangeIsReversed: false
}
describe "pending state", ->
editor1 = null
eventCount = null
beforeEach ->
waitsForPromise ->
atom.workspace.open('sample.txt', pending: true).then (o) -> editor1 = o
runs ->
eventCount = 0
editor1.onDidTerminatePendingState -> eventCount++
it "does not open file in pending state by default", ->
expect(editor.isPending()).toBe false
it "opens file in pending state if 'pending' option is true", ->
expect(editor1.isPending()).toBe true
it "terminates pending state if ::terminatePendingState is invoked", ->
editor1.terminatePendingState()
expect(editor1.isPending()).toBe false
expect(eventCount).toBe 1
it "terminates pending state when buffer is changed", ->
editor1.insertText('I\'ll be back!')
advanceClock(editor1.getBuffer().stoppedChangingDelay)
expect(editor1.isPending()).toBe false
expect(eventCount).toBe 1
it "only calls terminate handler once when text is modified twice", ->
editor1.insertText('Some text')
advanceClock(editor1.getBuffer().stoppedChangingDelay)
editor1.save()
editor1.insertText('More text')
advanceClock(editor1.getBuffer().stoppedChangingDelay)
expect(editor1.isPending()).toBe false
expect(eventCount).toBe 1
it "only calls terminate handler once when terminatePendingState is called twice", ->
editor1.terminatePendingState()
editor1.terminatePendingState()
expect(editor1.isPending()).toBe false
expect(eventCount).toBe 1

View File

@@ -588,19 +588,22 @@ describe "Workspace", ->
describe "when the file is already open in pending state", ->
it "should terminate the pending state", ->
editor = null
pane = null
waitsForPromise ->
atom.workspace.open('sample.js', pending: true).then (o) -> editor = o
atom.workspace.open('sample.js', pending: true).then (o) ->
editor = o
pane = atom.workspace.getActivePane()
runs ->
expect(editor.isPending()).toBe true
expect(pane.getPendingItem()).toEqual editor
waitsForPromise ->
atom.workspace.open('sample.js').then (o) -> editor = o
atom.workspace.open('sample.js')
runs ->
expect(editor.isPending()).toBe false
expect(pane.getPendingItem()).toBeNull()
describe "::reopenItem()", ->
it "opens the uri associated with the last closed pane that isn't currently open", ->
pane = workspace.getActivePane()
@@ -1551,11 +1554,12 @@ describe "Workspace", ->
describe "when the core.allowPendingPaneItems option is falsey", ->
it "does not open item with `pending: true` option as pending", ->
editor = null
pane = null
atom.config.set('core.allowPendingPaneItems', false)
waitsForPromise ->
atom.workspace.open('sample.js', pending: true).then (o) -> editor = o
atom.workspace.open('sample.js', pending: true).then ->
pane = atom.workspace.getActivePane()
runs ->
expect(editor.isPending()).toBeFalsy()
expect(pane.getPendingItem()).toBeFalsy()

View File

@@ -1,5 +1,5 @@
{find, compact, extend, last} = require 'underscore-plus'
{Emitter} = require 'event-kit'
{CompositeDisposable, Emitter} = require 'event-kit'
Model = require './model'
PaneAxis = require './pane-axis'
TextEditor = require './text-editor'
@@ -8,6 +8,11 @@ TextEditor = require './text-editor'
# Panes can contain multiple items, one of which is *active* at a given time.
# The view corresponding to the active item is displayed in the interface. In
# the default configuration, tabs are also displayed for each item.
#
# Each pane may also contain one *pending* item. When a pending item is added
# to a pane, it will replace the currently pending item, if any, instead of
# simply being added. In the default configuration, the text in the tab for
# pending items is shown in italics.
module.exports =
class Pane extends Model
container: undefined
@@ -37,7 +42,7 @@ class Pane extends Model
} = params
@emitter = new Emitter
@itemSubscriptions = new WeakMap
@subscriptionsPerItem = new WeakMap
@items = []
@addItems(compact(params?.items ? []))
@@ -260,8 +265,8 @@ class Pane extends Model
getPanes: -> [this]
unsubscribeFromItem: (item) ->
@itemSubscriptions.get(item)?.dispose()
@itemSubscriptions.delete(item)
@subscriptionsPerItem.get(item)?.dispose()
@subscriptionsPerItem.delete(item)
###
Section: Items
@@ -342,13 +347,17 @@ class Pane extends Model
# Public: Make the given item *active*, causing it to be displayed by
# the pane's view.
activateItem: (item) ->
#
# * `pending` (optional) {Boolean} indicating that the item should be added
# in a pending state if it does not yet exist in the pane. Existing pending
# items in a pane are replaced with new pending items when they are opened.
activateItem: (item, pending=false) ->
if item?
if @activeItem?.isPending?()
if @getPendingItem() is @activeItem
index = @getActiveItemIndex()
else
index = @getActiveItemIndex() + 1
@addItem(item, index, false)
@addItem(item, index, false, pending)
@setActiveItem(item)
# Public: Add the given item to the pane.
@@ -357,28 +366,48 @@ class Pane extends Model
# view.
# * `index` (optional) {Number} indicating the index at which to add the item.
# If omitted, the item is added after the current active item.
# * `pending` (optional) {Boolean} indicating that the item should be
# added in a pending state. Existing pending items in a pane are replaced with
# new pending items when they are opened.
#
# Returns the added item.
addItem: (item, index=@getActiveItemIndex() + 1, moved=false) ->
addItem: (item, index=@getActiveItemIndex() + 1, moved=false, pending=false) ->
throw new Error("Pane items must be objects. Attempted to add item #{item}.") unless item? and typeof item is 'object'
throw new Error("Adding a pane item with URI '#{item.getURI?()}' that has already been destroyed") if item.isDestroyed?()
return if item in @items
if item.isPending?()
for existingItem, i in @items
if existingItem.isPending?()
@destroyItem(existingItem)
break
pendingItem = @getPendingItem()
@destroyItem(pendingItem) if pendingItem?
@setPendingItem(item) if pending
if typeof item.onDidDestroy is 'function'
@itemSubscriptions.set item, item.onDidDestroy => @removeItem(item, false)
itemSubscriptions = new CompositeDisposable
itemSubscriptions.add item.onDidDestroy => @removeItem(item, false)
if typeof item.onDidTerminatePendingState is "function"
itemSubscriptions.add item.onDidTerminatePendingState =>
@clearPendingItem() if @getPendingItem() is item
itemSubscriptions.add item.onDidDestroy => @removeItem(item, false)
@subscriptionsPerItem.set item, itemSubscriptions
@items.splice(index, 0, item)
@emitter.emit 'did-add-item', {item, index, moved}
@setActiveItem(item) unless @getActiveItem()?
item
setPendingItem: (item) =>
@pendingItem = item if @pendingItem isnt item
@emitter.emit 'did-terminate-pending-state' if not item
getPendingItem: =>
@pendingItem or null
clearPendingItem: =>
@setPendingItem(null)
onDidTerminatePendingState: (callback) =>
@emitter.on 'did-terminate-pending-state', callback
# Public: Add the given items to the pane.
#
# * `items` An {Array} of items to add. Items can be views or models with
@@ -397,6 +426,8 @@ class Pane extends Model
index = @items.indexOf(item)
return if index is -1
@pendingItem = null if @getPendingItem() is item
@emitter.emit 'will-remove-item', {item, index, destroyed: not moved, moved}
@unsubscribeFromItem(item)

View File

@@ -92,7 +92,7 @@ class TextEditor extends Model
softWrapped, @displayBuffer, @selectionsMarkerLayer, buffer, suppressCursorCreation,
@mini, @placeholderText, lineNumberGutterVisible, largeFileMode, @config,
@notificationManager, @packageManager, @clipboard, @viewRegistry, @grammarRegistry,
@project, @assert, @applicationDelegate, @pending
@project, @assert, @applicationDelegate
} = params
throw new Error("Must pass a config parameter when constructing TextEditors") unless @config?
@@ -110,6 +110,7 @@ class TextEditor extends Model
@disposables = new CompositeDisposable
@cursors = []
@selections = []
@hasTerminatedPendingState = false
buffer ?= new TextBuffer
@displayBuffer ?= new DisplayBuffer({
@@ -150,7 +151,6 @@ class TextEditor extends Model
firstVisibleScreenColumn: @getFirstVisibleScreenColumn()
displayBuffer: @displayBuffer.serialize()
selectionsMarkerLayerId: @selectionsMarkerLayer.id
pending: @isPending()
subscribeToBuffer: ->
@buffer.retain()
@@ -162,12 +162,18 @@ class TextEditor extends Model
@disposables.add @buffer.onDidChangeEncoding =>
@emitter.emit 'did-change-encoding', @getEncoding()
@disposables.add @buffer.onDidDestroy => @destroy()
if @pending
@disposables.add @buffer.onDidChangeModified =>
@terminatePendingState() if @buffer.isModified()
@disposables.add @buffer.onDidChangeModified =>
@terminatePendingState() if not @hasTerminatedPendingState and @buffer.isModified()
@preserveCursorPositionOnBufferReload()
terminatePendingState: ->
@emitter.emit 'did-terminate-pending-state' if not @hasTerminatedPendingState
@hasTerminatedPendingState = true
onDidTerminatePendingState: (callback) ->
@emitter.on 'did-terminate-pending-state', callback
subscribeToDisplayBuffer: ->
@disposables.add @selectionsMarkerLayer.onDidCreateMarker @addSelection.bind(this)
@disposables.add @displayBuffer.onDidChangeGrammar @handleGrammarChange.bind(this)
@@ -574,13 +580,6 @@ class TextEditor extends Model
getEditorWidthInChars: ->
@displayBuffer.getEditorWidthInChars()
onDidTerminatePendingState: (callback) ->
@emitter.on 'did-terminate-pending-state', callback
terminatePendingState: ->
return if not @pending
@pending = false
@emitter.emit 'did-terminate-pending-state'
###
Section: File Details
@@ -665,9 +664,6 @@ class TextEditor extends Model
# Essential: Returns {Boolean} `true` if this editor has no content.
isEmpty: -> @buffer.isEmpty()
# Returns {Boolean} `true` if this editor is pending and `false` if it is permanent.
isPending: -> Boolean(@pending)
# Copies the current file path to the native clipboard.
copyPathToClipboard: (relative = false) ->
if filePath = @getPath()

View File

@@ -403,6 +403,9 @@ class Workspace extends Model
# containing pane. Defaults to `true`.
# * `activateItem` A {Boolean} indicating whether to call {Pane::activateItem}
# on containing pane. Defaults to `true`.
# * `pending` A {Boolean} indicating whether or not the item should be opened
# in a pending state. Existing pending items in a pane are replaced with
# new pending items when they are opened.
# * `searchAllPanes` A {Boolean}. If `true`, the workspace will attempt to
# activate an existing item for the given URI on any pane.
# If `false`, only the active pane will be searched for
@@ -477,7 +480,7 @@ class Workspace extends Model
if uri?
if item = pane.itemForURI(uri)
item.terminatePendingState?() if item.isPending?() and not options.pending
pane.clearPendingItem() if not options.pending and pane.getPendingItem() is item
item ?= opener(uri, options) for opener in @getOpeners() when not item
try
@@ -500,7 +503,7 @@ class Workspace extends Model
return item if pane.isDestroyed()
@itemOpened(item)
pane.activateItem(item) if activateItem
pane.activateItem(item, options.pending) if activateItem
pane.activate() if activatePane
initialLine = initialColumn = 0