StreamParser eof

[kevinresol]

https://github.com/haxetink/tink_io/commit/4c4dc3ee88e06efff9ca15f5222c6f3cf052939d#diff-9c9d45e7ba3bb2121d3006aeb61d4d9bR59

The above change was to mitigate a situation where consume is called more than once, even it returned {resume: false}. It happens when the stream has already been depleted when consume is called.

stepwise:

1. Parser parses the last chunk in the stream and emits Done
2. consume() is called
3. forEach ends with Depleted instead of Halted (yeah, even the handler emits Finish)
4. consume is called again

Not sure if the null check is a proper fix. Maybe something should be done in the doParse function instead.

[back2dos]

Hmmm. This is an intersting point:

forEach ends with Depleted instead of Halted (yeah, even the handler emits Finish)

Instead we could yield Halted with a depleted rest. That does give the caller one more case to care about, but at least it doesn't swallow information. What do you think?

One way or the other, we should handle that problem internally by avoiding to call consume after it returned { resume: false }.

[kevinresol]

That does give the caller one more case to care about, but at least it doesn't swallow information.

Shouldn't be a big deal I guess? Because I think it is more natural for handlers that emit Finish to expect more instead of expecting Depleted. It will be very likely for me to write this:

stream.forEach(function(_) return Finish).handle(function() switch o {
  case Depleted; throw 'unreachable';
  case Halted(_): // expected
  default: // errors
});

[back2dos]

Ok. Well then, let's make sure we emit Halted.

[kevinresol]

case Depleted; throw 'unreachable';

Well, second thought on this: This is wrong because the handler can never get run if the stream is empty. So let's just hold the change and fix the usage first.

[kevinresol]

Please review: https://github.com/haxetink/tink_io/commit/c9f3b3e1335df89009b39fd52c4151fbf818f954

[back2dos]

The change seems fine.

I really need to clean up that function though :D

[kevinresol]

Not sure if it is a problem:

Say I have a stream parser that transform a Source into Stream<Int> like this

source.parseStream(new SimpleBytewiseParser(function(c) return Done(c)));

This will always give -1 as the last stream item. There seems no way to tell eof() that we can safely end the parsing without a result. Because eof either results in a error from Progressed/Failed(err), or emits a result from Done(data).

[back2dos]

Hmm, I suppose for the pure branch we can check if the remaining source is depleted after an item is done and end the stream right there.

[kevinresol]

Hmm, I suppose for the pure branch we can check if the remaining source is depleted after an item is done and end the stream right there.

I don't quite get that...

[kevinresol]

So do you mean we don't have to eof() at all?

[kevinresol]

Ignore me, I think I figured out the fix

[kevinresol]

As at fdd03c8, if the parser keep emitting Progressed til the end of source. The parsed result will be null, and the eof() function is not called. Because it went into this branch

[kevinresol]

Looks like after updating the dependencies this is broken again: https://travis-ci.org/haxetink/tink_io/jobs/399863230#L580

[kevinresol]

I think this is done now.