Skip to content

Commit

Permalink
fix(dynenv): 🐛 unset properly list environment variables (#325)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
XaF authored Jan 2, 2024
1 parent aeb8516 commit ac26d2b
Showing 1 changed file with 24 additions and 14 deletions.
38 changes: 24 additions & 14 deletions src/internal/dynenv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -620,15 +624,18 @@ 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>>();
prev.len()
};

self.lists.get_mut(key).unwrap().push(DynamicEnvListValue {
operation: DynamicEnvListOperation::Add,
operation,
value: value.to_string(),
index,
});
Expand Down Expand Up @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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")]
Expand All @@ -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
}
Expand Down

0 comments on commit ac26d2b

Please sign in to comment.