Llewellyn
/
exa
mirror of https://github.com/Llewellynvdm/exa.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Tie value suggestions to their arguments 

This commit changes the definition of Arg so that it knows about which values it can accept, and can display them in the help text. They were already being shown in the help text, but they were passed in separately, so one argument could show two different sets of options if it wanted. Now, the argument itself knows whether there are suggestions, so it doesn’t have to be passed in separately.

This means we can use it for other things, including listing choices when an option is missed out, without having to repeat the list.

With Misfire::BadArgument now only having two fields, it’s not worth using a constructor function anymore.
This commit is contained in:
Benjamin Sago 2017-09-14 01:22:37 +01:00
parent 1824313cda
commit a8bf990674
7 changed files with 88 additions and 86 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
src/options/filter.rs
View File

@ -21,10 +21,6 @@ impl FileFilter {
}
}
const SORTS: &[&str] = &[ "name", "Name", "size", "extension",
"Extension", "modified", "accessed",
"created", "inode", "type", "none" ];
impl SortField {
/// Determines which sort field to use based on the `--sort` argument.
@ -75,7 +71,7 @@ impl SortField {
Ok(SortField::Unsorted)
}
else {
Err(Misfire::bad_argument(&flags::SORT, word, SORTS))
Err(Misfire::BadArgument(&flags::SORT, word.into()))
}
}
}
@ -185,12 +181,6 @@ mod test {
use options::flags;
use options::parser::Flag;
pub fn os(input: &'static str) -> OsString {
let mut os = OsString::new();
os.push(input);
os
}
macro_rules! test {
($name:ident: $type:ident <- $inputs:expr; $stricts:expr => $result:expr) => {
#[test]
@ -226,7 +216,7 @@ mod test {
test!(age: SortField <- ["-sage"]; Both => Ok(SortField::ModifiedAge));
// Errors
test!(error: SortField <- ["--sort=colour"]; Both => Err(Misfire::bad_argument(&flags::SORT, &os("colour"), super::SORTS)));
test!(error: SortField <- ["--sort=colour"]; Both => Err(Misfire::BadArgument(&flags::SORT, OsString::from("colour"))));
// Overriding
test!(overridden: SortField <- ["--sort=cr", "--sort", "mod"]; Last => Ok(SortField::ModifiedDate));

22
src/options/flags.rs
View File

@ -1,4 +1,4 @@
use options::parser::{Arg, Args, TakesValue};
use options::parser::{Arg, Args, Values, TakesValue};
// exa options
@ -14,8 +14,9 @@ pub static RECURSE: Arg = Arg { short: Some(b'R'), long: "recurse", takes_valu
pub static TREE: Arg = Arg { short: Some(b'T'), long: "tree", takes_value: TakesValue::Forbidden };
pub static CLASSIFY: Arg = Arg { short: Some(b'F'), long: "classify", takes_value: TakesValue::Forbidden };
pub static COLOR: Arg = Arg { short: None, long: "color", takes_value: TakesValue::Necessary };
pub static COLOUR: Arg = Arg { short: None, long: "colour", takes_value: TakesValue::Necessary };
pub static COLOR: Arg = Arg { short: None, long: "color", takes_value: TakesValue::Necessary(Some(COLOURS)) };
pub static COLOUR: Arg = Arg { short: None, long: "colour", takes_value: TakesValue::Necessary(Some(COLOURS)) };
const COLOURS: &[&str] = &["always", "auto", "never"];
pub static COLOR_SCALE: Arg = Arg { short: None, long: "color-scale", takes_value: TakesValue::Forbidden };
pub static COLOUR_SCALE: Arg = Arg { short: None, long: "colour-scale", takes_value: TakesValue::Forbidden };
@ -23,11 +24,14 @@ pub static COLOUR_SCALE: Arg = Arg { short: None, long: "colour-scale", takes_va
// filtering and sorting options
pub static ALL: Arg = Arg { short: Some(b'a'), long: "all", takes_value: TakesValue::Forbidden };
pub static LIST_DIRS: Arg = Arg { short: Some(b'd'), long: "list-dirs", takes_value: TakesValue::Forbidden };
pub static LEVEL: Arg = Arg { short: Some(b'L'), long: "level", takes_value: TakesValue::Necessary };
pub static LEVEL: Arg = Arg { short: Some(b'L'), long: "level", takes_value: TakesValue::Necessary(None) };
pub static REVERSE: Arg = Arg { short: Some(b'r'), long: "reverse", takes_value: TakesValue::Forbidden };
pub static SORT: Arg = Arg { short: Some(b's'), long: "sort", takes_value: TakesValue::Necessary };
pub static IGNORE_GLOB: Arg = Arg { short: Some(b'I'), long: "ignore-glob", takes_value: TakesValue::Necessary };
pub static SORT: Arg = Arg { short: Some(b's'), long: "sort", takes_value: TakesValue::Necessary(Some(SORTS)) };
pub static IGNORE_GLOB: Arg = Arg { short: Some(b'I'), long: "ignore-glob", takes_value: TakesValue::Necessary(None) };
pub static DIRS_FIRST: Arg = Arg { short: None, long: "group-directories-first", takes_value: TakesValue::Forbidden };
const SORTS: Values = &[ "name", "Name", "size", "extension",
"Extension", "modified", "accessed",
"created", "inode", "type", "none" ];
// display options
pub static BINARY: Arg = Arg { short: Some(b'b'), long: "binary", takes_value: TakesValue::Forbidden };
@ -38,10 +42,12 @@ pub static INODE: Arg = Arg { short: Some(b'i'), long: "inode", takes_
pub static LINKS: Arg = Arg { short: Some(b'H'), long: "links", takes_value: TakesValue::Forbidden };
pub static MODIFIED: Arg = Arg { short: Some(b'm'), long: "modified", takes_value: TakesValue::Forbidden };
pub static BLOCKS: Arg = Arg { short: Some(b'S'), long: "blocks", takes_value: TakesValue::Forbidden };
pub static TIME: Arg = Arg { short: Some(b't'), long: "time", takes_value: TakesValue::Necessary };
pub static TIME: Arg = Arg { short: Some(b't'), long: "time", takes_value: TakesValue::Necessary(Some(TIMES)) };
pub static ACCESSED: Arg = Arg { short: Some(b'u'), long: "accessed", takes_value: TakesValue::Forbidden };
pub static CREATED: Arg = Arg { short: Some(b'U'), long: "created", takes_value: TakesValue::Forbidden };
pub static TIME_STYLE: Arg = Arg { short: None, long: "time-style", takes_value: TakesValue::Necessary };
pub static TIME_STYLE: Arg = Arg { short: None, long: "time-style", takes_value: TakesValue::Necessary(Some(TIME_STYLES)) };
const TIMES: Values = &["modified", "accessed", "created"];
const TIME_STYLES: Values = &["default", "long-iso", "full-iso", "iso"];
// optional feature options
pub static GIT: Arg = Arg { short: None, long: "git", takes_value: TakesValue::Forbidden };

33
src/options/misfire.rs
View File

@ -1,4 +1,4 @@
use std::ffi::{OsStr, OsString};
use std::ffi::OsString;
use std::fmt;
use std::num::ParseIntError;
@ -17,7 +17,7 @@ pub enum Misfire {
InvalidOptions(ParseError),
/// The user supplied an illegal choice to an Argument.
BadArgument(&'static Arg, OsString, Choices),
BadArgument(&'static Arg, OsString),
/// The user asked for help. This isn’t strictly an error, which is why
/// this enum isn’t named Error!
@ -60,14 +60,6 @@ impl Misfire {
_ => true,
}
}
/// The Misfire that happens when an option gets given the wrong
/// argument. This has to use one of the `getopts` failure
/// variants--it’s meant to take just an option name, rather than an
/// option *and* an argument, but it works just as well.
pub fn bad_argument(option: &'static Arg, otherwise: &OsStr, legal: &'static [&'static str]) -> Misfire {
Misfire::BadArgument(option, otherwise.to_os_string(), Choices(legal))
}
}
impl From<glob::PatternError> for Misfire {
@ -78,10 +70,18 @@ impl From<glob::PatternError> for Misfire {
impl fmt::Display for Misfire {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
use options::parser::TakesValue;
use self::Misfire::*;
match *self {
BadArgument(ref a, ref b, ref c) => write!(f, "Option {} has no {:?} setting ({})", a, b, c),
BadArgument(ref arg, ref attempt) => {
if let TakesValue::Necessary(Some(values)) = arg.takes_value {
write!(f, "Option {} has no {:?} setting ({})", arg, attempt, Choices(values))
}
else {
write!(f, "Option {} has no {:?} setting", arg, attempt)
}
},
InvalidOptions(ref e) => write!(f, "{}", e),
Help(ref text) => write!(f, "{}", text),
Version(ref version) => write!(f, "{}", version),
@ -103,17 +103,18 @@ impl fmt::Display for ParseError {
use self::ParseError::*;
match *self {
NeedsValue { ref flag } => write!(f, "Flag {} needs a value", flag),
ForbiddenValue { ref flag } => write!(f, "Flag {} cannot take a value", flag),
UnknownShortArgument { ref attempt } => write!(f, "Unknown argument -{}", *attempt as char),
UnknownArgument { ref attempt } => write!(f, "Unknown argument --{}", attempt.to_string_lossy()),
NeedsValue { ref flag, values: None } => write!(f, "Flag {} needs a value", flag),
NeedsValue { ref flag, values: Some(cs) } => write!(f, "Flag {} needs a value ({})", flag, Choices(cs)),
ForbiddenValue { ref flag } => write!(f, "Flag {} cannot take a value", flag),
UnknownShortArgument { ref attempt } => write!(f, "Unknown argument -{}", *attempt as char),
UnknownArgument { ref attempt } => write!(f, "Unknown argument --{}", attempt.to_string_lossy()),
}
}
}
impl Misfire {
pub fn suggestion(&self) -> Option<&'static str> {
if let Misfire::BadArgument(ref time, ref r, ref _choices) = *self {
if let Misfire::BadArgument(ref time, ref r) = *self {
if *time == &flags::TIME && r == "r" {
return Some("To sort newest files first, try \"--sort modified\", or just \"-stime\"");
}

69
src/options/parser.rs
View File

@ -43,6 +43,13 @@ pub type ShortArg = u8;
/// which flag it was.
pub type LongArg = &'static str;
/// A **list of values** that an option can have, to be displayed when the
/// user enters an invalid one or skips it.
///
/// This is literally just help text, and won’t be used to validate a value to
/// see if it’s correct.
pub type Values = &'static [&'static str];
/// A **flag** is either of the two argument types, because they have to
/// be in the same array together.
#[derive(PartialEq, Debug, Clone)]
@ -88,7 +95,9 @@ pub enum Strictness {
pub enum TakesValue {
/// This flag has to be followed by a value.
Necessary,
/// If there’s a fixed set of possible values, they can be printed out
/// with the error text.
Necessary(Option<Values>),
/// This flag will throw an error if there’s a value after it.
Forbidden,
@ -171,8 +180,8 @@ impl Args {
let arg = self.lookup_long(before)?;
let flag = Flag::Long(arg.long);
match arg.takes_value {
Necessary => result_flags.push((flag, Some(after))),
Forbidden => return Err(ParseError::ForbiddenValue { flag })
Necessary(_) => result_flags.push((flag, Some(after))),
Forbidden => return Err(ParseError::ForbiddenValue { flag })
}
}
@ -182,13 +191,13 @@ impl Args {
let arg = self.lookup_long(long_arg_name)?;
let flag = Flag::Long(arg.long);
match arg.takes_value {
Forbidden => result_flags.push((flag, None)),
Necessary => {
Forbidden => result_flags.push((flag, None)),
Necessary(values) => {
if let Some(next_arg) = inputs.next() {
result_flags.push((flag, Some(next_arg)));
}
else {
return Err(ParseError::NeedsValue { flag })
return Err(ParseError::NeedsValue { flag, values })
}
}
}
@ -210,7 +219,7 @@ impl Args {
// -abcdx= => error
//
// There’s no way to give two values in a cluster like this:
// it's an error if any of the first set of arguments actually
// it’s an error if any of the first set of arguments actually
// takes a value.
if let Some((before, after)) = split_on_equals(short_arg) {
let (arg_with_value, other_args) = before.as_bytes().split_last().unwrap();
@ -220,8 +229,8 @@ impl Args {
let arg = self.lookup_short(*byte)?;
let flag = Flag::Short(*byte);
match arg.takes_value {
Forbidden => result_flags.push((flag, None)),
Necessary => return Err(ParseError::NeedsValue { flag })
Forbidden => result_flags.push((flag, None)),
Necessary(values) => return Err(ParseError::NeedsValue { flag, values })
}
}
@ -229,15 +238,15 @@ impl Args {
let arg = self.lookup_short(*arg_with_value)?;
let flag = Flag::Short(arg.short.unwrap());
match arg.takes_value {
Necessary => result_flags.push((flag, Some(after))),
Forbidden => return Err(ParseError::ForbiddenValue { flag })
Necessary(_) => result_flags.push((flag, Some(after))),
Forbidden => return Err(ParseError::ForbiddenValue { flag })
}
}
// If there’s no equals, then every character is parsed as
// its own short argument. However, if any of the arguments
// takes a value, then the *rest* of the string is used as
// its value, and if there's no rest of the string, then it
// its value, and if there’s no rest of the string, then it
// uses the next one in the iterator.
//
// -a => ‘a’
@ -251,8 +260,8 @@ impl Args {
let arg = self.lookup_short(*byte)?;
let flag = Flag::Short(*byte);
match arg.takes_value {
Forbidden => result_flags.push((flag, None)),
Necessary => {
Forbidden => result_flags.push((flag, None)),
Necessary(values) => {
if index < bytes.len() - 1 {
let remnants = &bytes[index+1 ..];
result_flags.push((flag, Some(OsStr::from_bytes(remnants))));
@ -262,7 +271,7 @@ impl Args {
result_flags.push((flag, Some(next_arg)));
}
else {
return Err(ParseError::NeedsValue { flag })
return Err(ParseError::NeedsValue { flag, values })
}
}
}
@ -366,7 +375,7 @@ impl<'a> MatchedFlags<'a> {
}
/// Returns the value of the argument that matches the predicate if it
/// was specified, nothing if it wasn't, and an error in strict mode if
/// was specified, nothing if it wasn’t, and an error in strict mode if
/// multiple arguments matched the predicate.
///
/// It’s not possible to tell which flag the value belonged to from this.
@ -407,15 +416,15 @@ impl<'a> MatchedFlags<'a> {
}
/// A problem with the user's input that meant it couldn't be parsed into a
/// A problem with the user’s input that meant it couldn’t be parsed into a
/// coherent list of arguments.
#[derive(PartialEq, Debug)]
pub enum ParseError {
/// A flag that has to take a value was not given one.
NeedsValue { flag: Flag },
NeedsValue { flag: Flag, values: Option<Values> },
/// A flag that can't take a value *was* given one.
/// A flag that can’t take a value *was* given one.
ForbiddenValue { flag: Flag },
/// A short argument, either alone or in a cluster, was not
@ -543,10 +552,13 @@ mod parse_test {
};
}
const SUGGESTIONS: Values = &[ "example" ];
static TEST_ARGS: &[&Arg] = &[
&Arg { short: Some(b'l'), long: "long", takes_value: TakesValue::Forbidden },
&Arg { short: Some(b'v'), long: "verbose", takes_value: TakesValue::Forbidden },
&Arg { short: Some(b'c'), long: "count", takes_value: TakesValue::Necessary }
&Arg { short: Some(b'c'), long: "count", takes_value: TakesValue::Necessary(None) },
&Arg { short: Some(b't'), long: "type", takes_value: TakesValue::Necessary(Some(SUGGESTIONS)) }
];
@ -569,10 +581,15 @@ mod parse_test {
// Long args with values
test!(bad_equals: ["--long=equals"] => error ForbiddenValue { flag: Flag::Long("long") });
test!(no_arg: ["--count"] => error NeedsValue { flag: Flag::Long("count") });
test!(no_arg: ["--count"] => error NeedsValue { flag: Flag::Long("count"), values: None });
test!(arg_equals: ["--count=4"] => frees: [], flags: [ (Flag::Long("count"), Some(OsStr::new("4"))) ]);
test!(arg_then: ["--count", "4"] => frees: [], flags: [ (Flag::Long("count"), Some(OsStr::new("4"))) ]);
// Long args with values and suggestions
test!(no_arg_s: ["--type"] => error NeedsValue { flag: Flag::Long("type"), values: Some(SUGGESTIONS) });
test!(arg_equals_s: ["--type=exa"] => frees: [], flags: [ (Flag::Long("type"), Some(OsStr::new("exa"))) ]);
test!(arg_then_s: ["--type", "exa"] => frees: [], flags: [ (Flag::Long("type"), Some(OsStr::new("exa"))) ]);
// Short args
test!(short: ["-l"] => frees: [], flags: [ (Flag::Short(b'l'), None) ]);
@ -582,13 +599,19 @@ mod parse_test {
// Short args with values
test!(bad_short: ["-l=equals"] => error ForbiddenValue { flag: Flag::Short(b'l') });
test!(short_none: ["-c"] => error NeedsValue { flag: Flag::Short(b'c') });
test!(short_none: ["-c"] => error NeedsValue { flag: Flag::Short(b'c'), values: None });
test!(short_arg_eq: ["-c=4"] => frees: [], flags: [(Flag::Short(b'c'), Some(OsStr::new("4"))) ]);
test!(short_arg_then: ["-c", "4"] => frees: [], flags: [(Flag::Short(b'c'), Some(OsStr::new("4"))) ]);
test!(short_two_together: ["-lctwo"] => frees: [], flags: [(Flag::Short(b'l'), None), (Flag::Short(b'c'), Some(OsStr::new("two"))) ]);
test!(short_two_equals: ["-lc=two"] => frees: [], flags: [(Flag::Short(b'l'), None), (Flag::Short(b'c'), Some(OsStr::new("two"))) ]);
test!(short_two_next: ["-lc", "two"] => frees: [], flags: [(Flag::Short(b'l'), None), (Flag::Short(b'c'), Some(OsStr::new("two"))) ]);
// Short args with values and suggestions
test!(short_none_s: ["-t"] => error NeedsValue { flag: Flag::Short(b't'), values: Some(SUGGESTIONS) });
test!(short_two_together_s: ["-texa"] => frees: [], flags: [(Flag::Short(b't'), Some(OsStr::new("exa"))) ]);
test!(short_two_equals_s: ["-t=exa"] => frees: [], flags: [(Flag::Short(b't'), Some(OsStr::new("exa"))) ]);
test!(short_two_next_s: ["-t", "exa"] => frees: [], flags: [(Flag::Short(b't'), Some(OsStr::new("exa"))) ]);
// Unknown args
test!(unknown_long: ["--quiet"] => error UnknownArgument { attempt: os("quiet") });
@ -619,7 +642,7 @@ mod matches_test {
}
static VERBOSE: Arg = Arg { short: Some(b'v'), long: "verbose", takes_value: TakesValue::Forbidden };
static COUNT: Arg = Arg { short: Some(b'c'), long: "count", takes_value: TakesValue::Necessary };
static COUNT: Arg = Arg { short: Some(b'c'), long: "count", takes_value: TakesValue::Necessary(None) };
test!(short_never: [], has VERBOSE => false);

13
src/options/style.rs
View File

@ -34,7 +34,6 @@ impl Default for TerminalColours {
}
}
const COLOURS: &[&str] = &["always", "auto", "never"];
impl TerminalColours {
@ -56,7 +55,7 @@ impl TerminalColours {
Ok(TerminalColours::Never)
}
else {
Err(Misfire::bad_argument(&flags::COLOR, word, COLOURS))
Err(Misfire::BadArgument(&flags::COLOR, word.into()))
}
}
}
@ -217,12 +216,6 @@ mod terminal_test {
use options::test::parse_for_test;
use options::test::Strictnesses::*;
pub fn os(input: &'static str) -> OsString {
let mut os = OsString::new();
os.push(input);
os
}
static TEST_ARGS: &[&Arg] = &[ &flags::COLOR, &flags::COLOUR ];
macro_rules! test {
@ -260,8 +253,8 @@ mod terminal_test {
test!(no_u_never: ["--color", "never"]; Both => Ok(TerminalColours::Never));
// Errors
test!(no_u_error: ["--color=upstream"]; Both => err Misfire::bad_argument(&flags::COLOR, &os("upstream"), super::COLOURS)); // the error is for --color
test!(u_error: ["--colour=lovers"]; Both => err Misfire::bad_argument(&flags::COLOR, &os("lovers"), super::COLOURS)); // and so is this one!
test!(no_u_error: ["--color=upstream"]; Both => err Misfire::BadArgument(&flags::COLOR, OsString::from("upstream"))); // the error is for --color
test!(u_error: ["--colour=lovers"]; Both => err Misfire::BadArgument(&flags::COLOR, OsString::from("lovers"))); // and so is this one!
// Overriding
test!(overridden_1: ["--colour=auto", "--colour=never"]; Last => Ok(TerminalColours::Never));

21
src/options/view.rs
View File

@ -248,8 +248,6 @@ impl SizeFormat {
}
const TIME_STYLES: &[&str] = &["default", "long-iso", "full-iso", "iso"];
impl TimeFormat {
/// Determine how time should be formatted in timestamp columns.
@ -274,14 +272,12 @@ impl TimeFormat {
Ok(TimeFormat::FullISO)
}
else {
Err(Misfire::bad_argument(&flags::TIME_STYLE, word, TIME_STYLES))
Err(Misfire::BadArgument(&flags::TIME_STYLE, word.into()))
}
}
}
static TIMES: &[&str] = &["modified", "accessed", "created"];
impl TimeTypes {
/// Determine which of a file’s time fields should be displayed for it
@ -320,7 +316,7 @@ impl TimeTypes {
Ok(TimeTypes { accessed: false, modified: false, created: true })
}
else {
Err(Misfire::bad_argument(&flags::TIME, word, TIMES))
Err(Misfire::BadArgument(&flags::TIME, word.into()))
}
}
else if modified || created || accessed {
@ -358,12 +354,6 @@ mod test {
use options::test::parse_for_test;
use options::test::Strictnesses::*;
pub fn os(input: &'static str) -> OsString {
let mut os = OsString::new();
os.push(input);
os
}
static TEST_ARGS: &[&Arg] = &[ &flags::BINARY, &flags::BYTES, &flags::TIME_STYLE,
&flags::TIME, &flags::MODIFIED, &flags::CREATED, &flags::ACCESSED,
&flags::HEADER, &flags::GROUP, &flags::INODE, &flags::GIT,
@ -461,7 +451,6 @@ mod test {
mod time_formats {
use super::*;
use std::ffi::OsStr;
// These tests use pattern matching because TimeFormat doesn’t
// implement PartialEq.
@ -483,7 +472,7 @@ mod test {
test!(nevermore: TimeFormat <- ["--time-style", "long-iso", "--time-style=full-iso"]; Complain => err Misfire::Duplicate(Flag::Long("time-style"), Flag::Long("time-style")));
// Errors
test!(daily: TimeFormat <- ["--time-style=24-hour"]; Both => err Misfire::bad_argument(&flags::TIME_STYLE, OsStr::new("24-hour"), TIME_STYLES));
test!(daily: TimeFormat <- ["--time-style=24-hour"]; Both => err Misfire::BadArgument(&flags::TIME_STYLE, OsString::from("24-hour")));
}
@ -515,8 +504,8 @@ mod test {
test!(time_uu: TimeTypes <- ["-uU"]; Both => Ok(TimeTypes { accessed: true, modified: false, created: true }));
// Errors
test!(time_tea: TimeTypes <- ["--time=tea"]; Both => err Misfire::bad_argument(&flags::TIME, &os("tea"), super::TIMES));
test!(time_ea: TimeTypes <- ["-tea"]; Both => err Misfire::bad_argument(&flags::TIME, &os("ea"), super::TIMES));
test!(time_tea: TimeTypes <- ["--time=tea"]; Both => err Misfire::BadArgument(&flags::TIME, OsString::from("tea")));
test!(time_ea: TimeTypes <- ["-tea"]; Both => err Misfire::BadArgument(&flags::TIME, OsString::from("ea")));
// Overriding
test!(overridden: TimeTypes <- ["-tcr", "-tmod"]; Last => Ok(TimeTypes { accessed: false, modified: true, created: false }));

2
xtests/error_value
View File

@ -1 +1 @@
Flag --time needs a value
Flag --time needs a value (choices: modified, accessed, created)