Add context to all option number parse errors

Fixes GH-839.

Author: ogham

src/options/dir_action.rs

@@ -1,7 +1,7 @@
 //! Parsing the options for `DirAction`.
 
 use crate::options::parser::MatchedFlags;
-use crate::options::{flags, OptionsError};
+use crate::options::{flags, OptionsError, NumberSource};
 
 use crate::fs::dir_action::{DirAction, RecurseOptions};
 
@@ -55,17 +55,21 @@ impl RecurseOptions {
     /// determined earlier. The maximum level should be a number, and this
     /// will fail with an `Err` if it isn’t.
     pub fn deduce(matches: &MatchedFlags<'_>, tree: bool) -> Result<Self, OptionsError> {
-        let max_depth = if let Some(level) = matches.get(&flags::LEVEL)? {
-            match level.to_string_lossy().parse() {
-                Ok(l)   => Some(l),
-                Err(e)  => return Err(OptionsError::FailedParse(e)),
+        if let Some(level) = matches.get(&flags::LEVEL)? {
+            let arg_str = level.to_string_lossy();
+            match arg_str.parse() {
+                Ok(l) => {
+                    Ok(Self { tree, max_depth: Some(l) })
+                }
+                Err(e) => {
+                    let source = NumberSource::Arg(&flags::LEVEL);
+                    Err(OptionsError::FailedParse(arg_str.to_string(), source, e))
+                }
             }
         }
         else {
-            None
-        };
-
-        Ok(Self { tree, max_depth })
+            Ok(Self { tree, max_depth: None })
+        }
     }
 }
 

src/options/error.rs

@@ -37,18 +37,38 @@ pub enum OptionsError {
     TreeAllAll,
 
     /// A numeric option was given that failed to be parsed as a number.
-    FailedParse(ParseIntError),
+    FailedParse(String, NumberSource, ParseIntError),
 
     /// A glob ignore was given that failed to be parsed as a pattern.
     FailedGlobPattern(String),
 }
 
+/// The source of a string that failed to be parsed as a number.
+#[derive(PartialEq, Debug)]
+pub enum NumberSource {
+
+    /// It came... from a command-line argument!
+    Arg(&'static Arg),
+
+    /// It came... from the enviroment!
+    Env(&'static str),
+}
+
 impl From<glob::PatternError> for OptionsError {
     fn from(error: glob::PatternError) -> Self {
         Self::FailedGlobPattern(error.to_string())
     }
 }
 
+impl fmt::Display for NumberSource {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            Self::Arg(arg) => write!(f, "option {}", arg),
+            Self::Env(env) => write!(f, "environment variable {}", env),
+        }
+    }
+}
+
 impl fmt::Display for OptionsError {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         use crate::options::parser::TakesValue;
@@ -71,7 +91,7 @@ impl fmt::Display for OptionsError {
             Self::Useless(a, true, b)        => write!(f, "Option {} is useless given option {}", a, b),
             Self::Useless2(a, b1, b2)        => write!(f, "Option {} is useless without options {} or {}", a, b1, b2),
             Self::TreeAllAll                 => write!(f, "Option --tree is useless given --all --all"),
-            Self::FailedParse(ref e)         => write!(f, "Failed to parse number: {}", e),
+            Self::FailedParse(s, n, e)       => write!(f, "Value {:?} not valid for {}: {}", s, n, e),
             Self::FailedGlobPattern(ref e)   => write!(f, "Failed to parse glob pattern: {}", e),
         }
     }

src/options/file_name.rs

@@ -1,4 +1,4 @@
-use crate::options::{flags, OptionsError};
+use crate::options::{flags, OptionsError, NumberSource};
 use crate::options::parser::MatchedFlags;
 use crate::options::vars::{self, Vars};
 
@@ -30,8 +30,13 @@ impl ShowIcons {
         }
         else if let Some(columns) = vars.get(vars::EXA_ICON_SPACING).and_then(|s| s.into_string().ok()) {
             match columns.parse() {
-                Ok(width)  => Ok(Self::On(width)),
-                Err(e)     => Err(OptionsError::FailedParse(e)),
+                Ok(width) => {
+                    Ok(Self::On(width))
+                }
+                Err(e) => {
+                    let source = NumberSource::Env(vars::EXA_ICON_SPACING);
+                    Err(OptionsError::FailedParse(columns, source, e))
+                }
             }
         }
         else {

src/options/mod.rs

@@ -84,7 +84,7 @@ mod theme;
 mod view;
 
 mod error;
-pub use self::error::OptionsError;
+pub use self::error::{OptionsError, NumberSource};
 
 mod help;
 use self::help::HelpString;

src/options/view.rs

@@ -1,5 +1,5 @@
 use crate::fs::feature::xattr;
-use crate::options::{flags, OptionsError, Vars};
+use crate::options::{flags, OptionsError, NumberSource, Vars};
 use crate::options::parser::MatchedFlags;
 use crate::output::{View, Mode, TerminalWidth, grid, details};
 use crate::output::grid_details::{self, RowThreshold};
@@ -151,8 +151,13 @@ impl TerminalWidth {
 
         if let Some(columns) = vars.get(vars::COLUMNS).and_then(|s| s.into_string().ok()) {
             match columns.parse() {
-                Ok(width)  => Ok(Self::Set(width)),
-                Err(e)     => Err(OptionsError::FailedParse(e)),
+                Ok(width) => {
+                    Ok(Self::Set(width))
+                }
+                Err(e) => {
+                    let source = NumberSource::Env(vars::COLUMNS);
+                    Err(OptionsError::FailedParse(columns, source, e))
+                }
             }
         }
         else {
@@ -168,8 +173,13 @@ impl RowThreshold {
 
         if let Some(columns) = vars.get(vars::EXA_GRID_ROWS).and_then(|s| s.into_string().ok()) {
             match columns.parse() {
-                Ok(rows)  => Ok(Self::MinimumRows(rows)),
-                Err(e)    => Err(OptionsError::FailedParse(e)),
+                Ok(rows) => {
+                    Ok(Self::MinimumRows(rows))
+                }
+                Err(e) => {
+                    let source = NumberSource::Env(vars::EXA_GRID_ROWS);
+                    Err(OptionsError::FailedParse(columns, source, e))
+                }
             }
         }
         else {

xtests/errors.toml

@@ -15,3 +15,85 @@ stdout = { empty = true }
 stderr = { string = "To sort newest files last, try \"--sort newest\", or just \"-snew\""}
 status = 3
 tags = [ 'error', 'long', 'sort' ]
+
+
+# Invalid values for $COLUMNS
+
+[[cmd]]
+name = "‘COLUMNS=999... exa’ shows an error about the number size"
+shell = "exa"
+environment = { "COLUMNS" = "99999999999999999999999" }
+stdout = { empty = true }
+stderr = { file = "outputs/error_columns_nines.ansitxt" }
+status = 3
+tags = [ 'error', 'env' ]
+
+[[cmd]]
+name = "‘COLUMNS=abcdef exa’ shows an error about invalid digits"
+shell = "exa"
+environment = { "COLUMNS" = "abcdef" }
+stdout = { empty = true }
+stderr = { file = "outputs/error_columns_invalid.ansitxt" }
+status = 3
+tags = [ 'error', 'env' ]
+
+
+# Invalid values for $EXA_GRID_ROWS
+
+[[cmd]]
+name = "‘EXA_GRID_ROWS=999... exa -lG’ shows an error about the number size"
+shell = "exa -lG"
+environment = { "EXA_GRID_ROWS" = "99999999999999999999999" }
+stdout = { empty = true }
+stderr = { file = "outputs/error_grid_rows_nines.ansitxt" }
+status = 3
+tags = [ 'error', 'env' ]
+
+[[cmd]]
+name = "‘EXA_GRID_ROWS=abcdef exa -lG’ shows an error about invalid digits"
+shell = "exa -lG"
+environment = { "EXA_GRID_ROWS" = "abcdef" }
+stdout = { empty = true }
+stderr = { file = "outputs/error_grid_rows_invalid.ansitxt" }
+status = 3
+tags = [ 'error', 'env' ]
+
+
+# Invalid values for $EXA_ICON_SPACING
+
+[[cmd]]
+name = "‘EXA_ICON_SPACING=999... exa --icons’ shows an error about the number size"
+shell = "exa --icons"
+environment = { "EXA_ICON_SPACING" = "99999999999999999999999" }
+stdout = { empty = true }
+stderr = { file = "outputs/error_icon_spacing_nines.ansitxt" }
+status = 3
+tags = [ 'error', 'env', 'icons' ]
+
+[[cmd]]
+name = "‘EXA_ICON_SPACING=abcdef exa --icons’ shows an error about invalid digits"
+shell = "exa --icons"
+environment = { "EXA_ICON_SPACING" = "abcdef" }
+stdout = { empty = true }
+stderr = { file = "outputs/error_icon_spacing_invalid.ansitxt" }
+status = 3
+tags = [ 'error', 'env', 'icons' ]
+
+
+# Invalid values for --level (-L)
+
+[[cmd]]
+name = "‘exa -TL999...’ shows an error about the number size"
+shell = "exa -TL99999999999999999999999"
+stdout = { empty = true }
+stderr = { file = "outputs/error_level_nines.ansitxt" }
+status = 3
+tags = [ 'error', 'tree', 'level' ]
+
+[[cmd]]
+name = "‘exa -TLabcdef’ shows an error about invalid digits"
+shell = "exa -TLabcdef"
+stdout = { empty = true }
+stderr = { file = "outputs/error_level_invalid.ansitxt" }
+status = 3
+tags = [ 'error', 'tree', 'level' ]

xtests/outputs/error_columns_invalid.ansitxt

@@ -0,0 +1 @@
+exa: Value "abcdef" not valid for environment variable COLUMNS: invalid digit found in string

xtests/outputs/error_columns_nines.ansitxt

@@ -0,0 +1 @@
+exa: Value "99999999999999999999999" not valid for environment variable COLUMNS: number too large to fit in target type

xtests/outputs/error_grid_rows_invalid.ansitxt

@@ -0,0 +1 @@
+exa: Value "abcdef" not valid for environment variable EXA_GRID_ROWS: invalid digit found in string

xtests/outputs/error_grid_rows_nines.ansitxt

@@ -0,0 +1 @@
+exa: Value "99999999999999999999999" not valid for environment variable EXA_GRID_ROWS: number too large to fit in target type