From fd730e436cbf90af1f4b41a7ef134f102105cf47 Mon Sep 17 00:00:00 2001 From: Benjamin Sago Date: Fri, 23 Oct 2020 23:04:22 +0100 Subject: [PATCH] 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. --- src/options/dir_action.rs | 8 +++-- src/options/mod.rs | 4 +-- src/options/parser.rs | 16 ++++++--- src/options/view.rs | 76 ++++++++++++++++++++++++++++++--------- 4 files changed, 79 insertions(+), 25 deletions(-) diff --git a/src/options/dir_action.rs b/src/options/dir_action.rs index c87f131..0cc31fa 100644 --- a/src/options/dir_action.rs +++ b/src/options/dir_action.rs @@ -12,7 +12,7 @@ impl DirAction { /// There are three possible actions, and they overlap somewhat: the /// `--tree` flag is another form of recursion, so those two are allowed /// to both be present, but the `--list-dirs` flag is used separately. - pub fn deduce(matches: &MatchedFlags<'_>) -> Result { + pub fn deduce(matches: &MatchedFlags<'_>, can_tree: bool) -> Result { let recurse = matches.has(&flags::RECURSE)?; let as_file = matches.has(&flags::LIST_DIRS)?; 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)?)) } else if recurse { @@ -83,7 +85,7 @@ mod test { use crate::options::test::Strictnesses::*; 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); } } diff --git a/src/options/mod.rs b/src/options/mod.rs index 2565d28..ae974c4 100644 --- a/src/options/mod.rs +++ b/src/options/mod.rs @@ -176,9 +176,9 @@ impl Options { /// Determines the complete set of options based on the given command-line /// arguments, after they’ve been parsed. fn deduce(matches: &MatchedFlags<'_>, vars: &V) -> Result { - let dir_action = DirAction::deduce(matches)?; - let filter = FileFilter::deduce(matches)?; 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)?; Ok(Self { dir_action, filter, view, theme }) diff --git a/src/options/parser.rs b/src/options/parser.rs index ade5744..f43d44e 100644 --- a/src/options/parser.rs +++ b/src/options/parser.rs @@ -394,13 +394,21 @@ impl<'a> MatchedFlags<'a> { else { Err(OptionsError::Duplicate(all[0].0, all[1].0)) } } else { - let any = self.flags.iter().rev() - .find(|tuple| tuple.1.is_none() && predicate(&tuple.0)) - .map(|tuple| &tuple.0); - Ok(any) + Ok(self.has_where_any(predicate)) } } + /// Returns the first found argument that satisfies the predicate, or + /// nothing if none is found, with strict mode having no effect. + /// + /// You’ll have to test the resulting flag to see which argument it was. + pub fn has_where_any

(&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. // Both ‘has’ and ‘get’ immediately begin with a conditional, which makes // me think the functionality could be moved to inside Strictness. diff --git a/src/options/view.rs b/src/options/view.rs index c8c385e..b9b38ba 100644 --- a/src/options/view.rs +++ b/src/options/view.rs @@ -19,22 +19,69 @@ impl View { impl Mode { - pub fn deduce(matches: &MatchedFlags<'_>, vars: &V) -> Result { - if matches.has(&flags::LONG)? { + /// Determine which viewing mode to use based on the user’s 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(matches: &MatchedFlags<'_>, vars: &V) -> Result { + 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)?; - 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 row_threshold = RowThreshold::deduce(vars)?; let grid_details = grid_details::Options { grid, details, row_threshold }; return Ok(Self::GridDetails(grid_details)); } else { + // the --tree case is handled by the DirAction parser later 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 hasn’t been passed, then check if we need to warn the // user about flags that won’t have any effect. if matches.is_strict() { @@ -53,18 +100,7 @@ impl Mode { } } - if matches.has(&flags::TREE)? { - 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)) + Ok(()) } } @@ -524,7 +560,6 @@ mod test { // Options that do nothing with --long 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 test!(just_header: Mode <- ["--header"], None; Last => like Ok(Mode::Grid(_))); @@ -548,5 +583,14 @@ mod test { #[cfg(feature = "git")] 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(_))); } }