From 5a8b197db19f9328063f0aa8aaecc725eb620be8 Mon Sep 17 00:00:00 2001 From: Jordan Eldredge Date: Fri, 11 Aug 2017 13:24:34 -0700 Subject: [PATCH] Ensure Pane.destroyItem always returns a promise Fixes #15157 --- spec/pane-spec.js | 25 ++++++++++++++++--------- src/pane.coffee | 5 ++++- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/spec/pane-spec.js b/spec/pane-spec.js index c36abbf6a..68e93c38f 100644 --- a/spec/pane-spec.js +++ b/spec/pane-spec.js @@ -551,10 +551,11 @@ describe('Pane', () => { itemURI = 'test' confirm.andReturn(0) - await pane.destroyItem(item1) + const success = await pane.destroyItem(item1) expect(item1.save).toHaveBeenCalled() expect(pane.getItems().includes(item1)).toBe(false) expect(item1.isDestroyed()).toBe(true) + expect(success).toBe(true) }) }) @@ -565,11 +566,12 @@ describe('Pane', () => { showSaveDialog.andReturn('/selected/path') confirm.andReturn(0) - await pane.destroyItem(item1) + const success = await pane.destroyItem(item1) expect(showSaveDialog).toHaveBeenCalled() expect(item1.saveAs).toHaveBeenCalledWith('/selected/path') expect(pane.getItems().includes(item1)).toBe(false) expect(item1.isDestroyed()).toBe(true) + expect(success).toBe(true) }) }) }) @@ -578,10 +580,11 @@ describe('Pane', () => { it('removes and destroys the item without saving it', async () => { confirm.andReturn(2) - await pane.destroyItem(item1) + const success = await pane.destroyItem(item1) expect(item1.save).not.toHaveBeenCalled() expect(pane.getItems().includes(item1)).toBe(false) expect(item1.isDestroyed()).toBe(true) + expect(success).toBe(true); }) }) @@ -589,19 +592,21 @@ describe('Pane', () => { it('does not save, remove, or destroy the item', async () => { confirm.andReturn(1) - await pane.destroyItem(item1) + const success = await pane.destroyItem(item1) expect(item1.save).not.toHaveBeenCalled() expect(pane.getItems().includes(item1)).toBe(true) expect(item1.isDestroyed()).toBe(false) + expect(success).toBe(false) }) }) describe('when force=true', () => { it('destroys the item immediately', async () => { - await pane.destroyItem(item1, true) + const success = await pane.destroyItem(item1, true) expect(item1.save).not.toHaveBeenCalled() expect(pane.getItems().includes(item1)).toBe(false) expect(item1.isDestroyed()).toBe(true) + expect(success).toBe(true) }) }) }) @@ -630,18 +635,20 @@ describe('Pane', () => { }) describe('when passed a permanent dock item', () => { - it("doesn't destroy the item", () => { + it("doesn't destroy the item", async () => { spyOn(item1, 'isPermanentDockItem').andReturn(true) - pane.destroyItem(item1) + const success = await pane.destroyItem(item1) expect(pane.getItems().includes(item1)).toBe(true) expect(item1.isDestroyed()).toBe(false) + expect(success).toBe(false); }) - it('destroy the item if force=true', () => { + it('destroy the item if force=true', async () => { spyOn(item1, 'isPermanentDockItem').andReturn(true) - pane.destroyItem(item1, true) + const success = await pane.destroyItem(item1, true) expect(pane.getItems().includes(item1)).toBe(false) expect(item1.isDestroyed()).toBe(true) + expect(success).toBe(true) }) }) }) diff --git a/src/pane.coffee b/src/pane.coffee index 9c0440e0a..23a60306b 100644 --- a/src/pane.coffee +++ b/src/pane.coffee @@ -621,12 +621,15 @@ class Pane destroyItem: (item, force) -> index = @items.indexOf(item) if index isnt -1 - return false if not force and @getContainer()?.getLocation() isnt 'center' and item.isPermanentDockItem?() + if not force and @getContainer()?.getLocation() isnt 'center' and item.isPermanentDockItem?() + return Promise.resolve(false) + @emitter.emit 'will-destroy-item', {item, index} @container?.willDestroyPaneItem({item, index, pane: this}) if force or not item?.shouldPromptToSave?() @removeItem(item, false) item.destroy?() + Promise.resolve(true) else @promptToSaveItem(item).then (result) => if result