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

Add support for comma separated values #348

Closed
kbknapp opened this issue Nov 17, 2015 · 9 comments
Closed

Add support for comma separated values #348

kbknapp opened this issue Nov 17, 2015 · 9 comments
Labels
A-parsing Area: Parser's logic and needs it changed somehow. E-medium Call for participation: Experience needed to fix: Medium / intermediate
Milestone

Comments

@kbknapp
Copy link
Member

kbknapp commented Nov 17, 2015

Example, these should all parse to exactly the same matches

$ prog --opt val1 val2 val3 
$ prog --opt val1,val2,val3
$ prog --opt=val1,val2,val3
$ prog -oval1,val2,val3
@kbknapp kbknapp added A-parsing Area: Parser's logic and needs it changed somehow. D: easy E-medium Call for participation: Experience needed to fix: Medium / intermediate labels Nov 17, 2015
@psyomn
Copy link

psyomn commented Nov 23, 2015

I'm going to start looking at this. I'll ping you back if I have questions!

@kbknapp
Copy link
Member Author

kbknapp commented Nov 23, 2015

👍

@kbknapp
Copy link
Member Author

kbknapp commented Nov 23, 2015

You should be able to look at the App:::add_value_to_arg in src/app/mod.rs to start

@psyomn
Copy link

psyomn commented Nov 30, 2015

Hey! Sorry for the late reply, it's been a little busy for me lately. I had the chance to work on this a little this weekend. I have a few questions!

Would introducing something like this be on the right path (notice take_list) ?

let matches = App::new("MyApp")
        .arg(Arg::with_name("input")
                        .help("the input file to use")
                        .takes_value(true)
                        .short("i")
                        .long("input")
                        .multiple(true)
                        .takes_list(true)
                        .required(true)
                        .conflicts_with("output"))
        .get_matches();

So let's say this implementation works. When we're asking for value_of of input for this example, we're expecting a &str right? I was thinking of returning a vector, but that doesn't seem that it would be going in the right direction. So in fact, we'd expect to get a string instead of a list, and leave the user to deal with said input.

@kbknapp
Copy link
Member Author

kbknapp commented Nov 30, 2015

No problem! 👍

Assuming the user runs MyApp -i val1,vla2,val3 calling value_of("input") would return val1 which isn't super useful, but that's how it works with other multiple values so this is fine. calling values_of("intput") you'll get a Vec<&str> which contains ["val1", "val2", "val3"]. This is actually something that needs to be changed (and I'll put the issue in for it in just a minute), it should return a &[&str].

For this issue, you shouldn't have to worry about anything in arg_matches.rs, it should only require changes src/app/mod.rs:add_val_to_arg and maybe src/args/arg_matcher.rs where you split the value by ,.

But you bring up a very interesting idea, that I like. Changing your takes_list(true) to allow_value_delims(bool) (and defaults to true). This would assume most people would be OK with -i val1,val2,val3 parsing to 3 values, but also give them the option to parse -i val1,val2,val3 as a single value if their application requires such. (i.e. most users would never have to use .allow_value_delims(true) because that's the default, they only need to use .allow_value_delims(false) if they want to turn that feature OFF)

Another setting that would go along very nicely with this is making the delimiter selectable but defaulting to , (comma). This would require something like value_delim(char) and allow things like -i val1;val2;val3 if .value_delim(';') was used.

For the above two features though I will create two issues, instead of trying to solve all three in this one issue.

@kbknapp kbknapp added this to the 1.6.0 milestone Dec 18, 2015
@kbknapp
Copy link
Member Author

kbknapp commented Dec 18, 2015

@psyomn How's it going?

@psyomn
Copy link

psyomn commented Dec 18, 2015

Hey, sorry for disappearing! I haven't had a chance to work on clap these
last weeks because I'm occupied with marking, trying to publish a paper
with a teacher, and on a job hunt as well! I will have time during holidays
if that's cool. If I'm holding people back, then I'll snatch another issue
when I'm not as pressed for time.

Thank you for the extensive writeup by the way. I wanted to express my
thanks, but didn't want to add too much noise to the issue tracker.

Hope everything is well on your end!

On Fri, Dec 18, 2015 at 3:58 AM, Kevin K. notifications@github.com wrote:

@psyomn https://github.com/psyomn How's it going?


Reply to this email directly or view it on GitHub
#348 (comment).

@kbknapp
Copy link
Member Author

kbknapp commented Dec 18, 2015

No worries :) Just wanted to check up. We may go ahead and implement this, but if its still open when your time frees up feel free to keep at it and put in a PR or ask if you have questions 😉

@kbknapp kbknapp modified the milestones: v2.0, 1.6.0 Jan 23, 2016
kbknapp added a commit that referenced this issue Jan 26, 2016
This commit adds support for values separated by commas such as
--option=val1,val2,val3. It also includes support for uses
without the equals and shorts (both with and without)

--option=val1,val2
--option val1,val2
-oval1,val2
-o=val1,val2
-o val1,val2

Closes #348
@kbknapp
Copy link
Member Author

kbknapp commented Jan 26, 2016

Closed with #387

@kbknapp kbknapp closed this as completed Jan 26, 2016
kbknapp added a commit that referenced this issue Jan 28, 2016
This commit adds support for values separated by commas such as
--option=val1,val2,val3. It also includes support for uses
without the equals and shorts (both with and without)

--option=val1,val2
--option val1,val2
-oval1,val2
-o=val1,val2
-o val1,val2

Closes #348
kbknapp added a commit that referenced this issue Jan 28, 2016
This commit adds support for values separated by commas such as
--option=val1,val2,val3. It also includes support for uses
without the equals and shorts (both with and without)

--option=val1,val2
--option val1,val2
-oval1,val2
-o=val1,val2
-o val1,val2

Closes #348
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. E-medium Call for participation: Experience needed to fix: Medium / intermediate
Projects
None yet
Development

No branches or pull requests

2 participants