Moved to using IntRange over SubBytes

crates/nu-command/src/bytes/at.rs

@@ -1,15 +1,14 @@
-use nu_cmd_base::{
+use std::ops::Bound;
-    input_handler::{operate, CmdArgument},
+
-    util,
+use nu_cmd_base::input_handler::{operate, CmdArgument};
-};
 use nu_engine::command_prelude::*;
-use nu_protocol::Range;
+use nu_protocol::{IntRange, Range};
 
 #[derive(Clone)]
 pub struct BytesAt;
 
 struct Arguments {
-    indexes: Subbytes,
+    range: IntRange,
     cell_paths: Option<Vec<CellPath>>,
 }
 
@@ -19,15 +18,6 @@ impl CmdArgument for Arguments {
     }
 }
 
-impl From<(isize, isize)> for Subbytes {
-    fn from(input: (isize, isize)) -> Self {
-        Self(input.0, input.1)
-    }
-}
-
-#[derive(Clone, Copy)]
-struct Subbytes(isize, isize);
-
 impl Command for BytesAt {
     fn name(&self) -> &str {
         "bytes at"
@@ -69,32 +59,32 @@ impl Command for BytesAt {
         call: &Call,
         input: PipelineData,
     ) -> Result<PipelineData, ShellError> {
-        let range: Range = call.req(engine_state, stack, 0)?;
+        let range = match call.req(engine_state, stack, 0)? {
-        let indexes: Subbytes = match util::process_range(&range) {
+            Range::IntRange(range) => range,
-            Ok(idxs) => idxs.into(),
+            _ => {
-            Err(processing_error) => {
+                return Err(ShellError::UnsupportedInput {
-                return Err(processing_error("could not perform subbytes", call.head));
+                    msg: "Float ranges are not supported for byte streams".into(),
+                    input: "value originates from here".into(),
+                    msg_span: call.head,
+                    input_span: call.head,
+                })
             }
         };
 
         let cell_paths: Vec<CellPath> = call.rest(engine_state, stack, 1)?;
         let cell_paths = (!cell_paths.is_empty()).then_some(cell_paths);
-        let args = Arguments {
-            indexes,
-            cell_paths,
-        };
 
         if let PipelineData::ByteStream(stream, metadata) = input {
-            let stream = stream.slice(
+            let stream = stream.slice(call.head, call.arguments_span(), range)?;
-                call.head,
-                call.arguments_span(),
-                args.indexes.0,
-                args.indexes.1,
-            )?;
-
             Ok(PipelineData::ByteStream(stream, metadata))
         } else {
-            operate(map_value, args, input, call.head, engine_state.signals())
+            operate(
+                map_value,
+                Arguments { range, cell_paths },
+                input,
+                call.head,
+                engine_state.signals(),
+            )
         }
     }
 
@@ -109,7 +99,7 @@ impl Command for BytesAt {
             },
             Example {
                 description: "Slice out `0x[10 01 13]` from `0x[33 44 55 10 01 13]`",
-                example: "0x[33 44 55 10 01 13] | bytes at 3..6",
+                example: "0x[33 44 55 10 01 13] | bytes at 3..5",
                 result: Some(Value::test_binary(vec![0x10, 0x01, 0x13])),
             },
             Example {
@@ -146,18 +136,15 @@ impl Command for BytesAt {
 }
 
 fn map_value(input: &Value, args: &Arguments, head: Span) -> Value {
-    let range = &args.indexes;
+    let range = &args.range;
     match input {
         Value::Binary { val, .. } => {
-            let (start, end) = resolve_relative_range(range, &val.len());
+            let len = val.len() as u64;
-            let iter = val.iter().copied();
+            let start: usize = range.absolute_start(len).try_into().unwrap();
-
+            let bytes: Vec<u8> = match range.absolute_end(len) {
-            let bytes: Vec<u8> = if start > end {
+                Bound::Unbounded => val[start..].into(),
-                vec![]
+                Bound::Included(end) => val[start..=end as usize].into(),
-            } else if end == usize::MAX {
+                Bound::Excluded(end) => val[start..end as usize].into(),
-                iter.skip(start).collect()
-            } else {
-                iter.skip(start).take(end - start + 1).collect()
             };
 
             Value::binary(bytes, head)
@@ -175,20 +162,6 @@ fn map_value(input: &Value, args: &Arguments, head: Span) -> Value {
     }
 }
 
-fn resolve_relative_range(range: &Subbytes, len: usize) -> (usize, usize) {
-    let start = match range.0 {
-        start if start < 0 => len.checked_sub(start.unsigned_abs()).unwrap_or(0),
-        start => start as usize,
-    };
-
-    let end = match range.1 {
-        end if end < 0 => len.checked_sub(end.unsigned_abs()).unwrap_or(0),
-        end => end as usize,
-    };
-
-    (start, end)
-}
-
 #[cfg(test)]
 mod test {
     use super::*;

crates/nu-protocol/src/pipeline/byte_stream.rs

@@ -1,8 +1,9 @@
 //! Module managing the streaming of raw bytes between pipeline elements
 #[cfg(feature = "os")]
 use crate::process::{ChildPipe, ChildProcess};
-use crate::{ErrSpan, IntoSpanned, PipelineData, ShellError, Signals, Span, Type, Value};
+use crate::{ErrSpan, IntRange, IntoSpanned, PipelineData, ShellError, Signals, Span, Type, Value};
 use serde::{Deserialize, Serialize};
+use std::ops::Bound;
 #[cfg(unix)]
 use std::os::fd::OwnedFd;
 #[cfg(windows)]
@@ -256,42 +257,46 @@ impl ByteStream {
 
     pub fn slice(
         self,
-        val_span: Span,
+        range_span: Span,
         call_span: Span,
-        start: isize,
+        range: IntRange,
-        end: isize,
     ) -> Result<Self, ShellError> {
         match self.known_size {
             Some(len) => {
-                let absolute_start = match start {
+                let absolute_start = range.absolute_start(len);
-                    start if start < 0 => (len as isize + start).max(0) as usize,
+                match range.absolute_end(len) {
-                    start => start.min(len as isize) as usize,
+                    Bound::Unbounded => self.skip(range_span, absolute_start),
-                };
+                    Bound::Included(end) => self.skip(range_span, absolute_start)
-
+                        .and_then(|stream| stream.take(range_span, end.saturating_sub(absolute_start) + 1)),
-                self.skip(val_span, absolute_start as u64)
+                    Bound::Excluded(end) => self.skip(range_span, absolute_start)
-                    .and_then(|stream| {
+                        .and_then(|stream| stream.take(range_span, end.saturating_sub(absolute_start))),
-                        let absolute_end = match end {
+                }
-                            end if end < 0 => (len as isize + end).max(0) as usize,
+            }
-                            end => end.min(len as isize) as usize,
+            None => match range.is_relative() {
-                        };
+                true => Err(ShellError::IncorrectValue {
-
+                    msg:
-                        if absolute_end < absolute_start {
+                        "Relative range values cannot be used with streams that don't specify a length"
-                            stream.take(val_span, 0)
+                            .into(),
-                        } else {
+                    val_span: range_span,
-                            stream.take(val_span, (absolute_end - absolute_start) as u64)
+                    call_span,
-                        }
+                }),
-                    })
+                false => {
+                    let skip = range.start();
+                    match range.end() {
+                        std::ops::Bound::Unbounded => self.skip(range_span, skip as u64),
+                        std::ops::Bound::Included(end) => match end - skip {
+                            take if take < 0 => self.take(range_span, 0),
+                            take => self.skip(range_span, skip as u64)
+                                .and_then(|stream| stream.take(range_span, take as u64 + 1)),
+                        },
+                        std::ops::Bound::Excluded(end) => match end - skip {
+                            take if take < 0 => self.take(range_span, 0),
+                            take => self.skip(range_span, skip as u64)
+                                .and_then(|stream| stream.take(range_span, take as u64)),
+                        },
+                    }
+                }
             }
-            None if start < 0 || end < 0 => Err(ShellError::IncorrectValue {
-                msg:
-                    "Relative range values cannot be used with streams that don't specify a length"
-                        .into(),
-                val_span,
-                call_span,
-            }),
-            None => self
-                .skip(val_span, start as u64)
-                .and_then(|stream| stream.take(val_span, end as u64)),
         }
     }
 

crates/nu-protocol/src/value/range.rs

@@ -81,10 +81,31 @@ mod int_range {
             self.start
         }
 
+        pub fn absolute_start(&self, len: u64) -> u64 {
+            match self.start {
+                start if start < 0 => len.saturating_sub(start.unsigned_abs().into()),
+                start => start as u64,
+            }
+        }
+
         pub fn end(&self) -> Bound<i64> {
             self.end
         }
 
+        pub fn absolute_end(&self, len: u64) -> Bound<u64> {
+            match self.end {
+                Bound::Unbounded => Bound::Unbounded,
+                Bound::Included(i) => Bound::Included(match i {
+                    i if i < 0 => len.saturating_sub(i.unsigned_abs() as u64),
+                    i => i as u64,
+                }),
+                Bound::Excluded(i) => Bound::Excluded(match i {
+                    i if i < 0 => len.saturating_sub(i.unsigned_abs() as u64),
+                    i => i as u64,
+                }),
+            }
+        }
+
         pub fn step(&self) -> i64 {
             self.step
         }
@@ -93,6 +114,21 @@ mod int_range {
             self.end == Bound::Unbounded
         }
 
+        pub fn is_relative(&self) -> bool {
+            self.is_start_relative() || self.is_end_relative()
+        }
+
+        pub fn is_start_relative(&self) -> bool {
+            self.start < 0
+        }
+
+        pub fn is_end_relative(&self) -> bool {
+            match self.end {
+                Bound::Included(end) | Bound::Excluded(end) => end < 0,
+                _ => false,
+            }
+        }
+
         pub fn contains(&self, value: i64) -> bool {
             if self.step < 0 {
                 // Decreasing range

crates/nu-protocol/tests/pipeline/byte_stream.rs

@@ -1,78 +1,252 @@
-use nu_protocol::{ByteStream, Signals, Span};
+use nu_protocol::{ast::RangeInclusion, ByteStream, IntRange, Signals, Span, Value};
 
 #[test]
-pub fn test_simple_positive_slice() {
+pub fn test_simple_positive_slice_exclusive() {
     let data = b"Hello World".to_vec();
     let stream = ByteStream::read_binary(data, Span::test_data(), Signals::empty());
     let sliced = stream
-        .slice(Span::test_data(), Span::test_data(), 0, 5)
+        .slice(
+            Span::test_data(),
+            Span::test_data(),
+            create_range(0, 5, RangeInclusion::RightExclusive),
+        )
         .unwrap();
     let result = sliced.into_bytes().unwrap();
     assert_eq!(result, b"Hello");
 }
 
 #[test]
-pub fn test_negative_start() {
+pub fn test_negative_start_exclusive() {
     let data = b"Hello World".to_vec();
     let stream = ByteStream::read_binary(data, Span::test_data(), Signals::empty());
     let sliced = stream
-        .slice(Span::test_data(), Span::test_data(), -5, 11)
+        .slice(
+            Span::test_data(),
+            Span::test_data(),
+            create_range(-5, 11, RangeInclusion::RightExclusive),
+        )
         .unwrap();
     let result = sliced.into_bytes().unwrap();
     assert_eq!(result, b"World");
 }
 
 #[test]
-pub fn test_negative_end() {
+pub fn test_negative_end_exclusive() {
     let data = b"Hello World".to_vec();
     let stream = ByteStream::read_binary(data, Span::test_data(), Signals::empty());
     let sliced = stream
-        .slice(Span::test_data(), Span::test_data(), 0, -6)
+        .slice(
+            Span::test_data(),
+            Span::test_data(),
+            create_range(0, -6, RangeInclusion::RightExclusive),
+        )
         .unwrap();
     let result = sliced.into_bytes().unwrap();
     assert_eq!(result, b"Hello");
 }
 
 #[test]
-pub fn test_empty_slice() {
+pub fn test_negative_start_and_end_exclusive() {
     let data = b"Hello World".to_vec();
     let stream = ByteStream::read_binary(data, Span::test_data(), Signals::empty());
     let sliced = stream
-        .slice(Span::test_data(), Span::test_data(), 5, 5)
+        .slice(
+            Span::test_data(),
+            Span::test_data(),
+            create_range(-5, -2, RangeInclusion::RightExclusive),
+        )
+        .unwrap();
+    let result = sliced.into_bytes().unwrap();
+    assert_eq!(result, b"Wor");
+}
+
+#[test]
+pub fn test_empty_slice_exclusive() {
+    let data = b"Hello World".to_vec();
+    let stream = ByteStream::read_binary(data, Span::test_data(), Signals::empty());
+    let sliced = stream
+        .slice(
+            Span::test_data(),
+            Span::test_data(),
+            create_range(5, 5, RangeInclusion::RightExclusive),
+        )
         .unwrap();
     let result = sliced.into_bytes().unwrap();
     assert_eq!(result, Vec::<u8>::new());
 }
 
 #[test]
-pub fn test_out_of_bounds() {
+pub fn test_out_of_bounds_exclusive() {
     let data = b"Hello World".to_vec();
     let stream = ByteStream::read_binary(data, Span::test_data(), Signals::empty());
     let sliced = stream
-        .slice(Span::test_data(), Span::test_data(), 0, 20)
+        .slice(
+            Span::test_data(),
+            Span::test_data(),
+            create_range(0, 20, RangeInclusion::RightExclusive),
+        )
         .unwrap();
     let result = sliced.into_bytes().unwrap();
     assert_eq!(result, b"Hello World");
 }
 
 #[test]
-pub fn test_invalid_range() {
+pub fn test_invalid_range_exclusive() {
     let data = b"Hello World".to_vec();
     let stream = ByteStream::read_binary(data, Span::test_data(), Signals::empty());
     let sliced = stream
-        .slice(Span::test_data(), Span::test_data(), 11, 5)
+        .slice(
+            Span::test_data(),
+            Span::test_data(),
+            create_range(11, 5, RangeInclusion::RightExclusive),
+        )
         .unwrap();
     let result = sliced.into_bytes().unwrap();
     assert_eq!(result, Vec::<u8>::new());
 }
 
 #[test]
-pub fn test_max_end() {
+pub fn test_max_end_exclusive() {
     let data = b"Hello World".to_vec();
     let stream = ByteStream::read_binary(data, Span::test_data(), Signals::empty());
     let sliced = stream
-        .slice(Span::test_data(), Span::test_data(), 6, isize::MAX)
+        .slice(
+            Span::test_data(),
+            Span::test_data(),
+            create_range(6, i64::MAX, RangeInclusion::RightExclusive),
+        )
         .unwrap();
     let result = sliced.into_bytes().unwrap();
     assert_eq!(result, b"World");
 }
+
+#[test]
+pub fn test_simple_positive_slice_inclusive() {
+    let data = b"Hello World".to_vec();
+    let stream = ByteStream::read_binary(data, Span::test_data(), Signals::empty());
+    let sliced = stream
+        .slice(
+            Span::test_data(),
+            Span::test_data(),
+            create_range(0, 5, RangeInclusion::RightExclusive),
+        )
+        .unwrap();
+    let result = sliced.into_bytes().unwrap();
+    assert_eq!(result, b"Hello");
+}
+
+#[test]
+pub fn test_negative_start_inclusive() {
+    let data = b"Hello World".to_vec();
+    let stream = ByteStream::read_binary(data, Span::test_data(), Signals::empty());
+    let sliced = stream
+        .slice(
+            Span::test_data(),
+            Span::test_data(),
+            create_range(-5, 11, RangeInclusion::Inclusive),
+        )
+        .unwrap();
+    let result = sliced.into_bytes().unwrap();
+    assert_eq!(result, b"World");
+}
+
+#[test]
+pub fn test_negative_end_inclusive() {
+    let data = b"Hello World".to_vec();
+    let stream = ByteStream::read_binary(data, Span::test_data(), Signals::empty());
+    let sliced = stream
+        .slice(
+            Span::test_data(),
+            Span::test_data(),
+            create_range(0, -7, RangeInclusion::Inclusive),
+        )
+        .unwrap();
+    let result = sliced.into_bytes().unwrap();
+    assert_eq!(result, b"Hello");
+}
+
+#[test]
+pub fn test_negative_start_and_end_inclusive() {
+    let data = b"Hello World".to_vec();
+    let stream = ByteStream::read_binary(data, Span::test_data(), Signals::empty());
+    let sliced = stream
+        .slice(
+            Span::test_data(),
+            Span::test_data(),
+            create_range(-5, -1, RangeInclusion::Inclusive),
+        )
+        .unwrap();
+    let result = sliced.into_bytes().unwrap();
+    assert_eq!(result, b"World");
+}
+
+#[test]
+pub fn test_empty_slice_inclusive() {
+    let data = b"Hello World".to_vec();
+    let stream = ByteStream::read_binary(data, Span::test_data(), Signals::empty());
+    let sliced = stream
+        .slice(
+            Span::test_data(),
+            Span::test_data(),
+            create_range(5, 5, RangeInclusion::Inclusive),
+        )
+        .unwrap();
+    let result = sliced.into_bytes().unwrap();
+    assert_eq!(result, b" ");
+}
+
+#[test]
+pub fn test_out_of_bounds_inclusive() {
+    let data = b"Hello World".to_vec();
+    let stream = ByteStream::read_binary(data, Span::test_data(), Signals::empty());
+    let sliced = stream
+        .slice(
+            Span::test_data(),
+            Span::test_data(),
+            create_range(0, 20, RangeInclusion::Inclusive),
+        )
+        .unwrap();
+    let result = sliced.into_bytes().unwrap();
+    assert_eq!(result, b"Hello World");
+}
+
+#[test]
+pub fn test_invalid_range_inclusive() {
+    let data = b"Hello World".to_vec();
+    let stream = ByteStream::read_binary(data, Span::test_data(), Signals::empty());
+    let sliced = stream
+        .slice(
+            Span::test_data(),
+            Span::test_data(),
+            create_range(11, 5, RangeInclusion::Inclusive),
+        )
+        .unwrap();
+    let result = sliced.into_bytes().unwrap();
+    assert_eq!(result, Vec::<u8>::new());
+}
+
+#[test]
+pub fn test_max_end_inclusive() {
+    let data = b"Hello World".to_vec();
+    let stream = ByteStream::read_binary(data, Span::test_data(), Signals::empty());
+    let sliced = stream
+        .slice(
+            Span::test_data(),
+            Span::test_data(),
+            create_range(6, i64::MAX, RangeInclusion::Inclusive),
+        )
+        .unwrap();
+    let result = sliced.into_bytes().unwrap();
+    assert_eq!(result, b"World");
+}
+
+fn create_range(start: i64, end: i64, inclusion: RangeInclusion) -> IntRange {
+    IntRange::new(
+        Value::int(start, Span::unknown()),
+        Value::nothing(Span::test_data()),
+        Value::int(end, Span::unknown()),
+        inclusion,
+        Span::unknown(),
+    )
+    .unwrap()
+}