refactor: move coloring logic from methods.rs into coloring.rs

src/visualizer/coloring.rs

@@ -17,18 +17,27 @@ impl<'a> Coloring<'a> {
         Coloring { ls_colors, map }
     }
 
-    /// Return `(color, ls_colors)` for a node, used to build a colored slice for rendering.
-    pub(super) fn node_color(
+    /// Look up the color for a node identified by its path components and whether it has children,
+    /// then wrap the given [`TreeHorizontalSlice`] in the appropriate colored or colorless variant.
+    fn maybe_color_tree_slice(
         &self,
         path_components: &[&'a OsStr],
         has_children: bool,
-    ) -> Option<(Color, &LsColors)> {
+        slice: TreeHorizontalSlice<String>,
+    ) -> MaybeColoredTreeHorizontalSlice<'_> {
         let color = if has_children {
             Some(Color::Directory)
         } else {
             self.map.get(path_components).copied()
-        }?;
-        Some((color, &self.ls_colors))
+        };
+        match color {
+            Some(color) => MaybeColoredTreeHorizontalSlice::Colorful(ColoredTreeHorizontalSlice {
+                slice,
+                color,
+                ls_colors: &self.ls_colors,
+            }),
+            None => MaybeColoredTreeHorizontalSlice::Colorless(slice),
+        }
     }
 }
 
@@ -46,19 +55,15 @@ pub enum Color {
 }
 
 impl Color {
-    // TODO: reconsider the visibility of this function once the TODOs in
-    //       `visualizer/methods.rs` have been dealt with.
     /// Get the ANSI prefix for this color from the given prefix table.
-    pub(super) fn ansi_prefix(self, prefixes: &LsColors) -> AnsiPrefix<'_> {
+    fn ansi_prefix(self, prefixes: &LsColors) -> AnsiPrefix<'_> {
         AnsiPrefix(prefixes.prefix_str(self))
     }
 }
 
-// TODO: reconsider the of this struct once the TODOs in
-//       `visualizer/methods.rs` have been dealt with.
 /// ANSI prefix wrapper for a [`Color`] variant, implements [`Display`].
 #[derive(Display)]
-pub(super) struct AnsiPrefix<'a>(&'a str);
+struct AnsiPrefix<'a>(&'a str);
 
 impl AnsiPrefix<'_> {
     /// Returns the reset suffix to emit after this prefix, or `""` if no prefix.
@@ -73,11 +78,9 @@ impl AnsiPrefix<'_> {
 
 /// A [`TreeHorizontalSlice`] with its color applied, used for rendering.
 pub(super) struct ColoredTreeHorizontalSlice<'a> {
-    // TODO: reconsider the following visibilities once the TODOs in
-    //       `visualizer/methods.rs` have been dealt with.
-    pub(super) slice: TreeHorizontalSlice<String>,
-    pub(super) color: Color,
-    pub(super) ls_colors: &'a LsColors,
+    slice: TreeHorizontalSlice<String>,
+    color: Color,
+    ls_colors: &'a LsColors,
 }
 
 impl fmt::Display for ColoredTreeHorizontalSlice<'_> {
@@ -106,6 +109,26 @@ impl Width for ColoredTreeHorizontalSlice<'_> {
     }
 }
 
+/// Wrap a [`TreeHorizontalSlice`] with color if coloring is available, otherwise return it as-is.
+///
+/// Path components are only constructed when coloring is enabled, avoiding
+/// unnecessary allocation in the common no-color case.
+pub(super) fn maybe_color_slice<'a, 'b>(
+    coloring: Option<&'b Coloring<'a>>,
+    ancestors: impl Iterator<Item = &'a OsStr>,
+    name: &'a OsStr,
+    has_children: bool,
+    slice: TreeHorizontalSlice<String>,
+) -> MaybeColoredTreeHorizontalSlice<'b> {
+    match coloring {
+        Some(coloring) => {
+            let path_components: Vec<&OsStr> = ancestors.chain(std::iter::once(name)).collect();
+            coloring.maybe_color_tree_slice(&path_components, has_children, slice)
+        }
+        None => MaybeColoredTreeHorizontalSlice::Colorless(slice),
+    }
+}
+
 /// Either a [`TreeHorizontalSlice`] (colorless) or a [`ColoredTreeHorizontalSlice`] (colorful).
 #[derive(Display)]
 pub(super) enum MaybeColoredTreeHorizontalSlice<'a> {

src/visualizer/methods.rs

@@ -12,13 +12,9 @@ use node_info::*;
 use table::*;
 use tree_table::*;
 
-use super::{
-    coloring::{ColoredTreeHorizontalSlice, MaybeColoredTreeHorizontalSlice},
-    ColumnWidthDistribution, Visualizer,
-};
+use super::{coloring::maybe_color_slice, ColumnWidthDistribution, Visualizer};
 use crate::size;
-use pipe_trait::Pipe;
-use std::{cmp::min, ffi::OsStr, fmt::Display, iter::once};
+use std::{cmp::min, ffi::OsStr, fmt::Display};
 use zero_copy_pads::{align_left, align_right};
 
 impl<'a, Name, Size> Visualizer<'a, Name, Size>
@@ -95,28 +91,13 @@ where
                     tree_horizontal_slice,
                 } = tree_row;
 
-                // TODO: move this `colored` code block into `visualizer/coloring.rs`.
-                let colored = self.coloring.and_then(|coloring| {
-                    let path_components: Vec<&OsStr> = initial_row
-                        .ancestors
-                        .iter()
-                        .map(|node| node.name.as_ref())
-                        .chain(initial_row.node_info.name.pipe_as_ref(once))
-                        .collect();
-                    coloring.node_color(&path_components, initial_row.node_info.children_count > 0)
-                });
-
-                // TODO: move this `tree` code block into `visualizer/coloring.rs`.
-                let tree = match colored {
-                    Some((color, ls_colors)) => {
-                        MaybeColoredTreeHorizontalSlice::Colorful(ColoredTreeHorizontalSlice {
-                            slice: tree_horizontal_slice,
-                            color,
-                            ls_colors,
-                        })
-                    }
-                    None => MaybeColoredTreeHorizontalSlice::Colorless(tree_horizontal_slice),
-                };
+                let tree = maybe_color_slice(
+                    self.coloring,
+                    initial_row.ancestors.iter().map(|node| node.name.as_ref()),
+                    initial_row.node_info.name.as_ref(),
+                    initial_row.node_info.children_count > 0,
+                    tree_horizontal_slice,
+                );
 
                 format!(
                     "{size} {tree}│{bar}│{ratio}",