diff --git a/crates/nu-command/src/filters/sort.rs b/crates/nu-command/src/filters/sort.rs index aae00037c4..8b8c2e6d60 100644 --- a/crates/nu-command/src/filters/sort.rs +++ b/crates/nu-command/src/filters/sort.rs @@ -1,8 +1,7 @@ -use alphanumeric_sort::compare_str; use nu_engine::command_prelude::*; +use nu_protocol::ast::PathMember; -use nu_utils::IgnoreCaseExt; -use std::cmp::Ordering; +use crate::Comparator; #[derive(Clone)] pub struct Sort; @@ -14,10 +13,13 @@ impl Command for Sort { fn signature(&self) -> nu_protocol::Signature { Signature::build("sort") - .input_output_types(vec![( - Type::List(Box::new(Type::Any)), - Type::List(Box::new(Type::Any)), - ), (Type::record(), Type::record()),]) + .input_output_types(vec![ + ( + Type::List(Box::new(Type::Any)), + Type::List(Box::new(Type::Any)) + ), + (Type::record(), Type::record()) + ]) .switch("reverse", "Sort in reverse order", Some('r')) .switch( "ignore-case", @@ -45,67 +47,66 @@ impl Command for Sort { vec![ Example { example: "[2 0 1] | sort", - description: "sort the list by increasing value", - result: Some(Value::list( - vec![Value::test_int(0), Value::test_int(1), Value::test_int(2)], - Span::test_data(), - )), + description: "Sort the list by increasing value", + result: Some(Value::test_list(vec![ + Value::test_int(0), + Value::test_int(1), + Value::test_int(2), + ])), }, Example { example: "[2 0 1] | sort --reverse", - description: "sort the list by decreasing value", - result: Some(Value::list( - vec![Value::test_int(2), Value::test_int(1), Value::test_int(0)], - Span::test_data(), - )), + description: "Sort the list by decreasing value", + result: Some(Value::test_list(vec![ + Value::test_int(2), + Value::test_int(1), + Value::test_int(0), + ])), }, Example { example: "[betty amy sarah] | sort", - description: "sort a list of strings", - result: Some(Value::list( - vec![ - Value::test_string("amy"), - Value::test_string("betty"), - Value::test_string("sarah"), - ], - Span::test_data(), - )), + description: "Sort a list of strings", + result: Some(Value::test_list(vec![ + Value::test_string("amy"), + Value::test_string("betty"), + Value::test_string("sarah"), + ])), }, Example { example: "[betty amy sarah] | sort --reverse", - description: "sort a list of strings in reverse", - result: Some(Value::list( - vec![ - Value::test_string("sarah"), - Value::test_string("betty"), - Value::test_string("amy"), - ], - Span::test_data(), - )), + description: "Sort a list of strings in reverse", + result: Some(Value::test_list(vec![ + Value::test_string("sarah"), + Value::test_string("betty"), + Value::test_string("amy"), + ])), }, Example { description: "Sort strings (case-insensitive)", example: "[airplane Truck Car] | sort -i", - result: Some(Value::list( - vec![ - Value::test_string("airplane"), - Value::test_string("Car"), - Value::test_string("Truck"), - ], - Span::test_data(), - )), + result: Some(Value::test_list(vec![ + Value::test_string("airplane"), + Value::test_string("Car"), + Value::test_string("Truck"), + ])), }, Example { description: "Sort strings (reversed case-insensitive)", example: "[airplane Truck Car] | sort -i -r", - result: Some(Value::list( - vec![ - Value::test_string("Truck"), - Value::test_string("Car"), - Value::test_string("airplane"), - ], - Span::test_data(), - )), + result: Some(Value::test_list(vec![ + Value::test_string("Truck"), + Value::test_string("Car"), + Value::test_string("airplane"), + ])), + }, + Example { + description: "Sort alphanumeric strings in natural order", + example: "[foo1 foo10 foo9] | sort -n", + result: Some(Value::test_list(vec![ + Value::test_string("foo1"), + Value::test_string("foo9"), + Value::test_string("foo10"), + ])), }, Example { description: "Sort record by key (case-insensitive)", @@ -134,233 +135,65 @@ impl Command for Sort { call: &Call, input: PipelineData, ) -> Result { - let head = call.head; let reverse = call.has_flag(engine_state, stack, "reverse")?; let insensitive = call.has_flag(engine_state, stack, "ignore-case")?; let natural = call.has_flag(engine_state, stack, "natural")?; + let sort_by_value = call.has_flag(engine_state, stack, "values")?; let metadata = input.metadata(); let span = input.span().unwrap_or(call.head); - match input { - // Records have two sorting methods, toggled by presence or absence of -v - PipelineData::Value(Value::Record { val, .. }, ..) => { - let sort_by_value = call.has_flag(engine_state, stack, "values")?; - let record = sort_record( + let value = input.into_value(span)?; + let sorted: Value = match value { + Value::Record { val, .. } => { + // Records have two sorting methods, toggled by presence or absence of -v + let record = crate::sort_record( val.into_owned(), - span, sort_by_value, reverse, insensitive, natural, - ); - Ok(record.into_pipeline_data()) + )?; + Value::record(record, span) } - // Other values are sorted here - PipelineData::Value(v, ..) - if !matches!(v, Value::List { .. } | Value::Range { .. }) => - { - Ok(v.into_pipeline_data()) - } - pipe_data => { - let mut vec: Vec<_> = pipe_data.into_iter().collect(); - - sort(&mut vec, head, insensitive, natural)?; + value @ Value::List { .. } => { + // If we have a table specifically, then we want to sort along each column. + // Record's PartialOrd impl dictates that columns are compared in alphabetical order, + // so we have to explicitly compare by each column. + let r#type = value.get_type(); + let mut vec = value.into_list().expect("matched list above"); + if let Type::Table(cols) = r#type { + let columns: Vec = cols + .iter() + .map(|col| vec![PathMember::string(col.0.clone(), false, Span::unknown())]) + .map(|members| CellPath { members }) + .map(Comparator::CellPath) + .collect(); + crate::sort_by(&mut vec, columns, span, insensitive, natural)?; + } else { + crate::sort(&mut vec, insensitive, natural)?; + } if reverse { vec.reverse() } - let iter = vec.into_iter(); - Ok(iter.into_pipeline_data_with_metadata( - head, - engine_state.signals().clone(), - metadata, - )) + Value::list(vec, span) } - } - } -} - -fn sort_record( - record: Record, - rec_span: Span, - sort_by_value: bool, - reverse: bool, - insensitive: bool, - natural: bool, -) -> Value { - let mut input_pairs: Vec<(String, Value)> = record.into_iter().collect(); - 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.coerce_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; - } - } + Value::Nothing { .. } => { + return Err(ShellError::PipelineEmpty { + dst_span: value.span(), + }) } - } 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.coerce_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; - } - } + _ => { + return Err(ShellError::PipelineMismatch { + exp_input_type: "record or list".to_string(), + dst_span: call.head, + src_span: value.span(), + }) } - } else { - b.0.clone() }; - - // Fold case if case-insensitive - let left = if insensitive { - left_res.to_folded_case() - } else { - left_res - }; - let right = if insensitive { - right_res.to_folded_case() - } else { - right_res - }; - - if natural { - compare_str(left, right) - } else { - left.cmp(&right) - } - }); - - if reverse { - input_pairs.reverse(); + Ok(sorted.into_pipeline_data_with_metadata(metadata)) } - - Value::record(input_pairs.into_iter().collect(), rec_span) -} - -pub fn sort( - vec: &mut [Value], - span: Span, - insensitive: bool, - natural: bool, -) -> Result<(), ShellError> { - match vec.first() { - Some(Value::Record { val, .. }) => { - let columns: Vec = val.columns().cloned().collect(); - vec.sort_by(|a, b| process(a, b, &columns, span, insensitive, natural)); - } - _ => { - vec.sort_by(|a, b| { - let span_a = a.span(); - let span_b = b.span(); - if insensitive { - let folded_left = match a { - Value::String { val, .. } => Value::string(val.to_folded_case(), span_a), - _ => a.clone(), - }; - - let folded_right = match b { - Value::String { val, .. } => Value::string(val.to_folded_case(), span_b), - _ => b.clone(), - }; - - if natural { - match ( - folded_left.coerce_into_string(), - folded_right.coerce_into_string(), - ) { - (Ok(left), Ok(right)) => compare_str(left, right), - _ => Ordering::Equal, - } - } else { - folded_left - .partial_cmp(&folded_right) - .unwrap_or(Ordering::Equal) - } - } else if natural { - match (a.coerce_str(), b.coerce_str()) { - (Ok(left), Ok(right)) => compare_str(left, right), - _ => Ordering::Equal, - } - } else { - a.partial_cmp(b).unwrap_or(Ordering::Equal) - } - }); - } - } - Ok(()) -} - -pub fn process( - left: &Value, - right: &Value, - columns: &[String], - span: Span, - insensitive: bool, - natural: bool, -) -> Ordering { - for column in columns { - let left_value = left.get_data_by_key(column); - - let left_res = match left_value { - Some(left_res) => left_res, - None => Value::nothing(span), - }; - - let right_value = right.get_data_by_key(column); - - let right_res = match right_value { - Some(right_res) => right_res, - None => Value::nothing(span), - }; - - let result = if insensitive { - let span_left = left_res.span(); - let span_right = right_res.span(); - let folded_left = match left_res { - Value::String { val, .. } => Value::string(val.to_folded_case(), span_left), - _ => left_res, - }; - - let folded_right = match right_res { - Value::String { val, .. } => Value::string(val.to_folded_case(), span_right), - _ => right_res, - }; - if natural { - match ( - folded_left.coerce_into_string(), - folded_right.coerce_into_string(), - ) { - (Ok(left), Ok(right)) => compare_str(left, right), - _ => Ordering::Equal, - } - } else { - folded_left - .partial_cmp(&folded_right) - .unwrap_or(Ordering::Equal) - } - } else { - left_res.partial_cmp(&right_res).unwrap_or(Ordering::Equal) - }; - if result != Ordering::Equal { - return result; - } - } - - Ordering::Equal } #[cfg(test)] diff --git a/crates/nu-command/src/filters/sort_by.rs b/crates/nu-command/src/filters/sort_by.rs index 440c16f490..357d3be13d 100644 --- a/crates/nu-command/src/filters/sort_by.rs +++ b/crates/nu-command/src/filters/sort_by.rs @@ -1,4 +1,6 @@ -use nu_engine::command_prelude::*; +use nu_engine::{command_prelude::*, ClosureEval}; + +use crate::Comparator; #[derive(Clone)] pub struct SortBy; @@ -18,24 +20,37 @@ impl Command for SortBy { (Type::record(), Type::table()), (Type::table(), Type::table()), ]) - .rest("columns", SyntaxShape::Any, "The column(s) to sort by.") + .rest( + "comparator", + SyntaxShape::OneOf(vec![ + SyntaxShape::CellPath, + SyntaxShape::Closure(Some(vec![SyntaxShape::Any])), // key closure + SyntaxShape::Closure(Some(vec![SyntaxShape::Any, SyntaxShape::Any])), // custom closure + ]), + "The cell path(s) or closure(s) to compare elements by.", + ) .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( "natural", - "Sort alphanumeric string-based columns naturally (1, 9, 10, 99, 100, ...)", + "Sort alphanumeric string-based data naturally (1, 9, 10, 99, 100, ...)", Some('n'), ) + .switch( + "custom", + "Use closures to specify a custom sort order, rather than to compute a comparison key", + Some('c'), + ) .allow_variants_without_examples(true) .category(Category::Filters) } fn description(&self) -> &str { - "Sort by the given columns, in increasing order." + "Sort by the given cell path or closure." } fn examples(&self) -> Vec { @@ -68,6 +83,41 @@ impl Command for SortBy { }), ])), }, + Example { + description: "Sort by a nested value", + example: "[[name info]; [Cairo {founded: 969}] [Kyoto {founded: 794}]] | sort-by info.founded", + result: Some(Value::test_list(vec![ + Value::test_record(record! { + "name" => Value::test_string("Kyoto"), + "info" => Value::test_record( + record! { "founded" => Value::test_int(794) }, + )}), + Value::test_record(record! { + "name" => Value::test_string("Cairo"), + "info" => Value::test_record( + record! { "founded" => Value::test_int(969) }, + )}) + ])), + }, + Example { + description: "Sort by the last value in a list", + example: "[[2 50] [10 1]] | sort-by { last }", + result: Some(Value::test_list(vec![ + Value::test_list(vec![Value::test_int(10), Value::test_int(1)]), + Value::test_list(vec![Value::test_int(2), Value::test_int(50)]) + ])) + }, + Example { + description: "Sort in a custom order", + example: "[7 3 2 8 4] | sort-by -c {|a, b| $a < $b}", + result: Some(Value::test_list(vec![ + Value::test_int(2), + Value::test_int(3), + Value::test_int(4), + Value::test_int(7), + Value::test_int(8), + ])) + } ] } @@ -79,39 +129,60 @@ impl Command for SortBy { input: PipelineData, ) -> Result { let head = call.head; - let columns: Vec = call.rest(engine_state, stack, 0)?; + let comparator_vals: Vec = call.rest(engine_state, stack, 0)?; let reverse = call.has_flag(engine_state, stack, "reverse")?; let insensitive = call.has_flag(engine_state, stack, "ignore-case")?; let natural = call.has_flag(engine_state, stack, "natural")?; + let custom = call.has_flag(engine_state, stack, "custom")?; let metadata = input.metadata(); let mut vec: Vec<_> = input.into_iter_strict(head)?.collect(); - if columns.is_empty() { + if comparator_vals.is_empty() { return Err(ShellError::MissingParameter { - param_name: "columns".into(), + param_name: "comparator".into(), span: head, }); } - crate::sort(&mut vec, columns, head, insensitive, natural)?; + let comparators = comparator_vals + .into_iter() + .map(|val| match val { + Value::CellPath { val, .. } => Ok(Comparator::CellPath(val)), + Value::Closure { val, .. } => { + let closure_eval = ClosureEval::new(engine_state, stack, *val); + if custom { + Ok(Comparator::CustomClosure(closure_eval)) + } else { + Ok(Comparator::KeyClosure(closure_eval)) + } + } + _ => Err(ShellError::TypeMismatch { + err_message: "Cannot sort using a value which is not a cell path or closure" + .into(), + span: val.span(), + }), + }) + .collect::>()?; + + crate::sort_by(&mut vec, comparators, head, insensitive, natural)?; if reverse { vec.reverse() } - let iter = vec.into_iter(); - Ok(iter.into_pipeline_data_with_metadata(head, engine_state.signals().clone(), metadata)) + let val = Value::list(vec, head); + Ok(val.into_pipeline_data_with_metadata(metadata)) } } #[cfg(test)] mod test { + use crate::{test_examples_with_commands, Last}; + use super::*; #[test] fn test_examples() { - use crate::test_examples; - - test_examples(SortBy {}) + test_examples_with_commands(SortBy {}, &[&Last]); } } diff --git a/crates/nu-command/src/sort_utils.rs b/crates/nu-command/src/sort_utils.rs index 01bb604eac..7d70121096 100644 --- a/crates/nu-command/src/sort_utils.rs +++ b/crates/nu-command/src/sort_utils.rs @@ -1,351 +1,286 @@ -use alphanumeric_sort::compare_str; -use nu_engine::column::nonexistent_column; -use nu_protocol::{ShellError, Span, Value}; +use nu_engine::ClosureEval; +use nu_protocol::{ast::CellPath, PipelineData, Record, ShellError, Span, Value}; use nu_utils::IgnoreCaseExt; use std::cmp::Ordering; -// This module includes sorting functionality that is useful in sort-by and elsewhere. -// Eventually it would be nice to find a better home for it; sorting logic is only coupled -// to commands for historical reasons. +/// A specification of sort order for `sort_by`. +/// +/// A closure comparator allows the user to return custom ordering to sort by. +/// A cell path comparator uses the value referred to by the cell path as the sorting key. +pub enum Comparator { + KeyClosure(ClosureEval), + CustomClosure(ClosureEval), + CellPath(CellPath), +} -/// Sort a value. This only makes sense for lists and list-like things, -/// so for everything else we just return the value as-is. -/// CustomValues are converted to their base value and then sorted. -pub fn sort_value( - val: &Value, - sort_columns: Vec, - ascending: bool, - insensitive: bool, - natural: bool, -) -> Result { - let span = val.span(); - match val { - Value::List { vals, .. } => { - let mut vals = vals.clone(); - sort(&mut vals, sort_columns, span, insensitive, natural)?; +/// Sort a slice of `Value`s. +/// +/// Sort has the following invariants, in order of precedence: +/// - Null values (Nothing type) are always sorted to the end. +/// - For natural sort, numeric values (numeric strings, ints, and floats) appear first, sorted by numeric value +/// - Values appear by order of `Value`'s `PartialOrd`. +/// - Sorting for values with equal ordering is stable. +/// +/// Generally, values of different types are ordered by order of appearance in the `Value` enum. +/// However, this is not always the case. For example, ints and floats will be grouped together since +/// `Value`'s `PartialOrd` defines a non-decreasing ordering between non-decreasing integers and floats. +pub fn sort(vec: &mut [Value], insensitive: bool, natural: bool) -> Result<(), ShellError> { + // allow the comparator function to indicate error + // by mutating this option captured by the closure, + // since sort_by closure must be infallible + let mut compare_err: Option = None; - if !ascending { - vals.reverse(); - } - - Ok(Value::list(vals, span)) + vec.sort_by(|a, b| { + // we've already hit an error, bail out now + if compare_err.is_some() { + return Ordering::Equal; } - Value::Custom { val, .. } => { - let base_val = val.to_base_value(span)?; - sort_value(&base_val, sort_columns, ascending, insensitive, natural) - } - _ => Ok(val.to_owned()), + + compare_values(a, b, insensitive, natural).unwrap_or_else(|err| { + compare_err.get_or_insert(err); + Ordering::Equal + }) + }); + + if let Some(err) = compare_err { + Err(err) + } else { + Ok(()) } } -/// Sort a value in-place. This is more efficient than sort_value() because it -/// avoids cloning, but it does not work for CustomValues; they are returned as-is. -pub fn sort_value_in_place( - val: &mut Value, - sort_columns: Vec, - ascending: bool, - insensitive: bool, - natural: bool, -) -> Result<(), ShellError> { - let span = val.span(); - if let Value::List { vals, .. } = val { - sort(vals, sort_columns, span, insensitive, natural)?; - if !ascending { - vals.reverse(); - } - } - Ok(()) -} - -pub fn sort( +/// Sort a slice of `Value`s by criteria specified by one or multiple `Comparator`s. +pub fn sort_by( vec: &mut [Value], - sort_columns: Vec, - span: Span, + mut comparators: Vec, + head_span: Span, insensitive: bool, natural: bool, ) -> Result<(), ShellError> { - let val_span = vec.first().map(|v| v.span()).unwrap_or(span); - match vec.first() { - Some(Value::Record { val: record, .. }) => { - if sort_columns.is_empty() { - // This uses the same format as the 'requires a column name' error in split_by.rs - return Err(ShellError::GenericError { - error: "expected name".into(), - msg: "requires a column name to sort table data".into(), - span: Some(span), - help: None, - inner: vec![], - }); - } - - if let Some(nonexistent) = nonexistent_column(&sort_columns, record.columns()) { - return Err(ShellError::CantFindColumn { - col_name: nonexistent, - span: Some(span), - src_span: val_span, - }); - } - - // check to make sure each value in each column in the record - // that we asked for is a string. So, first collect all the columns - // that we asked for into vals, then later make sure they're all - // strings. - let mut vals = vec![]; - for item in vec.iter() { - for col in &sort_columns { - let val = item - .get_data_by_key(col) - .unwrap_or_else(|| Value::nothing(Span::unknown())); - vals.push(val); - } - } - - let should_sort_case_insensitively = insensitive - && vals - .iter() - .all(|x| matches!(x.get_type(), nu_protocol::Type::String)); - - let should_sort_case_naturally = natural - && vals - .iter() - .all(|x| matches!(x.get_type(), nu_protocol::Type::String)); - - vec.sort_by(|a, b| { - compare( - a, - b, - &sort_columns, - span, - should_sort_case_insensitively, - should_sort_case_naturally, - ) - }); - } - _ => { - vec.sort_by(|a, b| { - if insensitive { - let span_a = a.span(); - let span_b = b.span(); - let folded_left = match a { - Value::String { val, .. } => Value::string(val.to_folded_case(), span_a), - _ => a.clone(), - }; - - let folded_right = match b { - Value::String { val, .. } => Value::string(val.to_folded_case(), span_b), - _ => b.clone(), - }; - - if natural { - match ( - folded_left.coerce_into_string(), - folded_right.coerce_into_string(), - ) { - (Ok(left), Ok(right)) => compare_str(left, right), - _ => Ordering::Equal, - } - } else { - folded_left - .partial_cmp(&folded_right) - .unwrap_or(Ordering::Equal) - } - } else if natural { - match (a.coerce_str(), b.coerce_str()) { - (Ok(left), Ok(right)) => compare_str(left, right), - _ => Ordering::Equal, - } - } else { - a.partial_cmp(b).unwrap_or(Ordering::Equal) - } - }); - } + if comparators.is_empty() { + // This uses the same format as the 'requires a column name' error in split_by.rs + return Err(ShellError::GenericError { + error: "expected name".into(), + msg: "requires a cell path or closure to sort data".into(), + span: Some(head_span), + help: None, + inner: vec![], + }); + } + + // allow the comparator function to indicate error + // by mutating this option captured by the closure, + // since sort_by closure must be infallible + let mut compare_err: Option = None; + + vec.sort_by(|a, b| { + compare_by( + a, + b, + &mut comparators, + head_span, + insensitive, + natural, + &mut compare_err, + ) + }); + + if let Some(err) = compare_err { + Err(err) + } else { + Ok(()) } - Ok(()) } -pub fn compare( +/// Sort a record's key-value pairs. +/// +/// Can sort by key or by value. +pub fn sort_record( + record: Record, + sort_by_value: bool, + reverse: bool, + insensitive: bool, + natural: bool, +) -> Result { + let mut input_pairs: Vec<(String, Value)> = record.into_iter().collect(); + + // allow the comparator function to indicate error + // by mutating this option captured by the closure, + // since sort_by closure must be infallible + let mut compare_err: Option = None; + + if sort_by_value { + input_pairs.sort_by(|a, b| { + // we've already hit an error, bail out now + if compare_err.is_some() { + return Ordering::Equal; + } + + compare_values(&a.1, &b.1, insensitive, natural).unwrap_or_else(|err| { + compare_err.get_or_insert(err); + Ordering::Equal + }) + }); + } else { + input_pairs.sort_by(|a, b| compare_strings(&a.0, &b.0, insensitive, natural)); + }; + + if let Some(err) = compare_err { + return Err(err); + } + + if reverse { + input_pairs.reverse() + } + + Ok(input_pairs.into_iter().collect()) +} + +pub fn compare_by( left: &Value, right: &Value, - columns: &[String], + comparators: &mut [Comparator], span: Span, insensitive: bool, natural: bool, + error: &mut Option, ) -> Ordering { - for column in columns { - let left_value = left.get_data_by_key(column); - - let left_res = match left_value { - Some(left_res) => left_res, - None => Value::nothing(span), - }; - - let right_value = right.get_data_by_key(column); - - let right_res = match right_value { - Some(right_res) => right_res, - None => Value::nothing(span), - }; - - let result = if insensitive { - let span_left = left_res.span(); - let span_right = right_res.span(); - let folded_left = match left_res { - Value::String { val, .. } => Value::string(val.to_folded_case(), span_left), - _ => left_res, - }; - - let folded_right = match right_res { - Value::String { val, .. } => Value::string(val.to_folded_case(), span_right), - _ => right_res, - }; - if natural { - match ( - folded_left.coerce_into_string(), - folded_right.coerce_into_string(), - ) { - (Ok(left), Ok(right)) => compare_str(left, right), - _ => Ordering::Equal, - } - } else { - folded_left - .partial_cmp(&folded_right) - .unwrap_or(Ordering::Equal) + // we've already hit an error, bail out now + if error.is_some() { + return Ordering::Equal; + } + for cmp in comparators.iter_mut() { + let result = match cmp { + Comparator::CellPath(cell_path) => { + compare_cell_path(left, right, cell_path, insensitive, natural) } - } else if natural { - match ( - left_res.coerce_into_string(), - right_res.coerce_into_string(), - ) { - (Ok(left), Ok(right)) => compare_str(left, right), - _ => Ordering::Equal, + Comparator::KeyClosure(closure) => { + compare_key_closure(left, right, closure, span, insensitive, natural) + } + Comparator::CustomClosure(closure) => { + compare_custom_closure(left, right, closure, span) } - } else { - left_res.partial_cmp(&right_res).unwrap_or(Ordering::Equal) }; - if result != Ordering::Equal { - return result; + match result { + Ok(Ordering::Equal) => {} + Ok(ordering) => return ordering, + Err(err) => { + // don't bother continuing through the remaining comparators as we've hit an error + // don't overwrite if there's an existing error + error.get_or_insert(err); + return Ordering::Equal; + } } } - Ordering::Equal } -#[cfg(test)] -mod tests { - use super::*; - use nu_protocol::{record, Value}; +/// Determines whether a value should be sorted as a string +/// +/// If we're natural sorting, we want to sort strings, integers, and floats alphanumerically, so we should string sort. +/// Otherwise, we only want to string sort if both values are strings or globs (to enable case insensitive comparison) +fn should_sort_as_string(val: &Value, natural: bool) -> bool { + matches!( + (val, natural), + (&Value::String { .. }, _) + | (&Value::Glob { .. }, _) + | (&Value::Int { .. }, true) + | (&Value::Float { .. }, true) + ) +} - #[test] - fn test_sort_value() { - let val = Value::test_list(vec![ - Value::test_record(record! { - "fruit" => Value::test_string("pear"), - "count" => Value::test_int(3), - }), - Value::test_record(record! { - "fruit" => Value::test_string("orange"), - "count" => Value::test_int(7), - }), - Value::test_record(record! { - "fruit" => Value::test_string("apple"), - "count" => Value::test_int(9), - }), - ]); +/// Simple wrapper around `should_sort_as_string` to determine if two values +/// should be compared as strings. +fn should_string_compare(left: &Value, right: &Value, natural: bool) -> bool { + should_sort_as_string(left, natural) && should_sort_as_string(right, natural) +} - let sorted_alphabetically = - sort_value(&val, vec!["fruit".to_string()], true, false, false).unwrap(); - assert_eq!( - sorted_alphabetically, - Value::test_list(vec![ - Value::test_record(record! { - "fruit" => Value::test_string("apple"), - "count" => Value::test_int(9), - }), - Value::test_record(record! { - "fruit" => Value::test_string("orange"), - "count" => Value::test_int(7), - }), - Value::test_record(record! { - "fruit" => Value::test_string("pear"), - "count" => Value::test_int(3), - }), - ],) - ); - - let sorted_by_count_desc = - sort_value(&val, vec!["count".to_string()], false, false, false).unwrap(); - assert_eq!( - sorted_by_count_desc, - Value::test_list(vec![ - Value::test_record(record! { - "fruit" => Value::test_string("apple"), - "count" => Value::test_int(9), - }), - Value::test_record(record! { - "fruit" => Value::test_string("orange"), - "count" => Value::test_int(7), - }), - Value::test_record(record! { - "fruit" => Value::test_string("pear"), - "count" => Value::test_int(3), - }), - ],) - ); - } - - #[test] - fn test_sort_value_in_place() { - let mut val = Value::test_list(vec![ - Value::test_record(record! { - "fruit" => Value::test_string("pear"), - "count" => Value::test_int(3), - }), - Value::test_record(record! { - "fruit" => Value::test_string("orange"), - "count" => Value::test_int(7), - }), - Value::test_record(record! { - "fruit" => Value::test_string("apple"), - "count" => Value::test_int(9), - }), - ]); - - sort_value_in_place(&mut val, vec!["fruit".to_string()], true, false, false).unwrap(); - assert_eq!( - val, - Value::test_list(vec![ - Value::test_record(record! { - "fruit" => Value::test_string("apple"), - "count" => Value::test_int(9), - }), - Value::test_record(record! { - "fruit" => Value::test_string("orange"), - "count" => Value::test_int(7), - }), - Value::test_record(record! { - "fruit" => Value::test_string("pear"), - "count" => Value::test_int(3), - }), - ],) - ); - - sort_value_in_place(&mut val, vec!["count".to_string()], false, false, false).unwrap(); - assert_eq!( - val, - Value::test_list(vec![ - Value::test_record(record! { - "fruit" => Value::test_string("apple"), - "count" => Value::test_int(9), - }), - Value::test_record(record! { - "fruit" => Value::test_string("orange"), - "count" => Value::test_int(7), - }), - Value::test_record(record! { - "fruit" => Value::test_string("pear"), - "count" => Value::test_int(3), - }), - ],) - ); +pub fn compare_values( + left: &Value, + right: &Value, + insensitive: bool, + natural: bool, +) -> Result { + if should_string_compare(left, right, natural) { + Ok(compare_strings( + &left.coerce_str()?, + &right.coerce_str()?, + insensitive, + natural, + )) + } else { + Ok(left.partial_cmp(right).unwrap_or(Ordering::Equal)) } } + +pub fn compare_strings(left: &str, right: &str, insensitive: bool, natural: bool) -> Ordering { + fn compare_inner(left: T, right: T, natural: bool) -> Ordering + where + T: AsRef + Ord, + { + if natural { + alphanumeric_sort::compare_str(left, right) + } else { + left.cmp(&right) + } + } + + // only allocate a String if necessary for case folding + if insensitive { + compare_inner(left.to_folded_case(), right.to_folded_case(), natural) + } else { + compare_inner(left, right, natural) + } +} + +pub fn compare_cell_path( + left: &Value, + right: &Value, + cell_path: &CellPath, + insensitive: bool, + natural: bool, +) -> Result { + let left = left.clone().follow_cell_path(&cell_path.members, false)?; + let right = right.clone().follow_cell_path(&cell_path.members, false)?; + compare_values(&left, &right, insensitive, natural) +} + +pub fn compare_key_closure( + left: &Value, + right: &Value, + closure_eval: &mut ClosureEval, + span: Span, + insensitive: bool, + natural: bool, +) -> Result { + let left_key = closure_eval + .run_with_value(left.clone())? + .into_value(span)?; + let right_key = closure_eval + .run_with_value(right.clone())? + .into_value(span)?; + compare_values(&left_key, &right_key, insensitive, natural) +} + +pub fn compare_custom_closure( + left: &Value, + right: &Value, + closure_eval: &mut ClosureEval, + span: Span, +) -> Result { + closure_eval + .add_arg(left.clone()) + .add_arg(right.clone()) + .run_with_input(PipelineData::Value( + Value::list(vec![left.clone(), right.clone()], span), + None, + )) + .and_then(|data| data.into_value(span)) + .map(|val| { + if val.is_true() { + Ordering::Less + } else { + Ordering::Greater + } + }) +} diff --git a/crates/nu-command/tests/commands/sort.rs b/crates/nu-command/tests/commands/sort.rs index 3deeba30d0..ef9b8a003e 100644 --- a/crates/nu-command/tests/commands/sort.rs +++ b/crates/nu-command/tests/commands/sort.rs @@ -33,6 +33,20 @@ fn sort_primitive_values() { assert_eq!(actual.out, "authors = [\"The Nushell Project Developers\"]"); } +#[test] +fn sort_table() { + // if a table's records are compared directly rather than holistically as a table, + // [100, 10, 5] will come before [100, 5, 8] because record comparison + // compares columns by alphabetical order, so price will be checked before quantity + let actual = + nu!("[[id, quantity, price]; [100, 10, 5], [100, 5, 8], [100, 5, 1]] | sort | to nuon"); + + assert_eq!( + actual.out, + r#"[[id, quantity, price]; [100, 5, 1], [100, 5, 8], [100, 10, 5]]"# + ); +} + #[test] fn sort_different_types() { let actual = nu!("[a, 1, b, 2, c, 3, [4, 5, 6], d, 4, [1, 2, 3]] | sort | to json --raw"); diff --git a/crates/nu-command/tests/main.rs b/crates/nu-command/tests/main.rs index 248151e74a..fa2a7168b5 100644 --- a/crates/nu-command/tests/main.rs +++ b/crates/nu-command/tests/main.rs @@ -6,6 +6,7 @@ use quickcheck_macros::quickcheck; mod commands; mod format_conversions; +mod sort_utils; fn create_default_context() -> EngineState { nu_command::add_shell_command_context(nu_cmd_lang::create_default_context()) diff --git a/crates/nu-command/tests/sort_utils.rs b/crates/nu-command/tests/sort_utils.rs new file mode 100644 index 0000000000..be305588e2 --- /dev/null +++ b/crates/nu-command/tests/sort_utils.rs @@ -0,0 +1,554 @@ +use nu_command::{sort, sort_by, sort_record, Comparator}; +use nu_protocol::{ + ast::{CellPath, PathMember}, + record, Record, Span, Value, +}; + +#[test] +fn test_sort_basic() { + let mut list = vec![ + Value::test_string("foo"), + Value::test_int(2), + Value::test_int(3), + Value::test_string("bar"), + Value::test_int(1), + Value::test_string("baz"), + ]; + + assert!(sort(&mut list, false, false).is_ok()); + assert_eq!( + list, + vec![ + Value::test_int(1), + Value::test_int(2), + Value::test_int(3), + Value::test_string("bar"), + Value::test_string("baz"), + Value::test_string("foo") + ] + ); +} + +#[test] +fn test_sort_nothing() { + // Nothing values should always be sorted to the end of any list + let mut list = vec![ + Value::test_int(1), + Value::test_nothing(), + Value::test_int(2), + Value::test_string("foo"), + Value::test_nothing(), + Value::test_string("bar"), + ]; + + assert!(sort(&mut list, false, false).is_ok()); + assert_eq!( + list, + vec![ + Value::test_int(1), + Value::test_int(2), + Value::test_string("bar"), + Value::test_string("foo"), + Value::test_nothing(), + Value::test_nothing() + ] + ); + + // Ensure that nothing values are sorted after *all* types, + // even types which may follow `Nothing` in the PartialOrd order + + // unstable_name_collision + // can be switched to std intersperse when stabilized + let mut values: Vec = + itertools::intersperse(Value::test_values(), Value::test_nothing()).collect(); + + let nulls = values + .iter() + .filter(|item| item == &&Value::test_nothing()) + .count(); + + assert!(sort(&mut values, false, false).is_ok()); + + // check if the last `nulls` values of the sorted list are indeed null + assert_eq!(&values[(nulls - 1)..], vec![Value::test_nothing(); nulls]) +} + +#[test] +fn test_sort_natural_basic() { + let mut list = vec![ + Value::test_string("foo99"), + Value::test_string("foo9"), + Value::test_string("foo1"), + Value::test_string("foo100"), + Value::test_string("foo10"), + Value::test_string("1"), + Value::test_string("10"), + Value::test_string("100"), + Value::test_string("9"), + Value::test_string("99"), + ]; + + assert!(sort(&mut list, false, false).is_ok()); + assert_eq!( + list, + vec![ + Value::test_string("1"), + Value::test_string("10"), + Value::test_string("100"), + Value::test_string("9"), + Value::test_string("99"), + Value::test_string("foo1"), + Value::test_string("foo10"), + Value::test_string("foo100"), + Value::test_string("foo9"), + Value::test_string("foo99"), + ] + ); + + assert!(sort(&mut list, false, true).is_ok()); + assert_eq!( + list, + vec![ + Value::test_string("1"), + Value::test_string("9"), + Value::test_string("10"), + Value::test_string("99"), + Value::test_string("100"), + Value::test_string("foo1"), + Value::test_string("foo9"), + Value::test_string("foo10"), + Value::test_string("foo99"), + Value::test_string("foo100"), + ] + ); +} + +#[test] +fn test_sort_natural_mixed_types() { + let mut list = vec![ + Value::test_string("1"), + Value::test_int(99), + Value::test_int(1), + Value::test_float(1000.0), + Value::test_int(9), + Value::test_string("9"), + Value::test_int(100), + Value::test_string("99"), + Value::test_float(2.0), + Value::test_string("100"), + Value::test_int(10), + Value::test_string("10"), + ]; + + assert!(sort(&mut list, false, false).is_ok()); + assert_eq!( + list, + vec![ + Value::test_int(1), + Value::test_float(2.0), + Value::test_int(9), + Value::test_int(10), + Value::test_int(99), + Value::test_int(100), + Value::test_float(1000.0), + Value::test_string("1"), + Value::test_string("10"), + Value::test_string("100"), + Value::test_string("9"), + Value::test_string("99") + ] + ); + + assert!(sort(&mut list, false, true).is_ok()); + assert_eq!( + list, + vec![ + Value::test_int(1), + Value::test_string("1"), + Value::test_float(2.0), + Value::test_int(9), + Value::test_string("9"), + Value::test_int(10), + Value::test_string("10"), + Value::test_int(99), + Value::test_string("99"), + Value::test_int(100), + Value::test_string("100"), + Value::test_float(1000.0), + ] + ); +} + +#[test] +fn test_sort_natural_no_numeric_values() { + // If list contains no numeric strings, it should be sorted the + // same with or without natural sorting + let mut normal = vec![ + Value::test_string("golf"), + Value::test_bool(false), + Value::test_string("alfa"), + Value::test_string("echo"), + Value::test_int(7), + Value::test_int(10), + Value::test_bool(true), + Value::test_string("uniform"), + Value::test_int(3), + Value::test_string("tango"), + ]; + let mut natural = normal.clone(); + + assert!(sort(&mut normal, false, false).is_ok()); + assert!(sort(&mut natural, false, true).is_ok()); + assert_eq!(normal, natural); +} + +#[test] +fn test_sort_natural_type_order() { + // This test is to prevent regression to a previous natural sort behavior + // where values of different types would be intermixed. + // Only numeric values (ints, floats, and numeric strings) should be intermixed + // + // This list would previously be incorrectly sorted like this: + // ╭────┬─────────╮ + // │ 0 │ 1 │ + // │ 1 │ golf │ + // │ 2 │ false │ + // │ 3 │ 7 │ + // │ 4 │ 10 │ + // │ 5 │ alfa │ + // │ 6 │ true │ + // │ 7 │ uniform │ + // │ 8 │ true │ + // │ 9 │ 3 │ + // │ 10 │ false │ + // │ 11 │ tango │ + // ╰────┴─────────╯ + + let mut list = vec![ + Value::test_string("golf"), + Value::test_int(1), + Value::test_bool(false), + Value::test_string("alfa"), + Value::test_int(7), + Value::test_int(10), + Value::test_bool(true), + Value::test_string("uniform"), + Value::test_bool(true), + Value::test_int(3), + Value::test_bool(false), + Value::test_string("tango"), + ]; + + assert!(sort(&mut list, false, true).is_ok()); + assert_eq!( + list, + vec![ + Value::test_bool(false), + Value::test_bool(false), + Value::test_bool(true), + Value::test_bool(true), + Value::test_int(1), + Value::test_int(3), + Value::test_int(7), + Value::test_int(10), + Value::test_string("alfa"), + Value::test_string("golf"), + Value::test_string("tango"), + Value::test_string("uniform") + ] + ); + + // Only ints, floats, and numeric strings should be intermixed + // While binary primitives and datetimes can be coerced into strings, it doesn't make sense to sort them with numbers + // Binary primitives can hold multiple values, not just one, so shouldn't be compared to single values + // Datetimes don't have a single obvious numeric representation, and if we chose one it would be ambiguous to the user + + let year_three = chrono::NaiveDate::from_ymd_opt(3, 1, 1) + .unwrap() + .and_hms_opt(0, 0, 0) + .unwrap() + .and_utc(); + + let mut list = vec![ + Value::test_int(10), + Value::test_float(6.0), + Value::test_int(1), + Value::test_binary([3]), + Value::test_string("2"), + Value::test_date(year_three.into()), + Value::test_int(4), + Value::test_binary([52]), + Value::test_float(9.0), + Value::test_string("5"), + Value::test_date(chrono::DateTime::UNIX_EPOCH.into()), + Value::test_int(7), + Value::test_string("8"), + Value::test_float(3.0), + Value::test_string("foobar"), + ]; + assert!(sort(&mut list, false, true).is_ok()); + assert_eq!( + list, + vec![ + Value::test_int(1), + Value::test_string("2"), + Value::test_float(3.0), + Value::test_int(4), + Value::test_string("5"), + Value::test_float(6.0), + Value::test_int(7), + Value::test_string("8"), + Value::test_float(9.0), + Value::test_int(10), + Value::test_string("foobar"), + // the ordering of date and binary here may change if the PartialOrd order is changed, + // but they should not be intermixed with the above + Value::test_date(year_three.into()), + Value::test_date(chrono::DateTime::UNIX_EPOCH.into()), + Value::test_binary([3]), + Value::test_binary([52]), + ] + ); +} + +#[test] +fn test_sort_insensitive() { + // Test permutations between insensitive and natural + // Ensure that strings with equal insensitive orderings + // are sorted stably. (FOO then foo, bar then BAR) + let source = vec![ + Value::test_string("FOO"), + Value::test_string("foo"), + Value::test_int(100), + Value::test_string("9"), + Value::test_string("bar"), + Value::test_int(10), + Value::test_string("baz"), + Value::test_string("BAR"), + ]; + let mut list; + + // sensitive + non-natural + list = source.clone(); + assert!(sort(&mut list, false, false).is_ok()); + assert_eq!( + list, + vec![ + Value::test_int(10), + Value::test_int(100), + Value::test_string("9"), + Value::test_string("BAR"), + Value::test_string("FOO"), + Value::test_string("bar"), + Value::test_string("baz"), + Value::test_string("foo"), + ] + ); + + // sensitive + natural + list = source.clone(); + assert!(sort(&mut list, false, true).is_ok()); + assert_eq!( + list, + vec![ + Value::test_string("9"), + Value::test_int(10), + Value::test_int(100), + Value::test_string("BAR"), + Value::test_string("FOO"), + Value::test_string("bar"), + Value::test_string("baz"), + Value::test_string("foo"), + ] + ); + + // insensitive + non-natural + list = source.clone(); + assert!(sort(&mut list, true, false).is_ok()); + assert_eq!( + list, + vec![ + Value::test_int(10), + Value::test_int(100), + Value::test_string("9"), + Value::test_string("bar"), + Value::test_string("BAR"), + Value::test_string("baz"), + Value::test_string("FOO"), + Value::test_string("foo"), + ] + ); + + // insensitive + natural + list = source.clone(); + assert!(sort(&mut list, true, true).is_ok()); + assert_eq!( + list, + vec![ + Value::test_string("9"), + Value::test_int(10), + Value::test_int(100), + Value::test_string("bar"), + Value::test_string("BAR"), + Value::test_string("baz"), + Value::test_string("FOO"), + Value::test_string("foo"), + ] + ); +} + +// Helper function to assert that two records are equal +// with their key-value pairs in the same order +fn assert_record_eq(a: Record, b: Record) { + assert_eq!( + a.into_iter().collect::>(), + b.into_iter().collect::>(), + ) +} + +#[test] +fn test_sort_record_keys() { + // Basic record sort test + let record = record! { + "golf" => Value::test_string("bar"), + "alfa" => Value::test_string("foo"), + "echo" => Value::test_int(123), + }; + + let sorted = sort_record(record, false, false, false, false).unwrap(); + assert_record_eq( + sorted, + record! { + "alfa" => Value::test_string("foo"), + "echo" => Value::test_int(123), + "golf" => Value::test_string("bar"), + }, + ); +} + +#[test] +fn test_sort_record_values() { + // This test is to prevent a regression where integers and strings would be + // intermixed non-naturally when sorting a record by value without the natural flag: + // + // This record would previously be incorrectly sorted like this: + // ╭─────────┬─────╮ + // │ alfa │ 1 │ + // │ charlie │ 1 │ + // │ india │ 10 │ + // │ juliett │ 10 │ + // │ foxtrot │ 100 │ + // │ hotel │ 100 │ + // │ delta │ 9 │ + // │ echo │ 9 │ + // │ bravo │ 99 │ + // │ golf │ 99 │ + // ╰─────────┴─────╯ + + let record = record! { + "alfa" => Value::test_string("1"), + "bravo" => Value::test_int(99), + "charlie" => Value::test_int(1), + "delta" => Value::test_int(9), + "echo" => Value::test_string("9"), + "foxtrot" => Value::test_int(100), + "golf" => Value::test_string("99"), + "hotel" => Value::test_string("100"), + "india" => Value::test_int(10), + "juliett" => Value::test_string("10"), + }; + + // non-natural sort + let sorted = sort_record(record.clone(), true, false, false, false).unwrap(); + assert_record_eq( + sorted, + record! { + "charlie" => Value::test_int(1), + "delta" => Value::test_int(9), + "india" => Value::test_int(10), + "bravo" => Value::test_int(99), + "foxtrot" => Value::test_int(100), + "alfa" => Value::test_string("1"), + "juliett" => Value::test_string("10"), + "hotel" => Value::test_string("100"), + "echo" => Value::test_string("9"), + "golf" => Value::test_string("99"), + }, + ); + + // natural sort + let sorted = sort_record(record.clone(), true, false, false, true).unwrap(); + assert_record_eq( + sorted, + record! { + "alfa" => Value::test_string("1"), + "charlie" => Value::test_int(1), + "delta" => Value::test_int(9), + "echo" => Value::test_string("9"), + "india" => Value::test_int(10), + "juliett" => Value::test_string("10"), + "bravo" => Value::test_int(99), + "golf" => Value::test_string("99"), + "foxtrot" => Value::test_int(100), + "hotel" => Value::test_string("100"), + }, + ); +} + +#[test] +fn test_sort_equivalent() { + // Ensure that sort, sort_by, and record sort have equivalent sorting logic + let phonetic = vec![ + "alfa", "bravo", "charlie", "delta", "echo", "foxtrot", "golf", "hotel", "india", + "juliett", "kilo", "lima", "mike", "november", "oscar", "papa", "quebec", "romeo", + "sierra", "tango", "uniform", "victor", "whiskey", "xray", "yankee", "zulu", + ]; + + // filter out errors, since we can't sort_by on those + let mut values: Vec = Value::test_values() + .into_iter() + .filter(|val| !matches!(val, Value::Error { .. })) + .collect(); + + // reverse sort test values + values.sort_by(|a, b| b.partial_cmp(a).unwrap()); + + let mut list = values.clone(); + let mut table: Vec = values + .clone() + .into_iter() + .map(|val| Value::test_record(record! { "value" => val })) + .collect(); + let record = Record::from_iter(phonetic.into_iter().map(str::to_string).zip(values)); + + let comparator = Comparator::CellPath(CellPath { + members: vec![PathMember::String { + val: "value".to_string(), + span: Span::test_data(), + optional: false, + }], + }); + + assert!(sort(&mut list, false, false).is_ok()); + assert!(sort_by( + &mut table, + vec![comparator], + Span::test_data(), + false, + false + ) + .is_ok()); + + let record_sorted = sort_record(record.clone(), true, false, false, false).unwrap(); + let record_vals: Vec = record_sorted.into_iter().map(|pair| pair.1).collect(); + + let table_vals: Vec = table + .clone() + .into_iter() + .map(|record| record.into_record().unwrap().remove("value").unwrap()) + .collect(); + + assert_eq!(list, record_vals); + assert_eq!(record_vals, table_vals); + // list == table_vals by transitive property +} diff --git a/crates/nu-protocol/src/value/mod.rs b/crates/nu-protocol/src/value/mod.rs index 5a6461ab18..ea7e258f3f 100644 --- a/crates/nu-protocol/src/value/mod.rs +++ b/crates/nu-protocol/src/value/mod.rs @@ -68,6 +68,21 @@ pub enum Value { #[serde(rename = "span")] internal_span: Span, }, + String { + val: String, + // note: spans are being refactored out of Value + // please use .span() instead of matching this span value + #[serde(rename = "span")] + internal_span: Span, + }, + Glob { + val: String, + no_expand: bool, + // note: spans are being refactored out of Value + // please use .span() instead of matching this span value + #[serde(rename = "span")] + internal_span: Span, + }, Filesize { val: i64, // note: spans are being refactored out of Value @@ -96,21 +111,6 @@ pub enum Value { #[serde(rename = "span")] internal_span: Span, }, - String { - val: String, - // note: spans are being refactored out of Value - // please use .span() instead of matching this span value - #[serde(rename = "span")] - internal_span: Span, - }, - Glob { - val: String, - no_expand: bool, - // note: spans are being refactored out of Value - // please use .span() instead of matching this span value - #[serde(rename = "span")] - internal_span: Span, - }, Record { val: SharedCow, // note: spans are being refactored out of Value @@ -132,12 +132,6 @@ pub enum Value { #[serde(rename = "span")] internal_span: Span, }, - Nothing { - // note: spans are being refactored out of Value - // please use .span() instead of matching this span value - #[serde(rename = "span")] - internal_span: Span, - }, Error { error: Box, // note: spans are being refactored out of Value @@ -166,6 +160,12 @@ pub enum Value { #[serde(rename = "span")] internal_span: Span, }, + Nothing { + // note: spans are being refactored out of Value + // please use .span() instead of matching this span value + #[serde(rename = "span")] + internal_span: Span, + }, } impl Clone for Value { @@ -370,6 +370,7 @@ impl Value { /// - `Int` /// - `Float` /// - `String` + /// - `Glob` /// - `Binary` (only if valid utf-8) /// - `Date` /// @@ -382,6 +383,7 @@ impl Value { /// Value::Int { .. } /// | Value::Float { .. } /// | Value::String { .. } + /// | Value::Glob { .. } /// | Value::Binary { .. } /// | Value::Date { .. } /// ), @@ -394,6 +396,7 @@ impl Value { Value::Int { val, .. } => Ok(Cow::Owned(val.to_string())), Value::Float { val, .. } => Ok(Cow::Owned(val.to_string())), Value::String { val, .. } => Ok(Cow::Borrowed(val)), + Value::Glob { val, .. } => Ok(Cow::Borrowed(val)), Value::Binary { val, .. } => match std::str::from_utf8(val) { Ok(s) => Ok(Cow::Borrowed(s)), Err(_) => self.cant_convert_to("string"), @@ -420,6 +423,7 @@ impl Value { /// - `Int` /// - `Float` /// - `String` + /// - `Glob` /// - `Binary` (only if valid utf-8) /// - `Date` /// @@ -432,6 +436,7 @@ impl Value { /// Value::Int { .. } /// | Value::Float { .. } /// | Value::String { .. } + /// | Value::Glob { .. } /// | Value::Binary { .. } /// | Value::Date { .. } /// ), @@ -449,6 +454,7 @@ impl Value { /// - `Int` /// - `Float` /// - `String` + /// - `Glob` /// - `Binary` (only if valid utf-8) /// - `Date` /// @@ -461,6 +467,7 @@ impl Value { /// Value::Int { .. } /// | Value::Float { .. } /// | Value::String { .. } + /// | Value::Glob { .. } /// | Value::Binary { .. } /// | Value::Date { .. } /// ), @@ -474,6 +481,7 @@ impl Value { Value::Int { val, .. } => Ok(val.to_string()), Value::Float { val, .. } => Ok(val.to_string()), Value::String { val, .. } => Ok(val), + Value::Glob { val, .. } => Ok(val), Value::Binary { val, .. } => match String::from_utf8(val) { Ok(s) => Ok(s), Err(err) => Value::binary(err.into_bytes(), span).cant_convert_to("string"), @@ -2073,183 +2081,183 @@ impl PartialOrd for Value { Value::Bool { val: rhs, .. } => lhs.partial_cmp(rhs), Value::Int { .. } => Some(Ordering::Less), Value::Float { .. } => Some(Ordering::Less), + Value::String { .. } => Some(Ordering::Less), + Value::Glob { .. } => Some(Ordering::Less), Value::Filesize { .. } => Some(Ordering::Less), Value::Duration { .. } => Some(Ordering::Less), Value::Date { .. } => Some(Ordering::Less), Value::Range { .. } => Some(Ordering::Less), - Value::String { .. } => Some(Ordering::Less), - Value::Glob { .. } => Some(Ordering::Less), Value::Record { .. } => Some(Ordering::Less), Value::List { .. } => Some(Ordering::Less), Value::Closure { .. } => Some(Ordering::Less), - Value::Nothing { .. } => Some(Ordering::Less), Value::Error { .. } => Some(Ordering::Less), Value::Binary { .. } => Some(Ordering::Less), Value::CellPath { .. } => Some(Ordering::Less), Value::Custom { .. } => Some(Ordering::Less), + Value::Nothing { .. } => Some(Ordering::Less), }, (Value::Int { val: lhs, .. }, rhs) => match rhs { Value::Bool { .. } => Some(Ordering::Greater), Value::Int { val: rhs, .. } => lhs.partial_cmp(rhs), Value::Float { val: rhs, .. } => compare_floats(*lhs as f64, *rhs), + Value::String { .. } => Some(Ordering::Less), + Value::Glob { .. } => Some(Ordering::Less), Value::Filesize { .. } => Some(Ordering::Less), Value::Duration { .. } => Some(Ordering::Less), Value::Date { .. } => Some(Ordering::Less), Value::Range { .. } => Some(Ordering::Less), - Value::String { .. } => Some(Ordering::Less), - Value::Glob { .. } => Some(Ordering::Less), Value::Record { .. } => Some(Ordering::Less), Value::List { .. } => Some(Ordering::Less), Value::Closure { .. } => Some(Ordering::Less), - Value::Nothing { .. } => Some(Ordering::Less), Value::Error { .. } => Some(Ordering::Less), Value::Binary { .. } => Some(Ordering::Less), Value::CellPath { .. } => Some(Ordering::Less), Value::Custom { .. } => Some(Ordering::Less), + Value::Nothing { .. } => Some(Ordering::Less), }, (Value::Float { val: lhs, .. }, rhs) => match rhs { Value::Bool { .. } => Some(Ordering::Greater), Value::Int { val: rhs, .. } => compare_floats(*lhs, *rhs as f64), Value::Float { val: rhs, .. } => compare_floats(*lhs, *rhs), + Value::String { .. } => Some(Ordering::Less), + Value::Glob { .. } => Some(Ordering::Less), Value::Filesize { .. } => Some(Ordering::Less), Value::Duration { .. } => Some(Ordering::Less), Value::Date { .. } => Some(Ordering::Less), Value::Range { .. } => Some(Ordering::Less), - Value::String { .. } => Some(Ordering::Less), - Value::Glob { .. } => Some(Ordering::Less), Value::Record { .. } => Some(Ordering::Less), Value::List { .. } => Some(Ordering::Less), Value::Closure { .. } => Some(Ordering::Less), - Value::Nothing { .. } => Some(Ordering::Less), Value::Error { .. } => Some(Ordering::Less), Value::Binary { .. } => Some(Ordering::Less), Value::CellPath { .. } => Some(Ordering::Less), Value::Custom { .. } => Some(Ordering::Less), - }, - (Value::Filesize { val: lhs, .. }, rhs) => match rhs { - Value::Bool { .. } => Some(Ordering::Greater), - Value::Int { .. } => Some(Ordering::Greater), - Value::Float { .. } => Some(Ordering::Greater), - Value::Filesize { val: rhs, .. } => lhs.partial_cmp(rhs), - Value::Duration { .. } => Some(Ordering::Less), - Value::Date { .. } => Some(Ordering::Less), - Value::Range { .. } => Some(Ordering::Less), - Value::String { .. } => Some(Ordering::Less), - Value::Glob { .. } => Some(Ordering::Less), - Value::Record { .. } => Some(Ordering::Less), - Value::List { .. } => Some(Ordering::Less), - Value::Closure { .. } => Some(Ordering::Less), Value::Nothing { .. } => Some(Ordering::Less), - Value::Error { .. } => Some(Ordering::Less), - Value::Binary { .. } => Some(Ordering::Less), - Value::CellPath { .. } => Some(Ordering::Less), - Value::Custom { .. } => Some(Ordering::Less), - }, - (Value::Duration { val: lhs, .. }, rhs) => match rhs { - Value::Bool { .. } => Some(Ordering::Greater), - Value::Int { .. } => Some(Ordering::Greater), - Value::Float { .. } => Some(Ordering::Greater), - Value::Filesize { .. } => Some(Ordering::Greater), - Value::Duration { val: rhs, .. } => lhs.partial_cmp(rhs), - Value::Date { .. } => Some(Ordering::Less), - Value::Range { .. } => Some(Ordering::Less), - Value::String { .. } => Some(Ordering::Less), - Value::Glob { .. } => Some(Ordering::Less), - Value::Record { .. } => Some(Ordering::Less), - Value::List { .. } => Some(Ordering::Less), - Value::Closure { .. } => Some(Ordering::Less), - Value::Nothing { .. } => Some(Ordering::Less), - Value::Error { .. } => Some(Ordering::Less), - Value::Binary { .. } => Some(Ordering::Less), - Value::CellPath { .. } => Some(Ordering::Less), - Value::Custom { .. } => Some(Ordering::Less), - }, - (Value::Date { val: lhs, .. }, rhs) => match rhs { - Value::Bool { .. } => Some(Ordering::Greater), - Value::Int { .. } => Some(Ordering::Greater), - Value::Float { .. } => Some(Ordering::Greater), - Value::Filesize { .. } => Some(Ordering::Greater), - Value::Duration { .. } => Some(Ordering::Greater), - Value::Date { val: rhs, .. } => lhs.partial_cmp(rhs), - Value::Range { .. } => Some(Ordering::Less), - Value::String { .. } => Some(Ordering::Less), - Value::Glob { .. } => Some(Ordering::Less), - Value::Record { .. } => Some(Ordering::Less), - Value::List { .. } => Some(Ordering::Less), - Value::Closure { .. } => Some(Ordering::Less), - Value::Nothing { .. } => Some(Ordering::Less), - Value::Error { .. } => Some(Ordering::Less), - Value::Binary { .. } => Some(Ordering::Less), - Value::CellPath { .. } => Some(Ordering::Less), - Value::Custom { .. } => Some(Ordering::Less), - }, - (Value::Range { val: lhs, .. }, rhs) => match rhs { - Value::Bool { .. } => Some(Ordering::Greater), - Value::Int { .. } => Some(Ordering::Greater), - Value::Float { .. } => Some(Ordering::Greater), - Value::Filesize { .. } => Some(Ordering::Greater), - Value::Duration { .. } => Some(Ordering::Greater), - Value::Date { .. } => Some(Ordering::Greater), - Value::Range { val: rhs, .. } => lhs.partial_cmp(rhs), - Value::String { .. } => Some(Ordering::Less), - Value::Glob { .. } => Some(Ordering::Less), - Value::Record { .. } => Some(Ordering::Less), - Value::List { .. } => Some(Ordering::Less), - Value::Closure { .. } => Some(Ordering::Less), - Value::Nothing { .. } => Some(Ordering::Less), - Value::Error { .. } => Some(Ordering::Less), - Value::Binary { .. } => Some(Ordering::Less), - Value::CellPath { .. } => Some(Ordering::Less), - Value::Custom { .. } => Some(Ordering::Less), }, (Value::String { val: lhs, .. }, rhs) => match rhs { Value::Bool { .. } => Some(Ordering::Greater), Value::Int { .. } => Some(Ordering::Greater), Value::Float { .. } => Some(Ordering::Greater), - Value::Filesize { .. } => Some(Ordering::Greater), - Value::Duration { .. } => Some(Ordering::Greater), - Value::Date { .. } => Some(Ordering::Greater), - Value::Range { .. } => Some(Ordering::Greater), Value::String { val: rhs, .. } => lhs.partial_cmp(rhs), Value::Glob { val: rhs, .. } => lhs.partial_cmp(rhs), + Value::Filesize { .. } => Some(Ordering::Less), + Value::Duration { .. } => Some(Ordering::Less), + Value::Date { .. } => Some(Ordering::Less), + Value::Range { .. } => Some(Ordering::Less), Value::Record { .. } => Some(Ordering::Less), Value::List { .. } => Some(Ordering::Less), Value::Closure { .. } => Some(Ordering::Less), - Value::Nothing { .. } => Some(Ordering::Less), Value::Error { .. } => Some(Ordering::Less), Value::Binary { .. } => Some(Ordering::Less), Value::CellPath { .. } => Some(Ordering::Less), Value::Custom { .. } => Some(Ordering::Less), + Value::Nothing { .. } => Some(Ordering::Less), }, (Value::Glob { val: lhs, .. }, rhs) => match rhs { Value::Bool { .. } => Some(Ordering::Greater), Value::Int { .. } => Some(Ordering::Greater), Value::Float { .. } => Some(Ordering::Greater), - Value::Filesize { .. } => Some(Ordering::Greater), - Value::Duration { .. } => Some(Ordering::Greater), - Value::Date { .. } => Some(Ordering::Greater), - Value::Range { .. } => Some(Ordering::Greater), Value::String { val: rhs, .. } => lhs.partial_cmp(rhs), Value::Glob { val: rhs, .. } => lhs.partial_cmp(rhs), + Value::Filesize { .. } => Some(Ordering::Less), + Value::Duration { .. } => Some(Ordering::Less), + Value::Date { .. } => Some(Ordering::Less), + Value::Range { .. } => Some(Ordering::Less), Value::Record { .. } => Some(Ordering::Less), Value::List { .. } => Some(Ordering::Less), Value::Closure { .. } => Some(Ordering::Less), - Value::Nothing { .. } => Some(Ordering::Less), Value::Error { .. } => Some(Ordering::Less), Value::Binary { .. } => Some(Ordering::Less), Value::CellPath { .. } => Some(Ordering::Less), Value::Custom { .. } => Some(Ordering::Less), + Value::Nothing { .. } => Some(Ordering::Less), + }, + (Value::Filesize { val: lhs, .. }, rhs) => match rhs { + Value::Bool { .. } => Some(Ordering::Greater), + Value::Int { .. } => Some(Ordering::Greater), + Value::Float { .. } => Some(Ordering::Greater), + Value::String { .. } => Some(Ordering::Greater), + Value::Glob { .. } => Some(Ordering::Greater), + Value::Filesize { val: rhs, .. } => lhs.partial_cmp(rhs), + Value::Duration { .. } => Some(Ordering::Less), + Value::Date { .. } => Some(Ordering::Less), + Value::Range { .. } => Some(Ordering::Less), + Value::Record { .. } => Some(Ordering::Less), + Value::List { .. } => Some(Ordering::Less), + Value::Closure { .. } => Some(Ordering::Less), + Value::Error { .. } => Some(Ordering::Less), + Value::Binary { .. } => Some(Ordering::Less), + Value::CellPath { .. } => Some(Ordering::Less), + Value::Custom { .. } => Some(Ordering::Less), + Value::Nothing { .. } => Some(Ordering::Less), + }, + (Value::Duration { val: lhs, .. }, rhs) => match rhs { + Value::Bool { .. } => Some(Ordering::Greater), + Value::Int { .. } => Some(Ordering::Greater), + Value::Float { .. } => Some(Ordering::Greater), + Value::String { .. } => Some(Ordering::Greater), + Value::Glob { .. } => Some(Ordering::Greater), + Value::Filesize { .. } => Some(Ordering::Greater), + Value::Duration { val: rhs, .. } => lhs.partial_cmp(rhs), + Value::Date { .. } => Some(Ordering::Less), + Value::Range { .. } => Some(Ordering::Less), + Value::Record { .. } => Some(Ordering::Less), + Value::List { .. } => Some(Ordering::Less), + Value::Closure { .. } => Some(Ordering::Less), + Value::Error { .. } => Some(Ordering::Less), + Value::Binary { .. } => Some(Ordering::Less), + Value::CellPath { .. } => Some(Ordering::Less), + Value::Custom { .. } => Some(Ordering::Less), + Value::Nothing { .. } => Some(Ordering::Less), + }, + (Value::Date { val: lhs, .. }, rhs) => match rhs { + Value::Bool { .. } => Some(Ordering::Greater), + Value::Int { .. } => Some(Ordering::Greater), + Value::Float { .. } => Some(Ordering::Greater), + Value::String { .. } => Some(Ordering::Greater), + Value::Glob { .. } => Some(Ordering::Greater), + Value::Filesize { .. } => Some(Ordering::Greater), + Value::Duration { .. } => Some(Ordering::Greater), + Value::Date { val: rhs, .. } => lhs.partial_cmp(rhs), + Value::Range { .. } => Some(Ordering::Less), + Value::Record { .. } => Some(Ordering::Less), + Value::List { .. } => Some(Ordering::Less), + Value::Closure { .. } => Some(Ordering::Less), + Value::Error { .. } => Some(Ordering::Less), + Value::Binary { .. } => Some(Ordering::Less), + Value::CellPath { .. } => Some(Ordering::Less), + Value::Custom { .. } => Some(Ordering::Less), + Value::Nothing { .. } => Some(Ordering::Less), + }, + (Value::Range { val: lhs, .. }, rhs) => match rhs { + Value::Bool { .. } => Some(Ordering::Greater), + Value::Int { .. } => Some(Ordering::Greater), + Value::Float { .. } => Some(Ordering::Greater), + Value::String { .. } => Some(Ordering::Greater), + Value::Glob { .. } => Some(Ordering::Greater), + Value::Filesize { .. } => Some(Ordering::Greater), + Value::Duration { .. } => Some(Ordering::Greater), + Value::Date { .. } => Some(Ordering::Greater), + Value::Range { val: rhs, .. } => lhs.partial_cmp(rhs), + Value::Record { .. } => Some(Ordering::Less), + Value::List { .. } => Some(Ordering::Less), + Value::Closure { .. } => Some(Ordering::Less), + Value::Error { .. } => Some(Ordering::Less), + Value::Binary { .. } => Some(Ordering::Less), + Value::CellPath { .. } => Some(Ordering::Less), + Value::Custom { .. } => Some(Ordering::Less), + Value::Nothing { .. } => Some(Ordering::Less), }, (Value::Record { val: lhs, .. }, rhs) => match rhs { Value::Bool { .. } => Some(Ordering::Greater), Value::Int { .. } => Some(Ordering::Greater), Value::Float { .. } => Some(Ordering::Greater), + Value::String { .. } => Some(Ordering::Greater), + Value::Glob { .. } => Some(Ordering::Greater), Value::Filesize { .. } => Some(Ordering::Greater), Value::Duration { .. } => Some(Ordering::Greater), Value::Date { .. } => Some(Ordering::Greater), Value::Range { .. } => Some(Ordering::Greater), - Value::String { .. } => Some(Ordering::Greater), - Value::Glob { .. } => Some(Ordering::Greater), Value::Record { val: rhs, .. } => { // reorder cols and vals to make more logically compare. // more general, if two record have same col and values, @@ -2279,127 +2287,127 @@ impl PartialOrd for Value { } Value::List { .. } => Some(Ordering::Less), Value::Closure { .. } => Some(Ordering::Less), - Value::Nothing { .. } => Some(Ordering::Less), Value::Error { .. } => Some(Ordering::Less), Value::Binary { .. } => Some(Ordering::Less), Value::CellPath { .. } => Some(Ordering::Less), Value::Custom { .. } => Some(Ordering::Less), + Value::Nothing { .. } => Some(Ordering::Less), }, (Value::List { vals: lhs, .. }, rhs) => match rhs { Value::Bool { .. } => Some(Ordering::Greater), Value::Int { .. } => Some(Ordering::Greater), Value::Float { .. } => Some(Ordering::Greater), + Value::String { .. } => Some(Ordering::Greater), + Value::Glob { .. } => Some(Ordering::Greater), Value::Filesize { .. } => Some(Ordering::Greater), Value::Duration { .. } => Some(Ordering::Greater), Value::Date { .. } => Some(Ordering::Greater), Value::Range { .. } => Some(Ordering::Greater), - Value::String { .. } => Some(Ordering::Greater), - Value::Glob { .. } => Some(Ordering::Greater), Value::Record { .. } => Some(Ordering::Greater), Value::List { vals: rhs, .. } => lhs.partial_cmp(rhs), Value::Closure { .. } => Some(Ordering::Less), - Value::Nothing { .. } => Some(Ordering::Less), Value::Error { .. } => Some(Ordering::Less), Value::Binary { .. } => Some(Ordering::Less), Value::CellPath { .. } => Some(Ordering::Less), Value::Custom { .. } => Some(Ordering::Less), + Value::Nothing { .. } => Some(Ordering::Less), }, (Value::Closure { val: lhs, .. }, rhs) => match rhs { Value::Bool { .. } => Some(Ordering::Greater), Value::Int { .. } => Some(Ordering::Greater), Value::Float { .. } => Some(Ordering::Greater), + Value::String { .. } => Some(Ordering::Greater), + Value::Glob { .. } => Some(Ordering::Greater), Value::Filesize { .. } => Some(Ordering::Greater), Value::Duration { .. } => Some(Ordering::Greater), Value::Date { .. } => Some(Ordering::Greater), Value::Range { .. } => Some(Ordering::Greater), - Value::String { .. } => Some(Ordering::Greater), - Value::Glob { .. } => Some(Ordering::Greater), Value::Record { .. } => Some(Ordering::Greater), Value::List { .. } => Some(Ordering::Greater), Value::Closure { val: rhs, .. } => lhs.block_id.partial_cmp(&rhs.block_id), + Value::Error { .. } => Some(Ordering::Less), + Value::Binary { .. } => Some(Ordering::Less), + Value::CellPath { .. } => Some(Ordering::Less), + Value::Custom { .. } => Some(Ordering::Less), Value::Nothing { .. } => Some(Ordering::Less), - Value::Error { .. } => Some(Ordering::Less), - Value::Binary { .. } => Some(Ordering::Less), - Value::CellPath { .. } => Some(Ordering::Less), - Value::Custom { .. } => Some(Ordering::Less), - }, - (Value::Nothing { .. }, rhs) => match rhs { - Value::Bool { .. } => Some(Ordering::Greater), - Value::Int { .. } => Some(Ordering::Greater), - Value::Float { .. } => Some(Ordering::Greater), - Value::Filesize { .. } => Some(Ordering::Greater), - Value::Duration { .. } => Some(Ordering::Greater), - Value::Date { .. } => Some(Ordering::Greater), - Value::Range { .. } => Some(Ordering::Greater), - Value::String { .. } => Some(Ordering::Greater), - Value::Glob { .. } => Some(Ordering::Greater), - Value::Record { .. } => Some(Ordering::Greater), - Value::List { .. } => Some(Ordering::Greater), - Value::Closure { .. } => Some(Ordering::Greater), - Value::Nothing { .. } => Some(Ordering::Equal), - Value::Error { .. } => Some(Ordering::Less), - Value::Binary { .. } => Some(Ordering::Less), - Value::CellPath { .. } => Some(Ordering::Less), - Value::Custom { .. } => Some(Ordering::Less), }, (Value::Error { .. }, rhs) => match rhs { Value::Bool { .. } => Some(Ordering::Greater), Value::Int { .. } => Some(Ordering::Greater), Value::Float { .. } => Some(Ordering::Greater), + Value::String { .. } => Some(Ordering::Greater), + Value::Glob { .. } => Some(Ordering::Greater), Value::Filesize { .. } => Some(Ordering::Greater), Value::Duration { .. } => Some(Ordering::Greater), Value::Date { .. } => Some(Ordering::Greater), Value::Range { .. } => Some(Ordering::Greater), - Value::String { .. } => Some(Ordering::Greater), - Value::Glob { .. } => Some(Ordering::Greater), Value::Record { .. } => Some(Ordering::Greater), Value::List { .. } => Some(Ordering::Greater), Value::Closure { .. } => Some(Ordering::Greater), - Value::Nothing { .. } => Some(Ordering::Greater), Value::Error { .. } => Some(Ordering::Equal), Value::Binary { .. } => Some(Ordering::Less), Value::CellPath { .. } => Some(Ordering::Less), Value::Custom { .. } => Some(Ordering::Less), + Value::Nothing { .. } => Some(Ordering::Less), }, (Value::Binary { val: lhs, .. }, rhs) => match rhs { Value::Bool { .. } => Some(Ordering::Greater), Value::Int { .. } => Some(Ordering::Greater), Value::Float { .. } => Some(Ordering::Greater), + Value::String { .. } => Some(Ordering::Greater), + Value::Glob { .. } => Some(Ordering::Greater), Value::Filesize { .. } => Some(Ordering::Greater), Value::Duration { .. } => Some(Ordering::Greater), Value::Date { .. } => Some(Ordering::Greater), Value::Range { .. } => Some(Ordering::Greater), - Value::String { .. } => Some(Ordering::Greater), - Value::Glob { .. } => Some(Ordering::Greater), Value::Record { .. } => Some(Ordering::Greater), Value::List { .. } => Some(Ordering::Greater), Value::Closure { .. } => Some(Ordering::Greater), - Value::Nothing { .. } => Some(Ordering::Greater), Value::Error { .. } => Some(Ordering::Greater), Value::Binary { val: rhs, .. } => lhs.partial_cmp(rhs), Value::CellPath { .. } => Some(Ordering::Less), Value::Custom { .. } => Some(Ordering::Less), + Value::Nothing { .. } => Some(Ordering::Less), }, (Value::CellPath { val: lhs, .. }, rhs) => match rhs { Value::Bool { .. } => Some(Ordering::Greater), Value::Int { .. } => Some(Ordering::Greater), Value::Float { .. } => Some(Ordering::Greater), + Value::String { .. } => Some(Ordering::Greater), + Value::Glob { .. } => Some(Ordering::Greater), Value::Filesize { .. } => Some(Ordering::Greater), Value::Duration { .. } => Some(Ordering::Greater), Value::Date { .. } => Some(Ordering::Greater), Value::Range { .. } => Some(Ordering::Greater), - Value::String { .. } => Some(Ordering::Greater), - Value::Glob { .. } => Some(Ordering::Greater), Value::Record { .. } => Some(Ordering::Greater), Value::List { .. } => Some(Ordering::Greater), Value::Closure { .. } => Some(Ordering::Greater), - Value::Nothing { .. } => Some(Ordering::Greater), Value::Error { .. } => Some(Ordering::Greater), Value::Binary { .. } => Some(Ordering::Greater), Value::CellPath { val: rhs, .. } => lhs.partial_cmp(rhs), Value::Custom { .. } => Some(Ordering::Less), + Value::Nothing { .. } => Some(Ordering::Less), }, (Value::Custom { val: lhs, .. }, rhs) => lhs.partial_cmp(rhs), + (Value::Nothing { .. }, rhs) => match rhs { + Value::Bool { .. } => Some(Ordering::Greater), + Value::Int { .. } => Some(Ordering::Greater), + Value::Float { .. } => Some(Ordering::Greater), + Value::String { .. } => Some(Ordering::Greater), + Value::Glob { .. } => Some(Ordering::Greater), + Value::Filesize { .. } => Some(Ordering::Greater), + Value::Duration { .. } => Some(Ordering::Greater), + Value::Date { .. } => Some(Ordering::Greater), + Value::Range { .. } => Some(Ordering::Greater), + Value::Record { .. } => Some(Ordering::Greater), + Value::List { .. } => Some(Ordering::Greater), + Value::Closure { .. } => Some(Ordering::Greater), + Value::Error { .. } => Some(Ordering::Greater), + Value::Binary { .. } => Some(Ordering::Greater), + Value::CellPath { .. } => Some(Ordering::Greater), + Value::Custom { .. } => Some(Ordering::Greater), + Value::Nothing { .. } => Some(Ordering::Equal), + }, } } }