Better choice of callapse on exit.

The question here is where to callapse the selection to, anchor or
focus?

When exiting visual mode, mimic vim.  When trasitioning between visual
and caret modes, do what's right to keep the selection in the same
place.

This also adds some related tests.

2 files changed, 29 insertions, 14 deletions

diff --git a/content_scripts/mode_visual.coffee b/content_scripts/mode_visual.coffee
index cbdf6ae2..011d6775 100644
--- a/content_scripts/mode_visual.coffee
+++ b/content_scripts/mode_visual.coffee
@@ -197,7 +197,15 @@ class VisualMode extends KeyHandlerMode
     "P": -> chrome.runtime.sendMessage handler: "openUrlInNewTab", url: @yank()
     "v": -> new VisualMode
     "V": -> new VisualLineMode
-    "c": -> @movement.collapseSelectionToAnchor(); new CaretMode
+    "c": ->
+      # If we're already in caret mode, or if the selection looks the same as it would in caret mode, then
+      # callapse to anchor (so that the caret-mode selection will seem unchanged).  Otherwise, we're in visual
+      # mode and the user has moved the focus, so collapse to that.
+      if @name == "caret" or @selection.toString().length <= 1
+        @movement.collapseSelectionToAnchor()
+      else
+        @movement.collapseSelectionToFocus()
+      new CaretMode
     "o": -> @movement.reverseSelection()
 
   constructor: (options = {}) ->
@@ -228,7 +236,11 @@ class VisualMode extends KeyHandlerMode
       commandHandler: @commandHandler.bind this
 
     @onExit (event = null) =>
-      @movement.collapseSelectionToAnchor()
+      # This mimics vim: when leaving visual mode via Escape, collapse to focus, otherwise collapse to anchor.
+      if event?.type == "keydown" and KeyboardUtils.isEscape(event) and @name != "caret"
+        @movement.collapseSelectionToFocus()
+      else
+        @movement.collapseSelectionToAnchor()
       # Don't leave the user in insert mode just because they happen to have selected an input.
       if document.activeElement and DomUtils.isEditable document.activeElement
         document.activeElement.blur() unless event?.type == "click"
diff --git a/tests/dom_tests/dom_tests.coffee b/tests/dom_tests/dom_tests.coffee
index 517dce99..2311b768 100644
--- a/tests/dom_tests/dom_tests.coffee
+++ b/tests/dom_tests/dom_tests.coffee
@@ -744,12 +744,6 @@ context "Caret mode",
       By thy long grey beard and glittering eye,
       Now wherefore stopp'st thou me?
     </pre></p>
-    <p><pre>
-      The Bridegroom's doors are opened wide,
-      And I am next of kin;
-      The guests are met, the feast is set:
-      May'st hear the merry din.
-    </pre></p>
     """
     initializeModeState()
     @initialVisualMode = new VisualMode
@@ -793,6 +787,21 @@ context "Caret mode",
     sendKeyboardEvent "k"
     assert.equal "I", getSelection()
 
+  should "re-use an existing selection", ->
+    assert.equal "I", getSelection()
+    sendKeyboardEvents "ww"
+    assert.equal "a", getSelection()
+    sendKeyboardEvent "escape"
+    new VisualMode
+    assert.equal "a", getSelection()
+
+  should "not move the selection on caret/visual mode toggle", ->
+    sendKeyboardEvents "ww"
+    assert.equal "a", getSelection()
+    for key in "vcvcvc".split()
+      sendKeyboardEvent key
+      assert.equal "a", getSelection()
+
 context "Visual mode",
   setup ->
     document.getElementById("test-div").innerHTML = """
@@ -802,12 +811,6 @@ context "Visual mode",
       By thy long grey beard and glittering eye,
       Now wherefore stopp'st thou me?
     </pre></p>
-    <p><pre>
-      The Bridegroom's doors are opened wide,
-      And I am next of kin;
-      The guests are met, the feast is set:
-      May'st hear the merry din.
-    </pre></p>
     """
     initializeModeState()
     @initialVisualMode = new VisualMode