Into binary changes (#3758)

* kind of works but not what we really want

* updated `into binary` and `first` to work better together

* attempt to fix wasm build problem

* attempt #2 to fix wasm stuff

crates/nu-command/src/commands/conversions/into/binary.rs

@@ -12,23 +12,10 @@ impl WholeStreamCommand for SubCommand {
     }
 
     fn signature(&self) -> Signature {
-        Signature::build("into binary")
+        Signature::build("into binary").rest(
-            .rest(
+            SyntaxShape::ColumnPath,
-                SyntaxShape::ColumnPath,
+            "column paths to convert to binary (for table input)",
-                "column paths to convert to binary (for table input)",
+        )
-            )
-            .named(
-                "skip",
-                SyntaxShape::Int,
-                "skip x number of bytes",
-                Some('s'),
-            )
-            .named(
-                "bytes",
-                SyntaxShape::Int,
-                "show y number of bytes",
-                Some('b'),
-            )
     }
 
     fn usage(&self) -> &str {
@@ -53,43 +40,21 @@ impl WholeStreamCommand for SubCommand {
                 )
                 .into()]),
             },
-            Example {
-                description: "convert string to a nushell binary primitive",
-                example:
-                    "echo 'This is a string that is exactly 52 characters long.' | into binary --skip 10",
-                result: Some(vec![UntaggedValue::binary(
-                    "string that is exactly 52 characters long."
-                        .to_string()
-                        .as_bytes()
-                        .to_vec(),
-                )
-                .into()]),
-            },
-            Example {
-                description: "convert string to a nushell binary primitive",
-                example:
-                    "echo 'This is a string that is exactly 52 characters long.' | into binary --skip 10 --bytes 10",
-                result: Some(vec![UntaggedValue::binary(
-                    "string tha"
-                        .to_string()
-                        .as_bytes()
-                        .to_vec(),
-                )
-                .into()]),
-            },
             Example {
                 description: "convert a number to a nushell binary primitive",
                 example: "echo 1 | into binary",
-                result: Some(vec![
+                result: Some(vec![UntaggedValue::binary(
-                    UntaggedValue::binary(i64::from(1).to_le_bytes().to_vec()).into()
+                    i64::from(1).to_le_bytes().to_vec(),
-                ]),
+                )
+                .into()]),
             },
             Example {
                 description: "convert a boolean to a nushell binary primitive",
                 example: "echo $true | into binary",
-                result: Some(vec![
+                result: Some(vec![UntaggedValue::binary(
-                    UntaggedValue::binary(i64::from(1).to_le_bytes().to_vec()).into()
+                    i64::from(1).to_le_bytes().to_vec(),
-                ]),
+                )
+                .into()]),
             },
             Example {
                 description: "convert a filesize to a nushell binary primitive",
@@ -113,23 +78,19 @@ impl WholeStreamCommand for SubCommand {
 }
 
 fn into_binary(args: CommandArgs) -> Result<OutputStream, ShellError> {
-    let skip: Option<Value> = args.get_flag("skip")?;
-    let bytes: Option<Value> = args.get_flag("bytes")?;
     let column_paths: Vec<ColumnPath> = args.rest(0)?;
 
     Ok(args
         .input
         .map(move |v| {
             if column_paths.is_empty() {
-                action(&v, v.tag(), &skip, &bytes)
+                action(&v, v.tag())
             } else {
                 let mut ret = v;
                 for path in &column_paths {
-                    let skip_clone = skip.clone();
-                    let bytes_clone = bytes.clone();
                     ret = ret.swap_data_by_column_path(
                         path,
-                        Box::new(move |old| action(old, old.tag(), &skip_clone, &bytes_clone)),
+                        Box::new(move |old| action(old, old.tag())),
                     )?;
                 }
 
@@ -141,54 +102,26 @@ fn into_binary(args: CommandArgs) -> Result<OutputStream, ShellError> {
 
 fn int_to_endian(n: i64) -> Vec<u8> {
     if cfg!(target_endian = "little") {
-        // eprintln!("Little Endian");
         n.to_le_bytes().to_vec()
     } else {
-        // eprintln!("Big Endian");
         n.to_be_bytes().to_vec()
     }
 }
 
 fn bigint_to_endian(n: &BigInt) -> Vec<u8> {
     if cfg!(target_endian = "little") {
-        // eprintln!("Little Endian");
         n.to_bytes_le().1
     } else {
-        // eprintln!("Big Endian");
         n.to_bytes_be().1
     }
 }
 
-pub fn action(
+pub fn action(input: &Value, tag: impl Into<Tag>) -> Result<Value, ShellError> {
-    input: &Value,
-    tag: impl Into<Tag>,
-    skip: &Option<Value>,
-    bytes: &Option<Value>,
-) -> Result<Value, ShellError> {
     let tag = tag.into();
-    let skip_bytes = match skip {
-        Some(s) => s.as_usize().unwrap_or(0),
-        None => 0usize,
-    };
-
-    let num_bytes = match bytes {
-        Some(b) => b.as_usize().unwrap_or(0),
-        None => 0usize,
-    };
 
     match &input.value {
         UntaggedValue::Primitive(prim) => Ok(UntaggedValue::binary(match prim {
-            Primitive::Binary(b) => {
+            Primitive::Binary(b) => b.to_vec(),
-                if num_bytes == 0usize {
-                    b.to_vec().into_iter().skip(skip_bytes).collect()
-                } else {
-                    b.to_vec()
-                        .into_iter()
-                        .skip(skip_bytes)
-                        .take(num_bytes)
-                        .collect()
-                }
-            }
             Primitive::Int(n_ref) => int_to_endian(*n_ref),
             Primitive::BigInt(n_ref) => bigint_to_endian(n_ref),
             Primitive::Decimal(dec) => match dec.to_bigint() {
@@ -207,25 +140,7 @@ pub fn action(
                     ));
                 }
             },
-            Primitive::String(a_string) => {
+            Primitive::String(a_string) => a_string.as_bytes().to_vec(),
-                // a_string.as_bytes().to_vec()
-                if num_bytes == 0usize {
-                    a_string
-                        .as_bytes()
-                        .to_vec()
-                        .into_iter()
-                        .skip(skip_bytes)
-                        .collect()
-                } else {
-                    a_string
-                        .as_bytes()
-                        .to_vec()
-                        .into_iter()
-                        .skip(skip_bytes)
-                        .take(num_bytes)
-                        .collect()
-                }
-            }
             Primitive::Boolean(a_bool) => match a_bool {
                 false => int_to_endian(0),
                 true => int_to_endian(1),

crates/nu-command/src/commands/filters/first.rs

@@ -23,7 +23,7 @@ impl WholeStreamCommand for First {
         "Show only the first number of rows."
     }
 
-    fn run_with_actions(&self, args: CommandArgs) -> Result<ActionStream, ShellError> {
+    fn run(&self, args: CommandArgs) -> Result<OutputStream, ShellError> {
         first(args)
     }
 
@@ -46,17 +46,94 @@ impl WholeStreamCommand for First {
     }
 }
 
-fn first(args: CommandArgs) -> Result<ActionStream, ShellError> {
+fn first(args: CommandArgs) -> Result<OutputStream, ShellError> {
     let rows: Option<Tagged<usize>> = args.opt(0)?;
-    let input = args.input;
+    let tag = args.call_info.name_tag;
 
-    let rows_desired = if let Some(quantity) = rows {
+    let mut rows_desired = if let Some(quantity) = rows {
         *quantity
     } else {
         1
     };
 
-    Ok(input.take(rows_desired).into_action_stream())
+    let mut input_peek = args.input.peekable();
+    match &mut input_peek.next_if(|val| val.is_binary()) {
+        Some(v) => match &v.value {
+            // We already know it's a binary so we don't have to match
+            // on the type of primitive
+            UntaggedValue::Primitive(_) => {
+                let bytes = match v.as_binary_vec() {
+                    Ok(b) => b,
+                    _ => {
+                        return Err(ShellError::labeled_error(
+                            "error converting data as_binary_vec",
+                            "error conversion",
+                            tag,
+                        ))
+                    }
+                };
+                // if the current 8192 chunk fits inside our rows_desired
+                // carve it up and return it
+                if bytes.len() >= rows_desired {
+                    // We only want to see a certain amount of the binary
+                    // so let's grab those parts
+                    let output_bytes = bytes[0..rows_desired].to_vec();
+                    Ok(OutputStream::one(UntaggedValue::binary(output_bytes)))
+                } else {
+                    // if we want more rows that the current chunk size (8192)
+                    // we must gradually get bigger chunks while testing
+                    // if it's within the requested rows_desired size
+                    let mut bigger: Vec<u8> = vec![];
+                    bigger.extend(bytes);
+                    while bigger.len() < rows_desired {
+                        match input_peek.next() {
+                            Some(data) => match data.value.into_value(&tag).as_binary_vec() {
+                                Ok(bits) => bigger.extend(bits),
+                                _ => {
+                                    return Err(ShellError::labeled_error(
+                                        "error converting data as_binary_vec",
+                                        "error conversion",
+                                        tag,
+                                    ))
+                                }
+                            },
+                            _ => {
+                                // We're at the end of our data so let's break out of this loop
+                                // and set the rows_desired to the size of our data
+                                rows_desired = bigger.len();
+                                break;
+                            }
+                        }
+                    }
+                    let output_bytes = bigger[0..rows_desired].to_vec();
+                    Ok(OutputStream::one(UntaggedValue::binary(output_bytes)))
+                }
+            }
+            UntaggedValue::Row(_) => Ok(input_peek.take(rows_desired).into_output_stream()),
+            UntaggedValue::Table(_) => Err(ShellError::labeled_error(
+                "unsure how to handle UntaggedValue::Table",
+                "found table",
+                tag,
+            )),
+            UntaggedValue::Error(_) => Err(ShellError::labeled_error(
+                "unsure how to handle UntaggedValue::Error",
+                "found error",
+                tag,
+            )),
+            UntaggedValue::Block(_) => Err(ShellError::labeled_error(
+                "unsure how to handled UntaggedValue::Block",
+                "found block",
+                tag,
+            )),
+            #[cfg(all(not(target_arch = "wasm32"), feature = "dataframe"))]
+            UntaggedValue::DataFrame(_) => Err(ShellError::labeled_error(
+                "unsure how to handled UntaggedValue::DataFrame",
+                "found dataframe",
+                tag,
+            )),
+        },
+        None => Ok(input_peek.take(rows_desired).into_output_stream()),
+    }
 }
 
 #[cfg(test)]

crates/nu-pretty-hex/src/pretty_hex.rs

@@ -172,14 +172,12 @@ where
         .map(|x| *x as u8)
         .collect();
 
-    let source_part = String::from_utf8_lossy(&source_part_vec[..]);
-
     if cfg.title {
         if use_color {
             writeln!(
                 writer,
                 "Length: {0} (0x{0:x}) bytes | {1}printable {2}whitespace {3}ascii_other {4}non_ascii{5}",
-                source_part.as_bytes().as_ref().len(),
+                source_part_vec.len(),
                 Style::default().fg(Color::Cyan).bold().prefix(),
                 Style::default().fg(Color::Green).bold().prefix(),
                 Style::default().fg(Color::Purple).bold().prefix(),
@@ -187,18 +185,14 @@ where
                 Style::default().fg(Color::Yellow).suffix()
             )?;
         } else {
-            writeln!(
+            writeln!(writer, "Length: {0} (0x{0:x}) bytes", source_part_vec.len(),)?;
-                writer,
-                "Length: {0} (0x{0:x}) bytes",
-                source_part.as_bytes().as_ref().len(),
-            )?;
         }
     }
 
-    let lines = source_part.as_bytes().as_ref().chunks(if cfg.width > 0 {
+    let lines = source_part_vec.chunks(if cfg.width > 0 {
         cfg.width
     } else {
-        source_part.as_bytes().as_ref().len()
+        source_part_vec.len()
     });
 
     let lines_len = lines.len();

crates/nu-protocol/src/value.rs

@@ -82,6 +82,11 @@ impl UntaggedValue {
         }
     }
 
+    /// Returns true if this value represents a binary
+    pub fn is_binary(&self) -> bool {
+        matches!(self, &UntaggedValue::Primitive(Primitive::Binary(_)))
+    }
+
     /// Returns true if this value represents boolean true
     pub fn is_true(&self) -> bool {
         matches!(self, UntaggedValue::Primitive(Primitive::Boolean(true)))
@@ -421,6 +426,14 @@ impl Value {
         }
     }
 
+    /// View a Primitive::Binary as a Vec<u8>, if possible
+    pub fn as_binary_vec(&self) -> Result<Vec<u8>, ShellError> {
+        match &self.value {
+            UntaggedValue::Primitive(Primitive::Binary(bin)) => Ok(bin.to_vec()),
+            _ => Err(ShellError::type_error("binary", self.spanned_type_name())),
+        }
+    }
+
     /// View into the borrowed string contents of a Value, if possible
     pub fn as_forgiving_string(&self) -> Result<&str, ShellError> {
         match &self.value {

crates/nu_plugin_binaryview/src/binaryview.rs

@@ -1,6 +1,6 @@
 use crossterm::{style::Attribute, ExecutableCommand};
 use nu_pretty_hex::*;
-use nu_protocol::{outln, Value};
+use nu_protocol::outln;
 use nu_source::AnchorLocation;
 
 #[derive(Default)]
@@ -16,8 +16,6 @@ pub fn view_binary(
     b: &[u8],
     source: Option<&AnchorLocation>,
     lores_mode: bool,
-    skip: Option<&Value>,
-    length: Option<&Value>,
 ) -> Result<(), Box<dyn std::error::Error>> {
     if b.len() > 3 {
         if let (0x4e, 0x45, 0x53) = (b[0], b[1], b[2]) {
@@ -26,7 +24,7 @@ pub fn view_binary(
         }
     }
 
-    view_contents(b, source, lores_mode, skip, length)?;
+    view_contents(b, source, lores_mode)?;
     Ok(())
 }
 
@@ -210,8 +208,6 @@ pub fn view_contents(
     buffer: &[u8],
     _source: Option<&AnchorLocation>,
     lores_mode: bool,
-    skip: Option<&Value>,
-    length: Option<&Value>,
 ) -> Result<(), Box<dyn std::error::Error>> {
     // Some 'bad actor' binaries turn off ansi support so we need to make sure
     // that ansi support is enabled in windows
@@ -220,18 +216,15 @@ pub fn view_contents(
         let _ = nu_ansi_term::enable_ansi_support();
     }
 
-    let skip_bytes = skip.map(|s| s.as_usize().unwrap_or(0));
+    // we never use skip and length here because the composable pipeline should do that part
-
+    let hex_config = HexConfig {
-    let num_bytes = length.map(|b| b.as_usize().unwrap_or(0));
-
-    let config = HexConfig {
         title: true,
         ascii: true,
         width: 16,
         group: 4,
         chunk: 1,
-        skip: skip_bytes,
+        skip: None,
-        length: num_bytes,
+        length: None,
     };
 
     let mut raw_image_buffer = load_from_png_buffer(buffer);
@@ -242,7 +235,7 @@ pub fn view_contents(
 
     if raw_image_buffer.is_err() {
         //Not yet supported
-        outln!("{}", config_hex(&buffer, config));
+        outln!("{}", config_hex(&buffer, hex_config));
         return Ok(());
     }
     let raw_image_buffer = raw_image_buffer?;
@@ -296,8 +289,7 @@ pub fn view_contents(
         }
         _ => {
             //Not yet supported
-            // outln!("{:?}", buffer.hex_dump());
+            outln!("{}", config_hex(&buffer, hex_config));
-            outln!("{}", config_hex(&buffer, config));
             return Ok(());
         }
     }

crates/nu_plugin_binaryview/src/nu/mod.rs

@@ -29,9 +29,7 @@ impl Plugin for BinaryView {
             let value_anchor = v.anchor();
             if let UntaggedValue::Primitive(Primitive::Binary(b)) = &v.value {
                 let low_res = call_info.args.has("lores");
-                let skip = call_info.args.get("skip");
+                let _ = view_binary(b, value_anchor.as_ref(), low_res);
-                let length = call_info.args.get("bytes");
-                let _ = view_binary(b, value_anchor.as_ref(), low_res, skip, length);
             }
         }
     }