From a785e64bc9c762088b3f667c12796221fbd840ad Mon Sep 17 00:00:00 2001 From: Herobs Date: Thu, 24 Aug 2023 20:08:58 +0800 Subject: [PATCH] Fix 9156 endian consistency (#9873) - fixed #9156 # Description I'm trying to fix the problems mentioned in the issue. It's my first attempt in Rust. Please let me know if there are any problems. # User-Facing Changes - The `--little-endian` option dropped, replaced with `--endian`. - Add the `--compact` option to the `into binary` command. - `into int` accepts binary input --- .../nu-command/src/conversions/into/binary.rs | 99 ++++++++++++++----- crates/nu-command/src/conversions/into/int.rs | 28 +++++- .../tests/commands/conversions/into/int.rs | 25 +++++ .../tests/commands/conversions/into/mod.rs | 1 + .../tests/commands/conversions/mod.rs | 1 + crates/nu-command/tests/commands/mod.rs | 1 + .../nu-command/tests/commands/skip/skip_.rs | 2 +- 7 files changed, 128 insertions(+), 29 deletions(-) create mode 100644 crates/nu-command/tests/commands/conversions/into/int.rs create mode 100644 crates/nu-command/tests/commands/conversions/into/mod.rs create mode 100644 crates/nu-command/tests/commands/conversions/mod.rs diff --git a/crates/nu-command/src/conversions/into/binary.rs b/crates/nu-command/src/conversions/into/binary.rs index 4a12578c35..b8baa0df89 100644 --- a/crates/nu-command/src/conversions/into/binary.rs +++ b/crates/nu-command/src/conversions/into/binary.rs @@ -9,6 +9,7 @@ use nu_protocol::{ pub struct Arguments { cell_paths: Option>, + compact: bool, } impl CmdArgument for Arguments { @@ -39,6 +40,7 @@ impl Command for SubCommand { (Type::Record(vec![]), Type::Record(vec![])), ]) .allow_variants_without_examples(true) // TODO: supply exhaustive examples + .switch("compact", "output without padding zeros", Some('c')) .rest( "rest", SyntaxShape::CellPath, @@ -82,7 +84,7 @@ impl Command for SubCommand { description: "convert a number to a nushell binary primitive", example: "1 | into binary", result: Some(Value::Binary { - val: i64::from(1).to_le_bytes().to_vec(), + val: i64::from(1).to_ne_bytes().to_vec(), span: Span::test_data(), }), }, @@ -90,7 +92,7 @@ impl Command for SubCommand { description: "convert a boolean to a nushell binary primitive", example: "true | into binary", result: Some(Value::Binary { - val: i64::from(1).to_le_bytes().to_vec(), + val: i64::from(1).to_ne_bytes().to_vec(), span: Span::test_data(), }), }, @@ -108,7 +110,16 @@ impl Command for SubCommand { description: "convert a decimal to a nushell binary primitive", example: "1.234 | into binary", result: Some(Value::Binary { - val: 1.234f64.to_le_bytes().to_vec(), + val: 1.234f64.to_ne_bytes().to_vec(), + span: Span::test_data(), + }), + }, + Example { + description: + "convert an integer to a nushell binary primitive with compact enabled", + example: "10 | into binary --compact", + result: Some(Value::Binary { + val: vec![10], span: Span::test_data(), }), }, @@ -145,41 +156,28 @@ fn into_binary( .into_pipeline_data()) } _ => { - let args = Arguments { cell_paths }; + let args = Arguments { + cell_paths, + compact: call.has_flag("compact"), + }; operate(action, args, input, call.head, engine_state.ctrlc.clone()) } } } -fn int_to_endian(n: i64) -> Vec { - if cfg!(target_endian = "little") { - n.to_le_bytes().to_vec() - } else { - n.to_be_bytes().to_vec() - } -} - -fn float_to_endian(n: f64) -> Vec { - if cfg!(target_endian = "little") { - n.to_le_bytes().to_vec() - } else { - n.to_be_bytes().to_vec() - } -} - pub fn action(input: &Value, _args: &Arguments, span: Span) -> Value { - match input { + let value = match input { Value::Binary { .. } => input.clone(), Value::Int { val, .. } => Value::Binary { - val: int_to_endian(*val), + val: val.to_ne_bytes().to_vec(), span, }, Value::Float { val, .. } => Value::Binary { - val: float_to_endian(*val), + val: val.to_ne_bytes().to_vec(), span, }, Value::Filesize { val, .. } => Value::Binary { - val: int_to_endian(*val), + val: val.to_ne_bytes().to_vec(), span, }, Value::String { val, .. } => Value::Binary { @@ -187,11 +185,11 @@ pub fn action(input: &Value, _args: &Arguments, span: Span) -> Value { span, }, Value::Bool { val, .. } => Value::Binary { - val: int_to_endian(i64::from(*val)), + val: i64::from(*val).to_ne_bytes().to_vec(), span, }, Value::Duration { val, .. } => Value::Binary { - val: int_to_endian(*val), + val: val.to_ne_bytes().to_vec(), span, }, Value::Date { val, .. } => Value::Binary { @@ -209,11 +207,38 @@ pub fn action(input: &Value, _args: &Arguments, span: Span) -> Value { src_span: other.expect_span(), }), }, + }; + + if _args.compact { + if let Value::Binary { val, span } = value { + let val = if cfg!(target_endian = "little") { + match val.iter().rposition(|&x| x != 0) { + Some(idx) => &val[..idx + 1], + None => &val, + } + } else { + match val.iter().position(|&x| x != 0) { + Some(idx) => &val[idx..], + None => &val, + } + }; + + Value::Binary { + val: val.to_vec(), + span, + } + } else { + value + } + } else { + value } } #[cfg(test)] mod test { + use rstest::rstest; + use super::*; #[test] @@ -222,4 +247,26 @@ mod test { test_examples(SubCommand {}) } + + #[rstest] + #[case(vec![10], vec![10], vec![10])] + #[case(vec![10, 0, 0], vec![10], vec![10, 0, 0])] + #[case(vec![0, 0, 10], vec![0, 0, 10], vec![10])] + #[case(vec![0, 10, 0, 0], vec![0, 10], vec![10, 0, 0])] + fn test_compact(#[case] input: Vec, #[case] little: Vec, #[case] big: Vec) { + let s = Value::test_binary(input); + let actual = action( + &s, + &Arguments { + cell_paths: None, + compact: true, + }, + Span::test_data(), + ); + if cfg!(target_endian = "little") { + assert_eq!(actual, Value::test_binary(little)); + } else { + assert_eq!(actual, Value::test_binary(big)); + } + } } diff --git a/crates/nu-command/src/conversions/into/int.rs b/crates/nu-command/src/conversions/into/int.rs index d10778728d..23bc33bb9f 100644 --- a/crates/nu-command/src/conversions/into/int.rs +++ b/crates/nu-command/src/conversions/into/int.rs @@ -38,6 +38,7 @@ impl Command for SubCommand { (Type::Date, Type::Int), (Type::Duration, Type::Int), (Type::Filesize, Type::Int), + (Type::Binary, Type::Int), (Type::Table(vec![]), Type::Table(vec![])), (Type::Record(vec![]), Type::Record(vec![])), ( @@ -72,7 +73,12 @@ impl Command for SubCommand { ]) .allow_variants_without_examples(true) .named("radix", SyntaxShape::Number, "radix of integer", Some('r')) - .switch("little-endian", "use little-endian byte decoding", None) + .named( + "endian", + SyntaxShape::String, + "byte encode endian, available options: native(default), little, big", + Some('e'), + ) .rest( "rest", SyntaxShape::CellPath, @@ -113,9 +119,27 @@ impl Command for SubCommand { Some(_) => 10, None => 10, }; + + let endian = call.get_flag::(engine_state, stack, "endian")?; + let little_endian = match endian { + Some(Value::String { val, span }) => match val.as_str() { + "native" => cfg!(target_endian = "little"), + "little" => true, + "big" => false, + _ => { + return Err(ShellError::TypeMismatch { + err_message: "Endian must be one of native, little, big".to_string(), + span, + }) + } + }, + Some(_) => false, + None => cfg!(target_endian = "little"), + }; + let args = Arguments { radix, - little_endian: call.has_flag("little-endian"), + little_endian, cell_paths, }; operate(action, args, input, call.head, engine_state.ctrlc.clone()) diff --git a/crates/nu-command/tests/commands/conversions/into/int.rs b/crates/nu-command/tests/commands/conversions/into/int.rs new file mode 100644 index 0000000000..850ad6361e --- /dev/null +++ b/crates/nu-command/tests/commands/conversions/into/int.rs @@ -0,0 +1,25 @@ +use nu_test_support::nu; + +#[test] +fn convert_back_and_forth() { + let actual = nu!(r#"1 | into binary | into int"#); + assert_eq!(actual.out, "1"); +} + +#[test] +fn convert_into_int_little_endian() { + let actual = nu!(r#"0x[01 00 00 00 00 00 00 00] | into int --endian little"#); + assert_eq!(actual.out, "1"); + + let actual = nu!(r#"0x[00 00 00 00 00 00 00 01] | into int --endian little"#); + assert_eq!(actual.out, "72057594037927936"); +} + +#[test] +fn convert_into_int_big_endian() { + let actual = nu!(r#"0x[00 00 00 00 00 00 00 01] | into int --endian big"#); + assert_eq!(actual.out, "1"); + + let actual = nu!(r#"0x[01 00 00 00 00 00 00 00] | into int --endian big"#); + assert_eq!(actual.out, "72057594037927936"); +} diff --git a/crates/nu-command/tests/commands/conversions/into/mod.rs b/crates/nu-command/tests/commands/conversions/into/mod.rs new file mode 100644 index 0000000000..a7a829445a --- /dev/null +++ b/crates/nu-command/tests/commands/conversions/into/mod.rs @@ -0,0 +1 @@ +mod int; diff --git a/crates/nu-command/tests/commands/conversions/mod.rs b/crates/nu-command/tests/commands/conversions/mod.rs new file mode 100644 index 0000000000..0d4ee04b0d --- /dev/null +++ b/crates/nu-command/tests/commands/conversions/mod.rs @@ -0,0 +1 @@ +mod into; diff --git a/crates/nu-command/tests/commands/mod.rs b/crates/nu-command/tests/commands/mod.rs index 7fb76f0f21..7197bf0a19 100644 --- a/crates/nu-command/tests/commands/mod.rs +++ b/crates/nu-command/tests/commands/mod.rs @@ -8,6 +8,7 @@ mod cal; mod cd; mod compact; mod continue_; +mod conversions; mod cp; mod date; mod def; diff --git a/crates/nu-command/tests/commands/skip/skip_.rs b/crates/nu-command/tests/commands/skip/skip_.rs index 127d6429a6..34fa2a7663 100644 --- a/crates/nu-command/tests/commands/skip/skip_.rs +++ b/crates/nu-command/tests/commands/skip/skip_.rs @@ -8,7 +8,7 @@ fn binary_skip() { open sample_data.ods --raw | skip 2 | take 2 | - into int + into int --endian big "# ));