Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Direct access to the i8 value of verbosity #53

Open
khultman opened this issue Jan 20, 2023 · 11 comments · May be fixed by #52
Open

Direct access to the i8 value of verbosity #53

khultman opened this issue Jan 20, 2023 · 11 comments · May be fixed by #52

Comments

@khultman
Copy link

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
Copy link

Zykino commented Jan 20, 2023

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
Copy link
Member

epage commented Jan 21, 2023

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
Copy link

Zykino commented Jan 21, 2023

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
Copy link
Contributor

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

@khultman
Copy link
Author

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
Copy link
Member

epage commented Jan 23, 2023

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.

@epage epage linked a pull request Jan 23, 2023 that will close this issue
@matthiasbeyer
Copy link
Contributor

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
Copy link

joshka commented Jan 8, 2024

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
Copy link
Member

epage commented Jan 8, 2024

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

@hadim
Copy link

hadim commented Jan 30, 2024

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
Copy link
Member

epage commented Jan 30, 2024

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants