From 89b9374814e4699a12ffc21109f82ac277d62b3c Mon Sep 17 00:00:00 2001
From: Yehuda Katz <wycats@gmail.com>
Date: Fri, 20 Jun 2014 23:26:19 -0700
Subject: [PATCH] Improve error messages

---
 src/cargo/util/config.rs          | 21 ++++++++++-----------
 src/cargo/util/errors.rs          |  8 +++-----
 src/cargo/util/important_paths.rs |  9 ++-------
 src/cargo/util/process_builder.rs | 13 ++++++-------
 4 files changed, 21 insertions(+), 30 deletions(-)

diff --git a/src/cargo/util/config.rs b/src/cargo/util/config.rs
index 411d3cb1f..4604ec9c5 100644
--- a/src/cargo/util/config.rs
+++ b/src/cargo/util/config.rs
@@ -12,7 +12,8 @@ impl Config {
     pub fn new() -> CargoResult<Config> {
         Ok(Config {
             home_path: try!(os::homedir().require(|| {
-                human("Couldn't find the home directory")
+                human("Cargo couldn't find your home directory. \
+                      This probably means that $HOME was not set.")
             }))
         })
     }
@@ -99,15 +100,15 @@ impl fmt::Show for ConfigValue {
 
 pub fn get_config(pwd: Path, key: &str) -> CargoResult<ConfigValue> {
     find_in_tree(&pwd, |file| extract_config(file, key)).map_err(|_|
-        internal(format!("config key not found; key={}", key)))
+        human(format!("`{}` not found in your configuration", key)))
 }
 
 pub fn all_configs(pwd: Path) -> CargoResult<HashMap<String, ConfigValue>> {
     let mut map = HashMap::new();
 
-    try!(walk_tree(&pwd, |file| {
-        extract_all_configs(file, &mut map)
-    }));
+    try!(walk_tree(&pwd, |file| extract_all_configs(file, &mut map)).map_err(|_|
+        human("Couldn't load Cargo configuration")));
+
 
     Ok(map)
 }
@@ -119,9 +120,8 @@ fn find_in_tree<T>(pwd: &Path,
     loop {
         let possible = current.join(".cargo").join("config");
         if possible.exists() {
-            let file = try!(io::fs::File::open(&possible).chain_error(|| {
-                internal("could not open file")
-            }));
+            let file = try!(io::fs::File::open(&possible));
+
             match walk(file) {
                 Ok(res) => return Ok(res),
                 _ => ()
@@ -142,9 +142,8 @@ fn walk_tree(pwd: &Path,
     loop {
         let possible = current.join(".cargo").join("config");
         if possible.exists() {
-            let file = try!(io::fs::File::open(&possible).chain_error(|| {
-                internal("could not open file")
-            }));
+            let file = try!(io::fs::File::open(&possible));
+
             match walk(file) {
                 Err(_) => err = false,
                 _ => ()
diff --git a/src/cargo/util/errors.rs b/src/cargo/util/errors.rs
index e1dc0aa65..c3c1ea4fa 100644
--- a/src/cargo/util/errors.rs
+++ b/src/cargo/util/errors.rs
@@ -1,4 +1,4 @@
-use std::io::process::{Command,ProcessOutput,ProcessExit,ExitStatus,ExitSignal};
+use std::io::process::{ProcessOutput, ProcessExit, ExitStatus, ExitSignal};
 use std::io::IoError;
 use std::fmt;
 use std::fmt::{Show, Formatter, FormatError};
@@ -141,7 +141,6 @@ from_error!(FormatError)
 
 pub struct ProcessError {
     pub msg: String,
-    pub command: String,
     pub exit: Option<ProcessExit>,
     pub output: Option<ProcessOutput>,
     pub detail: Option<String>,
@@ -275,16 +274,15 @@ impl CliError {
 }
 
 pub fn process_error<S: Str>(msg: S,
-                             command: &Command,
+                             cause: Option<IoError>,
                              status: Option<&ProcessExit>,
                              output: Option<&ProcessOutput>) -> ProcessError {
     ProcessError {
         msg: msg.as_slice().to_str(),
-        command: command.to_str(),
         exit: status.map(|o| o.clone()),
         output: output.map(|o| o.clone()),
         detail: None,
-        cause: None
+        cause: cause.map(|c| box c as Box<CargoError>)
     }
 }
 
diff --git a/src/cargo/util/important_paths.rs b/src/cargo/util/important_paths.rs
index f8fd25981..7bcc9a551 100644
--- a/src/cargo/util/important_paths.rs
+++ b/src/cargo/util/important_paths.rs
@@ -1,4 +1,4 @@
-use util::{CargoResult, CargoError, internal_error};
+use util::{CargoResult, human};
 
 pub fn find_project(pwd: Path, file: &str) -> CargoResult<Path> {
     let mut current = pwd.clone();
@@ -11,10 +11,5 @@ pub fn find_project(pwd: Path, file: &str) -> CargoResult<Path> {
         if !current.pop() { break; }
     }
 
-    Err(manifest_missing_err(&pwd, file.as_slice()))
-}
-
-fn manifest_missing_err(pwd: &Path, file: &str) -> Box<CargoError> {
-    internal_error("manifest not found",
-                   format!("pwd={}; file={}", pwd.display(), file))
+    Err(human(format!("no manifest found in `{}`", pwd.display())))
 }
diff --git a/src/cargo/util/process_builder.rs b/src/cargo/util/process_builder.rs
index 49e07b9c2..1dc3ca4e6 100644
--- a/src/cargo/util/process_builder.rs
+++ b/src/cargo/util/process_builder.rs
@@ -80,14 +80,13 @@ impl ProcessBuilder {
         let msg = || format!("Could not execute process `{}`",
                              self.debug_string());
 
-        let exit = try!(command.status().map_err(|_| {
-            process_error(msg(), &command, None, None)
-        }));
+        let exit = try!(command.status().map_err(|e|
+            process_error(msg(), Some(e), None, None)));
 
         if exit.success() {
             Ok(())
         } else {
-            Err(process_error(msg(), &command, Some(&exit), None))
+            Err(process_error(msg(), None, Some(&exit), None))
         }
     }
 
@@ -98,14 +97,14 @@ impl ProcessBuilder {
         let msg = || format!("Could not execute process `{}`",
                              self.debug_string());
 
-        let output = try!(command.output().map_err(|_| {
-            process_error(msg(), &command, None, None)
+        let output = try!(command.output().map_err(|e| {
+            process_error(msg(), Some(e), None, None)
         }));
 
         if output.status.success() {
             Ok(output)
         } else {
-            Err(process_error(msg(), &command, Some(&output.status),
+            Err(process_error(msg(), None, Some(&output.status),
                               Some(&output)))
         }
     }