From b662c2eb96db02823a82c3073b90e764b69edd0a Mon Sep 17 00:00:00 2001 From: WindSoilder Date: Wed, 23 Nov 2022 11:51:57 +0800 Subject: [PATCH] Make external command substitution works friendly(like fish shell, trailing ending newlines) (#7156) # Description As title, when execute external sub command, auto-trimming end new-lines, like how fish shell does. And if the command is executed directly like: `cat tmp`, the result won't change. Fixes: #6816 Fixes: #3980 Note that although nushell works correctly by directly replace output of external command to variable(or other places like string interpolation), it's not friendly to user, and users almost want to use `str trim` to trim trailing newline, I think that's why fish shell do this automatically. If the pr is ok, as a result, no more `str trim -r` is required when user is writing scripts which using external commands. # User-Facing Changes Before: img After: img # 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 --features=extra -- -D warnings -D clippy::unwrap_used -A clippy::needless_collect` to check that you're using the standard code style - `cargo test --workspace --features=extra` 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/core_commands/do_.rs | 4 ++ .../nu-command/src/env/config/config_env.rs | 1 + crates/nu-command/src/env/config/config_nu.rs | 1 + crates/nu-command/src/filesystem/open.rs | 1 + crates/nu-command/src/network/fetch.rs | 1 + crates/nu-command/src/network/post.rs | 1 + .../nu-command/src/strings/format/command.rs | 2 +- crates/nu-command/src/system/exec.rs | 1 + crates/nu-command/src/system/run_external.rs | 5 ++ crates/nu-command/src/viewers/table.rs | 2 + crates/nu-engine/src/eval.rs | 29 +++++++- crates/nu-parser/src/flatten.rs | 2 +- crates/nu-parser/src/parser.rs | 71 +++++++++++++++---- crates/nu-protocol/src/ast/expr.rs | 2 +- crates/nu-protocol/src/ast/expression.rs | 6 +- crates/nu-protocol/src/pipeline_data.rs | 38 ++++++++-- src/main.rs | 1 + tests/shell/pipeline/commands/external.rs | 19 +++++ 18 files changed, 162 insertions(+), 25 deletions(-) diff --git a/crates/nu-command/src/core_commands/do_.rs b/crates/nu-command/src/core_commands/do_.rs index d79f041d8b..54a4c9169a 100644 --- a/crates/nu-command/src/core_commands/do_.rs +++ b/crates/nu-command/src/core_commands/do_.rs @@ -116,6 +116,7 @@ impl Command for Do { exit_code, span, metadata, + trim_end_newline, }) if capture_errors => { let mut exit_code_ctrlc = None; let exit_code: Vec = match exit_code { @@ -149,6 +150,7 @@ impl Command for Do { )), span, metadata, + trim_end_newline, }) } Ok(PipelineData::ExternalStream { @@ -157,12 +159,14 @@ impl Command for Do { exit_code: _, span, metadata, + trim_end_newline, }) if ignore_program_errors => Ok(PipelineData::ExternalStream { stdout, stderr, exit_code: None, span, metadata, + trim_end_newline, }), Err(_) if ignore_shell_errors => Ok(PipelineData::new(call.head)), r => r, diff --git a/crates/nu-command/src/env/config/config_env.rs b/crates/nu-command/src/env/config/config_env.rs index 294de0108d..de48bac2e8 100644 --- a/crates/nu-command/src/env/config/config_env.rs +++ b/crates/nu-command/src/env/config/config_env.rs @@ -74,6 +74,7 @@ impl Command for ConfigEnv { redirect_stdout: false, redirect_stderr: false, env_vars: env_vars_str, + trim_end_newline: false, }; command.run_with_input(engine_state, stack, input, true) diff --git a/crates/nu-command/src/env/config/config_nu.rs b/crates/nu-command/src/env/config/config_nu.rs index da88d08177..0224438c4e 100644 --- a/crates/nu-command/src/env/config/config_nu.rs +++ b/crates/nu-command/src/env/config/config_nu.rs @@ -74,6 +74,7 @@ impl Command for ConfigNu { redirect_stdout: false, redirect_stderr: false, env_vars: env_vars_str, + trim_end_newline: false, }; command.run_with_input(engine_state, stack, input, true) diff --git a/crates/nu-command/src/filesystem/open.rs b/crates/nu-command/src/filesystem/open.rs index 4bdc46bc12..1deaa521ad 100644 --- a/crates/nu-command/src/filesystem/open.rs +++ b/crates/nu-command/src/filesystem/open.rs @@ -142,6 +142,7 @@ impl Command for Open { exit_code: None, span: call_span, metadata: None, + trim_end_newline: false, }; let ext = if raw { diff --git a/crates/nu-command/src/network/fetch.rs b/crates/nu-command/src/network/fetch.rs index 9452473510..3f57b5222e 100644 --- a/crates/nu-command/src/network/fetch.rs +++ b/crates/nu-command/src/network/fetch.rs @@ -582,6 +582,7 @@ fn response_to_buffer( exit_code: None, span, metadata: None, + trim_end_newline: false, } } diff --git a/crates/nu-command/src/network/post.rs b/crates/nu-command/src/network/post.rs index 0c34ba13a1..bdb486687f 100644 --- a/crates/nu-command/src/network/post.rs +++ b/crates/nu-command/src/network/post.rs @@ -430,6 +430,7 @@ fn response_to_buffer( exit_code: None, span, metadata: None, + trim_end_newline: false, } } // Only panics if the user agent is invalid but we define it statically so either diff --git a/crates/nu-command/src/strings/format/command.rs b/crates/nu-command/src/strings/format/command.rs index fe960e82ac..34e27ba276 100644 --- a/crates/nu-command/src/strings/format/command.rs +++ b/crates/nu-command/src/strings/format/command.rs @@ -273,7 +273,7 @@ fn format_record( } } FormatOperation::ValueNeedEval(_col_name, span) => { - let (exp, may_parse_err) = parse_expression(working_set, &[*span], &[]); + let (exp, may_parse_err) = parse_expression(working_set, &[*span], &[], false); match may_parse_err { None => { let parsed_result = eval_expression(engine_state, stack, &exp); diff --git a/crates/nu-command/src/system/exec.rs b/crates/nu-command/src/system/exec.rs index 91a29622b1..95b3d813bb 100644 --- a/crates/nu-command/src/system/exec.rs +++ b/crates/nu-command/src/system/exec.rs @@ -93,6 +93,7 @@ fn exec( env_vars, redirect_stdout: true, redirect_stderr: false, + trim_end_newline: false, }; let mut command = external_command.spawn_simple_command(&cwd.to_string_lossy())?; diff --git a/crates/nu-command/src/system/run_external.rs b/crates/nu-command/src/system/run_external.rs index dd474c3341..a5c1493206 100644 --- a/crates/nu-command/src/system/run_external.rs +++ b/crates/nu-command/src/system/run_external.rs @@ -36,6 +36,7 @@ impl Command for External { Signature::build(self.name()) .switch("redirect-stdout", "redirect stdout to the pipeline", None) .switch("redirect-stderr", "redirect stderr to the pipeline", None) + .switch("trim-end-newline", "trimming end newlines", None) .required("command", SyntaxShape::Any, "external command to run") .rest("args", SyntaxShape::Any, "arguments for external command") .category(Category::System) @@ -52,6 +53,7 @@ impl Command for External { let args: Vec = call.rest(engine_state, stack, 1)?; let redirect_stdout = call.has_flag("redirect-stdout"); let redirect_stderr = call.has_flag("redirect-stderr"); + let trim_end_newline = call.has_flag("trim-end-newline"); // Translate environment variables from Values to Strings let env_vars_str = env_to_strings(engine_state, stack)?; @@ -109,6 +111,7 @@ impl Command for External { redirect_stdout, redirect_stderr, env_vars: env_vars_str, + trim_end_newline, }; command.run_with_input(engine_state, stack, input, false) } @@ -137,6 +140,7 @@ pub struct ExternalCommand { pub redirect_stdout: bool, pub redirect_stderr: bool, pub env_vars: HashMap, + pub trim_end_newline: bool, } impl ExternalCommand { @@ -466,6 +470,7 @@ impl ExternalCommand { )), span: head, metadata: None, + trim_end_newline: self.trim_end_newline, }) } } diff --git a/crates/nu-command/src/viewers/table.rs b/crates/nu-command/src/viewers/table.rs index 294dedba47..9d4f841d8a 100644 --- a/crates/nu-command/src/viewers/table.rs +++ b/crates/nu-command/src/viewers/table.rs @@ -256,6 +256,7 @@ fn handle_table_command( exit_code: None, span: call.head, metadata: None, + trim_end_newline: false, }), PipelineData::Value(Value::List { vals, .. }, metadata) => handle_row_stream( engine_state, @@ -709,6 +710,7 @@ fn handle_row_stream( exit_code: None, span: head, metadata: None, + trim_end_newline: false, }) } diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs index bb0cef67a3..a9905198bf 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -200,6 +200,7 @@ pub fn redirect_env(engine_state: &EngineState, caller_stack: &mut Stack, callee /// Eval extarnal expression /// /// It returns PipelineData with a boolean flag, indicate that if the external runs to failed. +#[allow(clippy::too_many_arguments)] fn eval_external( engine_state: &EngineState, stack: &mut Stack, @@ -208,6 +209,7 @@ fn eval_external( input: PipelineData, redirect_stdout: bool, redirect_stderr: bool, + is_subexpression: bool, ) -> Result { let decl_id = engine_state .find_decl("run-external".as_bytes(), &[]) @@ -245,6 +247,17 @@ fn eval_external( )) } + if is_subexpression { + call.add_named(( + Spanned { + item: "trim-end-newline".into(), + span: head.span, + }, + None, + None, + )) + } + command.run(engine_state, stack, &call, input) } @@ -331,7 +344,7 @@ pub fn eval_expression( .into_value(call.head), ) } - Expr::ExternalCall(head, args) => { + Expr::ExternalCall(head, args, is_subexpression) => { let span = head.span; // FIXME: protect this collect with ctrl-c Ok(eval_external( @@ -342,6 +355,7 @@ pub fn eval_expression( PipelineData::new(span), false, false, + *is_subexpression, )? .into_value(span)) } @@ -681,7 +695,7 @@ pub fn eval_expression_with_input( } } Expression { - expr: Expr::ExternalCall(head, args), + expr: Expr::ExternalCall(head, args, is_subexpression), .. } => { input = eval_external( @@ -692,6 +706,7 @@ pub fn eval_expression_with_input( input, redirect_stdout, redirect_stderr, + *is_subexpression, )?; } @@ -727,6 +742,7 @@ fn might_consume_external_result(input: PipelineData) -> (PipelineData, bool) { mut exit_code, span, metadata, + trim_end_newline, } = input { let exit_code = exit_code.take(); @@ -772,6 +788,7 @@ fn might_consume_external_result(input: PipelineData) -> (PipelineData, bool) { exit_code: Some(ListStream::from_stream(exit_code.into_iter(), ctrlc)), span, metadata, + trim_end_newline, }, runs_to_failed, ) @@ -783,6 +800,7 @@ fn might_consume_external_result(input: PipelineData) -> (PipelineData, bool) { exit_code: None, span, metadata, + trim_end_newline, }, runs_to_failed, ), @@ -819,6 +837,7 @@ pub fn eval_element_with_input( exit_code, span, metadata, + trim_end_newline, .. }, ) => PipelineData::ExternalStream { @@ -827,6 +846,7 @@ pub fn eval_element_with_input( exit_code, span, metadata, + trim_end_newline, }, ( Redirection::StdoutAndStderr, @@ -836,6 +856,7 @@ pub fn eval_element_with_input( exit_code, span, metadata, + trim_end_newline, }, ) => match (stdout, stderr) { (Some(stdout), Some(stderr)) => PipelineData::ExternalStream { @@ -844,6 +865,7 @@ pub fn eval_element_with_input( exit_code, span, metadata, + trim_end_newline, }, (None, Some(stderr)) => PipelineData::ExternalStream { stdout: Some(stderr), @@ -851,6 +873,7 @@ pub fn eval_element_with_input( exit_code, span, metadata, + trim_end_newline, }, (Some(stdout), None) => PipelineData::ExternalStream { stdout: Some(stdout), @@ -858,6 +881,7 @@ pub fn eval_element_with_input( exit_code, span, metadata, + trim_end_newline, }, (None, None) => PipelineData::ExternalStream { stdout: None, @@ -865,6 +889,7 @@ pub fn eval_element_with_input( exit_code, span, metadata, + trim_end_newline, }, }, (_, input) => input, diff --git a/crates/nu-parser/src/flatten.rs b/crates/nu-parser/src/flatten.rs index f5a655d77a..4a3d9177fb 100644 --- a/crates/nu-parser/src/flatten.rs +++ b/crates/nu-parser/src/flatten.rs @@ -175,7 +175,7 @@ pub fn flatten_expression( output.extend(args); output } - Expr::ExternalCall(head, args) => { + Expr::ExternalCall(head, args, _) => { let mut output = vec![]; match **head { diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index 718541a2de..7a24376c00 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -276,6 +276,7 @@ pub fn parse_external_call( working_set: &mut StateWorkingSet, spans: &[Span], expand_aliases_denylist: &[usize], + is_subexpression: bool, ) -> (Expression, Option) { trace!("parse external"); @@ -297,7 +298,8 @@ pub fn parse_external_call( let mut error = None; let head = if head_contents.starts_with(b"$") || head_contents.starts_with(b"(") { - let (arg, err) = parse_expression(working_set, &[head_span], expand_aliases_denylist); + // the expression is inside external_call, so it's a subexpression + let (arg, err) = parse_expression(working_set, &[head_span], expand_aliases_denylist, true); error = error.or(err); Box::new(arg) } else { @@ -348,7 +350,7 @@ pub fn parse_external_call( } ( Expression { - expr: Expr::ExternalCall(head, args), + expr: Expr::ExternalCall(head, args, is_subexpression), span: span(spans), ty: Type::Any, custom_completion: None, @@ -663,8 +665,14 @@ pub fn parse_multispan_value( SyntaxShape::Expression => { trace!("parsing: expression"); - let (arg, err) = - parse_expression(working_set, &spans[*spans_idx..], expand_aliases_denylist); + // is it subexpression? + // Not sure, but let's make it not, so the behavior is the same as previous version of nushell. + let (arg, err) = parse_expression( + working_set, + &spans[*spans_idx..], + expand_aliases_denylist, + false, + ); error = error.or(err); *spans_idx = spans.len() - 1; @@ -986,6 +994,7 @@ pub fn parse_call( spans: &[Span], head: Span, expand_aliases_denylist: &[usize], + is_subexpression: bool, ) -> (Expression, Option) { trace!("parsing: call"); @@ -1050,8 +1059,12 @@ pub fn parse_call( parts: new_spans.clone(), }; - let (mut result, err) = - parse_builtin_commands(working_set, &lite_command, &expand_aliases_denylist); + let (mut result, err) = parse_builtin_commands( + working_set, + &lite_command, + &expand_aliases_denylist, + is_subexpression, + ); let result = result.elements.remove(0); @@ -1150,7 +1163,12 @@ pub fn parse_call( trace!("parsing: external call"); // Otherwise, try external command - parse_external_call(working_set, spans, expand_aliases_denylist) + parse_external_call( + working_set, + spans, + expand_aliases_denylist, + is_subexpression, + ) } } @@ -4692,6 +4710,7 @@ pub fn parse_expression( working_set: &mut StateWorkingSet, spans: &[Span], expand_aliases_denylist: &[usize], + is_subexpression: bool, ) -> (Expression, Option) { let mut pos = 0; let mut shorthand = vec![]; @@ -4767,6 +4786,7 @@ pub fn parse_expression( &spans[pos..], spans[0], expand_aliases_denylist, + is_subexpression, ) .0, Some(ParseError::BuiltinCommandInPipeline("def".into(), spans[0])), @@ -4777,6 +4797,7 @@ pub fn parse_expression( &spans[pos..], spans[0], expand_aliases_denylist, + is_subexpression, ) .0, Some(ParseError::BuiltinCommandInPipeline( @@ -4790,6 +4811,7 @@ pub fn parse_expression( &spans[pos..], spans[0], expand_aliases_denylist, + is_subexpression, ) .0, Some(ParseError::BuiltinCommandInPipeline("for".into(), spans[0])), @@ -4800,6 +4822,7 @@ pub fn parse_expression( &spans[pos..], spans[0], expand_aliases_denylist, + is_subexpression, ) .0, Some(ParseError::LetInPipeline( @@ -4822,6 +4845,7 @@ pub fn parse_expression( &spans[pos..], spans[0], expand_aliases_denylist, + is_subexpression, ) .0, Some(ParseError::MutInPipeline( @@ -4844,6 +4868,7 @@ pub fn parse_expression( &spans[pos..], spans[0], expand_aliases_denylist, + is_subexpression, ) .0, Some(ParseError::BuiltinCommandInPipeline( @@ -4857,6 +4882,7 @@ pub fn parse_expression( &spans[pos..], spans[0], expand_aliases_denylist, + is_subexpression, ) .0, Some(ParseError::BuiltinCommandInPipeline( @@ -4870,6 +4896,7 @@ pub fn parse_expression( &spans[pos..], spans[0], expand_aliases_denylist, + is_subexpression, ) .0, Some(ParseError::BuiltinCommandInPipeline("use".into(), spans[0])), @@ -4882,6 +4909,7 @@ pub fn parse_expression( &spans[pos..], spans[0], expand_aliases_denylist, + is_subexpression, ) } else { ( @@ -4890,6 +4918,7 @@ pub fn parse_expression( &spans[pos..], spans[0], expand_aliases_denylist, + is_subexpression, ) .0, Some(ParseError::BuiltinCommandInPipeline( @@ -4905,6 +4934,7 @@ pub fn parse_expression( &spans[pos..], spans[0], expand_aliases_denylist, + is_subexpression, ) .0, Some(ParseError::BuiltinCommandInPipeline( @@ -4918,6 +4948,7 @@ pub fn parse_expression( &spans[pos..], spans[0], expand_aliases_denylist, + is_subexpression, ) .0, Some(ParseError::UnexpectedKeyword("export".into(), spans[0])), @@ -4928,6 +4959,7 @@ pub fn parse_expression( &spans[pos..], spans[0], expand_aliases_denylist, + is_subexpression, ) .0, Some(ParseError::BuiltinCommandInPipeline( @@ -4942,6 +4974,7 @@ pub fn parse_expression( &spans[pos..], spans[0], expand_aliases_denylist, + is_subexpression, ) .0, Some(ParseError::BuiltinCommandInPipeline( @@ -4955,6 +4988,7 @@ pub fn parse_expression( &spans[pos..], spans[0], expand_aliases_denylist, + is_subexpression, ), } }; @@ -5042,6 +5076,7 @@ pub fn parse_builtin_commands( working_set: &mut StateWorkingSet, lite_command: &LiteCommand, expand_aliases_denylist: &[usize], + is_subexpression: bool, ) -> (Pipeline, Option) { let name = working_set.get_span_contents(lite_command.parts[0]); @@ -5070,8 +5105,12 @@ pub fn parse_builtin_commands( #[cfg(feature = "plugin")] b"register" => parse_register(working_set, &lite_command.parts, expand_aliases_denylist), _ => { - let (expr, err) = - parse_expression(working_set, &lite_command.parts, expand_aliases_denylist); + let (expr, err) = parse_expression( + working_set, + &lite_command.parts, + expand_aliases_denylist, + is_subexpression, + ); (Pipeline::from_vec(vec![expr]), err) } } @@ -5218,8 +5257,8 @@ pub fn parse_block( working_set, &command.parts, expand_aliases_denylist, + is_subexpression, ); - working_set.type_scope.add_type(expr.ty.clone()); if error.is_none() { @@ -5248,6 +5287,7 @@ pub fn parse_block( working_set, &command.parts, expand_aliases_denylist, + is_subexpression, ); working_set.type_scope.add_type(expr.ty.clone()); @@ -5263,6 +5303,7 @@ pub fn parse_block( working_set, &command.parts, expand_aliases_denylist, + is_subexpression, ); working_set.type_scope.add_type(expr.ty.clone()); @@ -5297,8 +5338,12 @@ pub fn parse_block( | LiteElement::Redirection(_, _, command) | LiteElement::And(_, command) | LiteElement::Or(_, command) => { - let (mut pipeline, err) = - parse_builtin_commands(working_set, command, expand_aliases_denylist); + let (mut pipeline, err) = parse_builtin_commands( + working_set, + command, + expand_aliases_denylist, + is_subexpression, + ); if idx == 0 { if let Some(let_decl_id) = working_set.find_decl(b"let", &Type::Any) { @@ -5542,7 +5587,7 @@ pub fn discover_captures_in_expr( } Expr::CellPath(_) => {} Expr::DateTime(_) => {} - Expr::ExternalCall(head, exprs) => { + Expr::ExternalCall(head, exprs, _) => { let result = discover_captures_in_expr(working_set, head, seen, seen_blocks)?; output.extend(&result); diff --git a/crates/nu-protocol/src/ast/expr.rs b/crates/nu-protocol/src/ast/expr.rs index e85cbe2560..24f69c8ed5 100644 --- a/crates/nu-protocol/src/ast/expr.rs +++ b/crates/nu-protocol/src/ast/expr.rs @@ -19,7 +19,7 @@ pub enum Expr { Var(VarId), VarDecl(VarId), Call(Box), - ExternalCall(Box, Vec), + ExternalCall(Box, Vec, bool), // head, args, is_subexpression Operator(Operator), RowCondition(BlockId), UnaryNot(Box), diff --git a/crates/nu-protocol/src/ast/expression.rs b/crates/nu-protocol/src/ast/expression.rs index 3f48ea74c8..04b9546419 100644 --- a/crates/nu-protocol/src/ast/expression.rs +++ b/crates/nu-protocol/src/ast/expression.rs @@ -177,7 +177,7 @@ impl Expression { } Expr::CellPath(_) => false, Expr::DateTime(_) => false, - Expr::ExternalCall(head, args) => { + Expr::ExternalCall(head, args, _) => { if head.has_in_variable(working_set) { return true; } @@ -374,7 +374,7 @@ impl Expression { } Expr::CellPath(_) => {} Expr::DateTime(_) => {} - Expr::ExternalCall(head, args) => { + Expr::ExternalCall(head, args, _) => { head.replace_in_variable(working_set, new_var_id); for arg in args { arg.replace_in_variable(working_set, new_var_id) @@ -534,7 +534,7 @@ impl Expression { } Expr::CellPath(_) => {} Expr::DateTime(_) => {} - Expr::ExternalCall(head, args) => { + Expr::ExternalCall(head, args, _) => { head.replace_span(working_set, replaced, new_span); for arg in args { arg.replace_span(working_set, replaced, new_span) diff --git a/crates/nu-protocol/src/pipeline_data.rs b/crates/nu-protocol/src/pipeline_data.rs index 5c322c1c8d..cc03e1b2c3 100644 --- a/crates/nu-protocol/src/pipeline_data.rs +++ b/crates/nu-protocol/src/pipeline_data.rs @@ -6,6 +6,12 @@ use crate::{ use nu_utils::{stderr_write_all_and_flush, stdout_write_all_and_flush}; use std::sync::{atomic::AtomicBool, Arc}; +const LINE_ENDING: &str = if cfg!(target_os = "windows") { + "\r\n" +} else { + "\n" +}; + /// The foundational abstraction for input and output to commands /// /// This represents either a single Value or a stream of values coming into the command or leaving a command. @@ -45,6 +51,7 @@ pub enum PipelineData { exit_code: Option, span: Span, metadata: Option, + trim_end_newline: bool, }, } @@ -121,6 +128,7 @@ impl PipelineData { PipelineData::ExternalStream { stdout: Some(mut s), exit_code, + trim_end_newline, .. } => { let mut items = vec![]; @@ -141,6 +149,8 @@ impl PipelineData { let _: Vec<_> = exit_code.into_iter().collect(); } + // NOTE: currently trim-end-newline only handles for string output. + // For binary, user might need origin data. if s.is_binary { let mut output = vec![]; for item in items { @@ -168,6 +178,9 @@ impl PipelineData { } } } + if trim_end_newline { + output.truncate(output.trim_end_matches(LINE_ENDING).len()) + } Value::String { val: output, span, // FIXME? @@ -193,7 +206,9 @@ impl PipelineData { PipelineData::ListStream(s, ..) => Ok(s.into_string(separator, config)), PipelineData::ExternalStream { stdout: None, .. } => Ok(String::new()), PipelineData::ExternalStream { - stdout: Some(s), .. + stdout: Some(s), + trim_end_newline, + .. } => { let mut output = String::new(); @@ -206,6 +221,9 @@ impl PipelineData { Err(e) => return Err(e), } } + if trim_end_newline { + output.truncate(output.trim_end_matches(LINE_ENDING).len()); + } Ok(output) } } @@ -294,11 +312,15 @@ impl PipelineData { } PipelineData::ExternalStream { stdout: Some(stream), + trim_end_newline, .. } => { let collected = stream.into_bytes()?; - if let Ok(st) = String::from_utf8(collected.clone().item) { + if let Ok(mut st) = String::from_utf8(collected.clone().item) { + if trim_end_newline { + st.truncate(st.trim_end_matches(LINE_ENDING).len()); + } Ok(f(Value::String { val: st, span: collected.span, @@ -348,11 +370,15 @@ impl PipelineData { } PipelineData::ExternalStream { stdout: Some(stream), + trim_end_newline, .. } => { let collected = stream.into_bytes()?; - if let Ok(st) = String::from_utf8(collected.clone().item) { + if let Ok(mut st) = String::from_utf8(collected.clone().item) { + if trim_end_newline { + st.truncate(st.trim_end_matches(LINE_ENDING).len()) + } Ok(f(Value::String { val: st, span: collected.span, @@ -397,11 +423,15 @@ impl PipelineData { } PipelineData::ExternalStream { stdout: Some(stream), + trim_end_newline, .. } => { let collected = stream.into_bytes()?; - if let Ok(st) = String::from_utf8(collected.clone().item) { + if let Ok(mut st) = String::from_utf8(collected.clone().item) { + if trim_end_newline { + st.truncate(st.trim_end_matches(LINE_ENDING).len()) + } let v = Value::String { val: st, span: collected.span, diff --git a/src/main.rs b/src/main.rs index 461abdafac..c72d16f18e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -320,6 +320,7 @@ fn main() -> Result<()> { exit_code: None, span: redirect_stdin.span, metadata: None, + trim_end_newline: false, } } else { PipelineData::new(Span::new(0, 0)) diff --git a/tests/shell/pipeline/commands/external.rs b/tests/shell/pipeline/commands/external.rs index 8781b72efd..64cd742607 100644 --- a/tests/shell/pipeline/commands/external.rs +++ b/tests/shell/pipeline/commands/external.rs @@ -120,6 +120,25 @@ fn command_not_found_error_suggests_typo_fix() { assert!(actual.err.contains("benchmark")); } +#[test] +fn command_substitution_wont_output_extra_newline() { + let actual = nu!( + cwd: ".", + r#" + with-env [FOO "bar"] { echo $"prefix (nu --testbin echo_env FOO) suffix" } + "# + ); + assert_eq!(actual.out, "prefix bar suffix"); + + let actual = nu!( + cwd: ".", + r#" + with-env [FOO "bar"] { (nu --testbin echo_env FOO) } + "# + ); + assert_eq!(actual.out, "bar"); +} + mod it_evaluation { use super::nu; use nu_test_support::fs::Stub::{EmptyFile, FileWithContent, FileWithContentToBeTrimmed};