Re: [15] 8235792: LineNumberReader.getLineNumber() behavior is inconsistent with respect to EOF

[Brian Burkhalter]

The CSR [1] has been updated as described below.

Thanks,

Brian

[1] https://bugs.openjdk.java.net/browse/JDK-8241020

> On Jul 22, 2020, at 11:36 AM, Brian Burkhalter  
> wrote:
> 
> Yes, I think that the sentence
> 
> + * Line terminators are compressed into single newline
> + * ('\n') characters.
> 
> should be added to the specs of read(char[],int,int) and readLine(), or some 
> equivalent statement be added to the class level doc.
> 
>> Besides, the JDK 14 API states that the invocation
>> "Returns:
>> The number of *bytes* read, or -1 if the end of the stream has already been 
>> reached"
>> 
>> I think it should say "characters" rather than "bytes" but perhaps this has 
>> already been corrected.
> 
> No, it has not been correct and I agree that it should be “characters,” not 
> “bytes.”

Re: [15] 8235792: LineNumberReader.getLineNumber() behavior is inconsistent with respect to EOF

[Brian Burkhalter]

Hello,

> On Jul 22, 2020, at 4:52 AM, Raffaello Giulietti 
>  wrote:
> 
> the CSR for read(char[],int,int) does not explicitly specify that "line 
> terminators are compressed into single newline ('\n') characters", as the 
> no-arg read() spec does.
> 
> Thus, it's not entirely clear what happens when the buffer is just large 
> enough to accept the \r in a \r\n sequence, potentially giving the impression 
> that this leaves a pending unread \n that might be counted again in later 
> invocations of either read() or read(char[],int,int).
> 
> It might be helpful to explicitly repeat the "compression" rule of the no-arg 
> read() even for read(char[],int,int).

Yes, I think that the sentence

+ * Line terminators are compressed into single newline
+ * ('\n') characters.

should be added to the specs of read(char[],int,int) and readLine(), or some 
equivalent statement be added to the class level doc.

> Besides, the JDK 14 API states that the invocation
> "Returns:
> The number of *bytes* read, or -1 if the end of the stream has already been 
> reached"
> 
> I think it should say "characters" rather than "bytes" but perhaps this has 
> already been corrected.

No, it has not been correct and I agree that it should be “characters,” not 
“bytes.”

Thanks,

Brian

[15] 8235792: LineNumberReader.getLineNumber() behavior is inconsistent with respect to EOF

[Raffaello Giulietti]

Hello,

the CSR for read(char[],int,int) does not explicitly specify that "line 
terminators are compressed into single newline ('\n') characters", as 
the no-arg read() spec does.


Thus, it's not entirely clear what happens when the buffer is just large 
enough to accept the \r in a \r\n sequence, potentially giving the 
impression that this leaves a pending unread \n that might be counted 
again in later invocations of either read() or read(char[],int,int).


It might be helpful to explicitly repeat the "compression" rule of the 
no-arg read() even for read(char[],int,int).




Besides, the JDK 14 API states that the invocation
"Returns:
The number of *bytes* read, or -1 if the end of the stream has already 
been reached"


I think it should say "characters" rather than "bytes" but perhaps this 
has already been corrected.



Greetings
Raffaello




The CSR [2] has been approved so unless there are further comments I’ll push 
this change [1] this week.

Thanks,

Brian


On Mar 19, 2020, at 7:43 AM, Brian Burkhalter  
wrote:

Another webrev [1] which is adjusted from the previous one per the comments on 
the CSR [2] is available for review. The only change is to the class-level 
specification:

--- a/src/java.base/share/classes/java/io/LineNumberReader.java
+++ b/src/java.base/share/classes/java/io/LineNumberReader.java
@@ -41,7 +41,8 @@
  *
  *  A line is considered to be terminated by any one of a
  * line feed ('\n'), a carriage return ('\r'), or a carriage return followed
- * immediately by a linefeed.
+ * immediately by a linefeed, or any of the previous terminators followed by
+ * end of stream, or end of stream not preceded by another terminator.
  *

Thanks,

Brian


On Mar 13, 2020, at 10:28 AM, Brian Burkhalter  
wrote:

An updated webrev is at [1] and a CSR has been filed [2].


[1] http://cr.openjdk.java.net/~bpb/8235792/webrev.02/
[2] https://bugs.openjdk.java.net/browse/JDK-8241020

Re: [15] 8235792: LineNumberReader.getLineNumber() behavior is inconsistent with respect to EOF

[Brian Burkhalter]

Push to JDK 16 that is.

> On Jul 20, 2020, at 10:04 AM, Brian Burkhalter  
> wrote:
> 
> The CSR [2] has been approved so unless there are further comments I’ll push 
> this change [1] this week.
> 
> Thanks,
> 
> Brian
> 
>> On Mar 19, 2020, at 7:43 AM, Brian Burkhalter  
>> wrote:
>> 
>> Another webrev [1] which is adjusted from the previous one per the comments 
>> on the CSR [2] is available for review. The only change is to the 
>> class-level specification:
>> 
>> --- a/src/java.base/share/classes/java/io/LineNumberReader.java
>> +++ b/src/java.base/share/classes/java/io/LineNumberReader.java
>> @@ -41,7 +41,8 @@
>>  *
>>  *  A line is considered to be terminated by any one of a
>>  * line feed ('\n'), a carriage return ('\r'), or a carriage return followed
>> - * immediately by a linefeed.
>> + * immediately by a linefeed, or any of the previous terminators followed by
>> + * end of stream, or end of stream not preceded by another terminator.
>>  *
>> 
>> Thanks,
>> 
>> Brian
>> 
>>> On Mar 13, 2020, at 10:28 AM, Brian Burkhalter 
>>>  wrote:
>>> 
>>> An updated webrev is at [1] and a CSR has been filed [2].
> 
> [1] http://cr.openjdk.java.net/~bpb/8235792/webrev.02/
> [2] https://bugs.openjdk.java.net/browse/JDK-8241020
> 

Re: [15] 8235792: LineNumberReader.getLineNumber() behavior is inconsistent with respect to EOF

[Brian Burkhalter]

The CSR [2] has been approved so unless there are further comments I’ll push 
this change [1] this week.

Thanks,

Brian

> On Mar 19, 2020, at 7:43 AM, Brian Burkhalter  
> wrote:
> 
> Another webrev [1] which is adjusted from the previous one per the comments 
> on the CSR [2] is available for review. The only change is to the class-level 
> specification:
> 
> --- a/src/java.base/share/classes/java/io/LineNumberReader.java
> +++ b/src/java.base/share/classes/java/io/LineNumberReader.java
> @@ -41,7 +41,8 @@
>   *
>   *  A line is considered to be terminated by any one of a
>   * line feed ('\n'), a carriage return ('\r'), or a carriage return followed
> - * immediately by a linefeed.
> + * immediately by a linefeed, or any of the previous terminators followed by
> + * end of stream, or end of stream not preceded by another terminator.
>   *
> 
> Thanks,
> 
> Brian
> 
>> On Mar 13, 2020, at 10:28 AM, Brian Burkhalter  
>> wrote:
>> 
>> An updated webrev is at [1] and a CSR has been filed [2].

[1] http://cr.openjdk.java.net/~bpb/8235792/webrev.02/
[2] https://bugs.openjdk.java.net/browse/JDK-8241020

Re: [15] 8235792: LineNumberReader.getLineNumber() behavior is inconsistent with respect to EOF

[Brian Burkhalter]

Another webrev [1] which is adjusted from the previous one per the comments on 
the CSR [2] is available for review. The only change is to the class-level 
specification:

--- a/src/java.base/share/classes/java/io/LineNumberReader.java
+++ b/src/java.base/share/classes/java/io/LineNumberReader.java
@@ -41,7 +41,8 @@
  *
  *  A line is considered to be terminated by any one of a
  * line feed ('\n'), a carriage return ('\r'), or a carriage return followed
- * immediately by a linefeed.
+ * immediately by a linefeed, or any of the previous terminators followed by
+ * end of stream, or end of stream not preceded by another terminator.
  *

Thanks,

Brian

> On Mar 13, 2020, at 10:28 AM, Brian Burkhalter  
> wrote:
> 
> An updated webrev is at [1] and a CSR has been filed [2].

[1] http://cr.openjdk.java.net/~bpb/8235792/webrev.02/
[2] https://bugs.openjdk.java.net/browse/JDK-8241020

Re: [15] 8235792: LineNumberReader.getLineNumber() behavior is inconsistent with respect to EOF

[Brian Burkhalter]

An updated webrev is at [1] and a CSR has been filed [2].

Thanks,

Brian

[1] http://cr.openjdk.java.net/~bpb/8235792/webrev.01/
[2] https://bugs.openjdk.java.net/browse/JDK-8241020

> On Mar 6, 2020, at 11:13 AM, Brian Burkhalter  
> wrote:
> 
> Thanks, Roger. I’ll let it hang until next week and then file a CSR.
> 
> Brian
> 
>> On Mar 6, 2020, at 10:49 AM, Roger Riggs  wrote:
>> 
>> :) Still looks fine.
>> 
>> On 3/5/20 4:53 PM, Brian Burkhalter wrote:
>>> Might anyone else have any comments on this thread the original post in 
>>> which was [1].
>>> 
>>> Thanks,
>>> 
>>> Brian
>>> 
>>> [1] 
>>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2020-January/064417.html
>>>  
>>> 

Re: [15] 8235792: LineNumberReader.getLineNumber() behavior is inconsistent with respect to EOF

[Brian Burkhalter]

Thanks, Roger. I’ll let it hang until next week and then file a CSR.

Brian

> On Mar 6, 2020, at 10:49 AM, Roger Riggs  wrote:
> 
> :) Still looks fine.
> 
> On 3/5/20 4:53 PM, Brian Burkhalter wrote:
>> Might anyone else have any comments on this thread the original post in 
>> which was [1].
>> 
>> Thanks,
>> 
>> Brian
>> 
>> [1] 
>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2020-January/064417.html
>>  
>> 

Re: [15] 8235792: LineNumberReader.getLineNumber() behavior is inconsistent with respect to EOF

[Roger Riggs]

:) Still looks fine.

On 3/5/20 4:53 PM, Brian Burkhalter wrote:

Might anyone else have any comments on this thread the original post in which 
was [1].

Thanks,

Brian

[1] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2020-January/064417.html


On Jan 31, 2020, at 12:21 PM, Brian Burkhalter  
wrote:


On Jan 31, 2020, at 12:20 PM, Alan Bateman mailto:alan.bate...@oracle.com>> wrote:

On 31/01/2020 20:10, Brian Burkhalter wrote:

:
OK. I’ll change /TERM/EOL/ but not post another webrev unless there are further 
comments. I will wait to check it in until Monday in case someone has comments.


I think it would be good to more eyes on this as LNR has a history of resisting 
change.

I’ll hold off on it then for a while.


I think you'll need a CSR too this re-specifies when the conditions when the 
line number is incremented.

Yes, I agree.

Re: [15] 8235792: LineNumberReader.getLineNumber() behavior is inconsistent with respect to EOF

[Brian Burkhalter]

Might anyone else have any comments on this thread the original post in which 
was [1].

Thanks,

Brian

[1] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2020-January/064417.html

> On Jan 31, 2020, at 12:21 PM, Brian Burkhalter  
> wrote:
> 
>> On Jan 31, 2020, at 12:20 PM, Alan Bateman > > wrote:
>> 
>> On 31/01/2020 20:10, Brian Burkhalter wrote:
>>> :
>>> OK. I’ll change /TERM/EOL/ but not post another webrev unless there are 
>>> further comments. I will wait to check it in until Monday in case someone 
>>> has comments.
>>> 
>> I think it would be good to more eyes on this as LNR has a history of 
>> resisting change.
> 
> I’ll hold off on it then for a while.
> 
>> I think you'll need a CSR too this re-specifies when the conditions when the 
>> line number is incremented.
> 
> Yes, I agree.

Re: [15] 8235792: LineNumberReader.getLineNumber() behavior is inconsistent with respect to EOF

[Brian Burkhalter]

> On Jan 31, 2020, at 12:20 PM, Alan Bateman  wrote:
> 
> On 31/01/2020 20:10, Brian Burkhalter wrote:
>> :
>> OK. I’ll change /TERM/EOL/ but not post another webrev unless there are 
>> further comments. I will wait to check it in until Monday in case someone 
>> has comments.
>> 
> I think it would be good to more eyes on this as LNR has a history of 
> resisting change.

I’ll hold off on it then for a while.

> I think you'll need a CSR too this re-specifies when the conditions when the 
> line number is incremented.

Yes, I agree.

Thanks,

Brian

Re: [15] 8235792: LineNumberReader.getLineNumber() behavior is inconsistent with respect to EOF

[Alan Bateman]

On 31/01/2020 20:10, Brian Burkhalter wrote:

:
OK. I’ll change /TERM/EOL/ but not post another webrev unless there are further 
comments. I will wait to check it in until Monday in case someone has comments.

I think it would be good to more eyes on this as LNR has a history of 
resisting change. I think you'll need a CSR too this re-specifies when 
the conditions when the line number is incremented.


-Alan.

Re: [15] 8235792: LineNumberReader.getLineNumber() behavior is inconsistent with respect to EOF

[Brian Burkhalter]

Hi Roger,

Thanks for the review.

> On Jan 31, 2020, at 12:05 PM, Roger Riggs  wrote:
> 
> The spec text describing the current behavior is ok.
> 
> On 1/16/20 8:35 PM, Brian Burkhalter wrote:
>> This issue [1] attempts to reinstate the change for [2] which was backed out 
>> to fix [3] thereby maintaining longstanding behavior. The proposed change 
>> [4] modifies the specification to increment the line number also when EOF is 
>> encountered and is not immediately preceded by a line terminator. If this 
>> all seems reasonable then a CSR will be filed after the code review.
> 
> LineNumberReader:
> 
> 55: I would probably use 'EOL' instead of 'TERM’.

Yes, that is better.

>> 
>> One thing I am uncertain about is the change to skip() at lines 293-295 of 
>> LineNumberReader. I am not sure that those  lines should be added.
> Leave them in.
> Not changing prevChar would be misleading since the prevChar isn't adjecent
> to the next read.

OK. I’ll change /TERM/EOL/ but not post another webrev unless there are further 
comments. I will wait to check it in until Monday in case someone has comments.

Thanks,

Brian

Re: [15] 8235792: LineNumberReader.getLineNumber() behavior is inconsistent with respect to EOF

[Roger Riggs]

Hi Brian,

The spec text describing the current behavior is ok.

On 1/16/20 8:35 PM, Brian Burkhalter wrote:

This issue [1] attempts to reinstate the change for [2] which was backed out to 
fix [3] thereby maintaining longstanding behavior. The proposed change [4] 
modifies the specification to increment the line number also when EOF is 
encountered and is not immediately preceded by a line terminator. If this all 
seems reasonable then a CSR will be filed after the code review.


LineNumberReader:

55: I would probably use 'EOL' instead of 'TERM'.



One thing I am uncertain about is the change to skip() at lines 293-295 of 
LineNumberReader. I am not sure that those  lines should be added.

Leave them in.
Not changing prevChar would be misleading since the prevChar isn't adjecent
to the next read.

Roger



Thanks,

Brian

[1] https://bugs.openjdk.java.net/browse/JDK-8235792
[2] https://bugs.openjdk.java.net/browse/JDK-8230342
[3] https://bugs.openjdk.java.net/browse/JDK-8235668
[4] http://cr.openjdk.java.net/~bpb/8235792/webrev.00/

Re: [15] 8235792: LineNumberReader.getLineNumber() behavior is inconsistent with respect to EOF

[Brian Burkhalter]

Ping ...

> On Jan 16, 2020, at 5:35 PM, Brian Burkhalter  
> wrote:
> 
> This issue [1] attempts to reinstate the change for [2] which was backed out 
> to fix [3] thereby maintaining longstanding behavior. The proposed change [4] 
> modifies the specification to increment the line number also when EOF is 
> encountered and is not immediately preceded by a line terminator. If this all 
> seems reasonable then a CSR will be filed after the code review.
> 
> One thing I am uncertain about is the change to skip() at lines 293-295 of 
> LineNumberReader. I am not sure that those  lines should be added.
> 
> Thanks,
> 
> Brian
> 
> [1] https://bugs.openjdk.java.net/browse/JDK-8235792
> [2] https://bugs.openjdk.java.net/browse/JDK-8230342
> [3] https://bugs.openjdk.java.net/browse/JDK-8235668
> [4] http://cr.openjdk.java.net/~bpb/8235792/webrev.00/

[15] 8235792: LineNumberReader.getLineNumber() behavior is inconsistent with respect to EOF

[Brian Burkhalter]

This issue [1] attempts to reinstate the change for [2] which was backed out to 
fix [3] thereby maintaining longstanding behavior. The proposed change [4] 
modifies the specification to increment the line number also when EOF is 
encountered and is not immediately preceded by a line terminator. If this all 
seems reasonable then a CSR will be filed after the code review.

One thing I am uncertain about is the change to skip() at lines 293-295 of 
LineNumberReader. I am not sure that those  lines should be added.

Thanks,

Brian

[1] https://bugs.openjdk.java.net/browse/JDK-8235792
[2] https://bugs.openjdk.java.net/browse/JDK-8230342
[3] https://bugs.openjdk.java.net/browse/JDK-8235668
[4] http://cr.openjdk.java.net/~bpb/8235792/webrev.00/