diff --git a/src/uu/ls/src/dired.rs b/src/uu/ls/src/dired.rs index 69122ccd08b..4d96c4d9c27 100644 --- a/src/uu/ls/src/dired.rs +++ b/src/uu/ls/src/dired.rs @@ -51,6 +51,7 @@ pub struct DiredOutput { pub dired_positions: Vec, pub subdired_positions: Vec, pub padding: usize, + pub line_offset: usize, } impl fmt::Display for BytePosition { @@ -62,21 +63,13 @@ impl fmt::Display for BytePosition { // When --dired is used, all lines starts with 2 spaces static DIRED_TRAILING_OFFSET: usize = 2; -fn get_offset_from_previous_line(dired_positions: &[BytePosition]) -> usize { - if let Some(last_position) = dired_positions.last() { - last_position.end + 1 - } else { - 0 - } -} - /// Calculates the byte positions for DIRED pub fn calculate_dired( - dired_positions: &[BytePosition], + dired: &DiredOutput, output_display_len: usize, dfn_len: usize, ) -> (usize, usize) { - let offset_from_previous_line = get_offset_from_previous_line(dired_positions); + let offset_from_previous_line = dired.line_offset; let start = output_display_len + offset_from_previous_line; let end = start + dfn_len; @@ -89,16 +82,8 @@ pub fn indent(out: &mut BufWriter) -> UResult<()> { } pub fn calculate_subdired(dired: &mut DiredOutput, path_len: usize) { - let offset_from_previous_line = get_offset_from_previous_line(&dired.dired_positions); - - let additional_offset = if dired.subdired_positions.is_empty() { - 0 - } else { - // if we have several directories: \n\n - 2 - }; - - let start = offset_from_previous_line + DIRED_TRAILING_OFFSET + additional_offset; + let offset_from_previous_line = dired.line_offset + dired.padding; + let start = offset_from_previous_line + DIRED_TRAILING_OFFSET; let end = start + path_len; dired.subdired_positions.push(BytePosition { start, end }); } @@ -113,7 +98,9 @@ pub fn print_dired_output( if !dired.dired_positions.is_empty() { print_positions("//DIRED//", &dired.dired_positions); } - if config.recursive { + // SUBDIRED is needed whenever directory headings are printed (multiple args or -R), + // so don't gate it on config.recursive. + if !dired.subdired_positions.is_empty() { print_positions("//SUBDIRED//", &dired.subdired_positions); } println!("//DIRED-OPTIONS// --quoting-style={}", config.quoting_style); @@ -131,10 +118,9 @@ fn print_positions(prefix: &str, positions: &Vec) { pub fn add_total(dired: &mut DiredOutput, total_len: usize) { if dired.padding == 0 { - let offset_from_previous_line = get_offset_from_previous_line(&dired.dired_positions); // when dealing with " total: xx", it isn't part of the //DIRED// // so, we just keep the size line to add it to the position of the next file - dired.padding = total_len + offset_from_previous_line + DIRED_TRAILING_OFFSET; + dired.padding = total_len + DIRED_TRAILING_OFFSET; } else { // += because if we are in -R, we have " dir:\n total X". So, we need to take the // previous padding too. @@ -145,8 +131,8 @@ pub fn add_total(dired: &mut DiredOutput, total_len: usize) { // when using -R, we have the dirname. we need to add it to the padding pub fn add_dir_name(dired: &mut DiredOutput, dir_len: usize) { - // 1 for the ":" in " dirname:" - dired.padding += dir_len + DIRED_TRAILING_OFFSET + 1; + // " dirname:\n" + dired.padding += dir_len + DIRED_TRAILING_OFFSET + 2; } /// Calculates byte positions and updates the dired structure. @@ -154,27 +140,23 @@ pub fn calculate_and_update_positions( dired: &mut DiredOutput, output_display_len: usize, dfn_len: usize, + line_len: usize, ) { - let offset = dired - .dired_positions - .last() - .map_or(DIRED_TRAILING_OFFSET, |last_position| { - last_position.start + DIRED_TRAILING_OFFSET - }); - let start = output_display_len + offset + DIRED_TRAILING_OFFSET; - let end = start + dfn_len; - update_positions(dired, start, end); + let (start, end) = calculate_dired(dired, output_display_len, dfn_len); + update_positions(dired, start, end, line_len); } /// Updates the dired positions based on the given start and end positions. /// update when it is the first element in the list (to manage "total X") /// insert when it isn't the about total -pub fn update_positions(dired: &mut DiredOutput, start: usize, end: usize) { +pub fn update_positions(dired: &mut DiredOutput, start: usize, end: usize, line_len: usize) { // padding can be 0 but as it doesn't matter + let padding = dired.padding; dired.dired_positions.push(BytePosition { - start: start + dired.padding, - end: end + dired.padding, + start: start + padding, + end: end + padding, }); + dired.line_offset = dired.line_offset + padding + line_len; // Remove the previous padding dired.padding = 0; } @@ -194,22 +176,18 @@ mod tests { fn test_calculate_dired() { let output_display = "sample_output".to_string(); let dfn = "sample_file".to_string(); - let dired_positions = vec![BytePosition { start: 5, end: 10 }]; - let (start, end) = calculate_dired(&dired_positions, output_display.len(), dfn.len()); + let dired = DiredOutput { + dired_positions: vec![BytePosition { start: 5, end: 10 }], + subdired_positions: vec![], + padding: 0, + line_offset: 11, + }; + let (start, end) = calculate_dired(&dired, output_display.len(), dfn.len()); assert_eq!(start, 24); assert_eq!(end, 35); } - #[test] - fn test_get_offset_from_previous_line() { - let positions = vec![ - BytePosition { start: 0, end: 3 }, - BytePosition { start: 4, end: 7 }, - BytePosition { start: 8, end: 11 }, - ]; - assert_eq!(get_offset_from_previous_line(&positions), 12); - } #[test] fn test_calculate_subdired() { let mut dired = DiredOutput { @@ -220,6 +198,7 @@ mod tests { ], subdired_positions: vec![], padding: 0, + line_offset: 12, }; let path_len = 5; calculate_subdired(&mut dired, path_len); @@ -239,6 +218,7 @@ mod tests { ], subdired_positions: vec![], padding: 0, + line_offset: 0, }; let dir_len = 5; add_dir_name(&mut dired, dir_len); @@ -251,8 +231,9 @@ mod tests { BytePosition { start: 8, end: 11 }, ], subdired_positions: vec![], - // 8 = 1 for the \n + 5 for dir_len + 2 for " " + 1 for : - padding: 8 + // 9 = 5 for dir_len + 2 for " " + 1 for : + 1 for \n + padding: 9, + line_offset: 0, } ); } @@ -267,12 +248,13 @@ mod tests { ], subdired_positions: vec![], padding: 0, + line_offset: 12, }; // if we have "total: 2" let total_len = 8; add_total(&mut dired, total_len); - // 22 = 8 (len) + 2 (padding) + 11 (previous position) + 1 (\n) - assert_eq!(dired.padding, 22); + // 10 = 8 (len) + 2 (" ") + assert_eq!(dired.padding, 10); } #[test] @@ -290,15 +272,16 @@ mod tests { ], subdired_positions: vec![], padding: 0, + line_offset: 12, }; let dir_len = 5; add_dir_name(&mut dired, dir_len); - // 8 = 2 (" ") + 1 (\n) + 5 + 1 (: of dirname) - assert_eq!(dired.padding, 8); + // 9 = 2 (" ") + 1 (\n) + 5 + 1 (: of dirname) + assert_eq!(dired.padding, 9); let total_len = 8; add_total(&mut dired, total_len); - assert_eq!(dired.padding, 18); + assert_eq!(dired.padding, 19); } #[test] @@ -307,16 +290,17 @@ mod tests { dired_positions: vec![BytePosition { start: 5, end: 10 }], subdired_positions: vec![], padding: 10, + line_offset: 0, }; // Test with adjust = true - update_positions(&mut dired, 15, 20); + update_positions(&mut dired, 15, 20, 1); let last_position = dired.dired_positions.last().unwrap(); assert_eq!(last_position.start, 25); // 15 + 10 (end of the previous position) assert_eq!(last_position.end, 30); // 20 + 10 (end of the previous position) // Test with adjust = false - update_positions(&mut dired, 30, 35); + update_positions(&mut dired, 30, 35, 1); let last_position = dired.dired_positions.last().unwrap(); assert_eq!(last_position.start, 30); assert_eq!(last_position.end, 35); @@ -332,10 +316,11 @@ mod tests { ], subdired_positions: vec![], padding: 5, + line_offset: 12, }; let output_display_len = 15; let dfn_len = 5; - calculate_and_update_positions(&mut dired, output_display_len, dfn_len); + calculate_and_update_positions(&mut dired, output_display_len, dfn_len, 20); assert_eq!( dired.dired_positions, vec![ diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index f2f9cc30137..cbb95055ae9 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -410,6 +410,11 @@ struct PaddingCollection { block_size: usize, } +struct DisplayItemName { + displayed: OsString, + dired_name_len: usize, +} + /// Extracts the format to display the information based on the options provided. /// /// # Returns @@ -2281,24 +2286,24 @@ pub fn list(locs: Vec<&Path>, config: &Config) -> UResult<()> { // Print dir heading - name... 'total' comes after error display if initial_locs_len > 1 || config.recursive { - if pos.eq(&0usize) && files.is_empty() { - if config.dired { - dired::indent(&mut state.out)?; - } - show_dir_name(path_data, &mut state.out, config)?; + let needs_blank_line = !(pos.eq(&0usize) && files.is_empty()); + if needs_blank_line { writeln!(state.out)?; if config.dired { - // First directory displayed - let dir_len = path_data.display_name().len(); - // add the //SUBDIRED// coordinates - dired::calculate_subdired(&mut dired, dir_len); - // Add the padding for the dir name - dired::add_dir_name(&mut dired, dir_len); + dired.padding += 1; } - } else { - writeln!(state.out)?; - show_dir_name(path_data, &mut state.out, config)?; - writeln!(state.out)?; + } + if config.dired { + dired::indent(&mut state.out)?; + } + show_dir_name(path_data, &mut state.out, config)?; + writeln!(state.out)?; + if config.dired { + let dir_len = path_data.display_name().len(); + // add the //SUBDIRED// coordinates + dired::calculate_subdired(&mut dired, dir_len); + // Add the padding for the dir name + dired::add_dir_name(&mut dired, dir_len); } } let mut listed_ancestors = FxHashSet::default(); @@ -2525,8 +2530,8 @@ fn enter_directory( if config.dired { // We already injected the first dir // Continue with the others - // 2 = \n + \n - dired.padding = 2; + // blank line between directory sections + dired.padding += 1; dired::indent(&mut state.out)?; let dir_name_size = e.path().as_os_str().len(); dired::calculate_subdired(dired, dir_name_size); @@ -2749,7 +2754,7 @@ fn display_items( LazyCell::new(Box::new(|| 0)), ); - names_vec.push(cell); + names_vec.push(cell.displayed); } let mut names = names_vec.into_iter(); @@ -2913,6 +2918,21 @@ fn display_grid( Ok(()) } +fn calculate_line_len(output_len: usize, item_len: usize, line_ending: LineEnding) -> usize { + output_len + item_len + line_ending.to_string().len() +} + +fn update_dired_for_item( + dired: &mut DiredOutput, + output_display_len: usize, + displayed_len: usize, + dired_name_len: usize, + line_ending: LineEnding, +) { + let line_len = calculate_line_len(output_display_len, displayed_len, line_ending); + dired::calculate_and_update_positions(dired, output_display_len, dired_name_len, line_len); +} + /// This writes to the [`BufWriter`] `state.out` a single string of the output of `ls -l`. /// /// It writes the following keys, in order: @@ -3033,7 +3053,7 @@ fn display_item_long( display_date(md, config, state, &mut output_display)?; output_display.extend(b" "); - let item_name = display_item_name( + let item_display = display_item_name( item, config, None, @@ -3044,22 +3064,32 @@ fn display_item_long( })), ); - let displayed_item = if quoted && !os_str_starts_with(&item_name, b"'") { + let needs_space = quoted && !os_str_starts_with(&item_display.displayed, b"'"); + + if config.dired { + let mut dired_name_len = item_display.dired_name_len; + if needs_space { + dired_name_len += 1; + } + let displayed_len = item_display.displayed.len() + usize::from(needs_space); + update_dired_for_item( + dired, + output_display.len(), + displayed_len, + dired_name_len, + config.line_ending, + ); + } + + let item_name = item_display.displayed; + let displayed_item = if needs_space { let mut ret: OsString = " ".into(); - ret.push(item_name); + ret.push(&item_name); ret } else { item_name }; - if config.dired { - let (start, end) = dired::calculate_dired( - &dired.dired_positions, - output_display.len(), - displayed_item.len(), - ); - dired::update_positions(dired, start, end); - } write_os_str(&mut output_display, &displayed_item)?; output_display.extend(config.line_ending.to_string().as_bytes()); } else { @@ -3151,12 +3181,15 @@ fn display_item_long( output_display.extend(b" "); if config.dired { - dired::calculate_and_update_positions( + update_dired_for_item( dired, output_display.len(), - displayed_item.to_string_lossy().trim().len(), + displayed_item.displayed.len(), + displayed_item.dired_name_len, + config.line_ending, ); } + let displayed_item = displayed_item.displayed; write_os_str(&mut output_display, &displayed_item)?; output_display.extend(config.line_ending.to_string().as_bytes()); } @@ -3329,7 +3362,7 @@ fn display_item_name( more_info: Option, state: &mut ListState, current_column: LazyCell usize + '_>>, -) -> OsString { +) -> DisplayItemName { // This is our return value. We start by `&path.display_name` and modify it along the way. let mut name = escape_name_with_locale(path.display_name(), config); @@ -3380,6 +3413,8 @@ fn display_item_name( } } + let dired_name_len = if config.dired { name.len() } else { 0 }; + if config.format == Format::Long && path.file_type().is_some_and(|ft| ft.is_symlink()) && !path.must_dereference @@ -3477,7 +3512,10 @@ fn display_item_name( } } - name + DisplayItemName { + displayed: name, + dired_name_len, + } } fn create_hyperlink(name: &OsStr, path: &PathData) -> OsString { diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 11ce1c5b9c9..d5e12f2340c 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -5207,6 +5207,51 @@ fn test_ls_dired_recursive() { .stdout_contains("//DIRED-OPTIONS// --quoting-style"); } +#[test] +fn test_ls_dired_subdired_multiple_dirs_non_recursive() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + at.mkdir("dir1"); + at.mkdir("dir2"); + at.touch("dir1/a"); + at.touch("dir2/b"); + + let result = scene + .ucmd() + .arg("--dired") + .arg("dir1") + .arg("dir2") + .succeeds(); + + let output = result.stdout_str().to_string(); + let subdired_line = output + .lines() + .find(|&line| line.starts_with("//SUBDIRED//")) + .unwrap(); + let positions: Vec = subdired_line + .split_whitespace() + .skip(1) + .map(|s| s.parse().unwrap()) + .collect(); + + assert_eq!(positions.len() % 2, 0); // Ensure there's an even number of positions + + let dirnames: Vec = positions + .chunks(2) + .map(|chunk| { + let start_pos = chunk[0]; + let end_pos = chunk[1]; + String::from_utf8(output.as_bytes()[start_pos..end_pos].to_vec()) + .unwrap() + .trim() + .to_string() + }) + .collect(); + + assert_eq!(dirnames, vec!["dir1", "dir2"]); +} + #[test] fn test_ls_dired_outputs_parent_offset() { let scene = TestScenario::new(util_name!()); @@ -5336,6 +5381,53 @@ fn test_ls_dired_simple() { assert_eq!(filename, "a1"); } +#[test] +fn test_ls_dired_symlink_name_only() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + at.mkdir("d"); + at.touch("d/target"); + at.relative_symlink_file("target", "d/link"); + + let result = scene + .ucmd() + .arg("--dired") + .arg("-l") + .arg("--color=never") + .arg("d") + .succeeds(); + + let output = result.stdout_str().to_string(); + assert!(output.contains("link -> target")); + + let dired_line = output + .lines() + .find(|&line| line.starts_with("//DIRED//")) + .unwrap(); + let positions: Vec = dired_line + .split_whitespace() + .skip(1) + .map(|s| s.parse().unwrap()) + .collect(); + + assert_eq!(positions.len() % 2, 0); + + let filenames: Vec = positions + .chunks(2) + .map(|chunk| { + let start_pos = chunk[0]; + let end_pos = chunk[1]; + String::from_utf8(output.as_bytes()[start_pos..end_pos].to_vec()) + .unwrap() + .trim() + .to_string() + }) + .collect(); + + assert_eq!(filenames, vec!["link", "target"]); +} + #[test] fn test_ls_dired_complex() { let scene = TestScenario::new(util_name!()); diff --git a/util/build-gnu.sh b/util/build-gnu.sh index 9b1477f7bec..57480b28e2f 100755 --- a/util/build-gnu.sh +++ b/util/build-gnu.sh @@ -299,9 +299,6 @@ sed -i '1s/^/exit 0 # Skip test - uutils du uses safe traversal that prevents t awk 'BEGIN {count=0} /compare exp out2/ && count < 6 {sub(/compare exp out2/, "grep -q \"cannot be used with\" out2"); count++} 1' tests/df/df-output.sh > tests/df/df-output.sh.tmp && mv tests/df/df-output.sh.tmp tests/df/df-output.sh -# with ls --dired, in case of error, we have a slightly different error position -sed -i -e "s|44 45|48 49|" tests/ls/stat-failed.sh - # small difference in the error message sed -i -e "s/ls: invalid argument 'XX' for 'time style'/ls: invalid --time-style argument 'XX'/" \ -e "s/Valid arguments are:/Possible values are:/" \