From ac26d2b94b2744cc4be9fe21c9fad960b9282f8f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rapha=C3=ABl=20Beamonte?= <raphael.beamonte@gmail.com>
Date: Tue, 2 Jan 2024 15:37:28 -0500
Subject: [PATCH] =?UTF-8?q?fix(dynenv):=20=F0=9F=90=9B=20unset=20properly?=
 =?UTF-8?q?=20list=20environment=20variables=20(#325)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

List environment variables were set back to the empty string instead of
being completely unset. This was an issue as empty strings-set
environment variables might not have the same behavior as unset
environment variables.

The issue manifested for the `GOPATH` variable since the recent changes
to handle `GOPATH` as a list in the dynenv.

This fixes that by having a new list operation 'create', which is used
when a list is being appended or prepended to for an environment
variable that is currently unset.
---
 src/internal/dynenv.rs | 38 ++++++++++++++++++++++++--------------
 1 file changed, 24 insertions(+), 14 deletions(-)

diff --git a/src/internal/dynenv.rs b/src/internal/dynenv.rs
index 8980295b..7709a5d7 100644
--- a/src/internal/dynenv.rs
+++ b/src/internal/dynenv.rs
@@ -601,13 +601,17 @@ impl DynamicEnvData {
             self.lists.insert(key.to_string(), Vec::new());
         }
 
+        let (cur_val, operation) = match self.env_get_var(key) {
+            Some(cur_val) => (cur_val, DynamicEnvListOperation::Add),
+            None => ("".to_string(), DynamicEnvListOperation::Create),
+        };
+
         self.lists.get_mut(key).unwrap().push(DynamicEnvListValue {
-            operation: DynamicEnvListOperation::Add,
+            operation,
             value: value.to_string(),
             index: 0,
         });
 
-        let cur_val = self.env_get_var(key).unwrap_or("".to_string());
         if cur_val.is_empty() {
             self.env_set_var(key, value);
         } else {
@@ -620,7 +624,10 @@ impl DynamicEnvData {
             self.lists.insert(key.to_string(), Vec::new());
         }
 
-        let cur_val = self.env_get_var(key).unwrap_or("".to_string());
+        let (cur_val, operation) = match self.env_get_var(key) {
+            Some(cur_val) => (cur_val, DynamicEnvListOperation::Add),
+            None => ("".to_string(), DynamicEnvListOperation::Create),
+        };
 
         let index = {
             let prev = cur_val.split(':').collect::<Vec<&str>>();
@@ -628,7 +635,7 @@ impl DynamicEnvData {
         };
 
         self.lists.get_mut(key).unwrap().push(DynamicEnvListValue {
-            operation: DynamicEnvListOperation::Add,
+            operation,
             value: value.to_string(),
             index,
         });
@@ -679,6 +686,14 @@ impl DynamicEnvData {
         }
 
         for (key, operations) in self.lists.clone().iter() {
+            if operations
+                .iter()
+                .any(|o| o.operation == DynamicEnvListOperation::Create)
+            {
+                self.env_unset_var(key);
+                continue;
+            }
+
             // Load the content of the variables, as we'll need to "undo" the
             // operations we've done to the closest of our ability; since it's
             // a list, we'll also split it, so we're ready to "search and update"
@@ -687,6 +702,9 @@ impl DynamicEnvData {
 
             for operation in operations.iter().rev() {
                 match operation.operation {
+                    DynamicEnvListOperation::Create => {
+                        unreachable!();
+                    }
                     DynamicEnvListOperation::Add => {
                         // Search for the operation.value in the current list, and return the closest index
                         // with operation.index in case the value is there multiple times
@@ -796,6 +814,8 @@ struct DynamicEnvValue {
 
 #[derive(Debug, Serialize, Deserialize, Clone, PartialEq)]
 enum DynamicEnvListOperation {
+    #[serde(rename = "c")]
+    Create,
     #[serde(rename = "a")]
     Add,
     #[serde(rename = "d")]
@@ -812,16 +832,6 @@ struct DynamicEnvListValue {
     index: usize,
 }
 
-impl DynamicEnvListValue {
-    // fn new(operation: DynamicEnvListOperation, value: &str, index: usize) -> Self {
-    // Self {
-    // operation: operation,
-    // value: value.to_string(),
-    // index: index,
-    // }
-    // }
-}
-
 fn set_none() -> Option<String> {
     None
 }