Skip to content

Commit

Permalink
Change how libgetopts handles options grouped together
Browse files Browse the repository at this point in the history
As soon as an option is found that takes an argument, consume the rest
of the string and store it into i_arg. Previously this would only happen
if the character after the option was not a recognized option.

Addresses issue #16348
  • Loading branch information
wickerwaka committed Aug 15, 2014
1 parent 1d12b6d commit 08d7fc7
Showing 1 changed file with 33 additions and 22 deletions.
55 changes: 33 additions & 22 deletions src/libgetopts/lib.rs
Expand Up @@ -567,7 +567,6 @@ pub fn getopts(args: &[String], optgrps: &[OptGroup]) -> Result {
}
} else {
let mut j = 1;
let mut last_valid_opt_id = None;
names = Vec::new();
while j < curlen {
let range = cur.as_slice().char_range_at(j);
Expand All @@ -580,27 +579,24 @@ pub fn getopts(args: &[String], optgrps: &[OptGroup]) -> Result {
interpreted correctly
*/

match find_opt(opts.as_slice(), opt.clone()) {
Some(id) => last_valid_opt_id = Some(id),
None => {
let arg_follows =
last_valid_opt_id.is_some() &&
match opts[last_valid_opt_id.unwrap()]
.hasarg {

Yes | Maybe => true,
No => false
};
if arg_follows && j < curlen {
i_arg = Some(cur.as_slice()
.slice(j, curlen).to_string());
break;
} else {
last_valid_opt_id = None;
}
}
}
let opt_id = match find_opt(opts.as_slice(), opt.clone()) {
Some(id) => id,
None => return Err(UnrecognizedOption(opt.to_string()))
};

names.push(opt);

let arg_follows = match opts[opt_id].hasarg {
Yes | Maybe => true,
No => false
};

if arg_follows && range.next < curlen {
i_arg = Some(cur.as_slice()
.slice(range.next, curlen).to_string());
break;
}

j = range.next;
}
}
Expand All @@ -613,7 +609,7 @@ pub fn getopts(args: &[String], optgrps: &[OptGroup]) -> Result {
};
match opts[optid].hasarg {
No => {
if !i_arg.is_none() {
if name_pos == names.len() && !i_arg.is_none() {
return Err(UnexpectedArgument(nm.to_string()));
}
vals.get_mut(optid).push(Given);
Expand Down Expand Up @@ -1437,6 +1433,21 @@ mod tests {

}

#[test]
fn test_nospace_conflict() {
let args = vec!("-vvLverbose".to_string(), "-v".to_string() );
let opts = vec!(optmulti("L", "", "library directory", "LIB"),
optflagmulti("v", "verbose", "Verbose"));
let matches = &match getopts(args.as_slice(), opts.as_slice()) {
result::Ok(m) => m,
result::Err(e) => fail!( "{}", e )
};
assert!(matches.opts_present(["L".to_string()]));
assert_eq!(matches.opts_str(["L".to_string()]).unwrap(), "verbose".to_string());
assert!(matches.opts_present(["v".to_string()]));
assert_eq!(3, matches.opt_count("v"));
}

#[test]
fn test_long_to_short() {
let mut short = Opt {
Expand Down

5 comments on commit 08d7fc7

@bors
Copy link
Contributor

@bors bors commented on 08d7fc7 Aug 22, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saw approval from brson
at wickerwaka@08d7fc7

@bors
Copy link
Contributor

@bors bors commented on 08d7fc7 Aug 22, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging wickerwaka/rust/getopt-16348 = 08d7fc7 into auto

@bors
Copy link
Contributor

@bors bors commented on 08d7fc7 Aug 22, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wickerwaka/rust/getopt-16348 = 08d7fc7 merged ok, testing candidate = 711d710

@bors
Copy link
Contributor

@bors bors commented on 08d7fc7 Aug 22, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fast-forwarding master to auto = 711d710

Please sign in to comment.