fix(parser)!: Store args in a group, rather than values

Now that `Id` is public, we can have `ArgMatches` report them.  If we
have to choose one behavior, this is more universal.  The user can still
look up the values, this works with groups whose args have different
types, and this allows people to make decisions off of it when otherwise
there isn't enogh information.

Fixes #2317
Fixes #3748

Author: epage

CHANGELOG.md

@@ -40,6 +40,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
   built-in flags and be copied to all subcommands, instead disable
   the built-in flags (`Command::disable_help_flag`,
   `Command::disable_version_flag`) and mark the custom flags as `global(true)`.
+- Looking up a group in `ArgMatches` now returns the arg `Id`s, rather than the values.
 - Various `Arg`, `Command`, and `ArgGroup` calls were switched from accepting `&[]` to `[]` via `IntoIterator`
 - *(help)* Make `DeriveDisplayOrder` the default and removed the setting.  To sort help, set `next_display_order(None)` (#2808)
 - *(help)* Subcommand display order respects `Command::next_display_order` instead of `DeriveDisplayOrder` and using its own initial display order value (#2808)

src/builder/arg_group.rs

@@ -49,10 +49,10 @@ use crate::util::Id;
 /// let err = result.unwrap_err();
 /// assert_eq!(err.kind(), ErrorKind::ArgumentConflict);
 /// ```
-/// This next example shows a passing parse of the same scenario
 ///
+/// This next example shows a passing parse of the same scenario
 /// ```rust
-/// # use clap::{Command, arg, ArgGroup};
+/// # use clap::{Command, arg, ArgGroup, Id};
 /// let result = Command::new("cmd")
 ///     .arg(arg!(--"set-ver" <ver> "set the version manually").required(false))
 ///     .arg(arg!(--major           "auto increase major"))
@@ -66,6 +66,13 @@ use crate::util::Id;
 /// let matches = result.unwrap();
 /// // We may not know which of the args was used, so we can test for the group...
 /// assert!(matches.contains_id("vers"));
+/// // We can also ask the group which arg was used
+/// assert_eq!(matches
+///     .get_one::<Id>("vers")
+///     .expect("`vers` is required")
+///     .as_str(),
+///     "major"
+/// );
 /// // we could also alternatively check each arg individually (not shown here)
 /// ```
 /// [`ArgGroup::multiple(true)`]: ArgGroup::multiple()

src/parser/parser.rs

@@ -17,6 +17,7 @@ use crate::mkeymap::KeyType;
 use crate::output::fmt::Stream;
 use crate::output::{fmt::Colorizer, Usage};
 use crate::parser::features::suggestions;
+use crate::parser::AnyValue;
 use crate::parser::{ArgMatcher, SubCommand};
 use crate::parser::{Validator, ValueSource};
 use crate::util::Id;
@@ -1052,15 +1053,19 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
             let value_parser = arg.get_value_parser();
             let val = value_parser.parse_ref(self.cmd, Some(arg), &raw_val)?;
 
-            // Increment or create the group "args"
-            for group in self.cmd.groups_for_arg(&arg.id) {
-                matcher.add_val_to(&group, val.clone(), raw_val.clone());
-            }
-
             matcher.add_val_to(&arg.id, val, raw_val);
             matcher.add_index_to(&arg.id, self.cur_idx.get());
         }
 
+        // Increment or create the group "args"
+        for group in self.cmd.groups_for_arg(&arg.id) {
+            matcher.add_val_to(
+                &group,
+                AnyValue::new(arg.get_id().clone()),
+                OsString::from(arg.get_id().as_str()),
+            );
+        }
+
         Ok(())
     }
 

tests/builder/groups.rs

@@ -1,6 +1,6 @@
 use super::utils;
 
-use clap::{arg, error::ErrorKind, Arg, ArgAction, ArgGroup, Command};
+use clap::{arg, error::ErrorKind, Arg, ArgAction, ArgGroup, Command, Id};
 
 static REQ_GROUP_USAGE: &str = "error: The following required arguments were not provided:
     <base|--delete>
@@ -94,10 +94,7 @@ fn group_single_value() {
 
     let m = res.unwrap();
     assert!(m.contains_id("grp"));
-    assert_eq!(
-        m.get_one::<String>("grp").map(|v| v.as_str()).unwrap(),
-        "blue"
-    );
+    assert_eq!(m.get_one::<Id>("grp").map(|v| v.as_str()).unwrap(), "color");
 }
 
 #[test]
@@ -141,13 +138,7 @@ fn group_multi_value_single_arg() {
 
     let m = res.unwrap();
     assert!(m.contains_id("grp"));
-    assert_eq!(
-        &*m.get_many::<String>("grp")
-            .unwrap()
-            .map(|v| v.as_str())
-            .collect::<Vec<_>>(),
-        &["blue", "red", "green"]
-    );
+    assert_eq!(m.get_one::<Id>("grp").map(|v| v.as_str()).unwrap(), "color");
 }
 
 #[test]
@@ -234,6 +225,13 @@ fn required_group_multiple_args() {
     let m = result.unwrap();
     assert!(*m.get_one::<bool>("flag").expect("defaulted by clap"));
     assert!(*m.get_one::<bool>("color").expect("defaulted by clap"));
+    assert_eq!(
+        &*m.get_many::<Id>("req")
+            .unwrap()
+            .map(|v| v.as_str())
+            .collect::<Vec<_>>(),
+        &["flag", "color"]
+    );
 }
 
 #[test]