Merge pull request VsVim#1530 from jaredpar/fixes-1522

Fixes 1522
sftcrl · Jan 2, 2015 · c8e5103 · c8e5103
2 parents cab3069 95bf3f0
commit c8e5103
Show file tree

Hide file tree

Showing 3 changed files with 82 additions and 16 deletions.
diff --git a/Src/VimCore/CommonOperations.fs b/Src/VimCore/CommonOperations.fs
@@ -540,27  540,29 @@ type internal CommonOperations
         let shouldMaintainCaretColumn = Util.IsFlagSet result.MotionResultFlags MotionResultFlags.MaintainCaretColumn
         match shouldMaintainCaretColumn, result.CaretColumn with
         | true, CaretColumn.InLastLine column ->
 
             // Mappings should occur visually 
             let visualLastLine = x.GetDirectionLastLineInVisualSnapshot result
 
             // First calculate the column in terms of spaces for the maintained caret.
             let caretColumnSpaces = 
                 let motionCaretColumnSpaces = x.GetSpacesToColumn x.CaretLine column
                 match _maintainCaretColumn with
                 | MaintainCaretColumn.None -> motionCaretColumnSpaces
                 | MaintainCaretColumn.Spaces maintainCaretColumnSpaces -> max maintainCaretColumnSpaces motionCaretColumnSpaces
-                | MaintainCaretColumn.EndOfLine -> max 0 (result.DirectionLastLine.Length - 1)
                 | MaintainCaretColumn.EndOfLine -> max 0 (visualLastLine.Length - 1)
 
             // The CaretColumn union is expressed in a position offset not a space offset 
             // which can differ with tabs.  Recalculate as appropriate.  
             let caretColumn = 
-                let lastLine = result.DirectionLastLine
-                let column = x.GetPointForSpaces lastLine caretColumnSpaces |> SnapshotPointUtil.GetColumn
                 let column = x.GetPointForSpaces visualLastLine caretColumnSpaces |> SnapshotPointUtil.GetColumn
                 CaretColumn.InLastLine column
             let result = 
                 { result with DesiredColumn = caretColumn }
 
             // Complete the motion with the updated value then reset the maintain caret.  Need
             // to do the save after the caret move since the move will clear out the saved value
-            x.MoveCaretToMotionResultCore result
             x.MoveCaretToMotionResultCore result 
             _maintainCaretColumn <-
                 if Util.IsFlagSet result.MotionResultFlags MotionResultFlags.EndOfLine then
                     MaintainCaretColumn.EndOfLine
@@ -582,28  584,43 @@ type internal CommonOperations
                     | CaretColumn.ScreenColumn column -> MaintainCaretColumn.Spaces column
                     | _ -> MaintainCaretColumn.None
 
     /// Many operations for moving a motion result need to be calculated in the
     /// visual snapshot.  This method will return the DirectionLastLine value
     /// in that snapshot or the original value if no mapping is possible. 
     member x.GetDirectionLastLineInVisualSnapshot (result : MotionResult) : ITextSnapshotLine =
         let line = result.DirectionLastLine
         let bufferGraph = _textView.BufferGraph
         let visualSnapshot = _textView.TextViewModel.VisualBuffer.CurrentSnapshot
         match BufferGraphUtil.MapSpanUpToSnapshot bufferGraph line.ExtentIncludingLineBreak SpanTrackingMode.EdgeInclusive visualSnapshot with
         | None -> line
         | Some col ->
             if col.Count = 0 then
                 line
             else
                 let span = NormalizedSnapshotSpanCollectionUtil.GetOverarchingSpan col
                 span.Start.GetContainingLine()
 
     /// Move the caret to the position dictated by the given MotionResult value
     ///
     /// Note: This method mixes points from the edit and visual snapshot.  Take care
     /// when changing this function to account for both. 
     member x.MoveCaretToMotionResultCore (result : MotionResult) =
 
         let point = 
 
-            let line = result.DirectionLastLine
             // All issues around caret column position should be calculated on the visual 
             // snapshot 
             let visualLine = x.GetDirectionLastLineInVisualSnapshot result
             if not result.IsForward then
                 match result.MotionKind, result.CaretColumn with
                 | MotionKind.LineWise, CaretColumn.InLastLine column -> 
                     // If we are moving linewise, but to a specific column, use
                     // that column as the target of the motion
-                    SnapshotLineUtil.GetColumnOrEnd column line
                     SnapshotLineUtil.GetColumnOrEnd column visualLine
                 | _, _ -> 
                     result.Span.Start
             else
 
-                // Get the point when moving the caret after the last line in the SnapshotSpan
-                let getAfterLastLine() = 
-                    match SnapshotUtil.TryGetLine x.CurrentSnapshot (line.LineNumber   1) with
-                    | None -> line.End
-                    | Some line -> line.Start
-
                 match result.MotionKind with 
                 | MotionKind.CharacterWiseExclusive ->
                     // Exclusive motions are straight forward.  Move to the end of the SnapshotSpan
@@ -624,13  641,22 @@ type internal CommonOperations
                 | MotionKind.LineWise -> 
                     match result.CaretColumn with
                     | CaretColumn.None -> 
-                        line.End
                         visualLine.End
                     | CaretColumn.InLastLine column ->
-                        SnapshotLineUtil.GetColumnOrEnd column line
                         SnapshotLineUtil.GetColumnOrEnd column visualLine
                     | CaretColumn.ScreenColumn column ->
-                        SnapshotLineUtil.GetColumnOrEnd (SnapshotPointUtil.GetColumn (x.GetPointForSpaces line column)) line
                         SnapshotLineUtil.GetColumnOrEnd (SnapshotPointUtil.GetColumn (x.GetPointForSpaces visualLine column)) visualLine
                     | CaretColumn.AfterLastLine ->
-                        getAfterLastLine()
                         match SnapshotUtil.TryGetLine visualLine.Snapshot (visualLine.LineNumber   1) with
                         | None -> visualLine.End
                         | Some visualLine -> visualLine.Start
 
         // The value 'point' may be in either the visual or edit snapshot at this point.  Map to 
         // ensure it's in the edit.
         let point = 
             match BufferGraphUtil.MapPointDownToSnapshot _textView.BufferGraph point x.CurrentSnapshot PointTrackingMode.Negative PositionAffinity.Predecessor with
             | None -> point
             | Some point -> point
 
         let viewFlags = 
             if result.OperationKind = OperationKind.LineWise && not (Util.IsFlagSet result.MotionResultFlags MotionResultFlags.ExclusiveLineWise) then
@@ -640,6  666,7 @@ type internal CommonOperations
             else
                 // Character wise motions should expand regions
                 ViewFlags.All
 
         x.MoveCaretToPoint point viewFlags
         _editorOperations.ResetSelection()
 

diff --git a/Src/VimCore/EditorUtil.fs b/Src/VimCore/EditorUtil.fs
@@ -1365,6  1365,15 @@ module BufferGraphUtil =
     let MapPointDownToSnapshotStandard (bufferGraph : IBufferGraph) point snapshot =
         MapPointDownToSnapshot bufferGraph point snapshot PointTrackingMode.Negative PositionAffinity.Predecessor
 
     /// Map the SnapshotSpan up to the given ITextSnapshot.  Returns None if the mapping is
     /// not possible
     let MapSpanUpToSnapshot (bufferGraph : IBufferGraph) span trackingMode snapshot =
         try
             bufferGraph.MapUpToSnapshot(span, trackingMode, snapshot) |> Some
         with
             | :? System.ArgumentException-> None
             | :? System.InvalidOperationException -> None
 
     /// Map the SnapshotSpan down to the given ITextSnapshot.  Returns None if the mapping is
     /// not possible
     let MapSpanDownToSnapshot (bufferGraph : IBufferGraph) span trackingMode snapshot =

diff --git a/Test/VimCoreTest/NormalModeIntegrationTest.cs b/Test/VimCoreTest/NormalModeIntegrationTest.cs
@@ -560,6  560,36 @@ public void Issue1511()
                 _vimBuffer.Process('*');
                 Assert.Equal(_textBuffer.GetLine(0).Start, _textView.GetCaretPoint());
             }
 
             /// <summary>
             /// Have to make sure that 'j' correctly maintains caret column when stepping 
             /// over collapsed regions.  
             ///
             /// In general this is straight forward because vim regions include the trailing
             /// new line.  A C# region though does not and we end up with the text of 2 
             /// lines (first and new line of last) in the same visual line.  This makes mapping
             /// considerably more difficult.  
             /// </summary>
             [Fact]
             public void Issue1522()
             {
                 Create("cat", "dog", "bear", "tree");
 
                 // The span should *not* include the line break of the last line.  
                 var span = new SnapshotSpan(
                     _textBuffer.GetLine(1).Start.Add(2),
                     _textBuffer.GetLine(2).End);
 
                 // Collapse the region specified above
                 var adhocOutliner = EditorUtilsFactory.GetOrCreateOutliner(_textBuffer);
                 adhocOutliner.CreateOutliningRegion(span, SpanTrackingMode.EdgeInclusive, "test", "test");
                 OutliningManagerService.GetOutliningManager(_textView).CollapseAll(span, _ => true);
 
                 _vimBuffer.ProcessNotation("j");
                 Assert.Equal(_textBuffer.GetLine(1).Start, _textView.GetCaretPoint());
                 _vimBuffer.ProcessNotation("j");
                 Assert.Equal(_textBuffer.GetLine(3).Start, _textView.GetCaretPoint());
             }
         }
 
         public sealed class MatchingTokenTest : NormalModeIntegrationTest