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

Add mut_group to Command #5038

Closed
2 tasks done
rgiot opened this issue Jul 21, 2023 · 6 comments · Fixed by #5247
Closed
2 tasks done

Add mut_group to Command #5038

rgiot opened this issue Jul 21, 2023 · 6 comments · Fixed by #5247
Labels
C-enhancement Category: Raise on the bar on expectations

Comments

@rgiot
Copy link

rgiot commented Jul 21, 2023

Please complete the following tasks

Clap Version

4.3.12

Describe your use case

I use a Command object that has been defined in another crate. This command has a group defined this way:

    .group(
        ArgGroup::new("ANY_INPUT")
            .args(&["INLINE", "INPUT", "LIST_EMBEDDED", "VIEW_EMBEDDED"])
    )

I modify this object in my crate to add some additional arguments and especially this one (I do not want to relly on the default implementation of version) :

            .arg(
                Arg::new("version")
                    .long("version")
                    .short('V')
                    .help("Print version")
                    .action(ArgAction::SetTrue)
                    .exclusive(true) // does not seem to work
                    .conflicts_with_all(["ANY_INPUT", "INLINE", "INPUT", "LIST_EMBEDDED", "VIEW_EMBEDDED"])
            )

both exclusive and conflicts_with_all and seems to be ignored, so as a workaround I have temporarily modified the Command definition in the other crate (so it does not fit the need of this initial crate

    .group(
        ArgGroup::new("ANY_INPUT")
            .args(&["INLINE", "INPUT", "LIST_EMBEDDED", "VIEW_EMBEDDED"])
          //  .required(true)
            .conflicts_with("version")
    )

Describe the solution you'd like

Clap already has mut_arg and mut_subcommand.
It would be nice to add mut_group to be consistent.

The solution to my problem would not require anymore to modify the former crate; only to code something similar to

command.mut_group("ANY_INPUT", |group| group.required(false).conflicts_with("version'))

I would say that the implementation of mut_group would be close to

pub fn mut_group<F>(mut self, arg_id: impl AsRef<str>, f: F) -> Self
    where
        F: FnOnce(ArgGroup) -> ArgGroup {
        let group = ... code to get the position, remove from the appropriate vec, handle absence ...
        self.groups.push(f(group));
        self
    }

Alternatives, if applicable

No response

Additional Context

For sure, this mut_group is of interest as it is missing from the library. But I wonder if in my use case it is just a workaround around a bug in the management of conflicting arguments

@rgiot rgiot added the C-enhancement Category: Raise on the bar on expectations label Jul 21, 2023
@epage
Copy link
Member

epage commented Jul 22, 2023

```rust
            .arg(
                Arg::new("version")
                    .long("version")
                    .short('V')
                    .help("Print version")
                    .action(ArgAction::SetTrue)
                    .exclusive(true) // does not seem to work
                    .conflicts_with_all(["ANY_INPUT", "INLINE", "INPUT", "LIST_EMBEDDED", "VIEW_EMBEDDED"])
            )

both exclusive and conflicts_with_all and seems to be ignored, so as a workaround I have temporarily modified the Command definition in the other crate (so it does not fit the need of this initial crate

...

For sure, this mut_group is of interest as it is missing from the library. But I wonder if in my use case it is just a workaround around a bug in the management of conflicting arguments

I feel like the first discussion to be having is about conflicts and exclusive...

@rgiot
Copy link
Author

rgiot commented Jul 22, 2023

You mean by added a dedicated issue with a minimal working example? Or I am misusing something?
(Jtlyk, I added the conflict when I noticed the exclusive was not working)

@epage
Copy link
Member

epage commented Jul 22, 2023

That'd work, thanks!

@rgiot
Copy link
Author

rgiot commented Jul 22, 2023

here is the accompanying issue #5041

@Hawk777
Copy link

Hawk777 commented Dec 3, 2023

I would like to reiterate that this would be really nice to have. My use case is that I want to use the derive API, but I want a specific ArgGroup to sometimes be required and sometimes be optional depending on information discovered at runtime. I’m currently working around it by not using a group and then conditionally using arg_mut to create required_unless_present relationships between the two arguments; however, the help output is not as nice that way (it just claims that both arguments are required, even though really only one or the other is), so I would prefer to use a group—but I can’t make a derived group conditionally required, because I can’t mutate the group at runtime!

epage added a commit to epage/clap that referenced this issue Dec 4, 2023
@epage
Copy link
Member

epage commented Dec 4, 2023

@Hawk777 thanks for the use case! Opened #5247 to address it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: Raise on the bar on expectations
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants