Ensure selection of columns are done once per column (#3012)

crates/nu-command/src/commands.rs

@@ -245,7 +245,7 @@ pub(crate) use reverse::Reverse;
 pub(crate) use rm::Remove;
 pub(crate) use run_external::RunExternalCommand;
 pub(crate) use save::Save;
-pub(crate) use select::Select;
+pub(crate) use select::Command as Select;
 pub(crate) use seq::Seq;
 pub(crate) use seq_dates::SeqDates;
 pub(crate) use shells::Shells;
@@ -299,6 +299,7 @@ mod tests {
             whole_stream_command(Move),
             whole_stream_command(Update),
             whole_stream_command(Empty),
+            //whole_stream_command(Select),
             // Str Command Suite
             whole_stream_command(Str),
             whole_stream_command(StrToDecimal),

crates/nu-command/src/commands/benchmark.rs

@@ -70,7 +70,7 @@ impl WholeStreamCommand for Benchmark {
 
 async fn benchmark(raw_args: CommandArgs) -> Result<OutputStream, ShellError> {
     let tag = raw_args.call_info.args.span;
-    let mut context = EvaluationContext::from_raw(&raw_args);
+    let mut context = EvaluationContext::from_args(&raw_args);
     let scope = raw_args.scope.clone();
     let (BenchmarkArgs { block, passthrough }, input) = raw_args.process().await?;
 

crates/nu-command/src/commands/do_.rs

@@ -55,7 +55,7 @@ impl WholeStreamCommand for Do {
 async fn do_(raw_args: CommandArgs) -> Result<OutputStream, ShellError> {
     let external_redirection = raw_args.call_info.args.external_redirection;
 
-    let context = EvaluationContext::from_raw(&raw_args);
+    let context = EvaluationContext::from_args(&raw_args);
     let (
         DoArgs {
             ignore_errors,

crates/nu-command/src/commands/each/command.rs

@@ -111,7 +111,7 @@ pub(crate) fn make_indexed_item(index: usize, item: Value) -> Value {
 }
 
 async fn each(raw_args: CommandArgs) -> Result<OutputStream, ShellError> {
-    let context = Arc::new(EvaluationContext::from_raw(&raw_args));
+    let context = Arc::new(EvaluationContext::from_args(&raw_args));
 
     let (each_args, input): (EachArgs, _) = raw_args.process().await?;
     let block = Arc::new(Box::new(each_args.block));

crates/nu-command/src/commands/each/group.rs

@@ -46,7 +46,7 @@ impl WholeStreamCommand for EachGroup {
     }
 
     async fn run(&self, raw_args: CommandArgs) -> Result<OutputStream, ShellError> {
-        let context = Arc::new(EvaluationContext::from_raw(&raw_args));
+        let context = Arc::new(EvaluationContext::from_args(&raw_args));
         let (each_args, input): (EachGroupArgs, _) = raw_args.process().await?;
         let block = Arc::new(Box::new(each_args.block));
 

crates/nu-command/src/commands/each/window.rs

@@ -51,7 +51,7 @@ impl WholeStreamCommand for EachWindow {
     }
 
     async fn run(&self, raw_args: CommandArgs) -> Result<OutputStream, ShellError> {
-        let context = Arc::new(EvaluationContext::from_raw(&raw_args));
+        let context = Arc::new(EvaluationContext::from_args(&raw_args));
         let (each_args, mut input): (EachWindowArgs, _) = raw_args.process().await?;
         let block = Arc::new(Box::new(each_args.block));
 

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

@@ -6,10 +6,10 @@ use nu_protocol::{
     hir::CapturedBlock, ColumnPath, Primitive, ReturnSuccess, Signature, SyntaxShape,
     UntaggedValue, Value,
 };
-use nu_source::Tagged;
-use nu_value_ext::{as_string, ValueExt};
 
+use crate::utils::arguments::arguments;
 use futures::stream::once;
+use nu_value_ext::{as_string, ValueExt};
 
 #[derive(Deserialize)]
 pub struct Arguments {
@@ -84,9 +84,10 @@ impl WholeStreamCommand for Command {
 async fn is_empty(args: CommandArgs) -> Result<OutputStream, ShellError> {
     let tag = args.call_info.name_tag.clone();
     let name_tag = Arc::new(args.call_info.name_tag.clone());
-    let context = Arc::new(EvaluationContext::from_raw(&args));
+    let context = Arc::new(EvaluationContext::from_args(&args));
-    let (Arguments { rest }, input) = args.process().await?;
+    let (Arguments { mut rest }, input) = args.process().await?;
-    let (columns, default_block): (Vec<ColumnPath>, Option<Box<CapturedBlock>>) = arguments(rest)?;
+    let (columns, default_block): (Vec<ColumnPath>, Option<Box<CapturedBlock>>) =
+        arguments(&mut rest)?;
     let default_block = Arc::new(default_block);
 
     if input.is_empty() {
@@ -130,37 +131,6 @@ async fn is_empty(args: CommandArgs) -> Result<OutputStream, ShellError> {
         .to_output_stream())
 }
 
-fn arguments(
-    rest: Vec<Value>,
-) -> Result<(Vec<ColumnPath>, Option<Box<CapturedBlock>>), ShellError> {
-    let mut rest = rest;
-    let mut columns = vec![];
-    let mut default = None;
-
-    let last_argument = rest.pop();
-
-    match last_argument {
-        Some(Value {
-            value: UntaggedValue::Block(call),
-            ..
-        }) => default = Some(call),
-        Some(other) => {
-            let Tagged { item: path, .. } = other.as_column_path()?;
-
-            columns = vec![path];
-        }
-        None => {}
-    };
-
-    for argument in rest {
-        let Tagged { item: path, .. } = argument.as_column_path()?;
-
-        columns.push(path);
-    }
-
-    Ok((columns, default))
-}
-
 async fn process_row(
     context: Arc<EvaluationContext>,
     input: Value,

crates/nu-command/src/commands/group_by.rs

@@ -130,7 +130,7 @@ enum Grouper {
 
 pub async fn group_by(args: CommandArgs) -> Result<OutputStream, ShellError> {
     let name = args.call_info.name_tag.clone();
-    let context = Arc::new(EvaluationContext::from_raw(&args));
+    let context = Arc::new(EvaluationContext::from_args(&args));
     let (Arguments { grouper }, input) = args.process().await?;
 
     let values: Vec<Value> = input.collect().await;

crates/nu-command/src/commands/if_.rs

@@ -66,7 +66,7 @@ impl WholeStreamCommand for If {
 }
 async fn if_command(raw_args: CommandArgs) -> Result<OutputStream, ShellError> {
     let tag = raw_args.call_info.name_tag.clone();
-    let context = Arc::new(EvaluationContext::from_raw(&raw_args));
+    let context = Arc::new(EvaluationContext::from_args(&raw_args));
 
     let (
         IfArgs {

crates/nu-command/src/commands/insert.rs

@@ -154,7 +154,7 @@ async fn process_row(
 }
 
 async fn insert(raw_args: CommandArgs) -> Result<OutputStream, ShellError> {
-    let context = Arc::new(EvaluationContext::from_raw(&raw_args));
+    let context = Arc::new(EvaluationContext::from_args(&raw_args));
     let (Arguments { column, value }, input) = raw_args.process().await?;
     let value = Arc::new(value);
     let column = Arc::new(column);

crates/nu-command/src/commands/merge.rs

@@ -47,7 +47,7 @@ impl WholeStreamCommand for Merge {
 }
 
 async fn merge(raw_args: CommandArgs) -> Result<OutputStream, ShellError> {
-    let context = EvaluationContext::from_raw(&raw_args);
+    let context = EvaluationContext::from_args(&raw_args);
     let name_tag = raw_args.call_info.name_tag.clone();
     let (merge_args, input): (MergeArgs, _) = raw_args.process().await?;
     let block = merge_args.block;

crates/nu-command/src/commands/reduce.rs

@@ -95,7 +95,7 @@ async fn process_row(
 
 async fn reduce(raw_args: CommandArgs) -> Result<OutputStream, ShellError> {
     let span = raw_args.call_info.name_tag.span;
-    let context = Arc::new(EvaluationContext::from_raw(&raw_args));
+    let context = Arc::new(EvaluationContext::from_args(&raw_args));
     let (reduce_args, mut input): (ReduceArgs, _) = raw_args.process().await?;
     let block = Arc::new(reduce_args.block);
     let (ioffset, start) = if !input.is_empty() {

crates/nu-command/src/commands/select.rs

@@ -1,30 +1,27 @@
 use crate::prelude::*;
+use crate::utils::arguments::arguments;
 use nu_engine::WholeStreamCommand;
 use nu_errors::ShellError;
 use nu_protocol::{
-    ColumnPath, PathMember, Primitive, ReturnSuccess, Signature, SyntaxShape, TaggedDictBuilder,
+    hir::CapturedBlock, ColumnPath, PathMember, Primitive, ReturnSuccess, Signature, SyntaxShape,
-    UnspannedPathMember, UntaggedValue, Value,
+    TaggedDictBuilder, UnspannedPathMember, UntaggedValue, Value,
 };
 use nu_value_ext::{as_string, get_data_by_column_path};
-
 #[derive(Deserialize)]
-struct SelectArgs {
+struct Arguments {
-    rest: Vec<ColumnPath>,
+    rest: Vec<Value>,
 }
 
-pub struct Select;
+pub struct Command;
 
 #[async_trait]
-impl WholeStreamCommand for Select {
+impl WholeStreamCommand for Command {
     fn name(&self) -> &str {
         "select"
     }
 
     fn signature(&self) -> Signature {
-        Signature::build("select").rest(
+        Signature::build("select").rest(SyntaxShape::Any, "the columns to select from the table")
-            SyntaxShape::ColumnPath,
-            "the columns to select from the table",
-        )
     }
 
     fn usage(&self) -> &str {
@@ -53,8 +50,10 @@ impl WholeStreamCommand for Select {
 
 async fn select(args: CommandArgs) -> Result<OutputStream, ShellError> {
     let name = args.call_info.name_tag.clone();
-    let (SelectArgs { rest: mut fields }, mut input) = args.process().await?;
+    let (Arguments { mut rest }, mut input) = args.process().await?;
-    if fields.is_empty() {
+    let (columns, _): (Vec<ColumnPath>, Option<Box<CapturedBlock>>) = arguments(&mut rest)?;
+
+    if columns.is_empty() {
         return Err(ShellError::labeled_error(
             "Select requires columns to select",
             "needs parameter",
@@ -62,18 +61,10 @@ async fn select(args: CommandArgs) -> Result<OutputStream, ShellError> {
         ));
     }
 
-    let member = fields.remove(0);
-    let member = vec![member];
-
-    let column_paths = vec![&member, &fields]
-        .into_iter()
-        .flatten()
-        .cloned()
-        .collect::<Vec<ColumnPath>>();
     let mut bring_back: indexmap::IndexMap<String, Vec<Value>> = indexmap::IndexMap::new();
 
     while let Some(value) = input.next().await {
-        for path in &column_paths {
+        for path in &columns {
             let fetcher = get_data_by_column_path(
                 &value,
                 &path,
@@ -165,16 +156,3 @@ async fn select(args: CommandArgs) -> Result<OutputStream, ShellError> {
     }))
     .to_output_stream())
 }
-
-#[cfg(test)]
-mod tests {
-    use super::Select;
-    use super::ShellError;
-
-    #[test]
-    fn examples_work_as_expected() -> Result<(), ShellError> {
-        use crate::examples::test as test_examples;
-
-        Ok(test_examples(Select {})?)
-    }
-}

crates/nu-command/src/commands/update.rs

@@ -174,7 +174,7 @@ async fn process_row(
 
 async fn update(raw_args: CommandArgs) -> Result<OutputStream, ShellError> {
     let name_tag = Arc::new(raw_args.call_info.name_tag.clone());
-    let context = Arc::new(EvaluationContext::from_raw(&raw_args));
+    let context = Arc::new(EvaluationContext::from_args(&raw_args));
     let (Arguments { field, replacement }, input) = raw_args.process().await?;
     let replacement = Arc::new(replacement);
     let field = Arc::new(field);

crates/nu-command/src/commands/where_.rs

@@ -61,7 +61,7 @@ impl WholeStreamCommand for Where {
     }
 }
 async fn where_command(raw_args: CommandArgs) -> Result<OutputStream, ShellError> {
-    let ctx = Arc::new(EvaluationContext::from_raw(&raw_args));
+    let ctx = Arc::new(EvaluationContext::from_args(&raw_args));
     let tag = raw_args.call_info.name_tag.clone();
     let (WhereArgs { block }, input) = raw_args.process().await?;
     let condition = {

crates/nu-command/src/commands/with_env.rs

@@ -69,7 +69,7 @@ impl WholeStreamCommand for WithEnv {
 }
 
 async fn with_env(raw_args: CommandArgs) -> Result<OutputStream, ShellError> {
-    let context = EvaluationContext::from_raw(&raw_args);
+    let context = EvaluationContext::from_args(&raw_args);
     let (WithEnvArgs { variable, block }, input) = raw_args.process().await?;
 
     let mut env = IndexMap::new();

crates/nu-command/src/utils.rs

@@ -1,2 +1,3 @@
+pub mod arguments;
 pub mod suggestions;
 pub mod test_bins;

crates/nu-command/src/utils/arguments.rs

@@ -0,0 +1,37 @@
+use indexmap::IndexSet;
+use nu_errors::ShellError;
+use nu_protocol::{hir::CapturedBlock, ColumnPath, UntaggedValue, Value};
+use nu_source::Tagged;
+use nu_value_ext::ValueExt;
+
+pub fn arguments(
+    rest: &mut Vec<Value>,
+) -> Result<(Vec<ColumnPath>, Option<Box<CapturedBlock>>), ShellError> {
+    let last_argument = rest.pop();
+
+    let mut columns: IndexSet<_> = rest.iter().collect();
+    let mut column_paths = vec![];
+
+    let mut default = None;
+
+    for argument in columns.drain(..) {
+        let Tagged { item: path, .. } = argument.as_column_path()?;
+
+        column_paths.push(path);
+    }
+
+    match last_argument {
+        Some(Value {
+            value: UntaggedValue::Block(call),
+            ..
+        }) => default = Some(call),
+        Some(other) => {
+            let Tagged { item: path, .. } = other.as_column_path()?;
+
+            column_paths.push(path);
+        }
+        None => {}
+    };
+
+    Ok((column_paths, default))
+}

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

@@ -4,21 +4,15 @@ use nu_test_support::{nu, pipeline};
 
 #[test]
 fn regular_columns() {
-    Playground::setup("select_test_1", |dirs, sandbox| {
+    let actual = nu!(cwd: ".", pipeline(
-        sandbox.with_files(vec![FileWithContentToBeTrimmed(
-            "los_tres_caballeros.csv",
         r#"
-                first_name,last_name,rusty_at,type
+            echo [
-                Andrés,Robalino,10/11/2013,A
+                [first_name, last_name, rusty_at, type];
-                Jonathan,Turner,10/12/2013,B
-                Yehuda,Katz,10/11/2013,A
-            "#,
-        )]);
 
-        let actual = nu!(
+                [Andrés Robalino 10/11/2013 A]
-            cwd: dirs.test(), pipeline(
+                [Jonathan Turner 10/12/2013 B]
-            r#"
+                [Yehuda Katz 10/11/2013 A]
-                open los_tres_caballeros.csv
+            ]
             | select rusty_at last_name
             | nth 0
             | get last_name
@@ -26,7 +20,6 @@ fn regular_columns() {
     ));
 
     assert_eq!(actual.out, "Robalino");
-    })
 }
 
 #[test]
@@ -73,52 +66,57 @@ fn complex_nested_columns() {
 
 #[test]
 fn allows_if_given_unknown_column_name_is_missing() {
-    Playground::setup("select_test_3", |dirs, sandbox| {
+    let actual = nu!(cwd: ".", pipeline(
-        sandbox.with_files(vec![FileWithContentToBeTrimmed(
-            "los_tres_caballeros.csv",
         r#"
-                    first_name,last_name,rusty_at,type
+            echo [
-                    Andrés,Robalino,10/11/2013,A
+                [first_name, last_name, rusty_at, type];
-                    Jonathan,Turner,10/12/2013,B
-                    Yehuda,Katz,10/11/2013,A
-                "#,
-        )]);
 
-        let actual = nu!(
+                [Andrés Robalino 10/11/2013 A]
-            cwd: dirs.test(), pipeline(
+                [Jonathan Turner 10/12/2013 B]
-            r#"
+                [Yehuda Katz 10/11/2013 A]
-                open los_tres_caballeros.csv
+            ]
             | select rrusty_at first_name
             | count
         "#
     ));
 
     assert_eq!(actual.out, "3");
-    })
 }
 
 #[test]
 fn column_names_with_spaces() {
-    Playground::setup("select_test_4", |dirs, sandbox| {
+    let actual = nu!(cwd: ".", pipeline(
-        sandbox.with_files(vec![FileWithContentToBeTrimmed(
-            "test_data.csv",
         r#"
-                    first name,last name
+            echo [
-                    Jonathan,Turner
+                ["first name", "last name"];
-                    Jonathan,Arns
-                "#,
-        )]);
 
-        let actual = nu!(
+                [Andrés Robalino]
-            cwd: dirs.test(), pipeline(
+                [Andrés Jnth]
-            r#"
+            ]
-                open test_data.csv
             | select "last name"
-                | to json
+            | get "last name"
+            | str collect " "
         "#
     ));
 
-        let expected_output = r#"[{"last name":"Turner"},{"last name":"Arns"}]"#;
+    assert_eq!(actual.out, "Robalino Jnth");
-        assert_eq!(actual.out, expected_output);
+}
-    })
+
+#[test]
+fn ignores_duplicate_columns_selected() {
+    let actual = nu!(cwd: ".", pipeline(
+        r#"
+            echo [
+                ["first name", "last name"];
+
+                [Andrés Robalino]
+                [Andrés Jnth]
+            ]
+            | select "first name" "last name" "first name"
+            | get
+            | str collect " "
+        "#
+    ));
+
+    assert_eq!(actual.out, "first name last name");
 }

crates/nu-engine/src/evaluation_context.rs

@@ -27,18 +27,6 @@ pub struct EvaluationContext {
 }
 
 impl EvaluationContext {
-    pub fn from_raw(raw_args: &CommandArgs) -> EvaluationContext {
-        EvaluationContext {
-            scope: raw_args.scope.clone(),
-            host: raw_args.host.clone(),
-            current_errors: raw_args.current_errors.clone(),
-            ctrl_c: raw_args.ctrl_c.clone(),
-            shell_manager: raw_args.shell_manager.clone(),
-            user_recently_used_autoenv_untrust: Arc::new(AtomicBool::new(false)),
-            windows_drives_previous_cwd: Arc::new(Mutex::new(std::collections::HashMap::new())),
-        }
-    }
-
     pub fn from_args(args: &CommandArgs) -> EvaluationContext {
         EvaluationContext {
             scope: args.scope.clone(),