extern/nushell
1
0
Fork 1
mirror of https://github.com/nushell/nushell.git synced 2024-11-22 16:33:37 +01:00
Code Issues Packages Projects Releases Wiki Activity

make the behaviours of last and first more consistent (#9582)

Browse Source 
<!--
if this PR closes one or more issues, you can automatically link the PR
with
them by using one of the [*linking
keywords*](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword),
e.g.
- this PR should close #xxxx
- fixes #xxxx

you can also mention related issues, PRs or discussions!
-->

# Description

- fixes #9567 

I have fixed everything mentioned in the issue, and made their help
messages more similar.

<!--
Thank you for improving Nushell. Please, check our [contributing
guide](../CONTRIBUTING.md) and talk to the core team before making major
changes.

Description of your pull request goes here. **Provide examples and/or
screenshots** if your changes affect the user experience.
-->

# User-Facing Changes
<!-- List of all changes that impact the user experience here. This
helps us keep track of breaking changes. -->

- Previously, `last` on binary data returned an integer. Now it returns
a binary
- Now, `[] | last` and `[] | first` are both errors.
- Now, `ls | table | first` and `ls | table | last` are both errors.
This commit is contained in:
baehyunsol 2023-07-04 00:19:50 +09:00 committed by GitHub
parent 9998fa50a3
commit 227d1d9508
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 207 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
crates/nu-command/src/filters/first.rs
View File

@ -105,6 +105,13 @@ fn first_helper(
let ctrlc = engine_state.ctrlc.clone();
let metadata = input.metadata();
// early exit for `first 0`
if rows_desired == 0 {
return Ok(Vec::<Value>::new()
.into_pipeline_data(ctrlc)
.set_metadata(metadata));
}
match input {
PipelineData::Value(val, _) => match val {
Value::List { vals, .. } => {
@ -123,11 +130,25 @@ fn first_helper(
}
}
Value::Binary { val, span } => {
let slice: Vec<u8> = val.into_iter().take(rows_desired).collect();
Ok(PipelineData::Value(
Value::Binary { val: slice, span },
metadata,
))
if return_single_element {
if val.is_empty() {
Err(ShellError::AccessEmptyContent { span: head })
} else {
Ok(PipelineData::Value(
Value::Int {
val: val[0] as i64,
span,
},
metadata,
))
}
} else {
let slice: Vec<u8> = val.into_iter().take(rows_desired).collect();
Ok(PipelineData::Value(
Value::Binary { val: slice, span },
metadata,
))
}
}
Value::Range { val, .. } => {
if return_single_element {

146
crates/nu-command/src/filters/last.rs
View File

@ -35,6 +35,7 @@ impl Command for Last {
Type::List(Box::new(Type::Any)),
Type::Any,
),
(Type::Binary, Type::Binary),
])
.optional(
"rows",
@ -52,7 +53,7 @@ impl Command for Last {
vec![
Example {
example: "[1,2,3] | last 2",
description: "Get the last 2 items",
description: "Return the last 2 items of a list/table",
result: Some(Value::List {
vals: vec![Value::test_int(2), Value::test_int(3)],
span: Span::test_data(),
@ -60,9 +61,17 @@ impl Command for Last {
},
Example {
example: "[1,2,3] | last",
description: "Get the last item",
description: "Return the last item of a list/table",
result: Some(Value::test_int(3)),
},
Example {
example: "0x[01 23 45] | last 2",
description: "Return the last 2 bytes of a binary value",
result: Some(Value::Binary {
val: vec![0x23, 0x45],
span: Span::test_data(),
}),
},
]
}
@ -73,45 +82,116 @@ impl Command for Last {
call: &Call,
input: PipelineData,
) -> Result<PipelineData, ShellError> {
let metadata = input.metadata();
let span = call.head;
let head = call.head;
let rows: Option<i64> = call.opt(engine_state, stack, 0)?;
let to_keep = match rows.unwrap_or(1) {
0 => {
// early exit for `last 0`
return Ok(Vec::<Value>::new()
.into_pipeline_data(engine_state.ctrlc.clone())
.set_metadata(metadata));
}
i if i < 0 => {
return Err(ShellError::NeedsPositiveValue(span));
}
i => i as usize,
// FIXME: Please read the FIXME message in `first.rs`'s `first_helper` implementation.
// It has the same issue.
let return_single_element = rows.is_none();
let rows_desired: usize = match rows {
Some(i) if i < 0 => return Err(ShellError::NeedsPositiveValue(head)),
Some(x) => x as usize,
None => 1,
};
// only keep last `to_keep` rows in memory
let mut buf = VecDeque::<_>::new();
for row in input.into_iter_strict(call.head)? {
if buf.len() == to_keep {
buf.pop_front();
}
let ctrlc = engine_state.ctrlc.clone();
let metadata = input.metadata();
buf.push_back(row);
// early exit for `last 0`
if rows_desired == 0 {
return Ok(Vec::<Value>::new()
.into_pipeline_data(ctrlc)
.set_metadata(metadata));
}
if rows.is_some() {
Ok(buf
.into_pipeline_data(engine_state.ctrlc.clone())
.set_metadata(metadata))
} else {
let last = buf.pop_back();
match input {
PipelineData::ListStream(_, _) | PipelineData::Value(Value::Range { .. }, _) => {
let iterator = input.into_iter_strict(head)?;
if let Some(last) = last {
Ok(last.into_pipeline_data().set_metadata(metadata))
} else {
Ok(PipelineData::empty().set_metadata(metadata))
// only keep last `rows_desired` rows in memory
let mut buf = VecDeque::<_>::new();
for row in iterator {
if buf.len() == rows_desired {
buf.pop_front();
}
buf.push_back(row);
}
if return_single_element {
if let Some(last) = buf.pop_back() {
Ok(last.into_pipeline_data().set_metadata(metadata))
} else {
Ok(PipelineData::empty().set_metadata(metadata))
}
} else {
Ok(buf.into_pipeline_data(ctrlc).set_metadata(metadata))
}
}
PipelineData::Value(val, _) => match val {
Value::List { vals, .. } => {
if return_single_element {
if let Some(v) = vals.last() {
Ok(v.clone().into_pipeline_data())
} else {
Err(ShellError::AccessEmptyContent { span: head })
}
} else {
Ok(vals
.into_iter()
.rev()
.take(rows_desired)
.rev()
.into_pipeline_data(ctrlc)
.set_metadata(metadata))
}
}
Value::Binary { val, span } => {
if return_single_element {
if let Some(b) = val.last() {
Ok(PipelineData::Value(
Value::Int {
val: *b as i64,
span,
},
metadata,
))
} else {
Err(ShellError::AccessEmptyContent { span: head })
}
} else {
let slice: Vec<u8> =
val.into_iter().rev().take(rows_desired).rev().collect();
Ok(PipelineData::Value(
Value::Binary { val: slice, span },
metadata,
))
}
}
// Propagate errors by explicitly matching them before the final case.
Value::Error { error } => Err(*error),
other => Err(ShellError::OnlySupportsThisInputType {
exp_input_type: "list, binary or range".into(),
wrong_type: other.get_type().to_string(),
dst_span: head,
src_span: other.expect_span(),
}),
},
PipelineData::ExternalStream { span, .. } => {
Err(ShellError::OnlySupportsThisInputType {
exp_input_type: "list, binary or range".into(),
wrong_type: "raw data".into(),
dst_span: head,
src_span: span,
})
}
PipelineData::Empty => Err(ShellError::OnlySupportsThisInputType {
exp_input_type: "list, binary or range".into(),
wrong_type: "null".into(),
dst_span: call.head,
src_span: call.head,
}),
}
}
}

34
crates/nu-command/tests/commands/first.rs
View File

@ -79,6 +79,28 @@ fn gets_first_row_as_list_when_amount_given() {
assert_eq!(actual.out, "list<int> (stream)");
}
#[test]
fn gets_first_bytes() {
let actual = nu!(pipeline(
r#"
(0x[aa bb cc] | first 2) == 0x[aa bb]
"#
));
assert_eq!(actual.out, "true");
}
#[test]
fn gets_first_byte() {
let actual = nu!(pipeline(
r#"
0x[aa bb cc] | first
"#
));
assert_eq!(actual.out, "170");
}
#[test]
// covers a situation where `first` used to behave strangely on list<binary> input
fn works_with_binary_list() {
@ -98,3 +120,15 @@ fn errors_on_negative_rows() {
assert!(actual.err.contains("use a positive value"));
}
#[test]
fn errors_on_empty_list_when_no_rows_given() {
let actual = nu!(pipeline(
r#"
[]
| first
"#
));
assert!(actual.err.contains("index too large"));
}

34
crates/nu-command/tests/commands/last.rs
View File

@ -79,6 +79,28 @@ fn gets_last_row_as_list_when_amount_given() {
assert_eq!(actual.out, "list<int> (stream)");
}
#[test]
fn gets_last_bytes() {
let actual = nu!(pipeline(
r#"
(0x[aa bb cc] | last 2) == 0x[bb cc]
"#
));
assert_eq!(actual.out, "true");
}
#[test]
fn gets_last_byte() {
let actual = nu!(pipeline(
r#"
0x[aa bb cc] | last
"#
));
assert_eq!(actual.out, "204");
}
#[test]
fn last_errors_on_negative_index() {
let actual = nu!(pipeline(
@ -97,3 +119,15 @@ fn fail_on_non_iterator() {
assert!(actual.err.contains("only_supports_this_input_type"));
}
#[test]
fn errors_on_empty_list_when_no_rows_given() {
let actual = nu!(pipeline(
r#"
[]
| last
"#
));
assert!(actual.err.contains("index too large"));
}