WIP

Author: MitMaro

src/application.rs

@@ -11,9 +11,9 @@ use crate::{
 	Args,
 	Exit,
 	config::{Config, DiffIgnoreWhitespaceSetting},
-	diff,
+	diff::{self, CommitDiffLoader, CommitDiffLoaderOptions},
 	display::Display,
-	git::{CommitDiffLoader, CommitDiffLoaderOptions, ConfigLoader, open_repository_from_env},
+	git::{ConfigLoader, open_repository_from_env},
 	help::build_help,
 	input::{Event, EventHandler, EventReaderFn, KeyBindings, StandardEvent},
 	module::{self, ExitStatus, ModuleHandler, State},

src/diff.rs

@@ -1,6 +1,17 @@
 mod action;
+mod commit_diff;
+mod commit_diff_loader;
+mod commit_diff_loader_options;
 mod state;
 mod thread;
 mod update_handler;
 
-pub(crate) use self::{action::Action, state::State, thread::Thread, update_handler::UpdateHandlerFn};
+pub(crate) use self::{
+	action::Action,
+	commit_diff::CommitDiff,
+	commit_diff_loader::CommitDiffLoader,
+	commit_diff_loader_options::CommitDiffLoaderOptions,
+	state::{LoadStatus, State},
+	thread::Thread,
+	update_handler::UpdateHandlerFn,
+};

src/git/commit_diff.rs renamed to src/diff/commit_diff.rs

@@ -1,20 +1,8 @@
 use crate::git::{Commit, FileStatus};
 
-#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
-pub(crate) enum LoadedStatus {
-	New,
-	StartQuickDiff,
-	QuickDiff(usize, usize),
-	CompleteQuickDiff,
-	Diff(usize, usize),
-	DiffComplete,
-	Canceled,
-}
-
 /// Represents a commit with a diff
 #[derive(Debug)]
 pub(crate) struct CommitDiff {
-	loaded_status: LoadedStatus,
 	commit: Commit,
 	parent: Option<Commit>,
 	file_statuses: Vec<FileStatus>,
@@ -33,7 +21,6 @@ impl CommitDiff {
 		number_deletions: usize,
 	) -> Self {
 		CommitDiff {
-			loaded_status: LoadedStatus::New,
 			commit,
 			parent,
 			file_statuses,
@@ -80,14 +67,8 @@ impl CommitDiff {
 		self.number_deletions
 	}
 
-	#[must_use]
-	pub(crate) fn status(&self) -> LoadedStatus {
-		self.loaded_status
-	}
-
 	/// Reset the diff back to an empty state
 	pub(crate) fn reset(&mut self, commit: Commit, parent: Option<Commit>) {
-		self.loaded_status = LoadedStatus::New;
 		self.commit = commit;
 		self.parent = parent;
 		self.file_statuses.clear();
@@ -109,10 +90,6 @@ impl CommitDiff {
 		self.number_insertions = number_insertions;
 		self.number_deletions = number_deletions;
 	}
-
-	pub(crate) fn update_status(&mut self, status: LoadedStatus) {
-		self.loaded_status = status;
-	}
 }
 
 #[cfg(test)]

src/git/commit_diff_loader.rs renamed to src/diff/commit_diff_loader.rs

@@ -8,26 +8,25 @@ use std::{
 use git2::{Diff, DiffFindOptions, DiffOptions, Repository};
 use parking_lot::{Mutex, RwLock};
 
-use crate::git::{
-	Commit,
-	CommitDiff,
-	CommitDiffLoaderOptions,
-	Delta,
-	DiffLine,
-	FileMode,
-	FileStatus,
-	FileStatusBuilder,
-	GitError,
-	Status,
-	commit_diff::LoadedStatus,
+use crate::{
+	diff::{CommitDiff, CommitDiffLoaderOptions, LoadStatus},
+	git::{Commit, Delta, DiffLine, FileMode, FileStatus, FileStatusBuilder, GitError, Status},
 };
 
 static UNKNOWN_PATH: LazyLock<PathBuf> = LazyLock::new(|| PathBuf::from("unknown"));
-const LONG_DIFF_TIME: Duration = Duration::from_millis(100);
 
-pub(crate) trait UpdateHandlerFn: Fn() -> bool + Sync + Send {}
+pub(crate) trait DiffUpdateHandlerFn: Fn(LoadStatus) -> bool + Sync + Send {}
 
-impl<FN: Fn() -> bool + Sync + Send> UpdateHandlerFn for FN {}
+impl<FN: Fn(LoadStatus) -> bool + Sync + Send> DiffUpdateHandlerFn for FN {}
+
+fn create_status_update(quick: bool, processed_files: usize, total_files: usize) -> LoadStatus {
+	if quick {
+		LoadStatus::QuickDiff(processed_files, total_files)
+	}
+	else {
+		LoadStatus::Diff(processed_files, total_files)
+	}
+}
 
 pub(crate) struct CommitDiffLoader {
 	config: CommitDiffLoaderOptions,
@@ -91,7 +90,7 @@ impl CommitDiffLoader {
 	fn diff<'r>(
 		repository: &'r Repository,
 		config: &CommitDiffLoaderOptions,
-		commit: &git2::Commit,
+		commit: &git2::Commit<'_>,
 		diff_options: &mut DiffOptions,
 	) -> Result<Diff<'r>, GitError> {
 		_ = diff_options
@@ -118,12 +117,13 @@ impl CommitDiffLoader {
 		.map_err(|e| GitError::DiffLoad { cause: e })
 	}
 
-	pub(crate) fn load_diff(&mut self, hash: &str, update_notifier: impl UpdateHandlerFn) -> Result<(), GitError> {
-		let mut commit_diff = self.commit_diff.write();
-		commit_diff.reset(self.find_commit(hash)?, self.find_first_parent(hash)?);
-		drop(commit_diff);
-		if update_notifier() {
-			return Ok(());
+	pub(crate) fn load_diff(&mut self, hash: &str, update_notifier: impl DiffUpdateHandlerFn) -> Result<(), GitError> {
+		{
+			let mut commit_diff = self.commit_diff.write();
+			commit_diff.reset(self.find_commit(hash)?, self.find_first_parent(hash)?);
+			if update_notifier(LoadStatus::New) {
+				return Ok(());
+			}
 		}
 
 		// TODO, duplicated with find_commit function
@@ -144,20 +144,23 @@ impl CommitDiffLoader {
 
 		let stats = diff.stats().map_err(|e| GitError::DiffLoad { cause: e })?;
 
-		std::thread::sleep(std::time::Duration::from_millis(3_000));
-		// when a diff contains a lot of track files, collecting the diff information can take upwards
-		// of a minute. This performs a quicker diff, that does not detect copies and renames against
-		// unmodified files.
+		// std::thread::sleep(std::time::Duration::from_millis(3_000));
+		// when a diff contains a lot of untracked files, collecting the diff information can take
+		// upwards of a minute. This performs a quicker diff, that does not detect copies and
+		// renames against unmodified files.
 		if self.config.copies && stats.files_changed() > 1 {
 			// self.config.quick_diff_threshold {
-			let mut diff_options = DiffOptions::new();
-			let quick_diff = Self::diff(&self.repository, &self.config, &commit, &mut diff_options)?;
-			self.collect(&quick_diff, LoadedStatus::Partial)?;
-			if update_notifier() {
+			self.collect(
+				&Self::diff(&self.repository, &self.config, &commit, &mut DiffOptions::new())?,
+				&update_notifier,
+				true,
+			)?;
+
+			if update_notifier(LoadStatus::CompleteQuickDiff) {
 				return Ok(());
 			}
 		}
-		std::thread::sleep(std::time::Duration::from_millis(3_000));
+		// std::thread::sleep(std::time::Duration::from_millis(3_000));
 
 		let mut diff_find_options = DiffFindOptions::new();
 		_ = diff_find_options
@@ -170,9 +173,10 @@ impl CommitDiffLoader {
 
 		diff.find_similar(Some(&mut diff_find_options))
 			.map_err(|e| GitError::DiffLoad { cause: e })?;
-		self.collect(&diff, LoadedStatus::Complete)?;
+		self.collect(&diff, &update_notifier, false)?;
 
-		if update_notifier() {
+		if update_notifier(LoadStatus::DiffComplete) {
+			_ = update_notifier(LoadStatus::Canceled);
 			return Ok(());
 		}
 		Ok(())
@@ -183,7 +187,8 @@ impl CommitDiffLoader {
 	pub(crate) fn collect(
 		&self,
 		diff: &Diff<'_>,
-		update_handler: UpdateHandlerFn,
+		update_handler: &impl DiffUpdateHandlerFn,
+		quick: bool,
 	) -> Result<(), GitError> {
 		let file_stats_builder = Mutex::new(FileStatusBuilder::new());
 		let mut unmodified_file_count: usize = 0;
@@ -192,6 +197,11 @@ impl CommitDiffLoader {
 		let stats = diff.stats().map_err(|e| GitError::DiffLoad { cause: e })?;
 		let total_files_changed = stats.files_changed();
 
+		if update_handler(create_status_update(quick, 0, total_files_changed)) {
+			return Ok(());
+		}
+		let mut time = Instant::now();
+
 		let collect_result = diff.foreach(
 			&mut |diff_delta, _| {
 				change_count += 1;
@@ -200,11 +210,10 @@ impl CommitDiffLoader {
 					unmodified_file_count += 1;
 					return true;
 				}
-				if change_count % 10 == 0 {
-					let mut commit_diff = self.commit_diff.write();
-					commit_diff.update_status(LoadedStatus::Partial(change_count, total_files_changed))
-					// early exit
-					if update_handler() {
+				if time.elapsed() > Duration::from_millis(10) {
+					time = Instant::now();
+					// std::thread::sleep(std::time::Duration::from_millis(500));
+					if update_handler(create_status_update(quick, change_count, total_files_changed)) {
 						return false;
 					}
 				}
@@ -247,23 +256,16 @@ impl CommitDiffLoader {
 
 		// error caused by early return
 		if collect_result.is_err() {
-			commit_diff.update_status(LoadedStatus::Canceled);
-			update_handler();
+			_ = update_handler(LoadStatus::Canceled);
 			return Ok(());
 		}
 
 		let number_files_changed = total_files_changed - unmodified_file_count;
 		let number_insertions = stats.insertions();
 		let number_deletions = stats.deletions();
 
-		eprintln!("Changes: {} {}", change_count, stats.files_changed());
 		let fsb = file_stats_builder.into_inner();
-		commit_diff.update(
-			fsb.build(),
-			number_files_changed,
-			number_insertions,
-			number_deletions,
-		);
+		commit_diff.update(fsb.build(), number_files_changed, number_insertions, number_deletions);
 		Ok(())
 	}
 }

src/git/commit_diff_loader_options.rs renamed to src/diff/commit_diff_loader_options.rs

@@ -1,19 +1,26 @@
-use std::{
-	sync::{
-		Arc,
-		atomic::{AtomicBool, Ordering},
-	},
-	time::Duration,
+use std::sync::{
+	Arc,
+	atomic::{AtomicBool, Ordering},
 };
 
 use parking_lot::RwLock;
 
-use crate::{diff::Action, git::CommitDiff};
+use crate::diff::{Action, CommitDiff};
 
-const RECEIVE_TIMEOUT: Duration = Duration::from_millis(500);
+// TODO move
+#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
+pub(crate) enum LoadStatus {
+	New,
+	QuickDiff(usize, usize),
+	CompleteQuickDiff,
+	Diff(usize, usize),
+	DiffComplete,
+	Canceled,
+}
 
 #[derive(Clone, Debug)]
 pub(crate) struct State {
+	load_status: Arc<RwLock<LoadStatus>>,
 	diff: Arc<RwLock<CommitDiff>>,
 	ended: Arc<AtomicBool>,
 	paused: Arc<AtomicBool>,
@@ -25,6 +32,7 @@ impl State {
 	pub(crate) fn new(diff: Arc<RwLock<CommitDiff>>) -> Self {
 		let (update_sender, update_receiver) = crossbeam_channel::unbounded();
 		Self {
+			load_status: Arc::new(RwLock::new(LoadStatus::New)),
 			diff,
 			ended: Arc::new(AtomicBool::from(false)),
 			paused: Arc::new(AtomicBool::from(false)),
@@ -33,6 +41,10 @@ impl State {
 		}
 	}
 
+	pub(crate) fn load_status(&self) -> Arc<RwLock<LoadStatus>> {
+		Arc::clone(&self.load_status)
+	}
+
 	pub(crate) fn diff(&self) -> Arc<RwLock<CommitDiff>> {
 		Arc::clone(&self.diff)
 	}

src/diff/state.rs

@@ -8,18 +8,12 @@ use captur::capture;
 use parking_lot::Mutex;
 
 use crate::{
-	diff::{Action, State, UpdateHandlerFn},
-	git::CommitDiffLoader,
+	diff::{Action, CommitDiffLoader, State, UpdateHandlerFn, state::LoadStatus},
 	runtime::{Installer, RuntimeError, Threadable},
 };
 
 pub(crate) const THREAD_NAME: &str = "diff";
 const MINIMUM_PAUSE_RATE: Duration = Duration::from_millis(50);
-const LONG_DIFF_TIME: Duration = Duration::from_millis(100);
-
-fn should_process_intermediate_diff(time: Instant) -> bool {
-	time.saturating_duration_since(Instant::now()) >= LONG_DIFF_TIME
-}
 
 #[derive(Debug)]
 pub(crate) struct Thread<UpdateHandler: UpdateHandlerFn> {
@@ -36,10 +30,11 @@ where UpdateHandler: UpdateHandlerFn + 'static
 
 		installer.spawn(THREAD_NAME, |notifier| {
 			let update_handler = Arc::clone(&self.update_handler);
-			let mut commit_diff_loader = Arc::clone(&self.commit_diff_loader);
+			let commit_diff_loader = Arc::clone(&self.commit_diff_loader);
 			move || {
 				capture!(notifier, state);
 
+				let load_status = state.load_status();
 				notifier.wait();
 				let mut time = Instant::now();
 
@@ -59,9 +54,10 @@ where UpdateHandler: UpdateHandlerFn + 'static
 					match msg {
 						Action::Load(hash) => {
 							time = Instant::now();
-							if let Err(e) = loader.load_diff(hash.as_str(), || {
+							if let Err(e) = loader.load_diff(hash.as_str(), |s: LoadStatus| {
 								eprintln!("Update!");
-
+								let mut ls = load_status.write();
+								*ls = s;
 								update_handler();
 								// TODO what about paused
 								return state.is_ended();

src/diff/thread.rs

@@ -9,9 +9,6 @@
 //! performance, they should only be used in test code.
 
 mod commit;
-mod commit_diff;
-mod commit_diff_loader;
-mod commit_diff_loader_options;
 mod config_loader;
 mod delta;
 mod diff_line;
@@ -29,9 +26,6 @@ pub(crate) use git2::{Config, ErrorCode};
 
 pub(crate) use self::{
 	commit::Commit,
-	commit_diff::{CommitDiff, LoadedStatus},
-	commit_diff_loader::CommitDiffLoader,
-	commit_diff_loader_options::CommitDiffLoaderOptions,
 	config_loader::{ConfigLoader, open_repository_from_env},
 	delta::Delta,
 	diff_line::DiffLine,