holm/nushell
1
1
Fork 0
forked from extern/nushell
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Make external command substitution works friendly(like fish shell, trailing ending newlines) (#7156)

Browse Source 
# 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 width="523" alt="img"
src="https://user-images.githubusercontent.com/22256154/202468810-86b04dbb-c147-459a-96a5-e0095eeaab3d.png">

After:
<img width="505" alt="img"
src="https://user-images.githubusercontent.com/22256154/202468599-7b537488-3d6b-458e-9d75-d85780826db0.png">


# 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.
This commit is contained in:
WindSoilder
2022-11-23 11:51:57 +08:00
committed by GitHub
parent 8cda641350
commit b662c2eb96
18 changed files with 162 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
crates/nu-parser/src/flatten.rs
View File

@ -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 {

71
crates/nu-parser/src/parser.rs
View File

@ -276,6 +276,7 @@ pub fn parse_external_call(
working_set: &mut StateWorkingSet,
spans: &[Span],
expand_aliases_denylist: &[usize],
is_subexpression: bool,
) -> (Expression, Option<ParseError>) {
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<ParseError>) {
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<ParseError>) {
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<ParseError>) {
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);