Let argument parser handle Profile conversion

Author: djc

src/cli/rustup_mode.rs

@@ -3,7 +3,6 @@ use std::fmt;
 use std::io::Write;
 use std::path::{Path, PathBuf};
 use std::process::ExitStatus;
-use std::str::FromStr;
 
 use anyhow::{anyhow, Error, Result};
 use clap::{
@@ -333,8 +332,8 @@ struct UpdateOpts {
     )]
     toolchain: Vec<PartialToolchainDesc>,
 
-    #[arg(long, value_parser = PossibleValuesParser::new(Profile::names()))]
-    profile: Option<String>,
+    #[arg(long, value_enum)]
+    profile: Option<Profile>,
 
     /// Add specific components on installation
     #[arg(short, long, value_delimiter = ',', num_args = 1..)]
@@ -518,11 +517,8 @@ enum SetSubcmd {
 
     /// The default components installed with a toolchain
     Profile {
-        #[arg(
-            default_value = Profile::default_name(),
-            value_parser = PossibleValuesParser::new(Profile::names()),
-        )]
-        profile_name: String,
+        #[arg(value_enum, default_value_t)]
+        profile_name: Profile,
     },
 
     /// The rustup auto self update mode
@@ -710,7 +706,7 @@ pub async fn main(current_dir: PathBuf) -> Result<utils::ExitCode> {
                 .set_default_host_triple(host_triple)
                 .map(|_| utils::ExitCode(0)),
             SetSubcmd::Profile { profile_name } => {
-                cfg.set_profile(&profile_name).map(|_| utils::ExitCode(0))
+                cfg.set_profile(profile_name).map(|_| utils::ExitCode(0))
             }
             SetSubcmd::AutoSelfUpdate {
                 auto_self_update_mode,
@@ -819,8 +815,7 @@ async fn update(cfg: &mut Cfg, opts: UpdateOpts) -> Result<utils::ExitCode> {
         && self_update_mode == SelfUpdateMode::Enable
         && !opts.no_self_update;
     let forced = opts.force_non_host;
-    if let Some(p) = &opts.profile {
-        let p = Profile::from_str(p)?;
+    if let Some(p) = opts.profile {
         cfg.set_profile_override(p);
     }
     let cfg = &cfg;

src/cli/self_update.rs

@@ -56,6 +56,8 @@ use std::{env, fmt};
 
 use anyhow::{anyhow, Context, Result};
 use cfg_if::cfg_if;
+use clap::ValueEnum;
+use itertools::Itertools;
 use same_file::Handle;
 use serde::{Deserialize, Serialize};
 
@@ -88,7 +90,7 @@ pub use windows::complete_windows_uninstall;
 pub(crate) struct InstallOpts<'a> {
     pub default_host_triple: Option<String>,
     pub default_toolchain: Option<MaybeOfficialToolchainName>,
-    pub profile: String,
+    pub profile: Profile,
     pub no_modify_path: bool,
     pub no_update_toolchain: bool,
     pub components: &'a [&'a str],
@@ -107,7 +109,7 @@ impl<'a> InstallOpts<'a> {
             targets,
         } = self;
 
-        cfg.set_profile(&profile)?;
+        cfg.set_profile(profile)?;
 
         if let Some(default_host_triple) = &default_host_triple {
             // Set host triple now as it will affect resolution of toolchain_str
@@ -202,13 +204,13 @@ impl<'a> InstallOpts<'a> {
                 .unwrap_or("stable".into()),
         )?)?);
 
-        self.profile = common::question_str(
+        self.profile = <Profile as FromStr>::from_str(&common::question_str(
             &format!(
                 "Profile (which tools and data to install)? ({})",
-                Profile::names().join("/")
+                Profile::value_variants().iter().join("/"),
             ),
-            &self.profile,
-        )?;
+            self.profile.as_str(),
+        )?)?;
 
         self.no_modify_path =
             !common::question_bool("Modify PATH variable?", !self.no_modify_path)?;
@@ -1313,7 +1315,7 @@ mod tests {
 
     use crate::cli::common;
     use crate::cli::self_update::InstallOpts;
-    use crate::dist::dist::PartialToolchainDesc;
+    use crate::dist::dist::{PartialToolchainDesc, Profile};
     use crate::test::{test_dir, with_rustup_home, Env};
     use crate::{currentprocess, for_host};
 
@@ -1329,8 +1331,8 @@ mod tests {
 
             let opts = InstallOpts {
                 default_host_triple: None,
-                default_toolchain: None,       // No toolchain specified
-                profile: "default".to_owned(), // default profile
+                default_toolchain: None,   // No toolchain specified
+                profile: Profile::Default, // default profile
                 no_modify_path: false,
                 components: &[],
                 targets: &[],

src/cli/setup_mode.rs

@@ -1,7 +1,7 @@
 use std::path::PathBuf;
 
 use anyhow::Result;
-use clap::{builder::PossibleValuesParser, Parser};
+use clap::Parser;
 
 use crate::{
     cli::{
@@ -43,12 +43,8 @@ struct RustupInit {
     #[arg(long)]
     default_toolchain: Option<MaybeOfficialToolchainName>,
 
-    #[arg(
-        long,
-        value_parser = PossibleValuesParser::new(Profile::names()),
-        default_value = Profile::default_name(),
-    )]
-    profile: String,
+    #[arg(long, value_enum, default_value_t)]
+    profile: Profile,
 
     /// Component name to also install
     #[arg(short, long, value_delimiter = ',', num_args = 1..)]
@@ -110,7 +106,7 @@ pub async fn main(current_dir: PathBuf) -> Result<utils::ExitCode> {
         return Ok(utils::ExitCode(0));
     }
 
-    if &profile == "complete" {
+    if profile == Profile::Complete {
         warn!("{}", common::WARN_COMPLETE_PROFILE);
     }
 

src/config.rs

@@ -370,19 +370,14 @@ impl Cfg {
         Ok(())
     }
 
-    pub(crate) fn set_profile(&mut self, profile: &str) -> Result<()> {
-        match Profile::from_str(profile) {
-            Ok(p) => {
-                self.profile_override = None;
-                self.settings_file.with_mut(|s| {
-                    s.profile = Some(p);
-                    Ok(())
-                })?;
-                (self.notify_handler)(Notification::SetProfile(profile));
-                Ok(())
-            }
-            Err(err) => Err(err),
-        }
+    pub(crate) fn set_profile(&mut self, profile: Profile) -> Result<()> {
+        self.profile_override = None;
+        self.settings_file.with_mut(|s| {
+            s.profile = Some(profile);
+            Ok(())
+        })?;
+        (self.notify_handler)(Notification::SetProfile(profile.as_str()));
+        Ok(())
     }
 
     pub(crate) fn set_auto_self_update(&mut self, mode: &str) -> Result<()> {
@@ -414,13 +409,8 @@ impl Cfg {
         if let Some(p) = self.profile_override {
             return Ok(p);
         }
-        self.settings_file.with(|s| {
-            let p = match s.profile {
-                Some(p) => p,
-                None => Profile::Default,
-            };
-            Ok(p)
-        })
+        self.settings_file
+            .with(|s| Ok(s.profile.unwrap_or_default()))
     }
 
     pub(crate) fn get_self_update_mode(&self) -> Result<SelfUpdateMode> {

src/dist/dist.rs

@@ -8,6 +8,9 @@ use std::str::FromStr;
 
 use anyhow::{anyhow, bail, Context, Result};
 use chrono::NaiveDate;
+use clap::builder::PossibleValue;
+use clap::ValueEnum;
+use itertools::Itertools;
 use once_cell::sync::Lazy;
 use regex::Regex;
 use serde::{Deserialize, Serialize};
@@ -611,6 +614,30 @@ pub enum Profile {
     Complete,
 }
 
+impl Profile {
+    pub(crate) fn as_str(&self) -> &'static str {
+        match self {
+            Self::Minimal => "minimal",
+            Self::Default => "default",
+            Self::Complete => "complete",
+        }
+    }
+}
+
+impl ValueEnum for Profile {
+    fn value_variants<'a>() -> &'a [Self] {
+        &[Profile::Minimal, Profile::Default, Profile::Complete]
+    }
+
+    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())
+    }
+}
+
 impl FromStr for Profile {
     type Err = anyhow::Error;
 
@@ -622,22 +649,12 @@ impl FromStr for Profile {
             _ => Err(anyhow!(format!(
                 "unknown profile name: '{}'; valid profile names are: {}",
                 name,
-                valid_profile_names()
+                Self::value_variants().iter().join(", ")
             ))),
         }
     }
 }
 
-impl Profile {
-    pub(crate) fn names() -> &'static [&'static str] {
-        &["minimal", "default", "complete"]
-    }
-
-    pub(crate) fn default_name() -> &'static str {
-        "default"
-    }
-}
-
 impl fmt::Display for TargetTriple {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         self.0.fmt(f)
@@ -680,22 +697,10 @@ impl fmt::Display for ToolchainDesc {
 
 impl fmt::Display for Profile {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        match *self {
-            Self::Minimal => write!(f, "minimal"),
-            Self::Default => write!(f, "default"),
-            Self::Complete => write!(f, "complete"),
-        }
+        write!(f, "{}", self.as_str())
     }
 }
 
-pub(crate) fn valid_profile_names() -> String {
-    Profile::names()
-        .iter()
-        .map(|s| format!("'{s}'"))
-        .collect::<Vec<_>>()
-        .join(", ")
-}
-
 // Installs or updates a toolchain from a dist server. If an initial
 // install then it will be installed with the default components. If
 // an upgrade then all the existing components will be upgraded.