Add -n flag to sort (formerly only available on sort-by) (#7293)

# Description

* `-n`, `--natural` flag from `sort-by` is now on the plain `sort`.
* The `-i`, `-n` and `-r` flags now work with single record sorting
(formerly they didn't)

# User-Facing Changes

See above.

# Tests + Formatting

Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A
clippy::needless_collect` to check that you're using the standard code
style
- `cargo test --workspace` to check that all tests pass

# After Submitting

If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.
This commit is contained in:
Leon 2022-12-01 23:11:30 +10:00 committed by GitHub
parent 11977759ce
commit 5c1606ed82
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 200 additions and 24 deletions

View File

@ -1,3 +1,4 @@
use alphanumeric_sort::compare_str;
use nu_protocol::{ use nu_protocol::{
ast::Call, ast::Call,
engine::{Command, EngineState, Stack}, engine::{Command, EngineState, Stack},
@ -23,14 +24,19 @@ impl Command for Sort {
.switch("reverse", "Sort in reverse order", Some('r')) .switch("reverse", "Sort in reverse order", Some('r'))
.switch( .switch(
"ignore-case", "ignore-case",
"Sort string-based columns case-insensitively", "Sort string-based data case-insensitively",
Some('i'), Some('i'),
) )
.switch( .switch(
"values", "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'), Some('v'),
) )
.switch(
"natural",
"Sort alphanumeric string-based values naturally (1, 9, 10, 99, 100, ...)",
Some('n'),
)
.category(Category::Filters) .category(Category::Filters)
} }
@ -105,7 +111,7 @@ impl Command for Sort {
}), }),
}, },
Example { Example {
description: "Sort record by key", description: "Sort record by key (case-insensitive)",
example: "{b: 3, a: 4} | sort", example: "{b: 3, a: 4} | sort",
result: Some(Value::Record { result: Some(Value::Record {
cols: vec!["a".to_string(), "b".to_string()], cols: vec!["a".to_string(), "b".to_string()],
@ -115,10 +121,10 @@ impl Command for Sort {
}, },
Example { Example {
description: "Sort record by value", description: "Sort record by value",
example: "{a: 4, b: 3} | sort", example: "{b: 4, a: 3, c:1} | sort -v",
result: Some(Value::Record { result: Some(Value::Record {
cols: vec!["b".to_string(), "a".to_string()], cols: vec!["c".to_string(), "a".to_string(), "b".to_string()],
vals: vec![Value::test_int(3), Value::test_int(4)], vals: vec![Value::test_int(1), Value::test_int(3), Value::test_int(4)],
span: Span::test_data(), span: Span::test_data(),
}), }),
}, },
@ -134,14 +140,25 @@ impl Command for Sort {
) -> Result<PipelineData, ShellError> { ) -> Result<PipelineData, ShellError> {
let reverse = call.has_flag("reverse"); let reverse = call.has_flag("reverse");
let insensitive = call.has_flag("ignore-case"); let insensitive = call.has_flag("ignore-case");
let natural = call.has_flag("natural");
let metadata = &input.metadata(); let metadata = &input.metadata();
match input { match input {
// Records have two sorting methods, toggled by presence or absence of -v
PipelineData::Value(Value::Record { cols, vals, span }, ..) => { PipelineData::Value(Value::Record { cols, vals, span }, ..) => {
let sort_by_value = call.has_flag("values"); 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()) Ok(record.into_pipeline_data())
} }
// Other values are sorted here
PipelineData::Value(v, ..) PipelineData::Value(v, ..)
if !matches!(v, Value::List { .. } | Value::Range { .. }) => if !matches!(v, Value::List { .. } | Value::Range { .. }) =>
{ {
@ -150,7 +167,7 @@ impl Command for Sort {
pipe_data => { pipe_data => {
let mut vec: Vec<_> = pipe_data.into_iter().collect(); let mut vec: Vec<_> = pipe_data.into_iter().collect();
sort(&mut vec, call.head, insensitive)?; sort(&mut vec, call.head, insensitive, natural)?;
if reverse { if reverse {
vec.reverse() vec.reverse()
@ -167,19 +184,80 @@ impl Command for Sort {
} }
} }
fn sort_record(cols: Vec<String>, vals: Vec<Value>, rec_span: Span, sort_by_value: bool) -> Value { fn sort_record(
cols: Vec<String>,
vals: Vec<Value>,
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(); let mut input_pairs: Vec<(String, Value)> = cols.into_iter().zip(vals).collect();
if sort_by_value { input_pairs.sort_by(|a, b| {
input_pairs.sort_by(|a, b| a.1.partial_cmp(&b.1).unwrap_or(Ordering::Equal)); // Extract the data (if sort_by_value) or the column names for comparison
} else { let left_res = if sort_by_value {
input_pairs.sort_by(|a, b| a.0.partial_cmp(&b.0).unwrap_or(Ordering::Equal)); 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_cols = Vec::with_capacity(input_pairs.len());
let mut new_vals = Vec::with_capacity(input_pairs.len()); let mut new_vals = Vec::with_capacity(input_pairs.len());
for (col, val) in input_pairs { for (col, val) in input_pairs {
new_cols.push(col); new_cols.push(col);
new_vals.push(val) new_vals.push(val)
} }
if reverse {
new_cols.reverse();
new_vals.reverse();
}
Value::Record { Value::Record {
cols: new_cols, cols: new_cols,
vals: new_vals, vals: new_vals,
@ -187,7 +265,12 @@ fn sort_record(cols: Vec<String>, vals: Vec<Value>, 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() { if vec.is_empty() {
return Err(ShellError::GenericError( return Err(ShellError::GenericError(
"no values to work with".to_string(), "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(); 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| { vec.sort_by(|a, b| {
@ -226,9 +309,21 @@ pub fn sort(vec: &mut [Value], span: Span, insensitive: bool) -> Result<(), Shel
_ => b.clone(), _ => b.clone(),
}; };
lowercase_left if natural {
.partial_cmp(&lowercase_right) match (lowercase_left.as_string(), lowercase_right.as_string()) {
.unwrap_or(Ordering::Equal) (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 { } else {
a.partial_cmp(b).unwrap_or(Ordering::Equal) a.partial_cmp(b).unwrap_or(Ordering::Equal)
} }
@ -244,6 +339,7 @@ pub fn process(
columns: &[String], columns: &[String],
span: Span, span: Span,
insensitive: bool, insensitive: bool,
natural: bool,
) -> Ordering { ) -> Ordering {
for column in columns { for column in columns {
let left_value = left.get_data_by_key(column); let left_value = left.get_data_by_key(column);
@ -276,9 +372,16 @@ pub fn process(
}, },
_ => right_res, _ => right_res,
}; };
lowercase_left if natural {
.partial_cmp(&lowercase_right) match (lowercase_left.as_string(), lowercase_right.as_string()) {
.unwrap_or(Ordering::Equal) (Ok(left), Ok(right)) => compare_str(left, right),
_ => Ordering::Equal,
}
} else {
lowercase_left
.partial_cmp(&lowercase_right)
.unwrap_or(Ordering::Equal)
}
} else { } else {
left_res.partial_cmp(&right_res).unwrap_or(Ordering::Equal) left_res.partial_cmp(&right_res).unwrap_or(Ordering::Equal)
}; };

View File

@ -26,7 +26,7 @@ impl Command for SortBy {
) )
.switch( .switch(
"natural", "natural",
"Sort alphanumeric string-based columns naturally", "Sort alphanumeric string-based columns naturally (1, 9, 10, 99, 100, ...)",
Some('n'), Some('n'),
) )
.category(Category::Filters) .category(Category::Filters)

View File

@ -75,6 +75,7 @@ mod semicolon;
mod seq_char; mod seq_char;
mod shells; mod shells;
mod skip; mod skip;
mod sort;
mod sort_by; mod sort_by;
mod source_env; mod source_env;
mod split_by; mod split_by;

View File

@ -1,6 +1,5 @@
use nu_test_support::{nu, pipeline}; use nu_test_support::{nu, pipeline};
#[test] #[test]
fn by_invalid_types() { fn by_invalid_types() {
let actual = nu!( 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]]"#; let json_output = r#"[1,2,3,4,"a","b","c","d",[1,2,3],[4,5,6]]"#;
assert_eq!(actual.out, json_output); 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}"#);
}