Add rest and glob support to 'open' (#8506)

# Description

This adds two different features to `open`:
* The ability to pass more than one file to `open`.
* Support for using globs in the filenames

`open` will create a list stream and stream the output if there is more
than one file opened

Examples:

```
open file1.csv file2.csv file3.csv
```

```
open *.nu | where $it =~ "echo"
```

# User-Facing Changes

Multi-file and glob support in `open`. Original `open` functionality
should continue as before.

# Tests + Formatting

Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A
clippy::needless_collect` to check that you're using the standard code
style
- `cargo test --workspace` to check that all tests pass

> **Note**
> from `nushell` you can also use the `toolkit` as follows
> ```bash
> use toolkit.nu # or use an `env_change` hook to activate it
automatically
> toolkit check pr
> ```

# After Submitting

If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.

crates/nu-command/src/filesystem/open.rs

@@ -1,10 +1,10 @@
-use nu_engine::{eval_block, CallExt};
+use nu_engine::{current_dir, eval_block, CallExt};
 use nu_protocol::ast::Call;
 use nu_protocol::engine::{Command, EngineState, Stack};
 use nu_protocol::util::BufferedReader;
 use nu_protocol::{
-    Category, Example, PipelineData, RawStream, ShellError, Signature, Spanned, SyntaxShape, Type,
-    Value,
+    Category, Example, IntoInterruptiblePipelineData, PipelineData, RawStream, ShellError,
+    Signature, Spanned, SyntaxShape, Type, Value,
 };
 use std::io::BufReader;
 
@@ -38,6 +38,11 @@ impl Command for Open {
         Signature::build("open")
             .input_output_types(vec![(Type::Nothing, Type::Any), (Type::String, Type::Any)])
             .optional("filename", SyntaxShape::Filepath, "the filename to use")
+            .rest(
+                "filenames",
+                SyntaxShape::Filepath,
+                "optional additional files to open",
+            )
             .switch("raw", "open file as raw binary", Some('r'))
             .category(Category::FileSystem)
     }
@@ -52,24 +57,16 @@ impl Command for Open {
         let raw = call.has_flag("raw");
         let call_span = call.head;
         let ctrlc = engine_state.ctrlc.clone();
-        let path = call.opt::<Spanned<String>>(engine_state, stack, 0)?;
+        let cwd = current_dir(engine_state, stack)?;
+        let req_path = call.opt::<Spanned<String>>(engine_state, stack, 0)?;
+        let mut path_params = call.rest::<Spanned<String>>(engine_state, stack, 1)?;
 
-        let path = {
-            if let Some(path_val) = path {
-                Some(Spanned {
-                    item: nu_utils::strip_ansi_string_unlikely(path_val.item),
-                    span: path_val.span,
-                })
-            } else {
-                path
-            }
-        };
+        // FIXME: JT: what is this doing here?
 
-        let path = if let Some(path) = path {
-            path
+        if let Some(filename) = req_path {
+            path_params.insert(0, filename);
         } else {
-            // Collect a filename from the input
-            match input {
+            let filename = match input {
                 PipelineData::Value(Value::Nothing { .. }, ..) => {
                     return Err(ShellError::MissingParameter {
                         param_name: "needs filename".to_string(),
@@ -83,11 +80,28 @@ impl Command for Open {
                         span: call.head,
                     });
                 }
+            };
+
+            path_params.insert(0, filename);
+        }
+
+        let mut output = vec![];
+
+        for path in path_params.into_iter() {
+            //FIXME: `open` should not have to do this
+            let path = {
+                Spanned {
+                    item: nu_utils::strip_ansi_string_unlikely(path.item),
+                    span: path.span,
                 }
             };
+
             let arg_span = path.span;
-        let path_no_whitespace = &path.item.trim_end_matches(|x| matches!(x, '\x09'..='\x0d'));
-        let path = Path::new(path_no_whitespace);
+            // let path_no_whitespace = &path.item.trim_end_matches(|x| matches!(x, '\x09'..='\x0d'));
+
+            for path in nu_engine::glob_from(&path, &cwd, call_span, None)?.1 {
+                let path = path?;
+                let path = Path::new(&path);
 
                 if permission_denied(path) {
                     #[cfg(unix)]
@@ -101,13 +115,13 @@ impl Command for Open {
 
                     #[cfg(not(unix))]
                     let error_msg = String::from("Permission denied");
-            Err(ShellError::GenericError(
+                    return Err(ShellError::GenericError(
                         "Permission denied".into(),
                         error_msg,
                         Some(arg_span),
                         None,
                         Vec::new(),
-            ))
+                    ));
                 } else {
                     #[cfg(feature = "sqlite")]
                     if !raw {
@@ -134,10 +148,10 @@ impl Command for Open {
 
                     let buf_reader = BufReader::new(file);
 
-            let output = PipelineData::ExternalStream {
+                    let file_contents = PipelineData::ExternalStream {
                         stdout: Some(RawStream::new(
                             Box::new(BufferedReader { input: buf_reader }),
-                    ctrlc,
+                            ctrlc.clone(),
                             call_span,
                             None,
                         )),
@@ -159,13 +173,25 @@ impl Command for Open {
                         match engine_state.find_decl(format!("from {ext}").as_bytes(), &[]) {
                             Some(converter_id) => {
                                 let decl = engine_state.get_decl(converter_id);
-                        if let Some(block_id) = decl.get_block_id() {
+                                let command_output = if let Some(block_id) = decl.get_block_id() {
                                     let block = engine_state.get_block(block_id);
-                            eval_block(engine_state, stack, block, output, false, false)
+                                    eval_block(
+                                        engine_state,
+                                        stack,
+                                        block,
+                                        file_contents,
+                                        false,
+                                        false,
+                                    )
                                 } else {
-                            decl.run(engine_state, stack, &Call::new(call_span), output)
-                        }
-                        .map_err(|inner| {
+                                    decl.run(
+                                        engine_state,
+                                        stack,
+                                        &Call::new(call_span),
+                                        file_contents,
+                                    )
+                                };
+                                output.push(command_output.map_err(|inner| {
                                     ShellError::GenericError(
                                         format!("Error while parsing as {ext}"),
                                         format!("Could not parse '{}' with `from {}`", path.display(), ext),
@@ -173,15 +199,25 @@ impl Command for Open {
                                         Some(format!("Check out `help from {}` or `help from` for more options or open raw data with `open --raw '{}'`", ext, path.display())),
                                         vec![inner],
                                     )
-                        })
+                                })?);
                             }
-                    None => Ok(output),
+                            None => output.push(file_contents),
                         }
                     } else {
-                Ok(output)
+                        output.push(file_contents)
                     }
                 }
             }
+        }
+
+        if output.is_empty() {
+            Ok(PipelineData::Empty)
+        } else if output.len() == 1 {
+            Ok(output.remove(0))
+        } else {
+            Ok(output.into_iter().flatten().into_pipeline_data(ctrlc))
+        }
+    }
 
     fn examples(&self) -> Vec<nu_protocol::Example> {
         vec![

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

@@ -222,7 +222,7 @@ fn errors_if_file_not_found() {
     //
     // This seems to be not directly affected by localization compared to the OS
     // provided error message
-    let expected = "(os error 2)";
+    let expected = "not found";
 
     assert!(
         actual.err.contains(expected),
@@ -232,27 +232,28 @@ fn errors_if_file_not_found() {
     );
 }
 
-// FIXME: jt: I think `open` on a directory is confusing. We should make discuss this one a bit more
-#[ignore]
 #[test]
-fn open_dir_is_ls() {
-    Playground::setup("open_dir", |dirs, sandbox| {
-        sandbox.with_files(vec![
-            EmptyFile("yehuda.txt"),
-            EmptyFile("jttxt"),
-            EmptyFile("andres.txt"),
-        ]);
-
+fn open_wildcard() {
     let actual = nu!(
-            cwd: dirs.test(), pipeline(
+        cwd: "tests/fixtures/formats", pipeline(
         r#"
-                open .
-                | length
+            open *.nu | where $it =~ echo | length
         "#
     ));
 
-        assert_eq!(actual.out, "3");
-    })
+    assert_eq!(actual.out, "3")
+}
+
+#[test]
+fn open_multiple_files() {
+    let actual = nu!(
+        cwd: "tests/fixtures/formats", pipeline(
+        r#"
+        open caco3_plastics.csv caco3_plastics.tsv | get tariff_item | math sum
+        "#
+    ));
+
+    assert_eq!(actual.out, "58309279992")
 }
 
 #[test]