From 43d933eca83ba0de92956854e84220462b7c4e91 Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Sat, 9 Jan 2016 17:18:07 -0800 Subject: [PATCH 01/12] Change order of tests --- spec/text-editor-spec.coffee | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/spec/text-editor-spec.coffee b/spec/text-editor-spec.coffee index 686bb6868..2e85870e6 100644 --- a/spec/text-editor-spec.coffee +++ b/spec/text-editor-spec.coffee @@ -5811,9 +5811,11 @@ describe "TextEditor", -> waitsForPromise -> atom.workspace.open('sample.txt', pending: true).then (o) -> editor1 = o - it "should open file in pending state if 'pending' option is true", -> + 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 - expect(editor.isPending()).toBe false # By default pending status is false it "invokes ::onDidTerminatePendingState observers if pending status is removed", -> events = [] @@ -5822,7 +5824,7 @@ describe "TextEditor", -> expect(editor1.isPending()).toBe false expect(events).toEqual [editor1] - it "should terminate pending state when buffer is changed", -> + it "terminates pending state when buffer is changed", -> events = [] editor1.onDidTerminatePendingState (event) -> events.push(event) expect(editor1.isPending()).toBe true From 2d00163e1804de6936cf6761e54ab5097f517c62 Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Sat, 9 Jan 2016 17:19:32 -0800 Subject: [PATCH 02/12] Refactor pending item tests --- spec/pane-spec.coffee | 40 ++++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/spec/pane-spec.coffee b/spec/pane-spec.coffee index 5c5cd9e95..a40a0a016 100644 --- a/spec/pane-spec.coffee +++ b/spec/pane-spec.coffee @@ -155,25 +155,33 @@ describe "Pane", -> pane.activateItem(pane.itemAtIndex(1)) expect(observed).toEqual [pane.itemAtIndex(1)] - it "replaces pending items", -> - itemC = new Item("C") - itemD = new Item("D") - itemC.pending = true - itemD.pending = true + describe "activating pending items", -> + itemC = null + itemD = null - expect(itemC.isPending()).toBe true - pane.activateItem(itemC) - expect(pane.getItems().length).toBe 3 - expect(pane.getActiveItem()).toBe pane.itemAtIndex(1) + beforeEach -> + itemC = new Item("C") + itemD = new Item("D") + itemC.pending = true + itemD.pending = true + pane.activateItem(itemC) - expect(itemD.isPending()).toBe true - pane.activateItem(itemD) - expect(pane.getItems().length).toBe 3 - expect(pane.getActiveItem()).toBe pane.itemAtIndex(1) + it "opens pending item", -> + expect(pane.getItems().length).toBe 3 + expect(pane.getActiveItem()).toBe pane.itemAtIndex(1) - pane.activateItem(pane.itemAtIndex(2)) - expect(pane.getItems().length).toBe 2 - expect(pane.getActiveItem()).toBe pane.itemAtIndex(1) + it "replaces original pending item when activating another pending item", -> + pane.activateItem(itemD) + + expect(pane.getItems().length).toBe 3 + expect(pane.getActiveItem()).toBe itemD + expect(pane.getActiveItem()).toBe pane.itemAtIndex(1) + + it "closes pending item when non-pending item is activated", -> + pane.activateItem(pane.itemAtIndex(0)) + + expect(pane.getItems().length).toBe 2 + expect(pane.getActiveItem()).toBe pane.itemAtIndex(0) describe "::activateNextItem() and ::activatePreviousItem()", -> it "sets the active item to the next/previous item, looping around at either end", -> From d0be1965f245832d2f205e0a7441e4defd494c84 Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Sat, 9 Jan 2016 17:26:44 -0800 Subject: [PATCH 03/12] :art: --- spec/text-editor-spec.coffee | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/spec/text-editor-spec.coffee b/spec/text-editor-spec.coffee index 2e85870e6..8976c1b51 100644 --- a/spec/text-editor-spec.coffee +++ b/spec/text-editor-spec.coffee @@ -5807,28 +5807,31 @@ describe "TextEditor", -> describe "pending state", -> editor1 = null + events = null + beforeEach -> waitsForPromise -> atom.workspace.open('sample.txt', pending: true).then (o) -> editor1 = o + runs -> + events = [] + editor1.onDidTerminatePendingState (event) -> events.push(event) + 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 "invokes ::onDidTerminatePendingState observers if pending status is removed", -> - events = [] - editor1.onDidTerminatePendingState (event) -> events.push(event) + it "terminates pending state if ::terminatePendingState is invoked", -> editor1.terminatePendingState() + expect(editor1.isPending()).toBe false expect(events).toEqual [editor1] it "terminates pending state when buffer is changed", -> - events = [] - editor1.onDidTerminatePendingState (event) -> events.push(event) - expect(editor1.isPending()).toBe true editor1.insertText('I\'ll be back!') advanceClock(500) + expect(editor1.isPending()).toBe false expect(events).toEqual [editor1] From 364205ca564296aa59ca2045f60666f67581e39e Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Sat, 9 Jan 2016 18:10:28 -0800 Subject: [PATCH 04/12] :white_check_mark: Ensure terminate handler is invoked only once --- spec/text-editor-spec.coffee | 19 +++++++++++++++++++ src/text-editor.coffee | 1 + 2 files changed, 20 insertions(+) diff --git a/spec/text-editor-spec.coffee b/spec/text-editor-spec.coffee index 8976c1b51..8d538f4eb 100644 --- a/spec/text-editor-spec.coffee +++ b/spec/text-editor-spec.coffee @@ -5835,3 +5835,22 @@ describe "TextEditor", -> expect(editor1.isPending()).toBe false expect(events).toEqual [editor1] + + it "only calls terminate handler once when text is modified twice", -> + editor1.insertText('Some text') + advanceClock(500) + + editor1.save() + + editor1.insertText('More text') + advanceClock(500) + + expect(editor1.isPending()).toBe false + expect(events).toEqual [editor1] + + it "only calls terminate handler once when terminatePendingState is called twice", -> + editor1.terminatePendingState() + editor1.terminatePendingState() + + expect(editor1.isPending()).toBe false + expect(events).toEqual [editor1] diff --git a/src/text-editor.coffee b/src/text-editor.coffee index 7410f8209..047ade98e 100644 --- a/src/text-editor.coffee +++ b/src/text-editor.coffee @@ -576,6 +576,7 @@ class TextEditor extends Model @emitter.on 'did-terminate-pending-state', callback terminatePendingState: -> + return if not @pending @pending = false @emitter.emit 'did-terminate-pending-state', this From 292ef2fe1ca03fcf8662252c0024b124479af1d3 Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Sat, 9 Jan 2016 21:00:53 -0800 Subject: [PATCH 05/12] Early return from promise if pane is destroyed --- src/workspace.coffee | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/workspace.coffee b/src/workspace.coffee index f64f58ee0..a90d314f7 100644 --- a/src/workspace.coffee +++ b/src/workspace.coffee @@ -483,6 +483,8 @@ class Workspace extends Model Promise.resolve(item) .then (item) => + return item if pane.isDestroyed() + @itemOpened(item) pane.activateItem(item) if activateItem pane.activate() if activatePane From 8d3b86acb52cfe77b1cbe6157646512ab3866092 Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Sun, 10 Jan 2016 12:53:39 -0800 Subject: [PATCH 06/12] Serialize pending state for editor --- src/text-editor.coffee | 1 + 1 file changed, 1 insertion(+) diff --git a/src/text-editor.coffee b/src/text-editor.coffee index 047ade98e..72e6c9a91 100644 --- a/src/text-editor.coffee +++ b/src/text-editor.coffee @@ -150,6 +150,7 @@ class TextEditor extends Model firstVisibleScreenColumn: @getFirstVisibleScreenColumn() displayBuffer: @displayBuffer.serialize() selectionsMarkerLayerId: @selectionsMarkerLayer.id + pending: @isPending() subscribeToBuffer: -> @buffer.retain() From 4d0b58ff46b1a11def04c27c35ab19e878510be1 Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Sun, 10 Jan 2016 13:01:01 -0800 Subject: [PATCH 07/12] Add spec for serialzing/deserialzing pending state for editor --- spec/text-editor-spec.coffee | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/spec/text-editor-spec.coffee b/spec/text-editor-spec.coffee index 8d538f4eb..39dca6ad4 100644 --- a/spec/text-editor-spec.coffee +++ b/spec/text-editor-spec.coffee @@ -12,7 +12,7 @@ describe "TextEditor", -> beforeEach -> waitsForPromise -> - atom.workspace.open('sample.js', autoIndent: false).then (o) -> editor = o + atom.workspace.open('sample.js', {autoIndent: false, pending: true}).then (o) -> editor = o runs -> buffer = editor.buffer @@ -55,6 +55,13 @@ describe "TextEditor", -> expect(editor.tokenizedLineForScreenRow(0).invisibles.eol).toBe '?' + it "restores pending tabs in pending state", -> + expect(editor.isPending()).toBe true + + editor2 = TextEditor.deserialize(editor.serialize(), atom) + + expect(editor2.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 From aea9c5804a04fe243fe90a5ed17d2e3339776eef Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 12 Jan 2016 18:07:24 -0800 Subject: [PATCH 08/12] Improve terminate pending state tests. Remove argument to remove pending state listeners. --- spec/fixtures/sample.txt | 2 +- spec/text-editor-spec.coffee | 31 +++++++++++++++++-------------- src/text-editor.coffee | 2 +- 3 files changed, 19 insertions(+), 16 deletions(-) diff --git a/spec/fixtures/sample.txt b/spec/fixtures/sample.txt index 3e715502b..9701a96c5 100644 --- a/spec/fixtures/sample.txt +++ b/spec/fixtures/sample.txt @@ -1 +1 @@ -Some text. +Some textSome textSome text. diff --git a/spec/text-editor-spec.coffee b/spec/text-editor-spec.coffee index 39dca6ad4..d77844015 100644 --- a/spec/text-editor-spec.coffee +++ b/spec/text-editor-spec.coffee @@ -12,7 +12,7 @@ describe "TextEditor", -> beforeEach -> waitsForPromise -> - atom.workspace.open('sample.js', {autoIndent: false, pending: true}).then (o) -> editor = o + atom.workspace.open('sample.js', {autoIndent: false}).then (o) -> editor = o runs -> buffer = editor.buffer @@ -56,11 +56,14 @@ describe "TextEditor", -> expect(editor.tokenizedLineForScreenRow(0).invisibles.eol).toBe '?' it "restores pending tabs in pending state", -> - expect(editor.isPending()).toBe true - + expect(editor.isPending()).toBe false editor2 = TextEditor.deserialize(editor.serialize(), atom) + expect(editor2.isPending()).toBe false - expect(editor2.isPending()).toBe true + 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", -> @@ -5814,15 +5817,15 @@ describe "TextEditor", -> describe "pending state", -> editor1 = null - events = null + eventCount = null beforeEach -> waitsForPromise -> atom.workspace.open('sample.txt', pending: true).then (o) -> editor1 = o runs -> - events = [] - editor1.onDidTerminatePendingState (event) -> events.push(event) + eventCount = 0 + editor1.onDidTerminatePendingState -> eventCount++ it "does not open file in pending state by default", -> expect(editor.isPending()).toBe false @@ -5834,30 +5837,30 @@ describe "TextEditor", -> editor1.terminatePendingState() expect(editor1.isPending()).toBe false - expect(events).toEqual [editor1] + expect(eventCount).toBe 1 it "terminates pending state when buffer is changed", -> editor1.insertText('I\'ll be back!') - advanceClock(500) + advanceClock(editor1.getBuffer().stoppedChangingDelay) expect(editor1.isPending()).toBe false - expect(events).toEqual [editor1] + expect(eventCount).toBe 1 it "only calls terminate handler once when text is modified twice", -> editor1.insertText('Some text') - advanceClock(500) + advanceClock(editor1.getBuffer().stoppedChangingDelay) editor1.save() editor1.insertText('More text') - advanceClock(500) + advanceClock(editor1.getBuffer().stoppedChangingDelay) expect(editor1.isPending()).toBe false - expect(events).toEqual [editor1] + 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(events).toEqual [editor1] + expect(eventCount).toBe 1 diff --git a/src/text-editor.coffee b/src/text-editor.coffee index 72e6c9a91..7be1bec31 100644 --- a/src/text-editor.coffee +++ b/src/text-editor.coffee @@ -579,7 +579,7 @@ class TextEditor extends Model terminatePendingState: -> return if not @pending @pending = false - @emitter.emit 'did-terminate-pending-state', this + @emitter.emit 'did-terminate-pending-state' ### Section: File Details From 24b2f5e0ccf659a253202e3017315b97c51133bc Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Wed, 13 Jan 2016 18:53:10 -0800 Subject: [PATCH 09/12] Keep pending pane upon losing focus --- src/pane.coffee | 1 - 1 file changed, 1 deletion(-) diff --git a/src/pane.coffee b/src/pane.coffee index 70687227a..3161a33ac 100644 --- a/src/pane.coffee +++ b/src/pane.coffee @@ -346,7 +346,6 @@ class Pane extends Model if item? if @activeItem?.isPending?() index = @getActiveItemIndex() - @destroyActiveItem() unless item is @activeItem else index = @getActiveItemIndex() + 1 @addItem(item, index, false) From f3f29c876f99a5af02ba15591e89835071a4f602 Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Wed, 13 Jan 2016 19:05:38 -0800 Subject: [PATCH 10/12] Use __dirname in path.resolve for specs --- spec/native-compile-cache-spec.coffee | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/native-compile-cache-spec.coffee b/spec/native-compile-cache-spec.coffee index 9d6cb89b4..1531deaf9 100644 --- a/spec/native-compile-cache-spec.coffee +++ b/spec/native-compile-cache-spec.coffee @@ -68,12 +68,12 @@ describe "NativeCompileCache", -> describe "when a previously required and cached file changes", -> beforeEach -> - fs.writeFileSync path.resolve('./spec/fixtures/native-cache/file-5'), """ + fs.writeFileSync path.resolve(__dirname + '/fixtures/native-cache/file-5'), """ module.exports = function () { return "file-5" } """ afterEach -> - fs.unlinkSync path.resolve('./spec/fixtures/native-cache/file-5') + fs.unlinkSync path.resolve(__dirname + '/fixtures/native-cache/file-5') it "removes it from the store and re-inserts it with the new cache", -> fn5 = require('./fixtures/native-cache/file-5') From 4fce3668b6eb77f853ae8fb8e34335faa03bcb27 Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Wed, 13 Jan 2016 21:40:32 -0800 Subject: [PATCH 11/12] Update specs for pending item behavior --- spec/pane-spec.coffee | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/pane-spec.coffee b/spec/pane-spec.coffee index a40a0a016..b3a341f58 100644 --- a/spec/pane-spec.coffee +++ b/spec/pane-spec.coffee @@ -177,10 +177,10 @@ describe "Pane", -> expect(pane.getActiveItem()).toBe itemD expect(pane.getActiveItem()).toBe pane.itemAtIndex(1) - it "closes pending item when non-pending item is activated", -> + it "keeps pending item when non-pending item is activated", -> pane.activateItem(pane.itemAtIndex(0)) - expect(pane.getItems().length).toBe 2 + expect(pane.getItems().length).toBe 3 expect(pane.getActiveItem()).toBe pane.itemAtIndex(0) describe "::activateNextItem() and ::activatePreviousItem()", -> From 44a648a9eaaad1c8a1beee61ff184e1a9eb89680 Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Thu, 14 Jan 2016 17:20:32 -0800 Subject: [PATCH 12/12] Destroy any existing pending pane item when adding a pending item --- spec/pane-spec.coffee | 36 ++++++++++++++++++++---------------- src/pane.coffee | 6 ++++++ 2 files changed, 26 insertions(+), 16 deletions(-) diff --git a/spec/pane-spec.coffee b/spec/pane-spec.coffee index b3a341f58..494dbc91a 100644 --- a/spec/pane-spec.coffee +++ b/spec/pane-spec.coffee @@ -132,6 +132,16 @@ describe "Pane", -> expect(-> pane.addItem('foo')).toThrow() expect(-> pane.addItem(1)).toThrow() + it "destroys any existing pending item if the new item is pending", -> + 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) + expect(itemA.isDestroyed()).toBe true + describe "::activateItem(item)", -> pane = null @@ -155,7 +165,7 @@ describe "Pane", -> pane.activateItem(pane.itemAtIndex(1)) expect(observed).toEqual [pane.itemAtIndex(1)] - describe "activating pending items", -> + describe "when the item being activated is pending", -> itemC = null itemD = null @@ -164,24 +174,18 @@ describe "Pane", -> itemD = new Item("D") itemC.pending = true itemD.pending = true + + it "replaces the active item if it is pending", -> pane.activateItem(itemC) - - it "opens pending item", -> - expect(pane.getItems().length).toBe 3 - expect(pane.getActiveItem()).toBe pane.itemAtIndex(1) - - it "replaces original pending item when activating another pending item", -> + expect(pane.getItems().map (item) -> item.name).toEqual ['A', 'C', 'B'] pane.activateItem(itemD) + expect(pane.getItems().map (item) -> item.name).toEqual ['A', 'D', 'B'] - expect(pane.getItems().length).toBe 3 - expect(pane.getActiveItem()).toBe itemD - expect(pane.getActiveItem()).toBe pane.itemAtIndex(1) - - it "keeps pending item when non-pending item is activated", -> - pane.activateItem(pane.itemAtIndex(0)) - - expect(pane.getItems().length).toBe 3 - expect(pane.getActiveItem()).toBe pane.itemAtIndex(0) + it "adds the item after the active item if it is not pending", -> + pane.activateItem(itemC) + pane.activateItemAtIndex(2) + pane.activateItem(itemD) + expect(pane.getItems().map (item) -> item.name).toEqual ['A', 'B', 'D'] describe "::activateNextItem() and ::activatePreviousItem()", -> it "sets the active item to the next/previous item, looping around at either end", -> diff --git a/src/pane.coffee b/src/pane.coffee index 3161a33ac..239e0eeff 100644 --- a/src/pane.coffee +++ b/src/pane.coffee @@ -365,6 +365,12 @@ class Pane extends Model return if item in @items + if item.isPending?() + for existingItem, i in @items + if existingItem.isPending?() + @destroyItem(existingItem) + break + if typeof item.onDidDestroy is 'function' @itemSubscriptions.set item, item.onDidDestroy => @removeItem(item, false)