Skip to content

Commit d78e31e

Browse files
CopilotKSXGitHub
andcommitted
fix: use full PathBuf key in build_coloring_map to prevent basename collision
Co-authored-by: KSXGitHub <11488886+KSXGitHub@users.noreply.github.com>
1 parent 2c0b44f commit d78e31e

6 files changed

Lines changed: 42 additions & 55 deletions

File tree

src/app/sub.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ where
197197
.or(deduplication_result);
198198
}
199199

200-
let coloring: Option<Coloring<OsStringDisplay>> = color.map(|ls_colors| {
200+
let coloring: Option<Coloring> = color.map(|ls_colors| {
201201
let mut map = HashMap::new();
202202
build_coloring_map(&data_tree, PathBuf::new(), &mut map);
203203
Coloring::new(ls_colors, map)
@@ -279,7 +279,7 @@ where
279279
}
280280
}
281281

282-
/// Recursively walk a pruned [`DataTree`] and build a map of leaf node names to [`Color`] values.
282+
/// Recursively walk a pruned [`DataTree`] and build a map of full paths to [`Color`] values.
283283
///
284284
/// The `path` argument should be the current path prefix for reconstructing full filesystem paths.
285285
/// Leaf nodes (files or childless directories after pruning) are added to the map.
@@ -288,7 +288,7 @@ where
288288
fn build_coloring_map(
289289
node: &DataTree<OsStringDisplay, impl size::Size>,
290290
path: PathBuf,
291-
map: &mut HashMap<OsStringDisplay, Color>,
291+
map: &mut HashMap<PathBuf, Color>,
292292
) {
293293
let node_path = path.join(node.name().as_os_str());
294294
if !node.children().is_empty() {
@@ -297,7 +297,8 @@ fn build_coloring_map(
297297
}
298298
return;
299299
}
300-
map.insert(node.name().clone(), file_color(&node_path));
300+
let color = file_color(&node_path);
301+
map.insert(node_path, color);
301302
}
302303

303304
fn file_color(path: &Path) -> Color {

src/visualizer.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ where
6262
/// Distribution and total number of characters/blocks can be placed in a line.
6363
pub column_width_distribution: ColumnWidthDistribution,
6464
/// Mapping of names to colors for colorful output.
65-
pub coloring: Option<&'a Coloring<Name>>,
65+
pub coloring: Option<&'a Coloring>,
6666
}
6767

6868
mod copy;

src/visualizer/coloring.rs

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,32 @@
11
use super::{ChildPosition, TreeHorizontalSlice};
22
use crate::ls_colors::LsColors;
33
use derive_more::Display;
4-
use std::{collections::HashMap, fmt, hash::Hash};
4+
use std::{
5+
collections::HashMap,
6+
fmt,
7+
path::{Path, PathBuf},
8+
};
59
use zero_copy_pads::Width;
610

7-
/// Coloring configuration: ANSI prefix strings from the environment and a name-to-color map.
11+
/// Coloring configuration: ANSI prefix strings from the environment and a full-path-to-color map.
812
#[derive(Debug)]
9-
pub struct Coloring<Name> {
13+
pub struct Coloring {
1014
ansi_prefixes: LsColors,
11-
map: HashMap<Name, Color>,
15+
map: HashMap<PathBuf, Color>,
1216
}
1317

14-
impl<Name: Hash + Eq> Coloring<Name> {
15-
/// Create a new [`Coloring`] from ANSI prefixes and a name-to-color map.
16-
pub fn new(ansi_prefixes: LsColors, map: HashMap<Name, Color>) -> Self {
18+
impl Coloring {
19+
/// Create a new [`Coloring`] from ANSI prefixes and a full-path-to-color map.
20+
pub fn new(ansi_prefixes: LsColors, map: HashMap<PathBuf, Color>) -> Self {
1721
Coloring { ansi_prefixes, map }
1822
}
1923

2024
/// Return `(color, prefixes)` for a node, used to build a colored slice for rendering.
21-
pub(super) fn node_color(&self, name: &Name, has_children: bool) -> Option<(Color, &LsColors)> {
25+
pub(super) fn node_color(&self, path: &Path, has_children: bool) -> Option<(Color, &LsColors)> {
2226
let color = if has_children {
2327
Some(Color::Directory)
2428
} else {
25-
self.map.get(name).copied()
29+
self.map.get(path).copied()
2630
}?;
2731
Some((color, &self.ansi_prefixes))
2832
}

src/visualizer/display.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
use super::{Direction::*, Visualizer};
22
use crate::size;
33
use std::{
4+
ffi::OsStr,
45
fmt::{Display, Error, Formatter},
56
hash::Hash,
67
};
78

89
impl<'a, Name, Size> Display for Visualizer<'a, Name, Size>
910
where
10-
Name: Display + Hash + Eq,
11+
Name: Display + Hash + Eq + AsRef<OsStr>,
1112
Size: size::Size + Into<u64>,
1213
{
1314
/// Create the ASCII chart.

src/visualizer/methods.rs

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,12 @@ use tree_table::*;
1414

1515
use super::{coloring::ColoredTreeHorizontalSlice, ColumnWidthDistribution, Visualizer};
1616
use crate::size;
17-
use std::{cmp::min, fmt::Display, hash::Hash};
17+
use std::{cmp::min, ffi::OsStr, fmt::Display, hash::Hash, path::PathBuf};
1818
use zero_copy_pads::{align_left, align_right};
1919

2020
impl<'a, Name, Size> Visualizer<'a, Name, Size>
2121
where
22-
Name: Display + Hash + Eq,
22+
Name: Display + Hash + Eq + AsRef<OsStr>,
2323
Size: size::Size + Into<u64>,
2424
{
2525
/// Create ASCII rows that visualize the [tree](crate::data_tree::DataTree), such rows
@@ -92,10 +92,15 @@ where
9292
} = tree_row;
9393

9494
let colored = self.coloring.and_then(|coloring| {
95-
coloring.node_color(
96-
initial_row.node_info.name,
97-
initial_row.node_info.children_count > 0,
98-
)
95+
let path: PathBuf = initial_row
96+
.ancestors
97+
.iter()
98+
.map(|a| a.name.as_ref() as &OsStr)
99+
.chain(std::iter::once(
100+
initial_row.node_info.name.as_ref() as &OsStr
101+
))
102+
.collect();
103+
coloring.node_color(&path, initial_row.node_info.children_count > 0)
99104
});
100105

101106
let aligned_colored_slice;

tests/usual_cli.rs

Lines changed: 10 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ use parallel_disk_usage::{
2727
visualizer::{Color, Coloring},
2828
};
2929
#[cfg(unix)]
30-
use std::collections::HashMap;
30+
use std::{collections::HashMap, path::PathBuf};
3131

3232
#[cfg(unix)]
3333
use parallel_disk_usage::get_size::{GetBlockCount, GetBlockSize};
@@ -932,39 +932,15 @@ fn color_always() {
932932

933933
let ls_colors = LsColors::from_ls_colors_string(LS_COLORS);
934934
let map = HashMap::from([
935-
(
936-
OsStringDisplay::os_string_from("file-a1.txt"),
937-
Color::Normal,
938-
),
939-
(
940-
OsStringDisplay::os_string_from("file-a2.txt"),
941-
Color::Normal,
942-
),
943-
(
944-
OsStringDisplay::os_string_from("file-a3.txt"),
945-
Color::Normal,
946-
),
947-
(
948-
OsStringDisplay::os_string_from("file-b1.txt"),
949-
Color::Normal,
950-
),
951-
(
952-
OsStringDisplay::os_string_from("file-root.txt"),
953-
Color::Normal,
954-
),
955-
(OsStringDisplay::os_string_from("link-dir"), Color::Symlink),
956-
(
957-
OsStringDisplay::os_string_from("link-file.txt"),
958-
Color::Symlink,
959-
),
960-
(
961-
OsStringDisplay::os_string_from("empty-dir-1"),
962-
Color::Directory,
963-
),
964-
(
965-
OsStringDisplay::os_string_from("empty-dir-2"),
966-
Color::Directory,
967-
),
935+
(PathBuf::from("./dir-a/file-a1.txt"), Color::Normal),
936+
(PathBuf::from("./dir-a/file-a2.txt"), Color::Normal),
937+
(PathBuf::from("./dir-a/subdir-a/file-a3.txt"), Color::Normal),
938+
(PathBuf::from("./dir-b/file-b1.txt"), Color::Normal),
939+
(PathBuf::from("./file-root.txt"), Color::Normal),
940+
(PathBuf::from("./link-dir"), Color::Symlink),
941+
(PathBuf::from("./link-file.txt"), Color::Symlink),
942+
(PathBuf::from("./empty-dir-1"), Color::Directory),
943+
(PathBuf::from("./empty-dir-2"), Color::Directory),
968944
]);
969945
let coloring = Coloring::new(ls_colors, map);
970946

0 commit comments

Comments
 (0)