Address review comments: refactor LsColors, add color test without LS_COLORS

Co-authored-by: KSXGitHub <11488886+KSXGitHub@users.noreply.github.com>

Author: Copilot

src/ls_colors.rs

@@ -1,7 +1,5 @@
 use crate::visualizer::coloring::Color;
-use lscolors::{Indicator, LsColors as LsColorsCrate};
-use std::convert::Infallible;
-use std::str::FromStr;
+use lscolors::{self, Indicator};
 
 /// ANSI color prefix strings for terminal output, initialized from the `LS_COLORS` environment
 /// variable.
@@ -16,10 +14,17 @@ pub struct LsColors {
 impl LsColors {
     /// Initialize by reading the current environment's `LS_COLORS`.
     pub fn from_env() -> Self {
-        Self::from_ls_colors_crate(&LsColorsCrate::from_env().unwrap_or_default())
+        Self::from_ls_colors_crate(&lscolors::LsColors::from_env().unwrap_or_default())
     }
 
-    fn from_ls_colors_crate(ls_colors: &LsColorsCrate) -> Self {
+    /// Parse an `LS_COLORS`-format string into an [`LsColors`].
+    ///
+    /// Unrecognized or invalid entries are silently ignored.
+    pub fn from_str(input: &str) -> Self {
+        Self::from_ls_colors_crate(&lscolors::LsColors::from_string(input))
+    }
+
+    fn from_ls_colors_crate(ls_colors: &lscolors::LsColors) -> Self {
         let prefix_for = |indicator: Indicator| {
             ls_colors
                 .style_for_indicator(indicator)
@@ -44,15 +49,3 @@ impl LsColors {
         }
     }
 }
-
-impl FromStr for LsColors {
-    type Err = Infallible;
-    /// Parse an `LS_COLORS`-format string into an [`LsColors`].
-    ///
-    /// This never fails; unrecognized or invalid entries are silently ignored.
-    fn from_str(input: &str) -> Result<Self, Self::Err> {
-        Ok(Self::from_ls_colors_crate(&LsColorsCrate::from_string(
-            input,
-        )))
-    }
-}

tests/usual_cli.rs

@@ -852,6 +852,56 @@ fn colorful_equals_colorless() {
     assert_eq!(colorful_stripped, colorless);
 }
 
+#[test]
+fn color_always_with_and_without_ls_colors() {
+    let workspace = SampleWorkspace::default();
+
+    let with_ls_colors = Command::new(PDU)
+        .with_current_dir(&workspace)
+        .with_arg("--color=always")
+        .with_arg("--total-width=100")
+        .with_env("LS_COLORS", LS_COLORS)
+        .pipe(stdio)
+        .output()
+        .expect("spawn command with --color=always and LS_COLORS");
+    inspect_stderr(&with_ls_colors.stderr);
+    assert!(
+        with_ls_colors.status.success(),
+        "pdu exited with non-zero status"
+    );
+    let with_ls_colors_stripped = with_ls_colors
+        .stdout
+        .pipe(String::from_utf8)
+        .expect("UTF-8")
+        .pipe(strip_ansi_escapes::strip_str)
+        .trim_end()
+        .to_string();
+
+    let mut without_ls_colors_cmd = Command::new(PDU)
+        .with_current_dir(&workspace)
+        .with_arg("--color=always")
+        .with_arg("--total-width=100");
+    without_ls_colors_cmd.env_remove("LS_COLORS");
+    let without_ls_colors = without_ls_colors_cmd
+        .pipe(stdio)
+        .output()
+        .expect("spawn command with --color=always and without LS_COLORS");
+    inspect_stderr(&without_ls_colors.stderr);
+    assert!(
+        without_ls_colors.status.success(),
+        "pdu exited with non-zero status"
+    );
+    let without_ls_colors_stripped = without_ls_colors
+        .stdout
+        .pipe(String::from_utf8)
+        .expect("UTF-8")
+        .pipe(strip_ansi_escapes::strip_str)
+        .trim_end()
+        .to_string();
+
+    assert_eq!(with_ls_colors_stripped, without_ls_colors_stripped);
+}
+
 #[cfg(unix)]
 #[test]
 fn color_always() {
@@ -880,7 +930,7 @@ fn color_always() {
     data_tree.par_sort_by(|left, right| left.size().cmp(&right.size()).reverse());
     *data_tree.name_mut() = OsStringDisplay::os_string_from(".");
 
-    let ls_colors: LsColors = LS_COLORS.parse().expect("parse LS_COLORS");
+    let ls_colors = LsColors::from_str(LS_COLORS);
     let map = HashMap::from([
         (
             OsStringDisplay::os_string_from("file-a1.txt"),