Skip to content

Commit

Permalink
Terminology correction. End with respect to spans and ranges should
Browse files Browse the repository at this point in the history
always be exclusive while Last should be inclusive.  The LineRange and
SnapshotLineRange types were incorrectly using End when they meant Last.
Corrected the problem
  • Loading branch information
jaredpar committed Nov 24, 2011
1 parent 9de1724 commit 89bf28b
Show file tree
Hide file tree
Showing 11 changed files with 49 additions and 46 deletions.
3 changes: 3 additions & 0 deletions CodingGuidelines.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@

F#
- Terminology
- Last is inclusive
- End is not inclusive
- Util classes should contain Create methods to create
- Revisit: Util classes should have Get for conversions
- Should be Of for type conversions in util classes
Expand Down
2 changes: 1 addition & 1 deletion VimCore/CommandUtil.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1048,7 +1048,7 @@ type internal CommandUtil

// Extend the range to at least 2 lines if possible
let range =
if range.Count = 1 && range.EndLineNumber = SnapshotUtil.GetLastLineNumber range.Snapshot then
if range.Count = 1 && range.LastLineNumber = SnapshotUtil.GetLastLineNumber range.Snapshot then
// Can't extend
range
elif range.Count = 1 then
Expand Down
6 changes: 3 additions & 3 deletions VimCore/CoreInterfaces.fs
Original file line number Diff line number Diff line change
Expand Up @@ -943,7 +943,7 @@ type CharacterSpan
if lineRange.Count = 1 then
span.Length
else
let diff = span.End.Position - lineRange.EndLine.Start.Position
let diff = span.End.Position - lineRange.LastLine.Start.Position
max 0 diff
CharacterSpan(span.Start, lineRange.Count, lastLineLength)

Expand Down Expand Up @@ -1139,7 +1139,7 @@ and [<RequireQualifiedAccess>] [<StructuralEquality>] [<NoComparison>] VisualSel
// and can be on any column in either
let line =
if isForward then
snapshotLineRange.EndLine
snapshotLineRange.LastLine
else
snapshotLineRange.StartLine

Expand Down Expand Up @@ -1216,7 +1216,7 @@ and [<RequireQualifiedAccess>] [<StructuralEquality>] [<NoComparison>] VisualSel
| VisualSpan.Character span ->
VisualSelection.Character (span, true)
| VisualSpan.Line lineRange ->
let column = SnapshotPointUtil.GetColumn lineRange.EndLine.End
let column = SnapshotPointUtil.GetColumn lineRange.LastLine.End
VisualSelection.Line (lineRange, true, column)
| VisualSpan.Block blockSpan ->
VisualSelection.Block (blockSpan, BlockCaretLocation.BottomRight)
Expand Down
44 changes: 22 additions & 22 deletions VimCore/EditorUtil.fs
Original file line number Diff line number Diff line change
Expand Up @@ -20,23 +20,23 @@ type LineRange
) =

member x.StartLineNumber = _startLine
member x.EndLineNumber = _startLine + (_count - 1)
member x.LastLineNumber = _startLine + (_count - 1)
member x.Count = _count
member x.LineNumbers =
let startLineNumber = x.StartLineNumber
let endLineNumber = x.EndLineNumber
let lastLineNumber = x.LastLineNumber
seq {
for i in startLineNumber .. endLineNumber do yield i
for i in startLineNumber .. lastLineNumber do yield i
}

member x.ContainsLineNumber lineNumber =
lineNumber >= _startLine && lineNumber <= x.EndLineNumber
lineNumber >= _startLine && lineNumber <= x.LastLineNumber

member x.Intersects (lineRange : LineRange) =
x.ContainsLineNumber lineRange.StartLineNumber ||
x.ContainsLineNumber x.EndLineNumber ||
x.EndLineNumber + 1 = lineRange.StartLineNumber ||
x.StartLineNumber = lineRange.EndLineNumber + 1
x.ContainsLineNumber x.LastLineNumber ||
x.LastLineNumber + 1 = lineRange.StartLineNumber ||
x.StartLineNumber = lineRange.LastLineNumber + 1

// Equality Functions
override x.GetHashCode() = _startLine ^^^ _count
Expand All @@ -53,17 +53,17 @@ type LineRange
static member op_Inequality(this,other) =
not (System.Collections.Generic.EqualityComparer<LineRange>.Default.Equals(this,other))

static member CreateFromBounds startLineNumber endLineNumber =
let count = (endLineNumber - startLineNumber) + 1
static member CreateFromBounds startLineNumber lastLineNumber =
let count = (lastLineNumber - startLineNumber) + 1
LineRange(startLineNumber, count)

static member CreateOverarching (leftLineRange : LineRange) (rightLineRange : LineRange) =
let startLineNumber = min leftLineRange.StartLineNumber rightLineRange.StartLineNumber
let endLineNumber = max leftLineRange.EndLineNumber rightLineRange.EndLineNumber
LineRange.CreateFromBounds startLineNumber endLineNumber
let lastLineNumber = max leftLineRange.LastLineNumber rightLineRange.LastLineNumber
LineRange.CreateFromBounds startLineNumber lastLineNumber

// Overrides
override x.ToString() = sprintf "%d - %d" _startLine x.EndLineNumber
override x.ToString() = sprintf "%d - %d" _startLine x.LastLineNumber

/// Represents a range of lines in an ITextSnapshot. Different from a SnapshotSpan
/// because it declaratively supports lines instead of a position range
Expand All @@ -87,19 +87,19 @@ type SnapshotLineRange

/// Number of lines in the SnapshotLineRange
member x.Count = _count
member x.EndLineNumber = _startLine + (_count - 1)
member x.EndLine = _snapshot.GetLineFromLineNumber x.EndLineNumber
member x.End = x.EndLine.End
member x.EndIncludingLineBreak = x.EndLine.EndIncludingLineBreak
member x.LastLineNumber = _startLine + (_count - 1)
member x.LastLine = _snapshot.GetLineFromLineNumber x.LastLineNumber
member x.End = x.LastLine.End
member x.EndIncludingLineBreak = x.LastLine.EndIncludingLineBreak
member x.Extent=
let startLine = x.StartLine
let endLine = x.EndLine
SnapshotSpan(startLine.Start, endLine.End)
let lastLine = x.LastLine
SnapshotSpan(startLine.Start, lastLine.End)
member x.ExtentIncludingLineBreak =
let startLine = x.StartLine
let endLine = x.EndLine
SnapshotSpan(startLine.Start, endLine.EndIncludingLineBreak)
member x.Lines = seq { for i in _startLine .. x.EndLineNumber do yield _snapshot.GetLineFromLineNumber(i) }
let lastLine = x.LastLine
SnapshotSpan(startLine.Start, lastLine.EndIncludingLineBreak)
member x.Lines = seq { for i in _startLine .. x.LastLineNumber do yield _snapshot.GetLineFromLineNumber(i) }
member x.GetText() = x.Extent.GetText()
member x.GetTextIncludingLineBreak() = x.ExtentIncludingLineBreak.GetText()

Expand All @@ -121,7 +121,7 @@ type SnapshotLineRange
not (System.Collections.Generic.EqualityComparer<SnapshotLineRange>.Default.Equals(this,other))

// Overrides
override x.ToString() = sprintf "%d - %d : %s" _startLine x.EndLineNumber (x.Extent.ToString())
override x.ToString() = sprintf "[%d - %d] : %s" _startLine x.LastLineNumber (x.Extent.ToString())

/// Contains operations to help fudge the Editor APIs to be more F# friendly. Does not
/// include any Vim specific logic
Expand Down
4 changes: 2 additions & 2 deletions VimCore/Modes_Command_CommandProcessor.fs
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ type internal CommandProcessor
|> CommandParseUtil.SkipRegister _registerMap

let range = _rangeUtil.RangeOrCurrentLine range
_commandOperations.PutLine reg range.EndLine bang
_commandOperations.PutLine reg range.LastLine bang

/// Process the search pattern command
member x.ProcessSearchPattern path (rest : char list) range _ =
Expand All @@ -402,7 +402,7 @@ type internal CommandProcessor
// The search should begin after the last line in the specified range
let startPoint =
let range = _rangeUtil.RangeOrCurrentLine range
range.EndLine.End
range.LastLine.End

let patternData = { Pattern = pattern; Path = path }
let result = _searchService.FindNextPattern patternData startPoint _vimBufferData.VimTextBuffer.WordNavigator 1
Expand Down
8 changes: 4 additions & 4 deletions VimCore/Modes_Command_RangeUtil.fs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ type internal RangeUtil

member x.ApplyCount count (range:SnapshotLineRange) =
let count = if count <= 1 then 1 else count
SnapshotLineRangeUtil.CreateForLineAndMaxCount range.EndLine count
SnapshotLineRangeUtil.CreateForLineAndMaxCount range.LastLine count

member x.TryApplyCount count range =
match count with
Expand All @@ -73,8 +73,8 @@ type internal RangeUtil
if left.StartLineNumber < right.StartLineNumber then left.StartLine
else right.StartLine
let endLine =
if left.EndLineNumber > right.EndLineNumber then left.EndLine
else right.EndLine
if left.LastLineNumber > right.LastLineNumber then left.LastLine
else right.LastLine
SnapshotLineRangeUtil.CreateForLineRange startLine endLine

member x.ParseNumber (input : char list) =
Expand Down Expand Up @@ -155,7 +155,7 @@ type internal RangeUtil
let number = max 0 (range.StartLineNumber - count)
range.Snapshot.GetLineFromLineNumber(number) |> SnapshotLineRangeUtil.CreateForLine
else
let endLineNumber = max 0 (range.EndLineNumber - count)
let endLineNumber = max 0 (range.LastLineNumber - count)
if endLineNumber = range.StartLineNumber then
SnapshotLineRangeUtil.CreateForLine range.StartLine
elif endLineNumber < range.StartLineNumber then
Expand Down
2 changes: 1 addition & 1 deletion VimCore/Modes_SubstituteConfirm_SubstituteConfirmMode.fs
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ type internal SubstituteConfirmMode
| None -> None
| Some regex ->
let isReplaceAll = Util.IsFlagSet data.Flags SubstituteFlags.ReplaceAll
let data = { Regex=regex; SubstituteText=data.Substitute; CurrentMatch =span; LastLineNumber=range.EndLineNumber; IsReplaceAll=isReplaceAll}
let data = { Regex=regex; SubstituteText=data.Substitute; CurrentMatch = span; LastLineNumber = range.LastLineNumber; IsReplaceAll=isReplaceAll}
Some data

member x.OnLeave () =
Expand Down
6 changes: 3 additions & 3 deletions VimCore/Tagger.fs
Original file line number Diff line number Diff line change
Expand Up @@ -373,10 +373,10 @@ type AsyncTagger<'TData, 'TTag when 'TTag :> ITag>
let getTagsByChunk (lineRange : SnapshotLineRange) =
let snapshot = lineRange.Snapshot
let mutable i = lineRange.StartLineNumber
while i < lineRange.EndLineNumber do
while i < lineRange.LastLineNumber do
cancellationToken.ThrowIfCancellationRequested()

let endLineNumber = min lineRange.EndLineNumber (i + chunkCount)
let endLineNumber = min lineRange.LastLineNumber (i + chunkCount)
let chunkLineRange = SnapshotLineRangeUtil.CreateForLineNumberRange snapshot i endLineNumber
getTags chunkLineRange

Expand All @@ -399,7 +399,7 @@ type AsyncTagger<'TData, 'TTag when 'TTag :> ITag>
getTagsByChunk lineRange

// Now do the lines below
if visibleLineRange.EndLineNumber < lineRange.EndLineNumber then
if visibleLineRange.LastLineNumber < lineRange.LastLineNumber then
let lineRange =
let span = SnapshotSpan(visibleLineRange.EndIncludingLineBreak, lineRange.EndIncludingLineBreak)
SnapshotLineRangeUtil.CreateForSpan span
Expand Down
2 changes: 1 addition & 1 deletion VimCoreTest/RangeUtilTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ private void ParseLineRange(string input, int startLine, int endLine, int contex
Assert.AreEqual(0, ret.AsSucceeded().Item2.Count());
var range = ret.AsSucceeded().Item1;
Assert.AreEqual(startLine, range.StartLineNumber);
Assert.AreEqual(endLine, range.EndLineNumber);
Assert.AreEqual(endLine, range.LastLineNumber);
}

private void ParseSingleLine(string input, int line)
Expand Down
16 changes: 8 additions & 8 deletions VimCoreTest/SnapshotLineRangeUtilTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public void CreateForLine()
Assert.AreEqual("hello", range.Extent.GetText());
Assert.AreEqual(1, range.Count);
Assert.AreEqual(0, range.StartLineNumber);
Assert.AreEqual(0, range.EndLineNumber);
Assert.AreEqual(0, range.LastLineNumber);
}

[Test]
Expand All @@ -36,7 +36,7 @@ public void CreateForLineNumberAndCount1()
Assert.AreEqual("hello", range.Extent.GetText());
Assert.AreEqual(1, range.Count);
Assert.AreEqual(0, range.StartLineNumber);
Assert.AreEqual(0, range.EndLineNumber);
Assert.AreEqual(0, range.LastLineNumber);
}

[Test]
Expand All @@ -47,7 +47,7 @@ public void CreateForLineNumberAndCount2()
Assert.AreEqual("hello" + Environment.NewLine + "world", range.Extent.GetText());
Assert.AreEqual(2, range.Count);
Assert.AreEqual(0, range.StartLineNumber);
Assert.AreEqual(1, range.EndLineNumber);
Assert.AreEqual(1, range.LastLineNumber);
}

[Test]
Expand All @@ -67,7 +67,7 @@ public void CreateForLineNumberRange1()
Assert.AreEqual("hello", range.Extent.GetText());
Assert.AreEqual(1, range.Count);
Assert.AreEqual(0, range.StartLineNumber);
Assert.AreEqual(0, range.EndLineNumber);
Assert.AreEqual(0, range.LastLineNumber);
}

[Test]
Expand All @@ -78,7 +78,7 @@ public void CreateForLineNumberRange2()
Assert.AreEqual("hello" + Environment.NewLine + "world", range.Extent.GetText());
Assert.AreEqual(2, range.Count);
Assert.AreEqual(0, range.StartLineNumber);
Assert.AreEqual(1, range.EndLineNumber);
Assert.AreEqual(1, range.LastLineNumber);
}

[Test]
Expand All @@ -89,7 +89,7 @@ public void CreateForLineAndMaxCount1()
var range = SnapshotLineRangeUtil.CreateForLineAndMaxCount(_buffer.GetLine(0), 1);
Assert.AreEqual(1, range.Count);
Assert.AreEqual(0, range.StartLineNumber);
Assert.AreEqual(0, range.EndLineNumber);
Assert.AreEqual(0, range.LastLineNumber);
}

[Test]
Expand All @@ -100,7 +100,7 @@ public void CreateForLineAndMaxCount2()
var range = SnapshotLineRangeUtil.CreateForLineAndMaxCount(_buffer.GetLine(0), 100);
Assert.AreEqual(2, range.Count);
Assert.AreEqual(0, range.StartLineNumber);
Assert.AreEqual(1, range.EndLineNumber);
Assert.AreEqual(1, range.LastLineNumber);
}

[Test]
Expand All @@ -111,7 +111,7 @@ public void CreateForLineAndMaxCount3()
var range = SnapshotLineRangeUtil.CreateForLineAndMaxCount(_buffer.GetLine(0), 2);
Assert.AreEqual(2, range.Count);
Assert.AreEqual(0, range.StartLineNumber);
Assert.AreEqual(1, range.EndLineNumber);
Assert.AreEqual(1, range.LastLineNumber);
}
}
}
2 changes: 1 addition & 1 deletion VimUnitTestUtils/Mock/MockObjectFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -559,7 +559,7 @@ public static Mock<ITextViewLineCollection> CreateTextViewLineCollection(
}

mock.SetupGet(x => x.FirstVisibleLine).Returns(CreateTextViewLine(range.StartLine, factory).Object);
mock.SetupGet(x => x.LastVisibleLine).Returns(CreateTextViewLine(range.EndLine, factory).Object);
mock.SetupGet(x => x.LastVisibleLine).Returns(CreateTextViewLine(range.LastLine, factory).Object);
mock.SetupGet(x => x.Count).Returns(range.Count);
return mock;
}
Expand Down

0 comments on commit 89bf28b

Please sign in to comment.