From 688df20a30cdb0e57d9c2736792671d085b45260 Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Wed, 17 Jun 2020 00:02:21 +0200 Subject: [PATCH] Error handling --- crates/nu-cli/src/cli.rs | 4 +- crates/nu-cli/src/env/TODO.org | 3 +- .../src/env/directory_specific_environment.rs | 69 +++++++++---------- crates/nu-cli/src/env/environment.rs | 9 +-- crates/nu-cli/src/env/environment_syncer.rs | 3 +- 5 files changed, 43 insertions(+), 45 deletions(-) diff --git a/crates/nu-cli/src/cli.rs b/crates/nu-cli/src/cli.rs index bd5097812..07ecbe620 100644 --- a/crates/nu-cli/src/cli.rs +++ b/crates/nu-cli/src/cli.rs @@ -237,7 +237,7 @@ pub fn create_default_context( syncer.load_environment(); let mut context = Context::basic()?; - syncer.sync_env_vars(&mut context); + syncer.sync_env_vars(&mut context)?; syncer.sync_path_vars(&mut context); { @@ -668,7 +668,7 @@ pub async fn cli( // TODO: make sure config is cached so we don't path this load every call // FIXME: we probably want to be a bit more graceful if we can't set the environment syncer.reload(); - syncer.sync_env_vars(&mut context); + syncer.sync_env_vars(&mut context)?; syncer.sync_path_vars(&mut context); match line { diff --git a/crates/nu-cli/src/env/TODO.org b/crates/nu-cli/src/env/TODO.org index 1b42c89e7..3bd961472 100644 --- a/crates/nu-cli/src/env/TODO.org +++ b/crates/nu-cli/src/env/TODO.org @@ -19,4 +19,5 @@ returning =None=, which completely skips running the code for dealing with direc - If a directory contains a .nu with an environment variable that was already set, the old value will be overwritten. - This holds even if the old value was set by a .nu in a parent directory. The overwritten value is restored when you leave the directory. ** Security - https://github.com/nushell/nushell/issues/1965 \ No newline at end of file + https://github.com/nushell/nushell/issues/1965 +** Nice errors \ No newline at end of file diff --git a/crates/nu-cli/src/env/directory_specific_environment.rs b/crates/nu-cli/src/env/directory_specific_environment.rs index 2b4a6b01b..be1535edc 100644 --- a/crates/nu-cli/src/env/directory_specific_environment.rs +++ b/crates/nu-cli/src/env/directory_specific_environment.rs @@ -1,4 +1,5 @@ use indexmap::{IndexMap, IndexSet}; +use nu_errors::ShellError; use nu_protocol::{Primitive, UntaggedValue, Value}; use std::io::Write; use std::{ @@ -8,7 +9,6 @@ use std::{ io::{Error, ErrorKind, Result}, path::PathBuf, }; -use nu_errors::ShellError; type EnvKey = String; type EnvVal = OsString; @@ -83,22 +83,12 @@ impl DirectorySpecificEnvironment { Ok(keyvals_to_restore) } - pub fn env_vars_to_add(&mut self) -> std::result::Result, ShellError> { + pub fn env_vars_to_add(&mut self) -> std::io::Result> { let current_dir = std::env::current_dir()?; let mut working_dir = Some(current_dir.as_path()); - let mut vars_to_add = IndexMap::new(); - // let mut file = OpenOptions::new() - // .write(true) - // .append(true) - // .create(true) - // .open("toadd.txt") - // .unwrap( - // ); - - // write!(&mut file, "1: {:?}\n", vars_to_add).unwrap(); - //WE CRASHING SOMEWHERE HERE + // //WE CRASHING SOMEWHERE HERE //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 { @@ -108,42 +98,49 @@ impl DirectorySpecificEnvironment { toml_doc .get("env") - .ok_or_else(|| Err(ShellError::untagged_runtime_error("env section missing")))? + .ok_or_else(|| Error::new(ErrorKind::InvalidData, "env section missing"))? .as_table() - .ok_or_else(|| Err(ShellError::untagged_runtime_error("env section malformed")))? + .ok_or_else(|| Error::new(ErrorKind::InvalidData, "env section malformed"))? .iter() - .for_each(|(directory_env_key, directory_env_val)| { - if !vars_to_add.contains_key(directory_env_key) { - let directory_env_val: EnvVal = - directory_env_val.as_str().unwrap().into(); + .for_each(|(dir_env_key, dir_env_val)| { + let dir_env_val: EnvVal = dir_env_val.as_str().unwrap().into(); - //If we are about to overwrite any environment variables, we save them first so they can be restored later. - if let Some(existing_val) = std::env::var_os(directory_env_key) { - if existing_val != directory_env_val { - self.overwritten_env_vars - .entry(wdir.to_path_buf()) - .or_insert(IndexMap::new()) - .insert(directory_env_key.clone(), existing_val); + //If we are about to overwrite any environment variables, we save them first so they can be restored later. + if let Some(existing_val) = std::env::var_os(dir_env_key) { + // if existing_val != dir_env_val { + // self.overwritten_env_vars + // .entry(wdir.to_path_buf()) + // .or_insert(IndexMap::new()) + // .insert(dir_env_key.clone(), existing_val); - vars_to_add.insert(directory_env_key.clone(), directory_env_val); - } - } else { - //Otherwise, we just track that we added it here - self.added_env_vars - .entry(wdir.to_path_buf()) - .or_insert(IndexSet::new()) - .insert(directory_env_key.clone()); + // vars_to_add.insert(dir_env_key.clone(), dir_env_val); + // } + } else { + //Otherwise, we just track that we added it here + self.added_env_vars + .entry(wdir.to_path_buf()) + .or_insert(IndexSet::new()) + .insert(dir_env_key.clone()); - vars_to_add.insert(directory_env_key.clone(), directory_env_val); - } + vars_to_add.insert(dir_env_key.clone(), dir_env_val); } }); } + working_dir = working_dir //Keep going up in the directory structure with .parent() .expect("This should not be None because of the while condition") .parent(); } + let mut file = OpenOptions::new() + .write(true) + .append(true) + .create(true) + .open("toadd.txt") + .unwrap(); + + write!(&mut file, "adding: {:?}\n", vars_to_add).unwrap(); + Ok(vars_to_add) } diff --git a/crates/nu-cli/src/env/environment.rs b/crates/nu-cli/src/env/environment.rs index d1fb49fb7..29a2796f5 100644 --- a/crates/nu-cli/src/env/environment.rs +++ b/crates/nu-cli/src/env/environment.rs @@ -5,6 +5,7 @@ use indexmap::{indexmap, IndexSet}; use nu_protocol::{UntaggedValue, Value}; use std::ffi::OsString; use std::{fs::OpenOptions, fmt::Debug}; +use nu_errors::ShellError; pub trait Env: Debug + Send { fn env(&self) -> Option; @@ -71,10 +72,10 @@ impl Environment { // self.add_env(&k, &v.to_string_lossy(), true); // }); - // self.direnv.env_vars_to_add()?.iter().for_each(|(k, v)| { - // std::env::set_var(k, v); - // self.add_env(&k, &v.to_string_lossy(), true); - // }); + self.direnv.env_vars_to_add()?.iter().for_each(|(k, v)| { + // std::env::set_var(k, v); + self.add_env(&k, &v.to_string_lossy(), true); + }); Ok(()) } diff --git a/crates/nu-cli/src/env/environment_syncer.rs b/crates/nu-cli/src/env/environment_syncer.rs index 195a733c4..a37a0dc1e 100644 --- a/crates/nu-cli/src/env/environment_syncer.rs +++ b/crates/nu-cli/src/env/environment_syncer.rs @@ -45,6 +45,7 @@ impl EnvironmentSyncer { pub fn sync_env_vars(&mut self, ctx: &mut Context) -> Result<(), ShellError> { let mut environment = self.env.lock(); + environment.maintain_directory_environment().ok(); if environment.env().is_some() { for (name, value) in ctx.with_host(|host| host.vars()) { if name != "path" && name != "PATH" { @@ -52,8 +53,6 @@ impl EnvironmentSyncer { // that aren't loaded from config. environment.add_env(&name, &value, false); - environment.maintain_directory_environment()?; - // clear the env var from the session // we are about to replace them ctx.with_host(|host| host.env_rm(std::ffi::OsString::from(name)));