From 5afd74f0b90b326b5009c821d04a9f0240ec98b2 Mon Sep 17 00:00:00 2001
From: WindSoilder <WindSoilder@outlook.com>
Date: Sat, 10 Jun 2023 16:09:19 +0800
Subject: [PATCH] don't allow `save` command to save both stdout and stderr to
 the same file (#9368)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

# Description
It's not a good idea to save `stdout` and `stderr` to the same file from
`save` command directly.

Because it saves `stdout` and `stderr` in different thread, which leads
to in-consistent output. As replace, we can use `o+e` redirection to fix
the issue

# User-Facing Changes
```
❯ do -i { "aa" } | save foo.txt -e foo.txt
Error:   × input and stderr input to same file
   ╭─[entry #3:1:1]
 1 │ do -i { "aa" } | save foo.txt -e foo.txt
   ·                                  ───┬───
   ·                                     ╰── can't save both input and stderr input to the same file
   ╰────
  help: you should use `o+e> file` instead
```

# 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 -A clippy::result_large_err` to check that
you're using the standard code style
- `cargo test --workspace` to check that all tests pass
- `cargo run -- crates/nu-std/tests/run.nu` to run the tests for the
standard library

> **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/save.rs | 20 +++++++-------------
 crates/nu-command/tests/commands/save.rs | 14 +++++---------
 2 files changed, 12 insertions(+), 22 deletions(-)

diff --git a/crates/nu-command/src/filesystem/save.rs b/crates/nu-command/src/filesystem/save.rs
index 5b73a6d2ff..05fe8ce125 100644
--- a/crates/nu-command/src/filesystem/save.rs
+++ b/crates/nu-command/src/filesystem/save.rs
@@ -301,18 +301,6 @@ fn open_file(path: &Path, span: Span, append: bool) -> Result<File, ShellError>
     })
 }
 
-fn clone_file(file: &File, span: Span) -> Result<File, ShellError> {
-    file.try_clone().map_err(|err| {
-        ShellError::GenericError(
-            "Permission denied".into(),
-            err.to_string(),
-            Some(span),
-            None,
-            Vec::new(),
-        )
-    })
-}
-
 /// Get output file and optional stderr file
 fn get_files(
     path: &Spanned<String>,
@@ -333,7 +321,13 @@ fn get_files(
     let stderr_file = stderr_path_and_span
         .map(|(stderr_path, stderr_path_span)| {
             if path == stderr_path {
-                clone_file(&file, stderr_path_span)
+                Err(ShellError::GenericError(
+                    "input and stderr input to same file".to_string(),
+                    "can't save both input and stderr input to the same file".to_string(),
+                    Some(stderr_path_span),
+                    Some("you should use `o+e> file` instead".to_string()),
+                    vec![],
+                ))
             } else {
                 open_file(stderr_path, stderr_path_span, append)
             }
diff --git a/crates/nu-command/tests/commands/save.rs b/crates/nu-command/tests/commands/save.rs
index b28caee570..8bc16727e4 100644
--- a/crates/nu-command/tests/commands/save.rs
+++ b/crates/nu-command/tests/commands/save.rs
@@ -84,13 +84,11 @@ fn save_append_will_not_overwrite_content() {
 }
 
 #[test]
-fn save_stderr_and_stdout_to_same_file() {
+fn save_stderr_and_stdout_to_afame_file() {
     Playground::setup("save_test_5", |dirs, sandbox| {
         sandbox.with_files(vec![]);
 
-        let expected_file = dirs.test().join("new-file.txt");
-
-        nu!(
+        let actual = nu!(
             cwd: dirs.root(),
             r#"
             let-env FOO = "bar";
@@ -98,11 +96,9 @@ fn save_stderr_and_stdout_to_same_file() {
             do -c {nu -c 'nu --testbin echo_env FOO; nu --testbin echo_env_stderr BAZ'} | save -r save_test_5/new-file.txt --stderr save_test_5/new-file.txt
             "#,
         );
-
-        let actual = file_contents(expected_file);
-        println!("{}, {}", actual, actual.contains("ZZZ"));
-        assert!(actual.contains("bar"));
-        assert!(actual.contains("ZZZ"));
+        assert!(actual
+            .err
+            .contains("can't save both input and stderr input to the same file"));
     })
 }