From 0c766c470f816092fd73cef61183d383e4c9a621 Mon Sep 17 00:00:00 2001 From: Leonid Emar-Kar Date: Sun, 30 Mar 2025 11:38:29 +0100 Subject: [PATCH 01/13] add -T option parsing --- src/uu/ls/src/ls.rs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 60c91e47828..cf40cd96f81 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -33,7 +33,7 @@ use clap::{ }; use glob::{MatchOptions, Pattern}; use lscolors::LsColors; -use term_grid::{Direction, Filling, Grid, GridOptions}; +use term_grid::{Direction, Filling, Grid, GridOptions, SPACES_IN_TAB}; use thiserror::Error; use uucore::error::USimpleError; use uucore::format::human::{SizeFormat, human_readable}; @@ -91,7 +91,7 @@ pub mod options { pub static LONG: &str = "long"; pub static COLUMNS: &str = "C"; pub static ACROSS: &str = "x"; - pub static TAB_SIZE: &str = "tabsize"; // silently ignored (see #3624) + pub static TAB_SIZE: &str = "tabsize"; pub static COMMAS: &str = "m"; pub static LONG_NO_OWNER: &str = "g"; pub static LONG_NO_GROUP: &str = "o"; @@ -385,6 +385,7 @@ pub struct Config { line_ending: LineEnding, dired: bool, hyperlink: bool, + tab_size: usize, } // Fields that can be removed or added to the long format @@ -1086,6 +1087,15 @@ impl Config { Dereference::DirArgs }; + let tab_size = match needs_color { + false => options + .get_one::(options::format::TAB_SIZE) + .and_then(|size| size.parse::().ok()) + .or_else(|| std::env::var("TABSIZE").ok().and_then(|s| s.parse().ok())), + _ => Some(0), + } + .unwrap_or(SPACES_IN_TAB); + Ok(Self { format, files, @@ -1123,6 +1133,7 @@ impl Config { line_ending: LineEnding::from_zero_flag(options.get_flag(options::ZERO)), dired, hyperlink, + tab_size, }) } } @@ -1239,13 +1250,12 @@ pub fn uu_app() -> Command { .action(ArgAction::SetTrue), ) .arg( - // silently ignored (see #3624) Arg::new(options::format::TAB_SIZE) .short('T') .long(options::format::TAB_SIZE) .env("TABSIZE") .value_name("COLS") - .help("Assume tab stops at each COLS instead of 8 (unimplemented)"), + .help("Assume tab stops at each COLS instead of 8"), ) .arg( Arg::new(options::format::COMMAS) From 4a233a778853b092fa31be0394d5907251640818 Mon Sep 17 00:00:00 2001 From: Leonid Emar-Kar Date: Sun, 30 Mar 2025 11:39:58 +0100 Subject: [PATCH 02/13] add usage of tab_size in display_grid --- src/uu/ls/src/ls.rs | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index cf40cd96f81..db7059dd18b 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2564,10 +2564,24 @@ fn display_items( match config.format { Format::Columns => { - display_grid(names, config.width, Direction::TopToBottom, out, quoted)?; + display_grid( + names, + config.width, + Direction::TopToBottom, + out, + quoted, + config.tab_size, + )?; } Format::Across => { - display_grid(names, config.width, Direction::LeftToRight, out, quoted)?; + display_grid( + names, + config.width, + Direction::LeftToRight, + out, + quoted, + config.tab_size, + )?; } Format::Commas => { let mut current_col = 0; @@ -2635,6 +2649,7 @@ fn display_grid( direction: Direction, out: &mut BufWriter, quoted: bool, + tab_size: usize, ) -> UResult<()> { if width == 0 { // If the width is 0 we print one single line @@ -2684,14 +2699,9 @@ fn display_grid( .map(|s| s.to_string_lossy().into_owned()) .collect(); - // Determine whether to use tabs for separation based on whether any entry ends with '/'. - // If any entry ends with '/', it indicates that the -F flag is likely used to classify directories. - let use_tabs = names.iter().any(|name| name.ends_with('/')); - - let filling = if use_tabs { - Filling::Text("\t".to_string()) - } else { - Filling::Spaces(2) + let filling = match tab_size { + 0 => Filling::Spaces(2), + _ => Filling::Tabs(tab_size), }; let grid = Grid::new( From 1c19599518ed5b267aa4e532cbbdb224fd5356fe Mon Sep 17 00:00:00 2001 From: Leonid Emar-Kar Date: Sun, 30 Mar 2025 11:40:43 +0100 Subject: [PATCH 03/13] fix test_ls_columns with \t since this behavior is on by default --- tests/by-util/test_ls.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 4123809fcb2..c6352397160 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -837,7 +837,7 @@ fn test_ls_columns() { for option in COLUMN_ARGS { let result = scene.ucmd().arg(option).succeeds(); - result.stdout_only("test-columns-1 test-columns-2 test-columns-3 test-columns-4\n"); + result.stdout_only("test-columns-1\ttest-columns-2\ttest-columns-3\ttest-columns-4\n"); } for option in COLUMN_ARGS { @@ -846,7 +846,7 @@ fn test_ls_columns() { .arg("-w=40") .arg(option) .succeeds() - .stdout_only("test-columns-1 test-columns-3\ntest-columns-2 test-columns-4\n"); + .stdout_only("test-columns-1\ttest-columns-3\ntest-columns-2\ttest-columns-4\n"); } // On windows we are always able to get the terminal size, so we can't simulate falling back to the @@ -859,7 +859,7 @@ fn test_ls_columns() { .env("COLUMNS", "40") .arg(option) .succeeds() - .stdout_only("test-columns-1 test-columns-3\ntest-columns-2 test-columns-4\n"); + .stdout_only("test-columns-1\ttest-columns-3\ntest-columns-2\ttest-columns-4\n"); } scene @@ -867,7 +867,7 @@ fn test_ls_columns() { .env("COLUMNS", "garbage") .arg("-C") .succeeds() - .stdout_is("test-columns-1 test-columns-2 test-columns-3 test-columns-4\n") + .stdout_is("test-columns-1\ttest-columns-2\ttest-columns-3\ttest-columns-4\n") .stderr_is("ls: ignoring invalid width in environment variable COLUMNS: 'garbage'\n"); } scene From 2a202e77aaacd33d887f58b2ef57f19d7e5635d1 Mon Sep 17 00:00:00 2001 From: Leonid Emar-Kar Date: Sun, 30 Mar 2025 11:41:29 +0100 Subject: [PATCH 04/13] update test_tabsize_formatting --- tests/by-util/test_ls.rs | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index c6352397160..5eb0579938d 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -4366,28 +4366,36 @@ fn test_tabsize_option() { scene.ucmd().arg("-T").fails(); } -#[ignore = "issue #3624"] #[test] fn test_tabsize_formatting() { - let (at, mut ucmd) = at_and_ucmd!(); + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; at.touch("aaaaaaaa"); at.touch("bbbb"); at.touch("cccc"); at.touch("dddddddd"); - ucmd.args(&["-T", "4"]) + // Need additional options to simulate columns output. + + scene + .ucmd() + .args(&["-x", "-w20", "-T4"]) .succeeds() - .stdout_is("aaaaaaaa bbbb\ncccc\t dddddddd"); + .stdout_is("aaaaaaaa bbbb\ncccc\t dddddddd\n"); - ucmd.args(&["-T", "2"]) + scene + .ucmd() + .args(&["-x", "-w20", "-T2"]) .succeeds() - .stdout_is("aaaaaaaa bbbb\ncccc\t\t dddddddd"); + .stdout_is("aaaaaaaa\tbbbb\ncccc\t\t\tdddddddd\n"); // use spaces - ucmd.args(&["-T", "0"]) + scene + .ucmd() + .args(&["-x", "-w20", "-T0"]) .succeeds() - .stdout_is("aaaaaaaa bbbb\ncccc dddddddd"); + .stdout_is("aaaaaaaa bbbb\ncccc dddddddd\n"); } #[cfg(any( From 35eb82bd625a7a1793258d56e39a05a67d8573af Mon Sep 17 00:00:00 2001 From: Leonid Emar-Kar Date: Sun, 30 Mar 2025 15:14:34 +0100 Subject: [PATCH 05/13] use grid DEFAULT_SEPARATOR_SIZE --- src/uu/ls/src/ls.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index db7059dd18b..d4ba624afd1 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -33,7 +33,7 @@ use clap::{ }; use glob::{MatchOptions, Pattern}; use lscolors::LsColors; -use term_grid::{Direction, Filling, Grid, GridOptions, SPACES_IN_TAB}; +use term_grid::{DEFAULT_SEPARATOR_SIZE, Direction, Filling, Grid, GridOptions, SPACES_IN_TAB}; use thiserror::Error; use uucore::error::USimpleError; use uucore::format::human::{SizeFormat, human_readable}; @@ -2700,7 +2700,7 @@ fn display_grid( .collect(); let filling = match tab_size { - 0 => Filling::Spaces(2), + 0 => Filling::Spaces(DEFAULT_SEPARATOR_SIZE), _ => Filling::Tabs(tab_size), }; From 48ca8ba4d507127faa5563af29a7f920228f172d Mon Sep 17 00:00:00 2001 From: Leonid Emar-Kar Date: Tue, 8 Apr 2025 23:40:26 +0100 Subject: [PATCH 06/13] update Tabs --- src/uu/ls/src/ls.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index d4ba624afd1..ca5f23f1ffc 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -3,7 +3,7 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -// spell-checker:ignore (ToDO) somegroup nlink tabsize dired subdired dtype colorterm stringly +// spell-checker:ignore (ToDO) somegroup nlink dired subdired dtype colorterm stringly #[cfg(windows)] use std::os::windows::fs::MetadataExt; @@ -2701,7 +2701,10 @@ fn display_grid( let filling = match tab_size { 0 => Filling::Spaces(DEFAULT_SEPARATOR_SIZE), - _ => Filling::Tabs(tab_size), + _ => Filling::Tabs { + spaces: DEFAULT_SEPARATOR_SIZE, + tab_size, + }, }; let grid = Grid::new( From e6e76e233388d46f6ce951c546208984d9e43bf8 Mon Sep 17 00:00:00 2001 From: Leonid Emar-Kar Date: Tue, 8 Apr 2025 23:40:36 +0100 Subject: [PATCH 07/13] fix test with column width --- tests/by-util/test_ls.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 5eb0579938d..ca36b4a96a7 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -2,7 +2,7 @@ // // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -// spell-checker:ignore (words) READMECAREFULLY birthtime doesntexist oneline somebackup lrwx somefile somegroup somehiddenbackup somehiddenfile tabsize aaaaaaaa bbbb cccc dddddddd ncccc neee naaaaa nbcdef nfffff dired subdired tmpfs mdir COLORTERM mexe bcdef mfoo +// spell-checker:ignore (words) READMECAREFULLY birthtime doesntexist oneline somebackup lrwx somefile somegroup somehiddenbackup somehiddenfile aaaaaaaa bbbb cccc dddddddd ncccc neee naaaaa nbcdef nfffff dired subdired tmpfs mdir COLORTERM mexe bcdef mfoo // spell-checker:ignore (words) fakeroot setcap #![allow( clippy::similar_names, @@ -4380,20 +4380,20 @@ fn test_tabsize_formatting() { scene .ucmd() - .args(&["-x", "-w20", "-T4"]) + .args(&["-x", "-w18", "-T4"]) .succeeds() .stdout_is("aaaaaaaa bbbb\ncccc\t dddddddd\n"); scene .ucmd() - .args(&["-x", "-w20", "-T2"]) + .args(&["-x", "-w18", "-T2"]) .succeeds() .stdout_is("aaaaaaaa\tbbbb\ncccc\t\t\tdddddddd\n"); // use spaces scene .ucmd() - .args(&["-x", "-w20", "-T0"]) + .args(&["-x", "-w18", "-T0"]) .succeeds() .stdout_is("aaaaaaaa bbbb\ncccc dddddddd\n"); } From 8b9af956c91d87dcf904ec110e21535493d8bb59 Mon Sep 17 00:00:00 2001 From: Leonid Emar-Kar Date: Thu, 10 Apr 2025 17:34:00 +0100 Subject: [PATCH 08/13] fix cspell --- src/uu/ls/src/ls.rs | 2 +- tests/by-util/test_ls.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index ca5f23f1ffc..f5c0ee490c6 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -3,7 +3,7 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -// spell-checker:ignore (ToDO) somegroup nlink dired subdired dtype colorterm stringly +// spell-checker:ignore (ToDO) somegroup nlink tabsize dired subdired dtype colorterm stringly #[cfg(windows)] use std::os::windows::fs::MetadataExt; diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index ca36b4a96a7..fc5b128c3e0 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -2,7 +2,7 @@ // // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -// spell-checker:ignore (words) READMECAREFULLY birthtime doesntexist oneline somebackup lrwx somefile somegroup somehiddenbackup somehiddenfile aaaaaaaa bbbb cccc dddddddd ncccc neee naaaaa nbcdef nfffff dired subdired tmpfs mdir COLORTERM mexe bcdef mfoo +// spell-checker:ignore (words) READMECAREFULLY birthtime doesntexist oneline somebackup lrwx somefile somegroup somehiddenbackup somehiddenfile tabsize aaaaaaaa bbbb cccc dddddddd ncccc neee naaaaa nbcdef nfffff dired subdired tmpfs mdir COLORTERM mexe bcdef mfoo // spell-checker:ignore (words) fakeroot setcap #![allow( clippy::similar_names, From c222dc4c2b20bed325559940d541fff6c5f8f2c9 Mon Sep 17 00:00:00 2001 From: Leonid Emar-Kar Date: Thu, 10 Apr 2025 17:36:16 +0100 Subject: [PATCH 09/13] fix linter warning on match bool --- src/uu/ls/src/ls.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index f5c0ee490c6..00607487d1f 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1087,12 +1087,13 @@ impl Config { Dereference::DirArgs }; - let tab_size = match needs_color { - false => options + let tab_size = if !needs_color { + options .get_one::(options::format::TAB_SIZE) .and_then(|size| size.parse::().ok()) - .or_else(|| std::env::var("TABSIZE").ok().and_then(|s| s.parse().ok())), - _ => Some(0), + .or_else(|| std::env::var("TABSIZE").ok().and_then(|s| s.parse().ok())) + } else { + Some(0) } .unwrap_or(SPACES_IN_TAB); From b4fe31956d4757b5aef1a70ecfa68f2804306d09 Mon Sep 17 00:00:00 2001 From: Leonid Emar-Kar Date: Thu, 10 Apr 2025 21:05:51 +0100 Subject: [PATCH 10/13] add comment for 0 tab_size --- src/uu/ls/src/ls.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 00607487d1f..353df263043 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2700,6 +2700,7 @@ fn display_grid( .map(|s| s.to_string_lossy().into_owned()) .collect(); + // Since tab_size=0 means no \t, use Spaces separator for optimization. let filling = match tab_size { 0 => Filling::Spaces(DEFAULT_SEPARATOR_SIZE), _ => Filling::Tabs { From 548c4ea17d6608166aea1b65b440b82965b48d2a Mon Sep 17 00:00:00 2001 From: Leonid Emar-Kar Date: Thu, 10 Apr 2025 21:06:02 +0100 Subject: [PATCH 11/13] update tabsize test with -C --- tests/by-util/test_ls.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index fc5b128c3e0..9713846a99b 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -4380,22 +4380,22 @@ fn test_tabsize_formatting() { scene .ucmd() - .args(&["-x", "-w18", "-T4"]) + .args(&["-C", "-w18", "-T4"]) .succeeds() - .stdout_is("aaaaaaaa bbbb\ncccc\t dddddddd\n"); + .stdout_is("aaaaaaaa cccc\nbbbb\t dddddddd\n"); scene .ucmd() - .args(&["-x", "-w18", "-T2"]) + .args(&["-C", "-w18", "-T2"]) .succeeds() - .stdout_is("aaaaaaaa\tbbbb\ncccc\t\t\tdddddddd\n"); + .stdout_is("aaaaaaaa\tcccc\nbbbb\t\t\tdddddddd\n"); // use spaces scene .ucmd() - .args(&["-x", "-w18", "-T0"]) + .args(&["-C", "-w18", "-T0"]) .succeeds() - .stdout_is("aaaaaaaa bbbb\ncccc dddddddd\n"); + .stdout_is("aaaaaaaa cccc\nbbbb dddddddd\n"); } #[cfg(any( From 17ab4cd3284669c83a091352463fcf1a7185adcc Mon Sep 17 00:00:00 2001 From: Leonid Emar-Kar Date: Fri, 11 Apr 2025 10:35:25 +0100 Subject: [PATCH 12/13] update one of the tabs tests to use -x --- tests/by-util/test_ls.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 9713846a99b..14783cba5c2 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -4380,9 +4380,9 @@ fn test_tabsize_formatting() { scene .ucmd() - .args(&["-C", "-w18", "-T4"]) + .args(&["-x", "-w18", "-T4"]) .succeeds() - .stdout_is("aaaaaaaa cccc\nbbbb\t dddddddd\n"); + .stdout_is("aaaaaaaa bbbb\ncccc\t dddddddd\n"); scene .ucmd() From 471144ce2c56026d1b889faa248c837ebba84481 Mon Sep 17 00:00:00 2001 From: Leonid Emar-Kar Date: Mon, 14 Apr 2025 14:35:14 +0100 Subject: [PATCH 13/13] remove comment and split tests for both x/C args --- tests/by-util/test_ls.rs | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 14783cba5c2..c8cf812a3cc 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -4376,20 +4376,36 @@ fn test_tabsize_formatting() { at.touch("cccc"); at.touch("dddddddd"); - // Need additional options to simulate columns output. - scene .ucmd() .args(&["-x", "-w18", "-T4"]) .succeeds() .stdout_is("aaaaaaaa bbbb\ncccc\t dddddddd\n"); + scene + .ucmd() + .args(&["-C", "-w18", "-T4"]) + .succeeds() + .stdout_is("aaaaaaaa cccc\nbbbb\t dddddddd\n"); + + scene + .ucmd() + .args(&["-x", "-w18", "-T2"]) + .succeeds() + .stdout_is("aaaaaaaa\tbbbb\ncccc\t\t\tdddddddd\n"); + scene .ucmd() .args(&["-C", "-w18", "-T2"]) .succeeds() .stdout_is("aaaaaaaa\tcccc\nbbbb\t\t\tdddddddd\n"); + scene + .ucmd() + .args(&["-x", "-w18", "-T0"]) + .succeeds() + .stdout_is("aaaaaaaa bbbb\ncccc dddddddd\n"); + // use spaces scene .ucmd()