Skip to content

Commit

Permalink
imp(Value Delimiters): changes the default value delimiter rules
Browse files Browse the repository at this point in the history
Prior to this change, values were always delimited by default. This was causing issues with code
where the arg had a single value, and contained valid commas and shouldn't be delimited. This
commit changes the rules slightly so that values are not delimited by default, *unless* one of the
methods which implies multiple values was used (max_values, value_names, etc.).

This means single value args should *not* be delimited by default. If one wishes to use the old
way, they can add `Arg::use_delimiter(true)` to such code.

Closes #655
  • Loading branch information
kbknapp committed Sep 10, 2016
1 parent 9881a4a commit f9e6925
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 19 deletions.
43 changes: 36 additions & 7 deletions src/args/arg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ impl<'a, 'b> Default for Arg<'a, 'b> {
validator: None,
overrides: None,
settings: ArgFlags::new(),
val_delim: Some(','),
val_delim: None,
default_val: None,
disp_ord: 999,
r_unless: None,
Expand Down Expand Up @@ -1277,8 +1277,12 @@ impl<'a, 'b> Arg<'a, 'b> {
/// [option]: ./struct.Arg.html#method.takes_value
/// [`Arg::number_of_values(1)`]: ./struct.Arg.html#method.number_of_values
/// [`Arg::multiple(true)`]: ./struct.Arg.html#method.multiple
pub fn multiple(self, multi: bool) -> Self {
pub fn multiple(mut self, multi: bool) -> Self {
if multi {
if self.settings.is_set(ArgSettings::ValueDelimiterNotSet) &&
self.settings.is_set(ArgSettings::TakesValue) {
self = self.use_delimiter(true);
}
self.set(ArgSettings::Multiple)
} else {
self.unset(ArgSettings::Multiple)
Expand Down Expand Up @@ -1649,6 +1653,13 @@ impl<'a, 'b> Arg<'a, 'b> {
/// ```
/// [`Arg::multiple(true)`]: ./struct.Arg.html#method.multiple
pub fn number_of_values(mut self, qty: u64) -> Self {
if qty > 1 && self.settings.is_set(ArgSettings::ValueDelimiterNotSet) {
self.unsetb(ArgSettings::ValueDelimiterNotSet);
self.setb(ArgSettings::UseValueDelimiter);
} else {
self = self.use_delimiter(false);
}
self.setb(ArgSettings::TakesValue);
self.num_vals = Some(qty);
self
}
Expand Down Expand Up @@ -1747,6 +1758,13 @@ impl<'a, 'b> Arg<'a, 'b> {
/// ```
/// [`Arg::multiple(true)`]: ./struct.Arg.html#method.multiple
pub fn max_values(mut self, qty: u64) -> Self {
if qty > 1 && self.settings.is_set(ArgSettings::ValueDelimiterNotSet) {
self.unsetb(ArgSettings::ValueDelimiterNotSet);
self.setb(ArgSettings::UseValueDelimiter);
} else {
self = self.use_delimiter(false);
}
self.setb(ArgSettings::TakesValue);
self.max_vals = Some(qty);
self
}
Expand Down Expand Up @@ -1857,11 +1875,16 @@ impl<'a, 'b> Arg<'a, 'b> {
/// [`Arg::value_delimiter`]: ./struct.Arg.html#method.value_delimiter
pub fn use_delimiter(mut self, d: bool) -> Self {
if d {
self.val_delim = Some(',');
self.set(ArgSettings::UseValueDelimiter)
if self.val_delim.is_none() {
self.val_delim = Some(',');
}
self.setb(ArgSettings::TakesValue);
self.setb(ArgSettings::UseValueDelimiter);
self.unset(ArgSettings::ValueDelimiterNotSet)
} else {
self.val_delim = None;
self.unset(ArgSettings::UseValueDelimiter)
self.unsetb(ArgSettings::UseValueDelimiter);
self.unset(ArgSettings::ValueDelimiterNotSet)
}
}

Expand Down Expand Up @@ -1939,6 +1962,7 @@ impl<'a, 'b> Arg<'a, 'b> {
/// ```
pub fn require_delimiter(mut self, d: bool) -> Self {
if d {
self.unsetb(ArgSettings::ValueDelimiterNotSet);
self.setb(ArgSettings::UseValueDelimiter);
self.set(ArgSettings::RequireDelimiter)
} else {
Expand Down Expand Up @@ -1972,8 +1996,9 @@ impl<'a, 'b> Arg<'a, 'b> {
/// [`Arg::use_delimiter(true)`]: ./struct.Arg.html#method.use_delimiter
/// [`Arg::takes_value(true)`]: ./struct.Arg.html#method.takes_value
pub fn value_delimiter(mut self, d: &str) -> Self {
self = self.set(ArgSettings::TakesValue);
self = self.set(ArgSettings::UseValueDelimiter);
self.unsetb(ArgSettings::ValueDelimiterNotSet);
self.setb(ArgSettings::TakesValue);
self.setb(ArgSettings::UseValueDelimiter);
self.val_delim = Some(d.chars()
.nth(0)
.expect("Failed to get value_delimiter from arg"));
Expand Down Expand Up @@ -2041,6 +2066,10 @@ impl<'a, 'b> Arg<'a, 'b> {
/// [`Arg::multiple(true)`]: ./struct.Arg.html#method.multiple
pub fn value_names(mut self, names: &[&'b str]) -> Self {
self.setb(ArgSettings::TakesValue);
if self.settings.is_set(ArgSettings::ValueDelimiterNotSet) {
self.unsetb(ArgSettings::ValueDelimiterNotSet);
self.setb(ArgSettings::UseValueDelimiter);
}
if let Some(ref mut vals) = self.val_names {
let mut l = vals.len();
for s in names {
Expand Down
29 changes: 17 additions & 12 deletions src/args/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,17 @@ use std::str::FromStr;

bitflags! {
flags Flags: u16 {
const REQUIRED = 0b0000000001,
const MULTIPLE = 0b0000000010,
const EMPTY_VALS = 0b0000000100,
const GLOBAL = 0b0000001000,
const HIDDEN = 0b0000010000,
const TAKES_VAL = 0b0000100000,
const USE_DELIM = 0b0001000000,
const NEXT_LINE_HELP = 0b0010000000,
const R_UNLESS_ALL = 0b0100000000,
const REQ_DELIM = 0b1000000000,
const REQUIRED = 0b00000000001,
const MULTIPLE = 0b00000000010,
const EMPTY_VALS = 0b00000000100,
const GLOBAL = 0b00000001000,
const HIDDEN = 0b00000010000,
const TAKES_VAL = 0b00000100000,
const USE_DELIM = 0b00001000000,
const NEXT_LINE_HELP = 0b00010000000,
const R_UNLESS_ALL = 0b00100000000,
const REQ_DELIM = 0b01000000000,
const DELIM_NOT_SET = 0b10000000000,
}
}

Expand All @@ -36,13 +37,14 @@ impl ArgFlags {
UseValueDelimiter => USE_DELIM,
NextLineHelp => NEXT_LINE_HELP,
RequiredUnlessAll => R_UNLESS_ALL,
RequireDelimiter => REQ_DELIM
RequireDelimiter => REQ_DELIM,
ValueDelimiterNotSet => DELIM_NOT_SET
}
}

impl Default for ArgFlags {
fn default() -> Self {
ArgFlags(EMPTY_VALS | USE_DELIM)
ArgFlags(EMPTY_VALS | DELIM_NOT_SET)
}
}

Expand Down Expand Up @@ -74,6 +76,8 @@ pub enum ArgSettings {
RequireDelimiter,
#[doc(hidden)]
RequiredUnlessAll,
#[doc(hidden)]
ValueDelimiterNotSet,
}

impl FromStr for ArgSettings {
Expand All @@ -90,6 +94,7 @@ impl FromStr for ArgSettings {
"nextlinehelp" => Ok(ArgSettings::NextLineHelp),
"requiredunlessall" => Ok(ArgSettings::RequiredUnlessAll),
"requiredelimiter" => Ok(ArgSettings::RequireDelimiter),
"valuedelimiternotset" => Ok(ArgSettings::ValueDelimiterNotSet),
_ => Err("unknown ArgSetting, cannot convert from str".to_owned()),
}
}
Expand Down
7 changes: 7 additions & 0 deletions src/usage_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,13 @@ impl<'a> UsageParser<'a> {
if dot_counter == 3 {
debugln!("setting multiple");
arg.setb(ArgSettings::Multiple);
if arg.settings.is_set(ArgSettings::TakesValue) {
arg.setb(ArgSettings::UseValueDelimiter);
arg.unsetb(ArgSettings::ValueDelimiterNotSet);
if arg.val_delim.is_none() {
arg.val_delim = Some(',');
}
}
self.prev = UsageToken::Multiple;
self.pos += 1;
break;
Expand Down

0 comments on commit f9e6925

Please sign in to comment.