Add Stdio.readByteSync and Stdio.readLineSync.

Add Stdio.readByteSync and Stdio.readLineSync.

BUG=https://code.google.com/p/dart/issues/detail?id=7136
R=whesse@google.com

Committed: https://code.google.com/p/dart/source/detail?r=24684

[Anders Johnsen, 2013-07-02 11:18:56]

Updated.

[Bill Hesse, 2013-07-02 11:31:54]

https://codereview.chromium.org/18516002/diff/1/sdk/lib/io/stdio.dart
File sdk/lib/io/stdio.dart (right):

https://codereview.chromium.org/18516002/diff/1/sdk/lib/io/stdio.dart#newcode41
sdk/lib/io/stdio.dart:41: * stdin pipe.
[Stdin] allows both synchronous and asynchronous reads from the standard input
stream.

https://codereview.chromium.org/18516002/diff/1/sdk/lib/io/stdio.dart#newcode50
sdk/lib/io/stdio.dart:50: * available. The line will contain the newline
character(s).
Synchronously read a line from stdin.

https://codereview.chromium.org/18516002/diff/1/sdk/lib/io/stdio.dart#newcode52
sdk/lib/io/stdio.dart:52: * If at end of file, `null` is returned.
If end-of-file is reached,

https://codereview.chromium.org/18516002/diff/1/sdk/lib/io/stdio.dart#newcode54
sdk/lib/io/stdio.dart:54: * If at end of file, while some data is already read,
that data is returned.
If end-of-file is reached after some data has already been read, that data is
returned.

https://codereview.chromium.org/18516002/diff/1/sdk/lib/io/stdio.dart#newcode84
sdk/lib/io/stdio.dart:84: * Read a byte from stdin. This call will block until a
byte is available.
Synchronously

https://codereview.chromium.org/18516002/diff/1/sdk/lib/io/stdio.dart#newcode182
sdk/lib/io/stdio.dart:182: external static IOSink _getStdioOutputStream(int fd);
Should these types be changed?

https://codereview.chromium.org/18516002/diff/1/tests/standalone/io/stdin_syn...
File tests/standalone/io/stdin_sync_script.dart (right):

https://codereview.chromium.org/18516002/diff/1/tests/standalone/io/stdin_syn...
tests/standalone/io/stdin_sync_script.dart:13: if (line != 'hej\x01\x00\r\n')
throw "bad line at $i";
No non-ascii tests?  Can we do a non-default UTF-8, and a non-ascii char, since
we are piping to the stdin anyway?

Should we also make the encoder throw, and test that we get the exception?

https://codereview.chromium.org/18516002/diff/6001/sdk/lib/io/stdio.dart
File sdk/lib/io/stdio.dart (right):

https://codereview.chromium.org/18516002/diff/6001/sdk/lib/io/stdio.dart#newc...
sdk/lib/io/stdio.dart:62: (chunk) {
Why is chunk not char, or rune?  It seems misleading, like it could be more than
a single char.

https://codereview.chromium.org/18516002/diff/6001/sdk/lib/io/stdio.dart#newc...
sdk/lib/io/stdio.dart:83: if (!retainNewlines) {
I would have put this into the sink object, with more logic and a
"lastCharWasCR" bool.  I think it is not too complex to put it there, and reads
better.

https://codereview.chromium.org/18516002/diff/6001/tests/standalone/io/stdin_...
File tests/standalone/io/stdin_sync_test.dart (right):

https://codereview.chromium.org/18516002/diff/6001/tests/standalone/io/stdin_...
tests/standalone/io/stdin_sync_test.dart:12:
process.stdin.write("hej\x01\x00\x0d\x0a\x0a4\x0a");
Also try a case with no newline at EOF?

[Anders Johnsen, 2013-07-02 12:21:27]

PTAL

https://codereview.chromium.org/18516002/diff/1/sdk/lib/io/stdio.dart
File sdk/lib/io/stdio.dart (right):

https://codereview.chromium.org/18516002/diff/1/sdk/lib/io/stdio.dart#newcode41
sdk/lib/io/stdio.dart:41: * stdin pipe.
On 2013/07/02 11:31:54, Bill Hesse wrote:
> [Stdin] allows both synchronous and asynchronous reads from the standard input
> stream.

Done.

https://codereview.chromium.org/18516002/diff/1/sdk/lib/io/stdio.dart#newcode50
sdk/lib/io/stdio.dart:50: * available. The line will contain the newline
character(s).
On 2013/07/02 11:31:54, Bill Hesse wrote:
> Synchronously read a line from stdin.

Done.

https://codereview.chromium.org/18516002/diff/1/sdk/lib/io/stdio.dart#newcode52
sdk/lib/io/stdio.dart:52: * If at end of file, `null` is returned.
On 2013/07/02 11:31:54, Bill Hesse wrote:
> If end-of-file is reached,

Done.

https://codereview.chromium.org/18516002/diff/1/sdk/lib/io/stdio.dart#newcode54
sdk/lib/io/stdio.dart:54: * If at end of file, while some data is already read,
that data is returned.
On 2013/07/02 11:31:54, Bill Hesse wrote:
> If end-of-file is reached after some data has already been read, that data is
> returned.

Done.

https://codereview.chromium.org/18516002/diff/1/sdk/lib/io/stdio.dart#newcode84
sdk/lib/io/stdio.dart:84: * Read a byte from stdin. This call will block until a
byte is available.
On 2013/07/02 11:31:54, Bill Hesse wrote:
> Synchronously

Done.

https://codereview.chromium.org/18516002/diff/1/sdk/lib/io/stdio.dart#newcode182
sdk/lib/io/stdio.dart:182: external static IOSink _getStdioOutputStream(int fd);
On 2013/07/02 11:31:54, Bill Hesse wrote:
> Should these types be changed?

Done.

https://codereview.chromium.org/18516002/diff/1/tests/standalone/io/stdin_syn...
File tests/standalone/io/stdin_sync_script.dart (right):

https://codereview.chromium.org/18516002/diff/1/tests/standalone/io/stdin_syn...
tests/standalone/io/stdin_sync_script.dart:13: if (line != 'hej\x01\x00\r\n')
throw "bad line at $i";
On 2013/07/02 11:31:54, Bill Hesse wrote:
> No non-ascii tests?  Can we do a non-default UTF-8, and a non-ascii char,
since
> we are piping to the stdin anyway?
> 
> Should we also make the encoder throw, and test that we get the exception?

Added non-ascii test. The exception will not happen due to replacement char.

https://codereview.chromium.org/18516002/diff/6001/sdk/lib/io/stdio.dart
File sdk/lib/io/stdio.dart (right):

https://codereview.chromium.org/18516002/diff/6001/sdk/lib/io/stdio.dart#newc...
sdk/lib/io/stdio.dart:62: (chunk) {
On 2013/07/02 11:31:54, Bill Hesse wrote:
> Why is chunk not char, or rune?  It seems misleading, like it could be more
than
> a single char.

Done.

https://codereview.chromium.org/18516002/diff/6001/sdk/lib/io/stdio.dart#newc...
sdk/lib/io/stdio.dart:83: if (!retainNewlines) {
On 2013/07/02 11:31:54, Bill Hesse wrote:
> I would have put this into the sink object, with more logic and a
> "lastCharWasCR" bool.  I think it is not too complex to put it there, and
reads
> better.

Done.

https://codereview.chromium.org/18516002/diff/6001/tests/standalone/io/stdin_...
File tests/standalone/io/stdin_sync_test.dart (right):

https://codereview.chromium.org/18516002/diff/6001/tests/standalone/io/stdin_...
tests/standalone/io/stdin_sync_test.dart:12:
process.stdin.write("hej\x01\x00\x0d\x0a\x0a4\x0a");
On 2013/07/02 11:31:54, Bill Hesse wrote:
> Also try a case with no newline at EOF?

Done.

[Bill Hesse, 2013-07-02 13:08:27]

LGTM, with comments.

https://codereview.chromium.org/18516002/diff/9003/sdk/lib/io/stdio.dart
File sdk/lib/io/stdio.dart (right):

https://codereview.chromium.org/18516002/diff/9003/sdk/lib/io/stdio.dart#newc...
sdk/lib/io/stdio.dart:65: if (!retainNewlines && char == '\r') {
You need to output 'foo\r' for 'foo\r\r\n'.  Move the \r test below the
lastCharWasCT test.

if (char == '\n') ...
else {
if (lastCharWasCR) ...

if (!retainNewlines && char == '\r')

 return
}
line.write(char);

https://codereview.chromium.org/18516002/diff/9003/tests/standalone/io/stdin_...
File tests/standalone/io/stdin_sync_test.dart (right):

https://codereview.chromium.org/18516002/diff/9003/tests/standalone/io/stdin_...
tests/standalone/io/stdin_sync_test.dart:36: test("hej\xFF\x00",
['hej\xFF\x00']);
This should fail on Windows, since you are writing with a UTF-8 encoding and
reading with a Windows code page.  At least this is what it looks like to me
(Encoding.UTF_8 on process.stdin.write and Encoding.SYSTEM on readLineSync. 
That raises the question of what encoding is happening to arguments in
Process.start.  Maybe not just \xFF but \u01234 should be tried.

https://codereview.chromium.org/18516002/diff/9003/tests/standalone/io/stdin_...
tests/standalone/io/stdin_sync_test.dart:39: 
"hej\r\r\nhej\r\rhej\n"

[Anders Johnsen, 2013-07-02 13:17:14]

https://codereview.chromium.org/18516002/diff/9003/sdk/lib/io/stdio.dart
File sdk/lib/io/stdio.dart (right):

https://codereview.chromium.org/18516002/diff/9003/sdk/lib/io/stdio.dart#newc...
sdk/lib/io/stdio.dart:65: if (!retainNewlines && char == '\r') {
On 2013/07/02 13:08:27, Bill Hesse wrote:
> You need to output 'foo\r' for 'foo\r\r\n'.  Move the \r test below the
> lastCharWasCT test.
> 
> if (char == '\n') ...
> else {
> if (lastCharWasCR) ...
> 
> if (!retainNewlines && char == '\r')
> 
>  return
> }
> line.write(char);
> 

Done.

https://codereview.chromium.org/18516002/diff/9003/tests/standalone/io/stdin_...
File tests/standalone/io/stdin_sync_test.dart (right):

https://codereview.chromium.org/18516002/diff/9003/tests/standalone/io/stdin_...
tests/standalone/io/stdin_sync_test.dart:36: test("hej\xFF\x00",
['hej\xFF\x00']);
On 2013/07/02 13:08:27, Bill Hesse wrote:
> This should fail on Windows, since you are writing with a UTF-8 encoding and
> reading with a Windows code page.  At least this is what it looks like to me
> (Encoding.UTF_8 on process.stdin.write and Encoding.SYSTEM on readLineSync. 
> That raises the question of what encoding is happening to arguments in
> Process.start.  Maybe not just \xFF but \u01234 should be tried.

Done.

https://codereview.chromium.org/18516002/diff/9003/tests/standalone/io/stdin_...
tests/standalone/io/stdin_sync_test.dart:39: 
On 2013/07/02 13:08:27, Bill Hesse wrote:
> "hej\r\r\nhej\r\rhej\n"

Done.

[Anders Johnsen, 2013-07-02 13:18:11]

Committed patchset #5 manually as r24684 (presubmit successful).