Looks good Brian!
nit:
> + reader.reset(); // reset to position after '\n'
actually it's 'after '\r'' now.
No need for new review!
best regards,
-- daniel
On 26/04/19 19:27, Brian Burkhalter wrote:
Daniel,
I modified the test as shown below. It passes both before and after the
implementation change.
Thanks,
Brian
--- a/test/jdk/java/io/LineNumberReader/MarkSplitCRLF.java
+++ b/test/jdk/java/io/LineNumberReader/MarkSplitCRLF.java
@@ -56,6 +56,24 @@
}
@Test
+ public static void testCRNotFollowedByLF() throws IOException {
+ final String string = "foo\rbar";
+ try (Reader reader =
+ new LineNumberReader(new StringReader(string), 5)) {
+ reader.read(); // 'f'
+ reader.read(); // 'o'
+ reader.read(); // 'o'
+ reader.read(); // '\r'
+ reader.mark(1); // mark position of next character
+ reader.read(); // 'b'
+ reader.reset(); // reset to position after '\n'
+ assertEquals(reader.read(), 'b');
+ assertEquals(reader.read(), 'a');
+ assertEquals(reader.read(), 'r');
+ }
+ }
+
+ @Test
On Apr 26, 2019, at 4:23 AM, Daniel Fuchs <[email protected]
<mailto:[email protected]>> wrote:
I had to convince myself that there was no issue when
'\r' is not followed by '\n' but I couldn't fault the
logic.
I wonder if adding a third test case with '\r' not
followed by '\n' would be a good idea?
