Allow config to work with column paths. (#2653)

Nu has many commands that allow the nuño to customize behavior such
as UI and behavior. Today, coloring can be customized, the line editor,
and other things. The more options there are, the higher the complexity
in managing them.

To mitigate this Nu can store configuration options as nested properties.

But to add and edit them can be taxing. With column path support we can
work with them easier.

Cargo.lock

@@ -3008,6 +3008,7 @@ dependencies = [
  "nu-protocol",
  "nu-source",
  "nu-table",
+ "nu-test-support",
  "nu-value-ext",
  "num-bigint 0.3.0",
  "num-traits 0.2.12",
@@ -3116,12 +3117,16 @@ dependencies = [
 name = "nu-test-support"
 version = "0.20.0"
 dependencies = [
+ "chrono",
  "dunce",
  "getset",
  "glob",
  "indexmap",
+ "nu-errors",
  "nu-protocol",
  "nu-source",
+ "nu-value-ext",
+ "num-bigint 0.3.0",
  "tempfile",
 ]
 
@@ -3134,6 +3139,7 @@ dependencies = [
  "nu-errors",
  "nu-protocol",
  "nu-source",
+ "nu-test-support",
  "num-traits 0.2.12",
 ]
 

crates/nu-cli/src/commands/config/get.rs

@@ -2,14 +2,13 @@ use crate::command_registry::CommandRegistry;
 use crate::commands::WholeStreamCommand;
 use crate::prelude::*;
 use nu_errors::ShellError;
-use nu_protocol::{ReturnSuccess, Signature, SyntaxShape, UntaggedValue, Value};
-use nu_source::Tagged;
+use nu_protocol::{ColumnPath, ReturnSuccess, Signature, SyntaxShape, UntaggedValue, Value};
 
 pub struct SubCommand;
 
 #[derive(Deserialize)]
 pub struct GetArgs {
-    get: Tagged<String>,
+    path: ColumnPath,
 }
 
 #[async_trait]
@@ -21,7 +20,7 @@ impl WholeStreamCommand for SubCommand {
     fn signature(&self) -> Signature {
         Signature::build("config get").required(
             "get",
-            SyntaxShape::Any,
+            SyntaxShape::ColumnPath,
             "value to get from the config",
         )
     }
@@ -51,17 +50,14 @@ pub async fn get(
     args: CommandArgs,
     registry: &CommandRegistry,
 ) -> Result<OutputStream, ShellError> {
-    let name_span = args.call_info.name_tag.clone();
-    let (GetArgs { get }, _) = args.process(&registry).await?;
+    let name_tag = args.call_info.name_tag.clone();
+    let (GetArgs { path }, _) = args.process(&registry).await?;
 
     // NOTE: None because we are not loading a new config file, we just want to read from the
     // existing config
-    let result = nu_data::config::read(name_span, &None)?;
+    let result = UntaggedValue::row(nu_data::config::read(&name_tag, &None)?).into_value(&name_tag);
 
-    let key = get.to_string();
-    let value = result
-        .get(&key)
-        .ok_or_else(|| ShellError::labeled_error("Missing key in config", "key", get.tag()))?;
+    let value = crate::commands::get::get_column_path(&path, &result)?;
 
     Ok(match value {
         Value {
@@ -75,9 +71,6 @@ pub async fn get(
 
             futures::stream::iter(list).to_output_stream()
         }
-        x => {
-            let x = x.clone();
-            OutputStream::one(ReturnSuccess::value(x))
-        }
+        x => OutputStream::one(ReturnSuccess::value(x)),
     })
 }

crates/nu-cli/src/commands/config/set.rs

@@ -2,14 +2,13 @@ use crate::command_registry::CommandRegistry;
 use crate::commands::WholeStreamCommand;
 use crate::prelude::*;
 use nu_errors::ShellError;
-use nu_protocol::{ReturnSuccess, Signature, SyntaxShape, UntaggedValue, Value};
-use nu_source::Tagged;
+use nu_protocol::{ColumnPath, ReturnSuccess, Signature, SyntaxShape, UntaggedValue, Value};
 
 pub struct SubCommand;
 
 #[derive(Deserialize)]
 pub struct SetArgs {
-    key: Tagged<String>,
+    path: ColumnPath,
     value: Value,
 }
 
@@ -21,7 +20,7 @@ impl WholeStreamCommand for SubCommand {
 
     fn signature(&self) -> Signature {
         Signature::build("config set")
-            .required("key", SyntaxShape::String, "variable name to set")
+            .required("key", SyntaxShape::ColumnPath, "variable name to set")
             .required("value", SyntaxShape::Any, "value to use")
     }
 
@@ -38,11 +37,28 @@ impl WholeStreamCommand for SubCommand {
     }
 
     fn examples(&self) -> Vec<Example> {
-        vec![Example {
-            description: "Set nonzero_exit_errors to true",
-            example: "config set nonzero_exit_errors $true",
-            result: None,
-        }]
+        vec![
+            Example {
+                description: "Set auto pivoting",
+                example: "config set pivot_mode always",
+                result: None,
+            },
+            Example {
+                description: "Set line editor options",
+                example: "config set line_editor [[edit_mode, completion_type]; [emacs circular]]",
+                result: None,
+            },
+            Example {
+                description: "Set coloring options",
+                example: "config set color_config [[header_align header_bold]; [left $true]]",
+                result: None,
+            },
+            Example {
+                description: "Set nested options",
+                example: "config set color_config.header_color white",
+                result: None,
+            },
+        ]
     }
 }
 
@@ -50,18 +66,32 @@ pub async fn set(
     args: CommandArgs,
     registry: &CommandRegistry,
 ) -> Result<OutputStream, ShellError> {
-    let name_span = args.call_info.name_tag.clone();
-    let (SetArgs { key, value }, _) = args.process(&registry).await?;
+    let name_tag = args.call_info.name_tag.clone();
+    let (SetArgs { path, mut value }, _) = args.process(&registry).await?;
 
     // NOTE: None because we are not loading a new config file, we just want to read from the
     // existing config
-    let mut result = nu_data::config::read(name_span, &None)?;
+    let raw_entries = nu_data::config::read(&name_tag, &None)?;
+    let configuration = UntaggedValue::row(raw_entries).into_value(&name_tag);
 
-    result.insert(key.to_string(), value.clone());
+    if let UntaggedValue::Table(rows) = &value.value {
+        if rows.len() == 1 && rows[0].is_row() {
+            value = rows[0].clone();
+        }
+    }
 
-    config::write(&result, &None)?;
+    match configuration.forgiving_insert_data_at_column_path(&path, value) {
+        Ok(Value {
+            value: UntaggedValue::Row(changes),
+            ..
+        }) => {
+            config::write(&changes.entries, &None)?;
 
-    Ok(OutputStream::one(ReturnSuccess::value(
-        UntaggedValue::Row(result.into()).into_value(&value.tag),
-    )))
+            Ok(OutputStream::one(ReturnSuccess::value(
+                UntaggedValue::Row(changes).into_value(name_tag),
+            )))
+        }
+        Ok(_) => Ok(OutputStream::empty()),
+        Err(reason) => Err(reason),
+    }
 }

crates/nu-data/Cargo.toml

@@ -33,6 +33,7 @@ nu-protocol = {version = "0.20.0", path = "../nu-protocol"}
 nu-source = {version = "0.20.0", path = "../nu-source"}
 nu-table = {version = "0.20.0", path = "../nu-table"}
 nu-value-ext = {version = "0.20.0", path = "../nu-value-ext"}
+nu-test-support = {version = "0.20.0", path = "../nu-test-support"}
 
 [target.'cfg(unix)'.dependencies]
 users = "0.10.0"

crates/nu-data/src/base.rs

@@ -165,38 +165,13 @@ pub fn coerce_compare_primitive(
 }
 #[cfg(test)]
 mod tests {
-    use indexmap::{indexmap, IndexMap};
     use nu_errors::ShellError;
-    use nu_protocol::{ColumnPath as ColumnPathValue, PathMember, UntaggedValue, Value};
-    use nu_source::*;
-    use nu_value_ext::{as_column_path, ValueExt};
-    use num_bigint::BigInt;
+    use nu_protocol::UntaggedValue;
+    use nu_source::SpannedItem;
+    use nu_test_support::value::*;
+    use nu_value_ext::ValueExt;
 
-    fn string(input: impl Into<String>) -> Value {
-        crate::utils::helpers::string(input)
-    }
-
-    fn int(input: impl Into<BigInt>) -> Value {
-        crate::utils::helpers::int(input)
-    }
-
-    fn row(entries: IndexMap<String, Value>) -> Value {
-        crate::utils::helpers::row(entries)
-    }
-
-    fn table(list: &[Value]) -> Value {
-        crate::utils::helpers::table(list)
-    }
-
-    fn error_callback(
-        reason: &'static str,
-    ) -> impl FnOnce(&Value, &PathMember, ShellError) -> ShellError {
-        move |_obj_source, _column_path_tried, _err| ShellError::unimplemented(reason)
-    }
-
-    fn column_path(paths: &[Value]) -> Result<Tagged<ColumnPathValue>, ShellError> {
-        as_column_path(&table(paths))
-    }
+    use indexmap::indexmap;
 
     #[test]
     fn gets_matching_field_from_a_row() -> Result<(), ShellError> {

crates/nu-protocol/src/value.rs

@@ -92,6 +92,11 @@ impl UntaggedValue {
         matches!(self, UntaggedValue::Table(_))
     }
 
+    /// Returns true if this value represents a row
+    pub fn is_row(&self) -> bool {
+        matches!(self, UntaggedValue::Row(_))
+    }
+
     /// Returns true if this value represents a string
     pub fn is_string(&self) -> bool {
         matches!(self, UntaggedValue::Primitive(Primitive::String(_)))

crates/nu-test-support/Cargo.toml

@@ -12,11 +12,15 @@ doctest = false
 [dependencies]
 nu-protocol = {path = "../nu-protocol", version = "0.20.0"}
 nu-source = {path = "../nu-source", version = "0.20.0"}
+nu-errors = {version = "0.20.0", path = "../nu-errors"}
+nu-value-ext = {version = "0.20.0", path = "../nu-value-ext"}
 
+chrono = "0.4.15"
 dunce = "1.0.1"
 getset = "0.1.1"
 glob = "0.3.0"
 indexmap = {version = "1.6.0", features = ["serde-1"]}
 tempfile = "3.1.0"
+num-bigint = {version = "0.3.0", features = ["serde"]}
 
 [build-dependencies]

crates/nu-test-support/src/lib.rs

@@ -2,6 +2,7 @@ pub mod commands;
 pub mod fs;
 pub mod macros;
 pub mod playground;
+pub mod value;
 
 pub fn pipeline(commands: &str) -> String {
     commands

crates/nu-test-support/src/value.rs

@@ -0,0 +1,50 @@
+use chrono::{DateTime, NaiveDate, Utc};
+use indexmap::IndexMap;
+use nu_errors::ShellError;
+use nu_protocol::{ColumnPath, PathMember, Primitive, UntaggedValue, Value};
+use nu_source::{Span, Tagged, TaggedItem};
+use nu_value_ext::as_column_path;
+use num_bigint::BigInt;
+
+pub fn int(s: impl Into<BigInt>) -> Value {
+    UntaggedValue::int(s).into_untagged_value()
+}
+
+pub fn decimal_from_float(f: f64, span: Span) -> Value {
+    UntaggedValue::decimal_from_float(f, span).into_untagged_value()
+}
+
+pub fn string(input: impl Into<String>) -> Value {
+    UntaggedValue::string(input.into()).into_untagged_value()
+}
+
+pub fn row(entries: IndexMap<String, Value>) -> Value {
+    UntaggedValue::row(entries).into_untagged_value()
+}
+
+pub fn table(list: &[Value]) -> Value {
+    UntaggedValue::table(list).into_untagged_value()
+}
+
+pub fn date(input: impl Into<String>) -> Value {
+    let key = input.into().tagged_unknown();
+
+    let date = NaiveDate::parse_from_str(key.borrow_tagged().item, "%Y-%m-%d")
+        .expect("date from string failed");
+
+    UntaggedValue::Primitive(Primitive::Date(DateTime::<Utc>::from_utc(
+        date.and_hms(12, 34, 56),
+        Utc,
+    )))
+    .into_untagged_value()
+}
+
+pub fn column_path(paths: &[Value]) -> Result<Tagged<ColumnPath>, ShellError> {
+    as_column_path(&table(paths))
+}
+
+pub fn error_callback(
+    reason: &'static str,
+) -> impl FnOnce(&Value, &PathMember, ShellError) -> ShellError {
+    move |_obj_source, _column_path_tried, _err| ShellError::unimplemented(reason)
+}

crates/nu-value-ext/Cargo.toml

@@ -18,4 +18,5 @@ indexmap = {version = "1.6.0", features = ["serde-1"]}
 itertools = "0.9.0"
 num-traits = "0.2.12"
 
-[build-dependencies]
+[dev-dependencies]
+nu-test-support = {path = "../nu-test-support", version = "0.20.0"}

crates/nu-value-ext/src/lib.rs

@@ -1,3 +1,4 @@
+use indexmap::indexmap;
 use indexmap::set::IndexSet;
 use itertools::Itertools;
 use nu_errors::{ExpectedRange, ShellError};
@@ -31,6 +32,11 @@ pub trait ValueExt {
         member: &PathMember,
         new_value: Value,
     ) -> Result<(), ShellError>;
+    fn forgiving_insert_data_at_column_path(
+        &self,
+        split_path: &ColumnPath,
+        new_value: Value,
+    ) -> Result<Value, ShellError>;
     fn insert_data_at_column_path(
         &self,
         split_path: &ColumnPath,
@@ -99,6 +105,14 @@ impl ValueExt for Value {
         insert_data_at_column_path(self, split_path, new_value)
     }
 
+    fn forgiving_insert_data_at_column_path(
+        &self,
+        split_path: &ColumnPath,
+        new_value: Value,
+    ) -> Result<Value, ShellError> {
+        forgiving_insert_data_at_column_path(self, split_path, new_value)
+    }
+
     fn replace_data_at_column_path(
         &self,
         split_path: &ColumnPath,
@@ -217,7 +231,7 @@ where
 pub fn swap_data_by_column_path<F>(
     value: &Value,
     path: &ColumnPath,
-    get_replacement: F,
+    callback: F,
 ) -> Result<Value, ShellError>
 where
     F: FnOnce(&Value) -> Result<Value, ShellError>,
@@ -357,8 +371,8 @@ where
             error
         });
 
-    let to_replace = to_replace?;
-    let replacement = get_replacement(&to_replace)?;
+    let old_value = to_replace?;
+    let replacement = callback(&old_value)?;
 
     value
         .replace_data_at_column_path(&path, replacement)
@@ -458,6 +472,101 @@ pub fn insert_data_at_member(
     }
 }
 
+pub fn missing_path_members_by_column_path(value: &Value, path: &ColumnPath) -> Option<usize> {
+    let mut current = value.clone();
+
+    for (idx, p) in path.iter().enumerate() {
+        if let Ok(value) = get_data_by_member(&current, p) {
+            current = value;
+        } else {
+            return Some(idx);
+        }
+    }
+
+    None
+}
+
+pub fn forgiving_insert_data_at_column_path(
+    value: &Value,
+    split_path: &ColumnPath,
+    new_value: Value,
+) -> Result<Value, ShellError> {
+    let mut original = value.clone();
+
+    if let Some(missed_at) = missing_path_members_by_column_path(value, split_path) {
+        let mut paths = split_path.iter().skip(missed_at + 1).collect::<Vec<_>>();
+        paths.reverse();
+
+        let mut candidate = new_value;
+
+        for member in paths.iter() {
+            match &member.unspanned {
+                UnspannedPathMember::String(column_name) => {
+                    candidate =
+                        UntaggedValue::row(indexmap! { column_name.into() => candidate.clone()})
+                            .into_value(&candidate.tag)
+                }
+                UnspannedPathMember::Int(int) => {
+                    let mut rows = vec![];
+                    let size = int.to_usize().unwrap_or_else(|| 0);
+
+                    for _ in 0..=size {
+                        rows.push(
+                            UntaggedValue::Primitive(Primitive::Nothing).into_value(&candidate.tag),
+                        );
+                    }
+                    rows.push(candidate.clone());
+                    candidate = UntaggedValue::Table(rows).into_value(&candidate.tag);
+                }
+            }
+        }
+
+        let cp = ColumnPath::new(
+            split_path
+                .iter()
+                .cloned()
+                .take(split_path.members().len() - missed_at + 1)
+                .collect::<Vec<_>>(),
+        );
+
+        if missed_at == 0 {
+            let current: &mut Value = &mut original;
+            insert_data_at_member(current, &cp.members()[0], candidate)?;
+            return Ok(original);
+        }
+
+        if value
+            .get_data_by_column_path(&cp, Box::new(move |_, _, err| err))
+            .is_ok()
+        {
+            return insert_data_at_column_path(&value, &cp, candidate);
+        } else if let Some((last, front)) = cp.split_last() {
+            let mut current: &mut Value = &mut original;
+
+            for member in front {
+                let type_name = current.spanned_type_name();
+
+                current = get_mut_data_by_member(current, &member).ok_or_else(|| {
+                    ShellError::missing_property(
+                        member.plain_string(std::usize::MAX).spanned(member.span),
+                        type_name,
+                    )
+                })?
+            }
+
+            insert_data_at_member(current, &last, candidate)?;
+
+            return Ok(original);
+        } else {
+            return Err(ShellError::untagged_runtime_error(
+                "Internal error: could not split column path correctly",
+            ));
+        }
+    }
+
+    insert_data_at_column_path(&value, split_path, new_value)
+}
+
 pub fn insert_data_at_column_path(
     value: &Value,
     split_path: &ColumnPath,
@@ -592,12 +701,24 @@ fn insert_data_at_index(
     index: Tagged<usize>,
     new_value: Value,
 ) -> Result<(), ShellError> {
-    if list.len() >= index.item {
-        Err(ShellError::range_error(
-            0..(list.len()),
-            &format_args!("{}", index.item).spanned(index.tag.span),
-            "insert at index",
-        ))
+    if index.item >= list.len() {
+        if index.item == list.len() {
+            list.push(new_value);
+            return Ok(());
+        }
+
+        let mut idx = list.len();
+
+        loop {
+            list.push(UntaggedValue::Primitive(Primitive::Nothing).into_value(&new_value.tag));
+
+            idx += 1;
+
+            if idx == index.item {
+                list.push(new_value);
+                return Ok(());
+            }
+        }
     } else {
         list[index.item] = new_value;
         Ok(())
@@ -689,3 +810,105 @@ pub(crate) fn get_mut_data_by_member<'value>(
         _ => None,
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use nu_test_support::value::*;
+
+    use indexmap::indexmap;
+
+    #[test]
+    fn forgiving_insertion_test_1() {
+        let field_path = column_path(&[string("crate"), string("version")]).unwrap();
+
+        let version = string("nuno");
+
+        let value = UntaggedValue::row(indexmap! {
+            "package".into() =>
+                row(indexmap! {
+                    "name".into()    =>     string("nu"),
+                    "version".into() =>  string("0.20.0")
+                })
+        });
+
+        assert_eq!(
+            *value
+                .into_untagged_value()
+                .forgiving_insert_data_at_column_path(&field_path, version)
+                .unwrap()
+                .get_data_by_column_path(&field_path, Box::new(error_callback("crate.version")))
+                .unwrap(),
+            *string("nuno")
+        );
+    }
+
+    #[test]
+    fn forgiving_insertion_test_2() {
+        let field_path = column_path(&[string("things"), int(0)]).unwrap();
+
+        let version = string("arepas");
+
+        let value = UntaggedValue::row(indexmap! {
+            "pivot_mode".into() => string("never"),
+            "things".into() => table(&[string("frijoles de Andrés"), int(1)]),
+            "color_config".into() =>
+                row(indexmap! {
+                    "header_align".into()    =>     string("left"),
+                    "index_color".into() =>  string("cyan_bold")
+                })
+        });
+
+        assert_eq!(
+            *value
+                .into_untagged_value()
+                .forgiving_insert_data_at_column_path(&field_path, version)
+                .unwrap()
+                .get_data_by_column_path(&field_path, Box::new(error_callback("things.0")))
+                .unwrap(),
+            *string("arepas")
+        );
+    }
+
+    #[test]
+    fn forgiving_insertion_test_3() {
+        let field_path = column_path(&[string("color_config"), string("arepa_color")]).unwrap();
+        let pizza_path = column_path(&[string("things"), int(0)]).unwrap();
+
+        let entry = string("amarillo");
+
+        let value = UntaggedValue::row(indexmap! {
+            "pivot_mode".into() => string("never"),
+            "things".into() => table(&[string("Arepas de Yehuda"), int(1)]),
+            "color_config".into() =>
+                row(indexmap! {
+                    "header_align".into()    =>     string("left"),
+                    "index_color".into() =>  string("cyan_bold")
+                })
+        });
+
+        assert_eq!(
+            *value
+                .clone()
+                .into_untagged_value()
+                .forgiving_insert_data_at_column_path(&field_path, entry.clone())
+                .unwrap()
+                .get_data_by_column_path(
+                    &field_path,
+                    Box::new(error_callback("color_config.arepa_color"))
+                )
+                .unwrap(),
+            *string("amarillo")
+        );
+
+        assert_eq!(
+            *value
+                .into_untagged_value()
+                .forgiving_insert_data_at_column_path(&field_path, entry)
+                .unwrap()
+                .get_data_by_column_path(&pizza_path, Box::new(error_callback("things.0")))
+                .unwrap(),
+            *string("Arepas de Yehuda")
+        );
+    }
+}