From 8df9ea6c68e2bb3397849458c2cf57f85e006d60 Mon Sep 17 00:00:00 2001
From: JT <jonathan.d.turner@gmail.com>
Date: Sun, 10 Oct 2021 05:58:33 +1300
Subject: [PATCH] Add a couple more tests to for

---
 crates/nu-command/src/core_commands/for_.rs | 96 +++++++++------------
 crates/nu-protocol/src/value/mod.rs         | 46 +++++++++-
 2 files changed, 87 insertions(+), 55 deletions(-)

diff --git a/crates/nu-command/src/core_commands/for_.rs b/crates/nu-command/src/core_commands/for_.rs
index 7da8a03eac..0ba33bfd0d 100644
--- a/crates/nu-command/src/core_commands/for_.rs
+++ b/crates/nu-command/src/core_commands/for_.rs
@@ -1,7 +1,7 @@
 use nu_engine::{eval_block, eval_expression};
 use nu_protocol::ast::Call;
 use nu_protocol::engine::{Command, EvaluationContext};
-use nu_protocol::{Example, IntoValueStream, Signature, Span, SyntaxShape, Value};
+use nu_protocol::{Example, Signature, Span, SyntaxShape, Value};
 
 pub struct For;
 
@@ -23,10 +23,7 @@ impl Command for For {
             )
             .required(
                 "range",
-                SyntaxShape::Keyword(
-                    b"in".to_vec(),
-                    Box::new(SyntaxShape::List(Box::new(SyntaxShape::Int))),
-                ),
+                SyntaxShape::Keyword(b"in".to_vec(), Box::new(SyntaxShape::Any)),
                 "range of the loop",
             )
             .required(
@@ -55,42 +52,22 @@ impl Command for For {
         let block = call.positional[2]
             .as_block()
             .expect("internal error: expected block");
+
         let context = context.clone();
 
-        match values {
-            Value::Stream { stream, .. } => Ok(Value::Stream {
-                stream: stream
-                    .map(move |x| {
-                        let engine_state = context.engine_state.borrow();
-                        let block = engine_state.get_block(block);
+        Ok(values.map(call.head, move |x| {
+            let engine_state = context.engine_state.borrow();
+            let block = engine_state.get_block(block);
 
-                        let state = context.enter_scope();
-                        state.add_var(var_id, x);
+            let state = context.enter_scope();
 
-                        //FIXME: DON'T UNWRAP
-                        eval_block(&state, block, Value::nothing()).unwrap()
-                    })
-                    .into_value_stream(),
-                span: call.head,
-            }),
-            Value::List { vals: val, .. } => Ok(Value::List {
-                vals: val
-                    .into_iter()
-                    .map(move |x| {
-                        let engine_state = context.engine_state.borrow();
-                        let block = engine_state.get_block(block);
+            state.add_var(var_id, x);
 
-                        let state = context.enter_scope();
-                        state.add_var(var_id, x);
-
-                        //FIXME: DON'T UNWRAP
-                        eval_block(&state, block, Value::nothing()).unwrap()
-                    })
-                    .collect(),
-                span: call.head,
-            }),
-            _ => Ok(Value::nothing()),
-        }
+            match eval_block(&state, block, Value::nothing()) {
+                Ok(value) => value,
+                Err(error) => Value::Error { error },
+            }
+        }))
     }
 
     fn examples(&self) -> Vec<Example> {
@@ -120,23 +97,36 @@ impl Command for For {
                     span: Span::unknown(),
                 }),
             },
-            Example {
-                description: "Number each item and echo a message",
-                example: "for $it in ['bob' 'fred'] --numbered { $\"($it.index) is ($it.item)\" }",
-                result: Some(Value::List {
-                    vals: vec![
-                        Value::String {
-                            val: "0 is bob".into(),
-                            span,
-                        },
-                        Value::String {
-                            val: "0 is fred".into(),
-                            span,
-                        },
-                    ],
-                    span: Span::unknown(),
-                }),
-            },
+            // FIXME? Numbered `for` is kinda strange, but was supported in previous nushell
+            // Example {
+            //     description: "Number each item and echo a message",
+            //     example: "for $it in ['bob' 'fred'] --numbered { $\"($it.index) is ($it.item)\" }",
+            //     result: Some(Value::List {
+            //         vals: vec![
+            //             Value::String {
+            //                 val: "0 is bob".into(),
+            //                 span,
+            //             },
+            //             Value::String {
+            //                 val: "0 is fred".into(),
+            //                 span,
+            //             },
+            //         ],
+            //         span: Span::unknown(),
+            //     }),
+            // },
         ]
     }
 }
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    #[test]
+    fn test_examples() {
+        use crate::test_examples;
+
+        test_examples(For {})
+    }
+}
diff --git a/crates/nu-protocol/src/value/mod.rs b/crates/nu-protocol/src/value/mod.rs
index 7e4a8c7e2d..334790ab27 100644
--- a/crates/nu-protocol/src/value/mod.rs
+++ b/crates/nu-protocol/src/value/mod.rs
@@ -379,6 +379,10 @@ impl Value {
                 stream: stream.map(f).into_value_stream(),
                 span,
             },
+            Value::Range { val, .. } => Value::Stream {
+                stream: val.into_iter().map(f).into_value_stream(),
+                span,
+            },
             v => {
                 if v.as_string().is_ok() {
                     Value::List {
@@ -413,6 +417,10 @@ impl Value {
                 stream: stream.map(f).flatten().into_value_stream(),
                 span,
             },
+            Value::Range { val, .. } => Value::Stream {
+                stream: val.into_iter().map(f).flatten().into_value_stream(),
+                span,
+            },
             v => {
                 if v.as_string().is_ok() {
                     Value::List {
@@ -482,8 +490,42 @@ impl PartialEq for Value {
                     stream: stream_rhs, ..
                 },
             ) => {
-                let vals_lhs = stream_lhs.clone().collect_string();
-                let vals_rhs = stream_rhs.clone().collect_string();
+                let vals_lhs: Vec<Value> = stream_lhs.clone().collect();
+                let vals_rhs: Vec<Value> = stream_rhs.clone().collect();
+
+                vals_lhs == vals_rhs
+            }
+            // Note: This may look a bit strange, but a Stream is still just a List,
+            // it just happens to be in an iterator form instead of a concrete form. If the contained
+            // values are the same then it should be treated as equal
+            (
+                Value::Stream {
+                    stream: stream_lhs, ..
+                },
+                Value::List {
+                    vals: stream_rhs, ..
+                },
+            ) => {
+                let vals_lhs: Vec<Value> = stream_lhs.clone().collect();
+                let vals_rhs: Vec<Value> =
+                    stream_rhs.clone().into_iter().into_value_stream().collect();
+
+                vals_lhs == vals_rhs
+            }
+            // Note: This may look a bit strange, but a Stream is still just a List,
+            // it just happens to be in an iterator form instead of a concrete form. If the contained
+            // values are the same then it should be treated as equal
+            (
+                Value::List {
+                    vals: stream_lhs, ..
+                },
+                Value::Stream {
+                    stream: stream_rhs, ..
+                },
+            ) => {
+                let vals_lhs: Vec<Value> =
+                    stream_lhs.clone().into_iter().into_value_stream().collect();
+                let vals_rhs: Vec<Value> = stream_rhs.clone().collect();
 
                 vals_lhs == vals_rhs
             }