mirror of
https://github.com/nushell/nushell.git
synced 2024-11-25 09:53:43 +01:00
Refactor first
and last
(#12478)
# Description - Refactors `first` and `last` using `Vec::truncate` and `Vec::drain`. - `std::mem::take` was also used to eliminate a few `Value` clones. - The `NeedsPositiveValue` error now uses the span of the `rows` argument instead of the call head span. - `last` now errors on an empty stream to match `first` which does error. - Made metadata preservation more consistent. # User-Facing Changes Breaking change: `last` now errors on an empty stream to match `first` which does error.
This commit is contained in:
parent
1bded8572c
commit
56cdee1fd8
@ -79,60 +79,60 @@ fn first_helper(
|
||||
input: PipelineData,
|
||||
) -> Result<PipelineData, ShellError> {
|
||||
let head = call.head;
|
||||
let rows: Option<i64> = call.opt(engine_state, stack, 0)?;
|
||||
let rows: Option<Spanned<i64>> = call.opt(engine_state, stack, 0)?;
|
||||
|
||||
// FIXME: for backwards compatibility reasons, if `rows` is not specified we
|
||||
// return a single element and otherwise we return a single list. We should probably
|
||||
// remove `rows` so that `first` always returns a single element; getting a list of
|
||||
// the first N elements is covered by `take`
|
||||
let return_single_element = rows.is_none();
|
||||
let rows_desired: usize = match rows {
|
||||
Some(i) if i < 0 => return Err(ShellError::NeedsPositiveValue { span: head }),
|
||||
Some(x) => x as usize,
|
||||
None => 1,
|
||||
let rows = if let Some(rows) = rows {
|
||||
if rows.item < 0 {
|
||||
return Err(ShellError::NeedsPositiveValue { span: rows.span });
|
||||
} else {
|
||||
rows.item as usize
|
||||
}
|
||||
} else {
|
||||
1
|
||||
};
|
||||
|
||||
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_with_metadata(metadata, ctrlc));
|
||||
if rows == 0 {
|
||||
return Ok(Value::list(Vec::new(), head).into_pipeline_data_with_metadata(metadata));
|
||||
}
|
||||
|
||||
match input {
|
||||
PipelineData::Value(val, _) => {
|
||||
let span = val.span();
|
||||
match val {
|
||||
Value::List { vals, .. } => {
|
||||
Value::List { mut vals, .. } => {
|
||||
if return_single_element {
|
||||
if vals.is_empty() {
|
||||
if let Some(val) = vals.first_mut() {
|
||||
Ok(std::mem::take(val).into_pipeline_data())
|
||||
} else {
|
||||
Err(ShellError::AccessEmptyContent { span: head })
|
||||
} else {
|
||||
Ok(vals[0].clone().into_pipeline_data())
|
||||
}
|
||||
} else {
|
||||
Ok(vals
|
||||
.into_iter()
|
||||
.take(rows_desired)
|
||||
.into_pipeline_data_with_metadata(metadata, ctrlc))
|
||||
vals.truncate(rows);
|
||||
Ok(Value::list(vals, span).into_pipeline_data_with_metadata(metadata))
|
||||
}
|
||||
}
|
||||
Value::Binary { val, .. } => {
|
||||
Value::Binary { mut val, .. } => {
|
||||
if return_single_element {
|
||||
if val.is_empty() {
|
||||
Err(ShellError::AccessEmptyContent { span: head })
|
||||
if let Some(&val) = val.first() {
|
||||
Ok(Value::int(val.into(), span).into_pipeline_data())
|
||||
} else {
|
||||
Ok(PipelineData::Value(
|
||||
Value::int(val[0] as i64, span),
|
||||
metadata,
|
||||
))
|
||||
Err(ShellError::AccessEmptyContent { span: head })
|
||||
}
|
||||
} else {
|
||||
let slice: Vec<u8> = val.into_iter().take(rows_desired).collect();
|
||||
Ok(PipelineData::Value(Value::binary(slice, span), metadata))
|
||||
val.truncate(rows);
|
||||
Ok(Value::binary(val, span).into_pipeline_data_with_metadata(metadata))
|
||||
}
|
||||
}
|
||||
Value::Range { val, .. } => {
|
||||
let ctrlc = engine_state.ctrlc.clone();
|
||||
let mut iter = val.into_range_iter(span, ctrlc.clone());
|
||||
if return_single_element {
|
||||
if let Some(v) = iter.next() {
|
||||
@ -142,7 +142,7 @@ fn first_helper(
|
||||
}
|
||||
} else {
|
||||
Ok(iter
|
||||
.take(rows_desired)
|
||||
.take(rows)
|
||||
.into_pipeline_data_with_metadata(metadata, ctrlc))
|
||||
}
|
||||
}
|
||||
@ -165,8 +165,8 @@ fn first_helper(
|
||||
}
|
||||
} else {
|
||||
Ok(ls
|
||||
.take(rows_desired)
|
||||
.into_pipeline_data_with_metadata(metadata, ctrlc))
|
||||
.take(rows)
|
||||
.into_pipeline_data_with_metadata(metadata, engine_state.ctrlc.clone()))
|
||||
}
|
||||
}
|
||||
PipelineData::ExternalStream { span, .. } => Err(ShellError::OnlySupportsThisInputType {
|
||||
|
@ -70,34 +70,41 @@ impl Command for Last {
|
||||
input: PipelineData,
|
||||
) -> Result<PipelineData, ShellError> {
|
||||
let head = call.head;
|
||||
let rows: Option<i64> = call.opt(engine_state, stack, 0)?;
|
||||
let rows: Option<Spanned<i64>> = call.opt(engine_state, stack, 0)?;
|
||||
|
||||
// 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 { span: head }),
|
||||
Some(x) => x as usize,
|
||||
None => 1,
|
||||
let rows = if let Some(rows) = rows {
|
||||
if rows.item < 0 {
|
||||
return Err(ShellError::NeedsPositiveValue { span: rows.span });
|
||||
} else {
|
||||
rows.item as usize
|
||||
}
|
||||
} else {
|
||||
1
|
||||
};
|
||||
|
||||
let ctrlc = engine_state.ctrlc.clone();
|
||||
let metadata = input.metadata();
|
||||
|
||||
// early exit for `last 0`
|
||||
if rows_desired == 0 {
|
||||
return Ok(Vec::<Value>::new().into_pipeline_data_with_metadata(metadata, ctrlc));
|
||||
if rows == 0 {
|
||||
return Ok(Value::list(Vec::new(), head).into_pipeline_data_with_metadata(metadata));
|
||||
}
|
||||
|
||||
match input {
|
||||
PipelineData::ListStream(_, _) | PipelineData::Value(Value::Range { .. }, _) => {
|
||||
let iterator = input.into_iter_strict(head)?;
|
||||
|
||||
// only keep last `rows_desired` rows in memory
|
||||
let mut buf = VecDeque::<_>::new();
|
||||
// only keep the last `rows` in memory
|
||||
let mut buf = VecDeque::new();
|
||||
|
||||
for row in iterator {
|
||||
if buf.len() == rows_desired {
|
||||
if nu_utils::ctrl_c::was_pressed(&engine_state.ctrlc) {
|
||||
return Err(ShellError::InterruptedByUser { span: Some(head) });
|
||||
}
|
||||
|
||||
if buf.len() == rows {
|
||||
buf.pop_front();
|
||||
}
|
||||
|
||||
@ -106,51 +113,41 @@ impl Command for Last {
|
||||
|
||||
if return_single_element {
|
||||
if let Some(last) = buf.pop_back() {
|
||||
Ok(last.into_pipeline_data_with_metadata(metadata))
|
||||
Ok(last.into_pipeline_data())
|
||||
} else {
|
||||
Ok(PipelineData::empty().set_metadata(metadata))
|
||||
Err(ShellError::AccessEmptyContent { span: head })
|
||||
}
|
||||
} else {
|
||||
Ok(buf.into_pipeline_data_with_metadata(metadata, ctrlc))
|
||||
Ok(Value::list(buf.into(), head).into_pipeline_data_with_metadata(metadata))
|
||||
}
|
||||
}
|
||||
PipelineData::Value(val, _) => {
|
||||
let val_span = val.span();
|
||||
|
||||
let span = val.span();
|
||||
match val {
|
||||
Value::List { vals, .. } => {
|
||||
Value::List { mut vals, .. } => {
|
||||
if return_single_element {
|
||||
if let Some(v) = vals.last() {
|
||||
Ok(v.clone().into_pipeline_data())
|
||||
if let Some(v) = vals.pop() {
|
||||
Ok(v.into_pipeline_data())
|
||||
} else {
|
||||
Err(ShellError::AccessEmptyContent { span: head })
|
||||
}
|
||||
} else {
|
||||
Ok(vals
|
||||
.into_iter()
|
||||
.rev()
|
||||
.take(rows_desired)
|
||||
.rev()
|
||||
.into_pipeline_data_with_metadata(metadata, ctrlc))
|
||||
let i = vals.len().saturating_sub(rows);
|
||||
vals.drain(..i);
|
||||
Ok(Value::list(vals, span).into_pipeline_data_with_metadata(metadata))
|
||||
}
|
||||
}
|
||||
Value::Binary { val, .. } => {
|
||||
Value::Binary { mut val, .. } => {
|
||||
if return_single_element {
|
||||
if let Some(b) = val.last() {
|
||||
Ok(PipelineData::Value(
|
||||
Value::int(*b as i64, val_span),
|
||||
metadata,
|
||||
))
|
||||
if let Some(val) = val.pop() {
|
||||
Ok(Value::int(val.into(), span).into_pipeline_data())
|
||||
} 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(slice, val_span),
|
||||
metadata,
|
||||
))
|
||||
let i = val.len().saturating_sub(rows);
|
||||
val.drain(..i);
|
||||
Ok(Value::binary(val, span).into_pipeline_data())
|
||||
}
|
||||
}
|
||||
// Propagate errors by explicitly matching them before the final case.
|
||||
|
@ -28,7 +28,7 @@ fn adds_row_data_if_column_missing() {
|
||||
|
||||
#[test]
|
||||
fn default_after_empty_filter() {
|
||||
let actual = nu!("[a b] | where $it == 'c' | last | default 'd'");
|
||||
let actual = nu!("[a b] | where $it == 'c' | get -i 0 | default 'd'");
|
||||
|
||||
assert_eq!(actual.out, "d");
|
||||
}
|
||||
|
@ -51,7 +51,7 @@ fn gets_first_row_when_no_amount_given() {
|
||||
fn gets_first_row_as_list_when_amount_given() {
|
||||
let actual = nu!("[1, 2, 3] | first 1 | describe");
|
||||
|
||||
assert_eq!(actual.out, "list<int> (stream)");
|
||||
assert_eq!(actual.out, "list<int>");
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
@ -51,7 +51,7 @@ fn requests_more_rows_than_table_has() {
|
||||
fn gets_last_row_as_list_when_amount_given() {
|
||||
let actual = nu!("[1, 2, 3] | last 1 | describe");
|
||||
|
||||
assert_eq!(actual.out, "list<int> (stream)");
|
||||
assert_eq!(actual.out, "list<int>");
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
Loading…
Reference in New Issue
Block a user