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

derive error for missing = value for attributes is confusing #3726

Open
2 tasks done
ravenexp opened this issue May 13, 2022 · 5 comments
Open
2 tasks done

derive error for missing = value for attributes is confusing #3726

ravenexp opened this issue May 13, 2022 · 5 comments
Labels
A-derive Area: #[derive]` macro API C-enhancement Category: Raise on the bar on expectations E-easy Call for participation: Experience needed to fix: Easy / not much

Comments

@ravenexp
Copy link

ravenexp commented May 13, 2022

Please complete the following tasks

Clap Version

3.1.18

Describe your use case

I'm trying to add an environment variable fallback for the --password option. I do not want the variable value to be accidentally revealed when --help is used.

I tried to write

#[clap(short, long, env = "PROGRAM_PASSWORD", hide_env_values)]
password: Option<String>,

but the error made me think that the attribute doesn't exist when it does:

error: unexpected attribute: hide_env_values

(Note: #3620 is for bool shorthand support)

Describe the solution you'd like

A clearer error message

Alternatives, if applicable

Additional Context

No response

@ravenexp ravenexp added the C-enhancement Category: Raise on the bar on expectations label May 13, 2022
ravenexp added a commit to ravenexp/maturin that referenced this issue May 13, 2022
Include it for feature parity with twine's `TWINE_USERNAME`.

Refactor `MATURIN_PASSWORD` variable handling:
note that `clap` attributes can not be used until
<clap-rs/clap#3726>
is resolved.
@epage
Copy link
Member

epage commented May 13, 2022

Boolean builder functions do not have an implied = true but you have to set it manually. The following should work

#[clap(short, long, env = "PROGRAM_PASSWORD", hide_env_values = true)]
password: Option<String>,

See #3620

@ravenexp
Copy link
Author

Thanks! I should've tried that before opening an issue 😮‍💨

I was getting

error: unexpected attribute: hide_env_values

when compiling, and it is not mentioned anywhere in the documentation, so I just assumed that this attribute was not implemented.

@epage
Copy link
Member

epage commented May 13, 2022

Thats an artifact of how we parse the attributes. It isn't in our hard coded list of attributes that work without a = or (). It would be nice to improve that message but unsure what to say.

@epage epage changed the title hide_env_values() can't be used via the Derive API derive error for missing = value for attributes is confusing May 13, 2022
@epage epage added E-easy Call for participation: Experience needed to fix: Easy / not much A-derive Area: #[derive]` macro API labels May 13, 2022
@danielparks
Copy link
Contributor

Would you be opposed to adding the boolean attributes to the list that are accepted without a value? I imagine they all could be treated as true, but I would go through them and check. (To be clear, I’m volunteering.)

Otherwise, I suppose the error could be something like:

error: unexpected attribute without value: hide_env_values

I‘m not sure how much clearer that is, though.

@epage
Copy link
Member

epage commented Aug 1, 2022

@danielparks see #3620

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-derive Area: #[derive]` macro API C-enhancement Category: Raise on the bar on expectations E-easy Call for participation: Experience needed to fix: Easy / not much
Projects
None yet
Development

No branches or pull requests

3 participants