Fix start/end handlers and tweak Finally callback

It's useful to have the success of the state in the Finally handler that is
called in-thread (the ITask-based Finally handlers already get this information),
so add that to the callback.

In order for the API not to be confusing, make the task affinity parameter
mandatory on the Finally overloads that run on a separate thread.

Fix the HookupHandlers method to use OnStart/Finally so that the callbacks get
called on the same thread and not spin up new threads just to set flags. They're
already running on non-main threads and doing very simple things so this should
be fine.

Author: shana

src/GitHub.Api/Git/RepositoryManager.cs

@@ -183,7 +183,7 @@ public ITask CommitAllFiles(string message, string body)
                 .AddAll()
                 .Then(GitClient.Commit(message, body));
 
-            return HookupHandlers(task, true, true);
+            return HookupHandlers(task, true);
         }
 
         public ITask CommitFiles(List<string> files, string message, string body)
@@ -192,79 +192,79 @@ public ITask CommitFiles(List<string> files, string message, string body)
                 .Add(files)
                 .Then(GitClient.Commit(message, body));
 
-            return HookupHandlers(task, true, true);
+            return HookupHandlers(task, true);
         }
 
         public ITask Fetch(string remote)
         {
             var task = GitClient.Fetch(remote);
-            return HookupHandlers(task, true, false);
+            return HookupHandlers(task, false);
         }
 
         public ITask Pull(string remote, string branch)
         {
             var task = GitClient.Pull(remote, branch);
-            return HookupHandlers(task, true, true);
+            return HookupHandlers(task, true);
         }
 
         public ITask Push(string remote, string branch)
         {
             var task = GitClient.Push(remote, branch);
-            return HookupHandlers(task, true, false);
+            return HookupHandlers(task, false);
         }
 
         public ITask Revert(string changeset)
         {
             var task = GitClient.Revert(changeset);
-            return HookupHandlers(task, true, true);
+            return HookupHandlers(task, true);
         }
 
         public ITask RemoteAdd(string remote, string url)
         {
             var task = GitClient.RemoteAdd(remote, url);
-            return HookupHandlers(task, true, false);
+            return HookupHandlers(task, false);
         }
 
         public ITask RemoteRemove(string remote)
         {
             var task = GitClient.RemoteRemove(remote);
-            return HookupHandlers(task, true, false);
+            return HookupHandlers(task, false);
         }
 
         public ITask RemoteChange(string remote, string url)
         {
             var task = GitClient.RemoteChange(remote, url);
-            return HookupHandlers(task, true, false);
+            return HookupHandlers(task, false);
         }
 
         public ITask SwitchBranch(string branch)
         {
             var task = GitClient.SwitchBranch(branch);
-            return HookupHandlers(task, true, true);
+            return HookupHandlers(task, true);
         }
 
         public ITask DeleteBranch(string branch, bool deleteUnmerged = false)
         {
             var task = GitClient.DeleteBranch(branch, deleteUnmerged);
-            return HookupHandlers(task, true, false);
+            return HookupHandlers(task, false);
         }
 
         public ITask CreateBranch(string branch, string baseBranch)
         {
             var task = GitClient.CreateBranch(branch, baseBranch);
-            return HookupHandlers(task, true, false);
+            return HookupHandlers(task, false);
         }
 
         public ITask LockFile(string file)
         {
             var task = GitClient.Lock(file);
-            return HookupHandlers(task, true, false).Then(UpdateLocks);
+            return HookupHandlers(task, false).Then(UpdateLocks);
         }
 
         public ITask UnlockFile(string file, bool force)
         {
             var task = GitClient.Unlock(file, force);
-            return HookupHandlers(task, true, false).Then(UpdateLocks);
+            return HookupHandlers(task, false).Then(UpdateLocks);
         }
 
         public void UpdateGitLog()
@@ -278,7 +278,7 @@ public void UpdateGitLog()
                         GitLogUpdated?.Invoke(logEntries);
                     }
                 });
-            task = HookupHandlers(task, false, false);
+            task = HookupHandlers(task, false);
             task.Start();
         }
 
@@ -293,7 +293,7 @@ public void UpdateGitStatus()
                         GitStatusUpdated?.Invoke(status);
                     }
                 });
-            task = HookupHandlers(task, true, false);
+            task = HookupHandlers(task, false);
             task.Start();
         }
 
@@ -331,13 +331,21 @@ public ITask DiscardChanges(GitStatusEntry[] gitStatusEntries)
                     if (itemsToRevert.Any())
                     {
                         gitDiscardTask = GitClient.Discard(itemsToRevert);
-                        task.Then(gitDiscardTask);
+                        task
+                            .Then(gitDiscardTask)
+                            // we're appending a new continuation, we need to reset the finally handler
+                            // so it runs after the discard task
+                            .Finally(s =>
+                            {
+                                watcher.Start();
+                                isBusy = false;
+                            });
                     }
                 }
                 , () => gitStatusEntries);
 
 
-            return HookupHandlers(task, true, true);
+            return HookupHandlers(task, true);
         }
 
         public void UpdateGitAheadBehindStatus()
@@ -351,16 +359,15 @@ public void UpdateGitAheadBehindStatus()
                 var name = configBranch.Value.Name;
                 var trackingName = configBranch.Value.IsTracking ? configBranch.Value.Remote.Value.Name + "/" + name : "[None]";
 
-                var task = GitClient
-                    .AheadBehindStatus(name, trackingName)
+                var task = GitClient.AheadBehindStatus(name, trackingName)
                     .Then((success, status) =>
                     {
                         if (success)
                         {
                             GitAheadBehindStatusUpdated?.Invoke(status);
                         }
                     });
-                task = HookupHandlers(task, true, false);
+                task = HookupHandlers(task, false);
                 task.Start();
             }
             else
@@ -371,51 +378,55 @@ public void UpdateGitAheadBehindStatus()
 
         public void UpdateLocks()
         {
-            var task = GitClient.ListLocks(false);
-            HookupHandlers(task, false, false);
-            task.Then((success, locks) =>
-            {
-                if (success)
+            GitClient.ListLocks(false)
+                .Finally((success, locks) =>
                 {
-                    GitLocksUpdated?.Invoke(locks);
-                }
-            }).Start();
+                    if (success)
+                    {
+                        GitLocksUpdated?.Invoke(locks);
+                    }
+                })
+                .Start();
         }
 
-        private ITask HookupHandlers(ITask task, bool isExclusive, bool filesystemChangesExpected)
+        private ITask<T> HookupHandlers<T>(ITask<T> task, bool filesystemChangesExpected)
         {
-            return new ActionTask(token, () => {
-                    if (isExclusive)
-                    {
-                        Logger.Trace("Starting Operation - Setting Busy Flag");
-                        IsBusy = true;
-                    }
+            return (ITask<T>)HookupHandlers((ITask)task, filesystemChangesExpected);
+        }
 
-                    if (filesystemChangesExpected)
-                    {
-                        Logger.Trace("Starting Operation - Disable Watcher");
-                        watcher.Stop();
-                    }
-                })
-                .Then(task)
-                .Finally((success, exception) => {
-                    if (filesystemChangesExpected)
-                    {
-                        Logger.Trace("Ended Operation - Enable Watcher");
-                        watcher.Start();
-                    }
+        private ITask HookupHandlers(ITask task, bool filesystemChangesExpected)
+        {
+            var isExclusive = task.IsChainExclusive();
+            task.GetTopOfChain().OnStart += t =>
+            {
+                if (t.Affinity == TaskAffinity.Exclusive)
+                {
+                    Logger.Trace("Starting Operation - Setting Busy Flag");
+                    IsBusy = true;
+                }
 
-                    if (isExclusive)
-                    {
-                        Logger.Trace("Ended Operation - Clearing Busy Flag");
-                        IsBusy = false;
-                    }
+                if (filesystemChangesExpected)
+                {
+                    Logger.Trace("Starting Operation - Disable Watcher");
+                    watcher.Stop();
+                }
+            };
 
-                    if (!success)
-                    {
-                        throw exception;
-                    }
-                });
+            task.Finally(success =>
+            {
+                if (filesystemChangesExpected)
+                {
+                    Logger.Trace("Ended Operation - Enable Watcher");
+                    watcher.Start();
+                }
+
+                if (isExclusive)
+                {
+                    Logger.Trace("Ended Operation - Clearing Busy Flag");
+                    IsBusy = false;
+                }
+            });
+            return task;
         }
 
         private void SetupWatcher()