REPL tool implementation code review

[Jan Lahoda]

Hi,
On 7.9.2015 18:26, Paul Sandoz wrote:
> On 7 Sep 2015, at 17:31, Jan Lahoda <jan.lahoda at oracle.com> wrote:
>> I've tried to resolve some of the comments from the original e-mail here:
>> http://hg.openjdk.java.net/kulla/dev/langtools/rev/45e459d45f64
>>
>
> Looks good, it’s ok to do:
>
>    forEach(results::add)
>
> rather than create additional collections if you know what you are doing.

Done here:

http://hg.openjdk.java.net/kulla/dev/langtools/rev/dbbedddaf7fe

>
>
>> On 7.9.2015 14:59, Paul Sandoz wrote:
>>> On 7 Sep 2015, at 11:52, Jan Lahoda <jan.lahoda at oracle.com> wrote:
>>>
>>> Is the asynchronous operation on the REPL in 4 an issue?
>>
>> The asynchronous operation is also somewhat tricky (but necessary, I think). I have another patch in my queue that reflects (some of) Brian comments on that.
>>
>
> Ok, i was more questioning whether it is safe rather than necessary to perform on the reading thread, since i could not determine if stopping the REPL concurrently in the reader thread would interfere with executing user code performed in the main thread.
>
>
>>>
>>> My inclination is you should try and merge the thread logic in InputBuffer.wrapInIfNeeded into CircularBuffer, then modify and rename CircularBuffer to extend from InputStream, thereby containing all the relevant multi-threaded logic and it becomes an obvious thing being deferred to. You could also add another state CLOSED, which i think more clearly indicates closure rather than continually writing -1 every time you read -1.
>>>
>>> Sorry for labouring the point here, but i think this kind of thing is worth spending some time on. Consider someone trying to modify the code in a few years time. Review-wise we could spin this off as a separate bug so as not to block the code going in.
>>
>> No problem. I'll look into doing the changes as you describe. Thanks for the comments!

I've done some improvements to the CircularBuffer (including renaming 
it) here:
http://hg.openjdk.java.net/kulla/dev/langtools/rev/4141dc296d4d

I've also cleaned up the related code a little in the change, to 
simplify how the ConsoleIOContext works.

How does it look?

Thanks,
    Jan

>>
>
> Thanks,
> Paul.
>
>