Skip to content

Commit

Permalink
Sort by path before outputting directory diff so that the order is al…
Browse files Browse the repository at this point in the history
…ways the same (#593)

* Sort by path before outputting directory diff so that the order is always the same

* Added feature flag --sort-paths (DFT_SORT_PATHS) to enable/disable sorting paths when diffing directory (default disabled)
  • Loading branch information
milandamen authored Nov 20, 2023
1 parent 1ec868e commit a8d6253
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 19 deletions.
52 changes: 33 additions & 19 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,8 @@ fn main() {
options::FileArgument::NamedPath(rhs_path),
) if lhs_path.is_dir() && rhs_path.is_dir() => {
let encountered_changes = encountered_changes.clone();

// Diffs in parallel when iterating this iterator.
let diff_iter = diff_directories(
lhs_path,
rhs_path,
Expand All @@ -263,32 +265,44 @@ fn main() {

display::json::print_directory(results);
} else {
// We want to diff files in the directory in
// parallel, but print the results serially (to
// prevent display interleaving).
// https://github.com/rayon-rs/rayon/issues/210#issuecomment-551319338
let (send, recv) = std::sync::mpsc::sync_channel(1);

let encountered_changes = encountered_changes.clone();
let print_options = display_options.clone();

let printing_thread = std::thread::spawn(move || {
for diff_result in recv.into_iter() {
print_diff_result(&print_options, &diff_result);
if display_options.sort_paths {
let mut result: Vec<DiffResult> = diff_iter.collect();
result.sort_unstable_by(|a, b| a.display_path.cmp(&b.display_path));
for diff_result in result {
print_diff_result(&display_options, &diff_result);

if diff_result.has_reportable_change() {
encountered_changes.store(true, Ordering::Relaxed);
}
}
});
} else {
// We want to diff files in the directory in
// parallel, but print the results serially (to
// prevent display interleaving).
// https://github.com/rayon-rs/rayon/issues/210#issuecomment-551319338
let (send, recv) = std::sync::mpsc::sync_channel(1);

let encountered_changes = encountered_changes.clone();
let print_options = display_options.clone();

let printing_thread = std::thread::spawn(move || {
for diff_result in recv.into_iter() {
print_diff_result(&print_options, &diff_result);

if diff_result.has_reportable_change() {
encountered_changes.store(true, Ordering::Relaxed);
}
}
});

diff_iter
.try_for_each_with(send, |s, diff_result| s.send(diff_result))
.expect("Receiver should be connected");
diff_iter
.try_for_each_with(send, |s, diff_result| s.send(diff_result))
.expect("Receiver should be connected");

printing_thread
.join()
.expect("Printing thread should not panic");
printing_thread
.join()
.expect("Printing thread should not panic");
}
}
}
_ => {
Expand Down
14 changes: 14 additions & 0 deletions src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ pub(crate) struct DisplayOptions {
pub(crate) display_width: usize,
pub(crate) num_context_lines: u32,
pub(crate) syntax_highlight: bool,
pub(crate) sort_paths: bool,
}

impl Default for DisplayOptions {
Expand All @@ -55,6 +56,7 @@ impl Default for DisplayOptions {
display_width: 80,
num_context_lines: 3,
syntax_highlight: true,
sort_paths: false,
}
}
}
Expand Down Expand Up @@ -287,6 +289,14 @@ When multiple overrides are specified, the first matching override wins."))
.hide(true)
.allow_invalid_utf8(true),
)
.arg(
Arg::new("sort-paths").long("sort-paths")
.value_name("on/off")
.env("DFT_SORT_PATHS")
.possible_values(["on", "off"])
.default_value("off")
.help("Enable or disable sorting of paths when displaying results of directory diff.")
)
.arg_required_else_help(true)
}

Expand Down Expand Up @@ -574,6 +584,8 @@ pub(crate) fn parse_args() -> Mode {

let syntax_highlight = matches.value_of("syntax-highlight") == Some("on");

let sort_paths = matches.value_of("sort-paths") == Some("on");

let graph_limit = matches
.value_of("graph-limit")
.expect("Always present as we've given clap a default")
Expand Down Expand Up @@ -667,6 +679,7 @@ pub(crate) fn parse_args() -> Mode {
display_width,
num_context_lines,
syntax_highlight,
sort_paths,
};

let display_path = path.to_string_lossy().to_string();
Expand Down Expand Up @@ -703,6 +716,7 @@ pub(crate) fn parse_args() -> Mode {
display_width,
num_context_lines,
syntax_highlight,
sort_paths,
};

Mode::Diff {
Expand Down

0 comments on commit a8d6253

Please sign in to comment.