From d53be3b28dcb876c09f5a1f898de0bc98e03b34a Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Thu, 5 Jul 2012 16:41:37 -0600 Subject: [PATCH 01/14] Add failing spec demonstrating inability to undo snippet expansion --- spec/extensions/snippets-spec.coffee | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/spec/extensions/snippets-spec.coffee b/spec/extensions/snippets-spec.coffee index 2533b4675..4293f5cab 100644 --- a/spec/extensions/snippets-spec.coffee +++ b/spec/extensions/snippets-spec.coffee @@ -41,6 +41,11 @@ describe "Snippets extension", -> go here ${1:first} and then here ${2:second} endsnippet + + snippet t5 "Caused problems with undo" + first line $1 + ${2:placeholder ending second line} + endsnippet """ describe "when the letters preceding the cursor trigger a snippet", -> @@ -144,6 +149,18 @@ describe "Snippets extension", -> expect(buffer.lineForRow(0)).toBe "xte var quicksort = function () {" expect(editor.getCursorScreenPosition()).toEqual [0, 5] + describe "when a previous snippet expansion has just been undone", -> + fit "expands the snippet based on the current prefix rather than jumping to the old snippet's tab stop", -> + editor.insertText 't5\n' + editor.setCursorBufferPosition [0, 2] + editor.trigger keydownEvent('tab', target: editor[0]) + expect(buffer.lineForRow(0)).toBe "first line" + editor.undo() + editor.undo() + expect(buffer.lineForRow(0)).toBe "t5" + editor.trigger keydownEvent('tab', target: editor[0]) + expect(buffer.lineForRow(0)).toBe "first line" + describe ".loadSnippetsFile(path)", -> it "loads the snippets in the given file", -> spyOn(fs, 'read').andReturn """ From 45a45bcbc8d8eef52fb07c0ca94681a9aef98578 Mon Sep 17 00:00:00 2001 From: David Graham & Nathan Sobo Date: Thu, 5 Jul 2012 17:17:42 -0600 Subject: [PATCH 02/14] Disable failing snippets spec for now --- spec/extensions/snippets-spec.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/extensions/snippets-spec.coffee b/spec/extensions/snippets-spec.coffee index 4293f5cab..39ad9a9b9 100644 --- a/spec/extensions/snippets-spec.coffee +++ b/spec/extensions/snippets-spec.coffee @@ -150,7 +150,7 @@ describe "Snippets extension", -> expect(editor.getCursorScreenPosition()).toEqual [0, 5] describe "when a previous snippet expansion has just been undone", -> - fit "expands the snippet based on the current prefix rather than jumping to the old snippet's tab stop", -> + xit "expands the snippet based on the current prefix rather than jumping to the old snippet's tab stop", -> editor.insertText 't5\n' editor.setCursorBufferPosition [0, 2] editor.trigger keydownEvent('tab', target: editor[0]) From 822b85a8bee3a8e1cf27453343c9d388d931aa60 Mon Sep 17 00:00:00 2001 From: David Graham & Nathan Sobo Date: Thu, 5 Jul 2012 17:18:11 -0600 Subject: [PATCH 03/14] Make @probablycorey happier --- src/app/buffer.coffee | 1 + 1 file changed, 1 insertion(+) diff --git a/src/app/buffer.coffee b/src/app/buffer.coffee index d549b85ca..86f3952e4 100644 --- a/src/app/buffer.coffee +++ b/src/app/buffer.coffee @@ -9,6 +9,7 @@ UndoManager = require 'undo-manager' module.exports = class Buffer @idCounter = 1 + undoManager: null modified: null lines: null file: null From 78f88a5c5cf803525187a42a17b5bb9851a07981 Mon Sep 17 00:00:00 2001 From: David Graham & Nathan Sobo Date: Thu, 5 Jul 2012 17:18:55 -0600 Subject: [PATCH 04/14] Begin extracting BufferChangeOperation from UndoManager --- src/app/undo-manager.coffee | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/src/app/undo-manager.coffee b/src/app/undo-manager.coffee index 45a06fd17..1ec9b3682 100644 --- a/src/app/undo-manager.coffee +++ b/src/app/undo-manager.coffee @@ -1,4 +1,7 @@ +_ = require 'underscore' + module.exports = + class UndoManager undoHistory: null redoHistory: null @@ -10,8 +13,9 @@ class UndoManager @startBatchCallCount = 0 @undoHistory = [] @redoHistory = [] - @buffer.on 'change', (op) => + @buffer.on 'change', (event) => unless @preserveHistory + op = new BufferChangeOperation(_.extend({ @buffer }, event)) if @currentBatch @currentBatch.push(op) else @@ -23,17 +27,15 @@ class UndoManager @preservingHistory => opsInReverse = new Array(batch...) opsInReverse.reverse() - for op in opsInReverse - @buffer.change op.newRange, op.oldText + op.undo() for op in opsInReverse @redoHistory.push batch batch.oldSelectionRanges redo: -> if batch = @redoHistory.pop() @preservingHistory => - for op in batch - @buffer.change op.oldRange, op.newText - @undoHistory.push batch + op.do() for op in batch + @undoHistory.push(batch) batch.newSelectionRanges startUndoBatch: (ranges) -> @@ -54,3 +56,11 @@ class UndoManager fn() @preserveHistory = false +class BufferChangeOperation + constructor: ({@buffer, @oldRange, @newRange, @oldText, @newText}) -> + + do: -> + @buffer.change @oldRange, @newText + + undo: -> + @buffer.change @newRange, @oldText From 6fbd019b1d181b3c36b0b0ab1702a4a4e1d7e7ca Mon Sep 17 00:00:00 2001 From: David Graham & Nathan Sobo Date: Thu, 5 Jul 2012 19:07:12 -0600 Subject: [PATCH 05/14] Pull out a BufferChangeOperation, which Buffer.change sends to UndoManager --- spec/app/buffer-spec.coffee | 5 +++ spec/app/undo-manager-spec.coffee | 2 +- src/app/buffer-change-operation.coffee | 53 ++++++++++++++++++++++++++ src/app/buffer.coffee | 30 ++++++--------- src/app/range.coffee | 2 +- src/app/undo-manager.coffee | 45 +++++++--------------- 6 files changed, 85 insertions(+), 52 deletions(-) create mode 100644 src/app/buffer-change-operation.coffee diff --git a/spec/app/buffer-spec.coffee b/spec/app/buffer-spec.coffee index a12885af5..932506576 100644 --- a/spec/app/buffer-spec.coffee +++ b/spec/app/buffer-spec.coffee @@ -220,6 +220,11 @@ describe 'Buffer', -> expect(event.oldText).toBe oldText expect(event.newText).toBe "foo\nbar" + it "allows a 'change' event handler to safely undo the change", -> + buffer.on 'change', -> buffer.undo() + buffer.change([0, 0], "hello") + expect(buffer.lineForRow(0)).toBe "var quicksort = function () {" + describe ".setText(text)", -> it "changes the entire contents of the buffer and emits a change event", -> lastRow = buffer.getLastRow() diff --git a/spec/app/undo-manager-spec.coffee b/spec/app/undo-manager-spec.coffee index d44902614..d6ccb1a3f 100644 --- a/spec/app/undo-manager-spec.coffee +++ b/spec/app/undo-manager-spec.coffee @@ -7,7 +7,7 @@ describe "UndoManager", -> beforeEach -> buffer = new Buffer(require.resolve('fixtures/sample.js')) - undoManager = new UndoManager(buffer) + undoManager = buffer.undoManager afterEach -> buffer.destroy() diff --git a/src/app/buffer-change-operation.coffee b/src/app/buffer-change-operation.coffee new file mode 100644 index 000000000..2aa0e6098 --- /dev/null +++ b/src/app/buffer-change-operation.coffee @@ -0,0 +1,53 @@ +Range = require 'range' + +module.exports = +class BufferChangeOperation + buffer: null + oldRange: null + oldText: null + newRange: null + newText: null + + constructor: ({@buffer, @oldRange, @newText}) -> + + do: -> + @oldText = @buffer.getTextInRange(@oldRange) + @newRange = @calculateNewRange(@oldRange, @newText) + @changeBuffer + oldRange: @oldRange + newRange: @newRange + oldText: @oldText + newText: @newText + + undo: -> + @changeBuffer + oldRange: @newRange + newRange: @oldRange + oldText: @newText + newText: @oldText + + changeBuffer: ({ oldRange, newRange, newText, oldText }) -> + { prefix, suffix } = @buffer.prefixAndSuffixForRange(oldRange) + + newTextLines = newText.split('\n') + if newTextLines.length == 1 + newTextLines = [prefix + newText + suffix] + else + lastLineIndex = newTextLines.length - 1 + newTextLines[0] = prefix + newTextLines[0] + newTextLines[lastLineIndex] += suffix + + @buffer.replaceLines(oldRange.start.row, oldRange.end.row, newTextLines) + @buffer.trigger 'change', { oldRange, newRange, oldText, newText } + newRange + + calculateNewRange: (oldRange, newText) -> + newRange = new Range(oldRange.start.copy(), oldRange.start.copy()) + newTextLines = newText.split('\n') + if newTextLines.length == 1 + newRange.end.column += newText.length + else + lastLineIndex = newTextLines.length - 1 + newRange.end.row += lastLineIndex + newRange.end.column = newTextLines[lastLineIndex].length + newRange diff --git a/src/app/buffer.coffee b/src/app/buffer.coffee index 86f3952e4..5614a0fda 100644 --- a/src/app/buffer.coffee +++ b/src/app/buffer.coffee @@ -5,6 +5,7 @@ Point = require 'point' Range = require 'range' EventEmitter = require 'event-emitter' UndoManager = require 'undo-manager' +BufferChangeOperation = require 'buffer-change-operation' module.exports = class Buffer @@ -130,29 +131,20 @@ class Buffer change: (oldRange, newText) -> oldRange = Range.fromObject(oldRange) - newRange = new Range(oldRange.start.copy(), oldRange.start.copy()) - prefix = @lines[oldRange.start.row][0...oldRange.start.column] - suffix = @lines[oldRange.end.row][oldRange.end.column..] - oldText = @getTextInRange(oldRange) - - newTextLines = newText.split('\n') - - if newTextLines.length == 1 - newRange.end.column += newText.length - newTextLines = [prefix + newText + suffix] + operation = new BufferChangeOperation({buffer: this, oldRange, newText}) + if @undoManager + @undoManager.perform(operation) else - lastLineIndex = newTextLines.length - 1 - newTextLines[0] = prefix + newTextLines[0] - newRange.end.row += lastLineIndex - newRange.end.column = newTextLines[lastLineIndex].length - newTextLines[lastLineIndex] += suffix + operation.do() - @lines[oldRange.start.row..oldRange.end.row] = newTextLines + prefixAndSuffixForRange: (range) -> + prefix: @lines[range.start.row][0...range.start.column] + suffix: @lines[range.end.row][range.end.column..] + + replaceLines: (startRow, endRow, newLines) -> + @lines[startRow..endRow] = newLines @modified = true - @trigger 'change', { oldRange, newRange, oldText, newText } - newRange - startUndoBatch: (selectedBufferRanges) -> @undoManager.startUndoBatch(selectedBufferRanges) diff --git a/src/app/range.coffee b/src/app/range.coffee index 4f2f9632a..924c6349a 100644 --- a/src/app/range.coffee +++ b/src/app/range.coffee @@ -22,7 +22,7 @@ class Range @start = pointB @end = pointA - copy: (range) -> + copy: -> new Range(@start.copy(), @end.copy()) isEqual: (other) -> diff --git a/src/app/undo-manager.coffee b/src/app/undo-manager.coffee index 1ec9b3682..5318614d0 100644 --- a/src/app/undo-manager.coffee +++ b/src/app/undo-manager.coffee @@ -6,36 +6,33 @@ class UndoManager undoHistory: null redoHistory: null currentBatch: null - preserveHistory: false startBatchCallCount: null constructor: (@buffer) -> @startBatchCallCount = 0 @undoHistory = [] @redoHistory = [] - @buffer.on 'change', (event) => - unless @preserveHistory - op = new BufferChangeOperation(_.extend({ @buffer }, event)) - if @currentBatch - @currentBatch.push(op) - else - @undoHistory.push([op]) - @redoHistory = [] + + perform: (operation) -> + if @currentBatch + @currentBatch.push(operation) + else + @undoHistory.push([operation]) + @redoHistory = [] + operation.do() undo: -> if batch = @undoHistory.pop() - @preservingHistory => - opsInReverse = new Array(batch...) - opsInReverse.reverse() - op.undo() for op in opsInReverse - @redoHistory.push batch + opsInReverse = new Array(batch...) + opsInReverse.reverse() + op.undo() for op in opsInReverse + @redoHistory.push batch batch.oldSelectionRanges redo: -> if batch = @redoHistory.pop() - @preservingHistory => - op.do() for op in batch - @undoHistory.push(batch) + op.do() for op in batch + @undoHistory.push(batch) batch.newSelectionRanges startUndoBatch: (ranges) -> @@ -50,17 +47,3 @@ class UndoManager @currentBatch.newSelectionRanges = ranges @undoHistory.push(@currentBatch) if @currentBatch.length > 0 @currentBatch = null - - preservingHistory: (fn) -> - @preserveHistory = true - fn() - @preserveHistory = false - -class BufferChangeOperation - constructor: ({@buffer, @oldRange, @newRange, @oldText, @newText}) -> - - do: -> - @buffer.change @oldRange, @newText - - undo: -> - @buffer.change @newRange, @oldText From c053be3394cb8390d8cb93bb2255bd6f907f0c64 Mon Sep 17 00:00:00 2001 From: David Graham & Nathan Sobo Date: Thu, 5 Jul 2012 19:46:08 -0600 Subject: [PATCH 06/14] Rename UndoManager.perform -> pushOperation I'm about to expand the types of operations so that some don't always have a `do` method. Therefore `perform` is a misnomer. --- src/app/buffer.coffee | 2 +- src/app/undo-manager.coffee | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/buffer.coffee b/src/app/buffer.coffee index 5614a0fda..a42f72608 100644 --- a/src/app/buffer.coffee +++ b/src/app/buffer.coffee @@ -133,7 +133,7 @@ class Buffer oldRange = Range.fromObject(oldRange) operation = new BufferChangeOperation({buffer: this, oldRange, newText}) if @undoManager - @undoManager.perform(operation) + @undoManager.pushOperation(operation) else operation.do() diff --git a/src/app/undo-manager.coffee b/src/app/undo-manager.coffee index 5318614d0..47ae7b1d7 100644 --- a/src/app/undo-manager.coffee +++ b/src/app/undo-manager.coffee @@ -13,7 +13,7 @@ class UndoManager @undoHistory = [] @redoHistory = [] - perform: (operation) -> + pushOperation: (operation) -> if @currentBatch @currentBatch.push(operation) else From 42eefb49a951595fde0aa5996ed2c0edce2bf750 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Thu, 5 Jul 2012 20:04:16 -0600 Subject: [PATCH 07/14] Add `UndoManager.prototype.transact` The `transact` method takes a function and batches all operations within that function as a single transaction to be undone and redone. The edit session now uses generic operations to restore selection state around transactions. The `undo` and `do` methods on operations are now optional. In addition, the undo manager now supports an optional `redo` method on an operation for code that should *only* be run on redo, and not when the operation is initially pushed. This is used by edit session to restore selection state after redo. --- spec/app/undo-manager-spec.coffee | 38 +++++++++++-------------------- src/app/buffer.coffee | 16 ++++++------- src/app/edit-session.coffee | 19 +++++++++------- src/app/undo-manager.coffee | 37 ++++++++++++++---------------- 4 files changed, 49 insertions(+), 61 deletions(-) diff --git a/spec/app/undo-manager-spec.coffee b/spec/app/undo-manager-spec.coffee index d6ccb1a3f..307412a74 100644 --- a/spec/app/undo-manager-spec.coffee +++ b/spec/app/undo-manager-spec.coffee @@ -63,49 +63,37 @@ describe "UndoManager", -> undoManager.redo() expect(buffer.getText()).toContain 'qsport' - describe "startUndoBatch() / endUndoBatch()", -> - it "causes changes in batch to be undone simultaneously and returns an array of ranges to select from undo and redo", -> + describe "transact(fn)", -> + it "causes changes in the transaction to be undone simultaneously", -> buffer.insert([0, 0], "foo") - ignoredRanges = [[[666, 666], [666, 666]], [[666, 666], [666, 666]]] - beforeRanges = [[[1, 2], [1, 2]], [[1, 9], [1, 9]]] - afterRanges =[[[1, 5], [1, 5]], [[1, 12], [1, 12]]] - - undoManager.startUndoBatch(beforeRanges) - undoManager.startUndoBatch(ignoredRanges) # calls can be nested - buffer.insert([1, 2], "111") - buffer.insert([1, 9], "222") - undoManager.endUndoBatch(ignoredRanges) # calls can be nested - undoManager.endUndoBatch(afterRanges) + undoManager.transact -> + undoManager.transact -> + buffer.insert([1, 2], "111") + buffer.insert([1, 9], "222") expect(buffer.lineForRow(1)).toBe ' 111var 222sort = function(items) {' - ranges = undoManager.undo() - expect(ranges).toBe beforeRanges + undoManager.undo() expect(buffer.lineForRow(1)).toBe ' var sort = function(items) {' expect(buffer.lineForRow(0)).toContain 'foo' - ranges = undoManager.undo() - expect(ranges).toBeUndefined() + undoManager.undo() expect(buffer.lineForRow(0)).not.toContain 'foo' - ranges = undoManager.redo() - expect(ranges).toBeUndefined() + undoManager.redo() expect(buffer.lineForRow(0)).toContain 'foo' - ranges = undoManager.redo() - expect(ranges).toBe afterRanges + undoManager.redo() expect(buffer.lineForRow(1)).toBe ' 111var 222sort = function(items) {' - ranges = undoManager.undo() - expect(ranges).toBe beforeRanges + undoManager.undo() expect(buffer.lineForRow(1)).toBe ' var sort = function(items) {' - it "does not store empty batches", -> + it "does not record empty transactions", -> buffer.insert([0,0], "foo") - undoManager.startUndoBatch() - undoManager.endUndoBatch() + undoManager.transact -> undoManager.undo() expect(buffer.lineForRow(0)).not.toContain("foo") diff --git a/src/app/buffer.coffee b/src/app/buffer.coffee index a42f72608..972ef5c34 100644 --- a/src/app/buffer.coffee +++ b/src/app/buffer.coffee @@ -132,10 +132,7 @@ class Buffer change: (oldRange, newText) -> oldRange = Range.fromObject(oldRange) operation = new BufferChangeOperation({buffer: this, oldRange, newText}) - if @undoManager - @undoManager.pushOperation(operation) - else - operation.do() + @pushOperation(operation) prefixAndSuffixForRange: (range) -> prefix: @lines[range.start.row][0...range.start.column] @@ -145,11 +142,14 @@ class Buffer @lines[startRow..endRow] = newLines @modified = true - startUndoBatch: (selectedBufferRanges) -> - @undoManager.startUndoBatch(selectedBufferRanges) + pushOperation: (operation) -> + if @undoManager + @undoManager.pushOperation(operation) + else + operation.do() - endUndoBatch: (selectedBufferRanges) -> - @undoManager.endUndoBatch(selectedBufferRanges) + transact: (fn) -> + @undoManager.transact(fn) undo: -> @undoManager.undo() diff --git a/src/app/edit-session.coffee b/src/app/edit-session.coffee index ea5073501..8428bd24f 100644 --- a/src/app/edit-session.coffee +++ b/src/app/edit-session.coffee @@ -177,12 +177,10 @@ class EditSession @insertText($native.readFromPasteboard()) undo: -> - if ranges = @buffer.undo() - @setSelectedBufferRanges(ranges) + @buffer.undo() redo: -> - if ranges = @buffer.redo() - @setSelectedBufferRanges(ranges) + @buffer.redo() foldSelection: -> selection.fold() for selection in @getSelections() @@ -234,10 +232,15 @@ class EditSession @tokenizedBuffer.toggleLineCommentsInRange(range) mutateSelectedText: (fn) -> - selections = @getSelections() - @buffer.startUndoBatch(@getSelectedBufferRanges()) - fn(selection) for selection in selections - @buffer.endUndoBatch(@getSelectedBufferRanges()) + @transact => fn(selection) for selection in @getSelections() + + transact: (fn) -> + @buffer.transact => + oldSelectedRanges = @getSelectedBufferRanges() + @buffer.pushOperation(undo: => @setSelectedBufferRanges(oldSelectedRanges)) + fn() + newSelectedRanges = @getSelectedBufferRanges() + @buffer.pushOperation(redo: => @setSelectedBufferRanges(newSelectedRanges)) getAnchors: -> new Array(@anchors...) diff --git a/src/app/undo-manager.coffee b/src/app/undo-manager.coffee index 47ae7b1d7..abfe16072 100644 --- a/src/app/undo-manager.coffee +++ b/src/app/undo-manager.coffee @@ -5,8 +5,7 @@ module.exports = class UndoManager undoHistory: null redoHistory: null - currentBatch: null - startBatchCallCount: null + currentTransaction: null constructor: (@buffer) -> @startBatchCallCount = 0 @@ -14,36 +13,34 @@ class UndoManager @redoHistory = [] pushOperation: (operation) -> - if @currentBatch - @currentBatch.push(operation) + if @currentTransaction + @currentTransaction.push(operation) else @undoHistory.push([operation]) @redoHistory = [] - operation.do() + operation.do?() + + transact: (fn) -> + if @currentTransaction + fn() + else + @currentTransaction = [] + fn() + @undoHistory.push(@currentTransaction) if @currentTransaction.length + @currentTransaction = null undo: -> if batch = @undoHistory.pop() opsInReverse = new Array(batch...) opsInReverse.reverse() - op.undo() for op in opsInReverse + op.undo?() for op in opsInReverse @redoHistory.push batch batch.oldSelectionRanges redo: -> if batch = @redoHistory.pop() - op.do() for op in batch + for op in batch + op.do?() + op.redo?() @undoHistory.push(batch) batch.newSelectionRanges - - startUndoBatch: (ranges) -> - @startBatchCallCount++ - return if @startBatchCallCount > 1 - @currentBatch = [] - @currentBatch.oldSelectionRanges = ranges - - endUndoBatch: (ranges) -> - @startBatchCallCount-- - return if @startBatchCallCount > 0 - @currentBatch.newSelectionRanges = ranges - @undoHistory.push(@currentBatch) if @currentBatch.length > 0 - @currentBatch = null From 53d6c4960a1040cdfc42524cb0308a730d555606 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Thu, 5 Jul 2012 20:06:09 -0600 Subject: [PATCH 08/14] Fix specs that broke due to no subscription on `Buffer` from `UndoManager` --- spec/app/editor-spec.coffee | 4 ++-- spec/app/window-spec.coffee | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/app/editor-spec.coffee b/spec/app/editor-spec.coffee index 23e737dd4..bce9373d3 100644 --- a/spec/app/editor-spec.coffee +++ b/spec/app/editor-spec.coffee @@ -120,8 +120,8 @@ describe "Editor", -> expect(otherEditSession.buffer.subscriptionCount()).toBeGreaterThan 1 editor.remove() - expect(previousEditSession.buffer.subscriptionCount()).toBe 1 - expect(otherEditSession.buffer.subscriptionCount()).toBe 1 + expect(previousEditSession.buffer.subscriptionCount()).toBe 0 + expect(otherEditSession.buffer.subscriptionCount()).toBe 0 describe "when 'close' is triggered", -> it "closes active edit session and loads next edit session", -> diff --git a/spec/app/window-spec.coffee b/spec/app/window-spec.coffee index b19a90940..efe050d53 100644 --- a/spec/app/window-spec.coffee +++ b/spec/app/window-spec.coffee @@ -61,4 +61,4 @@ describe "Window", -> $(window).trigger 'beforeunload' - expect(editor1.getBuffer().subscriptionCount()).toBe 1 # buffer has a self-subscription for the undo manager + expect(editor1.getBuffer().subscriptionCount()).toBe 0 From 523f240fe37df134c4be41693145a68fde95c1e9 Mon Sep 17 00:00:00 2001 From: Corey Johnson & Nathan Sobo Date: Fri, 6 Jul 2012 11:40:36 -0600 Subject: [PATCH 09/14] Properly dereference smart-pointer in native reload logic --- Atom/src/AtomController.mm | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Atom/src/AtomController.mm b/Atom/src/AtomController.mm index 8bc92a916..a8c4373c5 100644 --- a/Atom/src/AtomController.mm +++ b/Atom/src/AtomController.mm @@ -118,11 +118,13 @@ CefRefPtr retval; CefRefPtr exception; CefV8ValueList arguments; + global->GetValue("reload")->ExecuteFunction(global, arguments, retval, exception, true); - if (exception) _clientHandler->GetBrowser()->ReloadIgnoreCache(); + if (exception.get()) { + _clientHandler->GetBrowser()->ReloadIgnoreCache(); + } context->Exit(); - return YES; } From 6177b46cf9c233bba1a47579008a12c64d904043 Mon Sep 17 00:00:00 2001 From: Corey Johnson & Nathan Sobo Date: Fri, 6 Jul 2012 11:42:07 -0600 Subject: [PATCH 10/14] Restore selection on active edit session when undoing/redoing The `do`, `undo`, and `redo` methods on operations take an optional editSession argument, which can be used to determine the context in which they are being run. We restore selections on that edit session instead of the session where the operations originally occurred. --- spec/app/edit-session-spec.coffee | 24 ++++++++++++++++-------- src/app/buffer.coffee | 8 ++++---- src/app/edit-session.coffee | 14 ++++++++++---- src/app/project.coffee | 22 +++++++++++++--------- src/app/undo-manager.coffee | 12 ++++++------ 5 files changed, 49 insertions(+), 31 deletions(-) diff --git a/spec/app/edit-session-spec.coffee b/spec/app/edit-session-spec.coffee index 34e24d637..d8ae398c3 100644 --- a/spec/app/edit-session-spec.coffee +++ b/spec/app/edit-session-spec.coffee @@ -6,18 +6,16 @@ describe "EditSession", -> [buffer, editSession, lineLengths] = [] beforeEach -> - buffer = new Buffer(require.resolve('fixtures/sample.js')) - editSession = new EditSession - buffer: buffer - tabText: ' ' - autoIndent: false - softWrap: false - project: new Project() + buffer = new Buffer() + editSession = fixturesProject.open('sample.js', autoIndent: false) + console.log editSession.tabText + + buffer = editSession.buffer lineLengths = buffer.getLines().map (line) -> line.length afterEach -> - buffer.destroy() + fixturesProject.destroy() describe "cursor movement", -> describe ".setCursorScreenPosition(screenPosition)", -> @@ -1263,6 +1261,16 @@ describe "EditSession", -> expect(selections[0].getBufferRange()).toEqual [[1, 6], [1, 6]] expect(selections[1].getBufferRange()).toEqual [[1, 18], [1, 18]] + it "restores selected ranges even when the change occurred in another edit session", -> + otherEditSession = fixturesProject.open(editSession.getPath()) + otherEditSession.setSelectedBufferRange([[2, 2], [3, 3]]) + otherEditSession.delete() + + editSession.undo() + + expect(editSession.getSelectedBufferRange()).toEqual [[2, 2], [3, 3]] + expect(otherEditSession.getSelectedBufferRange()).toEqual [[3, 3], [3, 3]] + describe "when the buffer is changed (via its direct api, rather than via than edit session)", -> it "moves the cursor so it is in the same relative position of the buffer", -> expect(editSession.getCursorScreenPosition()).toEqual [0, 0] diff --git a/src/app/buffer.coffee b/src/app/buffer.coffee index 972ef5c34..d84692e93 100644 --- a/src/app/buffer.coffee +++ b/src/app/buffer.coffee @@ -151,11 +151,11 @@ class Buffer transact: (fn) -> @undoManager.transact(fn) - undo: -> - @undoManager.undo() + undo: (editSession) -> + @undoManager.undo(editSession) - redo: -> - @undoManager.redo() + redo: (editSession) -> + @undoManager.redo(editSession) save: -> @saveAs(@getPath()) diff --git a/src/app/edit-session.coffee b/src/app/edit-session.coffee index 8428bd24f..4300855b9 100644 --- a/src/app/edit-session.coffee +++ b/src/app/edit-session.coffee @@ -95,6 +95,7 @@ class EditSession new Point(row, column) getFileExtension: -> @buffer.getExtension() + getPath: -> @buffer.getPath() getEofBufferPosition: -> @buffer.getEofPosition() bufferRangeForBufferRow: (row) -> @buffer.rangeForRow(row) lineForBufferRow: (row) -> @buffer.lineForRow(row) @@ -177,10 +178,10 @@ class EditSession @insertText($native.readFromPasteboard()) undo: -> - @buffer.undo() + @buffer.undo(this) redo: -> - @buffer.redo() + @buffer.redo(this) foldSelection: -> selection.fold() for selection in @getSelections() @@ -237,10 +238,15 @@ class EditSession transact: (fn) -> @buffer.transact => oldSelectedRanges = @getSelectedBufferRanges() - @buffer.pushOperation(undo: => @setSelectedBufferRanges(oldSelectedRanges)) + @buffer.pushOperation + undo: (editSession) -> + editSession?.setSelectedBufferRanges(oldSelectedRanges) + fn() newSelectedRanges = @getSelectedBufferRanges() - @buffer.pushOperation(redo: => @setSelectedBufferRanges(newSelectedRanges)) + @buffer.pushOperation + redo: (editSession) -> + editSession?.setSelectedBufferRanges(newSelectedRanges) getAnchors: -> new Array(@anchors...) diff --git a/src/app/project.coffee b/src/app/project.coffee index 11a05c820..40ddfc523 100644 --- a/src/app/project.coffee +++ b/src/app/project.coffee @@ -75,26 +75,30 @@ class Project getSoftWrap: -> @softWrap setSoftWrap: (@softWrap) -> - open: (filePath) -> + open: (filePath, editSessionOptions={}) -> if filePath? filePath = @resolve(filePath) buffer = @bufferWithPath(filePath) ? @buildBuffer(filePath) else buffer = @buildBuffer() - editSession = new EditSession - project: this - buffer: buffer - tabText: @getTabText() - autoIndent: @getAutoIndent() - softTabs: @getSoftTabs() - softWrap: @getSoftWrap() - + @buildEditSession(buffer, editSessionOptions) + buildEditSession: (buffer, editSessionOptions) -> + options = _.extend(@defaultEditSessionOptions(), editSessionOptions) + options.project = this + options.buffer = buffer + editSession = new EditSession(options) @editSessions.push editSession @trigger 'new-edit-session', editSession editSession + defaultEditSessionOptions: -> + tabText: @getTabText() + autoIndent: @getAutoIndent() + softTabs: @getSoftTabs() + softWrap: @getSoftWrap() + destroy: -> for editSession in _.clone(@editSessions) @removeEditSession(editSession) diff --git a/src/app/undo-manager.coffee b/src/app/undo-manager.coffee index abfe16072..c3bcaa0df 100644 --- a/src/app/undo-manager.coffee +++ b/src/app/undo-manager.coffee @@ -7,7 +7,7 @@ class UndoManager redoHistory: null currentTransaction: null - constructor: (@buffer) -> + constructor: -> @startBatchCallCount = 0 @undoHistory = [] @redoHistory = [] @@ -29,18 +29,18 @@ class UndoManager @undoHistory.push(@currentTransaction) if @currentTransaction.length @currentTransaction = null - undo: -> + undo: (editSession) -> if batch = @undoHistory.pop() opsInReverse = new Array(batch...) opsInReverse.reverse() - op.undo?() for op in opsInReverse + op.undo?(editSession) for op in opsInReverse @redoHistory.push batch batch.oldSelectionRanges - redo: -> + redo: (editSession) -> if batch = @redoHistory.pop() for op in batch - op.do?() - op.redo?() + op.do?(editSession) + op.redo?(editSession) @undoHistory.push(batch) batch.newSelectionRanges From 9f9c636883f9285d439495a70e77e7d1112d6182 Mon Sep 17 00:00:00 2001 From: Corey Johnson & Nathan Sobo Date: Fri, 6 Jul 2012 12:09:45 -0600 Subject: [PATCH 11/14] When snippet expansion is undone, the snippet is destroyed This is still in progress. You can't *redo* snippet expansion and restore tab stops. Also, this commit performs all changes associated with snippet expansion in a transaction. --- spec/app/edit-session-spec.coffee | 3 --- spec/extensions/snippets-spec.coffee | 5 ++--- src/app/buffer.coffee | 4 ++-- src/app/edit-session.coffee | 7 +++++-- src/app/undo-manager.coffee | 4 ++-- src/extensions/snippets/snippet-expansion.coffee | 11 ++++++----- src/extensions/snippets/snippets.coffee | 5 ++++- 7 files changed, 21 insertions(+), 18 deletions(-) diff --git a/spec/app/edit-session-spec.coffee b/spec/app/edit-session-spec.coffee index d8ae398c3..2bdc18d68 100644 --- a/spec/app/edit-session-spec.coffee +++ b/spec/app/edit-session-spec.coffee @@ -8,9 +8,6 @@ describe "EditSession", -> beforeEach -> buffer = new Buffer() editSession = fixturesProject.open('sample.js', autoIndent: false) - - console.log editSession.tabText - buffer = editSession.buffer lineLengths = buffer.getLines().map (line) -> line.length diff --git a/spec/extensions/snippets-spec.coffee b/spec/extensions/snippets-spec.coffee index 39ad9a9b9..f10f1d785 100644 --- a/spec/extensions/snippets-spec.coffee +++ b/spec/extensions/snippets-spec.coffee @@ -43,7 +43,7 @@ describe "Snippets extension", -> endsnippet snippet t5 "Caused problems with undo" - first line $1 + first line$1 ${2:placeholder ending second line} endsnippet """ @@ -150,13 +150,12 @@ describe "Snippets extension", -> expect(editor.getCursorScreenPosition()).toEqual [0, 5] describe "when a previous snippet expansion has just been undone", -> - xit "expands the snippet based on the current prefix rather than jumping to the old snippet's tab stop", -> + it "expands the snippet based on the current prefix rather than jumping to the old snippet's tab stop", -> editor.insertText 't5\n' editor.setCursorBufferPosition [0, 2] editor.trigger keydownEvent('tab', target: editor[0]) expect(buffer.lineForRow(0)).toBe "first line" editor.undo() - editor.undo() expect(buffer.lineForRow(0)).toBe "t5" editor.trigger keydownEvent('tab', target: editor[0]) expect(buffer.lineForRow(0)).toBe "first line" diff --git a/src/app/buffer.coffee b/src/app/buffer.coffee index d84692e93..21aa60e57 100644 --- a/src/app/buffer.coffee +++ b/src/app/buffer.coffee @@ -142,9 +142,9 @@ class Buffer @lines[startRow..endRow] = newLines @modified = true - pushOperation: (operation) -> + pushOperation: (operation, editSession) -> if @undoManager - @undoManager.pushOperation(operation) + @undoManager.pushOperation(operation, editSession) else operation.do() diff --git a/src/app/edit-session.coffee b/src/app/edit-session.coffee index 4300855b9..90af7d268 100644 --- a/src/app/edit-session.coffee +++ b/src/app/edit-session.coffee @@ -238,16 +238,19 @@ class EditSession transact: (fn) -> @buffer.transact => oldSelectedRanges = @getSelectedBufferRanges() - @buffer.pushOperation + @pushOperation undo: (editSession) -> editSession?.setSelectedBufferRanges(oldSelectedRanges) fn() newSelectedRanges = @getSelectedBufferRanges() - @buffer.pushOperation + @pushOperation redo: (editSession) -> editSession?.setSelectedBufferRanges(newSelectedRanges) + pushOperation: (operation) -> + @buffer.pushOperation(operation, this) + getAnchors: -> new Array(@anchors...) diff --git a/src/app/undo-manager.coffee b/src/app/undo-manager.coffee index c3bcaa0df..21760ef62 100644 --- a/src/app/undo-manager.coffee +++ b/src/app/undo-manager.coffee @@ -12,13 +12,13 @@ class UndoManager @undoHistory = [] @redoHistory = [] - pushOperation: (operation) -> + pushOperation: (operation, editSession) -> if @currentTransaction @currentTransaction.push(operation) else @undoHistory.push([operation]) @redoHistory = [] - operation.do?() + operation.do?(editSession) transact: (fn) -> if @currentTransaction diff --git a/src/extensions/snippets/snippet-expansion.coffee b/src/extensions/snippets/snippet-expansion.coffee index aaec2522a..963bfd95f 100644 --- a/src/extensions/snippets/snippet-expansion.coffee +++ b/src/extensions/snippets/snippet-expansion.coffee @@ -5,11 +5,12 @@ class SnippetExpansion constructor: (snippet, @editSession) -> @editSession.selectToBeginningOfWord() startPosition = @editSession.getCursorBufferPosition() - @editSession.insertText(snippet.body) - if snippet.tabStops.length - @placeTabStopAnchorRanges(startPosition, snippet.tabStops) - if snippet.lineCount > 1 - @indentSubsequentLines(startPosition.row, snippet) + @editSession.transact => + @editSession.insertText(snippet.body) + if snippet.tabStops.length + @placeTabStopAnchorRanges(startPosition, snippet.tabStops) + if snippet.lineCount > 1 + @indentSubsequentLines(startPosition.row, snippet) placeTabStopAnchorRanges: (startPosition, tabStopRanges) -> @tabStopAnchorRanges = tabStopRanges.map ({start, end}) => diff --git a/src/extensions/snippets/snippets.coffee b/src/extensions/snippets/snippets.coffee index 78d7a1b26..dbc0bff43 100644 --- a/src/extensions/snippets/snippets.coffee +++ b/src/extensions/snippets/snippets.coffee @@ -28,7 +28,10 @@ module.exports = editSession = editor.activeEditSession prefix = editSession.getLastCursor().getCurrentWordPrefix() if snippet = @snippetsByExtension[editSession.getFileExtension()][prefix] - editSession.snippetExpansion = new SnippetExpansion(snippet, editSession) + editSession.transact -> + editSession.snippetExpansion = new SnippetExpansion(snippet, editSession) + editSession.pushOperation + undo: -> editSession.snippetExpansion.destroy() else e.abortKeyBinding() From 5554d15bdb8bc52fe76cefa030650a0429c3be64 Mon Sep 17 00:00:00 2001 From: Corey Johnson & Nathan Sobo Date: Fri, 6 Jul 2012 12:14:57 -0600 Subject: [PATCH 12/14] Add sample .atom directory to project root --- .atom/atom.coffee | 10 ++++++++++ .atom/snippets/coffee.snippets | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) create mode 100644 .atom/atom.coffee create mode 100644 .atom/snippets/coffee.snippets diff --git a/.atom/atom.coffee b/.atom/atom.coffee new file mode 100644 index 000000000..9571718da --- /dev/null +++ b/.atom/atom.coffee @@ -0,0 +1,10 @@ +requireExtension 'autocomplete' +requireExtension 'strip-trailing-whitespace' +requireExtension 'fuzzy-finder' +requireExtension 'tree-view' +requireExtension 'command-panel' +requireExtension 'keybindings-view' +requireExtension 'snippets' + +# status-bar is a bit broken until webkit gets a decent flexbox implementation +# requireExtension 'status-bar' diff --git a/.atom/snippets/coffee.snippets b/.atom/snippets/coffee.snippets new file mode 100644 index 000000000..57b3e3980 --- /dev/null +++ b/.atom/snippets/coffee.snippets @@ -0,0 +1,34 @@ +snippet de "Describe block" +describe "${1:description}", -> + ${2:body} +endsnippet + +snippet i "It block" +it "$1", -> + $2 +endsnippet + +snippet be "Before each" +beforeEach -> + $1 +endsnippet + +snippet ex "Expectation" +expect($1).to$2 +endsnippet + +snippet log "Console log" +console.log $1 +endsnippet + +snippet ra "Range array" +[[$1, $2], [$3, $4]] +endsnippet + +snippet pt "Point array" +[$1, $2] +endsnippet + +snippet spy "Jasmine spy" +jasmine.createSpy("${1:description}")$2 +endsnippet From d6912c5913e1f91f5c0a8b4ca5d75b294caa260d Mon Sep 17 00:00:00 2001 From: Corey Johnson & Nathan Sobo Date: Fri, 6 Jul 2012 11:50:42 -0700 Subject: [PATCH 13/14] When a snippet expansion is redone, tab stops are restored --- spec/extensions/snippets-spec.coffee | 13 +++++++++++++ src/extensions/snippets/snippet-expansion.coffee | 4 ++++ src/extensions/snippets/snippets.coffee | 6 +++++- 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/spec/extensions/snippets-spec.coffee b/spec/extensions/snippets-spec.coffee index f10f1d785..7b8c3bfcc 100644 --- a/spec/extensions/snippets-spec.coffee +++ b/spec/extensions/snippets-spec.coffee @@ -160,6 +160,19 @@ describe "Snippets extension", -> editor.trigger keydownEvent('tab', target: editor[0]) expect(buffer.lineForRow(0)).toBe "first line" + describe "when a snippet expansion is undone and redone", -> + it "recreates the snippet's tab stops", -> + editor.insertText ' t5\n' + editor.setCursorBufferPosition [0, 6] + editor.trigger keydownEvent('tab', target: editor[0]) + expect(buffer.lineForRow(0)).toBe " first line" + editor.undo() + editor.redo() + + expect(editor.getCursorBufferPosition()).toEqual [0, 14] + editor.trigger keydownEvent('tab', target: editor[0]) + expect(editor.getSelectedBufferRange()).toEqual [[1, 6], [1, 36]] + describe ".loadSnippetsFile(path)", -> it "loads the snippets in the given file", -> spyOn(fs, 'read').andReturn """ diff --git a/src/extensions/snippets/snippet-expansion.coffee b/src/extensions/snippets/snippet-expansion.coffee index 963bfd95f..66800e1da 100644 --- a/src/extensions/snippets/snippet-expansion.coffee +++ b/src/extensions/snippets/snippet-expansion.coffee @@ -54,3 +54,7 @@ class SnippetExpansion destroy: -> anchorRange.destroy() for anchorRange in @tabStopAnchorRanges @editSession.snippetExpansion = null + + restoreTabStops: -> + @tabStopAnchorRanges = @tabStopAnchorRanges.map (anchorRange) => + @editSession.addAnchorRange(anchorRange.getBufferRange()) diff --git a/src/extensions/snippets/snippets.coffee b/src/extensions/snippets/snippets.coffee index dbc0bff43..d283157da 100644 --- a/src/extensions/snippets/snippets.coffee +++ b/src/extensions/snippets/snippets.coffee @@ -29,9 +29,13 @@ module.exports = prefix = editSession.getLastCursor().getCurrentWordPrefix() if snippet = @snippetsByExtension[editSession.getFileExtension()][prefix] editSession.transact -> - editSession.snippetExpansion = new SnippetExpansion(snippet, editSession) + snippetExpansion = new SnippetExpansion(snippet, editSession) + editSession.snippetExpansion = snippetExpansion editSession.pushOperation undo: -> editSession.snippetExpansion.destroy() + redo: -> + editSession.snippetExpansion = snippetExpansion + snippetExpansion.restoreTabStops() else e.abortKeyBinding() From 478a334b73cc95e188e2a9087d2203fbb5d45255 Mon Sep 17 00:00:00 2001 From: Corey Johnson & Nathan Sobo Date: Fri, 6 Jul 2012 12:12:14 -0700 Subject: [PATCH 14/14] Redoing a snippet expansion restores tab stops to the *current* edit session --- spec/extensions/snippets-spec.coffee | 14 ++++++++++++++ src/extensions/snippets/snippet-expansion.coffee | 3 ++- src/extensions/snippets/snippets.coffee | 6 ++---- 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/spec/extensions/snippets-spec.coffee b/spec/extensions/snippets-spec.coffee index 7b8c3bfcc..f56cf7a2e 100644 --- a/spec/extensions/snippets-spec.coffee +++ b/spec/extensions/snippets-spec.coffee @@ -173,6 +173,20 @@ describe "Snippets extension", -> editor.trigger keydownEvent('tab', target: editor[0]) expect(editor.getSelectedBufferRange()).toEqual [[1, 6], [1, 36]] + it "restores tabs stops in active edit session even when the initial expansion was in a different edit session", -> + anotherEditor = editor.splitRight() + + editor.insertText ' t5\n' + editor.setCursorBufferPosition [0, 6] + editor.trigger keydownEvent('tab', target: editor[0]) + expect(buffer.lineForRow(0)).toBe " first line" + editor.undo() + + anotherEditor.redo() + expect(anotherEditor.getCursorBufferPosition()).toEqual [0, 14] + anotherEditor.trigger keydownEvent('tab', target: anotherEditor[0]) + expect(anotherEditor.getSelectedBufferRange()).toEqual [[1, 6], [1, 36]] + describe ".loadSnippetsFile(path)", -> it "loads the snippets in the given file", -> spyOn(fs, 'read').andReturn """ diff --git a/src/extensions/snippets/snippet-expansion.coffee b/src/extensions/snippets/snippet-expansion.coffee index 66800e1da..332e8e695 100644 --- a/src/extensions/snippets/snippet-expansion.coffee +++ b/src/extensions/snippets/snippet-expansion.coffee @@ -55,6 +55,7 @@ class SnippetExpansion anchorRange.destroy() for anchorRange in @tabStopAnchorRanges @editSession.snippetExpansion = null - restoreTabStops: -> + restore: (@editSession) -> + @editSession.snippetExpansion = this @tabStopAnchorRanges = @tabStopAnchorRanges.map (anchorRange) => @editSession.addAnchorRange(anchorRange.getBufferRange()) diff --git a/src/extensions/snippets/snippets.coffee b/src/extensions/snippets/snippets.coffee index d283157da..d1c51de9d 100644 --- a/src/extensions/snippets/snippets.coffee +++ b/src/extensions/snippets/snippets.coffee @@ -32,10 +32,8 @@ module.exports = snippetExpansion = new SnippetExpansion(snippet, editSession) editSession.snippetExpansion = snippetExpansion editSession.pushOperation - undo: -> editSession.snippetExpansion.destroy() - redo: -> - editSession.snippetExpansion = snippetExpansion - snippetExpansion.restoreTabStops() + undo: -> snippetExpansion.destroy() + redo: (editSession) -> snippetExpansion.restore(editSession) else e.abortKeyBinding()