Merge pull request #15 from jf-tech/scanline

Add ios.ByteReadLine

Author: jf-tech

ios/readers.go

@@ -134,39 +134,50 @@ func (r *BytesReplacingReader) Read(p []byte) (int, error) {
 	}
 }
 
-// ReadLine reads in a single line from a bufio.Reader.
-func ReadLine(r *bufio.Reader) (string, error) {
-	// Turns out even with various bufio.Reader.Read???() and bufio.Scanner, there is not simple clean
-	// way of reading a single text line in:
-	// - bufio.ReadSlice('\n') doesn't have '\r' dropping. We want a line returned without neither '\n' nor '\r'.
-	// - bufio.ReadLine() drops '\r' and '\n', but has a fixed buf so may be unable to read a whole line in one call.
-	// - bufio.ReadBytes no buf size issue, but doesn't offer '\r' and '\n' cleanup.
+// ByteReadLine reads in a single line from a bufio.Reader and returns it in []byte.
+// Note the returned []byte may be pointing directly into the bufio.Reader, so assume
+// the returned []byte will be invalidated and shouldn't be used upon next ByteReadLine
+// call.
+func ByteReadLine(r *bufio.Reader) ([]byte, error) {
+	// We want to read a line delimited by '\n' in cleanly with trailing '\r' '\n' dropped. Turns out
+	// none of the bufio.Reader.Read???() and bufio.Scanner can do that:
+	// - bufio.ReadSlice('\n') doesn't have '\r' dropping. Also it may need multiple calls to get a single (long) line.
+	// - bufio.ReadLine() drops '\r' and '\n', but may need multiple calls to get a single (long) line.
+	// - bufio.ReadBytes doesn't need multiple calls, but doesn't offer '\r' and '\n' cleanup. Also tons of allocations.
 	// - bufio.ReadString essentially the same as bufio.ReadBytes.
-	// - bufio.Scanner deals with '\r' and '\n' but has fixed buf issue.
-	// Oh, come on!!
+	// - bufio.Scanner deals with '\r' and '\n' but may need multiple calls to get a single (long) line.
 	//
-	// Also found net/textproto's Reader.ReadLine() which meets all the requirements. But to use it
-	// we need to create yet another type of Reader (net.textproto.Reader), as if the
-	// io.Reader -> bufio.Reader isn't enough for us. So decided instead, just shamelessly copy
-	// net.textproto.Reader.ReadLine() here, credit goes to
-	// https://github.com/golang/go/blob/master/src/net/textproto/reader.go. However its test code
-	// coverage is lacking, so create all the new test cases for this ReadLine implementation copy.
+	// Found net/textproto's Reader.ReadLine() which meets all the requirements. But it would become another
+	// dependency. So decided just shamelessly copy net.textproto.Reader.ReadLine() here, credit goes to
+	// https://github.com/golang/go/blob/master/src/net/textproto/reader.go. Its test code coverage is lacking,
+	// so create all the new test cases for this ReadLine implementation copy.
 	var line []byte
 	for {
 		l, more, err := r.ReadLine()
 		if err != nil {
-			return "", err
+			return nil, err
 		}
 		// Avoid the copy if the first call produced a full line.
 		if line == nil && !more {
-			return string(l), nil
+			line = l
+			goto returnLine
 		}
 		line = append(line, l...)
 		if !more {
 			break
 		}
 	}
-	return string(line), nil
+returnLine:
+	if len(line) == 0 {
+		return nil, nil
+	}
+	return line, nil
+}
+
+// ReadLine reads in a single line from a bufio.Reader and returns it in string.
+func ReadLine(r *bufio.Reader) (string, error) {
+	b, err := ByteReadLine(r)
+	return string(b), err
 }
 
 const bom = '\uFEFF'

ios/readers_test.go

@@ -163,57 +163,82 @@ func BenchmarkRegularReader_50KBLength_1000Targets(b *testing.B) {
 	}
 }
 
-func TestReadLine(t *testing.T) {
+func TestByteReadLineAndReadLine(t *testing.T) {
 	for _, test := range []struct {
-		name           string
-		input          string
-		bufsize        int
-		expectedOutput []string
+		name     string
+		input    string
+		bufsize  int
+		expected [][]byte
 	}{
 		{
-			name:           "empty",
-			input:          "",
-			bufsize:        1024,
-			expectedOutput: []string{},
+			name:     "empty",
+			input:    "",
+			bufsize:  1024,
+			expected: nil,
 		},
 		{
-			name:           "single-line with no newline",
-			input:          "   word1, word2 - word3 !@#$%^&*()",
-			bufsize:        1024,
-			expectedOutput: []string{"   word1, word2 - word3 !@#$%^&*()"},
+			name:    "single-line with no newline",
+			input:   "   word1, word2 - word3 !@#$%^&*()",
+			bufsize: 1024,
+			expected: [][]byte{
+				[]byte("   word1, word2 - word3 !@#$%^&*()"),
+			},
 		},
 		{
-			name:           "single-line with '\\r' and '\\n'",
-			input:          "line1\r\n",
-			bufsize:        1024,
-			expectedOutput: []string{"line1"},
+			name:     "single-line with CR and LF",
+			input:    "line1\r\n",
+			bufsize:  1024,
+			expected: [][]byte{[]byte("line1")},
 		},
 		{
-			name:           "multi-line - bufsize enough",
-			input:          "line1\r\nline2\nline3",
-			bufsize:        1024,
-			expectedOutput: []string{"line1", "line2", "line3"},
+			name:     "multi-line - bufsize enough",
+			input:    "line1\r\nline2\nline3",
+			bufsize:  1024,
+			expected: [][]byte{[]byte("line1"), []byte("line2"), []byte("line3")},
 		},
 		{
-			name:           "multi-line - bufsize not enough; also empty line",
-			input:          "line1-0123456789012345\r\n\nline3-0123456789012345",
-			bufsize:        16, // bufio.minReadBufferSize is 16.
-			expectedOutput: []string{"line1-0123456789012345", "", "line3-0123456789012345"},
+			name:     "multi-line - bufsize not enough; also empty line",
+			input:    "line1-0123456789012345\r\n\nline3-0123456789012345",
+			bufsize:  16, // bufio.minReadBufferSize is 16.
+			expected: [][]byte{[]byte("line1-0123456789012345"), nil, []byte("line3-0123456789012345")},
 		},
 	} {
 		t.Run(test.name, func(t *testing.T) {
 			r := bufio.NewReaderSize(strings.NewReader(test.input), test.bufsize)
-			output := []string{}
+			var outputBytes [][]byte
+			for {
+				line, err := ByteReadLine(r)
+				if err != nil {
+					assert.Nil(t, line)
+					assert.Equal(t, io.EOF, err)
+					break
+				}
+				if line != nil {
+					// note the []byte returned by ByteReadLine can be invalidated upon next call, so
+					// let's make a copy of it, thus the seemingly unnecessary `[]byte(string(line))`.
+					outputBytes = append(outputBytes, []byte(string(line)))
+				} else {
+					outputBytes = append(outputBytes, line)
+				}
+			}
+			assert.Equal(t, test.expected, outputBytes)
+
+			r = bufio.NewReaderSize(strings.NewReader(test.input), test.bufsize)
+			outputBytes = outputBytes[:0]
 			for {
 				line, err := ReadLine(r)
 				if err != nil {
 					assert.Equal(t, "", line)
 					assert.Equal(t, io.EOF, err)
 					break
 				}
-				output = append(output, line)
+				if line == "" {
+					outputBytes = append(outputBytes, nil)
+				} else {
+					outputBytes = append(outputBytes, []byte(line))
+				}
 			}
-			assert.Equal(t, test.expectedOutput, output)
+			assert.Equal(t, test.expected, outputBytes)
 		})
 	}
 }