Don't close STDOUT when not a tty.

[justinlittman]

In cases in which STDOUT is not a tty (e.g., when piped), LineWriter will close STDOUT. This causes subsequent uses of the LineWriter to report closed stream (IOError).

Refs sul-dlss/dlme-transform#54

[jrochkind]

Cool, thanks.

Are you interested in sharing what you are doing with "line_writer"? @billdueber added it as an abstraction, I was never completely sure if it was useful architecture. I'd definitely encourage you to be willing consider just write your own writer without sub-classing LineWriter, then you can make it do whatever you want, I'm not sure how much value LineWriter adds for the inheritance.

[justinlittman]

We're implementing our own custom JSON serializer on top of it: https://github.com/sul-dlss/dlme-transform/blob/master/lib/dlme_json_resource_writer.rb

It handles adjusting the cardinality of some values and performs validation.

[justinlittman]

This is also breaking DebugWriter for us, as it is a subclass of LineWriter.

[jrochkind]

I wish @billdueber was around to consult.

I'm not loving the increasing number of guards for the auto-close; feel like maybe it only oughta auto-close in cases where an output file path was provided, and not in other cases -- it shouldn't close when no output setting was given and it defaults to $stdout, but it also probably shouldn't auto-close if an output_stream setting is given

I think it should probably be setting a flag or otherwise detecting only when setting output_file is given and "white-listing' that, rather than try to guess. But I guess that would be a backwards incompat.

So this is probably fine for now, I can't see it harming anything (I don't know why you'd ever want to close $stdout, indeed).

I think this comes about because this stuff was written assuming command-line use where there's only one writer, and when it's done the process is about to exit anyway. I assume you are using things in a more programmatic way (which is fine), for it to matter that $stdout gets closed.

(I think overriding close like you are doing is just fine, btw-- are there any cases where you do want the stream to be closed?)

[jrochkind]

Btw, I also think it would be totally legit if you didn't sub-class LineWriter at all. I'm not sure what utility the inherited functionality is really providing, especially because you are overriding serialize too, you aren't actually inheriting that much at all, it seems fine to copy-paste the very minimal logic you do want from the LineWriter. I'm not sure how useful inheriting from the LineWriter is in general.

[jrochkind]

I'm going to make my own branch off this, so I can do a bit more work on it (and rebase on master so it can pass). I'll make sure your commits stay in there with your name attached! Thanks for the contribution!

[jrochkind]

Took it a bit further with an additional explicit option (and better docs) in #211. (Also rebased so tests would pass)

I notice there are no tests around this func, and I don't particularly wanna add em either. But @jcoyne or @justinlittman , do either of you want to test #211 and make sure it meets your needs?