From dc8fb6fdb5da493843c47ca49c8e1a360bbbab30 Mon Sep 17 00:00:00 2001 From: Billy Keyes Date: Mon, 25 Mar 2019 21:30:08 -0700 Subject: [PATCH] Support adjustable read-ahead in parser At the moment, we need only to read three lines, but now the value is easy to adjust as needed. I've only seen the Git implementation read two lines ahead so far, but it has the whole input in memory and may read more in other places. --- gitdiff/file_header.go | 3 +-- gitdiff/file_header_test.go | 2 +- gitdiff/parser.go | 45 +++++++++++++++++-------------------- gitdiff/parser_test.go | 29 +++++++++++++++++------- 4 files changed, 44 insertions(+), 35 deletions(-) diff --git a/gitdiff/file_header.go b/gitdiff/file_header.go index d848e5c..b679471 100644 --- a/gitdiff/file_header.go +++ b/gitdiff/file_header.go @@ -17,14 +17,13 @@ func (p *parser) ParseGitFileHeader(f *File, header string) error { } for err = p.Next(); err == nil; err = p.Next() { - end, err := parseGitHeaderData(f, p.Line(), defaultName) + end, err := parseGitHeaderData(f, p.Line(0), defaultName) if err != nil { return p.Errorf(1, "git file header: %v", err) } if end { break } - p.Line() } if err != nil && err != io.EOF { return err diff --git a/gitdiff/file_header_test.go b/gitdiff/file_header_test.go index 5587cbb..5c066e7 100644 --- a/gitdiff/file_header_test.go +++ b/gitdiff/file_header_test.go @@ -147,7 +147,7 @@ index deadbeef p.Next() var f File - err := p.ParseGitFileHeader(&f, p.Line()) + err := p.ParseGitFileHeader(&f, p.Line(0)) if test.Err { if err == nil { t.Fatalf("expected error parsing git file header, got nil") diff --git a/gitdiff/parser.go b/gitdiff/parser.go index ebef5b5..137cbc6 100644 --- a/gitdiff/parser.go +++ b/gitdiff/parser.go @@ -44,7 +44,7 @@ type parser struct { eof bool lineno int64 - lines [2]string + lines [3]string } const ( @@ -63,7 +63,7 @@ func (p *parser) ParseNextFileHeader() (file *File, err error) { // based on find_header() in git/apply.c for err = p.Next(); err == nil; err = p.Next() { - line := p.Line() + line := p.Line(0) // check for disconnected fragment headers (corrupt patch) if isMaybeFragmentHeader(line) { @@ -85,16 +85,12 @@ func (p *parser) ParseNextFileHeader() (file *File, err error) { } // check for a "traditional" patch - if strings.HasPrefix(line, oldFilePrefix) && strings.HasPrefix(p.PeekLine(), newFilePrefix) { - if err = p.Next(); err != nil { - return nil, err - } - + if strings.HasPrefix(line, oldFilePrefix) && strings.HasPrefix(p.Line(1), newFilePrefix) { oldFileLine := line - newFileLine := p.Line() + newFileLine := p.Line(1) // only a file header if followed by a (probable) unified fragment header - if !isMaybeFragmentHeader(p.PeekLine()) { + if !isMaybeFragmentHeader(p.Line(2)) { continue } @@ -127,9 +123,11 @@ func (p *parser) Next() error { } if p.lineno == 0 { - // on the first call, need extra shift to initialize both slots - if err := p.shiftLines(); err != nil && err != io.EOF { - return err + // on first call to next, need to shift in all lines + for i := 0; i < len(p.lines)-1; i++ { + if err := p.shiftLines(); err != nil && err != io.EOF { + return err + } } } @@ -145,21 +143,20 @@ func (p *parser) Next() error { } func (p *parser) shiftLines() (err error) { - p.lines[0] = p.lines[1] - p.lines[1], err = p.r.ReadString('\n') + for i := 0; i < len(p.lines)-1; i++ { + p.lines[i] = p.lines[i+1] + } + p.lines[len(p.lines)-1], err = p.r.ReadString('\n') return } -// Line returns the current line or an empty string if Next has returned io.EOF. -func (p *parser) Line() string { - return p.lines[0] -} - -// PeekLine returns the line following the current line or an empty string if -// the current line is the final line. If PeekLine returns an empty string, -// Next will return io.EOF on the next call. -func (p *parser) PeekLine() string { - return p.lines[1] +// Line returns a line from the parser without advancing it. A delta of 0 +// returns the current line, while higher deltas return read-ahead lines. It +// returns an empty string if the delta is higher than the available lines, +// either because of the buffer size or because the parser reached the end of +// the input. Valid lines always contain at least a newline character. +func (p *parser) Line(delta uint) string { + return p.lines[delta] } // Errorf generates an error and appends the current line information. diff --git a/gitdiff/parser_test.go b/gitdiff/parser_test.go index 6aef8e3..1bb3659 100644 --- a/gitdiff/parser_test.go +++ b/gitdiff/parser_test.go @@ -2,6 +2,7 @@ package gitdiff import ( "bufio" + "io" "reflect" "strings" "testing" @@ -14,14 +15,14 @@ func TestLineOperations(t *testing.T) { return &parser{r: bufio.NewReader(strings.NewReader(content))} } - t.Run("readLine", func(t *testing.T) { + t.Run("read", func(t *testing.T) { p := newParser() if err := p.Next(); err != nil { t.Fatalf("error advancing parser: %v", err) } - line := p.Line() + line := p.Line(0) if line != "the first line\n" { t.Fatalf("incorrect first line: %s", line) } @@ -30,30 +31,42 @@ func TestLineOperations(t *testing.T) { t.Fatalf("error advancing parser: %v", err) } - line = p.Line() - if p.Line() != "the second line\n" { + line = p.Line(0) + if line != "the second line\n" { t.Fatalf("incorrect second line: %s", line) } + + if err := p.Next(); err != nil { + t.Fatalf("error advancing parser: %v", err) + } + + line = p.Line(0) + if line != "the third line\n" { + t.Fatalf("incorrect third line: %s", line) + } + + if err := p.Next(); err != io.EOF { + t.Fatalf("expected EOF, but got: %v", err) + } }) - t.Run("peekLine", func(t *testing.T) { + t.Run("peek", func(t *testing.T) { p := newParser() if err := p.Next(); err != nil { t.Fatalf("error advancing parser: %v", err) } - line := p.PeekLine() + line := p.Line(1) if line != "the second line\n" { t.Fatalf("incorrect peek line: %s", line) } - // test that reading the line returns the same value if err := p.Next(); err != nil { t.Fatalf("error advancing parser: %v", err) } - line = p.Line() + line = p.Line(0) if line != "the second line\n" { t.Fatalf("incorrect line: %s", line) }