clap v2 #399
clap v2 #399
Conversation
This commit implements the base changes for clap 2.x
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
By using AppSettings::AllowLeadingHyphen values starting with a leading hyphen (such as a negative number) are supported. This setting should be used with caution as it silences certain circumstances which would otherwise be an error (like forgetting a value to an option argument). Closes #385
External subcommands are now supported via the following:
```rust
extern crate clap;
use clap::{App, AppSettings};
fn main() {
// Assume there is a third party subcommand named myprog-subcmd
let m = App::new("myprog")
.setting(AppSettings::AllowExternalSubcommands)
.get_matches_from(vec![
"myprog", "subcmd", "--option", "value", "-fff", "--flag"
]);
// All trailing arguments will be stored under the subcommands sub-matches under a
// value of their runtime name (in this case "subcmd")
match m.subcommand() {
(external, Some(ext_m)) => {
let ext_args: Vec<&str> = ext_m.values_of(external).unwrap().collect();
assert_eq!(ext_args ,["--option", "value", "-fff", "--flag"]);
},
_ => unreachable!()
}
}
```
Closes #372
| } | ||
|
|
||
| fn cause(&self) -> Option<&Error> { | ||
| match self.error_type { | ||
| fn cause(&self) -> Option<&StdError> { |
There was a problem hiding this comment.
cause is provided. It's better not to override it if it's not used for something. I don't know if this is used for something...
There was a problem hiding this comment.
I didn't know it provided a default impl, good catch!
Fixed.
|
I went through all the changes that I could see in GitHub webpage briefly. I am not sure about going through all other changes because I don't know how to comment those. I must say, good work! |
|
@sru I really appreciate the detailed look! All great comments and catches! |
|
@Vinatorul if you wouldn't mind taking a look too since this is a big merge 😉 |
|
@kbknapp ok, I will look into it now. |
| @@ -254,6 +259,7 @@ fn multiple_values_of_option_max_more() { | |||
| .short("o") | |||
| .help("multiple options") | |||
| .takes_value(true) | |||
| .multiple(true) | |||
There was a problem hiding this comment.
max_values implicitly sets multiple to true isn't it?
|
@kbknapp All looks great, I have little questions about tests, somewhere |
|
@Vinatorul nope it doesn't implicitly set it to multiple. I'm making a hard stance between multiple values and multiple occurrences. I think this, although somewhat confusing at first will be more explicit in the long run. |
| } | ||
|
|
||
| #[test] |
There was a problem hiding this comment.
This tests looks like unit tests
There was a problem hiding this comment.
Perhaps, but we can shuffle around where the test lies at a later date when we re-organize some things. 😜
|
@kbknapp Ah, I see you have deleted this, github repo search tricks me. I thought I was searching in |
|
As I see |
|
@kbknapp Is it intentionally that you have added first param (binary name) in every test except group? Or it doesn't matter? |
|
If y'all are good with it, I'll let one of you do the honors and merge whenever ready! 👍 |
|
@kbknapp only homu have this right :) |
|
@Vinatorul I added the binary name in examples so as not to confuse people, but for tests it doesn't really matter. The conflicts overridden is one I'm not 100% sure what the original did...so if it might be one of those technically breaking changes which wasn't supposed to be broken in the first place. So we'll see if anyone files a bug report against it, but I doubt it as POSIX overrides aren't super common. Here goes... @homu r+ |
|
📌 Commit 9054274 has been approved by |
|
⚡ Test exempted - status |
|
@kbknapp that was the bug of old parser. The problem was that old parsed checked conflicts on each interation. As I see now parser firtly will get all arguments, before checking conflicts |
|
@kbknapp I think you should tweet and crete post on reddit about v2 |
Finally ready...