ls: fix symlink target coloring to match target file type

src/uu/ls/src/colors.rs

@@ -151,6 +151,18 @@ pub(crate) fn color_name(
     style_manager: &mut StyleManager,
     target_symlink: Option<&PathData>,
     wrap: bool,
+) -> OsString {
+    color_name_with_dangling_hint(name, path, style_manager, target_symlink, false, wrap)
+}
+
+/// Colors the provided name, with a hint about whether the target is dangling
+pub(crate) fn color_name_with_dangling_hint(
+    name: OsString,
+    path: &PathData,
+    style_manager: &mut StyleManager,
+    target_symlink: Option<&PathData>,
+    target_is_dangling: bool,
+    wrap: bool,
 ) -> OsString {
     // Check if the file has capabilities
     #[cfg(all(unix, not(any(target_os = "android", target_os = "macos"))))]
@@ -172,17 +184,38 @@ pub(crate) fn color_name(
         }
     }
 
-    if !path.must_dereference {
-        // If we need to dereference (follow) a symlink, we will need to get the metadata
-        // There is a DirEntry, we don't need to get the metadata for the color
-        return style_manager.apply_style_based_on_colorable(path, name, wrap);
-    }
-
     if let Some(target) = target_symlink {
         // use the optional target_symlink
-        // Use fn symlink_metadata directly instead of get_metadata() here because ls
-        // should not exit with an err, if we are unable to obtain the target_metadata
-        style_manager.apply_style_based_on_colorable(target, name, wrap)
+        // Use the target's metadata to color it according to its actual type
+        let md_option = target.p_buf.metadata().ok();
+        let style = if let Some(md) = &md_option {
+            if md.is_dir() {
+                style_manager
+                    .colors
+                    .style_for_indicator(lscolors::Indicator::Directory)
+            } else {
+                // For files and other types, use Normal style (no special coloring)
+                style_manager
+                    .colors
+                    .style_for_indicator(lscolors::Indicator::Normal)
+            }
+        } else {
+            // If metadata is not available (dangling symlink), fall back to the original behavior
+            // which colors it based on the Colorable trait implementation
+            return style_manager.apply_style_based_on_colorable(target, name, wrap);
+        };
+        style_manager.apply_style(style, name, wrap)
+    } else if target_is_dangling {
+        // For dangling symlinks, color the target name like a symbolic link
+        // so symlink and target share the same visible style.
+        let symlink_style = style_manager
+            .colors
+            .style_for_indicator(Indicator::SymbolicLink);
+        style_manager.apply_style(symlink_style, name, wrap)
+    } else if !path.must_dereference {
+        // If we need to dereference (follow) a symlink, we will need to get the metadata
+        // There is a DirEntry, we don't need to get the metadata for the color
+        style_manager.apply_style_based_on_colorable(path, name, wrap)
     } else {
         let md_option: Option<Metadata> = path
             .metadata()

src/uu/ls/src/ls.rs

@@ -81,7 +81,7 @@ mod dired;
 use dired::{DiredOutput, is_dired_arg_present};
 mod colors;
 use crate::options::QUOTING_STYLE;
-use colors::{StyleManager, color_name};
+use colors::{StyleManager, color_name, color_name_with_dangling_hint};
 
 pub mod options {
     pub mod format {
@@ -1883,22 +1883,7 @@ impl PathData {
                 .unwrap_or_default()
         };
 
-        let must_dereference = match &config.dereference {
-            Dereference::All => true,
-            Dereference::Args => command_line,
-            Dereference::DirArgs => {
-                if command_line {
-                    if let Ok(md) = p_buf.metadata() {
-                        md.is_dir()
-                    } else {
-                        false
-                    }
-                } else {
-                    false
-                }
-            }
-            Dereference::None => false,
-        };
+        let must_dereference = compute_must_dereference(config, command_line, p_buf.as_path());
 
         // Why prefer to check the DirEntry file_type()?  B/c the call is
         // nearly free compared to a metadata() call on a Path
@@ -3248,31 +3233,29 @@ fn display_item_name(
                 // 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 state.style_manager {
-                    // 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 mut absolute_target = target_path.clone();
-                    if target_path.is_relative() {
-                        if let Some(parent) = path.path().parent() {
-                            absolute_target = parent.join(absolute_target);
-                        }
-                    }
+                    if target_path.exists() {
+                        // Target exists, create PathData and use enhanced coloring
+                        let target_data =
+                            PathData::new(target_path.clone(), None, None, config, false);
 
-                    let target_data = PathData::new(absolute_target, None, None, config, false);
-
-                    // If we have a symlink to a valid file, we use the metadata of said file.
-                    // Because we use an absolute path, we can assume this is guaranteed to exist.
-                    // Otherwise, we use path.md(), which will guarantee we color to the same
-                    // color of non-existent symlinks according to style_for_path_with_metadata.
-                    if path.metadata().is_none() && target_data.metadata().is_none() {
-                        name.push(target_path);
-                    } else {
+                        // Use the enhanced coloring logic that checks target metadata
                         name.push(color_name(
                             locale_aware_escape_name(target_path.as_os_str(), config.quoting_style),
                             path,
                             style_manager,
                             Some(&target_data),
                             is_wrap(name.len()),
                         ));
+                    } else {
+                        // Target doesn't exist (dangling symlink), use special coloring
+                        name.push(color_name_with_dangling_hint(
+                            locale_aware_escape_name(target_path.as_os_str(), config.quoting_style),
+                            path,
+                            style_manager,
+                            None,
+                            true, // target_is_dangling = true
+                            is_wrap(name.len()),
+                        ));
                     }
                 } else {
                     // If no coloring is required, we just use target as is.
@@ -3548,3 +3531,18 @@ fn os_str_starts_with(haystack: &OsStr, needle: &[u8]) -> bool {
 fn write_os_str<W: Write>(writer: &mut W, string: &OsStr) -> std::io::Result<()> {
     writer.write_all(&os_str_as_bytes_lossy(string))
 }
+
+/// Checks if the path is a directory when processed from the command line.
+fn is_command_line_directory(p_buf: &Path, command_line: bool) -> bool {
+    command_line && p_buf.metadata().map(|md| md.is_dir()).unwrap_or(false)
+}
+
+/// Computes whether to dereference the path based on config.
+fn compute_must_dereference(config: &Config, command_line: bool, p_buf: &Path) -> bool {
+    match &config.dereference {
+        Dereference::All => true,
+        Dereference::Args => command_line,
+        Dereference::DirArgs => is_command_line_directory(p_buf, command_line),
+        Dereference::None => false,
+    }
+}

tests/by-util/test_ls.rs

@@ -1260,9 +1260,9 @@ fn test_ls_long_symlink_color() {
         (None, "ln-file-invalid", Some([1, 1]), "dir1/invalid-target"),
         // We acquired [1, 1], the non-existent color.
         (Some([0, 0]), "ln-file1", None, "dir1/file1"),
-        (Some([1, 1]), "ln-dir-invalid", Some([1, 1]), "dir1/dir2"),
+        (Some([0, 0]), "ln-dir-invalid", Some([0, 1]), "dir1/dir2"),
         (Some([0, 0]), "ln-root", Some([0, 1]), "/"),
-        (Some([0, 0]), "ln-up2", None, "../.."),
+        (Some([0, 0]), "ln-up2", Some([0, 1]), "../.."),
     ];
 
     // We are only interested in lines or the ls output that are symlinks. These start with "lrwx".
@@ -6498,7 +6498,7 @@ fn test_f_overrides_sort_flags() {
 
     // Create files with different sizes for predictable sort order
     at.write("small.txt", "a"); // 1 byte
-    at.write("medium.txt", "bb"); // 2 bytes  
+    at.write("medium.txt", "bb"); // 2 bytes
     at.write("large.txt", "ccc"); // 3 bytes
 
     // Get baseline outputs (include -a to match -f behavior which shows all files)