Skip to content

Commit bc6faff

Browse files
CopilotKSXGitHub
Copilot
and
KSXGitHub
committed
refactor(r8): rename field, minimize visibility, move color calc, diverse-kinds workspace, fix tests
Co-authored-by: KSXGitHub <11488886+KSXGitHub@users.noreply.github.com>
1 parent ab5cc0a commit bc6faff

5 files changed

Lines changed: 108 additions & 47 deletions

File tree

‎src/app.rs‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,12 @@ impl App {
174174
ErrorReport::TEXT
175175
};
176176

177+
let color = match self.args.color {
178+
ColorWhen::Always => Some(LsColors::from_env()),
179+
ColorWhen::Never => None,
180+
ColorWhen::Auto => stdout().is_terminal().then(LsColors::from_env),
181+
};
182+
177183
trait GetSizeUtils: GetSize<Size: size::Size> {
178184
const INSTANCE: Self;
179185
const QUANTITY: Quantity;
@@ -298,7 +304,6 @@ impl App {
298304
no_sort,
299305
omit_json_shared_details,
300306
omit_json_shared_summary,
301-
color,
302307
..
303308
} => Sub {
304309
direction: Direction::from_top_down(top_down),
@@ -313,11 +318,7 @@ impl App {
313318
max_depth,
314319
min_ratio,
315320
no_sort,
316-
color: match color {
317-
ColorWhen::Always => Some(LsColors::from_env()),
318-
ColorWhen::Never => None,
319-
ColorWhen::Auto => stdout().is_terminal().then(LsColors::from_env),
320-
},
321+
color,
321322
}
322323
.run(),
323324
)*} };

‎src/visualizer/coloring.rs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ impl<Name: Hash + Eq> Coloring<Name> {
1818
}
1919

2020
/// Return `(color, prefixes)` for a node, used to build a colored slice for rendering.
21-
pub(crate) fn node_color(&self, name: &Name, has_children: bool) -> Option<(Color, &LsColors)> {
21+
pub(super) fn node_color(&self, name: &Name, has_children: bool) -> Option<(Color, &LsColors)> {
2222
let color = if has_children {
2323
Some(Color::Directory)
2424
} else {
@@ -64,10 +64,10 @@ impl AnsiPrefix<'_> {
6464
}
6565

6666
/// A [`TreeHorizontalSlice`] with its color applied, used for rendering.
67-
pub(crate) struct ColoredTreeHorizontalSlice<'a> {
68-
pub(crate) slice: TreeHorizontalSlice<String>,
69-
pub(crate) color: Color,
70-
pub(crate) ansi_prefixes: &'a LsColors,
67+
pub(super) struct ColoredTreeHorizontalSlice<'a> {
68+
pub(super) slice: TreeHorizontalSlice<String>,
69+
pub(super) color: Color,
70+
pub(super) ls_colors: &'a LsColors,
7171
}
7272

7373
impl fmt::Display for ColoredTreeHorizontalSlice<'_> {
@@ -84,7 +84,7 @@ impl fmt::Display for ColoredTreeHorizontalSlice<'_> {
8484
};
8585
write!(f, "{connector}")?;
8686
}
87-
let prefix = self.color.ansi_prefix(self.ansi_prefixes);
87+
let prefix = self.color.ansi_prefix(self.ls_colors);
8888
let suffix = prefix.suffix();
8989
write!(f, "{skeletal_component}{prefix}{name}{suffix}")
9090
}

‎src/visualizer/methods.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ where
105105
ColoredTreeHorizontalSlice {
106106
slice: tree_horizontal_slice,
107107
color,
108-
ansi_prefixes,
108+
ls_colors: ansi_prefixes,
109109
},
110110
tree_width,
111111
);

‎tests/_utils.rs‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,50 @@ impl SampleWorkspace {
167167
workspace
168168
}
169169

170+
/// Set up a temporary directory for tests.
171+
///
172+
/// This directory has a diverse mix of file kinds: non-empty directories, empty directories,
173+
/// regular files, and symbolic links — multiple of each kind.
174+
pub fn simple_tree_with_diverse_kinds() -> Self {
175+
use std::os::unix::fs::symlink;
176+
let temp = Temp::new_dir().expect("create working directory for sample workspace");
177+
178+
MergeableFileSystemTree::<&str, String>::from(dir! {
179+
"dir-a" => dir! {
180+
"file-a1.txt" => file!("a".repeat(100_000))
181+
"file-a2.txt" => file!("a".repeat(200_000))
182+
"subdir-a" => dir! {
183+
"file-a3.txt" => file!("a".repeat(300_000))
184+
}
185+
}
186+
"dir-b" => dir! {
187+
"file-b1.txt" => file!("a".repeat(150_000))
188+
}
189+
"empty-dir-1" => dir! {}
190+
"empty-dir-2" => dir! {}
191+
"file-root.txt" => file!("a".repeat(50_000))
192+
})
193+
.build(&temp)
194+
.expect("build filesystem tree for diverse-kinds sample workspace");
195+
196+
macro_rules! symlink {
197+
($link_name:literal -> $target:literal) => {
198+
let link_name = $link_name;
199+
let target = $target;
200+
if let Err(error) = symlink(target, temp.join(link_name)) {
201+
panic!(
202+
"Failed to create symbolic link {link_name} pointing to {target}: {error}"
203+
);
204+
}
205+
};
206+
}
207+
208+
symlink!("link-dir" -> "dir-a");
209+
symlink!("link-file.txt" -> "file-root.txt");
210+
211+
SampleWorkspace(temp)
212+
}
213+
170214
/// Set up a temporary directory for tests.
171215
///
172216
/// This directory would have a single file being hard-linked multiple times.

‎tests/usual_cli.rs‎

Lines changed: 50 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -10,18 +10,21 @@ use parallel_disk_usage::{
1010
fs_tree_builder::FsTreeBuilder,
1111
get_size::GetApparentSize,
1212
hardlink::HardlinkIgnorant,
13-
ls_colors::LsColors,
1413
os_string_display::OsStringDisplay,
1514
reporter::{ErrorOnlyReporter, ErrorReport},
16-
visualizer::{BarAlignment, Color, Coloring, ColumnWidthDistribution, Direction, Visualizer},
15+
visualizer::{BarAlignment, ColumnWidthDistribution, Direction, Visualizer},
1716
};
1817
use pipe_trait::Pipe;
1918
use pretty_assertions::assert_eq;
20-
use std::{
21-
collections::HashMap,
22-
path::PathBuf,
23-
process::{Command, Stdio},
19+
use std::process::{Command, Stdio};
20+
21+
#[cfg(unix)]
22+
use parallel_disk_usage::{
23+
ls_colors::LsColors,
24+
visualizer::{Color, Coloring},
2425
};
26+
#[cfg(unix)]
27+
use std::collections::HashMap;
2528

2629
#[cfg(unix)]
2730
use parallel_disk_usage::get_size::{GetBlockCount, GetBlockSize};
@@ -824,7 +827,9 @@ fn colorful_equals_colorless() {
824827
.expect("spawn command with --color=always");
825828
inspect_stderr(&colorful.stderr);
826829
assert!(colorful.status.success(), "pdu exited with non-zero status");
827-
let colorful_stripped = String::from_utf8(colorful.stdout)
830+
let colorful_stripped = colorful
831+
.stdout
832+
.pipe(String::from_utf8)
828833
.expect("UTF-8")
829834
.pipe(strip_ansi_escapes::strip_str)
830835
.trim_end()
@@ -842,37 +847,16 @@ fn colorful_equals_colorless() {
842847
assert_eq!(colorful_stripped, colorless);
843848
}
844849

845-
/// Recursively build a coloring map from a data tree.
846-
fn build_coloring_map(
847-
node: &DataTree<OsStringDisplay, impl parallel_disk_usage::size::Size>,
848-
path: PathBuf,
849-
map: &mut HashMap<OsStringDisplay, Color>,
850-
) {
851-
let node_path = path.join(node.name().as_os_str());
852-
if !node.children().is_empty() {
853-
for child in node.children() {
854-
build_coloring_map(child, node_path.clone(), map);
855-
}
856-
return;
857-
}
858-
let color = if node_path.is_symlink() {
859-
Color::Symlink
860-
} else if node_path.is_dir() {
861-
Color::Directory
862-
} else {
863-
Color::Normal
864-
};
865-
map.insert(node.name().clone(), color);
866-
}
867-
850+
#[cfg(unix)]
868851
#[test]
869852
fn color_always() {
870-
let workspace = SampleWorkspace::default();
853+
let workspace = SampleWorkspace::simple_tree_with_diverse_kinds();
871854

872855
let actual = Command::new(PDU)
873856
.with_current_dir(&workspace)
874857
.with_arg("--color=always")
875858
.with_arg("--total-width=100")
859+
.with_arg("--min-ratio=0")
876860
.pipe(stdio)
877861
.output()
878862
.expect("spawn command with --color=always")
@@ -887,13 +871,45 @@ fn color_always() {
887871
max_depth: 10,
888872
};
889873
let mut data_tree: DataTree<OsStringDisplay, _> = builder.into();
890-
data_tree.par_cull_insignificant_data(0.01);
891874
data_tree.par_sort_by(|left, right| left.size().cmp(&right.size()).reverse());
892875
*data_tree.name_mut() = OsStringDisplay::os_string_from(".");
893876

894877
let ls_colors = LsColors::from_env();
895-
let mut map = HashMap::new();
896-
build_coloring_map(&data_tree, workspace.to_path_buf(), &mut map);
878+
let map = HashMap::from([
879+
(
880+
OsStringDisplay::os_string_from("file-a1.txt"),
881+
Color::Normal,
882+
),
883+
(
884+
OsStringDisplay::os_string_from("file-a2.txt"),
885+
Color::Normal,
886+
),
887+
(
888+
OsStringDisplay::os_string_from("file-a3.txt"),
889+
Color::Normal,
890+
),
891+
(
892+
OsStringDisplay::os_string_from("file-b1.txt"),
893+
Color::Normal,
894+
),
895+
(
896+
OsStringDisplay::os_string_from("file-root.txt"),
897+
Color::Normal,
898+
),
899+
(OsStringDisplay::os_string_from("link-dir"), Color::Symlink),
900+
(
901+
OsStringDisplay::os_string_from("link-file.txt"),
902+
Color::Symlink,
903+
),
904+
(
905+
OsStringDisplay::os_string_from("empty-dir-1"),
906+
Color::Directory,
907+
),
908+
(
909+
OsStringDisplay::os_string_from("empty-dir-2"),
910+
Color::Directory,
911+
),
912+
]);
897913
let coloring = Coloring::new(ls_colors, map);
898914

899915
let visualizer = Visualizer::<OsStringDisplay, _> {

0 commit comments

Comments
 (0)