Refactor RunError::Backtick* to use OutputError (#186)

Add `output()` to get stdout of a command, return a OutputError if
it failes. Refactor backtick run errors to contain an OutputError.

src/app.rs

@@ -7,7 +7,7 @@ use ::prelude::*;
 use std::{convert, ffi};
 use std::collections::BTreeMap;
 use self::clap::{App, Arg, ArgGroup, AppSettings};
-use super::{Slurp, RunError, RunOptions, compile, DEFAULT_SHELL};
+use super::{Slurp, RunOptions, compile, DEFAULT_SHELL};
 
 macro_rules! warn {
   ($($arg:tt)*) => {{
@@ -353,9 +353,7 @@ pub fn app() {
         warn!("{}", run_error);
       }
     }
-    match run_error {
+
-      RunError::Code{code, .. } | RunError::BacktickCode{code, ..} => process::exit(code),
+    process::exit(run_error.code().unwrap_or(libc::EXIT_FAILURE));
-      _ => process::exit(libc::EXIT_FAILURE),
-    }
   }
 }

src/lib.rs

@@ -101,6 +101,48 @@ fn contains<T: PartialOrd>(range: &Range<T>,  i: T) -> bool {
   i >= range.start && i < range.end
 }
 
+#[derive(Debug)]
+enum OutputError {
+  /// Non-zero exit code
+  Code(i32),
+  /// IO error upon execution
+  Io(io::Error),
+  /// Terminated by signal
+  Signal(i32),
+  /// Unknown failure
+  Unknown,
+  /// Stdtout not UTF-8
+  Utf8(std::str::Utf8Error),
+}
+
+/// Run a command and return the data it wrote to stdout as a string
+fn output(mut command: process::Command) -> Result<String, OutputError> {
+  match command.output() {
+    Ok(output) => {
+      if let Some(code) = output.status.code() {
+        if code != 0 {
+          return Err(OutputError::Code(code));
+        }
+      } else {
+        return Err(output_error_from_signal(output.status));
+      }
+      match std::str::from_utf8(&output.stdout) {
+        Err(error) => Err(OutputError::Utf8(error)),
+        Ok(utf8) => {
+          Ok(if utf8.ends_with('\n') {
+            &utf8[0..utf8.len()-1]
+          } else if utf8.ends_with("\r\n") {
+            &utf8[0..utf8.len()-2]
+          } else {
+            utf8
+          }.to_string())
+        }
+      }
+    }
+    Err(io_error) => Err(OutputError::Io(io_error)),
+  }
+}
+
 #[derive(PartialEq, Debug)]
 struct Recipe<'a> {
   dependencies:      Vec<&'a str>,
@@ -215,15 +257,12 @@ fn error_from_signal(
   }
 }
 
-/// Return a RunError::BacktickSignal if the process was terminated by signal,
+/// Return an OutputError::Signal if the process was terminated by signal,
-/// otherwise return a RunError::BacktickUnknownFailure
+/// otherwise return an OutputError::UnknownFailure
-fn backtick_error_from_signal<'a>(
+fn output_error_from_signal(exit_status: process::ExitStatus) -> OutputError {
-  token:       &Token<'a>,
-  exit_status: process::ExitStatus
-) -> RunError<'a> {
   match Platform::signal_from_exit_status(exit_status) {
-    Some(signal) => RunError::BacktickSignal{token: token.clone(), signal: signal},
+    Some(signal) => OutputError::Signal(signal),
-    None => RunError::BacktickUnknownFailure{token: token.clone()},
+    None => OutputError::Unknown,
   }
 }
 
@@ -264,33 +303,7 @@ fn run_backtick<'a>(
     process::Stdio::inherit()
   });
 
-  match cmd.output() {
+  output(cmd).map_err(|output_error| RunError::Backtick{token: token.clone(), output_error: output_error})
-    Ok(output) => {
-      if let Some(code) = output.status.code() {
-        if code != 0 {
-          return Err(RunError::BacktickCode {
-            token: token.clone(),
-            code: code,
-          });
-        }
-      } else {
-        return Err(backtick_error_from_signal(token, output.status));
-      }
-      match std::str::from_utf8(&output.stdout) {
-        Err(error) => Err(RunError::BacktickUtf8Error{token: token.clone(), utf8_error: error}),
-        Ok(utf8) => {
-          Ok(if utf8.ends_with('\n') {
-            &utf8[0..utf8.len()-1]
-          } else if utf8.ends_with("\r\n") {
-            &utf8[0..utf8.len()-2]
-          } else {
-            utf8
-          }.to_string())
-        }
-      }
-    }
-    Err(error) => Err(RunError::BacktickIoError{token: token.clone(), io_error: error}),
-  }
 }
 
 impl<'a> Recipe<'a> {
@@ -1334,11 +1347,7 @@ impl<'a> Display for Justfile<'a> {
 #[derive(Debug)]
 enum RunError<'a> {
   ArgumentCountMismatch{recipe: &'a str, found: usize, min: usize, max: usize},
-  BacktickCode{token: Token<'a>, code: i32},
+  Backtick{token: Token<'a>, output_error: OutputError},
-  BacktickIoError{token: Token<'a>, io_error: io::Error},
-  BacktickSignal{token: Token<'a>, signal: i32},
-  BacktickUnknownFailure{token: Token<'a>},
-  BacktickUtf8Error{token: Token<'a>, utf8_error: std::str::Utf8Error},
   Code{recipe: &'a str, line_number: Option<usize>, code: i32},
   InternalError{message: String},
   IoError{recipe: &'a str, io_error: io::Error},
@@ -1350,6 +1359,16 @@ enum RunError<'a> {
   UnknownRecipes{recipes: Vec<&'a str>, suggestion: Option<&'a str>},
 }
 
+impl<'a> RunError<'a> {
+  fn code(&self) -> Option<i32> {
+    use RunError::*;
+    match *self {
+      Code{code, ..} | Backtick{output_error: OutputError::Code(code), ..} => Some(code),
+      _ => None,
+    }
+  }
+}
+
 impl<'a> Display for RunError<'a> {
   fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
     use RunError::*;
@@ -1431,34 +1450,36 @@ impl<'a> Display for RunError<'a> {
         write!(f, "Recipe `{}` could not be run because of an IO error while trying \
                   to create a temporary directory or write a file to that directory`:\n{}",
                   recipe, io_error)?,
-      BacktickCode{code, ref token} => {
+      Backtick{ref token, ref output_error} => match *output_error {
-        write!(f, "Backtick failed with exit code {}\n", code)?;
+        OutputError::Code(code) => {
-        error_token = Some(token);
+          write!(f, "Backtick failed with exit code {}\n", code)?;
-      }
+          error_token = Some(token);
-      BacktickSignal{ref token, signal} => {
+        }
-        write!(f, "Backtick was terminated by signal {}", signal)?;
+        OutputError::Signal(signal) => {
-        error_token = Some(token);
+          write!(f, "Backtick was terminated by signal {}", signal)?;
-      }
+          error_token = Some(token);
-      BacktickUnknownFailure{ref token} => {
+        }
-        write!(f, "Backtick failed for an unknown reason")?;
+        OutputError::Unknown => {
-        error_token = Some(token);
+          write!(f, "Backtick failed for an unknown reason")?;
-      }
+          error_token = Some(token);
-      BacktickIoError{ref token, ref io_error} => {
+        }
-        match io_error.kind() {
+        OutputError::Io(ref io_error) => {
-          io::ErrorKind::NotFound => write!(
+          match io_error.kind() {
-            f, "Backtick could not be run because just could not find `sh` the command:\n{}",
+            io::ErrorKind::NotFound => write!(
-            io_error),
+              f, "Backtick could not be run because just could not find `sh` the command:\n{}",
-          io::ErrorKind::PermissionDenied => write!(
+              io_error),
-            f, "Backtick could not be run because just could not run `sh`:\n{}", io_error),
+            io::ErrorKind::PermissionDenied => write!(
-          _ => write!(f, "Backtick could not be run because of an IO \
+              f, "Backtick could not be run because just could not run `sh`:\n{}", io_error),
-                          error while launching `sh`:\n{}", io_error),
+            _ => write!(f, "Backtick could not be run because of an IO \
-        }?;
+                            error while launching `sh`:\n{}", io_error),
-        error_token = Some(token);
+          }?;
-      }
+          error_token = Some(token);
-      BacktickUtf8Error{ref token, ref utf8_error} => {
+        }
-        write!(f, "Backtick succeeded but stdout was not utf8: {}", utf8_error)?;
+        OutputError::Utf8(ref utf8_error) => {
-        error_token = Some(token);
+          write!(f, "Backtick succeeded but stdout was not utf8: {}", utf8_error)?;
-      }
+          error_token = Some(token);
+        }
+      },
       InternalError{ref message} => {
         write!(f, "Internal error, this may indicate a bug in just: {} \
                    consider filing an issue: https://github.com/casey/just/issues/new",

src/unit.rs

@@ -3,8 +3,8 @@ extern crate brev;
 
 use super::{
   And, CompileError, ErrorKind, Justfile, Or,
-  RunError, RunOptions, Token, compile, contains,
+  OutputError, RunError, RunOptions, Token,
-  tokenize
+  compile, contains, tokenize
 };
 
 use super::TokenKind::*;
@@ -1016,7 +1016,7 @@ fn missing_all_defaults() {
 fn backtick_code() {
   match parse_success("a:\n echo {{`f() { return 100; }; f`}}")
         .run(&["a"], &Default::default()).unwrap_err() {
-    RunError::BacktickCode{code, token} => {
+    RunError::Backtick{token, output_error: OutputError::Code(code)} => {
       assert_eq!(code, 100);
       assert_eq!(token.lexeme, "`f() { return 100; }; f`");
     },
@@ -1054,7 +1054,7 @@ recipe:
   };
 
   match parse_success(text).run(&["recipe"], &options).unwrap_err() {
-    RunError::BacktickCode{code: _, token} => {
+    RunError::Backtick{token, output_error: OutputError::Code(_)} => {
       assert_eq!(token.lexeme, "`echo $exported_variable`");
     },
     other => panic!("expected a backtick code errror, but got: {}", other),