From 2f5817387006a9334f5a74bc7c17d7063e529108 Mon Sep 17 00:00:00 2001
From: Chawye Hsu <su+git@chawyehsu.com>
Date: Wed, 15 May 2024 19:17:10 +0800
Subject: [PATCH] feat(libscoop): remove env paths under isolated_path mode
 correctly

Signed-off-by: Chawye Hsu <su+git@chawyehsu.com>
---
 crates/libscoop/src/env.rs          | 26 +++++++++--------
 crates/libscoop/src/internal/env.rs | 44 +++++++++++------------------
 2 files changed, 30 insertions(+), 40 deletions(-)

diff --git a/crates/libscoop/src/env.rs b/crates/libscoop/src/env.rs
index fcec4bf..f15ea10 100644
--- a/crates/libscoop/src/env.rs
+++ b/crates/libscoop/src/env.rs
@@ -1,4 +1,4 @@
-use crate::{error::Fallible, internal, package::Package, Event, Session};
+use crate::{config, error::Fallible, internal, package::Package, Error, Event, Session};
 
 /// Unset all environment variables defined by a given package.
 pub fn remove(session: &Session, package: &Package) -> Fallible<()> {
@@ -12,7 +12,7 @@ pub fn remove(session: &Session, package: &Package) -> Fallible<()> {
 
         let keys = env_set.keys();
         for key in keys {
-            internal::env::set(key, "")?;
+            internal::env::set(key, None)?;
         }
 
         if let Some(tx) = session.emitter() {
@@ -22,8 +22,13 @@ pub fn remove(session: &Session, package: &Package) -> Fallible<()> {
 
     // Remove environment path
     if let Some(env_add_path) = package.manifest().env_add_path() {
-        let mut env_path_list = internal::env::get_env_path_list()?;
         let config = session.config();
+        let env_path_name = match config.use_isolated_path() {
+            Some(config::IsolatedPath::Named(name)) => name.to_owned(),
+            Some(config::IsolatedPath::Boolean(true)) => "SCOOP_PATH".to_owned(),
+            _ => "PATH".to_owned(),
+        };
+        let mut paths = internal::env::get_path_like_env(&env_path_name)?;
         let mut app_path = config.root_path().join("apps");
         app_path.push(package.name());
 
@@ -37,19 +42,16 @@ pub fn remove(session: &Session, package: &Package) -> Fallible<()> {
             "current"
         };
 
-        let paths = env_add_path
+        let env_add_path = env_add_path
             .into_iter()
-            .map(|p| {
-                internal::path::normalize_path(app_path.join(version).join(p))
-                    .to_str()
-                    .unwrap()
-                    .to_owned()
-            })
+            .map(|p| internal::path::normalize_path(app_path.join(version).join(p)))
             .collect::<Vec<_>>();
 
-        env_path_list.retain(|p| !paths.contains(p));
+        paths.retain(|p| !env_add_path.contains(p));
+
+        let updated = std::env::join_paths(paths).map_err(|e| Error::Custom(e.to_string()))?;
 
-        internal::env::set("PATH", &env_path_list.join(";"))?;
+        internal::env::set(&env_path_name, Some(&updated))?;
 
         if let Some(tx) = session.emitter() {
             let _ = tx.send(Event::PackageEnvPathRemoveDone);
diff --git a/crates/libscoop/src/internal/env.rs b/crates/libscoop/src/internal/env.rs
index 2885902..43e9907 100644
--- a/crates/libscoop/src/internal/env.rs
+++ b/crates/libscoop/src/internal/env.rs
@@ -1,7 +1,17 @@
+use std::path::PathBuf;
+
+use crate::error::Fallible;
+
 #[cfg(unix)]
-pub use unix::{get, get_env_path_list, set};
+pub use unix::{get, set};
 #[cfg(windows)]
-pub use windows::{get, get_env_path_list, set};
+pub use windows::{get, set};
+
+/// Get the value of a path-like environment variable.
+pub fn get_path_like_env(name: &str) -> Fallible<Vec<PathBuf>> {
+    let paths = get(name)?;
+    Ok(std::env::split_paths(&paths).collect())
+}
 
 #[cfg(windows)]
 mod windows {
@@ -26,29 +36,18 @@ mod windows {
 
     /// Set the value of an environment variable.
     /// If the value is an empty string, the variable is deleted.
-    pub fn set(key: &str, value: &str) -> Fallible<()> {
+    pub fn set(key: &str, value: Option<&OsString>) -> Fallible<()> {
         let path = Path::new("Environment");
         let (env, _) = HKCU.create_subkey(path)?;
 
-        if value.is_empty() {
+        if value.is_none() {
             // ignore error of deleting non-existent value
             let _ = env.delete_value(key);
         } else {
-            env.set_value(key, &value)?;
+            env.set_value(key, value.unwrap())?;
         }
         Ok(())
     }
-
-    /// Get the value of the `PATH` environment variable as a list of paths.
-    pub fn get_env_path_list() -> Fallible<Vec<String>> {
-        let env_path = get("PATH")?;
-        Ok(env_path
-            .into_string()
-            .unwrap()
-            .split(';')
-            .map(|s| s.to_owned())
-            .collect())
-    }
 }
 
 #[cfg(unix)]
@@ -65,19 +64,8 @@ mod unix {
 
     /// Set the value of an environment variable.
     /// If the value is an empty string, the variable is deleted.
-    pub fn set(key: &str, value: &str) -> Fallible<()> {
+    pub fn set(key: &str, value: Option<&OsString>) -> Fallible<()> {
         // no-op
         Ok(())
     }
-
-    /// Get the value of the `PATH` environment variable as a list of paths.
-    pub fn get_env_path_list() -> Fallible<Vec<String>> {
-        let env_path = get("PATH")?;
-        Ok(env_path
-            .into_string()
-            .unwrap()
-            .split(':')
-            .map(|s| s.to_owned())
-            .collect())
-    }
 }