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 for using ArgGroup as Enum with derive #2621

Open
2 tasks done
ModProg opened this issue Jul 25, 2021 · 20 comments
Open
2 tasks done

Support for using ArgGroup as Enum with derive #2621

ModProg opened this issue Jul 25, 2021 · 20 comments
Labels
A-derive Area: #[derive]` macro API C-enhancement Category: Raise on the bar on expectations E-hard Call for participation: Experience needed to fix: Hard / a lot E-help-wanted Call for participation: Help is requested to fix this issue. 💸 $20
Milestone

Comments

@ModProg
Copy link
Contributor

ModProg commented Jul 25, 2021

Maintainer's notes

Blocked on #3165 as that will lay some of the initial groundwork and #4211 to provide a fallback mechanism

Related


Please complete the following tasks

  • I have searched the discussions
  • I have searched the existing issues

Clap Version

3.0.0-beta.2

Describe your use case

I have multiple filters of witch only one can be set (--running, --exited, --restarting,...).

Describe the solution you'd like

I would like something with this usage:

#[derive(Clap)]
pub struct App{
    #[clap(flatten)]
    pub state_filter: StateFilter,
}

#[derive(Args)]
pub enum StateFilter {
    /// Only running servers are returned
    #[clap(long)]
    Running,
    /// Only exited servers are returned
    #[clap(long)]
    Exited,
    /// Only restarting servers are returned
    #[clap(long)]
    Restarting,
    #[clap(long)]
    Custom(String),
}

Resulting in:

OPTIONS:
    --exited
           Only exited servers are returned
    --restarting
           Only restarting servers are returned
    --running
           Only running servers are returned
    --cusxtom <CUSTOM>

Alternatives, if applicable

I could use an arggroup for this currently, but I would still need to check each boolean separately and could not just easily (and readable) match it.

Additional Context

No response

@epage
Copy link
Member

epage commented Jul 26, 2021

When I was implementing the Args trait, I was thinking of this case. I was assuming we could do

#[derive(Clap)]
pub struct App{
    #[clap(flatten)]
    pub state_filter: Option<StateFilter>,
}

#[derive(Args)]
pub enum StateFilter {
    /// Only running servers are returned
    #[clap(long)]
    Running,
    /// Only exited servers are returned
    #[clap(long)]
    Exited,
    /// Only restarting servers are returned
    #[clap(long)]
    Restarting,
}

(with an attribute on the enum to override the group name)

Though I hadn't considered

  • Option in the caller, I like it!
  • Enum variants as flags, which I also like!

@pksunkara pksunkara added the A-derive Area: #[derive]` macro API label Jul 27, 2021
@Skirmisher
Copy link

This would be lovely for my use case (an archive utility, where the operation modes are exclusive but otherwise share the same options, similar to tar). I find clap's subcommand system to be a bit heavy-handed for what I need, but I'm lacking another way to concisely describe and match "exactly one of these options" like is described here.

@epage
Copy link
Member

epage commented Oct 31, 2021

You can workaround this by manually defining the arg group and adding the arguments to it. While its using structopt instead of clap-derive, the principle and calls are pretty much the same in this code of mine.

@0x5c
Copy link

0x5c commented Nov 2, 2021

The problem with argument groups and the struct equivalent is that you can't ask clap what option was specified, all you can do is check each individual option of the group for presence in the arguments. We're back to to arguments not really being parsed into something useful.

With subcommands, clab directly talls you what subcommand was found in the arguments, as either an Enum or as the string ID of the subcommand. In both cases you can actually match on something instead of having an if-else chain or iteration simply to know which one it was

@MTCoster
Copy link

MTCoster commented Nov 2, 2021

I have a perfect use case for this right now: my app can accept a config file, or (perhaps if the config is very short) a string directly in the command line. For instance:

$ app --config 'foo=bar,baz=bat'
$ app --config-file ../longer-config.txt

The config string is not designed to be parsed by the app; it's passed directly through to the output. I have no desire to parse it.

Ideally, I'd write something like this:

#[derive(Parser)]
struct RootArgs {
    #[clap(flatten)]
    config: ConfigStringOrFile,
}

#[derive(Args)]
enum ConfigStringOrFile {
    #[clap(short = 'c', long = "config")]
    String(String),
    #[clap(short = 'C', long = "config-file")]
    File(PathBuf),
}

fn main() {
    let args = RootArgs::parse();

    let config = match args.config {
        ConfigStringOrFile::String(s) => s,
        ConfigStringOrFile::File(path) => { 
            // Read config from file...
        },
    };

    // ...
}

@epage
Copy link
Member

epage commented Nov 2, 2021

The problem with argument groups and the struct equivalent is that you can't ask clap what option was specified, all you can do is check each individual option of the group for presence in the arguments. We're back to to arguments not really being parsed into something useful.

Yes, its only a workaround to help keep people from being blocked and is not ideal. We are trying to focus on polishing up clap v3 before before working on new features. Once v3 is out (and maybe a little after as we do high priority work we postponed), we can look into this or mentoring someone who can look into this,

@jcdyer
Copy link

jcdyer commented Mar 3, 2022

I'd like to offer another example that I would like to support with this feature. It's slightly more complex than the existing one, so worth calling out explicitly I think. I have a tls configuration that requires three pem files, representing the root certificate, the application cert, and the application's private key. By convention, they all live in the same directory, as root.pem, cert.pem, and key.pem. I would like users to be able to pass a directory or a path to each file, but not both, meaning I would like something like:

#[derive(clap::Args)]
enum Pems {
    Dir { 
        #[clap(long)]
        pem_dir: PathBuf,
    }
    Files {
        #[clap(long)]
        key: PathBuf,
        #[clap(long)]
        cert: PathBuf,
        #[clap(long)]
        root: PathBuf,
    }
}

or

#[derive(clap::Args)]
enum Pems {
    Dir(PemDir),
    Files(PemFiles),
}
#[derive(clap::Args)]
struct PemDir {
    #[clap(long)]
    pem_dir: PathBuf,
}
#[derive(clap::Args)]
struct PemFiles {
    #[clap(long)]
    key: PathBuf,
    #[clap(long)]
    cert: PathBuf,
    #[clap(long)]
    root: PathBuf,
}

@blaenk
Copy link

blaenk commented Mar 22, 2022

For what it's worth, I thought I understood the clap "language" of modeling out commands and flags and naturally reached for this and it took me a while to realize (and find this issue) that it wasn't possible.

I'm making do with the ArgGroup workaround @epage helpfully provided, so all is fine, just a shame we can't leverage what would have been a pretty elegant solution (for now?):

#[derive(Parser, Debug)]
#[clap(group = clap::ArgGroup::new("image-tag").multiple(false))]
pub struct ImageTag {
    #[clap(long, group = "image-tag")]
    existing: bool,

    #[clap(long, group = "image-tag")]
    new: bool,

    #[clap(long, group = "image-tag")]
    tag: Option<String>,
}

versus something like:

#[derive(Parser, Debug)]
pub enum ImageTag {
    Existing,
    New,
    Tag(String)
}

Just wanted to register my interest. Thanks for providing a workaround!

@Skirmisher
Copy link

Are there any plans to implement this yet, now that clap v3 has been out for a while? I did take a crack at it after I first commented here (about 9 months ago now...); I tried a couple different approaches, angling to adapt existing ArgGroup code to accept enum variants, but I kept running into flaws in my type model that made the whole thing fall apart. It didn't help that it was my first experience writing proc macro code, either, though I got impressively far considering. All that being said, if someone can come up with a sound design, I might be able to help with implementation.

@epage
Copy link
Member

epage commented Jun 22, 2022

Are there any plans to implement this yet, now that clap v3 has been out for a while? I

This is not a priority of mine. My focus is on binary size, compile times, and extensibility. If someone else wants to work on a design proposal and then do an implementation, they are free to do so.

@StripedMonkey
Copy link

I have encountered a situation where supporting this would be extremely useful. Specifically I am wanting to provide different authentication methods for a service. The following is a rough sketch of how I was imagining such a feature:

#[derive(Debug, Args)]
struct UserPassAuth {
    #[clap(short, long)]
    username: String,
    #[clap(short, long)]
    password: String,
}

#[derive(Debug, Args)]
struct TokenAuth {
    #[clap(long)]
    token: String,
}

#[derive(Debug, Default, MutuallyExclusiveArgs)]
enum AuthenticationMethod {
    #[default]
    Anonymous,
    UserPass(UserPassAuth),
    Token(TokenAuth),
}

This isn't necessarily a priority in that argGroups do exist, and they can be made mutually exclusive through that mechanism, but this does significantly clean up the interface that you have to deal with externally.
Without enums you have to check what flags have been set, even if argGroups exist. With enums in theory it's as simple as:

match authentication {
   AuthenticationMethod::Anonymous => anonymous_login(),
   AuthenticationMethod::UserPass(login_info) => login(login_info),
   // ...
}

I'm not nearly confident enough to take a stab at implementing it, but if there's guidance on how this might be correctly implemented I would be interested in having direction.

@epage epage added this to the 4.x milestone Sep 13, 2022
@epage
Copy link
Member

epage commented Sep 13, 2022

I'd like to get to this during 4.x series.

Some ground work I need to do in 4.0.0

  • Reserve the group attribute
  • Add an empty ArgGroup::new("TypeName") to each Args derive.
  • Add a fn group_id(&self) -> Option<&str> to AugmentArgs
    • Technically this can be done later.

@KSXGitHub
Copy link

Correct me if I'm wrong, the feature that is being requested here would allow parsing mutually exclusive sets of flags into different variants of an enum, right? I really want to remove all the conflicts_with and required_by in my code.

@epage
Copy link
Member

epage commented May 18, 2023

Yes, this would allow conflicts to be expressed in the type system

nothingmuch added a commit to carol-computer/carol that referenced this issue May 22, 2023

Verified

This commit was signed with the committer’s verified signature.
rbuckton Ron Buckton
Given build options (cargo workspace options, e.g. `-p my_crate`), the
`upload` command will build before uploading, and given build or upload
options `actvate` will build or upload before activating.

These primary options are mutually exclusive, but may require nested
ArgGroups in the future which are supposed to be supported, perhaps
aided by clap-rs/clap#2621 if resolved. The current constraints and
reuse of shared options is rather crude (see in particular
UploadOptsWrapper).

Introduces some technical debt with regards to server/client parity,
which should be unified under the carol_core crate.
nothingmuch added a commit to carol-computer/carol that referenced this issue May 26, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Given build options (cargo workspace options, e.g. `-p my_crate`), the
`upload` command will build before uploading, and given build or upload
options `actvate` will build or upload before activating.

These primary options are mutually exclusive, but may require nested
ArgGroups in the future which are supposed to be supported, perhaps
aided by clap-rs/clap#2621 if resolved. The current constraints and
reuse of shared options is rather crude (see in particular
UploadOptsWrapper).

Introduces some technical debt with regards to server/client parity,
which should be unified under the carol_core crate.
nothingmuch added a commit to carol-computer/carol that referenced this issue May 27, 2023
Given build options (cargo workspace options, e.g. `-p my_crate`), the
`upload` command will build before uploading, and given build or upload
options `create` will build or upload before creating a machine.

These primary options are mutually exclusive, but may require nested
ArgGroups in the future which are supposed to be supported, perhaps
aided by clap-rs/clap#2621 if resolved. The current constraints and
reuse of shared options is rather crude (see in particular
UploadOptsWrapper).
nothingmuch added a commit to carol-computer/carol that referenced this issue Jun 2, 2023
Given build options (cargo workspace options, e.g. `-p my_crate`), the
`upload` command will build before uploading, and given build or upload
options `create` will build or upload before creating a machine.

These primary options are mutually exclusive, but may require nested
ArgGroups in the future which are supposed to be supported, perhaps
aided by clap-rs/clap#2621 if resolved. The current constraints and
reuse of shared options is rather crude (see in particular
UploadOptsWrapper).
nothingmuch added a commit to carol-computer/carol that referenced this issue Jun 2, 2023
Given build options (cargo workspace options, e.g. `-p my_crate`), the
`upload` command will build before uploading, and given build or upload
options `create` will build or upload before creating a machine.

These primary options are mutually exclusive, but may require nested
ArgGroups in the future which are supposed to be supported, perhaps
aided by clap-rs/clap#2621 if resolved. The current constraints and
reuse of shared options is rather crude (see in particular
UploadOptsWrapper).
nothingmuch added a commit to carol-computer/carol that referenced this issue Jun 2, 2023
Given build options (cargo workspace options, e.g. `-p my_crate`), the
`upload` command will build before uploading, and given build or upload
options `create` will build or upload before creating a machine.

These primary options are mutually exclusive, but may require nested
ArgGroups in the future which are supposed to be supported, perhaps
aided by clap-rs/clap#2621 if resolved. The current constraints and
reuse of shared options is rather crude (see in particular
UploadOptsWrapper).
@stevenroose
Copy link

Any updates on this? Is someone working on this?

@epage
Copy link
Member

epage commented Aug 8, 2023

Not at this time

github-merge-queue bot pushed a commit to n0-computer/iroh that referenced this issue Nov 2, 2023
## Description

- modifies the `blob download` ux to make it clear, and properly enforce
having either a ticket or arguments for a download
- All arguments apply to both ticket and hash but with different
constrains. In the case of the ticket options can be used to
override/remove ticket parts
- makes sure failures still report the help text

## Notes & open questions

### Sample output: 

This would previously only show an error
```
> blob download
Download data to the running node's database and provide it

Usage: blob download <COMMAND>

Commands:
  ticket  Use a blob ticket to download the data
  hash    Supply the content and node-addressing information directly to perform a download
  help    Print this message or the help of the given subcommand(s)

Options:
  -h, --help  Print help (see more with '--help')
```

### Calling commands:
#### With a ticket:
```
> blob download ticket blobedjoqrkky753mdwphqrxbr2wjzkizv4hkb5s5ovav73c2zfuaja7aaibaiah6aaaahcfoaiaaaaaaaaaaaaaaaaaaaaaaaabyvlqcz2d7d7ouqasz36mapwfxqf2dx3zznb25nx32phl7kbgum23wtegaa
```
#### With args:
```
> blob download hash bafkr4ibzusj4ulmrhrbymsgklwlcencx63jwcurmoimjhk4mutqdsqhnqa --node 2luekswh7o3a5tz4enymovsoksgnpb2qpmxlvifp6ywwjnacihya --derp-region 1
```
Both 
NOTE: Adding this a a subcommand is a consequence of
clap-rs/clap#2621

## Change checklist

- [x] Self-review.
- [x] Documentation updates if relevant.
- [ ] Tests if relevant.
c3potheds added a commit to c3potheds/todo that referenced this issue May 19, 2024
When clap-rs/clap#2621 is resolved, we will be
able to derive a set of mutually exclusive boolean flags from an enum
in a struct that derives clap::Parser.
c3potheds added a commit to c3potheds/todo that referenced this issue May 19, 2024
When clap-rs/clap#2621 is resolved, we will be
able to derive a set of mutually exclusive boolean flags from an enum
in a struct that derives clap::Parser.
@ysndr
Copy link

ysndr commented Aug 25, 2024

I took a stab at it here: #5700

At the moment it only supports struct like enums so far, but I'd like it to be a bit smarter and support more of the use cases described in this thread eventually.

#[derive(Parser, Debug, PartialEq, Eq)]
struct Opt {
    #[command(flatten)]
    source: Source,
}

#[derive(clap::Args, Clone, Debug, PartialEq, Eq)]
enum Source {
    A {
        #[arg(short)]
        a: bool,
        #[arg(long)]
        aaa: bool,
    },
    B {
        #[arg(short)]
        b: bool,
    },
}
prog -b -a

error: the argument '-b' cannot be used with:
  -a
  --aaa

Usage: prog -b -a

rklaehn pushed a commit to n0-computer/iroh-blobs that referenced this issue Oct 22, 2024
## Description

- modifies the `blob download` ux to make it clear, and properly enforce
having either a ticket or arguments for a download
- All arguments apply to both ticket and hash but with different
constrains. In the case of the ticket options can be used to
override/remove ticket parts
- makes sure failures still report the help text

## Notes & open questions

### Sample output: 

This would previously only show an error
```
> blob download
Download data to the running node's database and provide it

Usage: blob download <COMMAND>

Commands:
  ticket  Use a blob ticket to download the data
  hash    Supply the content and node-addressing information directly to perform a download
  help    Print this message or the help of the given subcommand(s)

Options:
  -h, --help  Print help (see more with '--help')
```

### Calling commands:
#### With a ticket:
```
> blob download ticket blobedjoqrkky753mdwphqrxbr2wjzkizv4hkb5s5ovav73c2zfuaja7aaibaiah6aaaahcfoaiaaaaaaaaaaaaaaaaaaaaaaaabyvlqcz2d7d7ouqasz36mapwfxqf2dx3zznb25nx32phl7kbgum23wtegaa
```
#### With args:
```
> blob download hash bafkr4ibzusj4ulmrhrbymsgklwlcencx63jwcurmoimjhk4mutqdsqhnqa --node 2luekswh7o3a5tz4enymovsoksgnpb2qpmxlvifp6ywwjnacihya --derp-region 1
```
Both 
NOTE: Adding this a a subcommand is a consequence of
clap-rs/clap#2621

## Change checklist

- [x] Self-review.
- [x] Documentation updates if relevant.
- [ ] Tests if relevant.
alexpovel added a commit to alexpovel/srgn that referenced this issue Mar 23, 2025
This feature allows matching, for Go and Rust for
now, some language entities naturally carrying
names, such as structs.

Previously, if one wanted to match structs,
classes, modules, interfaces etc. of specific
names, one would have to make do with the global
regex, applying to the entire scope. That made it
hard to express some queries (see added examples
in the README).

A new special syntax of e.g. `--rust
'struct~SomePattern` now allows to leverage
tree-sitter's native (at least with Rust bindings)
support for matching node names against regex
patterns.

More support might follow in the future, e.g.
Python and its functions, classes etc.

The main blocker was
clap-rs/clap#2621 . This
change introduces a custom `impl` for
`TypedValueParser`, which allows us to go from a
single CLI argument to the rich (`enum` *with*
member data, aka non-unit variants) expression,
with more or less ergonomic user feedback. The new
functionality is fully part of the help output,
which is great!

Technically this is a **breaking change for
library users**, e.g. the `Copy` impl was dropped.

Closes #172.
alexpovel added a commit to alexpovel/srgn that referenced this issue Mar 23, 2025
This feature allows matching, for Go and Rust for
now, some language entities naturally carrying
names, such as structs.

Previously, if one wanted to match structs,
classes, modules, interfaces etc. of specific
names, one would have to make do with the global
regex, applying to the entire scope. That made it
hard to express some queries (see added examples
in the README).

A new special syntax of e.g. `--rust
'struct~SomePattern` now allows to leverage
tree-sitter's native (at least with Rust bindings)
support for matching node names against regex
patterns.

More support might follow in the future, e.g.
Python and its functions, classes etc.

The main blocker was
clap-rs/clap#2621 . This
change introduces a custom `impl` for
`TypedValueParser`, which allows us to go from a
single CLI argument to the rich (`enum` *with*
member data, aka non-unit variants) expression,
with more or less ergonomic user feedback. The new
functionality is fully part of the help output,
which is great!

Technically this is a **breaking change for
library users**, e.g. the `Copy` impl was dropped.

Closes #172.
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-hard Call for participation: Experience needed to fix: Hard / a lot E-help-wanted Call for participation: Help is requested to fix this issue. 💸 $20
Projects
None yet
Development

No branches or pull requests