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

Handle parsing vector of custom structures #1704

Open
krystian-wojtas opened this issue Feb 22, 2020 · 21 comments
Open

Handle parsing vector of custom structures #1704

krystian-wojtas opened this issue Feb 22, 2020 · 21 comments
Labels
A-parsing Area: Parser's logic and needs it changed somehow. C-enhancement Category: Raise on the bar on expectations S-waiting-on-decision Status: Waiting on a go/no-go before implementing

Comments

@krystian-wojtas
Copy link

krystian-wojtas commented Feb 22, 2020

What are you trying to do?

I have group of options which could be used multiple times. All such groups should be kept without overwriting previous one.

$ prometheus_sensors_exporter \
    \
    `# 2 physical sensors located on physycial different i2c bus or address` \
    --sensor \
        --sensor-device=tmp102 \
        --sensor-name="temperature_tmp102_outdoor" \
        --sensor-i2c-bus=0 \
        --sensor-i2c-address=0x48 \
    --sensor \
        --sensor-device=tmp102 \
        --sensor-name="temperature_tmp102_indoor" \
        --sensor-i2c-bus=1 \
        --sensor-i2c-address=0x49 \

--sensor specifies instance of new group. Then suboptions are only for current group.

Ideally parsing would generate instance of Vector<Sensor> defined something like

pub enum Device {
    Tmp102,
    Si7021,
}

pub struct Sensor {
    sensor_device: Device,
    sensor_name: String,
    sensor_i2c_bus: Option<usize>,
    sensor_i2c_address: Option<u8>,
}

Could it be possible?

Or maybe would you recommend to model it in some another way which could be easier to implement?

What have you tried already?

I tired few examples, but I didn't find anything similar so far.

@krystian-wojtas
Copy link
Author

krystian-wojtas commented Feb 23, 2020

I tried some prove of concept. Actually it is implemented with structopt, not clap. Just to give some idea.

I found that structopt has method from_iter which I can use instead of from_args. Then I can provide some subgroup of all parameters and invoke it few times as many sensors are defined by provided user command line parameters.

To split all parameters in such subgroups I used Vector::split function with custom delimiter --sensor.

Code

Here is implementation with only 2 such sensor parameters

// Standard modules paths
use std::env;
use structopt::StructOpt;

#[derive(StructOpt, Debug)]
#[structopt()]
pub struct OptGeneric {

    // Socket

    /// Address of interface to bind
    #[structopt(long, default_value = "0.0.0.0")]
    socket_interface: String,

    /// Number in decimal
    #[structopt(long, default_value = "9898")]
    socket_port: u16,

}


#[derive(StructOpt, Debug)]
#[structopt()]
pub struct OptSensor {

    /// Number in decimal
    #[structopt(long, default_value = "1")]
    sensor_i2c_bus: usize,

    /// Number in hexadecimal
    #[structopt(long)]
    sensor_i2c_address: Option<u8>,

}


#[derive(Debug)]
pub struct Config {
    generic: OptGeneric,
    sensors: Vec<OptSensor>,
}

impl Config {

    pub fn parse(args: std::env::Args) -> Config {

        // Split command line arguments into groups separated by '--sensor' delimiter
        let arguments: Vec<String> = args.collect();
        let mut sensor_parameters_groups = arguments.split(|argument| argument == "--sensor");

         // First group of parameters is generic without any information for specific sensor
         let generic_parameters = sensor_parameters_groups.next()
             .unwrap();

         // Parse generic parameters
         let generic = OptGeneric::from_iter(generic_parameters);

         // Create vector for sensors to be parsed
         let mut sensors = Vec::new();

         // Iterate over rest of groups each containing complete sensor information
         for sensor_parameters in sensor_parameters_groups {

             // OptSensor::from_iter is going to skip first item, so it have to be put anything to be skipped
             // TODO maybe StructOpt::from_iter should not skip first item to not have to workaround it?
             let array_with_empty_item = [String::from("")];
             let iter_with_extra_first_item = array_with_empty_item.iter().chain(sensor_parameters);

             // Parse each sensor information
             let sensor = OptSensor::from_iter(iter_with_extra_first_item);

             // Fill sensors vector
             sensors.push(sensor);
         }

        // Return result
        Config{generic, sensors}
    }

}


fn main() -> Result<(), Box<dyn std::error::Error>> {

    // Parse configuration
    let config = Config::parse(env::args());

    // Debug purpose only
    println!("{:#?}", config);

    // Return success
    Ok(())
}

Run

Invocations are like that:

  • without any arguments
% cargo run --
    Finished dev [unoptimized + debuginfo] target(s) in 0.04s
     Running `target/debug/prometheus_sensors_exporter`
Config {
    generic: OptGeneric {
        socket_interface: "0.0.0.0",
        socket_port: 9898,
    },
    sensors: [],
}
  • some sensors
% cargo run -- \
    \
    `# generic configuration` \
    --socket-interface=1.1.1.1 \
    --socket-port=10000 \
    \
    `# 3 physical sensors located on physycial different i2c bus or address` \
    --sensor \
        --sensor-i2c-bus=0 \
        --sensor-i2c-address=1 \
    --sensor \
        --sensor-i2c-bus=0 \
        --sensor-i2c-address=2 \
    --sensor \
        --sensor-i2c-bus=1 \
        --sensor-i2c-address=1 \
>
    Finished dev [unoptimized + debuginfo] target(s) in 0.04s
     Running `target/debug/prometheus_sensors_exporter --socket-interface=1.1.1.1 --socket-port=10000 --sensor --sensor-i2c-bus=0 --sensor-i2c-address=1 --sensor --sensor-i2c-bus=0 --sensor-i2c-address=2 --sensor --sensor-i2c-bus=1 --sensor-i2c-address=1`
Config {
    generic: OptGeneric {
        socket_interface: "1.1.1.1",
        socket_port: 10000,
    },
    sensors: [
        OptSensor {
            sensor_i2c_bus: 0,
            sensor_i2c_address: Some(
                1,
            ),
        },
        OptSensor {
            sensor_i2c_bus: 0,
            sensor_i2c_address: Some(
                2,
            ),
        },
        OptSensor {
            sensor_i2c_bus: 1,
            sensor_i2c_address: Some(
                1,
            ),
        },
    ],
}
  • help message
% cargo run -- --help
    Finished dev [unoptimized + debuginfo] target(s) in 0.04s
     Running `target/debug/prometheus_sensors_exporter --help`
prometheus_sensors_exporter 0.1.2

USAGE:
    prometheus_sensors_exporter [OPTIONS]

FLAGS:
    -h, --help       Prints help information
    -V, --version    Prints version information

OPTIONS:
        --socket-interface <socket-interface>    Address of interface to bind [default: 0.0.0.0]
        --socket-port <socket-port>              Number in decimal [default: 9898]

Questions

  • Is it some valid approach?
  • Could clap be used to implement something similar?
  • How to include sensors parameters into help message?
  • There is some workaround in array_with_empty_item. Library is going to skip first element, so I have to provide anything to be skipped. Does clap also requires such workaround?

@krystian-wojtas
Copy link
Author

krystian-wojtas commented Feb 24, 2020

I have some another tries

subcommand enum

Code

// Standard modules paths
use structopt::StructOpt;

#[derive(StructOpt, Debug)]
pub struct OptGeneric {

    // Socket

    /// Address of interface to bind
    #[structopt(long, default_value = "0.0.0.0")]
    socket_interface: String,

    /// Number in decimal
    #[structopt(long, default_value = "9898")]
    socket_port: u16,

    #[structopt(subcommand)]
    sensor: OptSensor
}

#[derive(StructOpt, Debug)]
pub enum OptSensor {

    Sensor {
        /// Number in decimal
        #[structopt(long, default_value = "1")]
        sensor_i2c_bus: usize,

        /// Number in hexadecimal
        #[structopt(long)]
        sensor_i2c_address: Option<u8>,
    }
}


fn main() -> Result<(), Box<dyn std::error::Error>> {

    // Parse configuration
    let config = OptGeneric::from_args();

    // Debug purpose only
    println!("{:#?}", config);

    // Return success
    Ok(())
}

Run with one sensor

% cargo run -- \
    \
    `# generic configuration` \
    --socket-interface=1.1.1.1 \
    --socket-port=10000 \
    \
    sensor \
        --sensor-i2c-bus=0 \
        --sensor-i2c-address=1 \


    Finished dev [unoptimized + debuginfo] target(s) in 0.04s
     Running `target/debug/prometheus_sensors_exporter --socket-interface=1.1.1.1 --socket-port=10000 sensor --sensor-i2c-bus=0 --sensor-i2c-address=1`
OptGeneric {
    socket_interface: "1.1.1.1",
    socket_port: 10000,
    sensor: Sensor {
        sensor_i2c_bus: 0,
        sensor_i2c_address: Some(
            1,
        ),
    },
}

Run with 2 sensors

% cargo run -- \
    \
    `# generic configuration` \
    --socket-interface=1.1.1.1 \
    --socket-port=10000 \
    \
    sensor \
        --sensor-i2c-bus=0 \
        --sensor-i2c-address=1 \
    sensor \
      --sensor-i2c-bus=1 \
      --sensor-i2c-address=1 \
>
    Finished dev [unoptimized + debuginfo] target(s) in 0.04s
     Running `target/debug/prometheus_sensors_exporter --socket-interface=1.1.1.1 --socket-port=10000 sensor --sensor-i2c-bus=0 --sensor-i2c-address=1 sensor --sensor-i2c-bus=1 --sensor-i2c-address=1`
error: Found argument 'sensor' which wasn't expected, or isn't valid in this context

USAGE:
    prometheus_sensors_exporter sensor --sensor-i2c-address <sensor-i2c-address> --sensor-i2c-bus <sensor-i2c-bus>

For more information try --help

subcommand vec enum

Code

// Standard modules paths
use structopt::StructOpt;

#[derive(StructOpt, Debug)]
pub struct OptGeneric {

    // Socket

    /// Address of interface to bind
    #[structopt(long, default_value = "0.0.0.0")]
    socket_interface: String,

    /// Number in decimal
    #[structopt(long, default_value = "9898")]
    socket_port: u16,

    #[structopt(subcommand)]
    sensors: Vec<OptSensor>
}

#[derive(StructOpt, Debug)]
pub enum OptSensor {

    Sensor {
        /// Number in decimal
        #[structopt(long, default_value = "1")]
        sensor_i2c_bus: usize,

        /// Number in hexadecimal
        #[structopt(long)]
        sensor_i2c_address: Option<u8>,
    }
}


fn main() -> Result<(), Box<dyn std::error::Error>> {

    // Parse configuration
    let config = OptGeneric::from_args();

    // Debug purpose only
    println!("{:#?}", config);

    // Return success
    Ok(())
}

Cannot compile

% cargo build
   Compiling prometheus_sensors_exporter v0.1.2 (/home/k/project/prometheus_sensors_exporter)
error[E0277]: the trait bound `std::vec::Vec<OptSensor>: structopt::StructOptInternal` is not satisfied
  --> src/main.rs:17:17
   |
17 |     #[structopt(subcommand)]
   |                 ^^^^^^^^^^ the trait `structopt::StructOptInternal` is not implemented for `std::vec::Vec<OptSensor>`
   |
   = note: required by `structopt::StructOptInternal::from_subcommand`

error[E0277]: the trait bound `std::vec::Vec<OptSensor>: structopt::StructOptInternal` is not satisfied
 --> src/main.rs:4:10
  |
4 | #[derive(StructOpt, Debug)]
  |          ^^^^^^^^^ the trait `structopt::StructOptInternal` is not implemented for `std::vec::Vec<OptSensor>`
  |
  = note: required by `structopt::StructOptInternal::augment_clap`

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0277`.
error: could not compile `prometheus_sensors_exporter`.

To learn more, run the command again with --verbose.

Should I implement StructOptInternal trait? Is it something recommended in this case?

@pksunkara
Copy link
Member

Can you try with the master branch of clap? We have clap_derive

@krystian-wojtas
Copy link
Author

@pksunkara, sure I would try it.

@krystian-wojtas
Copy link
Author

Using master clap

subcommand enum

Code

// Standard modules paths
use clap::Clap;

#[derive(Clap, Debug)]
pub struct OptGeneric {

    // Socket

    /// Address of interface to bind
    #[clap(long, default_value = "0.0.0.0")]
    socket_interface: String,

    /// Number in decimal
    #[clap(long, default_value = "9898")]
    socket_port: u16,

    #[clap(subcommand)]
    sensor: OptSensor
}

#[derive(Clap, Debug)]
pub enum OptSensor {

    Sensor {
        /// Number in decimal
        #[clap(long, default_value = "1")]
        sensor_i2c_bus: usize,

        /// Number in hexadecimal
        #[clap(long)]
        sensor_i2c_address: Option<u8>,
    }
}


fn main() -> Result<(), Box<dyn std::error::Error>> {

    // Parse configuration
    let config = OptGeneric::parse();

    // Debug purpose only
    println!("{:#?}", config);

    // Return success
    Ok(())
}

Run with one sensor is ok

% cargo run -- \
    \
    `# generic configuration` \
    --socket-interface=1.1.1.1 \
    --socket-port=10000 \
    \
    sensor \
        --sensor-i2c-bus=0 \
        --sensor-i2c-address=1 \

   Compiling prometheus_sensors_exporter v0.1.2 (/home/k/project/prometheus_sensors_exporter)
    Finished dev [unoptimized + debuginfo] target(s) in 0.52s
     Running `target/debug/prometheus_sensors_exporter --socket-interface=1.1.1.1 --socket-port=10000 sensor --sensor-i2c-bus=0 --sensor-i2c-address=1`
OptGeneric {
    socket_interface: "1.1.1.1",
    socket_port: 10000,
    sensor: Sensor {
        sensor_i2c_bus: 0,
        sensor_i2c_address: Some(
            1,
        ),
    },
}

Run with 2 sensors is not allowed

% cargo run -- \
    \
    `# generic configuration` \
    --socket-interface=1.1.1.1 \
    --socket-port=10000 \
    \
    sensor \
        --sensor-i2c-bus=0 \
        --sensor-i2c-address=1 \
    sensor \
      --sensor-i2c-bus=1 \
      --sensor-i2c-address=1 \

    Finished dev [unoptimized + debuginfo] target(s) in 0.04s
     Running `target/debug/prometheus_sensors_exporter --socket-interface=1.1.1.1 --socket-port=10000 sensor --sensor-i2c-bus=0 --sensor-i2c-address=1 sensor --sensor-i2c-bus=1 --sensor-i2c-address=1`
error: Found argument 'sensor' which wasn't expected, or isn't valid in this context
If you tried to supply `sensor` as a PATTERN use `-- sensor`

USAGE:
    prometheus_sensors_exporter sensor [OPTIONS]

For more information try --help

subcommand vec enum

Code

// Standard modules paths
use clap::Clap;

#[derive(Clap, Debug)]
pub struct OptGeneric {

    // Socket

    /// Address of interface to bind
    #[clap(long, default_value = "0.0.0.0")]
    socket_interface: String,

    /// Number in decimal
    #[clap(long, default_value = "9898")]
    socket_port: u16,

    #[clap(subcommand)]
    sensors: Vec<OptSensor>
}

#[derive(Clap, Debug)]
pub enum OptSensor {

    Sensor {
        /// Number in decimal
        #[clap(long, default_value = "1")]
        sensor_i2c_bus: usize,

        /// Number in hexadecimal
        #[clap(long)]
        sensor_i2c_address: Option<u8>,
    }
}


fn main() -> Result<(), Box<dyn std::error::Error>> {

    // Parse configuration
    let config = OptGeneric::parse();

    // Debug purpose only
    println!("{:#?}", config);

    // Return success
    Ok(())
}

Cannot compile. Error message is much better than structopt

[Running 'cargo check']
    Checking prometheus_sensors_exporter v0.1.2 (/home/k/project/prometheus_sensors_exporter)
error[E0277]: the trait bound `std::vec::Vec<OptSensor>: clap::derive::Subcommand` is not satisfied
 --> src/main.rs:4:10
  |
4 | #[derive(Clap, Debug)]
  |          ^^^^ the trait `clap::derive::Subcommand` is not implemented for `std::vec::Vec<OptSensor>`
  |
  = note: required by `clap::derive::Subcommand::augment_subcommands`

error[E0277]: the trait bound `std::vec::Vec<OptSensor>: clap::derive::Subcommand` is not satisfied
  --> src/main.rs:17:12
   |
17 |     #[clap(subcommand)]
   |            ^^^^^^^^^^ the trait `clap::derive::Subcommand` is not implemented for `std::vec::Vec<OptSensor>`
   |
   = note: required by `clap::derive::Subcommand::from_subcommand`

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0277`.
error: could not compile `prometheus_sensors_exporter`.

To learn more, run the command again with --verbose.
[Finished running. Exit status: 101]

@krystian-wojtas
Copy link
Author

Clap has

Support their own sub-arguments, and sub-sub-commands independent of the parent

Maybe this is some solution? Could it be sub-sub-sub-...-commands also? Could it be wrapped into Option (with Box?) to brake infinite recursion?

This try is interesting as it compiles ok, but overflow stack in runtime

Code

// Standard modules paths
use clap::Clap;

#[derive(Clap, Debug)]
pub struct OptGeneric {

    // Socket

    /// Address of interface to bind
    #[clap(long, default_value = "0.0.0.0")]
    socket_interface: String,

    /// Number in decimal
    #[clap(long, default_value = "9898")]
    socket_port: u16,

    #[clap(subcommand)]
    sensors: OptSensor
}

#[derive(Clap, Debug)]
pub enum OptSensor {

    Sensor {
        /// Number in decimal
        #[clap(long, default_value = "1")]
        sensor_i2c_bus: usize,

        /// Number in hexadecimal
        #[clap(long)]
        sensor_i2c_address: Option<u8>,

        #[clap(subcommand)]
        sensors: Option<Box<OptSensor>>
    }
}


fn main() -> Result<(), Box<dyn std::error::Error>> {

    // Parse configuration
    let config = OptGeneric::parse();

    // Debug purpose only
    println!("{:#?}", config);

    // Return success
    Ok(())
}

Run

% cargo run -- \
    \
    `# generic configuration` \
    --socket-interface=1.1.1.1 \
    --socket-port=10000 \
    \
    sensor \
        --sensor-i2c-bus=0 \
        --sensor-i2c-address=1 \


   Compiling prometheus_sensors_exporter v0.1.2 (/home/k/project/prometheus_sensors_exporter)
    Finished dev [unoptimized + debuginfo] target(s) in 0.62s
     Running `target/debug/prometheus_sensors_exporter --socket-interface=1.1.1.1 --socket-port=10000 sensor --sensor-i2c-bus=0 --sensor-i2c-address=1`

thread 'main' has overflowed its stack
fatal runtime error: stack overflow
[1]    9558 abort (core dumped)  cargo run -- `# generic configuration` --socket-interface=1.1.1.1  sensor

@krystian-wojtas
Copy link
Author

I see that there is some proper example of subcommands on some nested levels
https://github.com/clap-rs/clap/blob/master/examples/20_subcommands.rs

@krystian-wojtas
Copy link
Author

I see that example from documentation have specified tree of subcommands known ahead at compile time.

Here is some another attempt

not multiple

Code

// Standard modules paths
use clap::Clap;

#[derive(Clap, Debug)]
pub struct OptGeneric {

    // Socket

    /// Address of interface to bind
    #[clap(long, default_value = "0.0.0.0")]
    socket_interface: String,

    /// Number in decimal
    #[clap(long, default_value = "9898")]
    socket_port: u16,

    #[clap(subcommand)]
    sensor: OptSensor,
}

#[derive(Clap, Debug)]
pub enum OptSensor {

    Sensor {

        /// Number in decimal
        #[clap(long, default_value = "1")]
        sensor_i2c_bus: usize,

        /// Number in hexadecimal
        #[clap(long)]
        sensor_i2c_address: Option<u8>,

        #[clap()]
        more_optional_sensors: Vec<String>,
    }
}


fn main() -> Result<(), Box<dyn std::error::Error>> {

    // Parse configuration
    let config = OptGeneric::parse();

    // Debug purpose only
    println!("{:#?}", config);

    // Return success
    Ok(())
}

Run help

% cargo run -- help
    Finished dev [unoptimized + debuginfo] target(s) in 0.04s
     Running `target/debug/prometheus_sensors_exporter help`
prometheus_sensors_exporter

USAGE:
    prometheus_sensors_exporter [OPTIONS] <SUBCOMMAND>

FLAGS:
    -h, --help       Prints help information
    -V, --version    Prints version information

OPTIONS:
        --socket-interface <socket-interface>    Address of interface to bind [default: 0.0.0.0]
        --socket-port <socket-port>              Number in decimal [default: 9898]

SUBCOMMANDS:
    help      Prints this message or the help of the given subcommand(s)
    sensor

Run help for subcommand

% cargo run -- help sensor
    Finished dev [unoptimized + debuginfo] target(s) in 0.04s
     Running `target/debug/prometheus_sensors_exporter help sensor`
prometheus_sensors_exporter-sensor

USAGE:
    prometheus_sensors_exporter sensor [OPTIONS] [more-optional-sensors]...

ARGS:
    <more-optional-sensors>...

FLAGS:
    -h, --help       Prints help information
    -V, --version    Prints version information

OPTIONS:
        --sensor-i2c-address <sensor-i2c-address>    Number in hexadecimal
        --sensor-i2c-bus <sensor-i2c-bus>            Number in decimal [default: 1]

Run with one sensor

% cargo run -- \
    \
    `# generic configuration` \
    --socket-interface=1.1.1.1 \
    --socket-port=10000 \
    \
    sensor \
        --sensor-i2c-bus=0 \
        --sensor-i2c-address=1 \
>
    Finished dev [unoptimized + debuginfo] target(s) in 0.04s
     Running `target/debug/prometheus_sensors_exporter --socket-interface=1.1.1.1 --socket-port=10000 sensor --sensor-i2c-bus=0 --sensor-i2c-address=1`
OptGeneric {
    socket_interface: "1.1.1.1",
    socket_port: 10000,
    sensor: Sensor {
        sensor_i2c_bus: 0,
        sensor_i2c_address: Some(
            1,
        ),
        more_optional_sensors: [],
    },
}

Run with 2 sensors

% cargo run -- \
    \
    `# generic configuration` \
    --socket-interface=1.1.1.1 \
    --socket-port=10000 \
    \
    sensor \
        --sensor-i2c-bus=0 \
        --sensor-i2c-address=1 \
    \
    sensor \
        --sensor-i2c-bus=1 \
        --sensor-i2c-address=1 \
>
    Finished dev [unoptimized + debuginfo] target(s) in 0.04s
     Running `target/debug/prometheus_sensors_exporter --socket-interface=1.1.1.1 --socket-port=10000 sensor --sensor-i2c-bus=0 --sensor-i2c-address=1 sensor --sensor-i2c-bus=1 --sensor-i2c-address=1`
error: The argument '--sensor-i2c-bus <sensor-i2c-bus>' was provided more than once, but cannot be used multiple times

USAGE:
    prometheus_sensors_exporter sensor [OPTIONS] [more-optional-sensors]...

For more information try --help

multiple

// Standard modules paths
use clap::Clap;

#[derive(Clap, Debug)]
pub struct OptGeneric {

    // Socket

    /// Address of interface to bind
    #[clap(long, default_value = "0.0.0.0")]
    socket_interface: String,

    /// Number in decimal
    #[clap(long, default_value = "9898")]
    socket_port: u16,

    #[clap(subcommand)]
    sensor: OptSensor,
}

#[derive(Clap, Debug)]
pub enum OptSensor {

    Sensor {

        /// Number in decimal
        #[clap(long, default_value = "1", multiple = true)]
        sensor_i2c_bus: usize,

        /// Number in hexadecimal
        #[clap(long, multiple = true)]
        sensor_i2c_address: Option<u8>,

        #[clap()]
        more_optional_sensors: Vec<String>,
    }
}


fn main() -> Result<(), Box<dyn std::error::Error>> {

    // Parse configuration
    let config = OptGeneric::parse();

    // Debug purpose only
    println!("{:#?}", config);

    // Return success
    Ok(())
}

Run with 2 sensors

% cargo run -- \
    \
    `# generic configuration` \
    --socket-interface=1.1.1.1 \
    --socket-port=10000 \
    \
    sensor \
        --sensor-i2c-bus=0 \
        --sensor-i2c-address=1 \
    \
    sensor \
        --sensor-i2c-bus=1 \
        --sensor-i2c-address=1 \

    Finished dev [unoptimized + debuginfo] target(s) in 0.04s
     Running `target/debug/prometheus_sensors_exporter --socket-interface=1.1.1.1 --socket-port=10000 sensor --sensor-i2c-bus=0 --sensor-i2c-address=1 sensor --sensor-i2c-bus=1 --sensor-i2c-address=1`
OptGeneric {
    socket_interface: "1.1.1.1",
    socket_port: 10000,
    sensor: Sensor {
        sensor_i2c_bus: 0,
        sensor_i2c_address: Some(
            1,
        ),
        more_optional_sensors: [
            "sensor",
        ],
    },
}

Parsing consumes all parameters overwriting previous ones and they are not available in more_optional_sensors strings vector.

@krystian-wojtas
Copy link
Author

Here is best result I have so far

structopt

Code

// Standard modules paths
use std::env;
use clap::Clap;

#[derive(Clap, Debug)]
pub struct OptBasic {

    // Socket

    /// Address of interface to bind
    #[clap(long, default_value = "0.0.0.0")]
    socket_interface: String,

    /// Number in decimal
    #[clap(long, default_value = "9898")]
    socket_port: u16,

    #[clap(flatten)]
    sensor: OptSensor,

    // Field is declared only for help message
    // Vector is never filled
    /// Each sensor definition should start with <sensor> keyword followed by its options
    #[clap()]
    more_optional_sensors: Vec<String>,
}


#[derive(Clap, Debug)]
pub struct OptSensor {

    /// Number in decimal
    #[clap(long, default_value = "1")]
    sensor_i2c_bus: usize,

    /// Number in hexadecimal
    #[clap(long)]
    sensor_i2c_address: Option<u8>,
}


#[derive(Debug)]
pub struct Config {

    // Address of interface to bind
    socket_interface: String,

    // Number in decimal
    socket_port: u16,

    // All sensors
    sensors: Vec<OptSensor>,
}

impl Config {

    pub fn parse(args: std::env::Args) -> Self {

        // Split command line arguments into groups separated by '--sensor' delimiter
        let arguments: Vec<String> = args.collect();
        let mut sensor_parameters_groups = arguments.split(|argument| argument == "sensor");

         // First group of parameters is basic without any information for specific sensor
         let basic_parameters = sensor_parameters_groups.next()
             .unwrap();

         // Parse basic parameters
         let basic = OptBasic::from_iter(basic_parameters);

         // Create vector to be filled with all parsed sensors
         let mut sensors = Vec::new();

         // Add already parsed basic sensor to list of all sensors
         sensors.push(basic.sensor);

         // Iterate over rest of groups each containing complete sensor information
         for sensor_parameters in sensor_parameters_groups {

             // OptSensor::from_iter is going to skip first item, so it have to be put anything to be skipped
             // TODO maybe StructOpt::from_iter should not skip first item to not have to workaround it?
             let array_with_empty_item = [String::from("")];
             let iter_with_extra_first_item = array_with_empty_item.iter().chain(sensor_parameters);

             // Parse each sensor information
             let sensor = OptSensor::from_iter(iter_with_extra_first_item);

             // Fill sensors vector
             sensors.push(sensor);
         }

        // Return result
        Config {
            socket_interface: basic.socket_interface,
            socket_port: basic.socket_port,
            sensors,
        }
    }

}


fn main() -> Result<(), Box<dyn std::error::Error>> {

    // Parse configuration
    let config = Config::parse(env::args());

    // Debug purpose only
    println!("{:#?}", config);

    // Return success
    Ok(())
}

Run help

% cargo run -- --help
    Finished dev [unoptimized + debuginfo] target(s) in 0.04s
     Running `target/debug/prometheus_sensors_exporter --help`
prometheus_sensors_exporter 0.1.2

USAGE:
    prometheus_sensors_exporter [OPTIONS] [more-optional-sensors]...

FLAGS:
    -h, --help       Prints help information
    -V, --version    Prints version information

OPTIONS:
        --sensor-i2c-address <sensor-i2c-address>    Number in hexadecimal
        --sensor-i2c-bus <sensor-i2c-bus>            Number in decimal [default: 1]
        --socket-interface <socket-interface>        Address of interface to bind [default: 0.0.0.0]
        --socket-port <socket-port>                  Number in decimal [default: 9898]

ARGS:
    <more-optional-sensors>...    Each sensor definition should start with <sensor> keyword followed by its options

Run with one sensor

% cargo run -- \
    \
    `# generic configuration` \
    --socket-interface=1.1.1.1 \
    --socket-port=10000 \
    \
    `# Basic sensor` \
    --sensor-i2c-bus=4 \

    Finished dev [unoptimized + debuginfo] target(s) in 0.04s
     Running `target/debug/prometheus_sensors_exporter --socket-interface=1.1.1.1 --socket-port=10000 --sensor-i2c-bus=4`
Config {
    socket_interface: "1.1.1.1",
    socket_port: 10000,
    sensors: [
        OptSensor {
            sensor_i2c_bus: 4,
            sensor_i2c_address: None,
        },
    ],
}

Run with 3 sensors

% cargo run -- \
    \
    `# generic configuration` \
    --socket-interface=1.1.1.1 \
    --socket-port=10000 \
    \
    `# Basic sensor` \
    --sensor-i2c-bus=4 \
    \
    `# More optional sensors` \
    sensor \
        --sensor-i2c-bus=0 \
        --sensor-i2c-address=1 \
    \
    sensor \
        --sensor-i2c-bus=1 \
        --sensor-i2c-address=1 \

    Finished dev [unoptimized + debuginfo] target(s) in 0.04s
     Running `target/debug/prometheus_sensors_exporter --socket-interface=1.1.1.1 --socket-port=10000 --sensor-i2c-bus=4 sensor --sensor-i2c-bus=0 --sensor-i2c-address=1 sensor --sensor-i2c-bus=1 --sensor-i2c-address=1`
Config {
    socket_interface: "1.1.1.1",
    socket_port: 10000,
    sensors: [
        OptSensor {
            sensor_i2c_bus: 4,
            sensor_i2c_address: None,
        },
        OptSensor {
            sensor_i2c_bus: 0,
            sensor_i2c_address: Some(
                1,
            ),
        },
        OptSensor {
            sensor_i2c_bus: 1,
            sensor_i2c_address: Some(
                1,
            ),
        },
    ],
}

clap

Code

// Standard modules paths
use std::env;
use clap::Clap;

#[derive(Clap, Debug)]
pub struct OptBasic {

    // Socket

    /// Address of interface to bind
    #[clap(long, default_value = "0.0.0.0")]
    socket_interface: String,

    /// Number in decimal
    #[clap(long, default_value = "9898")]
    socket_port: u16,

    #[clap(flatten)]
    sensor: OptSensor,

    // Field is declared only for help message
    // Vector is never filled
    /// Each sensor definition should start with <sensor> keyword followed by its options
    #[clap()]
    more_optional_sensors: Vec<String>,
}


#[derive(Clap, Debug)]
pub struct OptSensor {

    /// Number in decimal
    #[clap(long, default_value = "1")]
    sensor_i2c_bus: usize,

    /// Number in hexadecimal
    #[clap(long)]
    sensor_i2c_address: Option<u8>,
}


#[derive(Debug)]
pub struct Config {

    // Address of interface to bind
    socket_interface: String,

    // Number in decimal
    socket_port: u16,

    // All sensors
    sensors: Vec<OptSensor>,
}

impl Config {

    pub fn parse(args: std::env::Args) -> Self {

        // Split command line arguments into groups separated by '--sensor' delimiter
        let arguments: Vec<String> = args.collect();
        let mut sensor_parameters_groups = arguments.split(|argument| argument == "sensor");

         // First group of parameters is basic without any information for specific sensor
         let basic_parameters = sensor_parameters_groups.next()
             .unwrap();

         // Parse basic parameters
         let basic = OptBasic::from_iter(basic_parameters);

         // Create vector to be filled with all parsed sensors
         let mut sensors = Vec::new();

         // Add already parsed basic sensor to list of all sensors
         sensors.push(basic.sensor);

         // Iterate over rest of groups each containing complete sensor information
         for sensor_parameters in sensor_parameters_groups {

             // OptSensor::from_iter is going to skip first item, so it have to be put anything to be skipped
             // TODO maybe StructOpt::from_iter should not skip first item to not have to workaround it?
             let array_with_empty_item = [String::from("")];
             let iter_with_extra_first_item = array_with_empty_item.iter().chain(sensor_parameters);

             // Parse each sensor information
             let sensor = OptSensor::from_iter(iter_with_extra_first_item);

             // Fill sensors vector
             sensors.push(sensor);
         }

        // Return result
        Config {
            socket_interface: basic.socket_interface,
            socket_port: basic.socket_port,
            sensors,
        }
    }

}


fn main() -> Result<(), Box<dyn std::error::Error>> {

    // Parse configuration
    let config = Config::parse(env::args());

    // Debug purpose only
    println!("{:#?}", config);

    // Return success
    Ok(())
}

Cannot compile

[Running 'cargo check']
    Checking prometheus_sensors_exporter v0.1.2 (/home/k/project/prometheus_sensors_exporter)
error[E0599]: no function or associated item named `from_iter` found for type `OptBasic` in the current scope
  --> src/main.rs:68:32
   |
6  | pub struct OptBasic {
   | ------------------- function or associated item `from_iter` not found for this
...
68 |          let basic = OptBasic::from_iter(basic_parameters);
   |                                ^^^^^^^^^ function or associated item not found in `OptBasic`
   |
   = help: items from traits can only be used if the trait is implemented and in scope
   = note: the following trait defines an item `from_iter`, perhaps you need to implement it:
           candidate #1: `std::iter::FromIterator`

error[E0599]: no function or associated item named `from_iter` found for type `OptSensor` in the current scope
  --> src/main.rs:85:38
   |
30 | pub struct OptSensor {
   | -------------------- function or associated item `from_iter` not found for this
...
85 |              let sensor = OptSensor::from_iter(iter_with_extra_first_item);
   |                                      ^^^^^^^^^ function or associated item not found in `OptSensor`
   |
   = help: items from traits can only be used if the trait is implemented and in scope
   = note: the following trait defines an item `from_iter`, perhaps you need to implement it:
           candidate #1: `std::iter::FromIterator`

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0599`.
error: could not compile `prometheus_sensors_exporter`.

To learn more, run the command again with --verbose.
[Finished running. Exit status: 101]

Is there any similar method to pass custom iterator for clap parser?

@krystian-wojtas
Copy link
Author

structopt::from_iter is called clap::parse_from.

Here is same thing as previous implemented with clap.

clap

Code

// Standard modules paths
use std::env;
use clap::Clap;

#[derive(Clap, Debug)]
pub struct OptBasic {

    // Socket

    /// Address of interface to bind
    #[clap(long, default_value = "0.0.0.0")]
    socket_interface: String,

    /// Number in decimal
    #[clap(long, default_value = "9898")]
    socket_port: u16,

    #[clap(flatten)]
    sensor: OptSensor,

    // Field is declared only for help message
    // Vector is never filled
    /// Each sensor definition should start with <sensor> keyword followed by its options
    #[clap()]
    more_optional_sensors: Vec<String>,
}


#[derive(Clap, Debug)]
pub struct OptSensor {

    /// Number in decimal
    #[clap(long, default_value = "1")]
    sensor_i2c_bus: usize,

    /// Number in hexadecimal
    #[clap(long)]
    sensor_i2c_address: Option<u8>,
}


#[derive(Debug)]
pub struct Config {

    // Address of interface to bind
    socket_interface: String,

    // Number in decimal
    socket_port: u16,

    // All sensors
    sensors: Vec<OptSensor>,
}

impl Config {

    pub fn parse(args: std::env::Args) -> Self {

        // Split command line arguments into groups separated by '--sensor' delimiter
        let arguments: Vec<String> = args.collect();
        let mut sensor_parameters_groups = arguments.split(|argument| argument == "sensor");

         // First group of parameters is basic without any information for specific sensor
         let basic_parameters = sensor_parameters_groups.next()
             .unwrap();

         // Parse basic parameters
         let basic = OptBasic::parse_from(basic_parameters);

         // Create vector to be filled with all parsed sensors
         let mut sensors = Vec::new();

         // Add already parsed basic sensor to list of all sensors
         sensors.push(basic.sensor);

         // Iterate over rest of groups each containing complete sensor information
         for sensor_parameters in sensor_parameters_groups {

             // Parse each sensor information
             let sensor = OptSensor::parse_from(sensor_parameters);

             // Fill sensors vector
             sensors.push(sensor);
         }

        // Return result
        Config {
            socket_interface: basic.socket_interface,
            socket_port: basic.socket_port,
            sensors,
        }
    }

}


fn main() -> Result<(), Box<dyn std::error::Error>> {

    // Parse configuration
    let config = Config::parse(env::args());

    // Debug purpose only
    println!("{:#?}", config);

    // Return success
    Ok(())
}

Run with some sensors

% cargo run -- \
    \
    `# generic configuration` \
    --socket-interface=1.1.1.1 \
    --socket-port=10000 \
    \
    `# Basic sensor` \
    --sensor-i2c-bus=4 \
    \
    `# More optional sensors` \
    sensor \
        --sensor-i2c-bus=0 \
        --sensor-i2c-address=1 \
    \
    sensor \
        --sensor-i2c-bus=1 \
        --sensor-i2c-address=1 \

   Compiling prometheus_sensors_exporter v0.1.2 (/home/k/project/prometheus_sensors_exporter)
    Finished dev [unoptimized + debuginfo] target(s) in 0.65s
     Running `target/debug/prometheus_sensors_exporter --socket-interface=1.1.1.1 --socket-port=10000 --sensor-i2c-bus=4 sensor --sensor-i2c-bus=0 --sensor-i2c-address=1 sensor --sensor-i2c-bus=1 --sensor-i2c-address=1`
Config {
    socket_interface: "1.1.1.1",
    socket_port: 10000,
    sensors: [
        OptSensor {
            sensor_i2c_bus: 4,
            sensor_i2c_address: None,
        },
        OptSensor {
            sensor_i2c_bus: 1,
            sensor_i2c_address: Some(
                1,
            ),
        },
        OptSensor {
            sensor_i2c_bus: 1,
            sensor_i2c_address: Some(
                1,
            ),
        },
    ],
}

Help

% cargo run -- --help
    Finished dev [unoptimized + debuginfo] target(s) in 0.04s
     Running `target/debug/prometheus_sensors_exporter --help`
prometheus_sensors_exporter

USAGE:
    prometheus_sensors_exporter [OPTIONS] [more-optional-sensors]...

ARGS:
    <more-optional-sensors>...    Each sensor definition should start with <sensor> keyword followed by its options

FLAGS:
    -h, --help       Prints help information
    -V, --version    Prints version information

OPTIONS:
        --sensor-i2c-address <sensor-i2c-address>    Number in hexadecimal
        --sensor-i2c-bus <sensor-i2c-bus>            Number in decimal [default: 1]
        --socket-interface <socket-interface>        Address of interface to bind [default: 0.0.0.0]
        --socket-port <socket-port>                  Number in decimal [default: 9898]

It works fine. Clap and structopt are great :) Clap looks a little better.

@krystian-wojtas
Copy link
Author

krystian-wojtas commented Feb 25, 2020

To summarize:

I have parsing of command line arguments using this great library. For my case I had to split all commands to some smaller groups and run individual parsing for them.

I'm wondering if it could be possible to express this logic by clap itself? Is such case worth to implement? Would it be more users interested in it? Or there are a low chance that anyone else would like to use clap in similar way?

Is it useful to implement passing vector of enums? Something like

#[clap(subcommand)] 
// or #[clap(subcommands)]  ?
field: Vec<UserEnum>

Maybe there is some better way to model such command line arguments to fit into current clap implementation?

@pksunkara
Copy link
Member

@CreepySkeleton What do you think about this whole issue? It's a different way of doing things for sure.

@CreepySkeleton
Copy link
Contributor

I'm afraid there's no way to implement this exact design with clap, but you can get pretty close.

You see, --sensor takes arbitrary number of values up to the next occurrence of --sensor. There's no way to express this condition via clap API, unfortunately, so it couldn't work, not out of box.

You can overcome this limitation by applying some sort of external preprocessing, splitting the args into groups manually, as you do in the structopt example.

I can think about the alternative:

$ prometheus_sensors_exporter \
    \
    `# 2 physical sensors located on physycial different i2c bus or address` \
    --sensor \
        sensor-device=tmp102 \
        sensor-name="temperature_tmp102_outdoor" \
        sensor-i2c-bus=0 \
        sensor-i2c-address=0x48 \
    --sensor \
        sensor-device=tmp102 \
        sensor-name="temperature_tmp102_indoor" \
        sensor-i2c-bus=1 \
        sensor-i2c-address=0x49 \

The idea is to threat each group as one option with multiple arguments, each parameter is a key-value pair. Unfortunately, parsing these values is up to you, and the auto generated help message leaves much to be desired.

I'm wondering if it could be possible to express this logic by clap itself? Is such case worth to implement? Would it be more users interested in it? Or there are a low chance that anyone else would like to use clap in similar way?

Those are good questions, and whether we will consider adding support for this use case or not depends on the answers. This is the first such request, and it is also the first time I personally encounter this design.

Maybe there is some better way to model such command line arguments to fit into current clap implementation?

Ideas are welcome!

@leshow
Copy link

leshow commented Mar 3, 2020

I found this issue trying to do the exact same thing. The case is that I want to pass multiple --something where each Something has a set of values.

@CreepySkeleton CreepySkeleton added A-parsing Area: Parser's logic and needs it changed somehow. 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. T: new feature S-waiting-on-decision Status: Waiting on a go/no-go before implementing labels Jun 30, 2020
@epage epage added C-enhancement Category: Raise on the bar on expectations and removed T: RFC / question labels Dec 8, 2021
@epage
Copy link
Member

epage commented Dec 8, 2021

We've also talked some about this in #2683

@epage epage removed 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. labels Dec 9, 2021
@elkowar
Copy link

elkowar commented Sep 3, 2022

I'd still love to see this, currently my CLI api is very much suffering from this not being implemented ^^'

@epage
Copy link
Member

epage commented Sep 3, 2022

In the upcoming clap v4, we at least have a Builder example for position-sensitive options: https://github.com/clap-rs/clap/blob/master/examples/find.rs. That can be re-worked to handle grouping of arguments. The derive reference describes how to integrate sections of builder code into wider derive uses.

notmandatory added a commit to bitcoindevkit/bdk-cli that referenced this issue Nov 29, 2022
b20a140 Fix some clippies.. (rajarshimaitra)
1e21cc7 Fix wasm (rajarshimaitra)
ba1c165 Update cargo lock. (rajarshimaitra)
02e71b0 Reverse the recipient parsing string (rajarshimaitra)
95719b0 Remove vector of string from compact_filters options. (rajarshimaitra)
9fb651a Update wasm build to 1.57 in CI (rajarshimaitra)
7e87a65 update MSRV to 1.57 (rajarshimaitra)
3dcc269 Update Cargo lock (rajarshimaitra)
f8c0f2e Update recipient parsing in handlers. (rajarshimaitra)
5bbc45a Move from structopt to clap (rajarshimaitra)

Pull request description:

  <!-- You can erase any parts of this template not applicable to your Pull Request. -->

  ### Description

  Fixes #113.

  This is an attempt to migrate from `structopt` to `clap v0.3` which provides very similar kind of derives as `structopt`. Changes are straight forward. But this comes with few more problems.

   - with clap `v3.2.22` the MSRV pushes up to `1.57.0`.. The last clap of MSRV `1.56.0` was `clap 3.2.5`.. But even that doesn't seem to be working at MSRV `1.56.0` anymore, as bunch of underlying lib has upgraded.

   - `clap v3.0` doesn't seem to support custom vector parsing well, reported here clap-rs/clap#1704. This is required for `recipient` parsing which is a `Vec<(Script, u32)>`.  Workaround for that is to use vecs of strings and parse them at runtime in `create_tx` handler. Included in the PR.

  ### Notes to the reviewers

  `structopt` is currently freezed at `clap 2.0` and doesn't seem to intend on updating and currently its has unmaintained vulnerability. And `clap v3.0` onward seems to replacing everything that `structopt` did before.  So this means we should also look for ways to migrate from `strcutopt` to `clap`.. But `clap` seemed to have moved ahead than our MSRV `1.56.0`.. So we need to take up a call on that.. Opened this PR to facilitate that discussion..

  This is draft until we figure what to do..

  ### Checklists

  #### All Submissions:

  * [x] I've signed all my commits
  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk-cli/blob/master/CONTRIBUTING.md)
  * [x] I ran `cargo fmt` and `cargo clippy` before committing

ACKs for top commit:
  notmandatory:
    ACK b20a140

Tree-SHA512: de3511c1531185064e3c328f0c966c3e87294b7c6d07a89ba9f64e49f7c8b8ccaef0915f49fc6721b738f91a0128b30eeb61147a2da174ccac9abab094ab6798
@Isaac-Leonard
Copy link

Has there been any changes with this, I've been struggling to implement it for a few hours but it doesn't seem to be possible.

@epage
Copy link
Member

epage commented Mar 21, 2024

A thought I had: combining command-chaining (#2222) with subcommand flags would allow users to create something like this (when there is a struct-start argument)

@cavivie
Copy link

cavivie commented Sep 12, 2024

Uhh, fortunately I encountered this issue and found it opened here, unfortunately the solution is not yet in sight.

I don't think command-chaining is a solution to this issue, it just seems like a workaround.

An argument itself means a single value, but how a single value is parsed actually depends on the value parser of the argument itself. I expect Vec to be treated as a special object. If the argument has the "argument group" flag, then an argument parser (let's call it ArgGroupValueParser) is provided to parse the value after the argument into a Vec element type. Then we require that the elements in Vec can be parsed by ArgGroupValueParser. In fact, this requires clap to provide a capability to parse the value of the argument itself as a command line argument, which seems to have the meaning of Matryoshka doll.

@cavivie
Copy link

cavivie commented Sep 12, 2024

At present, without affecting the structural design, I use JSON Array strings to parse into all Vec<Struct> parameters (forHashMap, JSON Object is used), which makes the cli look weird. I hope a solution can be discussed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-parsing Area: Parser's logic and needs it changed somehow. C-enhancement Category: Raise on the bar on expectations S-waiting-on-decision Status: Waiting on a go/no-go before implementing
Projects
None yet
Development

No branches or pull requests

8 participants