Merge pull request #5278 from adzenith/tab-selection-fix

* Make atomic tokens clipping configurable
* Use `closest` as the default configuration

spec/display-buffer-spec.coffee

@@ -638,8 +638,11 @@ describe "DisplayBuffer", ->
         expect(displayBuffer.outermostFoldsInBufferRowRange(3, 18)).toEqual [fold1, fold3, fold5]
         expect(displayBuffer.outermostFoldsInBufferRowRange(5, 16)).toEqual [fold3]
 
-  describe "::clipScreenPosition(screenPosition, wrapBeyondNewlines: false, wrapAtSoftNewlines: false, skipAtomicTokens: false)", ->
+  describe "::clipScreenPosition(screenPosition, wrapBeyondNewlines: false, wrapAtSoftNewlines: false, clip: 'closest')", ->
     beforeEach ->
+      tabLength = 4
+
+      displayBuffer.setTabLength(tabLength)
       displayBuffer.setSoftWrapped(true)
       displayBuffer.setEditorWidthInChars(50)
 
@@ -698,19 +701,28 @@ describe "DisplayBuffer", ->
         expect(displayBuffer.clipScreenPosition([3, 58], wrapAtSoftNewlines: true)).toEqual [4, 4]
         expect(displayBuffer.clipScreenPosition([3, 1000], wrapAtSoftNewlines: true)).toEqual [4, 4]
 
-    describe "when skipAtomicTokens is false (the default)", ->
-      it "clips screen positions in the middle of atomic tab characters to the beginning of the character", ->
+    describe "when clip is 'closest' (the default)", ->
+      it "clips screen positions in the middle of atomic tab characters to the closest edge of the character", ->
         buffer.insert([0, 0], '\t')
         expect(displayBuffer.clipScreenPosition([0, 0])).toEqual [0, 0]
         expect(displayBuffer.clipScreenPosition([0, 1])).toEqual [0, 0]
+        expect(displayBuffer.clipScreenPosition([0, 2])).toEqual [0, 0]
+        expect(displayBuffer.clipScreenPosition([0, tabLength-1])).toEqual [0, tabLength]
         expect(displayBuffer.clipScreenPosition([0, tabLength])).toEqual [0, tabLength]
 
-    describe "when skipAtomicTokens is true", ->
+    describe "when clip is 'backward'", ->
+      it "clips screen positions in the middle of atomic tab characters to the beginning of the character", ->
+        buffer.insert([0, 0], '\t')
+        expect(displayBuffer.clipScreenPosition([0, 0], clip: 'backward')).toEqual [0, 0]
+        expect(displayBuffer.clipScreenPosition([0, tabLength-1], clip: 'backward')).toEqual [0, 0]
+        expect(displayBuffer.clipScreenPosition([0, tabLength], clip: 'backward')).toEqual [0, tabLength]
+
+    describe "when clip is 'forward'", ->
       it "clips screen positions in the middle of atomic tab characters to the end of the character", ->
         buffer.insert([0, 0], '\t')
-        expect(displayBuffer.clipScreenPosition([0, 0], skipAtomicTokens: true)).toEqual [0, 0]
-        expect(displayBuffer.clipScreenPosition([0, 1], skipAtomicTokens: true)).toEqual [0, tabLength]
-        expect(displayBuffer.clipScreenPosition([0, tabLength], skipAtomicTokens: true)).toEqual [0, tabLength]
+        expect(displayBuffer.clipScreenPosition([0, 0], clip: 'forward')).toEqual [0, 0]
+        expect(displayBuffer.clipScreenPosition([0, 1], clip: 'forward')).toEqual [0, tabLength]
+        expect(displayBuffer.clipScreenPosition([0, tabLength], clip: 'forward')).toEqual [0, tabLength]
 
   describe "::screenPositionForBufferPosition(bufferPosition, options)", ->
     it "clips the specified buffer position", ->
@@ -719,6 +731,25 @@ describe "DisplayBuffer", ->
       expect(displayBuffer.screenPositionForBufferPosition([100000, 0])).toEqual [12, 2]
       expect(displayBuffer.screenPositionForBufferPosition([100000, 100000])).toEqual [12, 2]
 
+    it "clips to the (left or right) edge of an atomic token without simply rounding up", ->
+      tabLength = 4
+      displayBuffer.setTabLength(tabLength)
+
+      buffer.insert([0, 0], '\t')
+      expect(displayBuffer.screenPositionForBufferPosition([0, 0])).toEqual [0, 0]
+      expect(displayBuffer.screenPositionForBufferPosition([0, 1])).toEqual [0, tabLength]
+
+    it "clips to the edge closest to the given position when it's inside a soft tab", ->
+      tabLength = 4
+      displayBuffer.setTabLength(tabLength)
+
+      buffer.insert([0, 0], '    ')
+      expect(displayBuffer.screenPositionForBufferPosition([0, 0])).toEqual [0, 0]
+      expect(displayBuffer.screenPositionForBufferPosition([0, 1])).toEqual [0, 0]
+      expect(displayBuffer.screenPositionForBufferPosition([0, 2])).toEqual [0, 0]
+      expect(displayBuffer.screenPositionForBufferPosition([0, 3])).toEqual [0, 4]
+      expect(displayBuffer.screenPositionForBufferPosition([0, 4])).toEqual [0, 4]
+
   describe "position translation in the presence of hard tabs", ->
     it "correctly translates positions on either side of a tab", ->
       buffer.setText('\t')

src/cursor.coffee

@@ -305,7 +305,7 @@ class Cursor extends Model
         columnCount-- # subtract 1 for the row move
 
       column = column - columnCount
-      @setScreenPosition({row, column})
+      @setScreenPosition({row, column}, clip: 'backward')
 
   # Public: Moves the cursor right one screen column.
   #
@@ -332,7 +332,7 @@ class Cursor extends Model
         columnsRemainingInLine = rowLength
 
       column = column + columnCount
-      @setScreenPosition({row, column}, skipAtomicTokens: true, wrapBeyondNewlines: true, wrapAtSoftNewlines: true)
+      @setScreenPosition({row, column}, clip: 'forward', wrapBeyondNewlines: true, wrapAtSoftNewlines: true)
 
   # Public: Moves the cursor to the top of the buffer.
   moveToTop: ->

src/marker.coffee

@@ -286,7 +286,6 @@ class Marker
   # * `screenPosition` The new {Point} to use
   # * `properties` (optional) {Object} properties to associate with the marker.
   setHeadScreenPosition: (screenPosition, properties) ->
-    screenPosition = @displayBuffer.clipScreenPosition(screenPosition, properties)
     @setHeadBufferPosition(@displayBuffer.bufferPositionForScreenPosition(screenPosition, properties))
 
   # Extended: Retrieves the buffer position of the marker's tail.
@@ -313,7 +312,6 @@ class Marker
   # * `screenPosition` The new {Point} to use
   # * `properties` (optional) {Object} properties to associate with the marker.
   setTailScreenPosition: (screenPosition, options) ->
-    screenPosition = @displayBuffer.clipScreenPosition(screenPosition, options)
     @setTailBufferPosition(@displayBuffer.bufferPositionForScreenPosition(screenPosition, options))
 
   # Extended: Returns a {Boolean} indicating whether the marker has a tail.

src/selection.coffee

@@ -393,7 +393,7 @@ class Selection extends Model
     if options.select
       @setBufferRange(newBufferRange, reversed: wasReversed)
     else
-      @cursor.setBufferPosition(newBufferRange.end, skipAtomicTokens: true) if wasReversed
+      @cursor.setBufferPosition(newBufferRange.end, clip: 'forward') if wasReversed
 
     if autoIndentFirstLine
       @editor.setIndentationForBufferRow(oldBufferRange.start.row, desiredIndentLevel)

src/tokenized-line.coffee

@@ -41,10 +41,20 @@ class TokenizedLine
   copy: ->
     new TokenizedLine({@tokens, @lineEnding, @ruleStack, @startBufferColumn, @fold})
 
+  # This clips a given screen column to a valid column that's within the line
+  # and not in the middle of any atomic tokens.
+  #
+  # column - A {Number} representing the column to clip
+  # options - A hash with the key clip. Valid values for this key:
+  #           'closest' (default): clip to the closest edge of an atomic token.
+  #           'forward': clip to the forward edge.
+  #           'backward': clip to the backward edge.
+  #
+  # Returns a {Number} representing the clipped column.
   clipScreenColumn: (column, options={}) ->
     return 0 if @tokens.length == 0
 
-    { skipAtomicTokens } = options
+    { clip } = options
     column = Math.min(column, @getMaxScreenColumn())
 
     tokenStartColumn = 0
@@ -55,10 +65,15 @@ class TokenizedLine
     if @isColumnInsideSoftWrapIndentation(tokenStartColumn)
       @softWrapIndentationDelta
     else if token.isAtomic and tokenStartColumn < column
-      if skipAtomicTokens
+      if clip == 'forward'
         tokenStartColumn + token.screenDelta
-      else
+      else if clip == 'backward'
         tokenStartColumn
+      else #'closest'
+        if column > tokenStartColumn + (token.screenDelta / 2)
+          tokenStartColumn + token.screenDelta
+        else
+          tokenStartColumn
     else
       column
 
@@ -67,7 +82,7 @@ class TokenizedLine
     screenColumn = 0
     currentBufferColumn = 0
     for token in @tokens
-      break if currentBufferColumn > bufferColumn
+      break if currentBufferColumn + token.bufferDelta > bufferColumn
       screenColumn += token.screenDelta
       currentBufferColumn += token.bufferDelta
     @clipScreenColumn(screenColumn + (bufferColumn - currentBufferColumn))