Skip to content

Commit

Permalink
fix bug of incorrect positions returned when retreating back over new…
Browse files Browse the repository at this point in the history
…lines
  • Loading branch information
a-h committed Apr 18, 2021
1 parent bbb8604 commit 8bf8fd3
Show file tree
Hide file tree
Showing 4 changed files with 161 additions and 45 deletions.
30 changes: 21 additions & 9 deletions input/position.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,24 @@ import "fmt"

// Position represents the character position within a text file.
type Position struct {
Line int
Col int
Index int64
Line int
Col int
// The lengths of each line we've seen.
lineLengths map[int]int
lineLengths map[int]int
carriageReturns map[int64]struct{}
lineFeeds map[int64]struct{}
}

// NewPosition creates a Position to represent the character position within a text file.
func NewPosition(line int, col int) Position {
return Position{
Line: line,
Col: col,
lineLengths: make(map[int]int),
Index: int64(-1),
Line: line,
Col: col,
lineLengths: make(map[int]int),
carriageReturns: make(map[int64]struct{}),
lineFeeds: make(map[int64]struct{}),
}
}

Expand All @@ -32,12 +38,14 @@ func (p *Position) Eq(cmp Position) bool {
// Advance advances the position by a line if the rune is'\n', does nothing if the rune
// is '\r' and advances by a col character if the rune is anything else.
func (p *Position) Advance(r rune) {
p.Index++
if r == '\r' {
p.carriageReturns[p.Index] = struct{}{}
return
}
if r == '\n' {
// Store the line length for when we retreat.
p.lineLengths[p.Line] = p.Col
p.lineFeeds[p.Index] = struct{}{}
p.Line++
p.Col = 0
return
Expand All @@ -48,12 +56,16 @@ func (p *Position) Advance(r rune) {
// Retreat decreases the position by a line if the rune is'\n', does nothing if the rune
// is '\r' and decreases by a col character if the rune is anything else.
func (p *Position) Retreat(r rune) {
p.Index--
if r == '\r' {
return
}
if r == '\n' {
lfIndex := p.Index + 1
if _, isRetreatingFromCR := p.carriageReturns[p.Index+1]; isRetreatingFromCR {
lfIndex++
}
if _, isRetreatingFromNewLine := p.lineFeeds[lfIndex]; isRetreatingFromNewLine {
p.Line--
// Retrieve the line length.
p.Col = p.lineLengths[p.Line]
return
}
Expand Down
115 changes: 103 additions & 12 deletions input/position_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,41 +5,63 @@ import (
)

func TestPositionAdvance(t *testing.T) {
p := NewPosition(1, 1)
p := NewPosition(1, 0)
p.Advance('\n')
p.Advance('a')
if p.Line != 2 {
t.Errorf("expected a newline to advance the position from one to two")
}
if p.Col != 1 {
t.Errorf("expected a newline to bounce to the zeroth character on the next line")
}
p.Advance('\n')
p.Advance('a')
if p.Line != 3 {
t.Errorf("expected a newline to advance the position from two to three")
}
if p.Col != 1 {
t.Errorf("expected a newline to bounce to the zeroth character on the next line")
}
p.Advance('\n')
p.Advance('a')
if p.Line != 4 {
t.Errorf("expected a newline to advance the position from three to four")
}
if p.Col != 1 {
t.Errorf("expected a newline to bounce to the zeroth character on the next line")
}
}

func TestPositionRetreat(t *testing.T) {
p := NewPosition(1, 1)
p.Advance('\n')
func TestPositionRetreatOverNewLine(t *testing.T) {
p := NewPosition(1, 0)
p.Advance('a') // Index 0
comparePosition(NewPosition(1, 1), p, t)
p.Advance('\n') // Index 1
comparePosition(NewPosition(2, 0), p, t)
p.Retreat('\n') // Index 0
comparePosition(NewPosition(1, 1), p, t)
p.Retreat('a') // Start of file
comparePosition(NewPosition(1, 0), p, t)
if p.Index != -1 {
t.Errorf("should be at start of file")
}
}

func TestPositionRetreat(t *testing.T) {
p := NewPosition(1, 0) // Start of file.
p.Advance('a')
comparePosition(NewPosition(2, 1), p, t)
comparePosition(NewPosition(1, 1), p, t)
p.Advance('b')
comparePosition(NewPosition(2, 2), p, t)
comparePosition(NewPosition(1, 2), p, t)
p.Advance('\n')
comparePosition(NewPosition(3, 0), p, t)
p.Advance('b')
comparePosition(NewPosition(3, 1), p, t)
p.Retreat('\n')
comparePosition(NewPosition(2, 2), p, t)
p.Retreat('b')
comparePosition(NewPosition(2, 0), p, t)
p.Advance('c')
comparePosition(NewPosition(2, 1), p, t)
p.Retreat('c')
comparePosition(NewPosition(2, 0), p, t)
p.Retreat('\n')
comparePosition(NewPosition(1, 2), p, t)
p.Retreat('b')
comparePosition(NewPosition(1, 1), p, t)
p.Retreat('a')
comparePosition(NewPosition(1, 0), p, t)
Expand All @@ -50,6 +72,75 @@ func comparePosition(expected, actual Position, t *testing.T) {
t.Errorf("expected %v, but got %v", expected.String(), actual.String())
}
}
func TestPositionAdvanceRetreatNewLine(t *testing.T) {
actual := NewPosition(1, 0)
actual.Advance('\n')
expected := NewPosition(2, 0)
if !expected.Eq(actual) {
t.Errorf("advance 0: '\\n': expected %v, but got %v", expected.String(), actual.String())
}
actual.Advance('\n')
expected = NewPosition(3, 0)
if !expected.Eq(actual) {
t.Errorf("advance 1: '\\n': expected %v, but got %v", expected.String(), actual.String())
}
actual.Retreat('\n')
expected = NewPosition(2, 0)
if !expected.Eq(actual) {
t.Errorf("retreat 2: '\\n': expected %v, but got %v", expected.String(), actual.String())
}
actual.Retreat('\n')
expected = NewPosition(1, 0)
if !expected.Eq(actual) {
t.Errorf("retreat 3: '\\n': expected %v, but got %v", expected.String(), actual.String())
}
}

func TestPositionAdvanceRetreat(t *testing.T) {
input := "\nab\nc\nd"
expectedAdvances := []Position{
NewPosition(2, 0), // \n
NewPosition(2, 1), // a
NewPosition(2, 2), // b
NewPosition(3, 0), // \n
NewPosition(3, 1), // c
NewPosition(4, 0), // \n
NewPosition(4, 1), // d
}
// Advance.
actual := NewPosition(1, 0)
for i, r := range input {
actual.Advance(r)
expected := expectedAdvances[i]
if !expected.Eq(actual) {
t.Errorf("advance %d (%s): expected %v, but got %v", i, string(r), expected.String(), actual.String())
}
}
if !actual.Eq(NewPosition(4, 1)) {
t.Errorf("after reading everything, expected %v, got %v", NewPosition(4, 1), actual)
}

// Unread everything.
input = "d\nc\nba\n"
expectedRetreats := []Position{
NewPosition(4, 0), // \d
NewPosition(3, 1), // \n
NewPosition(3, 0), // \c
NewPosition(2, 2), // \n
NewPosition(2, 1), // b
NewPosition(2, 0), // a
NewPosition(1, 0), // \n
}
// Retreat.
for i := 0; i > 0; i-- {
r := rune(input[i])
actual.Retreat(r)
expected := expectedRetreats[i]
if !expected.Eq(actual) {
t.Errorf("retreat %d (%s): expected %v, but got %v", i, string(r), expected.String(), actual.String())
}
}
}

func TestPositionString(t *testing.T) {
p := NewPosition(1, 1)
Expand Down
57 changes: 35 additions & 22 deletions input/streamposition_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,31 +117,29 @@ func TestStreamPositionRetreat(t *testing.T) {
name: "single line",
input: "12345",
expected: []Position{
NewPosition(1, 5),
NewPosition(1, 4),
NewPosition(1, 3),
NewPosition(1, 2),
NewPosition(1, 1),
NewPosition(1, 4), // -5
NewPosition(1, 3), // -4
NewPosition(1, 2), // -3
NewPosition(1, 1), // -2
},
},
{
name: "two lines",
input: "a\nb",
expected: []Position{
// b isn't included
NewPosition(2, 1), // \n
NewPosition(1, 1), //
NewPosition(1, 0), // start
NewPosition(2, 0), // \n
NewPosition(1, 1), // a
},
},
{
name: "windows line break",
input: "a\r\nb",
expected: []Position{
// b isn't included
NewPosition(2, 1), // \n, we still stay on the line
NewPosition(1, 1), // \r
NewPosition(1, 1), //
NewPosition(2, 0), // \n, we still stay on the line
NewPosition(2, 0), // \r
NewPosition(1, 1), // a
NewPosition(1, 0), // start
},
},
Expand All @@ -150,10 +148,10 @@ func TestStreamPositionRetreat(t *testing.T) {
input: "123\n456\n789",
expected: []Position{
// 9 isn't included
NewPosition(3, 3), // 8
NewPosition(3, 2), // 7
NewPosition(3, 2), // 8
NewPosition(3, 1), // 7

NewPosition(3, 1), // \n
NewPosition(3, 0), // \n

NewPosition(2, 3), // 6
NewPosition(2, 2), // 5
Expand All @@ -170,23 +168,38 @@ func TestStreamPositionRetreat(t *testing.T) {
},
}

retreatOperation := func(s *Stream) (rune, error) { return s.Retreat() }

for _, test := range tests {
actual := testPosition(test.input, len(test.input)+1, retreatOperation, t)
positions := make([]Position, 0)
ip := NewFromString(test.input)
for i := 0; i < len(test.input); i++ {
ip.Advance()
}

if len(test.expected) != len(actual) {
t.Errorf("name: '%s': expected %d positions, but got %d positions", test.name, len(test.expected), len(actual))
break
output := string(ip.CurrentRune)
for i := 0; i < len(test.expected); i++ {
ip.Retreat()
positions = append(positions, ip.position)
if ip.CurrentRune != 0 {
output += string(ip.CurrentRune)
}
}

// Check the positions.
if len(test.expected) != len(positions) {
t.Errorf("name: '%s': expected %d positions, but got %d positions", test.name, len(test.expected), len(positions))
}
for i, e := range test.expected {
a := actual[i]
a := positions[i]
if !e.Eq(a) {
t.Errorf("name '%s': index %d, expected position %v, but got %v", test.name, i, e.String(), a.String())
}
}
var reversedInput string
for _, v := range test.input {
reversedInput = string(v) + reversedInput
}
if reversedInput != output {
t.Errorf("name %q: expected output %q, got %q", test.name, reversedInput, output)
}
}
}

Expand Down
4 changes: 2 additions & 2 deletions parse/all.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ func All(combiner MultipleResultCombiner, functions ...Function) Function {
func all(pi Input, combiner MultipleResultCombiner, functions ...Function) Result {
results := make([]interface{}, len(functions))
start := pi.Index()
for i, f := range functions {
r := f(pi)
for i := 0; i < len(functions); i++ {
r := functions[i](pi)
if !r.Success {
rewind(pi, int(pi.Index()-start))
return r
Expand Down

0 comments on commit 8bf8fd3

Please sign in to comment.