Let argument parser handle SelfUpdateMode conversion

Author: djc

src/cli/rustup_mode.rs

@@ -5,10 +5,7 @@ use std::path::{Path, PathBuf};
 use std::process::ExitStatus;
 
 use anyhow::{anyhow, Error, Result};
-use clap::{
-    builder::{PossibleValue, PossibleValuesParser},
-    Args, CommandFactory, Parser, Subcommand, ValueEnum,
-};
+use clap::{builder::PossibleValue, Args, CommandFactory, Parser, Subcommand, ValueEnum};
 use clap_complete::Shell;
 use itertools::Itertools;
 
@@ -523,11 +520,8 @@ enum SetSubcmd {
 
     /// The rustup auto self update mode
     AutoSelfUpdate {
-        #[arg(
-            default_value = SelfUpdateMode::default_mode(),
-            value_parser = PossibleValuesParser::new(SelfUpdateMode::modes()),
-        )]
-        auto_self_update_mode: String,
+        #[arg(value_enum, default_value_t)]
+        auto_self_update_mode: SelfUpdateMode,
     },
 }
 
@@ -710,7 +704,7 @@ pub async fn main(current_dir: PathBuf) -> Result<utils::ExitCode> {
             }
             SetSubcmd::AutoSelfUpdate {
                 auto_self_update_mode,
-            } => set_auto_self_update(cfg, &auto_self_update_mode),
+            } => set_auto_self_update(cfg, auto_self_update_mode),
         },
         RustupSubcmd::Completions { shell, command } => output_completion_script(shell, command),
     }
@@ -1510,7 +1504,10 @@ async fn man(
     Ok(utils::ExitCode(0))
 }
 
-fn set_auto_self_update(cfg: &mut Cfg, auto_self_update_mode: &str) -> Result<utils::ExitCode> {
+fn set_auto_self_update(
+    cfg: &mut Cfg,
+    auto_self_update_mode: SelfUpdateMode,
+) -> Result<utils::ExitCode> {
     if self_update::NEVER_SELF_UPDATE {
         let process = process();
         let mut args = process.args_os();

src/cli/self_update.rs

@@ -56,6 +56,7 @@ use std::{env, fmt};
 
 use anyhow::{anyhow, Context, Result};
 use cfg_if::cfg_if;
+use clap::builder::PossibleValue;
 use clap::ValueEnum;
 use itertools::Itertools;
 use same_file::Handle;
@@ -243,21 +244,36 @@ pub(crate) const NEVER_SELF_UPDATE: bool = true;
 #[cfg(not(feature = "no-self-update"))]
 pub(crate) const NEVER_SELF_UPDATE: bool = false;
 
-#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)]
+#[derive(Clone, Copy, Debug, Default, Deserialize, Eq, PartialEq, Serialize)]
 #[serde(rename_all = "kebab-case")]
 pub enum SelfUpdateMode {
+    #[default]
     Enable,
     Disable,
     CheckOnly,
 }
 
 impl SelfUpdateMode {
-    pub(crate) fn modes() -> &'static [&'static str] {
-        &["enable", "disable", "check-only"]
+    pub(crate) fn as_str(&self) -> &'static str {
+        match self {
+            Self::Enable => "enable",
+            Self::Disable => "disable",
+            Self::CheckOnly => "check-only",
+        }
+    }
+}
+
+impl ValueEnum for SelfUpdateMode {
+    fn value_variants<'a>() -> &'a [Self] {
+        &[Self::Enable, Self::Disable, Self::CheckOnly]
     }
 
-    pub(crate) fn default_mode() -> &'static str {
-        "enable"
+    fn to_possible_value(&self) -> Option<PossibleValue> {
+        Some(PossibleValue::new(self.as_str()))
+    }
+
+    fn from_str(input: &str, _: bool) -> Result<Self, String> {
+        <Self as FromStr>::from_str(input).map_err(|e| e.to_string())
     }
 }
 
@@ -272,19 +288,15 @@ impl FromStr for SelfUpdateMode {
             _ => Err(anyhow!(format!(
                 "unknown self update mode: '{}'; valid modes are {}",
                 mode,
-                Self::modes().join(", "),
+                Self::value_variants().iter().join(", ")
             ))),
         }
     }
 }
 
 impl std::fmt::Display for SelfUpdateMode {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        f.write_str(match self {
-            SelfUpdateMode::Enable => "enable",
-            SelfUpdateMode::Disable => "disable",
-            SelfUpdateMode::CheckOnly => "check-only",
-        })
+        f.write_str(self.as_str())
     }
 }
 

src/config.rs

@@ -380,18 +380,13 @@ impl Cfg {
         Ok(())
     }
 
-    pub(crate) fn set_auto_self_update(&mut self, mode: &str) -> Result<()> {
-        match SelfUpdateMode::from_str(mode) {
-            Ok(update_mode) => {
-                self.settings_file.with_mut(|s| {
-                    s.auto_self_update = Some(update_mode);
-                    Ok(())
-                })?;
-                (self.notify_handler)(Notification::SetSelfUpdate(mode));
-                Ok(())
-            }
-            Err(err) => Err(err),
-        }
+    pub(crate) fn set_auto_self_update(&mut self, mode: SelfUpdateMode) -> Result<()> {
+        self.settings_file.with_mut(|s| {
+            s.auto_self_update = Some(mode);
+            Ok(())
+        })?;
+        (self.notify_handler)(Notification::SetSelfUpdate(mode.as_str()));
+        Ok(())
     }
 
     pub(crate) fn set_toolchain_override(&mut self, toolchain_override: &ResolvableToolchainName) {
@@ -415,11 +410,10 @@ impl Cfg {
 
     pub(crate) fn get_self_update_mode(&self) -> Result<SelfUpdateMode> {
         self.settings_file.with(|s| {
-            let mode = match &s.auto_self_update {
-                Some(mode) => mode.clone(),
+            Ok(match s.auto_self_update {
+                Some(mode) => mode,
                 None => SelfUpdateMode::Enable,
-            };
-            Ok(mode)
+            })
         })
     }