Enhance according to review

Author: kellda

src/cli/self_update.rs

@@ -344,7 +344,6 @@ pub fn install(
 
         if !opts.no_modify_path {
             do_add_to_path()?;
-            #[cfg(windows)]
             do_add_to_programs()?;
         }
         utils::create_rustup_home()?;
@@ -856,7 +855,6 @@ pub fn uninstall(no_prompt: bool) -> Result<utils::ExitCode> {
 
     // Remove CARGO_HOME/bin from PATH
     do_remove_from_path()?;
-    #[cfg(windows)]
     do_remove_from_programs()?;
 
     // Delete everything in CARGO_HOME *except* the rustup bin

src/cli/self_update/unix.rs

@@ -114,6 +114,14 @@ pub fn do_write_env_files() -> Result<()> {
     Ok(())
 }
 
+pub fn do_add_to_programs() -> Result<()> {
+    Ok(())
+}
+
+pub fn do_remove_from_programs() -> Result<()> {
+    Ok(())
+}
+
 /// Tell the upgrader to replace the rustup bins, then delete
 /// itself. Like with uninstallation, on Windows we're going to
 /// have to jump through hoops to make everything work right.

src/cli/self_update/windows.rs

@@ -282,15 +282,17 @@ pub fn do_remove_from_path() -> Result<()> {
     _apply_new_path(new_path)
 }
 
+const RUSTUP_UNINSTALL_ENTRY: &str = r"Software\Microsoft\Windows\CurrentVersion\Uninstall\Rustup";
+
 pub fn do_add_to_programs() -> Result<()> {
     use std::path::PathBuf;
 
     let key = RegKey::predef(HKEY_CURRENT_USER)
-        .create_subkey(r"Software\Microsoft\Windows\CurrentVersion\Uninstall\rustup")
+        .create_subkey(RUSTUP_UNINSTALL_ENTRY)
         .chain_err(|| ErrorKind::PermissionDenied)?
         .0;
 
-    // Don't overwrite registry if rustup is already installed
+    // Don't overwrite registry if Rustup is already installed
     let prev = key
         .get_raw_value("UninstallString")
         .map(|val| from_winreg_value(&val));
@@ -304,8 +306,9 @@ pub fn do_add_to_programs() -> Result<()> {
 
     let mut path = utils::cargo_home()?;
     path.push("bin\rustup.exe");
-    let mut uninstall_cmd = path.into_os_string();
-    uninstall_cmd.push(" self uninstall");
+    let mut uninstall_cmd = OsString::from("\"");
+    uninstall_cmd.push(path);
+    uninstall_cmd.push("\" self uninstall");
 
     let reg_value = RegValue {
         bytes: to_winreg_bytes(uninstall_cmd.encode_wide().collect()),
@@ -314,41 +317,18 @@ pub fn do_add_to_programs() -> Result<()> {
 
     key.set_raw_value("UninstallString", &reg_value)
         .chain_err(|| ErrorKind::PermissionDenied)?;
-    key.set_value("DisplayName", &"rustup - Rust toolchain manager")
+    key.set_value("DisplayName", &"Rustup: the Rust toolchain installer")
         .chain_err(|| ErrorKind::PermissionDenied)?;
 
     Ok(())
 }
 
 pub fn do_remove_from_programs() -> Result<()> {
-    use std::io;
-    use std::path::PathBuf;
-
-    let root = RegKey::predef(HKEY_CURRENT_USER);
-
-    let key = match root.open_subkey(r"Software\Microsoft\Windows\CurrentVersion\Uninstall\rustup")
-    {
-        Ok(key) => key,
-        Err(ref e) if e.kind() == io::ErrorKind::NotFound => return Ok(()),
-        Err(e) => return Err(e).chain_err(|| ErrorKind::PermissionDenied),
-    };
-
-    // Don't remove uninstall information from another installation
-    let cur = key
-        .get_raw_value("UninstallString")
-        .map(|val| from_winreg_value(&val));
-    if let Ok(Some(s)) = cur {
-        let mut reg_path = PathBuf::from(OsString::from_wide(&s));
-        reg_path.pop();
-        let mut path = utils::cargo_home()?;
-        path.push("bin");
-        if reg_path != path {
-            return Ok(());
-        }
+    match RegKey::predef(HKEY_CURRENT_USER).delete_subkey_all(RUSTUP_UNINSTALL_ENTRY) {
+        Ok(()) => Ok(()),
+        Err(ref e) if e.kind() == std::io::ErrorKind::NotFound => Ok(()),
+        Err(e) => Err(e).chain_err(|| ErrorKind::PermissionDenied),
     }
-
-    root.delete_subkey_all(r"Software\Microsoft\Windows\CurrentVersion\Uninstall\rustup")
-        .chain_err(|| ErrorKind::PermissionDenied)
 }
 
 /// Convert a vector UCS-2 chars to a null-terminated UCS-2 string in bytes