From a667b9faa81970719d40d6fee4af99040e54aad2 Mon Sep 17 00:00:00 2001 From: David Soria Parra Date: Mon, 12 Feb 2024 11:48:06 +0000 Subject: [PATCH 1/4] move PATH modification into it's own fn --- rye/src/cli/rye.rs | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/rye/src/cli/rye.rs b/rye/src/cli/rye.rs index 5a966f0b63..14cab7204f 100644 --- a/rye/src/cli/rye.rs +++ b/rye/src/cli/rye.rs @@ -547,11 +547,26 @@ fn perform_install( prompt_for_default_toolchain(registered_toolchain.unwrap(), config_doc)?; } + add_rye_to_path(&mode, shims.as_path())?; + + echo!(); + echo!("{}", style("All done!").green()); + + config.save()?; + + Ok(()) +} + +fn add_rye_to_path(mode: &InstallMode, shims: &Path) -> Result<(), Error> { + let rye_home = env::var("RYE_HOME") + .map(Cow::Owned) + .unwrap_or(Cow::Borrowed(DEFAULT_HOME)); + let rye_home = Path::new(&*rye_home); #[cfg(unix)] { if !env::split_paths(&env::var_os("PATH").unwrap()) - .any(|x| same_file::is_same_file(x, &shims).unwrap_or(false)) + .any(|x| same_file::is_same_file(x, shims).unwrap_or(false)) { echo!(); echo!( @@ -602,12 +617,6 @@ fn perform_install( crate::utils::windows::add_to_programs(rye_home)?; crate::utils::windows::add_to_path(rye_home)?; } - - echo!(); - echo!("{}", style("All done!").green()); - - config.save()?; - Ok(()) } From 678d79b237304521b2b42219ed8f326f4b53fa38 Mon Sep 17 00:00:00 2001 From: David Soria Parra Date: Fri, 16 Feb 2024 13:27:47 +0000 Subject: [PATCH 2/4] add comments to add_rye_to_path --- rye/src/cli/rye.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/rye/src/cli/rye.rs b/rye/src/cli/rye.rs index 14cab7204f..c3d1551bef 100644 --- a/rye/src/cli/rye.rs +++ b/rye/src/cli/rye.rs @@ -557,12 +557,16 @@ fn perform_install( Ok(()) } +/// Add rye to the users path. fn add_rye_to_path(mode: &InstallMode, shims: &Path) -> Result<(), Error> { let rye_home = env::var("RYE_HOME") .map(Cow::Owned) .unwrap_or(Cow::Borrowed(DEFAULT_HOME)); let rye_home = Path::new(&*rye_home); + // For unices, we ask the user if they want rye to be added to PATH. + // If they choose to do so, we add the "env" script to .profile. + // See [`crate::utils::unix::add_to_path`]. #[cfg(unix)] { if !env::split_paths(&env::var_os("PATH").unwrap()) @@ -612,6 +616,7 @@ fn add_rye_to_path(mode: &InstallMode, shims: &Path) -> Result<(), Error> { echo!("For more information read https://rye-up.com/guide/installation/"); } } + // On Windows, we add the rye directory to the user's PATH unconditionally. #[cfg(windows)] { crate::utils::windows::add_to_programs(rye_home)?; From b611fa8e53c23c5a442f5a22baddd8f76f11ea7e Mon Sep 17 00:00:00 2001 From: David Soria Parra Date: Fri, 16 Feb 2024 13:27:57 +0000 Subject: [PATCH 3/4] add --modify-path and --no-modify-path to rye self install This adds the option to never modify paths to rye install, even when `--yes` is provided. This addresses #620. We offer a --modify-path and a --no-modify-path option to either always modify or never modify. Providing --yes, will be the same as --modify-path. This is in line with other command line tools like curl, git and ripgrep. We do this by adding a new enum YesNoArg, which can be obtained by providing an ArgGroup with respective modify_path and no_modify_path boolean values. --- rye/src/cli/rye.rs | 66 +++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 63 insertions(+), 3 deletions(-) diff --git a/rye/src/cli/rye.rs b/rye/src/cli/rye.rs index c3d1551bef..d6ceddd753 100644 --- a/rye/src/cli/rye.rs +++ b/rye/src/cli/rye.rs @@ -96,6 +96,50 @@ pub struct InstallCommand { /// Use a specific toolchain version. #[arg(long)] toolchain_version: Option, + + #[command(flatten)] + mp: ModifyPath, +} + +#[derive(Parser, Debug)] +#[group(required = false, multiple = false)] +pub struct ModifyPath { + /// Always modify without asking the PATH environment variable. + #[arg(long)] + modify_path: bool, + /// Do not modify the PATH environment variable. + #[arg(long)] + no_modify_path: bool, +} + +#[derive(Debug, Copy, Clone)] +enum YesNoArg { + Yes, + No, + Ask, +} + +impl YesNoArg { + fn with_yes(&self, yes: bool) -> Self { + match (yes, self) { + (true, Self::Ask) => Self::Yes, + _ => *self, + } + } +} +impl From for YesNoArg { + fn from(other: ModifyPath) -> Self { + // Argument parsing logic is a bit complex here: + match (other.modify_path, other.no_modify_path) { + // 1. If --modify-path is set and --no-modify-path is not set, we always modify the path without prompting. + (true, false) => YesNoArg::Yes, + // 2. If --no-modify-path is set and --modify-path is not set, we never modify the path. + (false, true) => YesNoArg::No, + // 3. Otherwise we ask the user + (false, false) => YesNoArg::Ask, + (true, true) => unreachable!(), + } + } } #[derive(Debug, Copy, Clone)] @@ -263,6 +307,7 @@ fn install(args: InstallCommand) -> Result<(), Error> { }, args.toolchain.as_deref(), args.toolchain_version, + YesNoArg::from(args.mp).with_yes(args.yes), ) } @@ -351,6 +396,7 @@ fn perform_install( mode: InstallMode, toolchain_path: Option<&Path>, toolchain_version: Option, + modify_path: YesNoArg, ) -> Result<(), Error> { let mut config = Config::current(); let mut registered_toolchain: Option = None; @@ -547,7 +593,20 @@ fn perform_install( prompt_for_default_toolchain(registered_toolchain.unwrap(), config_doc)?; } - add_rye_to_path(&mode, shims.as_path())?; + match modify_path { + YesNoArg::Yes => { + add_rye_to_path(&mode, shims.as_path(), false)?; + } + YesNoArg::No => { + echo!( + "Skipping PATH modification. You will need to add {} to your PATH manually.", + style(shims.display()).cyan() + ); + } + YesNoArg::Ask => { + add_rye_to_path(&mode, shims.as_path(), true)?; + } + } echo!(); echo!("{}", style("All done!").green()); @@ -558,7 +617,7 @@ fn perform_install( } /// Add rye to the users path. -fn add_rye_to_path(mode: &InstallMode, shims: &Path) -> Result<(), Error> { +fn add_rye_to_path(mode: &InstallMode, shims: &Path, ask: bool) -> Result<(), Error> { let rye_home = env::var("RYE_HOME") .map(Cow::Owned) .unwrap_or(Cow::Borrowed(DEFAULT_HOME)); @@ -581,6 +640,7 @@ fn add_rye_to_path(mode: &InstallMode, shims: &Path) -> Result<(), Error> { echo!("It is highly recommended that you add it."); if matches!(mode, InstallMode::NoPrompts) + || !ask || dialoguer::Confirm::with_theme(tui_theme()) .with_prompt(format!( "Should the installer add Rye to {} via .profile?", @@ -670,7 +730,7 @@ pub fn auto_self_install() -> Result { crate::request_continue_prompt(); } - perform_install(InstallMode::AutoInstall, None, None)?; + perform_install(InstallMode::AutoInstall, None, None, YesNoArg::Yes)?; Ok(true) } } From 4fcf897baaee2df7161df4ddd09c8be7c02ad117 Mon Sep 17 00:00:00 2001 From: David Soria Parra Date: Sat, 17 Feb 2024 15:01:38 +0000 Subject: [PATCH 4/4] fix unused variable warnings on windows On windows the function doesn't use all variables. Use cfg_attr to allow unused_variables. --- rye/src/cli/rye.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/rye/src/cli/rye.rs b/rye/src/cli/rye.rs index d6ceddd753..e5e3d00750 100644 --- a/rye/src/cli/rye.rs +++ b/rye/src/cli/rye.rs @@ -617,6 +617,7 @@ fn perform_install( } /// Add rye to the users path. +#[cfg_attr(windows, allow(unused_variables))] fn add_rye_to_path(mode: &InstallMode, shims: &Path, ask: bool) -> Result<(), Error> { let rye_home = env::var("RYE_HOME") .map(Cow::Owned)