diff --git a/crates/nu-command/src/filters/sort.rs b/crates/nu-command/src/filters/sort.rs index 5c2b4c97d6..3dc2118517 100644 --- a/crates/nu-command/src/filters/sort.rs +++ b/crates/nu-command/src/filters/sort.rs @@ -1,3 +1,4 @@ +use alphanumeric_sort::compare_str; use nu_protocol::{ ast::Call, engine::{Command, EngineState, Stack}, @@ -23,14 +24,19 @@ impl Command for Sort { .switch("reverse", "Sort in reverse order", Some('r')) .switch( "ignore-case", - "Sort string-based columns case-insensitively", + "Sort string-based data case-insensitively", Some('i'), ) .switch( "values", - "If input is a single record, sort the record by values, ignored if input is not a single record", + "If input is a single record, sort the record by values; ignored if input is not a single record", Some('v'), ) + .switch( + "natural", + "Sort alphanumeric string-based values naturally (1, 9, 10, 99, 100, ...)", + Some('n'), + ) .category(Category::Filters) } @@ -105,7 +111,7 @@ impl Command for Sort { }), }, Example { - description: "Sort record by key", + description: "Sort record by key (case-insensitive)", example: "{b: 3, a: 4} | sort", result: Some(Value::Record { cols: vec!["a".to_string(), "b".to_string()], @@ -115,10 +121,10 @@ impl Command for Sort { }, Example { description: "Sort record by value", - example: "{a: 4, b: 3} | sort", + example: "{b: 4, a: 3, c:1} | sort -v", result: Some(Value::Record { - cols: vec!["b".to_string(), "a".to_string()], - vals: vec![Value::test_int(3), Value::test_int(4)], + cols: vec!["c".to_string(), "a".to_string(), "b".to_string()], + vals: vec![Value::test_int(1), Value::test_int(3), Value::test_int(4)], span: Span::test_data(), }), }, @@ -134,14 +140,25 @@ impl Command for Sort { ) -> Result { let reverse = call.has_flag("reverse"); let insensitive = call.has_flag("ignore-case"); + let natural = call.has_flag("natural"); let metadata = &input.metadata(); match input { + // Records have two sorting methods, toggled by presence or absence of -v PipelineData::Value(Value::Record { cols, vals, span }, ..) => { let sort_by_value = call.has_flag("values"); - let record = sort_record(cols, vals, span, sort_by_value); + let record = sort_record( + cols, + vals, + span, + sort_by_value, + reverse, + insensitive, + natural, + ); Ok(record.into_pipeline_data()) } + // Other values are sorted here PipelineData::Value(v, ..) if !matches!(v, Value::List { .. } | Value::Range { .. }) => { @@ -150,7 +167,7 @@ impl Command for Sort { pipe_data => { let mut vec: Vec<_> = pipe_data.into_iter().collect(); - sort(&mut vec, call.head, insensitive)?; + sort(&mut vec, call.head, insensitive, natural)?; if reverse { vec.reverse() @@ -167,19 +184,80 @@ impl Command for Sort { } } -fn sort_record(cols: Vec, vals: Vec, rec_span: Span, sort_by_value: bool) -> Value { +fn sort_record( + cols: Vec, + vals: Vec, + rec_span: Span, + sort_by_value: bool, + reverse: bool, + insensitive: bool, + natural: bool, +) -> Value { let mut input_pairs: Vec<(String, Value)> = cols.into_iter().zip(vals).collect(); - if sort_by_value { - input_pairs.sort_by(|a, b| a.1.partial_cmp(&b.1).unwrap_or(Ordering::Equal)); - } else { - input_pairs.sort_by(|a, b| a.0.partial_cmp(&b.0).unwrap_or(Ordering::Equal)); - } + input_pairs.sort_by(|a, b| { + // Extract the data (if sort_by_value) or the column names for comparison + let left_res = if sort_by_value { + match &a.1 { + Value::String { val, .. } => val.clone(), + val => { + if let Ok(val) = val.as_string() { + val + } else { + // Values that can't be turned to strings are disregarded by the sort + // (same as in sort_utils.rs) + return Ordering::Equal; + } + } + } + } else { + a.0.clone() + }; + let right_res = if sort_by_value { + match &b.1 { + Value::String { val, .. } => val.clone(), + val => { + if let Ok(val) = val.as_string() { + val + } else { + // Values that can't be turned to strings are disregarded by the sort + // (same as in sort_utils.rs) + return Ordering::Equal; + } + } + } + } else { + b.0.clone() + }; + + // Convert to lowercase if case-insensitive + let left = if insensitive { + left_res.to_ascii_lowercase() + } else { + left_res + }; + let right = if insensitive { + right_res.to_ascii_lowercase() + } else { + right_res + }; + + if natural { + compare_str(left, right) + } else { + left.partial_cmp(&right).unwrap_or(Ordering::Equal) + } + }); + let mut new_cols = Vec::with_capacity(input_pairs.len()); let mut new_vals = Vec::with_capacity(input_pairs.len()); for (col, val) in input_pairs { new_cols.push(col); new_vals.push(val) } + if reverse { + new_cols.reverse(); + new_vals.reverse(); + } Value::Record { cols: new_cols, vals: new_vals, @@ -187,7 +265,12 @@ fn sort_record(cols: Vec, vals: Vec, rec_span: Span, sort_by_valu } } -pub fn sort(vec: &mut [Value], span: Span, insensitive: bool) -> Result<(), ShellError> { +pub fn sort( + vec: &mut [Value], + span: Span, + insensitive: bool, + natural: bool, +) -> Result<(), ShellError> { if vec.is_empty() { return Err(ShellError::GenericError( "no values to work with".to_string(), @@ -205,7 +288,7 @@ pub fn sort(vec: &mut [Value], span: Span, insensitive: bool) -> Result<(), Shel .. } => { let columns = cols.clone(); - vec.sort_by(|a, b| process(a, b, &columns, span, insensitive)); + vec.sort_by(|a, b| process(a, b, &columns, span, insensitive, natural)); } _ => { vec.sort_by(|a, b| { @@ -226,9 +309,21 @@ pub fn sort(vec: &mut [Value], span: Span, insensitive: bool) -> Result<(), Shel _ => b.clone(), }; - lowercase_left - .partial_cmp(&lowercase_right) - .unwrap_or(Ordering::Equal) + if natural { + match (lowercase_left.as_string(), lowercase_right.as_string()) { + (Ok(left), Ok(right)) => compare_str(left, right), + _ => Ordering::Equal, + } + } else { + lowercase_left + .partial_cmp(&lowercase_right) + .unwrap_or(Ordering::Equal) + } + } else if natural { + match (a.as_string(), b.as_string()) { + (Ok(left), Ok(right)) => compare_str(left, right), + _ => Ordering::Equal, + } } else { a.partial_cmp(b).unwrap_or(Ordering::Equal) } @@ -244,6 +339,7 @@ pub fn process( columns: &[String], span: Span, insensitive: bool, + natural: bool, ) -> Ordering { for column in columns { let left_value = left.get_data_by_key(column); @@ -276,9 +372,16 @@ pub fn process( }, _ => right_res, }; - lowercase_left - .partial_cmp(&lowercase_right) - .unwrap_or(Ordering::Equal) + if natural { + match (lowercase_left.as_string(), lowercase_right.as_string()) { + (Ok(left), Ok(right)) => compare_str(left, right), + _ => Ordering::Equal, + } + } else { + lowercase_left + .partial_cmp(&lowercase_right) + .unwrap_or(Ordering::Equal) + } } else { left_res.partial_cmp(&right_res).unwrap_or(Ordering::Equal) }; diff --git a/crates/nu-command/src/filters/sort_by.rs b/crates/nu-command/src/filters/sort_by.rs index 1d953c710d..86317d89e0 100644 --- a/crates/nu-command/src/filters/sort_by.rs +++ b/crates/nu-command/src/filters/sort_by.rs @@ -26,7 +26,7 @@ impl Command for SortBy { ) .switch( "natural", - "Sort alphanumeric string-based columns naturally", + "Sort alphanumeric string-based columns naturally (1, 9, 10, 99, 100, ...)", Some('n'), ) .category(Category::Filters) diff --git a/crates/nu-command/tests/commands/mod.rs b/crates/nu-command/tests/commands/mod.rs index 1851fbadca..63d68eca36 100644 --- a/crates/nu-command/tests/commands/mod.rs +++ b/crates/nu-command/tests/commands/mod.rs @@ -75,6 +75,7 @@ mod semicolon; mod seq_char; mod shells; mod skip; +mod sort; mod sort_by; mod source_env; mod split_by; diff --git a/crates/nu-command/tests/commands/sort.rs b/crates/nu-command/tests/commands/sort.rs index e118487134..c72b3fa497 100644 --- a/crates/nu-command/tests/commands/sort.rs +++ b/crates/nu-command/tests/commands/sort.rs @@ -1,6 +1,5 @@ use nu_test_support::{nu, pipeline}; - #[test] fn by_invalid_types() { let actual = nu!( @@ -46,3 +45,76 @@ fn sort_different_types() { let json_output = r#"[1,2,3,4,"a","b","c","d",[1,2,3],[4,5,6]]"#; assert_eq!(actual.out, json_output); } + +#[test] +fn sort_natural() { + let actual = nu!( + cwd: ".", pipeline( + r#"['1' '2' '3' '4' '5' '10' '100'] | sort -n | to nuon"# + )); + + assert_eq!(actual.out, r#"["1", "2", "3", "4", "5", "10", "100"]"#); +} + +#[test] +fn sort_record_natural() { + let actual = nu!( + cwd: ".", pipeline( + r#"{10:0,99:0,1:0,9:0,100:0} | sort -n | to nuon"# + )); + + assert_eq!( + actual.out, + r#"{"1": 0, "9": 0, "10": 0, "99": 0, "100": 0}"# + ); +} + +#[test] +fn sort_record_insensitive() { + let actual = nu!( + cwd: ".", pipeline( + r#"{abe:1,zed:2,ABE:3} | sort -i | to nuon"# + )); + + assert_eq!(actual.out, r#"{abe: 1, ABE: 3, zed: 2}"#); +} + +#[test] +fn sort_record_insensitive_reverse() { + let actual = nu!( + cwd: ".", pipeline( + r#"{abe:1,zed:2,ABE:3} | sort -ir | to nuon"# + )); + + assert_eq!(actual.out, r#"{zed: 2, ABE: 3, abe: 1}"#); +} + +#[test] +fn sort_record_values_natural() { + let actual = nu!( + cwd: ".", pipeline( + r#"{1:"1",2:"2",4:"100",3:"10"} | sort -vn | to nuon"# + )); + + assert_eq!(actual.out, r#"{"1": "1", "2": "2", "3": "10", "4": "100"}"#); +} + +#[test] +fn sort_record_values_insensitive() { + let actual = nu!( + cwd: ".", pipeline( + r#"{1:abe,2:zed,3:ABE} | sort -vi | to nuon"# + )); + + assert_eq!(actual.out, r#"{"1": abe, "3": ABE, "2": zed}"#); +} + +#[test] +fn sort_record_values_insensitive_reverse() { + let actual = nu!( + cwd: ".", pipeline( + r#"{1:abe,2:zed,3:ABE} | sort -vir | to nuon"# + )); + + assert_eq!(actual.out, r#"{"2": zed, "3": ABE, "1": abe}"#); +}