Explicitly autoscroll when needed

Rather than when the selection’s marker changes. This is simpler than
suppressing autoscroll via state when we don’t want it. It also captures
the intent to autoscroll when attempting to move the cursor at the
beginning or end of the document.

Signed-off-by: Max Brunsfeld <maxbrunsfeld@gmail.com>

src/cursor.coffee

@@ -624,8 +624,8 @@ class Cursor extends Model
   clearAutoscroll: ->
 
   # Public: Deselects the current selection.
-  clearSelection: ->
-    @selection?.clear()
+  clearSelection: (options) ->
+    @selection?.clear(options)
 
   # Public: Get the RegExp used by the cursor to determine what a "word" is.
   #
@@ -648,13 +648,9 @@ class Cursor extends Model
   ###
 
   changePosition: (options, fn) ->
-    @clearSelection()
-    @editor.suppressAutoscroll = true if options.autoscroll is false
-    try
-      fn()
-    finally
-      @editor.suppressAutoscroll = false if options?.autoscroll is false
-    @autoscroll() if options.autoscroll is true
+    @clearSelection(options)
+    fn()
+    @autoscroll() if options.autoscroll ? @isLastCursor()
 
   getPixelRect: ->
     @editor.pixelRectForScreenRange(@getScreenRange())
@@ -664,8 +660,7 @@ class Cursor extends Model
     new Range(new Point(row, column), new Point(row, column + 1))
 
   autoscroll: (options) ->
-    unless @editor.suppressAutoscroll
-      @editor.scrollToScreenRange(@getScreenRange(), options)
+    @editor.scrollToScreenRange(@getScreenRange(), options)
 
   getBeginningOfNextParagraphBufferPosition: ->
     start = @getBufferPosition()

src/selection.coffee

@@ -1,6 +1,6 @@
 {Point, Range} = require 'text-buffer'
 {Model} = require 'theorist'
-{pick} = require 'underscore-plus'
+{pick} = _ = require 'underscore-plus'
 {Emitter} = require 'event-kit'
 Grim = require 'grim'
 
@@ -34,6 +34,9 @@ class Selection extends Model
   destroy: ->
     @marker.destroy()
 
+  isLastSelection: ->
+    this is @editor.getLastSelection()
+
   ###
   Section: Event Subscription
   ###
@@ -107,10 +110,8 @@ class Selection extends Model
     @modifySelection =>
       needsFlash = options.flash
       delete options.flash if options.flash?
-      @editor.suppressAutoscroll = true if options.autoscroll is false
       @marker.setBufferRange(bufferRange, options)
-      @editor.suppressAutoscroll = false if options.autoscroll is false
-      @autoscroll() if options?.autoscroll is true
+      @autoscroll() if options?.autoscroll ? @isLastSelection()
       @decoration.flash('flash', @editor.selectionFlashDuration) if needsFlash
 
   # Public: Returns the starting and ending buffer rows the selection is
@@ -192,11 +193,9 @@ class Selection extends Model
   #     range. Defaults to `true` if this is the most recently added selection,
   #     `false` otherwise.
   clear: (options) ->
-    @editor.suppressAutoscroll = true if options?.autoscroll is false
     @marker.setProperties(goalScreenRange: null)
     @marker.clearTail() unless @retainSelection
-    @editor.suppressAutoscroll = false if options?.autoscroll is false
-    @autoscroll() if options?.autoscroll is true
+    @autoscroll() if options?.autoscroll ? @isLastSelection()
     @finalize()
 
   # Public: Selects the text from the current cursor position to a given screen
@@ -407,6 +406,8 @@ class Selection extends Model
     else if options.autoDecreaseIndent and NonWhitespaceRegExp.test(text)
       @editor.autoDecreaseIndentForBufferRow(newBufferRange.start.row)
 
+    @autoscroll() if @isLastSelection()
+
     newBufferRange
 
   # Public: Removes the first character before the selection if the selection
@@ -722,7 +723,7 @@ class Selection extends Model
     else
       options.goalScreenRange = myGoalScreenRange ? otherGoalScreenRange
 
-    @setBufferRange(@getBufferRange().union(otherSelection.getBufferRange()), options)
+    @setBufferRange(@getBufferRange().union(otherSelection.getBufferRange()), _.extend(autoscroll: false, options))
     otherSelection.destroy()
 
   ###
@@ -746,12 +747,6 @@ class Selection extends Model
     {oldHeadBufferPosition, oldTailBufferPosition} = e
     {oldHeadScreenPosition, oldTailScreenPosition} = e
 
-    if this is @editor.getLastSelection()
-      if @marker.hasTail()
-        @autoscroll()
-      else
-        @cursor.autoscroll()
-
     eventObject =
       oldBufferRange: new Range(oldHeadBufferPosition, oldTailBufferPosition)
       oldScreenRange: new Range(oldHeadScreenPosition, oldTailScreenPosition)
@@ -770,8 +765,10 @@ class Selection extends Model
       @linewise = false
 
   autoscroll: ->
-    unless @editor.suppressAutoscroll
+    if @marker.hasTail()
       @editor.scrollToScreenRange(@getScreenRange(), reversed: @isReversed())
+    else
+      @cursor.autoscroll()
 
   clearAutoscroll: ->
 

src/text-editor.coffee

@@ -69,7 +69,6 @@ class TextEditor extends Model
   suppressSelectionMerging: false
   updateBatchDepth: 0
   selectionFlashDuration: 500
-  suppressAutoscroll: false
 
   @delegatesMethods 'suggestedIndentForBufferRow', 'autoIndentBufferRow', 'autoIndentBufferRows',
     'autoDecreaseIndentForBufferRow', 'toggleLineCommentForBufferRow', 'toggleLineCommentsForBufferRows',
@@ -1133,11 +1132,13 @@ class TextEditor extends Model
 
   # Essential: Undo the last change.
   undo: ->
-    @buffer.undo(this)
+    @buffer.undo()
+    @getLastSelection().autoscroll()
 
   # Essential: Redo the last change.
   redo: ->
     @buffer.redo(this)
+    @getLastSelection().autoscroll()
 
   # Extended: Batch multiple operations as a single undo/redo step.
   #
@@ -1608,9 +1609,8 @@ class TextEditor extends Model
   #
   # Returns a {Cursor}.
   addCursorAtBufferPosition: (bufferPosition, options) ->
-    @suppressAutoscroll = true if options?.autoscroll is false
     @markBufferPosition(bufferPosition, @getSelectionMarkerAttributes())
-    @suppressAutoscroll = false if options?.autoscroll is false
+    @getLastSelection().cursor.autoscroll() unless options?.autoscroll is false
     @getLastSelection().cursor
 
   # Essential: Add a cursor at the position in screen coordinates.
@@ -1619,9 +1619,8 @@ class TextEditor extends Model
   #
   # Returns a {Cursor}.
   addCursorAtScreenPosition: (screenPosition, options) ->
-    @suppressAutoscroll = true if options?.autoscroll is false
     @markScreenPosition(screenPosition, @getSelectionMarkerAttributes())
-    @suppressAutoscroll = false if options?.autoscroll is false
+    @getLastSelection().cursor.autoscroll() unless options?.autoscroll is false
     @getLastSelection().cursor
 
   # Essential: Returns {Boolean} indicating whether or not there are multiple cursors.
@@ -1951,9 +1950,8 @@ class TextEditor extends Model
   #
   # Returns the added {Selection}.
   addSelectionForBufferRange: (bufferRange, options={}) ->
-    @suppressAutoscroll = true if options.autoscroll is false
     @markBufferRange(bufferRange, _.defaults(@getSelectionMarkerAttributes(), options))
-    @suppressAutoscroll = false if options.autoscroll is false
+    @getLastSelection().autoscroll() unless options.autoscroll is false
     @getLastSelection()
 
   # Essential: Add a selection for the given range in screen coordinates.
@@ -1965,9 +1963,8 @@ class TextEditor extends Model
   #
   # Returns the added {Selection}.
   addSelectionForScreenRange: (screenRange, options={}) ->
-    @suppressAutoscroll = true if options.autoscroll is false
     @markScreenRange(screenRange, _.defaults(@getSelectionMarkerAttributes(), options))
-    @suppressAutoscroll = false if options.autoscroll is false
+    @getLastSelection().autoscroll() unless options.autoscroll is false
     @getLastSelection()
 
   # Essential: Select from the current cursor position to the given position in
@@ -2278,10 +2275,8 @@ class TextEditor extends Model
     if selection.destroyed
       for selection in @getSelections()
         if selection.intersectsBufferRange(selectionBufferRange)
-          selection.autoscroll()
           return selection
     else
-      selection.autoscroll() unless options.autoscroll is false
       @emit 'selection-added', selection
       @emitter.emit 'did-add-selection', selection
       selection