Skip to content

Commit 16af7d4

Browse files
authored
ls: Implement -f flag to disable sorting and enable -a (#8824)
* ls: implement -f flag with correct constant, color logic, and localization Fix the -f flag implementation to properly enable all files display, disable sorting, and handle color output correctly. Changes: - Correct UNSORTED_ALL constant from 'unsorted-all' to 'f' - Fix color logic to honor explicit --color flag regardless of -f position - Add French localization for ls-help-unsorted-all - Add comprehensive integration tests for -f flag behavior The explicit --color flag now always takes precedence over -f implicit color disabling, matching expected CLI behavior. * tests(ls): improve -f flag test assertions Address maintainer feedback from PR #8824 review: 1. test_f_flag_disables_sorting: Compare -f output with -a (sorted) and -U (unsorted) to prove sorting is actually disabled, not just that files appear in output 2. test_f_overrides_big_a: Rename from test_f_overrides_a_and_big_a and fix assertions to check for .. presence (distinguishes -f from -A) instead of .hidden (shown in both) 3. test_f_overrides_sort_flags: Use size-based files and explicit output comparisons to verify last-flag-wins behavior with deterministic ordering 4. test_big_u_overrides_f_sort: Use size-based files and verify -U participates in last-flag-wins by checking actual output order All tests now properly validate flag interactions instead of just checking file presence. * refactor(ls): use match expression in extract_sort Replace if/else chain with match expression for improved readability and more idiomatic Rust code. Uses match guards for index comparisons. Addresses maintainer feedback from PR #8824 review. No functional changes - behavior remains identical. * fix(ls): use snake_case for test variable names Follow Rust naming conventions by converting variable names in -f flag tests from mixed case (out_Af, out_fS, etc.) to snake_case (out_a_f, out_f_s, etc.). This eliminates clippy warnings about non_snake_case identifiers. * tests: fix flaky ls tests that depend on filesystem directory order The tests test_f_flag_disables_sorting, test_big_u_overrides_f_sort, and test_f_overrides_sort_flags made incorrect assumptions that unsorted directory order would always differ from sorted order. However, fs::read_dir() returns entries in filesystem-dependent order which may accidentally match sorted order on some filesystems. Changes: - Removed assertions comparing unsorted vs sorted outputs - Added deterministic checks (e.g., verifying --sort after -f works) - Added explicit order verification for size-sorted outputs - Tests now verify flag precedence without relying on directory order Fixes CI failures on Windows and SELinux platforms. Quality checks passed: - cargo fmt --check: ✓ - cargo clippy --test tests: ✓ - all 3 modified tests pass: ✓ * test: fix misleading test name for -U flag behavior Renamed test_big_u_overrides_f_sort to test_big_u_participates_in_sort_flag_wins to accurately reflect that it tests -U with -S interactions, not -f.
1 parent 2884718 commit 16af7d4

File tree

4 files changed

+448
-36
lines changed

4 files changed

+448
-36
lines changed

src/uu/ls/locales/en-US.ftl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ ls-help-author = Show author in long format. On the supported platforms,
8282
ls-help-all-files = Do not ignore hidden files (files with names that start with '.').
8383
ls-help-almost-all = In a directory, do not ignore all file names that start with '.',
8484
only ignore '.' and '..'.
85+
ls-help-unsorted-all = List all files in directory order, unsorted. Equivalent to -aU. Disables --color unless explicitly specified.
8586
ls-help-directory = Only list the names of directories, rather than listing directory contents.
8687
This will not follow symbolic links unless one of `--dereference-command-line
8788
(-H)`, `--dereference (-L)`, or `--dereference-command-line-symlink-to-dir` is

src/uu/ls/locales/fr-FR.ftl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ ls-help-author = Afficher l'auteur en format long. Sur les plateformes supporté
8282
ls-help-all-files = Ne pas ignorer les fichiers cachés (fichiers dont les noms commencent par '.').
8383
ls-help-almost-all = Dans un répertoire, ne pas ignorer tous les noms de fichiers qui commencent par '.',
8484
ignorer seulement '.' et '..'.
85+
ls-help-unsorted-all = Liste tous les fichiers dans l'ordre du répertoire, non triés. Équivalent à -aU. Désactive --color sauf si spécifié explicitement.
8586
ls-help-directory = Lister seulement les noms des répertoires, plutôt que le contenu des répertoires.
8687
Ceci ne suivra pas les liens symboliques à moins qu'une des options
8788
`--dereference-command-line (-H)`, `--dereference (-L)`, ou

src/uu/ls/src/ls.rs

Lines changed: 117 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ pub mod options {
9999
pub mod files {
100100
pub static ALL: &str = "all";
101101
pub static ALMOST_ALL: &str = "almost-all";
102+
pub static UNSORTED_ALL: &str = "f";
102103
}
103104

104105
pub mod sort {
@@ -437,12 +438,27 @@ fn extract_format(options: &clap::ArgMatches) -> (Format, Option<&'static str>)
437438
///
438439
/// A Files variant representing the type of files to display.
439440
fn extract_files(options: &clap::ArgMatches) -> Files {
440-
if options.get_flag(options::files::ALL) {
441-
Files::All
442-
} else if options.get_flag(options::files::ALMOST_ALL) {
441+
let get_last_index = |flag: &str| -> usize {
442+
if options.value_source(flag) == Some(clap::parser::ValueSource::CommandLine) {
443+
options.index_of(flag).unwrap_or(0)
444+
} else {
445+
0
446+
}
447+
};
448+
449+
let all_index = get_last_index(options::files::ALL);
450+
let almost_all_index = get_last_index(options::files::ALMOST_ALL);
451+
let unsorted_all_index = get_last_index(options::files::UNSORTED_ALL);
452+
453+
let max_index = all_index.max(almost_all_index).max(unsorted_all_index);
454+
455+
if max_index == 0 {
456+
Files::Normal
457+
} else if max_index == almost_all_index {
443458
Files::AlmostAll
444459
} else {
445-
Files::Normal
460+
// Either -a or -f wins, both show all files
461+
Files::All
446462
}
447463
}
448464

@@ -452,37 +468,69 @@ fn extract_files(options: &clap::ArgMatches) -> Files {
452468
///
453469
/// A Sort variant representing the sorting method to use.
454470
fn extract_sort(options: &clap::ArgMatches) -> Sort {
455-
if let Some(field) = options.get_one::<String>(options::SORT) {
456-
match field.as_str() {
457-
"none" => Sort::None,
458-
"name" => Sort::Name,
459-
"time" => Sort::Time,
460-
"size" => Sort::Size,
461-
"version" => Sort::Version,
462-
"extension" => Sort::Extension,
463-
"width" => Sort::Width,
464-
// below should never happen as clap already restricts the values.
465-
_ => unreachable!("Invalid field for --sort"),
466-
}
467-
} else if options.get_flag(options::sort::TIME) {
468-
Sort::Time
469-
} else if options.get_flag(options::sort::SIZE) {
470-
Sort::Size
471-
} else if options.get_flag(options::sort::NONE) {
472-
Sort::None
473-
} else if options.get_flag(options::sort::VERSION) {
474-
Sort::Version
475-
} else if options.get_flag(options::sort::EXTENSION) {
476-
Sort::Extension
477-
} else if !options.get_flag(options::format::LONG)
478-
&& (options.get_flag(options::time::ACCESS)
479-
|| options.get_flag(options::time::CHANGE)
480-
|| options.get_one::<String>(options::TIME).is_some())
481-
{
482-
// If -l is not specified, -u/-c/--time controls sorting.
483-
Sort::Time
484-
} else {
485-
Sort::Name
471+
let get_last_index = |flag: &str| -> usize {
472+
if options.value_source(flag) == Some(clap::parser::ValueSource::CommandLine) {
473+
options.index_of(flag).unwrap_or(0)
474+
} else {
475+
0
476+
}
477+
};
478+
479+
let sort_index = options
480+
.get_one::<String>(options::SORT)
481+
.and_then(|_| options.indices_of(options::SORT))
482+
.map(|mut indices| indices.next_back().unwrap_or(0))
483+
.unwrap_or(0);
484+
let time_index = get_last_index(options::sort::TIME);
485+
let size_index = get_last_index(options::sort::SIZE);
486+
let none_index = get_last_index(options::sort::NONE);
487+
let version_index = get_last_index(options::sort::VERSION);
488+
let extension_index = get_last_index(options::sort::EXTENSION);
489+
let unsorted_all_index = get_last_index(options::files::UNSORTED_ALL);
490+
491+
let max_sort_index = sort_index
492+
.max(time_index)
493+
.max(size_index)
494+
.max(none_index)
495+
.max(version_index)
496+
.max(extension_index)
497+
.max(unsorted_all_index);
498+
499+
match max_sort_index {
500+
0 => {
501+
// No sort flags specified, use default behavior
502+
if !options.get_flag(options::format::LONG)
503+
&& (options.get_flag(options::time::ACCESS)
504+
|| options.get_flag(options::time::CHANGE)
505+
|| options.get_one::<String>(options::TIME).is_some())
506+
{
507+
Sort::Time
508+
} else {
509+
Sort::Name
510+
}
511+
}
512+
idx if idx == unsorted_all_index || idx == none_index => Sort::None,
513+
idx if idx == sort_index => {
514+
if let Some(field) = options.get_one::<String>(options::SORT) {
515+
match field.as_str() {
516+
"none" => Sort::None,
517+
"name" => Sort::Name,
518+
"time" => Sort::Time,
519+
"size" => Sort::Size,
520+
"version" => Sort::Version,
521+
"extension" => Sort::Extension,
522+
"width" => Sort::Width,
523+
_ => unreachable!("Invalid field for --sort"),
524+
}
525+
} else {
526+
Sort::Name
527+
}
528+
}
529+
idx if idx == time_index => Sort::Time,
530+
idx if idx == size_index => Sort::Size,
531+
idx if idx == version_index => Sort::Version,
532+
idx if idx == extension_index => Sort::Extension,
533+
_ => Sort::Name,
486534
}
487535
}
488536

@@ -540,13 +588,40 @@ fn extract_color(options: &clap::ArgMatches) -> bool {
540588
return false;
541589
}
542590

543-
match options.get_one::<String>(options::COLOR) {
591+
let get_last_index = |flag: &str| -> usize {
592+
if options.value_source(flag) == Some(clap::parser::ValueSource::CommandLine) {
593+
options.index_of(flag).unwrap_or(0)
594+
} else {
595+
0
596+
}
597+
};
598+
599+
let color_index = options
600+
.get_one::<String>(options::COLOR)
601+
.and_then(|_| options.indices_of(options::COLOR))
602+
.map(|mut indices| indices.next_back().unwrap_or(0))
603+
.unwrap_or(0);
604+
let unsorted_all_index = get_last_index(options::files::UNSORTED_ALL);
605+
606+
let color_enabled = match options.get_one::<String>(options::COLOR) {
544607
None => options.contains_id(options::COLOR),
545608
Some(val) => match val.as_str() {
546609
"" | "always" | "yes" | "force" => true,
547610
"auto" | "tty" | "if-tty" => stdout().is_terminal(),
548611
/* "never" | "no" | "none" | */ _ => false,
549612
},
613+
};
614+
615+
// If --color was explicitly specified, always honor it regardless of -f
616+
// Otherwise, if -f is present without explicit color, disable color
617+
if color_index > 0 {
618+
// Color was explicitly specified
619+
color_enabled
620+
} else if unsorted_all_index > 0 {
621+
// -f present without explicit color, disable implicit color
622+
false
623+
} else {
624+
color_enabled
550625
}
551626
}
552627

@@ -1575,6 +1650,12 @@ pub fn uu_app() -> Command {
15751650
.help(translate!("ls-help-almost-all"))
15761651
.action(ArgAction::SetTrue),
15771652
)
1653+
.arg(
1654+
Arg::new(options::files::UNSORTED_ALL)
1655+
.short('f')
1656+
.help(translate!("ls-help-unsorted-all"))
1657+
.action(ArgAction::SetTrue),
1658+
)
15781659
.arg(
15791660
Arg::new(options::DIRECTORY)
15801661
.short('d')

0 commit comments

Comments
 (0)