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(ArgEnum)] fails silently on enums with non-unit variants #3543

Closed
epage opened this issue Mar 7, 2022 Discussed in #3533 · 1 comment
Closed

#[derive(ArgEnum)] fails silently on enums with non-unit variants #3543

epage opened this issue Mar 7, 2022 Discussed in #3533 · 1 comment
Labels
A-derive Area: #[derive]` macro API C-bug Category: Updating dependencies E-easy Call for participation: Experience needed to fix: Easy / not much
Milestone

Comments

@epage
Copy link
Member

epage commented Mar 7, 2022

Discussed in #3533

Originally posted by Shir0kamii March 4, 2022
Hi!

I stumbled on a case where the derive implementation of ArgEnum does not produce its impl block without warning. It happens if the enum contains a non-unit variant.

If you try to compile code such as

#[derive(ArgEnum)]
pub enum Test {
    Foo,
    Bar,
    Baz(u16),
}

The derive doesn't produce any code, and errors are later emitted, reporting that Test does not implement ArgEnum. I would consider that as a bug, because it is confusing and unfriendly. It would be better if an error was emitted from the macro.

I think I found the culprit and would be willing to contribute a fix.

However, while browsing similar issues, I found this discussion where one of you proposed then implemented a #[clap(skip)] attribute for skipping variants in ArgEnum implementation. I think that an enum with a skipped non-unit variant should not trigger any error, and instead produce a valid implementation. If you approve of it, I would like to help on that as well.

What do you think?

@epage epage added C-bug Category: Updating dependencies E-easy Call for participation: Experience needed to fix: Easy / not much A-derive Area: #[derive]` macro API labels Mar 7, 2022
@epage epage added this to the 3.2 milestone Mar 7, 2022
@Shir0kamii
Copy link
Contributor

This can be closed since #3591 has been merged.

@epage epage closed this as completed Mar 31, 2022
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-bug Category: Updating dependencies E-easy Call for participation: Experience needed to fix: Easy / not much
Projects
None yet
Development

No branches or pull requests

2 participants