From 52ac6b81ea65fdc8bb4d6f3b2e5b6fbcc889e686 Mon Sep 17 00:00:00 2001 From: Jacek Kopecky Date: Sun, 19 Jul 2015 16:55:01 +0100 Subject: [PATCH 1/5] remove redundancies --- lib/motions/general-motions.coffee | 6 ------ 1 file changed, 6 deletions(-) diff --git a/lib/motions/general-motions.coffee b/lib/motions/general-motions.coffee index d5b2f9a7..d33fda0d 100644 --- a/lib/motions/general-motions.coffee +++ b/lib/motions/general-motions.coffee @@ -300,8 +300,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]) @@ -332,7 +330,6 @@ class MoveToFirstCharacterOfLine extends Motion class MoveToFirstCharacterOfLineAndDown extends Motion operatesLinewise: true - operatesInclusively: true moveCursor: (cursor, count=0) -> _.times count-1, -> @@ -349,8 +346,6 @@ class MoveToLastCharacterOfLine extends Motion cursor.goalColumn = Infinity class MoveToLastNonblankCharacterOfLineAndDown extends Motion - operatesInclusively: true - # moves cursor to the last non-whitespace character on the line # similar to skipLeadingWhitespace() in atom's cursor.coffee skipTrailingWhitespace: (cursor) -> @@ -369,7 +364,6 @@ class MoveToLastNonblankCharacterOfLineAndDown extends Motion class MoveToFirstCharacterOfLineUp extends Motion operatesLinewise: true - operatesInclusively: true moveCursor: (cursor, count=1) -> _.times count, -> From 8a23ac7506d1ac0f243048fe5c1a8100c2027eff Mon Sep 17 00:00:00 2001 From: Jacek Kopecky Date: Sun, 19 Jul 2015 16:55:49 +0100 Subject: [PATCH 2/5] fix MoveToPreviousParagraph inclusivity --- lib/motions/general-motions.coffee | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/motions/general-motions.coffee b/lib/motions/general-motions.coffee index d33fda0d..107cbe1c 100644 --- a/lib/motions/general-motions.coffee +++ b/lib/motions/general-motions.coffee @@ -283,6 +283,8 @@ class MoveToNextParagraph extends Motion cursor.moveToBeginningOfNextParagraph() class MoveToPreviousParagraph extends Motion + operatesInclusively: false + moveCursor: (cursor, count=1) -> _.times count, -> cursor.moveToBeginningOfPreviousParagraph() From 656095118fb5997aa8ad2fe0829795dac481eab1 Mon Sep 17 00:00:00 2001 From: Jacek Kopecky Date: Sun, 19 Jul 2015 17:28:14 +0100 Subject: [PATCH 3/5] flip default for operatesInclusively MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is better because inclusive operations are the special case. CurrentSelection doesn’t get operatesInclusively because it redefines select() anyway. --- lib/motions/find-motion.coffee | 2 ++ lib/motions/general-motions.coffee | 24 ++++-------------------- lib/motions/move-to-mark-motion.coffee | 2 -- lib/motions/search-motion.coffee | 2 -- 4 files changed, 6 insertions(+), 24 deletions(-) diff --git a/lib/motions/find-motion.coffee b/lib/motions/find-motion.coffee index 555ca805..2a577a8b 100644 --- a/lib/motions/find-motion.coffee +++ b/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 diff --git a/lib/motions/general-motions.coffee b/lib/motions/general-motions.coffee index 107cbe1c..467ea962 100644 --- a/lib/motions/general-motions.coffee +++ b/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) -> @@ -159,15 +159,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 +192,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 +213,6 @@ class MoveToPreviousWholeWord extends Motion class MoveToNextWord extends Motion wordRegex: null - operatesInclusively: false moveCursor: (cursor, count=1, options) -> _.times count, => @@ -252,6 +243,7 @@ class MoveToNextWholeWord extends MoveToNextWord wordRegex: WholeWordOrEmptyLineRegex class MoveToEndOfWord extends Motion + operatesInclusively: true wordRegex: null moveCursor: (cursor, count=1) -> @@ -276,15 +268,11 @@ class MoveToEndOfWholeWord extends MoveToEndOfWord wordRegex: WholeWordRegex class MoveToNextParagraph extends Motion - operatesInclusively: false - moveCursor: (cursor, count=1) -> _.times count, -> cursor.moveToBeginningOfNextParagraph() class MoveToPreviousParagraph extends Motion - operatesInclusively: false - moveCursor: (cursor, count=1) -> _.times count, -> cursor.moveToBeginningOfPreviousParagraph() @@ -316,15 +304,11 @@ 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() @@ -340,14 +324,14 @@ class MoveToFirstCharacterOfLineAndDown extends Motion cursor.moveToFirstCharacterOfLine() class MoveToLastCharacterOfLine extends Motion - operatesInclusively: false - moveCursor: (cursor, count=1) -> _.times count, -> cursor.moveToEndOfLine() cursor.goalColumn = Infinity class MoveToLastNonblankCharacterOfLineAndDown extends Motion + operatesInclusively: true + # moves cursor to the last non-whitespace character on the line # similar to skipLeadingWhitespace() in atom's cursor.coffee skipTrailingWhitespace: (cursor) -> diff --git a/lib/motions/move-to-mark-motion.coffee b/lib/motions/move-to-mark-motion.coffee index 0187620a..3603e1ed 100644 --- a/lib/motions/move-to-mark-motion.coffee +++ b/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 diff --git a/lib/motions/search-motion.coffee b/lib/motions/search-motion.coffee index 9f492e4d..fa65e195 100644 --- a/lib/motions/search-motion.coffee +++ b/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 From 6e1e6de1d94a9b38125f9d2b64e7b7a6aa9d8f2e Mon Sep 17 00:00:00 2001 From: Jacek Kopecky Date: Sun, 19 Jul 2015 19:41:48 +0100 Subject: [PATCH 4/5] test inclusive behaviour of % Conflicts: spec/operators-spec.coffee --- spec/operators-spec.coffee | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/spec/operators-spec.coffee b/spec/operators-spec.coffee index 6305486b..5a460569 100644 --- a/spec/operators-spec.coffee +++ b/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", -> From 2698424da13067c73c78900263d319dfc3392cc8 Mon Sep 17 00:00:00 2001 From: Jacek Kopecky Date: Sun, 19 Jul 2015 19:43:05 +0100 Subject: [PATCH 5/5] distinguish between visual and inclusive selection and add comments, all for code clarity --- lib/motions/general-motions.coffee | 32 +++++++++++++++++++++++++----- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/lib/motions/general-motions.coffee b/lib/motions/general-motions.coffee index 467ea962..dd12dca0 100644 --- a/lib/motions/general-motions.coffee +++ b/lib/motions/general-motions.coffee @@ -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,10 +63,27 @@ 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 @@ -72,6 +91,7 @@ class Motion @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)