diff --git a/src/uu/sort/src/chunks.rs b/src/uu/sort/src/chunks.rs index 3387817b06a..ca9efcd13e8 100644 --- a/src/uu/sort/src/chunks.rs +++ b/src/uu/sort/src/chunks.rs @@ -20,9 +20,7 @@ use self_cell::self_cell; use uucore::error::{UResult, USimpleError, strip_errno}; use uucore::translate; -use crate::{ - GeneralBigDecimalParseResult, GlobalSettings, Line, SortMode, numeric_str_cmp::NumInfo, -}; +use crate::{GeneralBigDecimalParseResult, GlobalSettings, Line, numeric_str_cmp::NumInfo}; const ALLOC_CHUNK_SIZE: usize = 64 * 1024; const MAX_TOKEN_BUFFER_BYTES: usize = 4 * 1024 * 1024; @@ -312,7 +310,7 @@ fn parse_lines<'a>( .parsed_floats .reserve(estimated.saturating_mul(settings.precomputed.floats_per_line)); } - if settings.mode == SortMode::Numeric { + if settings.precomputed.whole_line_numeric { line_data.line_num_floats.reserve(estimated); } let mut start = 0usize; diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs index 7a8bd550fdb..f47f457eda9 100644 --- a/src/uu/sort/src/sort.rs +++ b/src/uu/sort/src/sort.rs @@ -332,6 +332,7 @@ struct Precomputed { fast_lexicographic: bool, fast_locale_collation: bool, fast_ascii_insensitive: bool, + whole_line_numeric: bool, tokenize_blank_thousands_sep: bool, tokenize_allow_unit_after_blank: bool, } @@ -397,6 +398,27 @@ impl GlobalSettings { self.precomputed.fast_locale_collation = disable_fast_lexicographic && self.can_use_fast_lexicographic(); self.precomputed.fast_ascii_insensitive = self.can_use_fast_ascii_insensitive(); + self.precomputed.whole_line_numeric = self.can_use_whole_line_numeric(); + } + + /// Returns true when a number parsed from the whole line can stand in for + /// the key. + /// + /// `-n` parses the line once up front and compares those numbers before + /// looking at any key. That is only the same comparison when the single + /// key spans the entire line: `-n -k1.2` sorts on the line's second + /// character onwards, and `-n -t. -k2` on its second field, neither of + /// which the line as a whole stands for. A key of its own `r` also has to + /// go the long way, since the shortcut only knows the global one. + fn can_use_whole_line_numeric(&self) -> bool { + self.mode == SortMode::Numeric && self.selectors.len() == 1 && { + let selector = &self.selectors[0]; + selector.settings.mode == SortMode::Numeric + && !selector.settings.reverse + && selector.from.field == 1 + && selector.from.char == 1 + && selector.to.is_none() + } } /// Returns true when the fast lexicographic path can be used safely. @@ -654,7 +676,7 @@ impl<'a> Line<'a> { || settings.precomputed.selections_per_line > 0 || settings.precomputed.num_infos_per_line > 0 || settings.precomputed.floats_per_line > 0 - || settings.mode == SortMode::Numeric; + || settings.precomputed.whole_line_numeric; if !needs_line_data { return Self { line, index }; } @@ -667,7 +689,7 @@ impl<'a> Line<'a> { &settings.precomputed, ); } - if settings.mode == SortMode::Numeric { + if settings.precomputed.whole_line_numeric { // exclude inf, nan, scientific notation; GNU -n does not treat '+' as a sign let line_num_float = (!line.iter().any(u8::is_ascii_alphabetic)) .then(|| std::str::from_utf8(line).ok()) diff --git a/tests/by-util/test_sort.rs b/tests/by-util/test_sort.rs index 0747ddc3935..44c59b90597 100644 --- a/tests/by-util/test_sort.rs +++ b/tests/by-util/test_sort.rs @@ -325,6 +325,56 @@ fn test_numeric_sort_rejects_leading_plus_sign() { .stdout_is("+1\n+10\n+2\n"); } +#[test] +fn test_numeric_sort_uses_the_key_not_the_whole_line() { + // `-n` compares a number parsed from the line as a whole, which only + // stands for the key when the key is the whole line. + + // Character offset: the key is "9" and "1", so 21 comes first. Comparing + // the lines instead puts 19 first. + new_ucmd!() + .args(&["-n", "-k1.2"]) + .pipe_in("19\n21\n") + .succeeds() + .stdout_is("21\n19\n"); + + // Field: with '.' as the separator the key is "9" and "1" again, while + // both lines parse as numbers on their own. + new_ucmd!() + .args(&["-n", "-t.", "-k2"]) + .pipe_in("1.9\n2.1\n") + .succeeds() + .stdout_is("2.1\n1.9\n"); + + // A key that reverses on its own: the shortcut only knows the global + // ordering, so it has to go the long way. + new_ucmd!() + .args(&["-n", "-k1r"]) + .pipe_in("19\n21\n3\n") + .succeeds() + .stdout_is("3\n21\n19\n"); + + // Equal keys fall back to comparing the lines byte by byte, not + // numerically. + new_ucmd!() + .args(&["-n", "-k1.2"]) + .pipe_in("1\n10\n2\n20\n3\n") + .succeeds() + .stdout_is("1\n10\n2\n20\n3\n"); + + // The whole-line cases keep working. + new_ucmd!() + .arg("-n") + .pipe_in("19\n21\n3\n") + .succeeds() + .stdout_is("3\n19\n21\n"); + new_ucmd!() + .args(&["-n", "-k1"]) + .pipe_in("19\n21\n3\n") + .succeeds() + .stdout_is("3\n19\n21\n"); +} + #[test] fn test_check_zero_terminated_failure() { new_ucmd!()