fix(ls): apply reviewed changes

Renames and documents PathDataDisplayName::Moo to _::Custom; also makes code more idiomatic and restores an accidentally-removed warning.
Co-authored-by: Daniel Hofstetter <daniel.hofstetter@42dh.com>
Co-authored-by: xtqqczze <45661989+xtqqczze@users.noreply.github.com>

Author: 3 people

src/uu/ls/src/config.rs

@@ -345,11 +345,6 @@ fn extract_time(options: &clap::ArgMatches) -> MetadataTimeField {
 fn is_color_compatible_term() -> bool {
     let term = std::env::var_os("TERM");
     let colorterm = std::env::var_os("COLORTERM");
-    let is_term_set = term.is_some();
-    let is_colorterm_set = colorterm.is_some();
-
-    let term = term.unwrap_or_default();
-    let colorterm = colorterm.unwrap_or_default();
 
     // Search function in the TERM struct to manage the wildcards
     let term_matches = |term: &OsStr| -> bool {
@@ -362,14 +357,11 @@ fn is_color_compatible_term() -> bool {
         })
     };
 
-    if is_term_set && term.is_empty() && is_colorterm_set && colorterm.is_empty() {
-        return false;
-    }
-
-    if !term.is_empty() && !term_matches(&term) {
-        return false;
+    match (term, colorterm) {
+        (Some(t), Some(c)) if t.is_empty() && c.is_empty() => false,
+        (Some(t), _) if !t.is_empty() => term_matches(&t),
+        _ => true,
     }
-    true
 }
 
 /// Extracts the color option to use based on the options provided.
@@ -1030,8 +1022,8 @@ fn parse_time_style(options: &clap::ArgMatches) -> Result<(String, Option<String
 
     if let Some(field) = options
         .get_one::<String>(options::TIME_STYLE)
-        .map(|s| Cow::Borrowed(s.as_str()))
-        .or_else(|| std::env::var("TIME_STYLE").ok().map(Cow::Owned))
+        .map(Cow::from)
+        .or_else(|| std::env::var("TIME_STYLE").ok().map(Cow::from))
     {
         //If both FULL_TIME and TIME_STYLE are present
         //The one added last is dominant

src/uu/ls/src/ls.rs

@@ -784,10 +784,12 @@ pub fn uu_app() -> Command {
     .after_help(translate!("ls-after-help"))
 }
 
+/// Represents the possible values of [`PathData::display_name`]. The reason this is a
+/// separate enum is to avoid a self-referential struct, as it is moved in hot loops.
 #[derive(Debug)]
 enum PathDataDisplayName<'a> {
     SelfReferential,
-    Moo(Cow<'a, OsStr>),
+    Custom(Cow<'a, OsStr>),
 }
 
 /// Represents a Path along with it's associated data.
@@ -821,11 +823,11 @@ impl<'a> PathData<'a> {
         // We cannot use `Path::ends_with` or `Path::Components`, because they remove occurrences of '.'
         // For '..', the filename is None
         let display_name = if let Some(name) = file_name {
-            PathDataDisplayName::Moo(name)
+            PathDataDisplayName::Custom(name)
         } else if command_line {
             PathDataDisplayName::SelfReferential
         } else {
-            PathDataDisplayName::Moo(
+            PathDataDisplayName::Custom(
                 dir_entry
                     .as_ref()
                     .map(DirEntry::file_name)
@@ -952,7 +954,7 @@ impl<'a> PathData<'a> {
     fn display_name(&self) -> &OsStr {
         match self.display_name {
             PathDataDisplayName::SelfReferential => self.p_buf.as_os_str(),
-            PathDataDisplayName::Moo(ref cow) => cow,
+            PathDataDisplayName::Custom(ref cow) => cow,
         }
     }
 }
@@ -1452,7 +1454,17 @@ fn get_security_context<'a>(
 
                 let res: String = match str::from_utf8(context) {
                     Ok(s) => s.to_string(),
-                    Err(_) => String::from_utf8_lossy(context).into_owned(),
+                    Err(e) => {
+                        show_warning!(
+                            "{}",
+                            translate!(
+                                "ls-warning-getting-security-context",
+                                "path" => path.quote(),
+                                "error" => e
+                            )
+                        );
+                        String::from_utf8_lossy(context).into_owned()
+                    }
                 };
 
                 return Cow::Owned(res);