From 1277bfe0fb256059232dbbfd565bd4ffa9b5c6c6 Mon Sep 17 00:00:00 2001 From: est31 Date: Mon, 9 Sep 2019 13:02:25 +0200 Subject: [PATCH 1/6] Fix setting configuration params Fixes #627 Fixes a regression caused by #579, specifically commit cc8872b4eec3f39896ccb11d9c25a30a79c04dd7 . The code was intended to perform a comparison between the wanted output type and "Tagged" in order to be able to provide a special-cased path for Tagged. When I wrote the code, I used "name" as a variable name and only later realized that it shadowed the "name" param to the function, so I renamed it to type_name, but forgot to change the comparison. This broke the special-casing, as the name param only contains the name of the struct without generics (like "Tagged"), while `std::any::type_name` (in the current implementation) contains the full paths of the struct including all generic params (like "nu::object::meta::Tagged"). --- src/parser/deserializer.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/parser/deserializer.rs b/src/parser/deserializer.rs index 33a23189f..d4d492966 100644 --- a/src/parser/deserializer.rs +++ b/src/parser/deserializer.rs @@ -328,7 +328,7 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut ConfigDeserializer<'de> { let type_name = std::any::type_name::(); let tagged_val_name = std::any::type_name::>(); - if name == tagged_val_name { + if type_name == tagged_val_name { return visit::, _>(value.val, name, fields, visitor); } From 1d3483b59015d9cfe67de624cad0e8bbb600fe95 Mon Sep 17 00:00:00 2001 From: est31 Date: Mon, 9 Sep 2019 13:39:43 +0200 Subject: [PATCH 2/6] Add a test --- src/parser/deserializer.rs | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/parser/deserializer.rs b/src/parser/deserializer.rs index d4d492966..d5427766b 100644 --- a/src/parser/deserializer.rs +++ b/src/parser/deserializer.rs @@ -467,3 +467,27 @@ impl<'a, 'de: 'a> de::SeqAccess<'de> for StructDeserializer<'a, 'de> { return Some(self.fields.len()); } } + +#[cfg(test)] +mod tests { + use super::*; + use std::any::type_name; + #[test] + fn check_type_name_properties() { + // This ensures that certain properties for the + // std::any::type_name function hold, that + // this code relies on. The type_name docs explicitly + // mention that the actual format of the output + // is unspecified and change is likely. + // This test makes sure that such change is detected + // by this test failing, and not things silently breaking. + // Specifically, we rely on this behaviour further above + // in the file to special case Tagged parsing. + let tuple = type_name::<()>(); + let tagged_tuple = type_name::>(); + let tagged_value = type_name::>(); + assert!(tuple != tagged_tuple); + assert!(tuple != tagged_value); + assert!(tagged_tuple != tagged_value); + } +} From 4a2172a7f2eecacff35cce7f3ebd24ebc31835a3 Mon Sep 17 00:00:00 2001 From: Vanessa Sochat Date: Mon, 9 Sep 2019 12:44:07 -0400 Subject: [PATCH 3/6] tweaking nightly build - didnt seem to go off Signed-off-by: Vanessa Sochat --- .circleci/config.yml | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index dbc60e702..009375321 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -116,6 +116,13 @@ workflows: docker push quay.io/nushell/nu:devel nightly: + triggers: + - schedule: + cron: "0 0 * * *" + filters: + branches: + only: + - master jobs: - docker/publish: image: nushell/nu-base @@ -123,9 +130,6 @@ workflows: tag: nightly dockerfile: docker/Dockerfile.nu-base extra_build_args: --cache-from=quay.io/nushell/nu-base:nightly --build-arg RELEASE=true - filters: - branches: - only: master before_build: - run: docker pull quay.io/nushell/nu:nightly - run: docker pull quay.io/nushell/nu-base:nightly @@ -139,11 +143,3 @@ workflows: command: | docker push quay.io/nushell/nu-base:nightly docker push quay.io/nushell/nu:nightly - - triggers: - - schedule: - cron: "0 22 * * *" - filters: - branches: - only: - - master From d1167151fc8a0a566189b31735cc9b85142e8273 Mon Sep 17 00:00:00 2001 From: Jonathan Turner Date: Tue, 10 Sep 2019 05:10:52 +1200 Subject: [PATCH 4/6] Add support for light tables --- src/commands/config.rs | 2 +- src/format/table.rs | 44 +++++++++++++++++++++++++++++++++--------- 2 files changed, 36 insertions(+), 10 deletions(-) diff --git a/src/commands/config.rs b/src/commands/config.rs index c1f000d04..440432c27 100644 --- a/src/commands/config.rs +++ b/src/commands/config.rs @@ -1,8 +1,8 @@ use crate::prelude::*; use crate::commands::WholeStreamCommand; -use crate::errors::ShellError; use crate::data::{config, Value}; +use crate::errors::ShellError; use crate::parser::hir::SyntaxType; use crate::parser::registry::{self}; use std::iter::FromIterator; diff --git a/src/format/table.rs b/src/format/table.rs index 8deaf30aa..717e3c4a2 100644 --- a/src/format/table.rs +++ b/src/format/table.rs @@ -16,6 +16,11 @@ pub struct TableView { entries: Vec>, } +enum TableMode { + Light, + Normal, +} + impl TableView { fn merge_descriptors(values: &[Tagged]) -> Vec { let mut ret = vec![]; @@ -198,15 +203,36 @@ impl RenderView for TableView { } let mut table = Table::new(); - table.set_format( - FormatBuilder::new() - .column_separator('│') - .separator(LinePosition::Top, LineSeparator::new('━', '┯', ' ', ' ')) - .separator(LinePosition::Title, LineSeparator::new('─', '┼', ' ', ' ')) - .separator(LinePosition::Bottom, LineSeparator::new('━', '┷', ' ', ' ')) - .padding(1, 1) - .build(), - ); + + let table_mode = crate::data::config::config(Span::unknown())? + .get("table_mode") + .map(|s| match s.as_string().unwrap().as_ref() { + "light" => TableMode::Light, + _ => TableMode::Normal, + }) + .unwrap_or(TableMode::Normal); + + match table_mode { + TableMode::Light => { + table.set_format( + FormatBuilder::new() + .separator(LinePosition::Title, LineSeparator::new('─', '─', ' ', ' ')) + .padding(1, 1) + .build(), + ); + } + _ => { + table.set_format( + FormatBuilder::new() + .column_separator('│') + .separator(LinePosition::Top, LineSeparator::new('━', '┯', ' ', ' ')) + .separator(LinePosition::Title, LineSeparator::new('─', '┼', ' ', ' ')) + .separator(LinePosition::Bottom, LineSeparator::new('━', '┷', ' ', ' ')) + .padding(1, 1) + .build(), + ); + } + } let header: Vec = self .headers From f61144006f6f5cdb657869ab2c3527f8787ae20b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20N=2E=20Robalino?= Date: Tue, 10 Sep 2019 05:08:01 -0500 Subject: [PATCH 5/6] config test harness. --- src/commands/config.rs | 45 +++++++++---- src/data/config.rs | 94 ++++++++++++++++++--------- src/lib.rs | 1 + tests/command_config_test.rs | 120 +++++++++++++++++++++++++++++++++++ tests/helpers/mod.rs | 22 +++++-- 5 files changed, 237 insertions(+), 45 deletions(-) create mode 100644 tests/command_config_test.rs diff --git a/src/commands/config.rs b/src/commands/config.rs index 440432c27..c60ea2f2d 100644 --- a/src/commands/config.rs +++ b/src/commands/config.rs @@ -1,16 +1,17 @@ -use crate::prelude::*; - use crate::commands::WholeStreamCommand; use crate::data::{config, Value}; use crate::errors::ShellError; use crate::parser::hir::SyntaxType; use crate::parser::registry::{self}; +use crate::prelude::*; use std::iter::FromIterator; +use std::path::PathBuf; pub struct Config; #[derive(Deserialize)] pub struct ConfigArgs { + load: Option>, set: Option<(Tagged, Tagged)>, get: Option>, clear: Tagged, @@ -25,6 +26,7 @@ impl WholeStreamCommand for Config { fn signature(&self) -> Signature { Signature::build("config") + .named("load", SyntaxType::Path) .named("set", SyntaxType::Any) .named("get", SyntaxType::Any) .named("remove", SyntaxType::Any) @@ -47,6 +49,7 @@ impl WholeStreamCommand for Config { pub fn config( ConfigArgs { + load, set, get, clear, @@ -55,7 +58,15 @@ pub fn config( }: ConfigArgs, RunnableContext { name, .. }: RunnableContext, ) -> Result { - let mut result = crate::data::config::config(name)?; + let name_span = name; + + let configuration = if let Some(supplied) = load { + Some(supplied.item().clone()) + } else { + None + }; + + let mut result = crate::data::config::read(name_span, &configuration)?; if let Some(v) = get { let key = v.to_string(); @@ -63,15 +74,27 @@ pub fn config( .get(&key) .ok_or_else(|| ShellError::string(&format!("Missing key {} in config", key)))?; - return Ok( - stream![value.clone()].into(), // futures::stream::once(futures::future::ready(ReturnSuccess::Value(value.clone()))).into(), - ); + let mut results = VecDeque::new(); + + match value { + Tagged { + item: Value::Table(list), + .. + } => { + for l in list { + results.push_back(ReturnSuccess::value(l.clone())); + } + } + x => results.push_back(ReturnSuccess::value(x.clone())), + } + + return Ok(results.to_output_stream()); } if let Some((key, value)) = set { result.insert(key.to_string(), value.clone()); - config::write_config(&result)?; + config::write(&result, &configuration)?; return Ok(stream![Tagged::from_simple_spanned_item( Value::Row(result.into()), @@ -87,7 +110,7 @@ pub fn config( { result.clear(); - config::write_config(&result)?; + config::write(&result, &configuration)?; return Ok(stream![Tagged::from_simple_spanned_item( Value::Row(result.into()), @@ -101,10 +124,10 @@ pub fn config( tag: Tag { span, .. }, } = path { - let path = config::config_path()?; + let path = config::default_path_for(&configuration)?; return Ok(stream![Tagged::from_simple_spanned_item( - Value::Primitive(Primitive::Path(path)), + Value::string(path.to_string_lossy()), span )] .from_input_stream()); @@ -115,7 +138,7 @@ pub fn config( if result.contains_key(&key) { result.remove(&key); - config::write_config(&result)?; + config::write(&result, &configuration)?; } else { return Err(ShellError::string(&format!( "{} does not exist in config", diff --git a/src/data/config.rs b/src/data/config.rs index cf384e8b6..08296c09a 100644 --- a/src/data/config.rs +++ b/src/data/config.rs @@ -11,52 +11,60 @@ use std::fs::{self, OpenOptions}; use std::io; use std::path::{Path, PathBuf}; -const APP_INFO: AppInfo = AppInfo { - name: "nu", - author: "nu shell developers", -}; - #[derive(Deserialize, Serialize)] struct Config { #[serde(flatten)] extra: IndexMap>, } -pub(crate) fn config_path() -> Result { - let location = app_root(AppDataType::UserConfig, &APP_INFO) - .map_err(|err| ShellError::string(&format!("Couldn't open config file:\n{}", err)))?; +pub const APP_INFO: AppInfo = AppInfo { + name: "nu", + author: "nu shell developers", +}; - Ok(location.join("config.toml")) +pub fn config_path() -> Result { + let path = app_root(AppDataType::UserConfig, &APP_INFO) + .map_err(|err| ShellError::string(&format!("Couldn't open config path:\n{}", err)))?; + + Ok(path) } -pub(crate) fn write_config(config: &IndexMap>) -> Result<(), ShellError> { - let location = app_root(AppDataType::UserConfig, &APP_INFO) - .map_err(|err| ShellError::string(&format!("Couldn't open config file:\n{}", err)))?; - - let filename = location.join("config.toml"); - touch(&filename)?; - - let contents = - value_to_toml_value(&Value::Row(Dictionary::new(config.clone())).tagged_unknown())?; - - let contents = toml::to_string(&contents)?; - - fs::write(&filename, &contents)?; - - Ok(()) +pub fn default_path() -> Result { + default_path_for(&None) } -pub(crate) fn config(span: impl Into) -> Result>, ShellError> { - let span = span.into(); +pub fn default_path_for(file: &Option) -> Result { + let filename = &mut config_path()?; + let filename = match file { + None => { + filename.push("config.toml"); + filename + } + Some(file) => { + filename.push(file); + filename + } + }; - let location = app_root(AppDataType::UserConfig, &APP_INFO) - .map_err(|err| ShellError::string(&format!("Couldn't open config file:\n{}", err)))?; + Ok(filename.clone()) +} + +pub fn read( + span: impl Into, + at: &Option, +) -> Result>, ShellError> { + let filename = default_path()?; + + let filename = match at { + None => filename, + Some(ref file) => file.clone(), + }; - let filename = location.join("config.toml"); touch(&filename)?; trace!("config file = {}", filename.display()); + let span = span.into(); let contents = fs::read_to_string(filename) .map(|v| v.simple_spanned(span)) .map_err(|err| ShellError::string(&format!("Couldn't read config file:\n{}", err)))?; @@ -75,6 +83,34 @@ pub(crate) fn config(span: impl Into) -> Result) -> Result>, ShellError> { + read(span, &None) +} + +pub fn write( + config: &IndexMap>, + at: &Option, +) -> Result<(), ShellError> { + let filename = &mut default_path()?; + let filename = match at { + None => filename, + Some(file) => { + filename.pop(); + filename.push(file); + filename + } + }; + + let contents = + value_to_toml_value(&Value::Row(Dictionary::new(config.clone())).tagged_unknown())?; + + let contents = toml::to_string(&contents)?; + + fs::write(&filename, &contents)?; + + Ok(()) +} + // A simple implementation of `% touch path` (ignores existing files) fn touch(path: &Path) -> io::Result<()> { match OpenOptions::new().create(true).write(true).open(path) { diff --git a/src/lib.rs b/src/lib.rs index 4e16352e0..a18343df9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -28,6 +28,7 @@ pub use crate::parser::parse::token_tree_builder::TokenTreeBuilder; pub use crate::plugin::{serve_plugin, Plugin}; pub use crate::utils::{AbsoluteFile, AbsolutePath, RelativePath}; pub use cli::cli; +pub use data::config::{APP_INFO, config_path}; pub use data::base::{Primitive, Value}; pub use data::dict::{Dictionary, TaggedDictBuilder}; pub use data::meta::{Span, Tag, Tagged, TaggedItem}; diff --git a/tests/command_config_test.rs b/tests/command_config_test.rs new file mode 100644 index 000000000..41038ddb1 --- /dev/null +++ b/tests/command_config_test.rs @@ -0,0 +1,120 @@ +mod helpers; + +use helpers as h; +use helpers::{Playground, Stub::*}; + +use std::path::PathBuf; + +#[test] +fn has_default_configuration_file() { + let expected = "config.toml"; + + Playground::setup("config_test_1", |dirs, _| { + + nu!(cwd: dirs.root(), "config"); + + assert_eq!( + dirs.config_path().join(expected), + nu::config_path().unwrap().join(expected) + ); + }) +} + +#[test] +fn shows_path_of_configuration_file() { + let expected = "config.toml"; + + Playground::setup("config_test_2", |dirs, _| { + + let actual = nu!( + cwd: dirs.test(), + "config --path | echo $it" + ); + + assert_eq!(PathBuf::from(actual), dirs.config_path().join(expected)); + }); +} + +#[test] +fn use_different_configuration() { + Playground::setup("config_test_3", |dirs, sandbox| { + sandbox + .with_files(vec![FileWithContent( + "test_3.toml", + r#" + caballero_1 = "Andrés N. Robalino" + caballero_2 = "Jonathan Turner" + caballero_3 = "Yehuda katz" + "# + )]); + + let actual = nu!( + cwd: dirs.root(), + "config --get caballero_1 --load {}/test_3.toml | echo $it", + dirs.test() + ); + + assert_eq!(actual, "Andrés N. Robalino"); + }); + + h::delete_file_at(nu::config_path().unwrap().join("test_3.toml")); +} + +#[test] +fn sets_configuration_value() { + Playground::setup("config_test_4", |dirs, sandbox| { + sandbox + .with_files(vec![FileWithContent( + "test_4.toml", + r#" + caballero_1 = "Andrés N. Robalino" + caballero_2 = "Jonathan Turner" + caballero_3 = "Yehuda katz" + "# + )]); + + nu!( + cwd: dirs.test(), + "config --load test_4.toml --set [caballero_4 jonas]" + ); + + let actual = nu!( + cwd: dirs.root(), + r#"open "{}/test_4.toml" | get caballero_4 | echo $it"#, + dirs.config_path() + ); + + assert_eq!(actual, "jonas"); + }); + + h::delete_file_at(nu::config_path().unwrap().join("test_4.toml")); +} + +#[test] +fn removes_configuration_value() { + Playground::setup("config_test_5", |dirs, sandbox| { + sandbox + .with_files(vec![FileWithContent( + "test_5.toml", + r#" + caballeros = [1, 1, 1] + podershell = [1, 1, 1] + "# + )]); + + nu!( + cwd: dirs.test(), + "config --load test_5.toml --remove podershell" + ); + + let actual = nu_error!( + cwd: dirs.root(), + r#"open "{}/test_5.toml" | get podershell | echo $it"#, + dirs.config_path() + ); + + assert!(actual.contains("table missing column")); + }); + + h::delete_file_at(nu::config_path().unwrap().join("test_5.toml")); +} \ No newline at end of file diff --git a/tests/helpers/mod.rs b/tests/helpers/mod.rs index 1c2bfef0e..04fd88992 100644 --- a/tests/helpers/mod.rs +++ b/tests/helpers/mod.rs @@ -4,6 +4,7 @@ use glob::glob; pub use std::path::Path; pub use std::path::PathBuf; +use app_dirs::{get_app_root, AppDataType}; use getset::Getters; use std::io::Read; use tempfile::{tempdir, TempDir}; @@ -177,6 +178,10 @@ impl Dirs { pub fn formats(&self) -> PathBuf { PathBuf::from(self.fixtures.join("formats")) } + + pub fn config_path(&self) -> PathBuf { + get_app_root(AppDataType::UserConfig, &nu::APP_INFO).unwrap() + } } impl Playground { @@ -227,11 +232,10 @@ impl Playground { playground_root.join(topic).display() )); - let root = - dunce::canonicalize(playground_root).expect(&format!( - "Couldn't canonicalize tests root path {}", - playground_root.display() - )); + let root = dunce::canonicalize(playground_root).expect(&format!( + "Couldn't canonicalize tests root path {}", + playground_root.display() + )); let dirs = Dirs { root, @@ -332,6 +336,14 @@ pub fn line_ending() -> String { } } +pub fn delete_file_at(full_path: impl AsRef) { + let full_path = full_path.as_ref(); + + if full_path.exists() { + std::fs::remove_file(full_path).expect("can not delete file"); + } +} + pub fn create_file_at(full_path: impl AsRef) -> Result<(), std::io::Error> { let full_path = full_path.as_ref(); From 11ef00749140dcd7304b4195309c6dc555e8cc72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20N=2E=20Robalino?= Date: Tue, 10 Sep 2019 05:28:15 -0500 Subject: [PATCH 6/6] Paths can be displayed as strings. --- src/commands/config.rs | 2 +- src/data/base.rs | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/commands/config.rs b/src/commands/config.rs index c60ea2f2d..78f1ad399 100644 --- a/src/commands/config.rs +++ b/src/commands/config.rs @@ -127,7 +127,7 @@ pub fn config( let path = config::default_path_for(&configuration)?; return Ok(stream![Tagged::from_simple_spanned_item( - Value::string(path.to_string_lossy()), + Value::Primitive(Primitive::Path(path)), span )] .from_input_stream()); diff --git a/src/data/base.rs b/src/data/base.rs index b48d69212..02ce4982b 100644 --- a/src/data/base.rs +++ b/src/data/base.rs @@ -558,6 +558,7 @@ impl Value { Value::Primitive(Primitive::Decimal(x)) => Ok(format!("{}", x)), Value::Primitive(Primitive::Int(x)) => Ok(format!("{}", x)), Value::Primitive(Primitive::Bytes(x)) => Ok(format!("{}", x)), + Value::Primitive(Primitive::Path(x)) => Ok(format!("{}", x.display())), // TODO: this should definitely be more general with better errors other => Err(ShellError::string(format!( "Expected string, got {:?}",