Allow IsTerminal passthrough for SyncLogger

[ereOn]

The current implementation of IsTerminal does not properly work when stdout or stderr are used through a SyncLogger.

This PR adresses that by defining and implementing a new interface for the SyncWriter that exposes the encapsulated io.Writer. This new interface is then used inside term.go which now performs the IsTerminal check on the deepest io.Writer instead.

This ensures transivity of the IsTerminal attribute.

There is no API break, tests are passing and a new test was added to cover the new interface.

[ereOn]

Note: This method is a pass-through for all io.Writer instances that do not implement the parentWriter interface, thus not breaking the existing behavior.

[ChrisHines]

Sorry it has taken me a few days to get back to this.

The solution here seems straightforward at first glance. However ... it points out the more general issue that passing any decorated io.Writer to IsTerminal causes it to return false. For example bufio.NewWriter(os.Stdout) also tricks IsTerminal. So even if we merge this PR there is still a problem with IsTerminal(log.NewSyncWriter(bufio.NewWriter(os.Stdout))). I'm not sure how I feel about this being only a partial fix.

Aside from the above I'm not crazy about the method name ChildWriter. I like WrappedWriter slightly better, but I'm not sold on that either. Suggestions anyone?

[ereOn]

@ChrisHines No problem about the delay !

I agree that the proposed solution does not account for all cases but I would still distinguish external types and types provided by go-kit.

For the first ones, it's harder (impossible ?) to make any guarantee on their exposed interface. For instance, in your example there is just no way to get the underlying io.Writer even if we detect the correct instance type: bufio.Writer just doesn't offer that possibility.

For the go-kit provided types, I'd say it's another deal because we do control exactly how they behave and what they expose.

I agree that added the support for any Writer is a nice ultimate goal, but this scoped change still has some consistency in its own I believe.

As for the name, I'm not too fond of ChildWriter either to be honest. What do you think of:

• WriterWrapper - It is indeed, after all, an interface for all Writer instances that wrap another one.
• CompositeWrapper
• NestedWrapper

[ereOn]

@ChrisHines ping ?

[ChrisHines]

Ping received, server overloaded at the moment. ;)

I haven't forgotten about this, just too busy this week.

[ereOn]

@ChrisHines No worries. I just wanted to make sure my PR had not fallen into a crack. :D

I understand being busy, trust me ;)

[ereOn]

Closing in favor or #501 which addresses the problem in a less intrusive manner.