Auto merge of #30 - killercup:feature/even-better-manifest-erros, r=k…

…illercup

Even Better NonExistentDependency Errors

Why didn't I do this before?

Makefile

@@ -7,8 +7,10 @@ release:
 	cargo build --release $(FEATURES)
 
 fmt:
+	rustfmt src/lib.rs --write-mode=overwrite && \
 	rustfmt src/bin/list/main.rs --write-mode=overwrite && \
 	rustfmt src/bin/add/main.rs --write-mode=overwrite && \
+	rustfmt src/bin/rm/main.rs --write-mode=overwrite && \
 	rustfmt tests/*.rs --write-mode=overwrite
 
 .PHONY: build release fmt

src/bin/rm/args.rs

@@ -1,5 +1,4 @@
 ///! Handle `cargo rm` arguments
-
 #[derive(Debug, RustcDecodable)]
 /// Docopts input args.
 pub struct Args {

src/bin/rm/main.rs

@@ -41,19 +41,21 @@ fn handle_rm(args: &Args) -> Result<(), Box<Error>> {
     manifest.remove_from_table(args.get_section(), &args.arg_crate.as_ref())
             .map_err(From::from)
             .and_then(|_| {
-                let mut file = try!(Manifest::find_file(&args.flag_manifest_path.as_ref().map(|s| &s[..])));
+                let mut file = try!(Manifest::find_file(&args.flag_manifest_path
+                                                             .as_ref()
+                                                             .map(|s| &s[..])));
                 manifest.write_to_file(&mut file)
             })
-           .or_else(|err| {
-               println!("Could not edit `Cargo.toml`.\n\nERROR: {}", err);
-               Err(err)
-           })
+            .or_else(|err| {
+                println!("Could not edit `Cargo.toml`.\n\nERROR: {}", err);
+                Err(err)
+            })
 }
 
 fn main() {
     let args = docopt::Docopt::new(USAGE)
-                    .and_then(|d| d.decode::<Args>())
-                    .unwrap_or_else(|err| err.exit());
+                   .and_then(|d| d.decode::<Args>())
+                   .unwrap_or_else(|err| err.exit());
 
     if args.flag_version {
         println!("cargo-rm version {}", env!("CARGO_PKG_VERSION"));

src/lib.rs

@@ -6,7 +6,8 @@
 #![cfg_attr(feature = "dev", feature(plugin))]
 #![cfg_attr(feature = "dev", plugin(clippy))]
 
-#[macro_use] extern crate quick_error;
+#[macro_use]
+extern crate quick_error;
 extern crate toml;
 
 mod manifest;

src/manifest.rs

@@ -20,14 +20,14 @@ quick_error! {
             display("Your Cargo.toml is missing.")
         }
         /// The TOML table could not be found.
-        NonExistentTable(name: String) {
+        NonExistentTable(table: String) {
             description("non existent table")
-            display("The table `{}` could not be found.", name)
+            display("The table `{}` could not be found.", table)
         }
         /// The dependency could not be found.
-        NonExistentDependency(name: String) {
+        NonExistentDependency(name: String, table: String) {
             description("non existent dependency")
-            display("The dependency `{}` could not be found.", name)
+            display("The dependency `{}` could not be found in `{}`.", name, table)
         }
     }
 }
@@ -180,10 +180,7 @@ impl Manifest {
     ///     assert!(manifest.remove_from_table("dependencies", &dep.0).is_err());
     /// # }
     /// ```
-    pub fn remove_from_table(&mut self,
-                             table: &str,
-                             name: &str)
-                             -> Result<(), ManifestError> {
+    pub fn remove_from_table(&mut self, table: &str, name: &str) -> Result<(), ManifestError> {
         let ref mut manifest = self.data;
         let entry = manifest.entry(String::from(table));
 
@@ -192,9 +189,11 @@ impl Manifest {
             Entry::Occupied(entry) => {
                 match *entry.into_mut() {
                     toml::Value::Table(ref mut deps) => {
-                        deps.remove(name).map(|_| ()).ok_or(ManifestError::NonExistentDependency(name.into()))
+                        deps.remove(name)
+                            .map(|_| ())
+                            .ok_or(ManifestError::NonExistentDependency(name.into(), table.into()))
                     }
-                    _ => Err(ManifestError::NonExistentTable(table.into()))
+                    _ => Err(ManifestError::NonExistentTable(table.into())),
                 }
             }
         }
@@ -237,8 +236,10 @@ mod tests {
         // Create a copy containing empty "dependencies" table because removing
         //   the last entry in a table does not remove the section.
         let mut copy = Manifest { data: toml::Table::new() };
-        copy.data.insert("dependencies".to_owned(), toml::Value::Table(BTreeMap::new()));
-        let dep = ("cargo-edit".to_owned(), toml::Value::String("0.1.0".to_owned()));
+        copy.data.insert("dependencies".to_owned(),
+                         toml::Value::Table(BTreeMap::new()));
+        let dep = ("cargo-edit".to_owned(),
+                   toml::Value::String("0.1.0".to_owned()));
         let _ = manifest.insert_into_table("dependencies", &dep);
         assert!(manifest.remove_from_table("dependencies", &dep.0).is_ok());
         assert_eq!(manifest, copy);
@@ -247,15 +248,18 @@ mod tests {
     #[test]
     fn remove_dependency_no_section() {
         let mut manifest = Manifest { data: toml::Table::new() };
-        let dep = ("cargo-edit".to_owned(), toml::Value::String("0.1.0".to_owned()));
+        let dep = ("cargo-edit".to_owned(),
+                   toml::Value::String("0.1.0".to_owned()));
         assert!(manifest.remove_from_table("dependencies", &dep.0).is_err());
     }
 
     #[test]
     fn remove_dependency_non_existent() {
         let mut manifest = Manifest { data: toml::Table::new() };
-        let dep = ("cargo-edit".to_owned(), toml::Value::String("0.1.0".to_owned()));
-        let other_dep = ("other-dep".to_owned(), toml::Value::String("0.1.0".to_owned()));
+        let dep = ("cargo-edit".to_owned(),
+                   toml::Value::String("0.1.0".to_owned()));
+        let other_dep = ("other-dep".to_owned(),
+                         toml::Value::String("0.1.0".to_owned()));
         let _ = manifest.insert_into_table("dependencies", &other_dep);
         assert!(manifest.remove_from_table("dependencies", &dep.0).is_err());