Add Target::Pipe

[TilCreator]

Now logs can be written to custom pipes and not just to stdout or stderr.

This is my take on #178.

I would be very happy to hear some feedback, since I'm quite unsure about a few changes:

• The example seams overly complicated, mostly because I didn't want an example to write to a file. Maybe there is a better candidate than std::sync::mpsc used as a pipe for that?
• Builder::target_pipe is my idea for not breaking too much, but maybe there is a solution to implement that into Builder::target directly?
• In Buffer it is assumed that if target_pipe is Some, test_target is always None. Is there a better way to enforce that?

[TilCreator]

I can't reproduce the failed check locally and I also didn't change anything on the code that is failing. Plz help?

[jplatte]

It's cargo clippy, not cargo check, that's failing (and with a nightly version of clippy). I've pushed a commit to master that fixes that lint plus one more, rebasing should get rid of the CI failure now.

[TilCreator]

Ah, I wasn't using the nightly version of clippy. I rebased master and the checks succeed now. thx

[jplatte]

You did the rebase the wrong way around (you should rebase your branch on top of master, not the other way around).

Builder::target_pipe is my idea for not breaking too much, but maybe there is a solution to implement that into Builder::target directly?

What do you mean by "not breaking too much"? The derived traits for Target? I think I'd rather see those removed than there being a separate target_pipe builder method / field. I wonder why e.g. Hash is implemented for that in the first place.

[jplatte]

By the way, thank you for working on this!

[TilCreator]

Ups, I knew there was something wrong after at least the 5th git command, is it still worth trying to fix? ^^

[jplatte]

It should be as easy as

// if you haven't set up the remote yet, though I guess you have
git remote add upstream https://github.com/env-logger-rs/env_logger
git fetch upstream
// rebase your branch on top of upstream's master branch
git rebase upstream/master

even now. (git should detect the duplicate commit on your branch and get rid of it)

Followed by git push --force (--force to tell git you are sure you want to rewrite the history of the remote branch).

[TilCreator]

I have tried myself on keeping the pipe in the Target::Pipe, but that is just such a mess, so many Arc<Mutex<...>>.... Target also gets used a lot after build() to check if stdout or stdout is used, so removing or "circumenting" the derived traits of Target is hard. (The Builder is also not consumed on build().)

I will try something that takes the value out of Target::Pipe on target() or build() tomorrow. Maybe I can even come up something better than enum Target {..., Pipe(Option<Box<dyn io::Write + Send + 'static>>)} after getting some sleep ^^

[TilCreator]

I'm quite happy with this version as it breaks very little if anything at all and it feels like a simple extension of the old target method. Now I just need a better example as the current one is just confusing and not super practical.

[mainrs]

I can take a closer look an Tuesday, up until then I don't have enough time for a full review :)

[TilCreator]

thx @SirWindfield (also for fixing my typos ^^)

[jplatte]

I can have a second look, sure. Probably going to take a while for me as well.

[jose-acevedoflores]

I've been playing around with this pr since I need support for logging to a file and this works great for my use case!

I did have one question about the possibility of having multiple targets like stdout and a file. Is this something out of scope for the goals of this library or is it something that could be entertained ?

[TilCreator]

(I'm just a contributor, not a maintainer) I think this is possibly better implemented by the user, it sounds like a quite specific use case. (Maybe this could be added to the example (and I'm still not entirely happy with the example)?)

[jplatte]

I'm still planning to have a look at this again, hopefully this or next week.

[jplatte]

Merged in #204.