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

Support cargo term variables. #921

Merged
merged 1 commit into from
Jul 8, 2022
Merged

Conversation

Alexhuszagh
Copy link
Contributor

@Alexhuszagh Alexhuszagh commented Jul 7, 2022

Use the CARGO_TERM_VERBOSE, CARGO_TERM_QUIET, and CARGO_TERM_COLOR environment variables for cross terminal output. This enables us to detect term settings via environment variables, which are overridden by command-line flags, which is our own behavior.

Cargo has CLI flags override environment variables, and verbose and quiet cannot be set for the same level. So if CARGO_TERM_VERBOSE=true and --quiet are used, then we use the quiet setting, but if --verbose and --quiet are used, then we print a fatal error message.

Closes #919.

@Alexhuszagh Alexhuszagh requested a review from a team as a code owner July 7, 2022 23:30
Copy link
Member

@Emilgardis Emilgardis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

the clap Option<String> should be converted to ColorChoice though, with default auto

@Alexhuszagh
Copy link
Contributor Author

Should I change everything to an enumerated values? I had it a list of options with a default earlier, but removed it because of envvars, but I can restore it.

https://github.com/clap-rs/clap/blob/v3.1.18/examples/tutorial_derive/README.md#enumerated-values

@Emilgardis
Copy link
Member

Emilgardis commented Jul 8, 2022

@Alexhuszagh
Copy link
Contributor Author

Was thinking this one: https://docs.rs/clap/3.2.8/clap/builder/struct.EnumValueParser.html or https://docs.rs/clap/3.2.8/clap/trait.ValueEnum.html

That's the same I believe, just using the derive API.

@Alexhuszagh
Copy link
Contributor Author

Alexhuszagh commented Jul 8, 2022

It creates really bad help messages when we change those, so I've just made sure the error messages for invalid colors are nicer.

Using Clap EnumParser

Screenshot_20220707_195514

Using Custom Validation

Screenshot_20220707_195611

I even tried the following, but it doesn't do enough:

    /// Coloring: auto, always, never
    #[clap(
        long,
        arg_enum,
        env = "CARGO_TERM_COLOR",
        hide_possible_values = true,
        hide_env_values = true,
        hide_default_value = true,
    )]
    pub color: ColorChoice,

I've made sure the output is similar to clap however, identical even on small terms.

Generic Clap

Screenshot_20220707_195535

Cross Error

Screenshot_20220707_204945

Use the `CARGO_TERM_VERBOSE`, `CARGO_TERM_QUIET`, and `CARGO_TERM_COLOR` environment variables for cross terminal output. This enables us to detect term settings via environment variables, which are overridden by command-line flags, which is our own behavior.

Cargo has CLI flags override environment variables, and verbose and quiet cannot be set for the same level. So if `CARGO_TERM_VERBOSE=true` and `--quiet` are used, then we use the quiet setting, but if `--verbose` and `--quiet` are used, then we print a fatal error message.
Copy link
Member

@Emilgardis Emilgardis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bors r+

@bors
Copy link
Contributor

bors bot commented Jul 8, 2022

Build succeeded:

@bors bors bot merged commit f6e67ce into cross-rs:main Jul 8, 2022
@Emilgardis Emilgardis mentioned this pull request Nov 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support CARGO_TERM_VERBOSE, etc.
2 participants