Repeated sets of argument (positional or otherwise) *without* writing custom parsers.

[hydra]

I don't want to have to write clap parsers when you need a repeating set of commands, e.g. set: Vec<KeyValueGroup>. where struct KeyValueGroup { key: String, value: String } where the KeyValueGroup fields are positional arguments.

e.g.

$ tool --set key1 value1 --set key2 value2

this is a simple example, it's much worse in the real world you where you have more complicated types other than String, and where you want specify the parser to use for the positional argument.

instead of this, you have to define a parser and change how you specify the key+value so you end up with a single argument string for each vec item.

e.g.

$ tool --set key1:value1 --set key2:value2

this is exponentially worse when one of your values needs to contain a seperator, e.g.

$ tool --design-sizing design_b:origin=x=20,y=25:offset=x=-10,y=-10:size=x=40,y=50 --design-sizing design_a:origin=x=10,y=15:offset=x=-10,y=-10:size=x=20,y=30

yikes!

also, the solution above is not at all user friendly when it comes to using the --help output.

Here's a code snippet for the above, which I'm sure we'll all agree really should be able to be done out-of-the-box.

#[derive(Debug, Subcommand)]
pub(crate) enum PcbCommand {
    /// Configure a PCB
    ConfigurePanelSizing {
        /// Design sizing (e.g. 'design_a:origin=x=15.25,y=15.25:offset=x=-10.0,y=-10.0:size=x=30.5,y=30.5')
        #[arg(long, value_name = "DESIGN_SIZING", action = clap::ArgAction::Append)]
        design_sizing: Vec<DesignSizingArgs>,
    },
}


#[derive(Debug, Clone)]
pub(crate) struct DesignSizingArgs {
    name: DesignName,
    origin: Vector2<Decimal>,
    offset: Vector2<Decimal>,
    size: Vector2<Decimal>,
}

// FIXME It would be better if all the code below was handled by clap after adding appropriate attributes to `DesignSizingArgs`

#[derive(Error, Debug)]
enum DesignSizingParserError {
    #[error("Missing name")]
    MissingName,
    #[error("Invalid name: {0}")]
    InvalidName(DesignNameError),
    
    #[error("Missing origin")]
    MissingOrigin,
    #[error("Invalid origin: {0}")]
    InvalidOrigin(String),
    
    #[error("Missing offset")]   
    MissingOffset,
    #[error("Invalid offset: {0}")]   
    InvalidOffset(String),
    
    #[error("Missing size")]
    MissingSize,
    #[error("Invalid size: {0}")]  
    InvalidSize(String),
    
    #[error("Invalid format, expected '<name>:origin=x=<x>,y=<y>:offset=x=<x>,y=<y>:size=x=<x>,y=<y>'")] 
    InvalidFormat(String),
}

impl FromStr for DesignSizingArgs {
    type Err = String;

    fn from_str(s: &str) -> Result<Self, Self::Err> {
        design_sizing_parser(s)
            .map_err(|e|e.to_string())
    }
}

/// e.g. 'design_b:origin=x=20,y=25:offset=x=-10,y=-10:size=x=40,y=50'
fn design_sizing_parser(s: &str) -> Result<DesignSizingArgs, DesignSizingParserError> {
    let (design_name, rest) = s.split_once(':')
        .ok_or(DesignSizingParserError::InvalidFormat(s.to_string()))?;

    let name: DesignName = DesignName::try_from(design_name.to_string())
        .map_err(DesignSizingParserError::InvalidName)?;
    
    let mut parts = rest.split(':').peekable();

    let required_keys = ["origin", "offset", "size"];
    
    let mut values: HashMap<&str, Vector2<Decimal>> = HashMap::with_capacity(required_keys.len());
    
    for label_chunk in parts {
        let (label, rest) = label_chunk
            .split_once('=')
            .ok_or_else(|| DesignSizingParserError::InvalidFormat(label_chunk.to_string()))?;

        if !required_keys.contains(&label) {
            return Err(DesignSizingParserError::InvalidFormat(s.to_string()))
        }

        let vector = vector2_decimal_parser(rest)
            .map_err(|e|match label {
                "origin" => DesignSizingParserError::InvalidOrigin(e),
                "offset" => DesignSizingParserError::InvalidOffset(e),
                "size" => DesignSizingParserError::InvalidSize(e),
                _ => unreachable!()
            })?;

        if let Some(_existing_key) = values.insert(label, vector) {
            // same key specified twice
            return Err(DesignSizingParserError::InvalidFormat(s.to_string()))
        }
    }
    
    Ok(DesignSizingArgs {
        name,
        origin: values.remove("origin").ok_or(DesignSizingParserError::MissingOrigin)?,
        offset: values.remove("offset").ok_or(DesignSizingParserError::MissingOffset)?,
        size: values.remove("size").ok_or(DesignSizingParserError::MissingSize)?,
    })
}

If anyone has any idea on how to do this better, I'm all ears.

If we need a new issue for this need, perhaps for Clap V5, let me know and I can open one.

[epage]

If I'm understanding the concern, it is

I don't want to have to write clap parsers when you need a repeating set of commands

also, the solution above is not at all user friendly when it comes to using the --help output.

To me, the solution you gave seems reasonable. I see it as unlikely that there is enough of an agreed-to sub-language for content within values for it to be worth clap including such a parser.

As for help, if its that complicated of content you are populating, I feel like any kind of auto-generated help output would be insufficient. Something that may be worth experimenting with is to hint at the structure through your value_name. I've been considering more structure-focused value names in #5156.

[hydra]

hiya,

I'm fine with writing parsers for data types, like Vector2<Decimal> etc, in an ideal situation, those should be the only parsers that I would need to write, given the requirement for repeated --design-sizing arguments.

However, I never want to have to write a parser for the DesignSizingArgs struct, or going back to the original example a struct like this, when either are used in a Vec:

struct KeyValueGroup { 
    key: String,
    value: String
}

as far as I can see, it should be possible to have repeated sets of named or positional arguments when the structure is used in a Vec.

Regarding the help output, there's nothing wrong with this:

#[derive(Debug, Subcommand)]
pub(crate) enum PcbCommand {
    /// Configure a PCB
    ConfigurePanelSizing {
        /// Design sizing, repeat for each design on the panel
        #[arg(long, value_name = "DESIGN_SIZING", action = clap::ArgAction::Append)]
        design_sizing: Vec<DesignSizingArgs>,
    },
}

#[derive(Debug, Clone)]
pub(crate) struct DesignSizingArgs {
    /// The EDA design name
    name: DesignName,
    /// The origin of the design. format: x=<x>, y=<y>.
    origin: Vector2<Decimal>,
    /// The EDA export offset. format: x=<x>, y=<y>.
    offset: Vector2<Decimal>,
    /// The size of the design. format: x=<x>, y=<y>.
    size: Vector2<Decimal>,
}

The help output could be something like this (positional args):

Configure a PCB

Usage: planner_cli pcb --pcb-file <PCB_FILE> configure-panel-sizing [OPTIONS]

Options:
      --design-sizing <DESIGN_SIZING>...
          Design sizing, repeat for each design on the panel
  -h, --help
          Print help
DESIGN_SIZING:
      <NAME>
        The EDA design name
      <ORIGIN>
        The origin of the design. format: x=<x>, y=<y>.
      <OFFSET>
        The EDA export offset. format: x=<x>, y=<y>.
      <SIZE>
        The size of the design. format: x=<x>, y=<y>.

A valid command would then be:

$ planner_cli pcb
                     --pcb_file myfile.pcb 
                     configure-panel-sizing
                     --design-sizing design_a x=25,y=25 x=-10,y=-10 x=50,y=50
                     --design-sizing design_b x=50,y=50 x=-10,y=-10 x=100,y=100

The help output for named arguments could be something like this:

Configure a PCB

Usage: planner_cli pcb --pcb-file <PCB_FILE> configure-panel-sizing [OPTIONS]

Options:
      --design-sizing <DESIGN_SIZING>...
          Design sizing, repeat for each design on the panel
  -h, --help
          Print help
DESIGN_SIZING:
      --name <NAME>
        The EDA design name
      --origin <ORIGIN>
        The origin of the design. format: x=<x>, y=<y>.
      --offset <OFFSET>
        The EDA export offset. format: x=<x>, y=<y>.
      --size <SIZE>
        The size of the design. format: x=<x>, y=<y>.

A valid command would then be:

$ planner_cli pcb
                     --pcb_file myfile.pcb 
                     configure-panel-sizing
                     --design-sizing --name design_a --origin x=25,y=25 --offset x=-10,y=-10 --size x=50,y=50
                     --design-sizing --name design_b --offset x=50,y=50 --offset x=-10,y=-10 --size x=100,y=100

All seems doable to me, and without forcing everyone to write their own fragile parsers when you need a group of commands.

[epage]

I guess it wasn't too clear. You were saying that you own custom syntax was a workaround for not being able to collect values made of groups of arguments and that is what you want instead?

The most basic case can be done today

$ tool --set key1 value1 --set key2 value2

With a derive, this would be a Vec<String> with num_args=2. You'd then have to handle the pairs of args and conversion to specific types as needed. #1717 is our issue for allowing situations like Vec<(String, u32)> and that working as expected.

Another route through the builder API is occurrences. We don't have an issue specific to supporting that in the derive API but we do have #4626 for how we infer from types more generally.

For the more general case where multiple flags are involved, we are tracking this in #1704 but I suspect the most likely way we'd resolve that is implementing #2222 combined with our flag subcommands.