Refactor HookupHandlers in order accurately wrap tasks

[StanleyGoldman]

Fixes #424

HookupHandlers was refactored to use the task system in order to accurately wrap operations performed in RepositoryManager.
Tasks that are exclusive or batches of exclusive operations should toggle the busy flag.
Tasks that directly cause changes to the file system should stop the watcher.

RepositoryManager Method	Tasks Invoked	Exclusive operations	File system changes
CommitAllFiles	GitAddTask, GitCommitTask	👍
CommitFiles	GitAddTask, GitCommitTask	👍
Log	GitLogTask
Status	GitStatusTask	👍
Fetch	GitFetchTask	👍
Pull	GitPullTask	👍	👍
Push	GitPushTask	👍
Revert	GitRevertTask	👍	👍
RemoteAdd	GitRemoteAddTask	👍
RemoteRemove	GitRemoteRemoveTask	👍
RemoteChange	GitRemoteChangeTask	👍
SwitchBranch	GitSwitchBranchesTask	👍	👍
DeleteBranch	GitBranchDeleteTask	👍
CreateBranch	GitBranchCreateTask	👍
ListLocks	GitListLocksTask
LockFile	GitLockTask	👍
UnlockFile	GitUnlockTask	👍

[jcansdale]

I've looked at this PR, but don't feel I understand the ITask infrastructure well enough to give it the green light. I'll leave giving the thumbs up to someone more familiar with this.

[shana]

@jcansdale The ITask infrastructure is an equivalent of the TPL on .NET with custom task schedulers for running tasks on specific threads as needed. Then, ThenInUI, methods schedule the next task as a continuation (literally ContinueWith) of the previous task in a specific thread scheduler, Finally, FinallyInUI methods schedule continuations regardless of final task state (success or exceptions end up always in Finally methods).

The task classes shown here are implementations of ITask and shell out to git to run commands.

HookupHandlers in this case adds extra continuations to the end of the task chain (Then methods always return the task that was passed in), in this case stopping the watcher and setting a busy state when the task starts, and resuming the watcher/toggling the busy state when it ends (tasks have Start and End events for this purpose.

Which actually all brings me to a question: @StanleyGoldman feels like you're wanting to stop the watcher before Add runs and start the watcher after Commit finishes? Right now it only does the handlers for the commit task, not for the whole chain...

[shana]

Hmmmm this will only hook up handlers for the Commit task, not the AddAll task (which will also fire events). Is that what we want? If we want to handle the whole chain of tasks in one go, we can hook up to the Start of the AddAll task and do a Finally on the Commit one (so that if the AddAll fails we can still reset the busy state and restart the watcher - if the AddAll fails the Commit will never start so handlers in HookupHandlers for it will never run.

[StanleyGoldman]

I added a test that demonstrates what @shana was saying in her previous comment.

Unity/src/tests/TaskSystemIntegrationTests/Tests.cs

Lines 111 to 147 in 134558c

	[Test]
	public async Task ProcessOnStartOnEndTaskOrder()
	{
	var values = new List<string>();
	string process1Value = null;
	string process2Value = null;
	var process1Task = new FirstNonNullLineProcessTask(Token, TestApp, @"-s 100 -d process1")
	.Configure(ProcessManager, true).Then((b, s) => {
	process1Value = s;
	values.Add(s);
	});
	var process2Task = new FirstNonNullLineProcessTask(Token, TestApp, @"-s 100 -d process2")
	.Configure(ProcessManager, true).Then((b, s) => {
	process2Value = s;
	values.Add(s);
	});
	var combinedTask = process1Task
	.Then(process2Task);
	combinedTask.OnStart += task => {
	values.Add("OnStart");
	};
	combinedTask.OnEnd += task => {
	values.Add("OnEnd");
	};
	await combinedTask
	.StartAsAsync();
	Assert.AreEqual(process1Value, "process1");
	Assert.AreEqual(process2Value, "process2");
	Assert.True(values.SequenceEqual(new []{ "process1", "OnStart", "process2", "OnEnd" }));
	}

I have to fix the code.

Changed significantly

[drguthals]

Seems like a very clean approach.