From 65559d4d76fa0a1da21a893ed50e8327f27e8db9 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Fri, 25 Aug 2023 16:23:56 -0400 Subject: [PATCH 01/10] Support ls --dired --- src/uu/ls/src/dired.rs | 196 +++++++++++++++++++ src/uu/ls/src/ls.rs | 172 +++++++++------- src/uucore/src/lib/features/quoting_style.rs | 86 +++++++- tests/by-util/test_ls.rs | 125 +++++++++++- 4 files changed, 510 insertions(+), 69 deletions(-) create mode 100644 src/uu/ls/src/dired.rs diff --git a/src/uu/ls/src/dired.rs b/src/uu/ls/src/dired.rs new file mode 100644 index 00000000000..d3a8e88fab5 --- /dev/null +++ b/src/uu/ls/src/dired.rs @@ -0,0 +1,196 @@ +// This file is part of the uutils coreutils package. +// +// For the full copyright and license information, please view the LICENSE +// file that was distributed with this source code. +// spell-checker:ignore dired subdired dired + +use crate::Config; +use std::fmt; +use std::io::{BufWriter, Stdout, Write}; +use uucore::error::UResult; + +#[derive(Debug, Clone)] +pub struct BytePosition { + pub start: usize, + pub end: usize, + pub is_total: Option, +} + +/// Represents the output structure for DIRED, containing positions for both DIRED and SUBDIRED. +#[derive(Debug, Clone, Default)] +pub struct DiredOutput { + pub dired_positions: Vec, + pub subdired_positions: Vec, +} + +impl fmt::Display for BytePosition { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{} {}", self.start, self.end) + } +} + +// When --dired is used, all lines starts with 2 spaces +static DIRED_TRAILING_OFFSET: usize = 2; + +/// Calculates the byte positions for DIRED +pub fn calculate_dired_byte_positions( + output_display_len: usize, + dfn_len: usize, + dired_positions: &[BytePosition], +) -> (usize, usize) { + let offset_from_previous_line = if let Some(last_position) = dired_positions.last() { + last_position.end + 1 + } else { + 0 + }; + + let start = output_display_len + offset_from_previous_line + DIRED_TRAILING_OFFSET; + let end = start + dfn_len; + (start, end) +} + +pub fn indent(out: &mut BufWriter) -> UResult<()> { + write!(out, " ")?; + Ok(()) +} + +pub fn calculate_offset_and_push(dired: &mut DiredOutput, path_len: usize) { + let offset = if dired.subdired_positions.is_empty() { + DIRED_TRAILING_OFFSET + } else { + dired.subdired_positions[dired.subdired_positions.len() - 1].start + DIRED_TRAILING_OFFSET + }; + dired.subdired_positions.push(BytePosition { + start: offset, + end: path_len + offset, + is_total: None, + }); +} + +/// Prints the dired output based on the given configuration and dired structure. +pub fn print_dired_output( + config: &Config, + dired: &DiredOutput, + out: &mut BufWriter, +) -> UResult<()> { + out.flush()?; + if config.recursive { + print_positions("//SUBDIRED//", &dired.subdired_positions); + } else if dired.dired_positions.last().map_or(false, |last_position| { + !last_position.is_total.unwrap_or(false) + }) { + print_positions("//DIRED//", &dired.dired_positions); + } + println!("//DIRED-OPTIONS// --quoting-style={}", config.quoting_style); + Ok(()) +} + +/// Helper function to print positions with a given prefix. +fn print_positions(prefix: &str, positions: &Vec) { + print!("{}", prefix); + for c in positions { + print!(" {}", c); + } + println!(); +} + +pub fn add_total(total_len: usize, dired: &mut DiredOutput) { + dired.dired_positions.push(BytePosition { + start: 0, + // the 2 is from the trailing spaces + // the 1 is from the line ending (\n) + end: total_len + DIRED_TRAILING_OFFSET - 1, + is_total: Some(true), + }); +} + +/// Calculates byte positions and updates the dired structure. +pub fn calculate_and_update_positions( + output_display_len: usize, + dfn_len: usize, + dired: &mut DiredOutput, +) { + 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(start, end, dired, true); +} + +/// 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(start: usize, end: usize, dired: &mut DiredOutput, adjust: bool) { + if let Some(last_position) = dired.dired_positions.last() { + if last_position.is_total.unwrap_or(false) { + if let Some(last_position) = dired.dired_positions.last_mut() { + *last_position = BytePosition { + start: if adjust { + start + last_position.end + } else { + start + }, + end: if adjust { end + last_position.end } else { end }, + is_total: Some(false), + }; + } + } else { + dired.dired_positions.push(BytePosition { + start, + end, + is_total: None, + }); + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_calculate_dired_byte_positions() { + let output_display = "sample_output".to_string(); + let dfn = "sample_file".to_string(); + let dired_positions = vec![BytePosition { + start: 5, + end: 10, + is_total: None, + }]; + let (start, end) = + calculate_dired_byte_positions(output_display.len(), dfn.len(), &dired_positions); + + assert_eq!(start, 26); + assert_eq!(end, 37); + } + + #[test] + fn test_dired_update_positions() { + let mut dired = DiredOutput { + dired_positions: vec![BytePosition { + start: 5, + end: 10, + is_total: Some(true), + }], + subdired_positions: vec![], + }; + + // Test with adjust = true + update_positions(15, 20, &mut dired, true); + 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) + assert_eq!(last_position.is_total, Some(false)); + + // Test with adjust = false + update_positions(30, 35, &mut dired, false); + let last_position = dired.dired_positions.last().unwrap(); + assert_eq!(last_position.start, 30); + assert_eq!(last_position.end, 35); + assert_eq!(last_position.is_total, None); + } +} diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 8d559ed15f1..af5b8980141 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) cpio svgz webm somegroup nlink rmvb xspf tabsize dired +// spell-checker:ignore (ToDO) cpio svgz webm somegroup nlink rmvb xspf tabsize dired subdired dired use clap::{ builder::{NonEmptyStringValueParser, ValueParser}, @@ -53,6 +53,7 @@ use uucore::libc::{S_IXGRP, S_IXOTH, S_IXUSR}; use uucore::line_ending::LineEnding; use uucore::quoting_style::{escape_name, QuotingStyle}; use uucore::{ + crash, display::Quotable, error::{set_exit_code, UError, UResult}, format_usage, @@ -61,7 +62,8 @@ use uucore::{ version_cmp::version_cmp, }; use uucore::{help_about, help_section, help_usage, parse_glob, show, show_error, show_warning}; - +mod dired; +use dired::DiredOutput; #[cfg(not(feature = "selinux"))] static CONTEXT_HELP_TEXT: &str = "print any security context of each file (not enabled)"; #[cfg(feature = "selinux")] @@ -167,6 +169,7 @@ enum LsError { IOError(std::io::Error), IOErrorContext(std::io::Error, PathBuf, bool), BlockSizeParseError(String), + ConflictingArgumentDired(), AlreadyListedError(PathBuf), TimeStyleParseError(String, Vec), } @@ -179,6 +182,7 @@ impl UError for LsError { Self::IOErrorContext(_, _, false) => 1, Self::IOErrorContext(_, _, true) => 2, Self::BlockSizeParseError(_) => 1, + Self::ConflictingArgumentDired() => 0, Self::AlreadyListedError(_) => 2, Self::TimeStyleParseError(_, _) => 1, } @@ -193,6 +197,10 @@ impl Display for LsError { Self::BlockSizeParseError(s) => { write!(f, "invalid --block-size argument {}", s.quote()) } + Self::ConflictingArgumentDired() => { + write!(f, "--dired requires --format=long") + } + Self::TimeStyleParseError(s, possible_time_styles) => { write!( f, @@ -406,6 +414,7 @@ pub struct Config { selinux_supported: bool, group_directories_first: bool, line_ending: LineEnding, + dired: bool, } // Fields that can be removed or added to the long format @@ -610,6 +619,8 @@ fn extract_quoting_style(options: &clap::ArgMatches, show_control: bool) -> Quot QuotingStyle::C { quotes: quoting_style::Quotes::Double, } + } else if options.get_flag(options::DIRED) { + QuotingStyle::Literal { show_control } } else { // TODO: use environment variable if available QuotingStyle::Shell { @@ -954,6 +965,11 @@ impl Config { None }; + let dired = options.get_flag(options::DIRED); + if dired && format != Format::Long { + crash!(1, "{}", LsError::ConflictingArgumentDired()); + } + let dereference = if options.get_flag(options::dereference::ALL) { Dereference::All } else if options.get_flag(options::dereference::ARGS) { @@ -1003,6 +1019,7 @@ impl Config { }, group_directories_first: options.get_flag(options::GROUP_DIRECTORIES_FIRST), line_ending: LineEnding::from_zero_flag(options.get_flag(options::ZERO)), + dired, }) } } @@ -1135,7 +1152,7 @@ pub fn uu_app() -> Command { Arg::new(options::DIRED) .long(options::DIRED) .short('D') - .hide(true) + .help("generate output designed for Emacs' dired (Directory Editor) mode") .action(ArgAction::SetTrue), ) // The next four arguments do not override with the other format @@ -1844,6 +1861,7 @@ pub fn list(locs: Vec<&Path>, config: &Config) -> UResult<()> { let mut files = Vec::::new(); let mut dirs = Vec::::new(); let mut out = BufWriter::new(stdout()); + let mut dired = DiredOutput::default(); let initial_locs_len = locs.len(); for loc in locs { @@ -1877,7 +1895,7 @@ pub fn list(locs: Vec<&Path>, config: &Config) -> UResult<()> { sort_entries(&mut files, config, &mut out); sort_entries(&mut dirs, config, &mut out); - display_items(&files, config, &mut out)?; + display_items(&files, config, &mut out, &mut dired)?; for (pos, path_data) in dirs.iter().enumerate() { // Do read_dir call here to match GNU semantics by printing @@ -1899,7 +1917,13 @@ 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 out)?; + } writeln!(out, "{}:", path_data.p_buf.display())?; + if config.dired { + dired::calculate_offset_and_push(&mut dired, path_data.display_name.len()); + } } else { writeln!(out, "\n{}:", path_data.p_buf.display())?; } @@ -1909,9 +1933,18 @@ pub fn list(locs: Vec<&Path>, config: &Config) -> UResult<()> { &path_data.p_buf, path_data.must_dereference, )?); - enter_directory(path_data, read_dir, config, &mut out, &mut listed_ancestors)?; + enter_directory( + path_data, + read_dir, + config, + &mut out, + &mut listed_ancestors, + &mut dired, + )?; + } + if config.dired { + dired::print_dired_output(config, &dired, &mut out)?; } - Ok(()) } @@ -2022,6 +2055,7 @@ fn enter_directory( config: &Config, out: &mut BufWriter, listed_ancestors: &mut HashSet, + dired: &mut DiredOutput, ) -> UResult<()> { // Create vec of entries with initial dot files let mut entries: Vec = if config.files == Files::All { @@ -2067,10 +2101,14 @@ fn enter_directory( // Print total after any error display if config.format == Format::Long || config.alloc_size { - display_total(&entries, config, out)?; + let total = return_total(&entries, config, out)?; + write!(out, "{}", total.as_str())?; + if config.dired { + dired::add_total(total.len(), dired); + } } - display_items(&entries, config, out)?; + display_items(&entries, config, out, dired)?; if config.recursive { for e in entries @@ -2095,7 +2133,7 @@ fn enter_directory( .insert(FileInformation::from_path(&e.p_buf, e.must_dereference)?) { writeln!(out, "\n{}:", e.p_buf.display())?; - enter_directory(e, rd, config, out, listed_ancestors)?; + enter_directory(e, rd, config, out, listed_ancestors, dired)?; listed_ancestors .remove(&FileInformation::from_path(&e.p_buf, e.must_dereference)?); } else { @@ -2154,7 +2192,11 @@ fn pad_right(string: &str, count: usize) -> String { format!("{string:) -> UResult<()> { +fn return_total( + items: &[PathData], + config: &Config, + out: &mut BufWriter, +) -> UResult { let mut total_size = 0; for item in items { total_size += item @@ -2162,13 +2204,14 @@ fn display_total(items: &[PathData], config: &Config, out: &mut BufWriter) -> UResult<()> { +fn display_items( + items: &[PathData], + config: &Config, + out: &mut BufWriter, + dired: &mut DiredOutput, +) -> UResult<()> { // `-Z`, `--context`: // Display the SELinux security context or '?' if none is found. When used with the `-l` // option, print the security context to the left of the size column. @@ -2220,6 +2268,7 @@ fn display_items(items: &[PathData], config: &Config, out: &mut BufWriter, + dired: &mut DiredOutput, ) -> UResult<()> { + let mut output_display: String = String::new(); + if config.dired { + write!(out, "{}", " ")?; + } if let Some(md) = item.md(out) { - write!( - out, + output_display += &format!( "{}{} {}", display_permissions(md, true), if item.security_context.len() > 1 { @@ -2416,49 +2469,32 @@ fn display_item_long( "" }, pad_left(&display_symlink_count(md), padding.link_count) - )?; + ); if config.long.owner { - write!( - out, - " {}", - pad_right(&display_uname(md, config), padding.uname) - )?; + output_display += &format!(" {}", pad_right(&display_uname(md, config), padding.uname)); } if config.long.group { - write!( - out, - " {}", - pad_right(&display_group(md, config), padding.group) - )?; + output_display += &format!(" {}", pad_right(&display_group(md, config), padding.group)); } if config.context { - write!( - out, - " {}", - pad_right(&item.security_context, padding.context) - )?; + output_display += &format!(" {}", pad_right(&item.security_context, padding.context)); } // Author is only different from owner on GNU/Hurd, so we reuse // the owner, since GNU/Hurd is not currently supported by Rust. if config.long.author { - write!( - out, - " {}", - pad_right(&display_uname(md, config), padding.uname) - )?; + output_display += &format!(" {}", pad_right(&display_uname(md, config), padding.uname)); } match display_len_or_rdev(md, config) { SizeOrDeviceId::Size(size) => { - write!(out, " {}", pad_left(&size, padding.size))?; + output_display += &format!(" {}", pad_left(&size, padding.size)); } SizeOrDeviceId::Device(major, minor) => { - write!( - out, + output_display += &format!( " {}, {}", pad_left( &major, @@ -2478,19 +2514,22 @@ fn display_item_long( #[cfg(unix)] padding.minor, ), - )?; + ); } }; - let dfn = display_file_name(item, config, None, String::new(), out).contents; + output_display += &format!(" {} ", display_date(md, config)); - write!( - out, - " {} {}{}", - display_date(md, config), - dfn, - config.line_ending - )?; + let dfn = display_file_name(item, config, None, String::new(), out).contents; + if config.dired { + let (start, end) = dired::calculate_dired_byte_positions( + output_display.len(), + dfn.len(), + &dired.dired_positions, + ); + dired::update_positions(start, end, dired, false); + } + output_display += &format!("{}{}", dfn, config.line_ending); } else { #[cfg(unix)] let leading_char = { @@ -2525,8 +2564,7 @@ fn display_item_long( } }; - write!( - out, + output_display += &format!( "{}{} {}", format_args!("{leading_char}?????????"), if item.security_context.len() > 1 { @@ -2537,41 +2575,41 @@ fn display_item_long( "" }, pad_left("?", padding.link_count) - )?; + ); if config.long.owner { - write!(out, " {}", pad_right("?", padding.uname))?; + output_display += &format!(" {}", pad_right("?", padding.uname)); } if config.long.group { - write!(out, " {}", pad_right("?", padding.group))?; + output_display += &format!(" {}", pad_right("?", padding.group)); } if config.context { - write!( - out, - " {}", - pad_right(&item.security_context, padding.context) - )?; + output_display += &format!(" {}", pad_right(&item.security_context, padding.context)); } // Author is only different from owner on GNU/Hurd, so we reuse // the owner, since GNU/Hurd is not currently supported by Rust. if config.long.author { - write!(out, " {}", pad_right("?", padding.uname))?; + output_display += &format!(" {}", pad_right("?", padding.uname)); } let dfn = display_file_name(item, config, None, String::new(), out).contents; let date_len = 12; - writeln!( - out, - " {} {} {}", + output_display += &format!( + " {} {} ", pad_left("?", padding.size), pad_left("?", date_len), - dfn, - )?; + ); + + if config.dired { + dired::calculate_and_update_positions(output_display.len(), dfn.trim().len(), dired); + } + output_display += &format!("{}{}", dfn, config.line_ending); } + write!(out, "{}", output_display)?; Ok(()) } diff --git a/src/uucore/src/lib/features/quoting_style.rs b/src/uucore/src/lib/features/quoting_style.rs index 1b9a76aae22..f9d791ac853 100644 --- a/src/uucore/src/lib/features/quoting_style.rs +++ b/src/uucore/src/lib/features/quoting_style.rs @@ -4,12 +4,14 @@ // file that was distributed with this source code. use std::char::from_digit; use std::ffi::OsStr; +use std::fmt; // These are characters with special meaning in the shell (e.g. bash). // The first const contains characters that only have a special meaning when they appear at the beginning of a name. const SPECIAL_SHELL_CHARS_START: &[char] = &['~', '#']; const SPECIAL_SHELL_CHARS: &str = "`$&*()|[]{};\\'\"<>?! "; +#[derive(Clone, Copy, Debug, Eq, PartialEq)] pub enum QuotingStyle { Shell { escape: bool, @@ -24,7 +26,7 @@ pub enum QuotingStyle { }, } -#[derive(Clone, Copy)] +#[derive(Clone, Copy, Debug, Eq, PartialEq)] pub enum Quotes { None, Single, @@ -316,6 +318,42 @@ pub fn escape_name(name: &OsStr, style: &QuotingStyle) -> String { } } +impl fmt::Display for QuotingStyle { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match *self { + Self::Shell { + escape, + always_quote, + show_control, + } => { + let mut style = "shell".to_string(); + if escape { + style.push_str("-escape"); + } + if always_quote { + style.push_str("-always-quote"); + } + if show_control { + style.push_str("-show-control"); + } + f.write_str(&style) + } + Self::C { .. } => f.write_str("C"), + Self::Literal { .. } => f.write_str("literal"), + } + } +} + +impl fmt::Display for Quotes { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match *self { + Self::None => f.write_str("None"), + Self::Single => f.write_str("Single"), + Self::Double => f.write_str("Double"), + } + } +} + #[cfg(test)] mod tests { use crate::quoting_style::{escape_name, Quotes, QuotingStyle}; @@ -732,4 +770,50 @@ mod tests { ], ); } + + #[test] + fn test_quoting_style_display() { + let style = QuotingStyle::Shell { + escape: true, + always_quote: false, + show_control: false, + }; + assert_eq!(format!("{}", style), "shell-escape"); + + let style = QuotingStyle::Shell { + escape: false, + always_quote: true, + show_control: false, + }; + assert_eq!(format!("{}", style), "shell-always-quote"); + + let style = QuotingStyle::Shell { + escape: false, + always_quote: false, + show_control: true, + }; + assert_eq!(format!("{}", style), "shell-show-control"); + + let style = QuotingStyle::C { + quotes: Quotes::Double, + }; + assert_eq!(format!("{}", style), "C"); + + let style = QuotingStyle::Literal { + show_control: false, + }; + assert_eq!(format!("{}", style), "literal"); + } + + #[test] + fn test_quotes_display() { + let q = Quotes::None; + assert_eq!(format!("{}", q), "None"); + + let q = Quotes::Single; + assert_eq!(format!("{}", q), "Single"); + + let q = Quotes::Double; + assert_eq!(format!("{}", q), "Double"); + } } diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 5f2b7e44350..a18a19929f3 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 +// 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 #[cfg(any(unix, feature = "feat_selinux"))] use crate::common::util::expected_result; @@ -3522,3 +3522,126 @@ fn test_ls_perm_io_errors() { .code_is(1) .stderr_contains("Permission denied"); } + +#[test] +fn test_ls_dired_incompatible() { + let scene = TestScenario::new(util_name!()); + + scene + .ucmd() + .arg("--dired") + .fails() + .code_is(1) + .stderr_contains("--dired requires --format=long"); +} + +#[test] +fn test_ls_dired_recursive() { + let scene = TestScenario::new(util_name!()); + + scene + .ucmd() + .arg("--dired") + .arg("-l") + .arg("-R") + .succeeds() + .stdout_does_not_contain("//DIRED//") + .stdout_contains(" total 0") + .stdout_contains("//SUBDIRED// 2 3") + .stdout_contains("//DIRED-OPTIONS// --quoting-style"); +} + +#[test] +fn test_ls_dired_simple() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + scene + .ucmd() + .arg("--dired") + .arg("-l") + .succeeds() + .stdout_contains(" total 0"); + + at.mkdir("d"); + at.touch("d/a1"); + let mut cmd = scene.ucmd(); + cmd.arg("--dired").arg("-l").arg("d"); + let result = cmd.succeeds(); + result.stdout_contains(" total 0"); + println!(" result.stdout = {:#?}", result.stdout_str()); + + let dired_line = result + .stdout_str() + .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); + + let start_pos = positions[0]; + let end_pos = positions[1]; + + // Extract the filename using the positions + let filename = + String::from_utf8(result.stdout_str().as_bytes()[start_pos..=end_pos].to_vec()).unwrap(); + + assert_eq!(filename, "a1\n"); +} + +#[test] +fn test_ls_dired_complex() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + at.mkdir("d"); + at.mkdir("d/d"); + at.touch("d/a1"); + at.touch("d/a22"); + at.touch("d/a333"); + at.touch("d/a4444"); + + let mut cmd = scene.ucmd(); + cmd.arg("--dired").arg("-l").arg("d"); + let result = cmd.succeeds(); + // Number of blocks + #[cfg(target_os = "linux")] + result.stdout_contains(" total 4"); + + let output = result.stdout_str().to_string(); + println!("Output:\n{}", output); + + 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(); + println!("{:?}", positions); + println!("Parsed byte positions: {:?}", positions); + assert_eq!(positions.len() % 2, 0); // Ensure there's an even number of positions + + let filenames: Vec = positions + .chunks(2) + .map(|chunk| { + let start_pos = chunk[0]; + let end_pos = chunk[1]; + let filename = String::from_utf8(output.as_bytes()[start_pos..=end_pos].to_vec()) + .unwrap() + .trim() + .to_string(); + println!("Extracted filename: {}", filename); + filename + }) + .collect(); + + println!("Extracted filenames: {:?}", filenames); + assert_eq!(filenames, vec!["a1", "a22", "a333", "a4444", "d"]); +} From dd65685b0b328ec295a2fc36e6520fb267055c41 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sat, 16 Sep 2023 21:07:36 +0200 Subject: [PATCH 02/10] stat-failed.sh: update of the test - we have a small difference --- util/build-gnu.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/util/build-gnu.sh b/util/build-gnu.sh index 2d949bbb333..ca3c959eabe 100755 --- a/util/build-gnu.sh +++ b/util/build-gnu.sh @@ -261,6 +261,10 @@ sed -i -e "s/Try 'mv --help' for more information/For more information, try '--h # disable these test cases sed -i -E "s|^([^#]*2_31.*)$|#\1|g" tests/printf/printf-cov.pl + +# with ls --dired, in case of error, we have a slightly different error position +sed -i -e "s|44 45|45 46|" tests/ls/stat-failed.sh + sed -i -e "s/du: invalid -t argument/du: invalid --threshold argument/" -e "s/du: option requires an argument/error: a value is required for '--threshold ' but none was supplied/" -e "/Try 'du --help' for more information./d" tests/du/threshold.sh # disable two kind of tests: From a8c2b757946c70f5daf25b3a4295833a0eabf032 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sat, 16 Sep 2023 22:54:37 +0200 Subject: [PATCH 03/10] ls --dired: address some of the comments --- src/uu/ls/src/dired.rs | 43 +++++++++++++++++------------------------- src/uu/ls/src/ls.rs | 7 +++---- util/build-gnu.sh | 2 +- 3 files changed, 21 insertions(+), 31 deletions(-) diff --git a/src/uu/ls/src/dired.rs b/src/uu/ls/src/dired.rs index d3a8e88fab5..957930fcd4a 100644 --- a/src/uu/ls/src/dired.rs +++ b/src/uu/ls/src/dired.rs @@ -13,7 +13,6 @@ use uucore::error::UResult; pub struct BytePosition { pub start: usize, pub end: usize, - pub is_total: Option, } /// Represents the output structure for DIRED, containing positions for both DIRED and SUBDIRED. @@ -21,6 +20,7 @@ pub struct BytePosition { pub struct DiredOutput { pub dired_positions: Vec, pub subdired_positions: Vec, + pub just_printed_total: bool, } impl fmt::Display for BytePosition { @@ -44,7 +44,7 @@ pub fn calculate_dired_byte_positions( 0 }; - let start = output_display_len + offset_from_previous_line + DIRED_TRAILING_OFFSET; + let start = output_display_len + offset_from_previous_line; let end = start + dfn_len; (start, end) } @@ -63,7 +63,6 @@ pub fn calculate_offset_and_push(dired: &mut DiredOutput, path_len: usize) { dired.subdired_positions.push(BytePosition { start: offset, end: path_len + offset, - is_total: None, }); } @@ -76,9 +75,7 @@ pub fn print_dired_output( out.flush()?; if config.recursive { print_positions("//SUBDIRED//", &dired.subdired_positions); - } else if dired.dired_positions.last().map_or(false, |last_position| { - !last_position.is_total.unwrap_or(false) - }) { + } else if dired.just_printed_total == false { print_positions("//DIRED//", &dired.dired_positions); } println!("//DIRED-OPTIONS// --quoting-style={}", config.quoting_style); @@ -95,12 +92,12 @@ fn print_positions(prefix: &str, positions: &Vec) { } pub fn add_total(total_len: usize, dired: &mut DiredOutput) { + dired.just_printed_total = true; dired.dired_positions.push(BytePosition { start: 0, // the 2 is from the trailing spaces // the 1 is from the line ending (\n) end: total_len + DIRED_TRAILING_OFFSET - 1, - is_total: Some(true), }); } @@ -125,26 +122,20 @@ pub fn calculate_and_update_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(start: usize, end: usize, dired: &mut DiredOutput, adjust: bool) { - if let Some(last_position) = dired.dired_positions.last() { - if last_position.is_total.unwrap_or(false) { - if let Some(last_position) = dired.dired_positions.last_mut() { - *last_position = BytePosition { - start: if adjust { - start + last_position.end - } else { - start - }, - end: if adjust { end + last_position.end } else { end }, - is_total: Some(false), - }; - } - } else { - dired.dired_positions.push(BytePosition { - start, - end, - is_total: None, - }); + if dired.just_printed_total { + if let Some(last_position) = dired.dired_positions.last_mut() { + *last_position = BytePosition { + start: if adjust { + start + last_position.end + } else { + start + }, + end: if adjust { end + last_position.end } else { end }, + }; + dired.just_printed_total = false; } + } else { + dired.dired_positions.push(BytePosition { start, end }); } } diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index af5b8980141..2231c581348 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -53,7 +53,6 @@ use uucore::libc::{S_IXGRP, S_IXOTH, S_IXUSR}; use uucore::line_ending::LineEnding; use uucore::quoting_style::{escape_name, QuotingStyle}; use uucore::{ - crash, display::Quotable, error::{set_exit_code, UError, UResult}, format_usage, @@ -182,7 +181,7 @@ impl UError for LsError { Self::IOErrorContext(_, _, false) => 1, Self::IOErrorContext(_, _, true) => 2, Self::BlockSizeParseError(_) => 1, - Self::ConflictingArgumentDired() => 0, + Self::ConflictingArgumentDired() => 1, Self::AlreadyListedError(_) => 2, Self::TimeStyleParseError(_, _) => 1, } @@ -967,7 +966,7 @@ impl Config { let dired = options.get_flag(options::DIRED); if dired && format != Format::Long { - crash!(1, "{}", LsError::ConflictingArgumentDired()); + return Err(Box::new(LsError::ConflictingArgumentDired())); } let dereference = if options.get_flag(options::dereference::ALL) { @@ -2455,7 +2454,7 @@ fn display_item_long( ) -> UResult<()> { let mut output_display: String = String::new(); if config.dired { - write!(out, "{}", " ")?; + output_display += &format!("{}", " "); } if let Some(md) = item.md(out) { output_display += &format!( diff --git a/util/build-gnu.sh b/util/build-gnu.sh index ca3c959eabe..a5671db07f2 100755 --- a/util/build-gnu.sh +++ b/util/build-gnu.sh @@ -263,7 +263,7 @@ sed -i -E "s|^([^#]*2_31.*)$|#\1|g" tests/printf/printf-cov.pl # with ls --dired, in case of error, we have a slightly different error position -sed -i -e "s|44 45|45 46|" tests/ls/stat-failed.sh +sed -i -e "s|44 45|47 48|" tests/ls/stat-failed.sh sed -i -e "s/du: invalid -t argument/du: invalid --threshold argument/" -e "s/du: option requires an argument/error: a value is required for '--threshold ' but none was supplied/" -e "/Try 'du --help' for more information./d" tests/du/threshold.sh From af4a91a78d0d6a1aa90d5c50e18f671449d80260 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sat, 16 Sep 2023 23:12:19 +0200 Subject: [PATCH 04/10] fix warnings --- src/uu/ls/src/dired.rs | 11 ++++------- src/uu/ls/src/ls.rs | 2 +- util/build-gnu.sh | 2 +- 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/uu/ls/src/dired.rs b/src/uu/ls/src/dired.rs index 957930fcd4a..5e722ee333b 100644 --- a/src/uu/ls/src/dired.rs +++ b/src/uu/ls/src/dired.rs @@ -75,7 +75,7 @@ pub fn print_dired_output( out.flush()?; if config.recursive { print_positions("//SUBDIRED//", &dired.subdired_positions); - } else if dired.just_printed_total == false { + } else if !dired.just_printed_total { print_positions("//DIRED//", &dired.dired_positions); } println!("//DIRED-OPTIONS// --quoting-style={}", config.quoting_style); @@ -150,13 +150,12 @@ mod tests { let dired_positions = vec![BytePosition { start: 5, end: 10, - is_total: None, }]; let (start, end) = calculate_dired_byte_positions(output_display.len(), dfn.len(), &dired_positions); - assert_eq!(start, 26); - assert_eq!(end, 37); + assert_eq!(start, 24); + assert_eq!(end, 35); } #[test] @@ -165,9 +164,9 @@ mod tests { dired_positions: vec![BytePosition { start: 5, end: 10, - is_total: Some(true), }], subdired_positions: vec![], + just_printed_total: true, }; // Test with adjust = true @@ -175,13 +174,11 @@ mod tests { 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) - assert_eq!(last_position.is_total, Some(false)); // Test with adjust = false update_positions(30, 35, &mut dired, false); let last_position = dired.dired_positions.last().unwrap(); assert_eq!(last_position.start, 30); assert_eq!(last_position.end, 35); - assert_eq!(last_position.is_total, None); } } diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 2231c581348..2bc3715dc3e 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2454,7 +2454,7 @@ fn display_item_long( ) -> UResult<()> { let mut output_display: String = String::new(); if config.dired { - output_display += &format!("{}", " "); + output_display += " "; } if let Some(md) = item.md(out) { output_display += &format!( diff --git a/util/build-gnu.sh b/util/build-gnu.sh index a5671db07f2..75be52587dd 100755 --- a/util/build-gnu.sh +++ b/util/build-gnu.sh @@ -3,7 +3,7 @@ # # UU_MAKE_PROFILE == 'debug' | 'release' ## build profile for *uutils* build; may be supplied by caller, defaults to 'release' -# spell-checker:ignore (paths) abmon deref discrim eacces getlimits getopt ginstall inacc infloop inotify reflink ; (misc) INT_OFLOW OFLOW baddecode submodules ; (vars/env) SRCDIR vdir rcexp xpart +# spell-checker:ignore (paths) abmon deref discrim eacces getlimits getopt ginstall inacc infloop inotify reflink ; (misc) INT_OFLOW OFLOW baddecode submodules ; (vars/env) SRCDIR vdir rcexp xpart dired set -e From 74f189093a38edcdeee73cce0bb70d9b6e757c63 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 17 Sep 2023 09:53:46 +0200 Subject: [PATCH 05/10] use unwrap() --- src/uu/ls/src/dired.rs | 10 ++---- src/uu/ls/src/ls.rs | 73 ++++++++++++++++++++++++++++++------------ 2 files changed, 55 insertions(+), 28 deletions(-) diff --git a/src/uu/ls/src/dired.rs b/src/uu/ls/src/dired.rs index 5e722ee333b..f3d39327efb 100644 --- a/src/uu/ls/src/dired.rs +++ b/src/uu/ls/src/dired.rs @@ -147,10 +147,7 @@ mod tests { fn test_calculate_dired_byte_positions() { let output_display = "sample_output".to_string(); let dfn = "sample_file".to_string(); - let dired_positions = vec![BytePosition { - start: 5, - end: 10, - }]; + let dired_positions = vec![BytePosition { start: 5, end: 10 }]; let (start, end) = calculate_dired_byte_positions(output_display.len(), dfn.len(), &dired_positions); @@ -161,10 +158,7 @@ mod tests { #[test] fn test_dired_update_positions() { let mut dired = DiredOutput { - dired_positions: vec![BytePosition { - start: 5, - end: 10, - }], + dired_positions: vec![BytePosition { start: 5, end: 10 }], subdired_positions: vec![], just_printed_total: true, }; diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 2bc3715dc3e..2db40d6e391 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2457,7 +2457,8 @@ fn display_item_long( output_display += " "; } if let Some(md) = item.md(out) { - output_display += &format!( + write!( + output_display, "{}{} {}", display_permissions(md, true), if item.security_context.len() > 1 { @@ -2468,32 +2469,54 @@ fn display_item_long( "" }, pad_left(&display_symlink_count(md), padding.link_count) - ); + ) + .unwrap(); if config.long.owner { - output_display += &format!(" {}", pad_right(&display_uname(md, config), padding.uname)); + write!( + output_display, + " {}", + pad_right(&display_uname(md, config), padding.uname) + ) + .unwrap(); } if config.long.group { - output_display += &format!(" {}", pad_right(&display_group(md, config), padding.group)); + write!( + output_display, + " {}", + pad_right(&display_group(md, config), padding.group) + ) + .unwrap(); } if config.context { - output_display += &format!(" {}", pad_right(&item.security_context, padding.context)); + write!( + output_display, + " {}", + pad_right(&item.security_context, padding.context) + ) + .unwrap(); } // Author is only different from owner on GNU/Hurd, so we reuse // the owner, since GNU/Hurd is not currently supported by Rust. if config.long.author { - output_display += &format!(" {}", pad_right(&display_uname(md, config), padding.uname)); + write!( + output_display, + " {}", + pad_right(&display_uname(md, config), padding.uname) + ) + .unwrap(); } match display_len_or_rdev(md, config) { SizeOrDeviceId::Size(size) => { - output_display += &format!(" {}", pad_left(&size, padding.size)); + write!(output_display, " {}", pad_left(&size, padding.size)).unwrap(); } SizeOrDeviceId::Device(major, minor) => { - output_display += &format!( + write!( + output_display, " {}, {}", pad_left( &major, @@ -2513,11 +2536,12 @@ fn display_item_long( #[cfg(unix)] padding.minor, ), - ); + ) + .unwrap(); } }; - output_display += &format!(" {} ", display_date(md, config)); + write!(output_display, " {} ", display_date(md, config)).unwrap(); let dfn = display_file_name(item, config, None, String::new(), out).contents; if config.dired { @@ -2528,7 +2552,7 @@ fn display_item_long( ); dired::update_positions(start, end, dired, false); } - output_display += &format!("{}{}", dfn, config.line_ending); + write!(output_display, "{}{}", dfn, config.line_ending).unwrap(); } else { #[cfg(unix)] let leading_char = { @@ -2563,7 +2587,8 @@ fn display_item_long( } }; - output_display += &format!( + write!( + output_display, "{}{} {}", format_args!("{leading_char}?????????"), if item.security_context.len() > 1 { @@ -2574,39 +2599,47 @@ fn display_item_long( "" }, pad_left("?", padding.link_count) - ); + ) + .unwrap(); if config.long.owner { - output_display += &format!(" {}", pad_right("?", padding.uname)); + write!(output_display, " {}", pad_right("?", padding.uname)).unwrap(); } if config.long.group { - output_display += &format!(" {}", pad_right("?", padding.group)); + write!(output_display, " {}", pad_right("?", padding.group)).unwrap(); } if config.context { - output_display += &format!(" {}", pad_right(&item.security_context, padding.context)); + write!( + output_display, + " {}", + pad_right(&item.security_context, padding.context) + ) + .unwrap(); } // Author is only different from owner on GNU/Hurd, so we reuse // the owner, since GNU/Hurd is not currently supported by Rust. if config.long.author { - output_display += &format!(" {}", pad_right("?", padding.uname)); + write!(output_display, " {}", pad_right("?", padding.uname)).unwrap(); } let dfn = display_file_name(item, config, None, String::new(), out).contents; let date_len = 12; - output_display += &format!( + write!( + output_display, " {} {} ", pad_left("?", padding.size), pad_left("?", date_len), - ); + ) + .unwrap(); if config.dired { dired::calculate_and_update_positions(output_display.len(), dfn.trim().len(), dired); } - output_display += &format!("{}{}", dfn, config.line_ending); + write!(output_display, "{}{}", dfn, config.line_ending).unwrap(); } write!(out, "{}", output_display)?; From 00290a048f784ef768f01be4a44f871f0c7b2d58 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Mon, 18 Sep 2023 11:10:30 +0200 Subject: [PATCH 06/10] Improve test Co-authored-by: Daniel Hofstetter --- 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 a18a19929f3..87b3066569a 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -3588,9 +3588,9 @@ fn test_ls_dired_simple() { // Extract the filename using the positions let filename = - String::from_utf8(result.stdout_str().as_bytes()[start_pos..=end_pos].to_vec()).unwrap(); + String::from_utf8(result.stdout_str().as_bytes()[start_pos..end_pos].to_vec()).unwrap(); - assert_eq!(filename, "a1\n"); + assert_eq!(filename, "a1"); } #[test] From ffedba1b044acb5e96aa78dc35bb6611d1a3f6df Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Mon, 18 Sep 2023 11:10:41 +0200 Subject: [PATCH 07/10] Simplify test Co-authored-by: Daniel Hofstetter --- src/uucore/src/lib/features/quoting_style.rs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/uucore/src/lib/features/quoting_style.rs b/src/uucore/src/lib/features/quoting_style.rs index f9d791ac853..b517dbd8db5 100644 --- a/src/uucore/src/lib/features/quoting_style.rs +++ b/src/uucore/src/lib/features/quoting_style.rs @@ -807,13 +807,8 @@ mod tests { #[test] fn test_quotes_display() { - let q = Quotes::None; - assert_eq!(format!("{}", q), "None"); - - let q = Quotes::Single; - assert_eq!(format!("{}", q), "Single"); - - let q = Quotes::Double; - assert_eq!(format!("{}", q), "Double"); + assert_eq!(format!("{}", Quotes::None), "None"); + assert_eq!(format!("{}", Quotes::Single), "Single"); + assert_eq!(format!("{}", Quotes::Double), "Double"); } } From 48769c1f28b57d95a1bec5b058fd65c17aa767bc Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Mon, 18 Sep 2023 14:51:43 +0200 Subject: [PATCH 08/10] Remove a word from the spell ignore Co-authored-by: Daniel Hofstetter --- src/uu/ls/src/ls.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 2db40d6e391..0edeaaa68e9 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) cpio svgz webm somegroup nlink rmvb xspf tabsize dired subdired dired +// spell-checker:ignore (ToDO) cpio svgz webm somegroup nlink rmvb xspf tabsize dired subdired use clap::{ builder::{NonEmptyStringValueParser, ValueParser}, From 2816df78093315a7de577bd4564719d2b15d2784 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Mon, 18 Sep 2023 15:15:53 +0200 Subject: [PATCH 09/10] remove duplication of the spell ignore Co-authored-by: Daniel Hofstetter --- src/uu/ls/src/dired.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/ls/src/dired.rs b/src/uu/ls/src/dired.rs index f3d39327efb..f66d22b38dd 100644 --- a/src/uu/ls/src/dired.rs +++ b/src/uu/ls/src/dired.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 dired subdired dired +// spell-checker:ignore dired subdired use crate::Config; use std::fmt; From 568fa33067c59fbf5b7d31d78d5440f6917b70ee Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Wed, 20 Sep 2023 00:19:01 +0200 Subject: [PATCH 10/10] rustfmt --- src/uu/ls/src/ls.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 7675ece9e22..3d67cccaad1 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2553,8 +2553,7 @@ fn display_item_long( dired::update_positions(start, end, dired, false); } write!(output_display, "{}{}", displayed_file, config.line_ending).unwrap(); - - } else { + } else { #[cfg(unix)] let leading_char = { if let Some(Some(ft)) = item.ft.get() { @@ -2638,7 +2637,11 @@ fn display_item_long( .unwrap(); if config.dired { - dired::calculate_and_update_positions(output_display.len(), displayed_file.trim().len(), dired); + dired::calculate_and_update_positions( + output_display.len(), + displayed_file.trim().len(), + dired, + ); } write!(output_display, "{}{}", displayed_file, config.line_ending).unwrap(); }