Target::Pipe doesn't work

[oxalica]

In #204, we introduce Target::Pipe to redirect log messages into custom writer. And it's included in 0.8.4 now.
But when I try the example in that PR, it doesn't print the message from pipe at all, and not even call WriteAdapter::write. The log::error message just go straight to stderr.
How to make it catch all log message into my custom writer object?

[TilCreator]

I also just noticed that. I read the code (of #204), but never actually tested it... I kind of just assumed that it would work, because it didn't change much of my code (#195).
I'm currently looking into it (I also already found at least one error, but it's just in the documentation)

[jplatte]

Thanks for looking into it, and sorry for breaking things.

[TilCreator]

I have found the error in fmt::writer::termcolor::extern_impl::BufferWriter
The change removed the target_pipe which is good, less convoluted.
In fmt::writer::termcolor::extern_impl::BufferWriter::print the if for the pipe was removed and integrated into the match a few lines down, but that only works if there is a test_target, because BufferWriter::inner (a termcolor::BufferWriter) is unable to write to the pipe.

[TilCreator]

A easy solution would be to change BufferWriter::pipe to always set test_target, but that feels quite hacky... (it also removes color, so not really a solution)

[jplatte]

I think we should no longer unconditionally hold that BufferWriter, it doesn't make sense for the pipe target.

it also removes color, so not really a solution

Removes color in general, or when using a Pipe writer? For the latter, I would expect colorization to be turned off. You could print the escape sequences, but that's unix-specific and would lead to very ugly output if it's not expected on the "other side" of the pipe.

[TilCreator]

I explicitly enabled colors, else the default for pipes would be to disable color.

[TilCreator]

I will try the idea without the BufferWriter

[TilCreator]

I think we should no longer unconditionally hold that BufferWriter, it doesn't make sense for the pipe target.

Something like this?
TilCreator@fbeebcf

[tubzby]

Same problem here.

[Polochon-street]

Can report having the same problem - that would be a really awesome addition if that worked though 😄

[TilCreator]

Ups, it has been a few days...
Sry, I opened #211 which fixes this (but it's not perfect).

[mainrs]

I do also think that colors disabled makes more sense for pipes (by default). We could think about adding some kind of way to enable them back again.

[TilCreator]

It is already disabled by default for pipes, the example just enables them to show that it's possible.

[harlanc]

Is this problem fixed now? Both 0.8.4 and 0.9 versions have problems..

[jplatte]

Seems like #211 would fix this.

[harlanc]

when will this patch be merged and released? @TilCreator

[TilCreator]

No idea, but I hope soon

[tubzby]

Any news on this?
I think this is a critical issue because I can't imagine a process without a log file in the production environment.

[jplatte]

#217 contains an incomplete fix for this. I'm not sure it can really be completed without breaking changes though.

[TilCreator]

I'm quite busy right now, so I will sadly not be of much help, sry (also my non critical applications currently work fine with #211)

[yihuaf]

@jplatte Any update? Can we re-open #211 if it fix the issue? Being able to log to file is a critical feature.

[jplatte]

I think you'd be better off just using a different logger like simplelog if you need file output. If you really want it work in env_logger, you could try working on #217.

[hasezoey]

just encountered this issue as well, it is confusing that this feature (Target::Pipe) does not work (silently) without having documentation mentioning that it does not work

also is there any progress on #217 to be ready for merge?

[jplatte]

Yeah, I suppose some docs plus a deprecated attribute (because that is the only way you can make usage of it warn, AFAIK) would be a good interim solution. There has been no progress on #217, help wanted.

[walkie]

@niklasha:

this was how I fixed it locally: https://github.com/niklasha/env_logger/tree/issue-208-pipe-fix
This is basically just making the uncolored case be used always with Pipe, be it in testing context or not. I know there is already a rejected proposal, but i thought I'd at least share what I did, should anyone like it.

I've read through all of the various issues and pull requests, and there seems to be a consensus that disabling color for pipe targets (at least initially) is acceptable.

For example, this comment from @mainrs:

I do also think that colors disabled makes more sense for pipes (by default). We could think about adding some kind of way to enable them back again.

Is there any reason that @niklasha's patch is not a valid (perhaps interim) solution to this issue?

If not, I'm happy to create a small PR updating the documentation to clarify that this feature is only supported in test contexts. It's definitely confusing in its current state, as the last few commenters here can attest. :-)

[jplatte]

That approach sounds reasonable, I'd be happy about a PR.

[SolraBizna]

My PR over at the termcolor repo will make a simple path for enabling color for pipe targets down the road. But it doesn't do us any good until it's on crates.io.

[TilCreator]

@walkie I thinks that color output should be disabled be default, but I think it still is an important feature, for example I use the color output to combine the indicatif progress bar and pretty env logger in https://gitlab.com/TilCreator/rustycomicscraper/-/blob/master/src/main.rs

[jplatte]

I've published version 0.9.1 that deprecates Target::Pipe because nobody is working on fixing it.

[niklasha]

I've published version 0.9.1 that deprecates Target::Pipe because nobody is working on fixing it.

I offer to fix it, if I only knew what the requirements on a solution are. I have already devised a solution that "works for me" above: #208 (comment) but I understand you have issues with it?

[jplatte]

I already told you that I would be happy to review a PR with your patch, but I don't think you created one?

[niklasha]

Eh, you did? I must have misunderstood then. I will prepare a PR.

[epage]

@jplatte did 84b701d resolve this Issue?

[jplatte]

It should have, yes.