From 3d213a63c5fc1db69ff41a422460b9aa4449a7fd Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Mon, 22 Jun 2020 18:42:46 +0200 Subject: [PATCH 1/3] Non-working errors --- crates/nu-cli/src/cli.rs | 4 +- .../src/env/directory_specific_environment.rs | 59 +++++++++++-------- crates/nu-cli/src/env/environment.rs | 3 +- crates/nu-cli/src/env/environment_syncer.rs | 4 +- 4 files changed, 42 insertions(+), 28 deletions(-) diff --git a/crates/nu-cli/src/cli.rs b/crates/nu-cli/src/cli.rs index 26790dcbf8..97deb47a65 100644 --- a/crates/nu-cli/src/cli.rs +++ b/crates/nu-cli/src/cli.rs @@ -238,7 +238,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); { @@ -677,7 +677,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/directory_specific_environment.rs b/crates/nu-cli/src/env/directory_specific_environment.rs index 49f1c4269b..626e2fea12 100644 --- a/crates/nu-cli/src/env/directory_specific_environment.rs +++ b/crates/nu-cli/src/env/directory_specific_environment.rs @@ -1,6 +1,7 @@ use crate::commands::{self, autoenv::Trusted}; use commands::autoenv; use indexmap::{IndexMap, IndexSet}; +use nu_errors::ShellError; use std::{ collections::hash_map::DefaultHasher, ffi::OsString, @@ -32,7 +33,10 @@ impl DirectorySpecificEnvironment { } } - fn toml_if_directory_is_trusted(&self, mut wdirenv: PathBuf) -> std::io::Result { + fn toml_if_directory_is_trusted( + &self, + mut wdirenv: PathBuf, + ) -> Result { if let Some(trusted) = &self.trusted { wdirenv.push(".nu-env"); if wdirenv.exists() { @@ -42,41 +46,48 @@ impl DirectorySpecificEnvironment { if trusted.files.get(wdirenv.to_str().unwrap()) == Some(&hasher.finish().to_string()) { - return Ok(content.parse::()?); + return Ok(content.parse::().or_else(|_| { + Err( + ShellError::untagged_runtime_error( + "Could not parse .nu-env file. Is it well-formed?", + ), + ) + })?); + } else { + return Err(ShellError::untagged_runtime_error("Found .nu-env file in this directory, but it is not trusted. You can run 'autoenv trust' to allow it. This needs to be done after each change to the file")); } } } - Err(Error::new(ErrorKind::Other, "No trusted directories")) + Err(ShellError::untagged_runtime_error("No trusted directories")) } - pub fn env_vars_to_add(&mut self) -> std::io::Result> { + pub fn env_vars_to_add(&mut self) -> Result, ShellError> { let current_dir = std::env::current_dir()?; let mut working_dir = Some(current_dir.as_path()); let mut vars_to_add = IndexMap::new(); //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 let Ok(toml_doc) = self.toml_if_directory_is_trusted(wdir.to_path_buf()) { - toml_doc - .get("env") - .ok_or_else(|| Error::new(ErrorKind::InvalidData, "env section missing"))? - .as_table() - .ok_or_else(|| Error::new(ErrorKind::InvalidData, "env section malformed"))? - .iter() - .for_each(|(dir_env_key, dir_env_val)| { - let dir_env_val: EnvVal = dir_env_val.as_str().unwrap().into(); + let toml_doc = self.toml_if_directory_is_trusted(wdir.to_path_buf())?; + toml_doc + .get("env") + .ok_or_else(|| Error::new(ErrorKind::InvalidData, "env section missing"))? + .as_table() + .ok_or_else(|| Error::new(ErrorKind::InvalidData, "env section malformed"))? + .iter() + .for_each(|(dir_env_key, dir_env_val)| { + let dir_env_val: EnvVal = dir_env_val.as_str().unwrap().into(); - //This condition is to make sure variables in parent directories don't overwrite variables set by subdirectories. - if !vars_to_add.contains_key(dir_env_key) { - vars_to_add.insert(dir_env_key.clone(), dir_env_val); + //This condition is to make sure variables in parent directories don't overwrite variables set by subdirectories. + if !vars_to_add.contains_key(dir_env_key) { + vars_to_add.insert(dir_env_key.clone(), dir_env_val); - self.added_env_vars - .entry(wdir.to_path_buf()) - .or_insert(IndexSet::new()) - .insert(dir_env_key.clone()); - } - }); - } + self.added_env_vars + .entry(wdir.to_path_buf()) + .or_insert(IndexSet::new()) + .insert(dir_env_key.clone()); + } + }); working_dir = working_dir //Keep going up in the directory structure with .parent() .expect("This should not be None because of the while condition") @@ -88,7 +99,7 @@ impl DirectorySpecificEnvironment { //If the user has left directories which added env vars through .nu, we clear those vars //once they are marked for deletion, remove them from added_env_vars - pub fn env_vars_to_delete(&mut self) -> std::io::Result> { + pub fn env_vars_to_delete(&mut self) -> Result, ShellError> { let current_dir = std::env::current_dir()?; let mut working_dir = Some(current_dir.as_path()); diff --git a/crates/nu-cli/src/env/environment.rs b/crates/nu-cli/src/env/environment.rs index 8df68f035c..64b7a790be 100644 --- a/crates/nu-cli/src/env/environment.rs +++ b/crates/nu-cli/src/env/environment.rs @@ -4,6 +4,7 @@ use indexmap::{indexmap, IndexSet}; use nu_protocol::{UntaggedValue, Value}; use std::ffi::OsString; use std::fmt::Debug; +use nu_errors::ShellError; pub trait Env: Debug + Send { fn env(&self) -> Option; @@ -57,7 +58,7 @@ impl Environment { } } - pub fn maintain_directory_environment(&mut self) -> std::io::Result<()> { + pub fn maintain_directory_environment(&mut self) -> Result<(), ShellError> { self.direnv.env_vars_to_delete()?.iter().for_each(|k| { self.remove_env(&k); }); diff --git a/crates/nu-cli/src/env/environment_syncer.rs b/crates/nu-cli/src/env/environment_syncer.rs index 1f1d34d99a..4f5d6af12a 100644 --- a/crates/nu-cli/src/env/environment_syncer.rs +++ b/crates/nu-cli/src/env/environment_syncer.rs @@ -3,6 +3,7 @@ use crate::data::config::{Conf, NuConfig}; use crate::env::environment::{Env, Environment}; use parking_lot::Mutex; use std::sync::Arc; +use nu_errors::ShellError; pub struct EnvironmentSyncer { pub env: Arc>>, @@ -41,7 +42,7 @@ impl EnvironmentSyncer { environment.morph(&*self.config); } - pub fn sync_env_vars(&mut self, ctx: &mut Context) { + pub fn sync_env_vars(&mut self, ctx: &mut Context) -> Result<(), ShellError>{ let mut environment = self.env.lock(); environment.maintain_directory_environment().ok(); @@ -71,6 +72,7 @@ impl EnvironmentSyncer { } } } + Ok(()) } pub fn sync_path_vars(&mut self, ctx: &mut Context) { From 595b188855e44ec2f9f0dda5246ebaacdffac170 Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Tue, 23 Jun 2020 18:37:50 +0200 Subject: [PATCH 2/3] Errors --- crates/nu-cli/src/cli.rs | 4 +- .../src/env/directory_specific_environment.rs | 99 ++++++++++++------- crates/nu-cli/src/env/environment_syncer.rs | 7 +- 3 files changed, 68 insertions(+), 42 deletions(-) diff --git a/crates/nu-cli/src/cli.rs b/crates/nu-cli/src/cli.rs index 97deb47a65..26790dcbf8 100644 --- a/crates/nu-cli/src/cli.rs +++ b/crates/nu-cli/src/cli.rs @@ -238,7 +238,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); { @@ -677,7 +677,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/directory_specific_environment.rs b/crates/nu-cli/src/env/directory_specific_environment.rs index 626e2fea12..e1ae5ff116 100644 --- a/crates/nu-cli/src/env/directory_specific_environment.rs +++ b/crates/nu-cli/src/env/directory_specific_environment.rs @@ -2,6 +2,7 @@ use crate::commands::{self, autoenv::Trusted}; use commands::autoenv; use indexmap::{IndexMap, IndexSet}; use nu_errors::ShellError; +use nu_source::Span; use std::{ collections::hash_map::DefaultHasher, ffi::OsString, @@ -33,30 +34,51 @@ impl DirectorySpecificEnvironment { } } + // fn toml_if_directory_is_trusted( + // &self, + // mut wdirenv: PathBuf, + // ) -> Result { + // if let Some(trusted) = &self.trusted { + // wdirenv.push(".nu-env"); + // if wdirenv.exists() { + // let content = std::fs::read_to_string(&wdirenv)?; + // let mut hasher = DefaultHasher::new(); + // content.hash(&mut hasher); + // if trusted.files.get(wdirenv.to_str().unwrap()) + // == Some(&hasher.finish().to_string()) + // { + // return Ok(content.parse::().or_else(|_| { + // Err(ShellError::unexpected_eof( + // "Could not parse .nu-env file. Is it well-formed?", + // Span::default(), + // )) + // })?); + // } else { + // return Err(ShellError::unexpected_eof("Found .nu-env file in this directory, but it is not trusted. You can run 'autoenv trust' to allow it. This needs to be done after each change to the file", + // Span::default())); + // } + // } + // } + // Err(ShellError::unexpected_eof("No trusted directories", Span::default())) + // } + fn toml_if_directory_is_trusted( &self, - mut wdirenv: PathBuf, + wdirenv: PathBuf, ) -> Result { if let Some(trusted) = &self.trusted { - wdirenv.push(".nu-env"); - if wdirenv.exists() { - let content = std::fs::read_to_string(&wdirenv)?; - let mut hasher = DefaultHasher::new(); - content.hash(&mut hasher); - if trusted.files.get(wdirenv.to_str().unwrap()) - == Some(&hasher.finish().to_string()) - { - return Ok(content.parse::().or_else(|_| { - Err( - ShellError::untagged_runtime_error( - "Could not parse .nu-env file. Is it well-formed?", - ), - ) - })?); - } else { - return Err(ShellError::untagged_runtime_error("Found .nu-env file in this directory, but it is not trusted. You can run 'autoenv trust' to allow it. This needs to be done after each change to the file")); - } + let content = std::fs::read_to_string(&wdirenv)?; + let mut hasher = DefaultHasher::new(); + content.hash(&mut hasher); + + if trusted.files.get(wdirenv.to_str().unwrap()) == Some(&hasher.finish().to_string()) { + return Ok(content.parse::().or_else(|_| { + Err(ShellError::untagged_runtime_error( + "Could not parse .nu-env file. Is it well-formed?", + )) + })?); } + return Err(ShellError::untagged_runtime_error("Found .nu-env file in this directory, but it is not trusted. You can run 'autoenv trust' to allow it. This needs to be done after each change to the file")); } Err(ShellError::untagged_runtime_error("No trusted directories")) } @@ -68,26 +90,29 @@ 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 { - let toml_doc = self.toml_if_directory_is_trusted(wdir.to_path_buf())?; - toml_doc - .get("env") - .ok_or_else(|| Error::new(ErrorKind::InvalidData, "env section missing"))? - .as_table() - .ok_or_else(|| Error::new(ErrorKind::InvalidData, "env section malformed"))? - .iter() - .for_each(|(dir_env_key, dir_env_val)| { - let dir_env_val: EnvVal = dir_env_val.as_str().unwrap().into(); + let wdirenv = wdir.join(".nu-env"); + if wdirenv.exists() { + let toml_doc = self.toml_if_directory_is_trusted(wdirenv)?; + toml_doc + .get("env") + .ok_or_else(|| Error::new(ErrorKind::InvalidData, "env section missing"))? + .as_table() + .ok_or_else(|| Error::new(ErrorKind::InvalidData, "env section malformed"))? + .iter() + .for_each(|(dir_env_key, dir_env_val)| { + let dir_env_val: EnvVal = dir_env_val.as_str().unwrap().into(); - //This condition is to make sure variables in parent directories don't overwrite variables set by subdirectories. - if !vars_to_add.contains_key(dir_env_key) { - vars_to_add.insert(dir_env_key.clone(), dir_env_val); + //This condition is to make sure variables in parent directories don't overwrite variables set by subdirectories. + if !vars_to_add.contains_key(dir_env_key) { + vars_to_add.insert(dir_env_key.clone(), dir_env_val); - self.added_env_vars - .entry(wdir.to_path_buf()) - .or_insert(IndexSet::new()) - .insert(dir_env_key.clone()); - } - }); + self.added_env_vars + .entry(wdir.to_path_buf()) + .or_insert(IndexSet::new()) + .insert(dir_env_key.clone()); + } + }); + } working_dir = working_dir //Keep going up in the directory structure with .parent() .expect("This should not be None because of the while condition") diff --git a/crates/nu-cli/src/env/environment_syncer.rs b/crates/nu-cli/src/env/environment_syncer.rs index 4f5d6af12a..ca59313522 100644 --- a/crates/nu-cli/src/env/environment_syncer.rs +++ b/crates/nu-cli/src/env/environment_syncer.rs @@ -42,10 +42,12 @@ impl EnvironmentSyncer { environment.morph(&*self.config); } - pub fn sync_env_vars(&mut self, ctx: &mut Context) -> Result<(), ShellError>{ + pub fn sync_env_vars(&mut self, ctx: &mut Context) { let mut environment = self.env.lock(); - environment.maintain_directory_environment().ok(); + if let Err(e) = environment.maintain_directory_environment() { + ctx.error(e); + } if environment.env().is_some() { for (name, value) in ctx.with_host(|host| host.vars()) { if name != "path" && name != "PATH" { @@ -72,7 +74,6 @@ impl EnvironmentSyncer { } } } - Ok(()) } pub fn sync_path_vars(&mut self, ctx: &mut Context) { From b85a2529e3ecc024bda20c7a343125f1ccaf6866 Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Tue, 23 Jun 2020 19:19:38 +0200 Subject: [PATCH 3/3] Errors print, but not nicely --- .../src/env/directory_specific_environment.rs | 34 ++----------------- crates/nu-cli/src/env/environment_syncer.rs | 3 +- 2 files changed, 5 insertions(+), 32 deletions(-) diff --git a/crates/nu-cli/src/env/directory_specific_environment.rs b/crates/nu-cli/src/env/directory_specific_environment.rs index e1ae5ff116..5fa5512660 100644 --- a/crates/nu-cli/src/env/directory_specific_environment.rs +++ b/crates/nu-cli/src/env/directory_specific_environment.rs @@ -34,34 +34,6 @@ impl DirectorySpecificEnvironment { } } - // fn toml_if_directory_is_trusted( - // &self, - // mut wdirenv: PathBuf, - // ) -> Result { - // if let Some(trusted) = &self.trusted { - // wdirenv.push(".nu-env"); - // if wdirenv.exists() { - // let content = std::fs::read_to_string(&wdirenv)?; - // let mut hasher = DefaultHasher::new(); - // content.hash(&mut hasher); - // if trusted.files.get(wdirenv.to_str().unwrap()) - // == Some(&hasher.finish().to_string()) - // { - // return Ok(content.parse::().or_else(|_| { - // Err(ShellError::unexpected_eof( - // "Could not parse .nu-env file. Is it well-formed?", - // Span::default(), - // )) - // })?); - // } else { - // return Err(ShellError::unexpected_eof("Found .nu-env file in this directory, but it is not trusted. You can run 'autoenv trust' to allow it. This needs to be done after each change to the file", - // Span::default())); - // } - // } - // } - // Err(ShellError::unexpected_eof("No trusted directories", Span::default())) - // } - fn toml_if_directory_is_trusted( &self, wdirenv: PathBuf, @@ -78,7 +50,7 @@ impl DirectorySpecificEnvironment { )) })?); } - return Err(ShellError::untagged_runtime_error("Found .nu-env file in this directory, but it is not trusted. You can run 'autoenv trust' to allow it. This needs to be done after each change to the file")); + return Err(ShellError::untagged_runtime_error("Found untrusted .nu-env file in this directory. Run 'autoenv trust' and restart nushell to allow it. This needs to be done after each change to the file.")); } Err(ShellError::untagged_runtime_error("No trusted directories")) } @@ -95,9 +67,9 @@ impl DirectorySpecificEnvironment { let toml_doc = self.toml_if_directory_is_trusted(wdirenv)?; toml_doc .get("env") - .ok_or_else(|| Error::new(ErrorKind::InvalidData, "env section missing"))? + .ok_or_else(|| Error::new(ErrorKind::InvalidData, "env section missing in .nu-env"))? .as_table() - .ok_or_else(|| Error::new(ErrorKind::InvalidData, "env section malformed"))? + .ok_or_else(|| Error::new(ErrorKind::InvalidData, "malformed env section in .nu-env"))? .iter() .for_each(|(dir_env_key, dir_env_val)| { let dir_env_val: EnvVal = dir_env_val.as_str().unwrap().into(); diff --git a/crates/nu-cli/src/env/environment_syncer.rs b/crates/nu-cli/src/env/environment_syncer.rs index ca59313522..28d8cf01c9 100644 --- a/crates/nu-cli/src/env/environment_syncer.rs +++ b/crates/nu-cli/src/env/environment_syncer.rs @@ -4,6 +4,7 @@ use crate::env::environment::{Env, Environment}; use parking_lot::Mutex; use std::sync::Arc; use nu_errors::ShellError; +use nu_source::Text; pub struct EnvironmentSyncer { pub env: Arc>>, @@ -46,7 +47,7 @@ impl EnvironmentSyncer { let mut environment = self.env.lock(); if let Err(e) = environment.maintain_directory_environment() { - ctx.error(e); + crate::cli::print_err(e, &Text::from("")); } if environment.env().is_some() { for (name, value) in ctx.with_host(|host| host.vars()) {