Simplify empty?, improve default (#4797)

* Simplify empty?, improve default

* improve test

crates/nu-command/src/filters/default.rs

@@ -1,7 +1,7 @@
 use nu_engine::CallExt;
 use nu_protocol::ast::Call;
 use nu_protocol::engine::{Command, EngineState, Stack};
-use nu_protocol::{Category, Example, PipelineData, Signature, Spanned, SyntaxShape, Value};
+use nu_protocol::{Category, Example, PipelineData, Signature, Span, Spanned, SyntaxShape, Value};
 
 #[derive(Clone)]
 pub struct Default;
@@ -13,12 +13,12 @@ impl Command for Default {
 
     fn signature(&self) -> Signature {
         Signature::build("default")
-            .required("column name", SyntaxShape::String, "the name of the column")
             .required(
-                "column value",
+                "default value",
                 SyntaxShape::Any,
-                "the value of the column to default",
+                "the value to use as a default",
             )
+            .optional("column name", SyntaxShape::String, "the name of the column")
             .category(Category::Filters)
     }
 
@@ -37,11 +37,26 @@ impl Command for Default {
     }
 
     fn examples(&self) -> Vec<Example> {
-        vec![Example {
+        vec![
-            description: "Give a default 'target' to all file entries",
+            Example {
-            example: "ls -la | default target 'nothing'",
+                description: "Give a default 'target' column to all file entries",
-            result: None,
+                example: "ls -la | default 'nothing' target ",
-        }]
+                result: None,
+            },
+            Example {
+                description: "Default the `$nothing` value in a list",
+                example: "[1, 2, $nothing, 4] | default 3",
+                result: Some(Value::List {
+                    vals: vec![
+                        Value::test_int(1),
+                        Value::test_int(2),
+                        Value::test_int(3),
+                        Value::test_int(4),
+                    ],
+                    span: Span::test_data(),
+                }),
+            },
+        ]
     }
 }
 
@@ -51,42 +66,52 @@ fn default(
     call: &Call,
     input: PipelineData,
 ) -> Result<nu_protocol::PipelineData, nu_protocol::ShellError> {
-    let column: Spanned<String> = call.req(engine_state, stack, 0)?;
+    let value: Value = call.req(engine_state, stack, 0)?;
-    let value: Value = call.req(engine_state, stack, 1)?;
+    let column: Option<Spanned<String>> = call.opt(engine_state, stack, 1)?;
 
     let ctrlc = engine_state.ctrlc.clone();
 
-    input.map(
+    if let Some(column) = column {
-        move |item| match item {
+        input.map(
-            Value::Record {
+            move |item| match item {
-                mut cols,
+                Value::Record {
-                mut vals,
+                    mut cols,
-                span,
+                    mut vals,
-            } => {
+                    span,
-                let mut idx = 0;
+                } => {
-                let mut found = false;
+                    let mut idx = 0;
+                    let mut found = false;
 
-                while idx < cols.len() {
+                    while idx < cols.len() {
-                    if cols[idx] == column.item {
+                        if cols[idx] == column.item {
-                        found = true;
+                            found = true;
-                        if matches!(vals[idx], Value::Nothing { .. }) {
+                            if matches!(vals[idx], Value::Nothing { .. }) {
-                            vals[idx] = value.clone();
+                                vals[idx] = value.clone();
+                            }
                         }
+                        idx += 1;
                     }
-                    idx += 1;
-                }
 
-                if !found {
+                    if !found {
-                    cols.push(column.item.clone());
+                        cols.push(column.item.clone());
-                    vals.push(value.clone());
+                        vals.push(value.clone());
-                }
+                    }
 
-                Value::Record { cols, vals, span }
+                    Value::Record { cols, vals, span }
-            }
+                }
-            _ => item,
+                _ => item,
-        },
+            },
-        ctrlc,
+            ctrlc,
-    )
+        )
+    } else {
+        input.map(
+            move |item| match item {
+                Value::Nothing { .. } => value.clone(),
+                x => x,
+            },
+            ctrlc,
+        )
+    }
 }
 
 #[cfg(test)]

crates/nu-command/src/filters/empty.rs

@@ -1,8 +1,8 @@
-use nu_engine::{eval_block, CallExt};
+use nu_engine::CallExt;
 use nu_protocol::ast::{Call, CellPath};
 use nu_protocol::engine::{Command, EngineState, Stack};
 use nu_protocol::{
-    Category, Example, PipelineData, ShellError, Signature, Span, SyntaxShape, Value,
+    Category, Example, IntoPipelineData, PipelineData, Signature, Span, SyntaxShape, Value,
 };
 
 #[derive(Clone)]
@@ -20,12 +20,6 @@ impl Command for Empty {
                 SyntaxShape::CellPath,
                 "the names of the columns to check emptiness",
             )
-            .named(
-                "block",
-                SyntaxShape::Block(Some(vec![SyntaxShape::Any])),
-                "an optional block to replace if empty",
-                Some('b'),
-            )
             .category(Category::Filters)
     }
 
@@ -46,7 +40,7 @@ impl Command for Empty {
     fn examples(&self) -> Vec<Example> {
         vec![
             Example {
-                description: "Check if a value is empty",
+                description: "Check if a string is empty",
                 example: "'' | empty?",
                 result: Some(Value::Bool {
                     val: true,
@@ -54,46 +48,21 @@ impl Command for Empty {
                 }),
             },
             Example {
-                description: "more than one column",
+                description: "Check if a list is empty",
-                example: "[[meal size]; [arepa small] [taco '']] | empty? meal size",
+                example: "[] | empty?",
-                result: Some(
+                result: Some(Value::Bool {
-                    Value::List {
+                    val: true,
-                        vals: vec![
+                    span: Span::test_data(),
-                            Value::Record{cols: vec!["meal".to_string(), "size".to_string()], vals: vec![
+                }),
-                                Value::Bool{val: false, span: Span::test_data()},
-                                Value::Bool{val: false, span: Span::test_data()}
-                            ], span: Span::test_data()},
-                            Value::Record{cols: vec!["meal".to_string(), "size".to_string()], vals: vec![
-                                Value::Bool{val: false, span: Span::test_data()},
-                                Value::Bool{val: true, span: Span::test_data()}
-                            ], span: Span::test_data()}
-                        ], span: Span::test_data()
-                    })
             },
             Example {
-                description: "use a block if setting the empty cell contents is wanted",
+                description: "Check if more than one column are empty",
-                example: "[[2020/04/16 2020/07/10 2020/11/16]; ['' [27] [37]]] | empty? 2020/04/16 -b { |_| [33 37] }",
+                example: "[[meal size]; [arepa small] [taco '']] | empty? meal size",
-                result: Some(
+                result: Some(Value::Bool {
-                    Value::List {
+                    val: false,
-                        vals: vec![
+                    span: Span::test_data(),
-                            Value::Record{
+                }),
-                            cols: vec!["2020/04/16".to_string(), "2020/07/10".to_string(), "2020/11/16".to_string()],
+            },
-                            vals: vec![
-                                Value::List{vals: vec![
-                                    Value::Int{val: 33, span: Span::test_data()},
-                                    Value::Int{val: 37, span: Span::test_data()}
-                                ], span: Span::test_data()},
-                                Value::List{vals: vec![
-                                    Value::Int{val: 27, span: Span::test_data()},
-                                ], span: Span::test_data()},
-                                Value::List{vals: vec![
-                                    Value::Int{val: 37, span: Span::test_data()},
-                                ], span: Span::test_data()},
-                            ], span: Span::test_data()}
-                        ], span: Span::test_data()
-                    }
-                )
-            }
         ]
     }
 }
@@ -106,126 +75,77 @@ fn empty(
 ) -> Result<nu_protocol::PipelineData, nu_protocol::ShellError> {
     let head = call.head;
     let columns: Vec<CellPath> = call.rest(engine_state, stack, 0)?;
-    let has_block = call.has_flag("block");
 
-    let block = if has_block {
+    if !columns.is_empty() {
-        let block_expr = call
+        for val in input {
-            .get_flag_expr("block")
+            for column in &columns {
-            .expect("internal error: expected block");
+                let val = val.clone();
+                match val.follow_cell_path(&column.members) {
+                    Ok(Value::Nothing { .. }) => {}
+                    Ok(_) => {
+                        return Ok(Value::Bool {
+                            val: false,
+                            span: head,
+                        }
+                        .into_pipeline_data())
+                    }
+                    Err(err) => return Err(err),
+                }
+            }
+        }
 
-        let block_id = block_expr
+        Ok(Value::Bool {
-            .as_block()
+            val: true,
-            .ok_or_else(|| ShellError::TypeMismatch("expected row condition".to_owned(), head))?;
+            span: head,
-
+        }
-        let b = engine_state.get_block(block_id);
+        .into_pipeline_data())
-        let evaluated_block = eval_block(
-            engine_state,
-            stack,
-            b,
-            PipelineData::new(head),
-            call.redirect_stdout,
-            call.redirect_stderr,
-        )?;
-        Some(evaluated_block.into_value(head))
     } else {
-        None
+        match input {
-    };
+            PipelineData::ExternalStream { stdout, .. } => match stdout {
+                Some(s) => {
+                    let bytes = s.into_bytes();
 
-    input.map(
+                    match bytes {
-        move |value| {
+                        Ok(s) => Ok(Value::Bool {
-            let columns = columns.clone();
+                            val: s.item.is_empty(),
-
+                            span: head,
-            process_row(value, block.as_ref(), columns, head)
-        },
-        engine_state.ctrlc.clone(),
-    )
-}
-
-fn process_row(
-    input: Value,
-    default_block: Option<&Value>,
-    column_paths: Vec<CellPath>,
-    head: Span,
-) -> Value {
-    match input {
-        Value::Record {
-            cols: _,
-            ref vals,
-            span,
-        } => {
-            if column_paths.is_empty() {
-                let is_empty = vals.iter().all(|v| v.clone().is_empty());
-                if default_block.is_some() {
-                    if is_empty {
-                        Value::Bool { val: true, span }
-                    } else {
-                        input.clone()
-                    }
-                } else {
-                    Value::Bool {
-                        val: is_empty,
-                        span,
-                    }
-                }
-            } else {
-                let mut obj = input.clone();
-                for column in column_paths {
-                    let path = column.into_string();
-                    let data = input.get_data_by_key(&path);
-                    let is_empty = match data {
-                        Some(x) => x.is_empty(),
-                        None => true,
-                    };
-
-                    let default = if let Some(x) = default_block {
-                        if is_empty {
-                            x.clone()
-                        } else {
-                            Value::Bool { val: true, span }
                         }
-                    } else {
+                        .into_pipeline_data()),
-                        Value::Bool {
+                        Err(err) => Err(err),
-                            val: is_empty,
-                            span,
-                        }
-                    };
-                    let r = obj.update_cell_path(&column.members, Box::new(move |_| default));
-                    if let Err(error) = r {
-                        return Value::Error { error };
                     }
                 }
-                obj
+                None => Ok(Value::Bool {
-            }
+                    val: true,
-        }
+                    span: head,
-        Value::List { vals, .. } if vals.iter().all(|v| v.as_record().is_ok()) => {
-            {
-                // we have records
-                if column_paths.is_empty() {
-                    let is_empty = vals.is_empty() && vals.iter().all(|v| v.clone().is_empty());
-
-                    Value::Bool {
-                        val: is_empty,
-                        span: head,
-                    }
-                } else {
-                    Value::Bool {
-                        val: true,
-                        span: head,
-                    }
                 }
-            }
+                .into_pipeline_data()),
-        }
+            },
-        Value::List { vals, .. } => {
+            PipelineData::ListStream(s, ..) => Ok(Value::Bool {
-            let empty = vals.iter().all(|v| v.clone().is_empty());
+                val: s.count() == 0,
-            Value::Bool {
-                val: empty,
                 span: head,
             }
+            .into_pipeline_data()),
+            PipelineData::Value(value, ..) => {
+                let answer = is_empty(value);
+
+                Ok(Value::Bool {
+                    val: answer,
+                    span: head,
+                }
+                .into_pipeline_data())
+            }
         }
-        other => Value::Bool {
+    }
-            val: other.is_empty(),
+}
-            span: head,
+
-        },
+pub fn is_empty(value: Value) -> bool {
+    match value {
+        Value::List { vals, .. } => vals.is_empty(),
+        Value::String { val, .. } => val.is_empty(),
+        Value::Binary { val, .. } => val.is_empty(),
+        Value::Nothing { .. } => true,
+        Value::Record { cols, .. } => cols.is_empty(),
+        _ => false,
     }
 }
 

crates/nu-command/tests/commands/rename.rs

@@ -50,7 +50,7 @@ fn keeps_remaining_original_names_given_less_new_names_than_total_original_names
                 open los_cuatro_mosqueteros.txt
                 | lines
                 | wrap name
-                | default hit "arepa!"
+                | default "arepa!" hit
                 | rename mosqueteros
                 | get hit
                 | length

src/tests/test_table_operations.rs

@@ -235,7 +235,7 @@ fn length_for_rows() -> TestResult {
 #[test]
 fn length_defaulted_columns() -> TestResult {
     run_test(
-        r#"echo [[name, age]; [test, 10]] | default age 11 | get 0 | columns | length"#,
+        r#"echo [[name, age]; [test, 10]] | default 11 age | get 0 | columns | length"#,
         "2",
     )
 }