Merge pull request #783 from jacekkopecky/check-motion-inclusiveness

Inclusiveness fixes

Author: Max Brunsfeld

lib/motions/find-motion.coffee

@@ -3,6 +3,8 @@
 {Point, Range} = require 'atom'
 
 class Find extends MotionWithInput
+  operatesInclusively: true
+
   constructor: (@editor, @vimState, opts={}) ->
     super(@editor, @vimState)
     @offset = 0

lib/motions/general-motions.coffee

@@ -11,7 +11,7 @@ class MotionError
     @name = 'Motion Error'
 
 class Motion
-  operatesInclusively: true
+  operatesInclusively: false
   operatesLinewise: false
 
   constructor: (@editor, @vimState) ->
@@ -20,7 +20,9 @@ class Motion
     value = for selection in @editor.getSelections()
       if @isLinewise()
         @moveSelectionLinewise(selection, count, options)
-      else if @isInclusive()
+      else if @vimState.mode is 'visual'
+        @moveSelectionVisual(selection, count, options)
+      else if @operatesInclusively
         @moveSelectionInclusively(selection, count, options)
       else
         @moveSelection(selection, count, options)
@@ -61,17 +63,35 @@ class Motion
       selection.setBufferRange([[newStartRow, 0], [newEndRow + 1, 0]])
 
   moveSelectionInclusively: (selection, count, options) ->
+    return @moveSelectionVisual(selection, count, options) unless selection.isEmpty()
+
+    selection.modifySelection =>
+      @moveCursor(selection.cursor, count, options)
+      return if selection.isEmpty()
+
+      if selection.isReversed()
+        # for backward motion, add the original starting character of the motion
+        {start, end} = selection.getBufferRange()
+        selection.setBufferRange([start, [end.row, end.column + 1]])
+      else
+        # for forward motion, add the ending character of the motion
+        selection.cursor.moveRight()
+
+  moveSelectionVisual: (selection, count, options) ->
     selection.modifySelection =>
       range = selection.getBufferRange()
       [oldStart, oldEnd] = [range.start, range.end]
 
+      # in visual mode, atom cursor is after the last selected character,
+      # so here put cursor in the expected place for the following motion
       wasEmpty = selection.isEmpty()
       wasReversed = selection.isReversed()
       unless wasEmpty or wasReversed
         selection.cursor.moveLeft()
 
       @moveCursor(selection.cursor, count, options)
 
+      # put cursor back after the last character so it is also selected
       isEmpty = selection.isEmpty()
       isReversed = selection.isReversed()
       unless isEmpty or isReversed
@@ -80,10 +100,15 @@ class Motion
       range = selection.getBufferRange()
       [newStart, newEnd] = [range.start, range.end]
 
+      # if we reversed or emptied a normal selection
+      # we need to select again the last character deselected above the motion
       if (isReversed or isEmpty) and not (wasReversed or wasEmpty)
         selection.setBufferRange([newStart, [newEnd.row, oldStart.column + 1]])
+
+      # if we re-reversed a reversed non-empty selection,
+      # we need to keep the last character of the old selection selected
       if wasReversed and not wasEmpty and not isReversed
-        selection.setBufferRange([[newStart.row, oldEnd.column - 1], newEnd])
+        selection.setBufferRange([[oldEnd.row, oldEnd.column - 1], newEnd])
 
       # keep a single-character selection non-reversed
       range = selection.getBufferRange()
@@ -104,9 +129,6 @@ class Motion
     else
       @operatesLinewise
 
-  isInclusive: ->
-    @vimState.mode is 'visual' or @operatesInclusively
-
 class CurrentSelection extends Motion
   constructor: (@editor, @vimState) ->
     super(@editor, @vimState)
@@ -159,15 +181,11 @@ class MotionWithInput extends Motion
     @complete = true
 
 class MoveLeft extends Motion
-  operatesInclusively: false
-
   moveCursor: (cursor, count=1) ->
     _.times count, ->
       cursor.moveLeft() if not cursor.isAtBeginningOfLine() or settings.wrapLeftRightMotion()
 
 class MoveRight extends Motion
-  operatesInclusively: false
-
   moveCursor: (cursor, count=1) ->
     _.times count, =>
       wrapToNextLine = settings.wrapLeftRightMotion()
@@ -196,15 +214,11 @@ class MoveDown extends Motion
         cursor.moveDown()
 
 class MoveToPreviousWord extends Motion
-  operatesInclusively: false
-
   moveCursor: (cursor, count=1) ->
     _.times count, ->
       cursor.moveToBeginningOfWord()
 
 class MoveToPreviousWholeWord extends Motion
-  operatesInclusively: false
-
   moveCursor: (cursor, count=1) ->
     _.times count, =>
       cursor.moveToBeginningOfWord()
@@ -221,7 +235,6 @@ class MoveToPreviousWholeWord extends Motion
 
 class MoveToNextWord extends Motion
   wordRegex: null
-  operatesInclusively: false
 
   moveCursor: (cursor, count=1, options) ->
     _.times count, =>
@@ -252,6 +265,7 @@ class MoveToNextWholeWord extends MoveToNextWord
   wordRegex: WholeWordOrEmptyLineRegex
 
 class MoveToEndOfWord extends Motion
+  operatesInclusively: true
   wordRegex: null
 
   moveCursor: (cursor, count=1) ->
@@ -276,8 +290,6 @@ class MoveToEndOfWholeWord extends MoveToEndOfWord
   wordRegex: WholeWordRegex
 
 class MoveToNextParagraph extends Motion
-  operatesInclusively: false
-
   moveCursor: (cursor, count=1) ->
     _.times count, ->
       cursor.moveToBeginningOfNextParagraph()
@@ -300,8 +312,6 @@ class MoveToAbsoluteLine extends MoveToLine
     cursor.moveToEndOfLine() if cursor.getBufferColumn() is 0
 
 class MoveToRelativeLine extends MoveToLine
-  operatesLinewise: true
-
   moveCursor: (cursor, count=1) ->
     {row, column} = cursor.getBufferPosition()
     cursor.setBufferPosition([row + (count - 1), 0])
@@ -316,23 +326,18 @@ class MoveToScreenLine extends MoveToLine
     cursor.setScreenPosition([@getDestinationRow(count), 0])
 
 class MoveToBeginningOfLine extends Motion
-  operatesInclusively: false
-
   moveCursor: (cursor, count=1) ->
     _.times count, ->
       cursor.moveToBeginningOfLine()
 
 class MoveToFirstCharacterOfLine extends Motion
-  operatesInclusively: false
-
   moveCursor: (cursor, count=1) ->
     _.times count, ->
       cursor.moveToBeginningOfLine()
       cursor.moveToFirstCharacterOfLine()
 
 class MoveToFirstCharacterOfLineAndDown extends Motion
   operatesLinewise: true
-  operatesInclusively: true
 
   moveCursor: (cursor, count=0) ->
     _.times count-1, ->
@@ -341,8 +346,6 @@ class MoveToFirstCharacterOfLineAndDown extends Motion
     cursor.moveToFirstCharacterOfLine()
 
 class MoveToLastCharacterOfLine extends Motion
-  operatesInclusively: false
-
   moveCursor: (cursor, count=1) ->
     _.times count, ->
       cursor.moveToEndOfLine()
@@ -369,7 +372,6 @@ class MoveToLastNonblankCharacterOfLineAndDown extends Motion
 
 class MoveToFirstCharacterOfLineUp extends Motion
   operatesLinewise: true
-  operatesInclusively: true
 
   moveCursor: (cursor, count=1) ->
     _.times count, ->

lib/motions/move-to-mark-motion.coffee

@@ -4,8 +4,6 @@
 
 module.exports =
 class MoveToMark extends MotionWithInput
-  operatesInclusively: false
-
   constructor: (@editor, @vimState, @linewise=true) ->
     super(@editor, @vimState)
     @operatesLinewise = @linewise

lib/motions/search-motion.coffee

@@ -6,8 +6,6 @@ SearchViewModel = require '../view-models/search-view-model'
 settings = require '../settings'
 
 class SearchBase extends MotionWithInput
-  operatesInclusively: false
-
   constructor: (@editor, @vimState, options = {}) ->
     super(@editor, @vimState)
     @reverse = @initiallyReversed = false

spec/operators-spec.coffee

@@ -737,8 +737,7 @@ describe "Operators", ->
           keydown('c')
           keydown('%')
           editor.insertText('x')
-          # this differs from VIM, which deletes the character originally under cursor
-          expect(editor.getText()).toBe("12345x67)8\nabcx)e\nAx)BCDE")
+          expect(editor.getText()).toBe("12345x7)8\nabcxe\nAxBCDE")
           expect(vimState.mode).toBe "insert"
 
       describe "after or without brackets", ->
@@ -774,15 +773,13 @@ describe "Operators", ->
         it "repeats correctly inside brackets", ->
           editor.setCursorScreenPosition([1, 4])
           keydown('.')
-          # this differs from VIM, which deletes the character originally under cursor
-          expect(editor.getText()).toBe("1x8\nabcxd)e\nA()BCDE")
+          expect(editor.getText()).toBe("1x8\nabcx)e\nA()BCDE")
           expect(vimState.mode).toBe "normal"
 
         it "repeats correctly on the closing bracket", ->
           editor.setCursorScreenPosition([1, 5])
           keydown('.')
-          # this differs from VIM, which deletes the character originally under cursor
-          expect(editor.getText()).toBe("1x8\nabcx)e\nA()BCDE")
+          expect(editor.getText()).toBe("1x8\nabcxe\nA()BCDE")
           expect(vimState.mode).toBe "normal"
 
         it "does nothing when repeated after a bracket", ->