Merge pull request #1582 from jaredpar/bugs

Bugs

Src/VimCore/EditorUtil.fs

@@ -1496,7 +1496,7 @@ module TextViewUtil =
         | None -> 50
         | Some textViewLines -> textViewLines.Count
 
-    /// Return the overaching SnapshotLineRange for the visible lines in the ITextView
+    /// Return the overarching SnapshotLineRange for the visible lines in the ITextView
     let GetVisibleSnapshotLineRange (textView : ITextView) =
         Extensions.GetVisibleSnapshotLineRange(textView)
 
@@ -1506,6 +1506,27 @@ module TextViewUtil =
         | NullableUtil.HasValue lineRange -> lineRange.Lines
         | NullableUtil.Null -> Seq.empty
 
+    /// Returns the overarching SnapshotLineRange for the visible lines in the ITextView on the
+    /// Visual snapshot.
+    let GetVisibleVisualSnapshotLineRange (textView : ITextView) = 
+        match GetVisibleSnapshotLineRange textView with
+        | NullableUtil.Null -> NullableUtil.CreateNull<SnapshotLineRange>()
+        | NullableUtil.HasValue range ->
+            match BufferGraphUtil.MapSpanUpToSnapshot textView.BufferGraph range.ExtentIncludingLineBreak SpanTrackingMode.EdgeInclusive textView.TextViewModel.VisualBuffer.CurrentSnapshot with
+            | Some collection -> 
+                collection 
+                |> NormalizedSnapshotSpanCollectionUtil.GetOverarchingSpan 
+                |> SnapshotLineRange.CreateForSpan
+                |> NullableUtil.Create
+            | None -> NullableUtil.CreateNull<SnapshotLineRange>()
+
+    /// Returns a sequence of ITextSnapshotLine values representing the visible lines in the buffer
+    /// on the Visual snapshot
+    let GetVisibleVisualSnapshotLines (textView : ITextView) =
+        match GetVisibleVisualSnapshotLineRange textView with
+        | NullableUtil.HasValue lineRange -> lineRange.Lines
+        | NullableUtil.Null -> Seq.empty
+
     /// Ensure the caret is currently on the visible screen
     let EnsureCaretOnScreen textView = 
         let caret = GetCaret textView

Src/VimCore/FSharpUtil.fs

@@ -680,6 +680,9 @@ module internal NullableUtil =
     let Create (x : 'T) =
         System.Nullable<'T>(x)
 
+    let CreateNull<'T when 'T : (new : unit -> 'T) and 'T : struct and 'T :> System.ValueType> () =
+        System.Nullable<'T>()
+
     let ToOption (x : System.Nullable<_>) =
         if x.HasValue then
             Some x.Value

Src/VimCore/MotionUtil.fs

@@ -823,9 +823,9 @@ type internal MotionUtil
     member x.GetSections sectionKind path point = 
         _textObjectUtil.GetSections sectionKind path point
 
-    member x.SpanAndForwardFromLines (line1:ITextSnapshotLine) (line2:ITextSnapshotLine) = 
-        if line1.LineNumber <= line2.LineNumber then SnapshotSpan(line1.Start, line2.End),true
-        else SnapshotSpan(line2.Start, line1.End),false
+    member x.SpanAndForwardFromLines (line1: ITextSnapshotLine) (line2: ITextSnapshotLine) = 
+        if line1.LineNumber <= line2.LineNumber then SnapshotSpan(line1.Start, line2.EndIncludingLineBreak), true
+        else SnapshotSpan(line2.Start, line1.EndIncludingLineBreak), false
 
     /// Apply the 'startofline' option to the given MotionResult.  This function must be 
     /// called with the MotionData mapped back to the edit snapshot
@@ -2211,57 +2211,72 @@ type internal MotionUtil
             let span = SnapshotSpan(startPoint, endPoint)
             MotionResult.Create span isForward MotionKind.CharacterWiseInclusive)
 
-    // TODO: Need to convert this to use the visual snapshot
+    // Line from the top of the visual buffer
     member x.LineFromTopOfVisibleWindow countOpt = 
         _jumpList.Add x.CaretPoint 
 
-        let caretPoint, caretLine = TextViewUtil.GetCaretPointAndLine _textView
-        let lines = TextViewUtil.GetVisibleSnapshotLines _textView |> List.ofSeq
-        let span = 
-            if lines.Length = 0 then
-                caretLine.Extent
+        match TextViewUtil.GetVisibleVisualSnapshotLineRange _textView with 
+        | NullableUtil.Null -> None
+        | NullableUtil.HasValue range -> 
+            if range.Count = 0 then
+                None
             else
-                let count = Util.CountOrDefault countOpt 
-                let count = min count lines.Length
-                let startLine = lines.Head
-                SnapshotPointUtil.GetLineRangeSpan startLine.Start count
-        let isForward = caretPoint.Position <= span.End.Position
-        MotionResult.Create span isForward MotionKind.LineWise
-        |> x.ApplyStartOfLineOption
-
-    // TODO: Need to convert this to use the visual snapshot
+                let count = (Util.CountOrDefault countOpt) - 1
+                let count = min count range.Count
+                let visualLine = SnapshotUtil.GetLine range.Snapshot (count + range.StartLineNumber)
+                match BufferGraphUtil.MapPointDownToSnapshotStandard _textView.BufferGraph visualLine.Start x.CurrentSnapshot with
+                | None -> None
+                | Some point -> 
+                    let line = SnapshotPointUtil.GetContainingLine point
+                    let span, isForward = x.SpanAndForwardFromLines x.CaretLine line
+                    MotionResult.Create span isForward MotionKind.LineWise
+                    |> x.ApplyStartOfLineOption
+                    |> Some
+
+    // Line from the top of the visual buffer
     member x.LineFromBottomOfVisibleWindow countOpt =
         _jumpList.Add x.CaretPoint 
 
-        let caretPoint,caretLine = TextViewUtil.GetCaretPointAndLine _textView
-        let lines = TextViewUtil.GetVisibleSnapshotLines _textView |> List.ofSeq
-        let span,isForward = 
-            if lines.Length = 0 then caretLine.Extent,true
+        match TextViewUtil.GetVisibleVisualSnapshotLineRange _textView with
+        | NullableUtil.Null -> None
+        | NullableUtil.HasValue range -> 
+            if range.Count = 0 then 
+                None
             else
-                let endLine = 
-                    match countOpt with 
-                    | None -> List.nth lines (lines.Length-1)
-                    | Some(count) ->    
-                        let count = lines.Length - count
-                        List.nth lines count
-                x.SpanAndForwardFromLines caretLine endLine
-        MotionResult.Create span isForward MotionKind.LineWise
-        |> x.ApplyStartOfLineOption
-
-    // TODO: Need to convert this to use the visual snapshot
+                let count = (Util.CountOrDefault countOpt) - 1
+                let count = min count (range.Count - 1)
+                let number = range.LastLineNumber - count
+                let visualLine = SnapshotUtil.GetLine range.Snapshot number
+                match BufferGraphUtil.MapPointDownToSnapshotStandard _textView.BufferGraph visualLine.Start x.CurrentSnapshot with
+                | None -> None
+                | Some point -> 
+                    let line = SnapshotPointUtil.GetContainingLine point
+                    let span, isForward = x.SpanAndForwardFromLines x.CaretLine line
+                    MotionResult.Create span isForward MotionKind.LineWise
+                    |> x.ApplyStartOfLineOption
+                    |> Some
+
+    /// Motion to put the caret in the middle of the visible window.  
     member x.LineInMiddleOfVisibleWindow () =
         _jumpList.Add x.CaretPoint 
 
-        let caretLine = TextViewUtil.GetCaretLine _textView
-        let lines = TextViewUtil.GetVisibleSnapshotLines _textView |> List.ofSeq
-        let middleLine =
-            if lines.Length = 0 then caretLine
-            else 
-                let index = lines.Length / 2
-                List.nth lines index
-        let span, isForward = x.SpanAndForwardFromLines caretLine middleLine
-        MotionResult.Create span isForward MotionKind.LineWise
-        |> x.ApplyStartOfLineOption
+        match TextViewUtil.GetVisibleVisualSnapshotLineRange _textView with
+        | NullableUtil.Null -> None
+        | NullableUtil.HasValue range -> 
+            if range.Count = 0 then
+                None
+            else
+                let number = (range.Count / 2) + range.StartLineNumber
+                let middleVisualLine = SnapshotUtil.GetLine range.Snapshot number
+                let middleLine = 
+                    match BufferGraphUtil.MapPointDownToSnapshotStandard _textView.BufferGraph middleVisualLine.Start x.CurrentSnapshot with
+                    | None -> x.CaretLine
+                    | Some point -> SnapshotPointUtil.GetContainingLine point
+
+                let span, isForward = x.SpanAndForwardFromLines x.CaretLine middleLine
+                MotionResult.Create span isForward MotionKind.LineWise
+                |> x.ApplyStartOfLineOption
+                |> Some
 
     /// Implements the core portion of section backward motions
     member x.SectionBackwardCore sectionKind count = 
@@ -2635,9 +2650,9 @@ type internal MotionUtil
             | Motion.LastSearch isReverse -> x.LastSearch isReverse motionArgument.Count
             | Motion.LineDown -> x.LineDown motionArgument.Count
             | Motion.LineDownToFirstNonBlank -> x.LineDownToFirstNonBlank motionArgument.Count |> Some
-            | Motion.LineFromBottomOfVisibleWindow -> x.LineFromBottomOfVisibleWindow motionArgument.RawCount |> Some
-            | Motion.LineFromTopOfVisibleWindow -> x.LineFromTopOfVisibleWindow motionArgument.RawCount |> Some
-            | Motion.LineInMiddleOfVisibleWindow -> x.LineInMiddleOfVisibleWindow() |> Some
+            | Motion.LineFromBottomOfVisibleWindow -> x.LineFromBottomOfVisibleWindow motionArgument.RawCount 
+            | Motion.LineFromTopOfVisibleWindow -> x.LineFromTopOfVisibleWindow motionArgument.RawCount 
+            | Motion.LineInMiddleOfVisibleWindow -> x.LineInMiddleOfVisibleWindow() 
             | Motion.LineOrFirstToFirstNonBlank -> x.LineOrFirstToFirstNonBlank motionArgument.RawCount |> Some
             | Motion.LineOrLastToFirstNonBlank -> x.LineOrLastToFirstNonBlank motionArgument.RawCount |> Some
             | Motion.LineUp -> x.LineUp motionArgument.Count

Src/VimWpf/Implementation/Misc/ClipboardDevice.cs

@@ -1,5 +1,6 @@
 using System;
 using System.ComponentModel.Composition;
+using System.Threading;
 using System.Windows;
 
 namespace Vim.UI.Wpf.Implementation.Misc
@@ -24,40 +25,72 @@ internal ClipboardDevice(IVimProtectedOperations protectedOperations)
 
         private string GetText()
         {
-            try
-            {
-                var text = _useTextMethods
-                    ? Clipboard.GetText()
-                    : (string)Clipboard.GetData(DataFormats.UnicodeText);
+            string text = null;
+            Action action = () =>
+                {
+                    text = _useTextMethods
+                        ? Clipboard.GetText()
+                        : (string)Clipboard.GetData(DataFormats.UnicodeText);
+                };
 
-                return text ?? string.Empty;
-            }
-            catch (Exception ex)
+            if (!TryAccessClipboard(action))
             {
-                _protectedOperations.Report(ex);
-                _useTextMethods = false;
-                return string.Empty;
+                text = string.Empty;
             }
+
+            return text;
         }
 
         private void SetText(string text)
         {
-            try
+            Action action = () =>
+                {
+                    if (_useTextMethods)
+                    {
+                        Clipboard.SetText(text);
+                    }
+                    else
+                    {
+                        Clipboard.SetDataObject(text);
+                    }
+                };
+
+            TryAccessClipboard(action);
+        }
+
+        /// <summary>
+        /// The clipboard is a shared resource across all applications.  It can only be used 
+        /// by one application at a time and has no mechanism for synchronizing access.  
+        ///
+        /// Use of the clipboard should be short lived though so most applications guard 
+        /// against races by simply retrying the access a number of times.  This is how 
+        /// WinForms handles the race.  WPF does not do this hence we have to implement it 
+        /// manually here. 
+        /// </summary>
+        private bool TryAccessClipboard(Action action, int retryCount = 5, int pauseMilliseconds = 100)
+        {
+            var i = retryCount;
+            do
             {
-                if (_useTextMethods)
+                try
                 {
-                    Clipboard.SetText(text);
+                    action();
+                    return true;
                 }
-                else
+                catch (Exception ex)
                 {
-                    Clipboard.SetDataObject(text);
+                    i--;
+                    if (i == 0)
+                    {
+                        _protectedOperations.Report(ex);
+                        _useTextMethods = true;
+                        return false;
+                    }
                 }
-            }
-            catch (Exception ex)
-            {
-                _protectedOperations.Report(ex);
-                _useTextMethods = false;
-            }
+
+                Thread.Sleep(TimeSpan.FromMilliseconds(pauseMilliseconds));
+                i--;
+            } while (true);
         }
 
         #region IClipboardDevice

Test/VimCoreTest/CommandUtilTest.cs

@@ -910,7 +910,7 @@ public sealed class ScrollPagesTest : CommandUtilTest
             public ScrollPagesTest()
             {
                 Create("a", "b", "c", "d", "e");
-                _textView.SetVisibleLineCount(count: 1);
+                _textView.SetTextViewLineCount(count: 2);
             }
 
             [Fact]

Test/VimCoreTest/Extensions.cs

@@ -1099,15 +1099,90 @@ public static int GetLastVisibleLineNumber(this ITextView textView)
         #region IWpfTextView
 
         /// <summary>
-        /// Make only a single line visible in the IWpfTextView.  This is really useful when testing
-        /// actions like scrolling
+        /// Set the number of visible lines to the specified count.  This will control the 
+        /// range between <see cref="ITextViewLineCollection.FirstVisibleLine"/> and 
+        /// <see cref="ITextViewLineCollection.LastVisibleLine"/>.  The <see cref="ITextViewLineCollection.Count"/>
+        /// value can potentially be greater than the specified <param name="count"/>.
         /// </summary>
         public static void SetVisibleLineCount(this IWpfTextView wpfTextView, int count)
         {
             var oldSize = wpfTextView.VisualElement.RenderSize;
             var height = wpfTextView.TextViewLines.FirstVisibleLine.Height * (double)count;
-            var size = new Size(oldSize.Width, height);
-            wpfTextView.VisualElement.RenderSize = size;
+
+            do
+            {
+                var size = new Size(oldSize.Width, height);
+                wpfTextView.VisualElement.RenderSize = size;
+                ForceLayout(wpfTextView);
+
+                var startLine = wpfTextView.TextViewLines.FirstVisibleLine.Start.GetContainingLine();
+                var lastLine = wpfTextView.TextViewLines.LastVisibleLine.Start.GetContainingLine();
+                var visibleCount = (lastLine.LineNumber - startLine.LineNumber) + 1;
+
+                if (visibleCount == count)
+                {
+                    break;
+                }
+                else if (visibleCount < count)
+                {
+                    height += 5;
+                }
+                else
+                {
+                    height -= 5;
+                }
+            }
+            while (true);
+        }
+
+        /// <summary>
+        /// Set the <see cref="ITextViewLineCollection.Count"/> value.
+        /// </summary>
+        public static void SetTextViewLineCount(this IWpfTextView wpfTextView, int count)
+        {
+            var oldSize = wpfTextView.VisualElement.RenderSize;
+            var height = wpfTextView.TextViewLines.FirstVisibleLine.Height * (double)count;
+
+            do
+            {
+                var size = new Size(oldSize.Width, height);
+                wpfTextView.VisualElement.RenderSize = size;
+                ForceLayout(wpfTextView);
+
+                var visibleCount = wpfTextView.TextViewLines.Count;
+                if (visibleCount == count)
+                {
+                    break;
+                }
+                else if (visibleCount < count)
+                {
+                    height += 5;
+                }
+                else
+                {
+                    height -= 5;
+                }
+            }
+            while (true);
+        }
+
+        /// <summary>
+        /// Make only the specified line range visible.
+        /// </summary>
+        public static void SetVisibleLineRange(this IWpfTextView wpfTextView, int start, int length)
+        {
+            var startLine = wpfTextView.TextSnapshot.GetLineFromLineNumber(start);
+            wpfTextView.DisplayTextLineContainingBufferPosition(startLine.Start, 0, ViewRelativePosition.Top);
+            SetVisibleLineCount(wpfTextView, length);
+        }
+
+        public static void ForceLayout(this IWpfTextView wpfTextView)
+        {
+            var method = wpfTextView
+                .GetType()
+                .GetMethods(BindingFlags.NonPublic | BindingFlags.Instance)
+                .Single(x => x.Name == "PerformLayout" && x.GetParameters().Length == 2);
+            method.Invoke(wpfTextView, new[] { wpfTextView.TextSnapshot, wpfTextView.VisualSnapshot });
         }
 
         #endregion