From 0363eea494602e3002f45f1790eaff3435151949 Mon Sep 17 00:00:00 2001
From: Pieter de Bie <pdebie@ai.rug.nl>
Date: Thu, 17 Sep 2009 23:29:06 +0200
Subject: [PATCH] HistoryView: don't load in commit information in a separate
 thread anymore

I've seen this hang or crash a few times, so I hope this works better.
Instead of running a task in a separate thread, we just let it go through
the run loop and catch it when the task is done.

This ruins the second subview in the history view, but I don't think
anybody ever used that, so I'm going to remove it.
---
 PBGitCommit.m                 | 14 ++------------
 PBWebHistoryController.m      | 32 ++++++++++++++++++++++++++++++++
 html/views/history/history.js | 22 ++++++++++++++++------
 3 files changed, 50 insertions(+), 18 deletions(-)

diff --git a/PBGitCommit.m b/PBGitCommit.m
index b883c95..9ebe3b5 100644
--- a/PBGitCommit.m
+++ b/PBGitCommit.m
@@ -66,20 +66,10 @@
 	return str;
 }
 
-// NOTE: This method should remain threadsafe, as we load it in async
-// from the web view.
+// FIXME: Remove this method once it's unused.
 - (NSString*) details
 {
-	if (details != nil)
-		return details;
-
-	NSMutableArray *arguments = [NSMutableArray arrayWithObjects:@"show", @"--pretty=raw", @"-M", @"--no-color", [self realSha], nil];
-	if (![PBGitDefaults showWhitespaceDifferences])
-		[arguments insertObject:@"-w" atIndex:1];
-
-	details = [self.repository outputForArguments:arguments];
-
-	return details;
+	return @"";
 }
 
 - (NSString *) patch
diff --git a/PBWebHistoryController.m b/PBWebHistoryController.m
index 35979a4..1934b73 100644
--- a/PBWebHistoryController.m
+++ b/PBWebHistoryController.m
@@ -7,6 +7,7 @@
 //
 
 #import "PBWebHistoryController.h"
+#import "PBGitDefaults.h"
 
 @implementation PBWebHistoryController
 
@@ -49,6 +50,37 @@
 
 	NSArray *arguments = [NSArray arrayWithObjects:content, [[[historyController repository] headRef] simpleRef], nil];
 	[[self script] callWebScriptMethod:@"loadCommit" withArguments: arguments];
+
+	// Now we load the extended details. We used to do this in a separate thread,
+	// but this caused some funny behaviour because NSTask's and NSThread's don't really
+	// like each other. Instead, just do it async.
+
+	NSMutableArray *taskArguments = [NSMutableArray arrayWithObjects:@"show", @"--pretty=raw", @"-M", @"--no-color", currentSha, nil];
+	if (![PBGitDefaults showWhitespaceDifferences])
+		[taskArguments insertObject:@"-w" atIndex:1];
+
+	NSFileHandle *handle = [repository handleForArguments:taskArguments];
+	NSNotificationCenter *nc = [NSNotificationCenter defaultCenter];
+	// Remove notification, in case we have another one running
+	[nc removeObserver:self];
+	[nc addObserver:self selector:@selector(commitDetailsLoaded:) name:NSFileHandleReadToEndOfFileCompletionNotification object:handle]; 
+	[handle readToEndOfFileInBackgroundAndNotify];
+}
+
+- (void)commitDetailsLoaded:(NSNotification *)notification
+{
+	NSData *data = [[notification userInfo] valueForKey:NSFileHandleNotificationDataItem];
+	if (!data)
+		return;
+
+	NSString *details = [[NSString alloc] initWithData:data encoding:NSUTF8StringEncoding];
+	if (!details)
+		details = [[NSString alloc] initWithData:data encoding:NSISOLatin1StringEncoding];
+
+	if (!details)
+		return;
+
+	[[view windowScriptObject] callWebScriptMethod:@"loadCommitDetails" withArguments:[NSArray arrayWithObject:details]];
 }
 
 - (void) selectCommit: (NSString*) sha
diff --git a/html/views/history/history.js b/html/views/history/history.js
index df2a2b0..47d72b7 100644
--- a/html/views/history/history.js
+++ b/html/views/history/history.js
@@ -1,4 +1,7 @@
 var commit;
+
+// Create a new Commit object
+// obj: PBGitCommit object
 var Commit = function(obj) {
 	this.object = obj;
 
@@ -11,8 +14,9 @@ var Commit = function(obj) {
 	// TODO:
 	// this.author_date instant
 
-	// This all needs to be async
-	this.loadedRaw = function(details) {
+	// This can be called later with the output of
+	// 'git show' to fill in missing commit details (such as a diff)
+	this.parseDetails = function(details) {
 		this.raw = details;
 
 		var diffStart = this.raw.indexOf("\ndiff ");
@@ -138,6 +142,7 @@ var selectCommit = function(a) {
 	Controller.selectCommit_(a);
 }
 
+// Relead only refs
 var reload = function() {
 	$("notification").style.display = "none";
 	commit.reloadRefs();
@@ -159,10 +164,11 @@ var showRefs = function() {
 
 var loadCommit = function(commitObject, currentRef) {
 	// These are only the things we can do instantly.
-	// Other information will be loaded later by loadExtendedCommit
+	// Other information will be loaded later by loadCommitDetails,
+	// Which will be called from the controller once
+	// the commit details are in.
+
 	commit = new Commit(commitObject);
-	Controller.callSelector_onObject_callBack_("details", commitObject,
-		function(data) { commit.loadedRaw(data); loadExtendedCommit(commit); });
 	commit.currentRef = currentRef;
 
 	notify("Loading commit…", 0);
@@ -199,6 +205,8 @@ var loadCommit = function(commitObject, currentRef) {
 }
 
 var showDiff = function() {
+
+	// Callback for the diff highlighter. Used to generate a filelist
 	var newfile = function(name1, name2, id, mode_change, old_mode, new_mode) {
 		var button = document.createElement("div");
 		var p = document.createElement("p");
@@ -270,8 +278,10 @@ var enableFeatures = function()
 	enableFeature("gravatar", $("gravatar"))
 }
 
-var loadExtendedCommit = function(commit)
+var loadCommitDetails = function(data)
 {
+	commit.parseDetails(data);
+
 	var formatEmail = function(name, email) {
 		return email ? name + " &lt;<a href='mailto:" + email + "'>" + email + "</a>&gt;" : name;
 	}
-- 
2.11.4.GIT