Make --no-default-groups work with --all-groups#10891
Make --no-default-groups work with --all-groups#10891j178 wants to merge 1 commit intoastral-sh:mainfrom
--no-default-groups work with --all-groups#10891Conversation
crates/uv-cli/src/lib.rs
Outdated
| #[arg(long, conflicts_with_all = ["only_group", "all_groups"])] | ||
| pub group: Vec<GroupName>, |
There was a problem hiding this comment.
I don't think these need to conflict, they're just redundant? (--group foo --all-groups)
| /// `--group` can be used to include specific groups. | ||
| #[arg(long, conflicts_with_all = ["no_group", "only_group"])] | ||
| #[arg(long, conflicts_with_all = ["only_group"])] | ||
| pub no_default_groups: bool, |
There was a problem hiding this comment.
👍 yeah this was wrong
Similarly, I think --only-group is just redundant? --only-group foo --no-default-groups doesn't need to error.
There was a problem hiding this comment.
The current flag conflicts in groups are inconsistent (for example, --only-dev should conflict with --group, but it doesn't). I'd like to address these in a separate PR.
crates/uv-cli/src/lib.rs
Outdated
| #[arg(long, conflicts_with_all = ["group", "no_default_groups", "all_groups"])] | ||
| pub only_group: Vec<GroupName>, |
There was a problem hiding this comment.
Agree this should conflict with --all-groups
38c263e to
26f70b0
Compare
--no-default-groups works with --all-groups--no-default-groups work with --all-groups
| DevMode::Include | DevMode::Only => group == &*DEV_DEPENDENCIES, | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This method is not used anywhere.
|
|
||
| /// Returns `true` if the specification includes the given group. | ||
| pub fn contains(&self, group: &GroupName) -> bool { | ||
| pub fn contains(&self, group: &GroupName, is_default_group: bool) -> bool { |
There was a problem hiding this comment.
The is_default_group: bool argument is not ideal, but it seems to be the simplest solution I could come up with.
|
Thanks for the PR! I agreed the code was hard to augment, so I overhauled the subsystem and landed fixes for these issues in #11224 |
Summary
Closes #10890