From dfd208880adc50def669924cc81ad77961f6d114 Mon Sep 17 00:00:00 2001 From: Dawit Worku Date: Wed, 27 May 2026 17:42:46 +0300 Subject: [PATCH] fix(display): use uniform column widths in grid layout GNU ls uses uniform column widths (all columns = max cell width) when laying out filenames in grid mode. lsd was using term_grid's fit_into_width, which assigns each column its own variable width based on the widest cell in that column. This could produce misleading output where a short filename appeared to be the last entry in a row while a later entry was hidden in an unexpectedly wide column. Track the maximum cell width while building the grid, then compute the largest number of columns that fit in the terminal width assuming every column is that width. Pass that count to fit_into_columns instead of calling fit_into_width. Fixes #594 --- src/display.rs | 84 +++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 76 insertions(+), 8 deletions(-) diff --git a/src/display.rs b/src/display.rs index 842ffb248..532de66b4 100644 --- a/src/display.rs +++ b/src/display.rs @@ -92,6 +92,7 @@ fn inner_display_grid( ) -> String { let mut output = String::new(); let mut cells = Vec::new(); + let mut max_cell_width = 0usize; let padding_rules = get_padding_rules(metas, flags); let mut grid = match flags.layout { @@ -134,8 +135,10 @@ fn inner_display_grid( ); for block in blocks { + let w = get_visible_width(&block, flags.hyperlink == HyperlinkOption::Always); + max_cell_width = max_cell_width.max(w); cells.push(Cell { - width: get_visible_width(&block, flags.hyperlink == HyperlinkOption::Always), + width: w, contents: block, }); } @@ -152,14 +155,17 @@ fn inner_display_grid( if flags.layout == Layout::Grid { if let Some(tw) = term_width { - if let Some(gridded_output) = grid.fit_into_width(tw) { - output += &gridded_output.to_string(); + // Use uniform column widths (like GNU ls) so that no entry appears + // to be the last in a row while a later entry is hidden off-screen. + // Variable-width columns (fit_into_width) can place a short entry + // just before a wide column whose left edge is beyond the terminal. + let filling = 2; // matches Filling::Spaces(2) above + let num_cols = if max_cell_width == 0 { + 1 } else { - //does not fit into grid, usually because (some) filename(s) - //are longer or almost as long as term_width - //print line by line instead! - output += &grid.fit_into_columns(1).to_string(); - } + ((tw + filling) / (max_cell_width + filling)).max(1) + }; + output += &grid.fit_into_columns(num_cols).to_string(); } else { output += &grid.fit_into_columns(1).to_string(); } @@ -833,6 +839,68 @@ mod tests { assert!(output.ends_with("└── two\n")); } + #[test] + fn test_grid_uniform_column_widths() { + // Regression test for https://github.com/lsd-rs/lsd/issues/594: + // When filenames have mixed lengths, grid must use uniform column widths + // (like GNU ls) so that short entries in a narrow column never appear to + // be the last entry on a row while a later entry sits in a hidden wide column. + let term_width = 91usize; + let filling = 2usize; + + let dir = assert_fs::TempDir::new().unwrap(); + // Two long names (~60 chars) matching the issue's reproduce case. + dir.child("file_00_abcdefghijklmnopqrstuvwxyz_abcdefghiklmnopqrstuvwxyz") + .touch() + .unwrap(); + for i in 1..=14usize { + dir.child(format!("file_{i:02}")).touch().unwrap(); + } + dir.child("file_15_abcdefghijklmnopqrstuvwxyz_abcdefghiklmnopqrstuvwxyz") + .touch() + .unwrap(); + dir.child("file_16").touch().unwrap(); + + let argv = ["lsd"]; + let cli = Cli::try_parse_from(argv).unwrap(); + let flags = Flags::configure_from(&cli, &Config::with_none()).unwrap(); + let metas = Meta::from_path(Path::new(dir.path()), false, PermissionFlag::Rwx) + .unwrap() + .recurse_into(1, &flags, None) + .unwrap() + .0 + .unwrap(); + + let colors = Colors::new(color::ThemeOption::NoColor); + let icons = Icons::new(false, IconOption::Never, FlagTheme::Fancy, " ".to_string()); + let owner_cache = OwnerCache::default(); + let output = inner_display_grid( + &DisplayOption::FileName, + &metas, + &owner_cache, + &flags, + &colors, + &icons, + &GitTheme::new(), + 0, + Some(term_width), + ); + + // Every rendered line must fit within the terminal width. + for line in output.lines() { + let visible: usize = line + .chars() + .map(|c| unicode_width::UnicodeWidthChar::width(c).unwrap_or(0)) + .sum(); + assert!( + visible <= term_width + filling, // last column needs no trailing fill + "line exceeds terminal width ({visible} > {term_width}): {line:?}", + ); + } + + dir.close().unwrap(); + } + #[test] fn test_grid_all_block_headers() { let argv = [