Skip to content

Commit

Permalink
Rename/deprecate range to slice (nushell#14825)
Browse files Browse the repository at this point in the history
# Description
As the `range` command has an ambiguous name (does it construct a range
type?, does it iterate a range like `seq`) replace it with a more
descriptive verb of what it does: `slice`

Closes nushell#14130
# User-Facing Changes
`range` is now deprecated and replaced in whole by `slice` with the same
behavior.
`range` will be removed in `0.103.0`

# Tests + Formatting
Tests have been updated to use `slice`

# After submitting

- [ ] prepare PR for `nu_scripts` (several usages of `range` to be
fixed)
- [ ] update documentation usages of `range` after release
  • Loading branch information
sholderbach authored Jan 17, 2025
1 parent 089c522 commit 4dcaf2a
Show file tree
Hide file tree
Showing 7 changed files with 167 additions and 74 deletions.
1 change: 1 addition & 0 deletions crates/nu-command/src/default_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ pub fn add_shell_command_context(mut engine_state: EngineState) -> EngineState {
Skip,
SkipUntil,
SkipWhile,
Slice,
Sort,
SortBy,
SplitList,
Expand Down
2 changes: 2 additions & 0 deletions crates/nu-command/src/filters/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ mod select;
#[cfg(feature = "rand")]
mod shuffle;
mod skip;
mod slice;
mod sort;
mod sort_by;
mod take;
Expand Down Expand Up @@ -99,6 +100,7 @@ pub use select::Select;
#[cfg(feature = "rand")]
pub use shuffle::Shuffle;
pub use skip::*;
pub use slice::Slice;
pub use sort::Sort;
pub use sort_by::SortBy;
pub use take::*;
Expand Down
78 changes: 12 additions & 66 deletions crates/nu-command/src/filters/range.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use nu_engine::command_prelude::*;
use nu_protocol::Range as NumRange;
use std::ops::Bound;
use nu_protocol::{report_parse_warning, ParseWarning};

#[derive(Clone)]
pub struct Range;
Expand All @@ -17,7 +16,7 @@ impl Command for Range {
Type::List(Box::new(Type::Any)),
)])
.required("rows", SyntaxShape::Range, "Range of rows to return.")
.category(Category::Filters)
.category(Category::Deprecated)
}

fn description(&self) -> &str {
Expand Down Expand Up @@ -65,69 +64,16 @@ impl Command for Range {
input: PipelineData,
) -> Result<PipelineData, ShellError> {
let head = call.head;
let metadata = input.metadata();
let rows: Spanned<NumRange> = call.req(engine_state, stack, 0)?;

match rows.item {
NumRange::IntRange(range) => {
let start = range.start();
let end = match range.end() {
Bound::Included(end) => end,
Bound::Excluded(end) => end - 1,
Bound::Unbounded => {
if range.step() < 0 {
i64::MIN
} else {
i64::MAX
}
}
};

// only collect the input if we have any negative indices
if start < 0 || end < 0 {
let v: Vec<_> = input.into_iter().collect();
let vlen: i64 = v.len() as i64;

let from = if start < 0 {
(vlen + start) as usize
} else {
start as usize
};

let to = if end < 0 {
(vlen + end) as usize
} else if end > v.len() as i64 {
v.len()
} else {
end as usize
};

if from > to {
Ok(PipelineData::Value(Value::nothing(head), None))
} else {
let iter = v.into_iter().skip(from).take(to - from + 1);
Ok(iter.into_pipeline_data(head, engine_state.signals().clone()))
}
} else {
let from = start as usize;
let to = end as usize;

if from > to {
Ok(PipelineData::Value(Value::nothing(head), None))
} else {
let iter = input.into_iter().skip(from).take(to - from + 1);
Ok(iter.into_pipeline_data(head, engine_state.signals().clone()))
}
}
.map(|x| x.set_metadata(metadata))
}
NumRange::FloatRange(_) => Err(ShellError::UnsupportedInput {
msg: "float range".into(),
input: "value originates from here".into(),
msg_span: call.head,
input_span: rows.span,
}),
}
report_parse_warning(
&StateWorkingSet::new(engine_state),
&ParseWarning::DeprecatedWarning {
old_command: "range".into(),
new_suggestion: "use `slice`".into(),
span: head,
url: "`help slice`".into(),
},
);
super::Slice::run(&super::Slice, engine_state, stack, call, input)
}
}

Expand Down
144 changes: 144 additions & 0 deletions crates/nu-command/src/filters/slice.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
use nu_engine::command_prelude::*;
use nu_protocol::Range;
use std::ops::Bound;

#[derive(Clone)]
pub struct Slice;

impl Command for Slice {
fn name(&self) -> &str {
"slice"
}

fn signature(&self) -> Signature {
Signature::build("slice")
.input_output_types(vec![(
Type::List(Box::new(Type::Any)),
Type::List(Box::new(Type::Any)),
)])
.required("rows", SyntaxShape::Range, "Range of rows to return.")
.category(Category::Filters)
}

fn description(&self) -> &str {
"Return only the selected rows."
}

fn search_terms(&self) -> Vec<&str> {
vec!["filter", "head", "tail", "range"]
}

fn examples(&self) -> Vec<Example> {
vec![
Example {
example: "[0,1,2,3,4,5] | slice 4..5",
description: "Get the last 2 items",
result: Some(Value::list(
vec![Value::test_int(4), Value::test_int(5)],
Span::test_data(),
)),
},
Example {
example: "[0,1,2,3,4,5] | slice (-2)..",
description: "Get the last 2 items",
result: Some(Value::list(
vec![Value::test_int(4), Value::test_int(5)],
Span::test_data(),
)),
},
Example {
example: "[0,1,2,3,4,5] | slice (-3)..-2",
description: "Get the next to last 2 items",
result: Some(Value::list(
vec![Value::test_int(3), Value::test_int(4)],
Span::test_data(),
)),
},
]
}

fn run(
&self,
engine_state: &EngineState,
stack: &mut Stack,
call: &Call,
input: PipelineData,
) -> Result<PipelineData, ShellError> {
let head = call.head;
let metadata = input.metadata();
let rows: Spanned<Range> = call.req(engine_state, stack, 0)?;

match rows.item {
Range::IntRange(range) => {
let start = range.start();
let end = match range.end() {
Bound::Included(end) => end,
Bound::Excluded(end) => end - 1,
Bound::Unbounded => {
if range.step() < 0 {
i64::MIN
} else {
i64::MAX
}
}
};

// only collect the input if we have any negative indices
if start < 0 || end < 0 {
let v: Vec<_> = input.into_iter().collect();
let vlen: i64 = v.len() as i64;

let from = if start < 0 {
(vlen + start) as usize
} else {
start as usize
};

let to = if end < 0 {
(vlen + end) as usize
} else if end > v.len() as i64 {
v.len()
} else {
end as usize
};

if from > to {
Ok(PipelineData::Value(Value::nothing(head), None))
} else {
let iter = v.into_iter().skip(from).take(to - from + 1);
Ok(iter.into_pipeline_data(head, engine_state.signals().clone()))
}
} else {
let from = start as usize;
let to = end as usize;

if from > to {
Ok(PipelineData::Value(Value::nothing(head), None))
} else {
let iter = input.into_iter().skip(from).take(to - from + 1);
Ok(iter.into_pipeline_data(head, engine_state.signals().clone()))
}
}
.map(|x| x.set_metadata(metadata))
}
Range::FloatRange(_) => Err(ShellError::UnsupportedInput {
msg: "float range".into(),
input: "value originates from here".into(),
msg_span: call.head,
input_span: rows.span,
}),
}
}
}

#[cfg(test)]
mod test {
use super::*;

#[test]
fn test_examples() {
use crate::test_examples;

test_examples(Slice {})
}
}
2 changes: 1 addition & 1 deletion crates/nu-command/tests/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ mod print;
#[cfg(feature = "sqlite")]
mod query;
mod random;
mod range;
mod redirection;
mod reduce;
mod reject;
Expand All @@ -100,6 +99,7 @@ mod seq;
mod seq_char;
mod seq_date;
mod skip;
mod slice;
mod sort;
mod sort_by;
mod source_env;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,15 @@ use nu_test_support::{nu, pipeline};

#[test]
fn selects_a_row() {
Playground::setup("range_test_1", |dirs, sandbox| {
Playground::setup("slice_test_1", |dirs, sandbox| {
sandbox.with_files(&[EmptyFile("notes.txt"), EmptyFile("tests.txt")]);

let actual = nu!(
cwd: dirs.test(), pipeline(
"
ls
| sort-by name
| range 0..0
| slice 0..0
| get name.0
"
));
Expand All @@ -23,7 +23,7 @@ fn selects_a_row() {

#[test]
fn selects_some_rows() {
Playground::setup("range_test_2", |dirs, sandbox| {
Playground::setup("slice_test_2", |dirs, sandbox| {
sandbox.with_files(&[
EmptyFile("notes.txt"),
EmptyFile("tests.txt"),
Expand All @@ -35,7 +35,7 @@ fn selects_some_rows() {
"
ls
| get name
| range 1..2
| slice 1..2
| length
"
));
Expand All @@ -46,7 +46,7 @@ fn selects_some_rows() {

#[test]
fn negative_indices() {
Playground::setup("range_test_negative_indices", |dirs, sandbox| {
Playground::setup("slice_test_negative_indices", |dirs, sandbox| {
sandbox.with_files(&[
EmptyFile("notes.txt"),
EmptyFile("tests.txt"),
Expand All @@ -58,7 +58,7 @@ fn negative_indices() {
"
ls
| get name
| range (-1..)
| slice (-1..)
| length
"
));
Expand Down
2 changes: 1 addition & 1 deletion tests/repl/test_engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ fn better_operator_spans() -> TestResult {

#[test]
fn range_right_exclusive() -> TestResult {
run_test(r#"[1, 4, 5, 8, 9] | range 1..<3 | math sum"#, "9")
run_test(r#"[1, 4, 5, 8, 9] | slice 1..<3 | math sum"#, "9")
}

/// Issue #7872
Expand Down

0 comments on commit 4dcaf2a

Please sign in to comment.