From 99aea0c71c01d985e9122572a3f78a45c7c74399 Mon Sep 17 00:00:00 2001 From: Artemiy Date: Thu, 9 Feb 2023 21:29:34 +0300 Subject: [PATCH] Filesystem commands print --verbose to stderr (#8014) # Description Makes `mkdir`, `cp`, `mv` and `rm` return nothing and print info to stderr: ![image](https://user-images.githubusercontent.com/17511668/217859228-feffa4bc-c22d-45d3-b330-1903f5a4d938.png) See https://github.com/nushell/nushell/pull/7925#issuecomment-1420861638 and [discord](https://discord.com/channels/601130461678272522/615329862395101194/1072523941865857055). # User-Facing Changes `mkdir`, `cp`, `mv` and `rm` will return nothing and print info to stderr with `--verbose` flag. # 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 # 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/cp.rs | 11 ++++++--- crates/nu-command/src/filesystem/mkdir.rs | 8 ++++--- crates/nu-command/src/filesystem/mv.rs | 8 ++++--- crates/nu-command/src/filesystem/rm.rs | 9 +++++--- crates/nu-command/tests/commands/mkdir.rs | 4 +++- crates/nu-protocol/src/pipeline_data.rs | 27 +++++++++++++++++++++++ 6 files changed, 54 insertions(+), 13 deletions(-) diff --git a/crates/nu-command/src/filesystem/cp.rs b/crates/nu-command/src/filesystem/cp.rs index 4e6aa422a..36099ed03 100644 --- a/crates/nu-command/src/filesystem/cp.rs +++ b/crates/nu-command/src/filesystem/cp.rs @@ -279,14 +279,19 @@ impl Command for Cp { } if verbose { - Ok(result.into_iter().into_pipeline_data(ctrlc)) + result + .into_iter() + .into_pipeline_data(ctrlc) + .print_not_formatted(engine_state, false, true)?; } else { // filter to only errors - Ok(result + result .into_iter() .filter(|v| matches!(v, Value::Error { .. })) - .into_pipeline_data(ctrlc)) + .into_pipeline_data(ctrlc) + .print_not_formatted(engine_state, false, true)?; } + Ok(PipelineData::empty()) } fn examples(&self) -> Vec { diff --git a/crates/nu-command/src/filesystem/mkdir.rs b/crates/nu-command/src/filesystem/mkdir.rs index c66bd4fef..739ab0321 100644 --- a/crates/nu-command/src/filesystem/mkdir.rs +++ b/crates/nu-command/src/filesystem/mkdir.rs @@ -19,7 +19,7 @@ impl Command for Mkdir { fn signature(&self) -> Signature { Signature::build("mkdir") - .input_output_types(vec![(Type::Nothing, Type::List(Box::new(Type::String)))]) + .input_output_types(vec![(Type::Nothing, Type::Nothing)]) .rest( "rest", SyntaxShape::Directory, @@ -83,9 +83,11 @@ impl Command for Mkdir { } } - Ok(stream + stream .into_iter() - .into_pipeline_data(engine_state.ctrlc.clone())) + .into_pipeline_data(engine_state.ctrlc.clone()) + .print_not_formatted(engine_state, false, true)?; + Ok(PipelineData::empty()) } fn examples(&self) -> Vec { diff --git a/crates/nu-command/src/filesystem/mv.rs b/crates/nu-command/src/filesystem/mv.rs index 6f37298e3..ca17bb88b 100644 --- a/crates/nu-command/src/filesystem/mv.rs +++ b/crates/nu-command/src/filesystem/mv.rs @@ -35,7 +35,7 @@ impl Command for Mv { fn signature(&self) -> nu_protocol::Signature { Signature::build("mv") - .input_output_types(vec![(Type::Nothing, Type::List(Box::new(Type::String)))]) + .input_output_types(vec![(Type::Nothing, Type::Nothing)]) .required( "source", SyntaxShape::GlobPattern, @@ -177,7 +177,7 @@ impl Command for Mv { } let span = call.head; - Ok(sources + sources .into_iter() .flatten() .filter_map(move |entry| { @@ -212,7 +212,9 @@ impl Command for Mv { None } }) - .into_pipeline_data(ctrlc)) + .into_pipeline_data(ctrlc) + .print_not_formatted(engine_state, false, true)?; + Ok(PipelineData::empty()) } fn examples(&self) -> Vec { diff --git a/crates/nu-command/src/filesystem/rm.rs b/crates/nu-command/src/filesystem/rm.rs index 76c51fe80..00127088d 100644 --- a/crates/nu-command/src/filesystem/rm.rs +++ b/crates/nu-command/src/filesystem/rm.rs @@ -45,7 +45,7 @@ impl Command for Rm { fn signature(&self) -> Signature { let sig = Signature::build("rm") - .input_output_types(vec![(Type::Nothing, Type::List(Box::new(Type::String)))]) + .input_output_types(vec![(Type::Nothing, Type::Nothing)]) .required( "filename", SyntaxShape::Filepath, @@ -329,7 +329,7 @@ fn rm( } } - Ok(all_targets + all_targets .into_keys() .map(move |f| { let is_empty = || match f.read_dir() { @@ -450,5 +450,8 @@ fn rm( } }) .filter(|x| !matches!(x.get_type(), Type::Nothing)) - .into_pipeline_data(ctrlc)) + .into_pipeline_data(ctrlc) + .print_not_formatted(engine_state, false, true)?; + + Ok(PipelineData::empty()) } diff --git a/crates/nu-command/tests/commands/mkdir.rs b/crates/nu-command/tests/commands/mkdir.rs index 82ed7597d..f35a18e49 100644 --- a/crates/nu-command/tests/commands/mkdir.rs +++ b/crates/nu-command/tests/commands/mkdir.rs @@ -79,6 +79,8 @@ fn print_created_paths() { dirs.test() )); - assert_eq!(actual.out, "3"); + assert!(actual.err.contains("dir_1")); + assert!(actual.err.contains("dir_2")); + assert!(actual.err.contains("dir_3")); }) } diff --git a/crates/nu-protocol/src/pipeline_data.rs b/crates/nu-protocol/src/pipeline_data.rs index d3b4eae8a..725dc25e3 100644 --- a/crates/nu-protocol/src/pipeline_data.rs +++ b/crates/nu-protocol/src/pipeline_data.rs @@ -644,6 +644,33 @@ impl PipelineData { Ok(0) } + /// Consume and print self data immediately. + /// + /// Unlike [print] does not call `table` to format data and just prints it + /// one element on a line + /// * `no_newline` controls if we need to attach newline character to output. + /// * `to_stderr` controls if data is output to stderr, when the value is false, the data is output to stdout. + pub fn print_not_formatted( + self, + engine_state: &EngineState, + no_newline: bool, + to_stderr: bool, + ) -> Result { + let config = engine_state.get_config(); + + if let PipelineData::ExternalStream { + stdout: stream, + stderr: stderr_stream, + exit_code, + .. + } = self + { + print_if_stream(stream, stderr_stream, to_stderr, exit_code) + } else { + self.write_all_and_flush(engine_state, config, no_newline, to_stderr) + } + } + fn write_all_and_flush( self, engine_state: &EngineState,