diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 60c91e478..353df2630 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::{DEFAULT_SEPARATOR_SIZE, 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,16 @@ impl Config { Dereference::DirArgs }; + 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())) + } else { + Some(0) + } + .unwrap_or(SPACES_IN_TAB); + Ok(Self { format, files, @@ -1123,6 +1134,7 @@ impl Config { line_ending: LineEnding::from_zero_flag(options.get_flag(options::ZERO)), dired, hyperlink, + tab_size, }) } } @@ -1239,13 +1251,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) @@ -2554,10 +2565,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; @@ -2625,6 +2650,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 @@ -2674,14 +2700,13 @@ 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) + // 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 { + spaces: DEFAULT_SEPARATOR_SIZE, + tab_size, + }, }; let grid = Grid::new( diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 4123809fc..c8cf812a3 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 @@ -4366,28 +4366,52 @@ 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"]) + scene + .ucmd() + .args(&["-x", "-w18", "-T4"]) .succeeds() - .stdout_is("aaaaaaaa bbbb\ncccc\t dddddddd"); + .stdout_is("aaaaaaaa bbbb\ncccc\t dddddddd\n"); - ucmd.args(&["-T", "2"]) + scene + .ucmd() + .args(&["-C", "-w18", "-T4"]) .succeeds() - .stdout_is("aaaaaaaa bbbb\ncccc\t\t dddddddd"); + .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 - ucmd.args(&["-T", "0"]) + scene + .ucmd() + .args(&["-C", "-w18", "-T0"]) .succeeds() - .stdout_is("aaaaaaaa bbbb\ncccc dddddddd"); + .stdout_is("aaaaaaaa cccc\nbbbb dddddddd\n"); } #[cfg(any(