fix(ls): take care of performance regressions

Co-authored-by: Guillem L. Jara <4lon3ly0@tutanota.com>

Author: joknarf

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<LocaleQuoting>,
-    pub(crate) indicator_style: IndicatorStyle,
+    pub(crate) indicator_style: Option<IndicatorStyle>,
     pub(crate) time_format_recent: String, // Time format for recent dates
     pub(crate) time_format_older: Option<String>, // 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<IndicatorStyle> {
     if let Some(field) = options.get_one::<String>(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::<String>(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

src/uu/ls/src/display.rs

@@ -95,7 +95,6 @@ pub(crate) struct DisplayItemName {
 
 #[derive(Clone, Copy, PartialEq, Eq)]
 pub(crate) enum IndicatorStyle {
-    None,
     Slash,
     FileType,
     Classify,
@@ -733,14 +732,14 @@ fn display_item_name(
                 // 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.clone(),
+                        Some(p) => &p.join(&target_path),
+                        None => &target_path,
                     }
                 } else {
-                    target_path.clone()
+                    &target_path
                 };
 
-                match fs::canonicalize(&absolute_target) {
+                match fs::canonicalize(absolute_target) {
                     Ok(resolved_target) => {
                         let target_data = PathData::new(
                             resolved_target.as_path().into(),
@@ -750,27 +749,32 @@ fn display_item_name(
                             false,
                         );
 
-                        let mut target_display = escaped_target.clone();
-                        if let Some(style_manager) = &mut style_manager {
-                            let md_option: Option<Metadata> = target_data
-                                .metadata()
-                                .cloned()
-                                .or_else(|| target_data.p_buf.symlink_metadata().ok());
-
+                        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
                             if style_manager
                                 .colors
-                                .style_for_path_with_metadata(&target_data.p_buf, md_option.as_ref())
+                                .style_for_path_with_metadata(&target_data.p_buf, md.as_deref())
                                 .is_some()
                             {
-                                target_display = color_name(
-                                    escaped_target.clone(),
+                                // Only apply coloring if there's actually a style
+                                &color_name(
+                                    escaped_target,
                                     &target_data,
                                     style_manager,
                                     None,
                                     is_wrap(name.len()),
-                                );
+                                )
+                            } else {
+                                // For regular files with no coloring, just use plain text
+                                &escaped_target
                             }
-                        }
+                        } else {
+                            &escaped_target
+                        };
 
                         name.push(target_display);
                         if let Some(c) = indicator_char(&target_data, config.indicator_style) {
@@ -786,6 +790,8 @@ fn display_item_name(
                                 ),
                             );
                         } else {
+                            // If no coloring is required, we just use target as is.
+                            // with the right quoting
                             name.push(escaped_target);
                         }
                     }
@@ -1103,7 +1109,8 @@ fn display_item_long(
     Ok(())
 }
 
-fn indicator_char(path: &PathData, style: IndicatorStyle) -> Option<char> {
+fn indicator_char(path: &PathData, style: Option<IndicatorStyle>) -> Option<char> {
+    let style = style?;
     let sym = classify_file(path);
 
     match style {
@@ -1116,7 +1123,6 @@ fn indicator_char(path: &PathData, style: IndicatorStyle) -> Option<char> {
             Some('/') => Some('/'),
             _ => None,
         },
-        IndicatorStyle::None => None,
     }
 }