fork of go-gitdiff with jj support

Improve clarity of LineReaderAt implementation

Try to avoid confusion about whether variables refer to byte or line
counts/positions. It still isn't perfect, but I'm not sure how to
further clarify without being overly verbose.

Changed files
+36 -31
gitdiff
+36 -31
gitdiff/io.go
··· 31 31 } 32 32 33 33 func (r *lineReaderAt) ReadLinesAt(lines [][]byte, offset int64) (n int, err error) { 34 - // TODO(bkeyes): revisit variable names 35 - // - it's generally not clear when something is bytes vs lines 36 - // - offset is a good example of this 37 - 38 34 if offset < 0 { 39 35 return 0, errors.New("ReadLinesAt: negative offset") 40 36 } ··· 42 38 return 0, nil 43 39 } 44 40 45 - endLine := offset + int64(len(lines)) 41 + count := len(lines) 42 + startLine := offset 43 + endLine := startLine + int64(count) 44 + 46 45 if endLine > int64(len(r.index)) && !r.eof { 47 46 if err := r.indexTo(endLine); err != nil { 48 47 return 0, err 49 48 } 50 49 } 51 - if offset > int64(len(r.index)) { 50 + if startLine > int64(len(r.index)) { 52 51 return 0, io.EOF 53 52 } 54 53 55 - size, readOffset := lookupLines(r.index, offset, int64(len(lines))) 56 - 57 - b := make([]byte, size) 58 - if _, err := r.r.ReadAt(b, readOffset); err != nil { 59 - if err == io.EOF { 60 - err = errors.New("ReadLinesAt: corrupt line index or changed source data") 61 - } 54 + buf, byteOffset, err := r.readBytes(startLine, int64(count)) 55 + if err != nil { 62 56 return 0, err 63 57 } 64 58 65 - for n = 0; n < len(lines) && offset+int64(n) < int64(len(r.index)); n++ { 66 - i := offset + int64(n) 67 - start, end := readOffset, r.index[i] 68 - if i > 0 { 69 - start = r.index[i-1] 59 + for n = 0; n < count && startLine+int64(n) < int64(len(r.index)); n++ { 60 + lineno := startLine + int64(n) 61 + start, end := int64(0), r.index[lineno]-byteOffset 62 + if lineno > 0 { 63 + start = r.index[lineno-1] - byteOffset 70 64 } 71 - lines[n] = b[start-readOffset : end-readOffset] 65 + lines[n] = buf[start:end] 72 66 } 73 67 74 - if n < len(lines) || b[size-1] != '\n' { 68 + if n < count || buf[len(buf)-1] != '\n' { 75 69 return n, io.EOF 76 70 } 77 71 return n, nil ··· 110 104 return nil 111 105 } 112 106 113 - // lookupLines gets the byte offset and size of a range of lines from an index 114 - // where the value at n is the offset of the first byte after line number n. 115 - func lookupLines(index []int64, start, n int64) (size int64, offset int64) { 116 - if start > int64(len(index)) { 117 - offset = index[len(index)-1] 118 - } else if start > 0 { 119 - offset = index[start-1] 107 + // readBytes reads the bytes of the n lines starting at line and returns the 108 + // bytes and the offset of the first byte in the underlying source. 109 + func (r *lineReaderAt) readBytes(line, n int64) (b []byte, offset int64, err error) { 110 + indexLen := int64(len(r.index)) 111 + 112 + var size int64 113 + if line > indexLen { 114 + offset = r.index[indexLen-1] 115 + } else if line > 0 { 116 + offset = r.index[line-1] 120 117 } 121 118 if n > 0 { 122 - if start+n > int64(len(index)) { 123 - size = index[len(index)-1] - offset 119 + if line+n > indexLen { 120 + size = r.index[indexLen-1] - offset 124 121 } else { 125 - size = index[start+n-1] - offset 122 + size = r.index[line+n-1] - offset 126 123 } 127 124 } 128 - return 125 + 126 + b = make([]byte, size) 127 + if _, err := r.r.ReadAt(b, offset); err != nil { 128 + if err == io.EOF { 129 + err = errors.New("ReadLinesAt: corrupt line index or changed source data") 130 + } 131 + return nil, 0, err 132 + } 133 + return b, offset, nil 129 134 } 130 135 131 136 func isLen(r io.ReaderAt, n int64) (bool, error) {