comma separated features

[tofay]

cargo allows features to be a comma or space separated string. This crate only allows features to be space separated.

(I tried fixing myself, but it's not obvious to me how to fix this with clap's value parser - I expected https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=5c27f13cdd493d2271c589e7464497c3 to work, but I hit a panic in clap.)

[epage]

Yes, this has been lacking.

The reason the proposed fix didn't work is because it assumed the value parser's type would be String since we use Vec to indicate ArgAction::Append (multiple_occurrences).

There is no way to implement this without a breaking change. We either need to make the fields private and do our own splitting or we need to do a Vec<Vec<String>> to get multiple occurrences and splitting.

Ideally, clap would allow having multiple delimiters and it'd just do it for us but unsure how I feel about the API for that from clap's side.

[tofay]

Ideally, clap would allow having multiple delimiters and it'd just do it for us but unsure how I feel about the API for that from clap's side.

Yeah, this doesn't feel like a particular common feature!

There is no way to implement this without a breaking change. We either need to make the fields private and do our own splitting or we need to do a Vec<Vec> to get multiple occurrences and splitting.

I don't think users should have to deal with a Vec<Vec<String>>, so in either case I expect we'd want to make the features field private and I expose via a getter. I'm happy to submit a PR for the former (make field private and do own splitting) if you're ok with that approach?

[epage]

I don't think users should have to deal with a Vec<Vec>, so in either case I expect we'd want to make the features field private and I expose via a getter. I'm happy to submit a PR for the former (make field private and do own splitting) if you're ok with that approach?

Forgot to mention, some downsides

• Can't as easily construct the types from this crate if needed for something like testing
• Clap's future dynamic completion system won't understand the , separator.

We could just keep all the fields public and add a function. The problem will be people who don't realize its there and directly access it.

An alternative would be to rename the field features to raw_features, make its type a String (to make the intent clear), and then provide a features(&self) -> impl Iterator<Item=&'_ str> accessor.

Thoughts?

[tofay]

An alternative would be to rename the field features to raw_features, make its type a String (to make the intent clear), and then provide a features(&self) -> impl Iterator<Item=&'_ str> accessor.

I don't think that works with multiple occurrences?

Can't as easily construct the types from this crate if needed for something like testing

We could add a Features::new for that?

Clap's future dynamic completion system won't understand the , separator.

Wow, dynamic completion sounds awesome! Seems more of a drawback overall that cargo itself won't be able to benefit from that... Maybe adding multiple delimiter support to clap is a better option than a workaround here, so that cargo can also use it?