From a8d6253509532068c24ea4a2dc0347d50bae0eae Mon Sep 17 00:00:00 2001 From: Milan Damen Date: Mon, 20 Nov 2023 10:24:35 +0000 Subject: [PATCH] Sort by path before outputting directory diff so that the order is always 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) --- src/main.rs | 52 ++++++++++++++++++++++++++++++++------------------ src/options.rs | 14 ++++++++++++++ 2 files changed, 47 insertions(+), 19 deletions(-) diff --git a/src/main.rs b/src/main.rs index 73b5505279..00be858d69 100644 --- a/src/main.rs +++ b/src/main.rs @@ -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, @@ -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 = 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"); + } } } _ => { diff --git a/src/options.rs b/src/options.rs index 6ed719f1ac..329dfce820 100644 --- a/src/options.rs +++ b/src/options.rs @@ -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 { @@ -55,6 +56,7 @@ impl Default for DisplayOptions { display_width: 80, num_context_lines: 3, syntax_highlight: true, + sort_paths: false, } } } @@ -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) } @@ -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") @@ -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(); @@ -703,6 +716,7 @@ pub(crate) fn parse_args() -> Mode { display_width, num_context_lines, syntax_highlight, + sort_paths, }; Mode::Diff {