clap-rs / clap-verbosity-flag

Easily add a --verbose flag to CLIs using Clap
https://docs.rs/clap-verbosity-flag
Apache License 2.0
176 stars 24 forks source link

Direct access to the i8 value of verbosity #53

Open khultman opened 1 year ago

khultman commented 1 year ago

Unsure why the default api to make this value private, but making the value public allows more flexible use of this library. PR submitted for this change.

Zykino commented 1 year ago

I see that supporting tracing is an aim for you. Did you see the example? https://github.com/clap-rs/clap-verbosity-flag/blob/master/examples/example_tracing.rs

I kind of want this feature but only because I want to compare on the log::LevelFilter enum:

if level < warn {
    // suppress a bit more
}
if level < error {
    // suppress a bit more
}
epage commented 1 year ago

I kind of want this feature but only because I want to compare on the log::LevelFilter enum:

Is Verbosity::log_level_filter not sufficient for that?

Zykino commented 1 year ago

Well, this is working so I don’t know what I did. I think I got confused between LogLevel, LevelFilter and Option<Level>.

    if opt.verbose.log_level_filter() <= LevelFilter::Warn {
        eprintln!("Hide command's stdout");
    }

    if opt.verbose.log_level_filter() <= LevelFilter::Error {
        eprintln!("Hide command's stderr");
    }

Sorry for the noise, and thank you for the pointer.

matthiasbeyer commented 1 year ago

Is the original issue (from @khultman) resolved then?

khultman commented 1 year ago

I see that supporting tracing is an aim for you. Did you see the example? https://github.com/clap-rs/clap-verbosity-flag/blob/master/examples/example_tracing.rs

I kind of want this feature but only because I want to compare on the log::LevelFilter enum:

if level < warn {
    // suppress a bit more
}
if level < error {
    // suppress a bit more
}

Thanks, I did see this. I'm giving using tracing as an example, but my thought here is that direct access to the i8 value gives the end-user a lot more control. For instance, relying on log::LevelFilter either directly or indirectly limits the user to simply the Off -> Trace values it provides. Whereas having direct access to the int can yield a value between -128 & 127, allowing someone to implement their own enum which could have a significantly wider range than ~6 values of the log:;LevelFilter enum.

epage commented 1 year ago

The current definition of verbosity is:

    fn verbosity(&self) -> i8 {
        level_value(L::default()) - (self.quiet as i8) + (self.verbose as i8)
    }

I feel that the i8 returned by this function is an implementation detail and that the way to get semantic meaning from it is to use an enum.

One way around this problem is if we have a function that returns the net result of the verbosity flags. The difference is in the name / documentation and in not including default.

I am somewhat hesitant to include such a function as the use case is a bit off the beaten path, relying on custom log levels decoupled from log and tracing.

matthiasbeyer commented 1 year ago

I feel that the i8 returned by this function is an implementation detail and that the way to get semantic meaning from it is to use an enum.

I agree.

I am somewhat hesitant to include such a function

After re-visiting this I agree and would opt for not include such a function.

joshka commented 9 months ago

A use case for this is enabling cli apps to use the verbosity flags to have control over how e.g. color_eyre displays backtrace / spantraces.

This is pretty similar to the replacing the RUST_LOG environment variables with flags, and in effect replaces the RUST_BACKTRACE / RUST_LIB_BACKTRACE vars. (Right now there's no programmatic interface other than setting the environment variables of the app for those).

I suppose that could be implemented as:

match cli.verbose.log_level() {
    Level::Warn => env::set_var("RUST_BACKTRACE", "1"),
    Level::Error => env::set_var("RUST_BACKTRACE", "full"),
    _ => {}
}
epage commented 9 months ago

imo log_level() offers an easier to read solution than exposing the integer.

hadim commented 9 months ago

Having direct access to verbose and quiet would allow configuring the app with env variables: MY_APP_VERBOSE=1 or MY_APP_QUIET=1.

Is that something you think it's possible to achieve without having those variables as public?

epage commented 9 months ago

At least in cargo's case, CARGO_VERBOSE and CARGO_QUIET are bools and shadowed by the CLI. See https://github.com/rust-lang/cargo/blob/master/src/cargo/util/config/mod.rs#L1037-L1049

Are you wanting your env variables to be bools or to be numeric levels? And are you trying to add them to the CLI, rather than the CLI shadowing?