Skip to content
Draft
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 28 additions & 10 deletions src/app/sub.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,19 @@ where

let coloring: Option<Coloring> = color.map(|ls_colors| {
let mut map = HashMap::new();
build_coloring_map(&data_tree, &mut Vec::new(), &mut map);
if only_one_arg {
build_coloring_map(&data_tree, &mut Vec::new(), &mut Vec::new(), &mut map);
} else {
// For multi-arg invocations the root is the synthetic "(total)" node.
// Include it in the map key (the visualizer's ancestor chain contains it)
// but skip it in the filesystem path (it doesn't exist on disk).
let root_name = data_tree.name().as_os_str();
for child in data_tree.children() {
let mut key_stack = vec![root_name];
let mut fs_stack = Vec::new();
build_coloring_map(child, &mut key_stack, &mut fs_stack, &mut map);
}
}
Coloring::new(ls_colors, map)
});

Expand Down Expand Up @@ -282,27 +294,33 @@ where

/// Recursively walk a pruned [`DataTree`] and build a map of path-component vectors to [`Color`] values.
///
/// The `path_stack` argument is a reusable buffer of path components representing the current
/// ancestor chain. Each recursive call pushes the node's name and pops it on return, so no
/// cloning occurs during traversal — only at leaf insertions.
/// `key_stack` tracks the ancestor chain used as the HashMap key (must match what the
/// [`Visualizer`] constructs). `fs_path_stack` tracks the real filesystem path used for
/// file-type detection. These two stacks diverge when the root is a synthetic node like
/// `(total)` that has no corresponding directory on disk.
///
/// Leaf nodes (files or childless directories after pruning) are added to the map.
/// Nodes with children are skipped because the [`Visualizer`] uses the children count to
/// determine their color at render time.
fn build_coloring_map<'a>(
node: &'a DataTree<OsStringDisplay, impl size::Size>,
path_stack: &mut Vec<&'a OsStr>,
key_stack: &mut Vec<&'a OsStr>,
fs_path_stack: &mut Vec<&'a OsStr>,
map: &mut HashMap<Vec<&'a OsStr>, Color>,
) {
path_stack.push(node.name().as_os_str());
let name = node.name().as_os_str();
key_stack.push(name);
fs_path_stack.push(name);
if node.children().is_empty() {
let color = file_color(&path_stack.iter().collect::<PathBuf>());
map.insert(path_stack.clone(), color);
let color = file_color(&fs_path_stack.iter().collect::<PathBuf>());
map.insert(key_stack.clone(), color);
} else {
for child in node.children() {
build_coloring_map(child, path_stack, map);
build_coloring_map(child, key_stack, fs_path_stack, map);
}
}
path_stack.pop();
key_stack.pop();
fs_path_stack.pop();
}

fn file_color(path: &Path) -> Color {
Expand Down
94 changes: 94 additions & 0 deletions tests/usual_cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -880,6 +880,100 @@ fn color_always() {
assert_eq!(actual, expected);
}

#[cfg(unix)]
#[test]
fn color_always_multiple_args() {
let workspace = SampleWorkspace::simple_tree_with_diverse_kinds();

let args = [
"dir-a",
"dir-b",
"file-root.txt",
"link-dir",
"link-file.txt",
"empty-dir-1",
"empty-dir-2",
];

let actual = {
let mut cmd = Command::new(PDU);
cmd = cmd
.with_current_dir(&workspace)
.with_arg("--color=always")
.with_arg("--total-width=100")
.with_arg("--min-ratio=0")
.with_env("LS_COLORS", LS_COLORS);
for arg in &args {
cmd = cmd.with_arg(arg);
}
cmd.pipe(stdio)
.output()
.expect("spawn command with --color=always and multiple args")
.pipe(stdout_text)
};
eprintln!("ACTUAL:\n{actual}\n");

let mut data_tree = args
.iter()
.map(|name| {
let builder = FsTreeBuilder {
root: workspace.to_path_buf().join(name),
size_getter: DEFAULT_GET_SIZE,
hardlinks_recorder: &HardlinkIgnorant,
reporter: &ErrorOnlyReporter::new(ErrorReport::SILENT),
max_depth: 10,
};
let mut data_tree: DataTree<OsStringDisplay, _> = builder.into();
*data_tree.name_mut() = OsStringDisplay::os_string_from(name);
data_tree
})
.pipe(|children| {
DataTree::dir(
OsStringDisplay::os_string_from("(total)"),
0.into(),
children.collect(),
)
})
.into_par_sorted(|left, right| left.size().cmp(&right.size()).reverse());
data_tree.par_cull_insignificant_data(0.0);

let ls_colors = LsColors::from_str(LS_COLORS);
let leaf_colors = [
("(total)/dir-a/file-a1.txt", Color::Normal),
("(total)/dir-a/file-a2.txt", Color::Normal),
("(total)/dir-a/subdir-a/file-a3.txt", Color::Normal),
("(total)/dir-b/file-b1.txt", Color::Normal),
("(total)/file-root.txt", Color::Normal),
("(total)/link-dir", Color::Symlink),
("(total)/link-file.txt", Color::Symlink),
("(total)/empty-dir-1", Color::Directory),
("(total)/empty-dir-2", Color::Directory),
];
let leaf_colors = HashMap::from(leaf_colors.map(|(path, color)| {
(
path.split('/')
.map(AsRef::<OsStr>::as_ref)
.collect::<Vec<_>>(),
color,
)
}));
let coloring = Coloring::new(ls_colors, leaf_colors);
Comment on lines +937 to +955
Copy link

Copilot AI Mar 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new test expects multi-arg output to correctly color symlinks and empty directories under the synthetic "(total)" root. In the current CLI implementation, leaf colors are computed by build_coloring_map via file_color(PathBuf::from(path_components)); when multiple args are used, that PathBuf includes the fake "(total)" component, so is_symlink()/is_dir() will return false and leaves will be misclassified as Normal (breaking the behavior this test asserts). To make this test pass, the production code needs to compute the filesystem stat path without the synthetic root component (e.g., maintain a separate real-path stack, or special-case the fake root when building the PathBuf for file_color) while keeping the display-path key used for coloring lookups.

Copilot uses AI. Check for mistakes.

let visualizer = Visualizer::<OsStringDisplay, _> {
data_tree: &data_tree,
bytes_format: BytesFormat::MetricUnits,
direction: Direction::BottomUp,
bar_alignment: BarAlignment::Left,
column_width_distribution: ColumnWidthDistribution::total(100),
coloring: Some(&coloring),
};
let expected = format!("{visualizer}");
let expected = expected.trim_end();
eprintln!("EXPECTED:\n{expected}\n");

assert_eq!(actual, expected);
}

#[test]
fn colorful_equals_colorless() {
let workspace = SampleWorkspace::default();
Expand Down
Loading