From 46b672a3b50ae33241e23ce123d33b5b00b875f6 Mon Sep 17 00:00:00 2001 From: Chris AtLee Date: Wed, 15 Apr 2026 20:29:19 -0400 Subject: [PATCH] feat: make -t sort by modification time (GNU ls compat) Previously -t was short for --time (which time field to display) and required a value. This made common GNU ls idioms like 'ls -trl' fail. Now -t/--sort-time sorts by modification time (newest first), matching GNU ls behavior. The --time long form still works for selecting which time column to display. Changes: - Remove -t short form from --time flag - Add new -t/--sort-time flag (no value, sorts by modification time) - Error if -t and --sort are both given - Update help text and tests --- src/options/error.rs | 11 ++--------- src/options/filter.rs | 20 ++++++++++++++++++++ src/options/flags.rs | 5 +++-- src/options/help.rs | 3 ++- src/options/view.rs | 14 +++++++------- 5 files changed, 34 insertions(+), 19 deletions(-) diff --git a/src/options/error.rs b/src/options/error.rs index 8083ffbba..34845c786 100644 --- a/src/options/error.rs +++ b/src/options/error.rs @@ -30,14 +30,14 @@ pub enum OptionsError { Conflict(&'static Arg, &'static Arg), /// An option was given that does nothing when another one either is or - /// isn’t present. + /// isn't present. Useless(&'static Arg, bool, &'static Arg), /// An option was given that does nothing when either of two other options /// are not present. Useless2(&'static Arg, &'static Arg, &'static Arg), - /// A very specific edge case where --tree can’t be used with --all twice. + /// A very specific edge case where --tree can't be used with --all twice. TreeAllAll, /// A numeric option was given that failed to be parsed as a number. @@ -111,14 +111,7 @@ impl OptionsError { /// went wrong. #[must_use] pub fn suggestion(&self) -> Option<&'static str> { - // ‘ls -lt’ and ‘ls -ltr’ are common combinations match self { - Self::BadArgument(time, r) if *time == &flags::TIME && r == "r" => { - Some("To sort oldest files last, try \"--sort oldest\", or just \"-sold\"") - } - Self::Parse(ParseError::NeedsValue { ref flag, .. }) if *flag == Flag::Short(b't') => { - Some("To sort newest files last, try \"--sort newest\", or just \"-snew\"") - } _ => None, } } diff --git a/src/options/filter.rs b/src/options/filter.rs index e475f0a46..474ef74e0 100644 --- a/src/options/filter.rs +++ b/src/options/filter.rs @@ -55,10 +55,20 @@ impl SortField { /// Returns the default sort field if none is given, or `Err` if the /// value doesn’t correspond to a sort field we know about. fn deduce(matches: &MatchedFlags<'_>) -> Result { + let sort_time = matches.has(&flags::SORT_TIME)?; + let Some(word) = matches.get(&flags::SORT)? else { + // -t with no --sort means sort by modification time (GNU ls compat) + if sort_time { + return Ok(Self::ModifiedDate); + } return Ok(Self::default()); }; + if sort_time { + return Err(OptionsError::Conflict(&flags::SORT_TIME, &flags::SORT)); + } + // Get String because we can’t match an OsStr let Some(word) = word.to_str() else { return Err(OptionsError::BadArgument(&flags::SORT, word.into())); @@ -223,6 +233,8 @@ mod test { static TEST_ARGS: &[&Arg] = &[ &flags::SORT, + &flags::SORT_TIME, + &flags::REVERSE, &flags::ALL, &flags::ALMOST_ALL, &flags::TREE, @@ -256,6 +268,14 @@ mod test { test!(newest: SortField <- ["--sort=oldest"]; Both => Ok(SortField::ModifiedAge)); test!(age: SortField <- ["-sage"]; Both => Ok(SortField::ModifiedAge)); + // -t flag (GNU ls compat: sort by modification time) + test!(sort_time: SortField <- ["-t"]; Both => Ok(SortField::ModifiedDate)); + test!(sort_time_long: SortField <- ["--sort-time"]; Both => Ok(SortField::ModifiedDate)); + test!(sort_time_r: SortField <- ["-tr"]; Both => Ok(SortField::ModifiedDate)); + + // -t conflicts with --sort + test!(sort_time_conflict: SortField <- ["-t", "--sort=name"]; Both => Err(OptionsError::Conflict(&flags::SORT_TIME, &flags::SORT))); + test!(mix_hidden_lowercase: SortField <- ["--sort", ".name"]; Both => Ok(SortField::NameMixHidden(SortCase::AaBbCc))); test!(mix_hidden_uppercase: SortField <- ["--sort", ".Name"]; Both => Ok(SortField::NameMixHidden(SortCase::ABCabc))); diff --git a/src/options/flags.rs b/src/options/flags.rs index 95f974c28..683af6927 100644 --- a/src/options/flags.rs +++ b/src/options/flags.rs @@ -45,6 +45,7 @@ pub static LIST_DIRS: Arg = Arg { short: None, long: "list-dirs", ta pub static LEVEL: Arg = Arg { short: Some(b'L'), long: "level", takes_value: TakesValue::Necessary(None) }; pub static REVERSE: Arg = Arg { short: Some(b'r'), long: "reverse", takes_value: TakesValue::Forbidden }; pub static SORT: Arg = Arg { short: Some(b's'), long: "sort", takes_value: TakesValue::Necessary(Some(SORTS)) }; +pub static SORT_TIME: Arg = Arg { short: Some(b't'), long: "sort-time", takes_value: TakesValue::Forbidden }; pub static IGNORE_GLOB: Arg = Arg { short: Some(b'I'), long: "ignore-glob", takes_value: TakesValue::Necessary(None) }; pub static GIT_IGNORE: Arg = Arg { short: None, long: "git-ignore", takes_value: TakesValue::Forbidden }; pub static DIRS_FIRST: Arg = Arg { short: None, long: "group-directories-first", takes_value: TakesValue::Forbidden }; @@ -73,7 +74,7 @@ pub static MODIFIED: Arg = Arg { short: Some(b'm'), long: "modified", take pub static CHANGED: Arg = Arg { short: None, long: "changed", takes_value: TakesValue::Forbidden }; pub static BLOCKSIZE: Arg = Arg { short: Some(b'S'), long: "blocksize", takes_value: TakesValue::Forbidden }; pub static TOTAL_SIZE: Arg = Arg { short: None, long: "total-size", takes_value: TakesValue::Forbidden }; -pub static TIME: Arg = Arg { short: Some(b't'), long: "time", takes_value: TakesValue::Necessary(Some(TIMES)) }; +pub static TIME: Arg = Arg { short: None, long: "time", takes_value: TakesValue::Necessary(Some(TIMES)) }; pub static ACCESSED: Arg = Arg { short: Some(b'u'), long: "accessed", takes_value: TakesValue::Forbidden }; pub static CREATED: Arg = Arg { short: Some(b'U'), long: "created", takes_value: TakesValue::Forbidden }; pub static TIME_STYLE: Arg = Arg { short: None, long: "time-style", takes_value: TakesValue::Necessary(Some(TIME_STYLES)) }; @@ -107,7 +108,7 @@ pub static ALL_ARGS: Args = Args(&[ &COLOR, &COLOUR, &COLOR_SCALE, &COLOUR_SCALE, &COLOR_SCALE_MODE, &COLOUR_SCALE_MODE, &WIDTH, &NO_QUOTES, &ABSOLUTE, - &ALL, &ALMOST_ALL, &TREAT_DIRS_AS_FILES, &LIST_DIRS, &LEVEL, &REVERSE, &SORT, &DIRS_FIRST, &DIRS_LAST, + &ALL, &ALMOST_ALL, &TREAT_DIRS_AS_FILES, &LIST_DIRS, &LEVEL, &REVERSE, &SORT, &SORT_TIME, &DIRS_FIRST, &DIRS_LAST, &IGNORE_GLOB, &GIT_IGNORE, &ONLY_DIRS, &ONLY_FILES, &BINARY, &BYTES, &GROUP, &NUMERIC, &HEADER, &ICONS, &INODE, &LINKS, &MODIFIED, &CHANGED, diff --git a/src/options/help.rs b/src/options/help.rs index c11391000..d82c425b3 100644 --- a/src/options/help.rs +++ b/src/options/help.rs @@ -51,6 +51,7 @@ FILTERING AND SORTING OPTIONS -L, --level DEPTH limit the depth of recursion -r, --reverse reverse the sort order -s, --sort SORT_FIELD which field to sort by + -t, --sort-time sort by modification time, newest first (like ls -t) --group-directories-first list directories before other files --group-directories-last list directories after other files -I, --ignore-glob GLOBS glob patterns (pipe-separated) of files to ignore"; @@ -75,7 +76,7 @@ LONG VIEW OPTIONS -n, --numeric list numeric user and group IDs -O, --flags list file flags (Mac, BSD, and Windows only) -S, --blocksize show size of allocated file system blocks - -t, --time FIELD which timestamp field to list (modified, accessed, created) + --time FIELD which timestamp field to list (modified, accessed, created) -m, --modified use the modified timestamp field -u, --accessed use the accessed timestamp field -U, --created use the created timestamp field diff --git a/src/options/view.rs b/src/options/view.rs index 5c5746b2b..a30724669 100644 --- a/src/options/view.rs +++ b/src/options/view.rs @@ -739,7 +739,7 @@ mod test { test!(modified: TimeTypes <- ["--modified"]; Both => Ok(TimeTypes { modified: true, changed: false, accessed: false, created: false })); test!(m: TimeTypes <- ["-m"]; Both => Ok(TimeTypes { modified: true, changed: false, accessed: false, created: false })); test!(time_mod: TimeTypes <- ["--time=modified"]; Both => Ok(TimeTypes { modified: true, changed: false, accessed: false, created: false })); - test!(t_m: TimeTypes <- ["-tmod"]; Both => Ok(TimeTypes { modified: true, changed: false, accessed: false, created: false })); + test!(t_m: TimeTypes <- ["--time", "mod"]; Both => Ok(TimeTypes { modified: true, changed: false, accessed: false, created: false })); // Changed #[cfg(target_family = "unix")] @@ -747,30 +747,30 @@ mod test { #[cfg(target_family = "unix")] test!(time_ch: TimeTypes <- ["--time=changed"]; Both => Ok(TimeTypes { modified: false, changed: true, accessed: false, created: false })); #[cfg(target_family = "unix")] - test!(t_ch: TimeTypes <- ["-t", "ch"]; Both => Ok(TimeTypes { modified: false, changed: true, accessed: false, created: false })); + test!(t_ch: TimeTypes <- ["--time", "ch"]; Both => Ok(TimeTypes { modified: false, changed: true, accessed: false, created: false })); // Accessed test!(acc: TimeTypes <- ["--accessed"]; Both => Ok(TimeTypes { modified: false, changed: false, accessed: true, created: false })); test!(a: TimeTypes <- ["-u"]; Both => Ok(TimeTypes { modified: false, changed: false, accessed: true, created: false })); test!(time_acc: TimeTypes <- ["--time", "accessed"]; Both => Ok(TimeTypes { modified: false, changed: false, accessed: true, created: false })); - test!(time_a: TimeTypes <- ["-t", "acc"]; Both => Ok(TimeTypes { modified: false, changed: false, accessed: true, created: false })); + test!(time_a: TimeTypes <- ["--time", "acc"]; Both => Ok(TimeTypes { modified: false, changed: false, accessed: true, created: false })); // Created test!(cr: TimeTypes <- ["--created"]; Both => Ok(TimeTypes { modified: false, changed: false, accessed: false, created: true })); test!(c: TimeTypes <- ["-U"]; Both => Ok(TimeTypes { modified: false, changed: false, accessed: false, created: true })); test!(time_cr: TimeTypes <- ["--time=created"]; Both => Ok(TimeTypes { modified: false, changed: false, accessed: false, created: true })); - test!(t_cr: TimeTypes <- ["-tcr"]; Both => Ok(TimeTypes { modified: false, changed: false, accessed: false, created: true })); + test!(t_cr: TimeTypes <- ["--time=cr"]; Both => Ok(TimeTypes { modified: false, changed: false, accessed: false, created: true })); // Multiples test!(time_uu: TimeTypes <- ["-u", "--modified"]; Both => Ok(TimeTypes { modified: true, changed: false, accessed: true, created: false })); // Errors test!(time_tea: TimeTypes <- ["--time=tea"]; Both => err OptionsError::BadArgument(&flags::TIME, OsString::from("tea"))); - test!(t_ea: TimeTypes <- ["-tea"]; Both => err OptionsError::BadArgument(&flags::TIME, OsString::from("ea"))); + test!(t_ea: TimeTypes <- ["--time=tea"]; Both => err OptionsError::BadArgument(&flags::TIME, OsString::from("tea"))); // Overriding - test!(overridden: TimeTypes <- ["-tcr", "-tmod"]; Last => Ok(TimeTypes { modified: true, changed: false, accessed: false, created: false })); - test!(overridden_2: TimeTypes <- ["-tcr", "-tmod"]; Complain => err OptionsError::Duplicate(Flag::Short(b't'), Flag::Short(b't'))); + test!(overridden: TimeTypes <- ["--time=cr", "--time=mod"]; Last => Ok(TimeTypes { modified: true, changed: false, accessed: false, created: false })); + test!(overridden_2: TimeTypes <- ["--time=cr", "--time=mod"]; Complain => err OptionsError::Duplicate(Flag::Long("time"), Flag::Long("time"))); } mod views {