Make View command-line args position-dependent

This commit changes the way the View (long mode, lines mode, grid mode, etc) is parsed from the command-line arguments.

Previously, it checked for long and long-grid, then tree, then lines, then grid, in that order, no matter which order the arguments were given in on the command-line. Now, it bases the view on whichever argument comes last in the list.

Unfortunately, the options-parsing code for Views is getting really complicated, but I can't see a way to simplify it while retaining the existing functionality.

It also links the parsing of DirAction to the result of parsing the View, so that you can't use tree mode if your view isn't Details. This is to fix an issue where `exa --tree --oneline` would just emit ".", because the DirAction was treating directories as files, and the argument was ".", and the View made it use lines view. Now, the --tree is ignored, as the view isn't Details.

Fixes GH-407 and GH-583.
This commit is contained in:
Benjamin Sago 2020-10-23 23:04:22 +01:00
parent bf3d58aa80
commit fd730e436c
4 changed files with 79 additions and 25 deletions

View File

@ -12,7 +12,7 @@ impl DirAction {
/// There are three possible actions, and they overlap somewhat: the /// There are three possible actions, and they overlap somewhat: the
/// `--tree` flag is another form of recursion, so those two are allowed /// `--tree` flag is another form of recursion, so those two are allowed
/// to both be present, but the `--list-dirs` flag is used separately. /// to both be present, but the `--list-dirs` flag is used separately.
pub fn deduce(matches: &MatchedFlags<'_>) -> Result<Self, OptionsError> { pub fn deduce(matches: &MatchedFlags<'_>, can_tree: bool) -> Result<Self, OptionsError> {
let recurse = matches.has(&flags::RECURSE)?; let recurse = matches.has(&flags::RECURSE)?;
let as_file = matches.has(&flags::LIST_DIRS)?; let as_file = matches.has(&flags::LIST_DIRS)?;
let tree = matches.has(&flags::TREE)?; let tree = matches.has(&flags::TREE)?;
@ -30,7 +30,9 @@ impl DirAction {
} }
} }
if tree { if tree && can_tree {
// Tree is only appropriate in details mode, so this has to
// examine the View, which should have already been deduced by now
Ok(Self::Recurse(RecurseOptions::deduce(matches, true)?)) Ok(Self::Recurse(RecurseOptions::deduce(matches, true)?))
} }
else if recurse { else if recurse {
@ -83,7 +85,7 @@ mod test {
use crate::options::test::Strictnesses::*; use crate::options::test::Strictnesses::*;
static TEST_ARGS: &[&Arg] = &[&flags::RECURSE, &flags::LIST_DIRS, &flags::TREE, &flags::LEVEL ]; static TEST_ARGS: &[&Arg] = &[&flags::RECURSE, &flags::LIST_DIRS, &flags::TREE, &flags::LEVEL ];
for result in parse_for_test($inputs.as_ref(), TEST_ARGS, $stricts, |mf| $type::deduce(mf)) { for result in parse_for_test($inputs.as_ref(), TEST_ARGS, $stricts, |mf| $type::deduce(mf, true)) {
assert_eq!(result, $result); assert_eq!(result, $result);
} }
} }

View File

@ -176,9 +176,9 @@ impl Options {
/// Determines the complete set of options based on the given command-line /// Determines the complete set of options based on the given command-line
/// arguments, after theyve been parsed. /// arguments, after theyve been parsed.
fn deduce<V: Vars>(matches: &MatchedFlags<'_>, vars: &V) -> Result<Self, OptionsError> { fn deduce<V: Vars>(matches: &MatchedFlags<'_>, vars: &V) -> Result<Self, OptionsError> {
let dir_action = DirAction::deduce(matches)?;
let filter = FileFilter::deduce(matches)?;
let view = View::deduce(matches, vars)?; let view = View::deduce(matches, vars)?;
let dir_action = DirAction::deduce(matches, matches!(view.mode, Mode::Details(_)))?;
let filter = FileFilter::deduce(matches)?;
let theme = ThemeOptions::deduce(matches, vars)?; let theme = ThemeOptions::deduce(matches, vars)?;
Ok(Self { dir_action, filter, view, theme }) Ok(Self { dir_action, filter, view, theme })

View File

@ -394,13 +394,21 @@ impl<'a> MatchedFlags<'a> {
else { Err(OptionsError::Duplicate(all[0].0, all[1].0)) } else { Err(OptionsError::Duplicate(all[0].0, all[1].0)) }
} }
else { else {
let any = self.flags.iter().rev() Ok(self.has_where_any(predicate))
.find(|tuple| tuple.1.is_none() && predicate(&tuple.0))
.map(|tuple| &tuple.0);
Ok(any)
} }
} }
/// Returns the first found argument that satisfies the predicate, or
/// nothing if none is found, with strict mode having no effect.
///
/// Youll have to test the resulting flag to see which argument it was.
pub fn has_where_any<P>(&self, predicate: P) -> Option<&Flag>
where P: Fn(&Flag) -> bool {
self.flags.iter().rev()
.find(|tuple| tuple.1.is_none() && predicate(&tuple.0))
.map(|tuple| &tuple.0)
}
// This code could probably be better. // This code could probably be better.
// Both has and get immediately begin with a conditional, which makes // Both has and get immediately begin with a conditional, which makes
// me think the functionality could be moved to inside Strictness. // me think the functionality could be moved to inside Strictness.

View File

@ -19,22 +19,69 @@ impl View {
impl Mode { impl Mode {
pub fn deduce<V: Vars>(matches: &MatchedFlags<'_>, vars: &V) -> Result<Self, OptionsError> {
if matches.has(&flags::LONG)? { /// Determine which viewing mode to use based on the users options.
///
/// As with the other options, arguments are scanned right-to-left and the
/// first flag found is matched, so `exa --oneline --long` will pick a
/// details view, and `exa --long --oneline` will pick the lines view.
///
/// This is complicated a little by the fact that `--grid` and `--tree`
/// can also combine with `--long`, so care has to be taken to use the
pub fn deduce<V: Vars>(matches: &MatchedFlags<'_>, vars: &V) -> Result<Self, OptionsError> {
let flag = matches.has_where_any(|f| f.matches(&flags::LONG) || f.matches(&flags::ONE_LINE)
|| f.matches(&flags::GRID) || f.matches(&flags::TREE));
let flag = match flag {
Some(f) => f,
None => {
Self::strict_check_long_flags(matches)?;
let grid = grid::Options::deduce(matches)?;
return Ok(Self::Grid(grid));
}
};
if flag.matches(&flags::LONG)
|| (flag.matches(&flags::TREE) && matches.has(&flags::LONG)?)
|| (flag.matches(&flags::GRID) && matches.has(&flags::LONG)?)
{
let _ = matches.has(&flags::LONG)?;
let details = details::Options::deduce_long(matches, vars)?; let details = details::Options::deduce_long(matches, vars)?;
if matches.has(&flags::GRID)? { let flag = matches.has_where_any(|f| f.matches(&flags::GRID) || f.matches(&flags::TREE));
if flag.is_some() && flag.unwrap().matches(&flags::GRID) {
let _ = matches.has(&flags::GRID)?;
let grid = grid::Options::deduce(matches)?; let grid = grid::Options::deduce(matches)?;
let row_threshold = RowThreshold::deduce(vars)?; let row_threshold = RowThreshold::deduce(vars)?;
let grid_details = grid_details::Options { grid, details, row_threshold }; let grid_details = grid_details::Options { grid, details, row_threshold };
return Ok(Self::GridDetails(grid_details)); return Ok(Self::GridDetails(grid_details));
} }
else { else {
// the --tree case is handled by the DirAction parser later
return Ok(Self::Details(details)); return Ok(Self::Details(details));
} }
} }
Self::strict_check_long_flags(matches)?;
if flag.matches(&flags::TREE) {
let _ = matches.has(&flags::TREE)?;
let details = details::Options::deduce_tree(matches)?;
return Ok(Self::Details(details));
}
if flag.matches(&flags::ONE_LINE) {
let _ = matches.has(&flags::ONE_LINE)?;
let lines = lines::Options::deduce(matches)?;
return Ok(Self::Lines(lines));
}
let grid = grid::Options::deduce(matches)?;
Ok(Self::Grid(grid))
}
fn strict_check_long_flags(matches: &MatchedFlags<'_>) -> Result<(), OptionsError> {
// If --long hasnt been passed, then check if we need to warn the // If --long hasnt been passed, then check if we need to warn the
// user about flags that wont have any effect. // user about flags that wont have any effect.
if matches.is_strict() { if matches.is_strict() {
@ -53,18 +100,7 @@ impl Mode {
} }
} }
if matches.has(&flags::TREE)? { Ok(())
let details = details::Options::deduce_tree(matches)?;
return Ok(Self::Details(details));
}
if matches.has(&flags::ONE_LINE)? {
let lines = lines::Options::deduce(matches)?;
return Ok(Self::Lines(lines));
}
let grid = grid::Options::deduce(matches)?;
Ok(Self::Grid(grid))
} }
} }
@ -524,7 +560,6 @@ mod test {
// Options that do nothing with --long // Options that do nothing with --long
test!(long_across: Mode <- ["--long", "--across"], None; Last => like Ok(Mode::Details(_))); test!(long_across: Mode <- ["--long", "--across"], None; Last => like Ok(Mode::Details(_)));
test!(long_oneline: Mode <- ["--long", "--oneline"], None; Last => like Ok(Mode::Details(_)));
// Options that do nothing without --long // Options that do nothing without --long
test!(just_header: Mode <- ["--header"], None; Last => like Ok(Mode::Grid(_))); test!(just_header: Mode <- ["--header"], None; Last => like Ok(Mode::Grid(_)));
@ -548,5 +583,14 @@ mod test {
#[cfg(feature = "git")] #[cfg(feature = "git")]
test!(just_git_2: Mode <- ["--git"], None; Complain => err OptionsError::Useless(&flags::GIT, false, &flags::LONG)); test!(just_git_2: Mode <- ["--git"], None; Complain => err OptionsError::Useless(&flags::GIT, false, &flags::LONG));
// Contradictions and combinations
test!(lgo: Mode <- ["--long", "--grid", "--oneline"], None; Both => like Ok(Mode::Lines(_)));
test!(lgt: Mode <- ["--long", "--grid", "--tree"], None; Both => like Ok(Mode::Details(_)));
test!(tgl: Mode <- ["--tree", "--grid", "--long"], None; Both => like Ok(Mode::GridDetails(_)));
test!(tlg: Mode <- ["--tree", "--long", "--grid"], None; Both => like Ok(Mode::GridDetails(_)));
test!(ot: Mode <- ["--oneline", "--tree"], None; Both => like Ok(Mode::Details(_)));
test!(og: Mode <- ["--oneline", "--grid"], None; Both => like Ok(Mode::Grid(_)));
test!(tg: Mode <- ["--tree", "--grid"], None; Both => like Ok(Mode::Grid(_)));
} }
} }