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

feat(mangen): clap/env is itself a feature of mangen #3729

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

AndreasBackx
Copy link
Contributor

This adds env as a feature to clap_mangen and enables it by default to keep the default behaviour. I wish there was a way to make it automatically enable the feature if it detects the used library/binary uses clap/env, though I'm unsure of how to exactly do that. I found this StackOverflow post, though it would be somewhat non-trivial and I would appreciate some input.

Fixes #3355.

Comment on lines +217 to 218
#[cfg(feature = "env")]
fn option_environment(opt: &clap::Arg) -> Option<Vec<Inline>> {
Copy link
Member

@epage epage May 15, 2022

Choose a reason for hiding this comment

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

I had missed this when fixing some of the other feature flags. In my opinion, stable but optional features should have no-op reflection functions when disabled, so instead we'd offer a version of get_env that returns None and is_hide_env_set returns true when env is not set. That'd make it so clap_mangen needs to do nothing extra.

I'm also tempted to just enable it. It doesn't really buy us much in terms of compile times or binary size and as we modularize, it'll shifting to being optional but without feature flags which aren't ideal to work with.

@thalesfragoso
Copy link

I wish there was a way to make it automatically enable the feature if it detects the used library/binary uses clap/env

It's (kinda) possible with a build script, see cargo:rustc-cfg

It doesn't make it possible to enable a cargo feature, but you can still achieve conditional compilation with that. Not sure if adding a build script is worth though.

@epage
Copy link
Member

epage commented Jun 14, 2022

Build scripts bring in their own build-time performance issues so I lean towards keeping things simple until the next MSRV bump where we get the new feature features.

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 this pull request may close these issues.

clap_man forces the env feature on
3 participants