From f6d419c2f4563dae446592eb928d4540bb841af7 Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Mon, 7 Mar 2016 10:01:28 -0800 Subject: [PATCH] Merge pull request #11057 from atom/mkt-improve-pane-add-item-options Move Pane::addItem 'pending' option to options object (cherry picked from commit 53a9c22554177c787751e20c7e7b38bd4eb8e69b) --- package.json | 2 +- spec/pane-spec.coffee | 57 +++++++++++++++++++++++++++++++++++-------- src/pane.coffee | 41 ++++++++++++++++++++----------- src/workspace.coffee | 2 +- 4 files changed, 76 insertions(+), 26 deletions(-) diff --git a/package.json b/package.json index ff30c0d07..5cd1367fa 100644 --- a/package.json +++ b/package.json @@ -110,7 +110,7 @@ "status-bar": "1.1.0", "styleguide": "0.45.2", "symbols-view": "0.111.1", - "tabs": "0.91.1", + "tabs": "0.91.3", "timecop": "0.33.1", "tree-view": "0.202.0", "update-package-dependencies": "0.10.0", diff --git a/spec/pane-spec.coffee b/spec/pane-spec.coffee index 873749a19..d0b191f38 100644 --- a/spec/pane-spec.coffee +++ b/spec/pane-spec.coffee @@ -1,5 +1,6 @@ {extend} = require 'underscore-plus' {Emitter} = require 'event-kit' +Grim = require 'grim' Pane = require '../src/pane' PaneAxis = require '../src/pane-axis' PaneContainer = require '../src/pane-container' @@ -92,7 +93,7 @@ describe "Pane", -> pane = new Pane(paneParams(items: [new Item("A"), new Item("B")])) [item1, item2] = pane.getItems() item3 = new Item("C") - pane.addItem(item3, 1) + pane.addItem(item3, index: 1) expect(pane.getItems()).toEqual [item1, item3, item2] it "adds the item after the active item if no index is provided", -> @@ -115,7 +116,7 @@ describe "Pane", -> pane.onDidAddItem (event) -> events.push(event) item = new Item("C") - pane.addItem(item, 1) + pane.addItem(item, index: 1) expect(events).toEqual [{item, index: 1, moved: false}] it "throws an exception if the item is already present on a pane", -> @@ -137,9 +138,9 @@ describe "Pane", -> itemA = new Item("A") itemB = new Item("B") itemC = new Item("C") - pane.addItem(itemA, undefined, false, false) - pane.addItem(itemB, undefined, false, true) - pane.addItem(itemC, undefined, false, false) + pane.addItem(itemA, pending: false) + pane.addItem(itemB, pending: true) + pane.addItem(itemC, pending: false) expect(itemB.isDestroyed()).toBe true it "adds the new item before destroying any existing pending item", -> @@ -148,7 +149,7 @@ describe "Pane", -> pane = new Pane(paneParams(items: [])) itemA = new Item("A") itemB = new Item("B") - pane.addItem(itemA, undefined, false, true) + pane.addItem(itemA, pending: true) pane.onDidAddItem ({item}) -> eventOrder.push("add") if item is itemB @@ -164,6 +165,25 @@ describe "Pane", -> runs -> expect(eventOrder).toEqual ["add", "remove"] + describe "when using the old API of ::addItem(item, index)", -> + beforeEach -> + spyOn Grim, "deprecate" + + it "supports the older public API", -> + pane = new Pane(paneParams(items: [])) + itemA = new Item("A") + itemB = new Item("B") + itemC = new Item("C") + pane.addItem(itemA, 0) + pane.addItem(itemB, 0) + pane.addItem(itemC, 0) + expect(pane.getItems()).toEqual [itemC, itemB, itemA] + + it "shows a deprecation warning", -> + pane = new Pane(paneParams(items: [])) + pane.addItem(new Item(), 2) + expect(Grim.deprecate).toHaveBeenCalledWith "Pane::addItem(item, 2) is deprecated in favor of Pane::addItem(item, {index: 2})" + describe "::activateItem(item)", -> pane = null @@ -196,15 +216,15 @@ describe "Pane", -> itemD = new Item("D") it "replaces the active item if it is pending", -> - pane.activateItem(itemC, true) + pane.activateItem(itemC, pending: true) expect(pane.getItems().map (item) -> item.name).toEqual ['A', 'C', 'B'] - pane.activateItem(itemD, true) + pane.activateItem(itemD, pending: 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, true) + pane.activateItem(itemC, pending: true) pane.activateItemAtIndex(2) - pane.activateItem(itemD, true) + pane.activateItem(itemD, pending: true) expect(pane.getItems().map (item) -> item.name).toEqual ['A', 'B', 'D'] describe "::setPendingItem", -> @@ -706,6 +726,23 @@ describe "Pane", -> expect(pane2.isDestroyed()).toBe true expect(item4.isDestroyed()).toBe false + describe "when the item being moved is pending", -> + it "is made permanent in the new pane", -> + item6 = new Item("F") + pane1.addItem(item6, pending: true) + expect(pane1.getPendingItem()).toEqual item6 + pane1.moveItemToPane(item6, pane2, 0) + expect(pane2.getPendingItem()).not.toEqual item6 + + describe "when the target pane has a pending item", -> + it "does not destroy the pending item", -> + item6 = new Item("F") + pane1.addItem(item6, pending: true) + expect(pane1.getPendingItem()).toEqual item6 + pane2.moveItemToPane(item5, pane1, 0) + expect(pane1.getPendingItem()).toEqual item6 + + describe "split methods", -> [pane1, item1, container] = [] diff --git a/src/pane.coffee b/src/pane.coffee index 70f8b4bab..3ff62993c 100644 --- a/src/pane.coffee +++ b/src/pane.coffee @@ -1,3 +1,4 @@ +Grim = require 'grim' {find, compact, extend, last} = require 'underscore-plus' {CompositeDisposable, Emitter} = require 'event-kit' Model = require './model' @@ -394,30 +395,42 @@ class Pane extends Model # Public: Make the given item *active*, causing it to be displayed by # the pane's view. # - # * `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) -> + # * `options` (optional) {Object} + # * `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, options={}) -> if item? if @getPendingItem() is @activeItem index = @getActiveItemIndex() else index = @getActiveItemIndex() + 1 - @addItem(item, index, false, pending) + @addItem(item, extend({}, options, {index: index})) @setActiveItem(item) # Public: Add the given item to the pane. # # * `item` The item to add. It can be a model with an associated view or a # 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. + # * `options` (optional) {Object} + # * `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, pending=false) -> + addItem: (item, options={}) -> + # Backward compat with old API: + # addItem(item, index=@getActiveItemIndex() + 1) + if typeof options is "number" + Grim.deprecate("Pane::addItem(item, #{options}) is deprecated in favor of Pane::addItem(item, {index: #{options}})") + options = index: options + + index = options.index ? @getActiveItemIndex() + 1 + moved = options.moved ? false + pending = options.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?() @@ -437,7 +450,7 @@ class Pane extends Model @setPendingItem(item) if pending @emitter.emit 'did-add-item', {item, index, moved} - @destroyItem(lastPendingItem) if lastPendingItem? + @destroyItem(lastPendingItem) if lastPendingItem? and not moved @setActiveItem(item) unless @getActiveItem()? item @@ -467,7 +480,7 @@ class Pane extends Model # Returns an {Array} of added items. addItems: (items, index=@getActiveItemIndex() + 1) -> items = items.filter (item) => not (item in @items) - @addItem(item, index + i, false) for item, i in items + @addItem(item, {index: index + i}) for item, i in items items removeItem: (item, moved) -> @@ -516,7 +529,7 @@ class Pane extends Model # given pane. moveItemToPane: (item, pane, index) -> @removeItem(item, true) - pane.addItem(item, index, true) + pane.addItem(item, {index: index, moved: true}) # Public: Destroy the active item and activate the next item. destroyActiveItem: -> diff --git a/src/workspace.coffee b/src/workspace.coffee index 873d90370..9b1bf14fc 100644 --- a/src/workspace.coffee +++ b/src/workspace.coffee @@ -509,7 +509,7 @@ class Workspace extends Model return item if pane.isDestroyed() @itemOpened(item) - pane.activateItem(item, options.pending) if activateItem + pane.activateItem(item, {pending: options.pending}) if activateItem pane.activate() if activatePane initialLine = initialColumn = 0