diff --git a/src/uu/ls/src/config.rs b/src/uu/ls/src/config.rs index e2e797f1628..7d1640c4fb6 100644 --- a/src/uu/ls/src/config.rs +++ b/src/uu/ls/src/config.rs @@ -172,7 +172,7 @@ pub struct Config { // Dir and vdir needs access to this field pub quoting_style: QuotingStyle, pub(crate) locale_quoting: Option, - pub(crate) indicator_style: IndicatorStyle, + pub(crate) indicator_style: Option, pub(crate) time_format_recent: String, // Time format for recent dates pub(crate) time_format_older: Option, // Time format for older dates (optional, if not present, time_format_recent is used) pub(crate) context: bool, @@ -554,34 +554,28 @@ fn extract_quoting_style( /// # Returns /// /// An [`IndicatorStyle`] variant representing the indicator style to use. -fn extract_indicator_style(options: &clap::ArgMatches) -> IndicatorStyle { +fn extract_indicator_style(options: &clap::ArgMatches) -> Option { if let Some(field) = options.get_one::(options::INDICATOR_STYLE) { match field.as_str() { - "none" => IndicatorStyle::None, - "file-type" => IndicatorStyle::FileType, - "classify" => IndicatorStyle::Classify, - "slash" => IndicatorStyle::Slash, - &_ => IndicatorStyle::None, + "none" => None, + "file-type" => Some(IndicatorStyle::FileType), + "classify" => Some(IndicatorStyle::Classify), + "slash" => Some(IndicatorStyle::Slash), + &_ => None, } } else if let Some(field) = options.get_one::(options::indicator_style::CLASSIFY) { match field.as_str() { - "never" | "no" | "none" => IndicatorStyle::None, - "always" | "yes" | "force" => IndicatorStyle::Classify, - "auto" | "tty" | "if-tty" => { - if stdout().is_terminal() { - IndicatorStyle::Classify - } else { - IndicatorStyle::None - } - } - &_ => IndicatorStyle::None, + "never" | "no" | "none" => None, + "always" | "yes" | "force" => Some(IndicatorStyle::Classify), + "auto" | "tty" | "if-tty" => stdout().is_terminal().then_some(IndicatorStyle::Classify), + &_ => None, } } else if options.get_flag(options::indicator_style::SLASH) { - IndicatorStyle::Slash + Some(IndicatorStyle::Slash) } else if options.get_flag(options::indicator_style::FILE_TYPE) { - IndicatorStyle::FileType + Some(IndicatorStyle::FileType) } else { - IndicatorStyle::None + None } } @@ -954,7 +948,7 @@ impl Config { } else if options.get_flag(options::dereference::DIR_ARGS) { Dereference::DirArgs } else if options.get_flag(options::DIRECTORY) - || indicator_style == IndicatorStyle::Classify + || indicator_style == Some(IndicatorStyle::Classify) || format == Format::Long { Dereference::None diff --git a/src/uu/ls/src/display.rs b/src/uu/ls/src/display.rs index 929ce7e4610..7a30d1dd8a7 100644 --- a/src/uu/ls/src/display.rs +++ b/src/uu/ls/src/display.rs @@ -93,9 +93,8 @@ pub(crate) struct DisplayItemName { pub(crate) dired_name_len: usize, } -#[derive(PartialEq, Eq)] +#[derive(Clone, Copy, PartialEq, Eq)] pub(crate) enum IndicatorStyle { - None, Slash, FileType, Classify, @@ -704,29 +703,12 @@ fn display_item_name( } } - if config.indicator_style != IndicatorStyle::None { - let sym = classify_file(path); - - let char_opt = match config.indicator_style { - IndicatorStyle::Classify => sym, - IndicatorStyle::FileType => { - // Don't append an asterisk. - match sym { - Some('*') => None, - _ => sym, - } - } - IndicatorStyle::Slash => { - // Append only a slash. - match sym { - Some('/') => Some('/'), - _ => None, - } - } - IndicatorStyle::None => None, - }; + let is_long_symlink = config.format == Format::Long + && path.file_type().is_some_and(FileType::is_symlink) + && !path.must_dereference; - if let Some(c) = char_opt { + if !is_long_symlink { + if let Some(c) = indicator_char(path, config.indicator_style) { let _ = name.write_char(c); } } @@ -744,66 +726,75 @@ fn display_item_name( // We might as well color the symlink output after the arrow. // This makes extra system calls, but provides important information that // people run `ls -l --color` are very interested in. - if let Some(style_manager) = &mut style_manager { - let escaped_target = escape_name_with_locale(target_path.as_os_str(), config); - // We get the absolute path to be able to construct PathData with valid Metadata. - // This is because relative symlinks will fail to get_metadata. - let absolute_target = if target_path.is_relative() { - match path.path().parent() { - Some(p) => &p.join(&target_path), - None => &target_path, - } - } else { - &target_path - }; - - match fs::canonicalize(absolute_target) { - Ok(resolved_target) => { - let target_data = PathData::new( - resolved_target.as_path().into(), - None, - target_path.file_name().map(Cow::Borrowed), - config, - false, - ); - + let escaped_target = escape_name_with_locale(target_path.as_os_str(), config); + + // We get the absolute path to be able to construct PathData with valid Metadata. + // This is because relative symlinks will fail to get_metadata. + let absolute_target = if target_path.is_relative() { + match path.path().parent() { + Some(p) => &p.join(&target_path), + None => &target_path, + } + } else { + &target_path + }; + + match fs::canonicalize(absolute_target) { + Ok(resolved_target) => { + let target_data = PathData::new( + resolved_target.as_path().into(), + None, + target_path.file_name().map(Cow::Borrowed), + config, + false, + ); + + let target_display = if let Some(style_manager) = style_manager { + let md = match target_data.metadata() { + Some(md) => Some(Cow::Borrowed(md)), + None => target_data.p_buf.symlink_metadata().ok().map(Cow::Owned), + }; // Check if the target actually needs coloring - let md_option: Option = target_data - .metadata() - .cloned() - .or_else(|| target_data.p_buf.symlink_metadata().ok()); - let style = style_manager.colors.style_for_path_with_metadata( - &target_data.p_buf, - md_option.as_ref(), - ); - - if style.is_some() { + if style_manager + .colors + .style_for_path_with_metadata(&target_data.p_buf, md.as_deref()) + .is_some() + { // Only apply coloring if there's actually a style - name.push(color_name( + &color_name( escaped_target, &target_data, style_manager, None, is_wrap(name.len()), - )); + ) } else { // For regular files with no coloring, just use plain text - name.push(escaped_target); + &escaped_target } + } else { + &escaped_target + }; + + name.push(target_display); + if let Some(c) = indicator_char(&target_data, config.indicator_style) { + let _ = name.write_char(c); } - Err(_) => { + } + Err(_) => { + if let Some(style_manager) = &mut style_manager { name.push( style_manager.apply_missing_target_style( escaped_target, is_wrap(name.len()), ), ); + } else { + // If no coloring is required, we just use target as is. + // with the right quoting + name.push(escaped_target); } } - } else { - // If no coloring is required, we just use target as is. - // Apply the right quoting - name.push(escape_name_with_locale(target_path.as_os_str(), config)); } } Err(err) => { @@ -1118,6 +1109,23 @@ fn display_item_long( Ok(()) } +fn indicator_char(path: &PathData, style: Option) -> Option { + let style = style?; + let sym = classify_file(path); + + match style { + IndicatorStyle::Classify => sym, + IndicatorStyle::FileType => match sym { + Some('*') => None, + _ => sym, + }, + IndicatorStyle::Slash => match sym { + Some('/') => Some('/'), + _ => None, + }, + } +} + fn classify_file(path: &PathData) -> Option { let file_type = path.file_type()?; diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index fbbba1a0a22..12bd5e6d818 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -3386,6 +3386,29 @@ fn test_ls_indicator_style() { } } +#[test] +#[cfg(not(windows))] +fn test_ls_indicator_style_symlink_target_long() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + at.mkdir("dir"); + assert!(at.dir_exists("dir")); + + at.symlink_dir("dir", "dir_link"); + assert!(at.is_symlink("dir_link")); + + scene + .ucmd() + .arg("--classify") + .arg("-l") + .arg("dir_link") + .succeeds() + .stdout_contains("dir_link -> ") + .stdout_does_not_contain("dir_link@ -> ") + .stdout_contains("/dir/"); +} + // Essentially the same test as above, but only test symlinks and directories, // not pipes or sockets. #[test]