daa84 / neovim-lib

Rust library for Neovim clients
GNU Lesser General Public License v3.0
192 stars 26 forks source link

Debug logging #12

Closed KillTheMule closed 6 years ago

KillTheMule commented 6 years ago

This is something of a "semantical" issue, but I wanted to bring it up anyways. This line is very much spamming my logs when I use the debug log level. That's because it also prints out the payload of the messages, which in my case is kind of big. Moreover, it puts all of it on one line, which wreaks havok with basically every tool that I use to look at the logs (even less is ... less than amused :)).

That leaves me in a bit of a conundrum: I want debug printout often (I make a lot of mistakes), but this is too much. Switching the level to info leaves me without messages. From my personal understanding, payload would belong into the trace level debug. Of course, just switching this call to trace! instead of debug! really just moves my problem...

With a bit of control flow though, we could output a useful debug message, and leave the full print to trace only. Would you consider that? I'd make a PR, but wanted to discuss this first. I'd tend to asking in the forums how other people handle this problem. I think there should be something like a log_exact makro that does only log if a certain level is enabled, but not any others.

Thanks for your consideration!

KillTheMule commented 6 years ago

I've had some discussion with the author of msgpack-rust, and I think this would best be solved on the neovim-lib level. There are 2 ways I could imagine:

My favorite is the config flag, because it's the least surprising and interrupting change for everybody else. What do you think?

daa84 commented 6 years ago

If you need just disable this output you can use env_logger, that allow to disable this message by setting log level for module RUST_LOG=module::name=info In case you want some condensed info i prefer something like:

if log_enabled!(Debug) {
  debug!("{}");
}
else if log_enabled!(Trace) {
  trace!("{:?}");
}
KillTheMule commented 6 years ago

I did not know env_logger could do that, nifty! That would certainly solve my use case, so if you don't see my idea as an improvement, feel free to simply close the issue. Thanks!

Now, you're idea is almost what I had in mind... it's just that both {} and {:?} print the full payload for a value. We'd need something like

if log_enabled!(Debug) { 
    debug!("{}", short_debug(msg)); 
} else if log_enabled!(Trace) { 
    trace!("{:?}", msg ); 
}

and then

fn short_debug(v: &Value) -> String {
    match *v {
        Value::Nil => "nil".to_owned() ,
        Value::Boolean(val) => format!("{}", val),
        // skip
        Value::Array(ref vec) => format!("Array (length {})", vec.len())
       // continue like this
}

This could of course be behind a trait or whatnot :)

daa84 commented 6 years ago

Ok, then i close this issue as i just don't have any problems here. If you still need this functionality just create pull request 😄