From e1581ec156d831b8745a0b93dcb1af40090c54c2 Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Sun, 7 Jun 2020 07:01:51 +0200 Subject: [PATCH] Cleanup and handle errors --- .../src/env/directory_specific_environment.rs | 64 +++++++++++++------ crates/nu-cli/src/env/environment_syncer.rs | 3 +- 2 files changed, 44 insertions(+), 23 deletions(-) diff --git a/crates/nu-cli/src/env/directory_specific_environment.rs b/crates/nu-cli/src/env/directory_specific_environment.rs index ba7e4ee2fd..a7c8cac640 100644 --- a/crates/nu-cli/src/env/directory_specific_environment.rs +++ b/crates/nu-cli/src/env/directory_specific_environment.rs @@ -1,16 +1,17 @@ use indexmap::IndexMap; use nu_protocol::{Primitive, UntaggedValue, Value}; +use std::io::{Error, ErrorKind, Result}; use std::{ffi::OsString, fmt::Debug, path::PathBuf}; #[derive(Debug, Default)] pub struct DirectorySpecificEnvironment { - pub whitelisted_directories: Vec, + whitelisted_directories: Vec, //Directory -> Env key. If an environment var has been added from a .nu in a directory, we track it here so we can remove it when the user leaves the directory. - pub added_env_vars: IndexMap>, + added_env_vars: IndexMap>, //Directory -> (env_key, value). If a .nu overwrites some existing environment variables, they are added here so that they can be restored later. - pub overwritten_env_values: IndexMap>, + overwritten_env_values: IndexMap>, } impl DirectorySpecificEnvironment { @@ -22,16 +23,17 @@ impl DirectorySpecificEnvironment { { wrapped_directories .iter() - .fold(vec![], |mut directories, dirval| { + .filter_map(|dirval| { if let Value { value: UntaggedValue::Primitive(Primitive::String(ref dir)), tag: _, } = dirval { - directories.push(PathBuf::from(&dir)); + return Some(PathBuf::from(&dir)); } - directories + None }) + .collect() } else { vec![] }; @@ -44,7 +46,7 @@ impl DirectorySpecificEnvironment { } } - pub fn overwritten_values_to_restore(&mut self) -> std::io::Result> { + pub fn overwritten_values_to_restore(&mut self) -> Result> { let current_dir = std::env::current_dir()?; let mut keyvals_to_restore = IndexMap::new(); @@ -60,7 +62,11 @@ impl DirectorySpecificEnvironment { new_overwritten.insert(directory.clone(), keyvals.clone()); break; } else { - working_dir = working_dir.expect("Root directory has no parent").parent(); + working_dir = working_dir //Keep going up in the directory structure with .parent() + .ok_or_else(|| { + Error::new(ErrorKind::NotFound, "Root directory has no parent") + })? + .parent(); } } if re_add_keyvals { @@ -68,7 +74,9 @@ impl DirectorySpecificEnvironment { keyvals_to_restore.insert( k.clone(), v.to_str() - .expect("Filepath is not valid unicode") + .ok_or_else(|| { + Error::new(ErrorKind::Other, "Filepath is not valid unicode") + })? .to_string(), ); } @@ -79,7 +87,7 @@ impl DirectorySpecificEnvironment { Ok(keyvals_to_restore) } - pub fn env_vars_to_add(&mut self) -> std::io::Result> { + pub fn env_vars_to_add(&mut self) -> Result> { let current_dir = std::env::current_dir()?; let mut vars_to_add = IndexMap::new(); @@ -89,15 +97,24 @@ impl DirectorySpecificEnvironment { //Start in the current directory, then traverse towards the root with working_dir to see if we are in a subdirectory of a valid directory. while let Some(wdir) = working_dir { if wdir == dir.as_path() { - //Read the .nu file and parse it into a nice map let toml_doc = std::fs::read_to_string(wdir.join(".nu").as_path())? .parse::()?; let vars_in_current_file = toml_doc .get("env") - .ok_or(std::io::Error::new(std::io::ErrorKind::InvalidData, "No [env] section in .nu-file"))? + .ok_or_else(|| { + std::io::Error::new( + std::io::ErrorKind::InvalidData, + "No [env] section in .nu-file", + ) + })? .as_table() - .ok_or(std::io::Error::new(std::io::ErrorKind::InvalidData, "Malformed [env] section in .nu-file"))?; + .ok_or_else(|| { + Error::new( + ErrorKind::InvalidData, + "Malformed [env] section in .nu-file", + ) + })?; let mut keys_in_current_nufile = vec![]; for (k, v) in vars_in_current_file { @@ -105,24 +122,29 @@ impl DirectorySpecificEnvironment { keys_in_current_nufile.push(k.clone()); //this is used to keep track of which directory added which variables } + //If we are about to overwrite any environment variables, we save them first so they can be restored later. self.overwritten_env_values.insert( - //If we are about to overwrite any environment variables, we save them first so they can be restored later. wdir.to_path_buf(), keys_in_current_nufile .iter() - .fold(vec![], |mut keyvals, key| { + .filter_map(|key| { if let Some(val) = std::env::var_os(key) { - keyvals.push((key.clone(), val)); + return Some((key.clone(), val)); } - keyvals - }), + None + }) + .collect(), ); self.added_env_vars .insert(wdir.to_path_buf(), keys_in_current_nufile); break; } else { - working_dir = working_dir.expect("Root directory has no parent").parent(); + working_dir = working_dir //Keep going up in the directory structure with .parent() + .ok_or_else(|| { + Error::new(ErrorKind::NotFound, "Root directory has no parent") + })? + .parent(); } } } @@ -131,7 +153,7 @@ impl DirectorySpecificEnvironment { } //If the user has left directories which added env vars through .nu, we clear those vars - pub fn env_vars_to_delete(&mut self) -> std::io::Result> { + pub fn env_vars_to_delete(&mut self) -> Result> { let current_dir = std::env::current_dir()?; //Gather up all environment variables that should be deleted. @@ -142,7 +164,7 @@ impl DirectorySpecificEnvironment { let mut working_dir = Some(current_dir.as_path()); while let Some(wdir) = working_dir { - if &wdir == directory { + if wdir == directory { return vars_to_delete; } else { working_dir = working_dir.expect("Root directory has no parent").parent(); diff --git a/crates/nu-cli/src/env/environment_syncer.rs b/crates/nu-cli/src/env/environment_syncer.rs index 177f32fb8e..7c0a17071b 100644 --- a/crates/nu-cli/src/env/environment_syncer.rs +++ b/crates/nu-cli/src/env/environment_syncer.rs @@ -52,8 +52,7 @@ impl EnvironmentSyncer { environment.add_env(&name, &value, false); environment - .maintain_directory_environment() - .expect("Could not set directory-based environment variables"); + .maintain_directory_environment().ok(); // clear the env var from the session // we are about to replace them