give user application access to underlying input and print streams

[dwightguth]

This is a reopening of PR #48. The K Framework has been using a quite old version of nailgun with some patches we created for some time now and I figured it was time to try to migrate ourselves to the upstream version of nailgun, but unfortunately, our code won't build without this commit. The issue is that we use JANSI to ensure we don't write terminal escape sequences to stdout unless stdout isatty, and this requirse replacing the current implementation of System.out. We can't do this with nailgun using System.setOut() because we need to do it on a per-nail basis. Thus, exposing this functionality publically instead of package-locally is required.

[dwightguth]

I don't understand why this change would break the python tests on Windows. Were they already failing prior to this?

[dwightguth]

I'm open to another solution, but essentially the problem is that nailgun calls System.setOut and System.setErr, and if you have an application that you want to run with nailgun AND modify the default behavior of System.out and System.err, then you need System.out to pass first through ThreadLocalPrintStream and /then/ through your custom System.out implementation, but if application code calls System.setOut inside a nail, you end up setting it up to go first through your custom System.out and then through ThreadLocalPrintStream, which as you can imagine, breaks some stuff. So this is a way to allow users to have something similar to System.setOut that can be called in the context of a nail.

I'm fine if you want to come up with an alternate API to allow this that you feel is more suitable, though.

[sbalabanov-zz]

I'd rather come up with separate API for that, like NGContext.setIn NGContext.setOut NGContext.setErr and make sure default streams are closed properly

[dwightguth]

Can you clarify what you mean by making sure the default streams are closed properly? The behavior in our use case is that we set System.out to a PrintStream that wraps the old System.out, so it wouldn't really make sense for NGContext.setOut to close the old System.out.

[sbalabanov-zz]

Inside NGContext.setOut, we'll probably have to instantiate a new instance of thread local stream and wrap what was provided as an argument to setOut. However NGContext.out is already set to another thread local stream instance (it happens before nailgun starts to execute any nail), so this one has to be properly disposed, either immediately or at nail completion.

[dwightguth]

You shouldn't actually have to create a new ThreadLocalPrintStream, I shouldn't think. You just need to modify the PrintStream /inside/ the current thread. Take a look at what I wrote and let me know what you think; I tested it on our application and it's sufficient. Also, unless I'm mistaken, NGContext.out gets set to the NGOutputStream inside the ThreadLocalPrintStream, not the ThradLocalPrintStream.

[dwightguth]

What is the status of this?

[dwightguth]

@sbalabanov Can I get an update on what I need to do for this? Note that if I were to create a new ThreadLocalPrintStream it would disrupt the streams of other nails, so I don't believe that's what we actually want to do. Have you had a chance to look at the modified code I proposed? I'd appreciate knowing what you think I need to do, if anything, for this PR to proceed further.

[dwightguth]

@sbalabanov I addressed your comments. Let me know if you think I should be throwing a different exception than the one I chose.

[dwightguth]

Awesome, thanks! Do you know when you plan to do the next release?

[sbalabanov-zz]

As there is not much significant changes, we are not planning a release yet. You can just use master/snapshot version to have your changes in.