Fix bugs in new diff patch code

This fixes all the bugs in the new diff patch code.  The only
really interesting one is that when we merge two diffs, we now
have to actually exclude diff delta records that are not supposed
to be tracked, as opposed to before where they could be included
because they would be skipped silently by `git_diff_foreach()`.
Other than that, there are just minor errors.

3 files changed, 74 insertions, 46 deletions

diff --git a/src/diff.c b/src/diff.c
index 7c8e2a9bb..7ee919b5a 100644
--- a/src/diff.c
+++ b/src/diff.c
@@ -807,6 +807,28 @@ on_error:
 	return -1;
 }
 
+
+bool git_diff_delta__should_skip(
+	git_diff_options *opts, git_diff_delta *delta)
+{
+	uint32_t flags = opts ? opts->flags : 0;
+
+	if (delta->status == GIT_DELTA_UNMODIFIED &&
+		(flags & GIT_DIFF_INCLUDE_UNMODIFIED) == 0)
+		return true;
+
+	if (delta->status == GIT_DELTA_IGNORED &&
+		(flags & GIT_DIFF_INCLUDE_IGNORED) == 0)
+		return true;
+
+	if (delta->status == GIT_DELTA_UNTRACKED &&
+		(flags & GIT_DIFF_INCLUDE_UNTRACKED) == 0)
+		return true;
+
+	return false;
+}
+
+
 int git_diff_merge(
 	git_diff_list *onto,
 	const git_diff_list *from)
@@ -843,6 +865,14 @@ int git_diff_merge(
 			j++;
 		}
 
+		/* the ignore rules for the target may not match the source
+		 * or the result of a merged delta could be skippable...
+		 */
+		if (git_diff_delta__should_skip(&onto->opts, delta)) {
+			git__free(delta);
+			continue;
+		}
+
 		if ((error = !delta ? -1 : git_vector_insert(&onto_new, delta)) < 0)
 			break;
 	}
diff --git a/src/diff.h b/src/diff.h
index 862c33c1b..64fe00928 100644
--- a/src/diff.h
+++ b/src/diff.h
@@ -50,5 +50,8 @@ extern void git_diff__cleanup_modes(
 
 extern void git_diff_list_addref(git_diff_list *diff);
 
+extern bool git_diff_delta__should_skip(
+	git_diff_options *opts, git_diff_delta *delta);
+
 #endif
 
diff --git a/src/diff_output.c b/src/diff_output.c
index b84b0c2a4..141859742 100644
--- a/src/diff_output.c
+++ b/src/diff_output.c
@@ -51,26 +51,6 @@ static int parse_hunk_header(git_diff_range *range, const char *header)
 	return 0;
 }
 
-static bool diff_delta_should_skip(
-	diff_context *ctxt, git_diff_delta *delta)
-{
-	uint32_t flags = ctxt->opts ? ctxt->opts->flags : 0;
-
-	if (delta->status == GIT_DELTA_UNMODIFIED &&
-		(flags & GIT_DIFF_INCLUDE_UNMODIFIED) == 0)
-		return true;
-
-	if (delta->status == GIT_DELTA_IGNORED &&
-		(flags & GIT_DIFF_INCLUDE_IGNORED) == 0)
-		return true;
-
-	if (delta->status == GIT_DELTA_UNTRACKED &&
-		(flags & GIT_DIFF_INCLUDE_UNTRACKED) == 0)
-		return true;
-
-	return false;
-}
-
 #define KNOWN_BINARY_FLAGS (GIT_DIFF_FILE_BINARY|GIT_DIFF_FILE_NOT_BINARY)
 #define NOT_BINARY_FLAGS   (GIT_DIFF_FILE_NOT_BINARY|GIT_DIFF_FILE_NO_DATA)
 
@@ -411,7 +391,7 @@ static void diff_context_init(
 	setup_xdiff_options(ctxt->opts, &ctxt->xdiff_config, &ctxt->xdiff_params);
 }
 
-static bool diff_delta_file_callback(
+static int diff_delta_file_callback(
 	diff_context *ctxt, git_diff_delta *delta, size_t idx)
 {
 	float progress;
@@ -419,18 +399,18 @@ static bool diff_delta_file_callback(
 	if (!ctxt->file_cb)
 		return 0;
 
-	progress = (float)idx / ctxt->diff->deltas.length;
+	progress = ctxt->diff ? ((float)idx / ctxt->diff->deltas.length) : 1.0f;
 
 	if (ctxt->file_cb(ctxt->cb_data, delta, progress) != 0)
-		return GIT_EUSER;
+		ctxt->cb_error = GIT_EUSER;
 
-	return 0;
+	return ctxt->cb_error;
 }
 
 static void diff_patch_init(
 	diff_context *ctxt, git_diff_patch *patch)
 {
-	memset(patch, 0, sizeof(*patch));
+	memset(patch, 0, sizeof(git_diff_patch));
 
 	patch->diff = ctxt->diff;
 	patch->ctxt = ctxt;
@@ -454,7 +434,7 @@ static git_diff_patch *diff_patch_alloc(
 
 	git_diff_list_addref(patch->diff);
 
-	GIT_REFCOUNT_INC(&patch);
+	GIT_REFCOUNT_INC(patch);
 
 	patch->delta = delta;
 	patch->flags = GIT_DIFF_PATCH_ALLOCATED;
@@ -836,6 +816,8 @@ static int diff_patch_line_cb(
 		}
 	}
 
+	hunk->line_count++;
+
 	return 0;
 }
 
@@ -847,7 +829,7 @@ int git_diff_foreach(
 	git_diff_hunk_fn hunk_cb,
 	git_diff_data_fn data_cb)
 {
-	int error;
+	int error = 0;
 	diff_context ctxt;
 	size_t idx;
 	git_diff_patch patch;
@@ -861,14 +843,20 @@ int git_diff_foreach(
 	git_vector_foreach(&diff->deltas, idx, patch.delta) {
 
 		/* check flags against patch status */
-		if (diff_delta_should_skip(&ctxt, patch.delta))
+		if (git_diff_delta__should_skip(ctxt.opts, patch.delta))
 			continue;
 
-		if (!(error = diff_patch_load(&ctxt, &patch)) &&
-			!(error = diff_delta_file_callback(&ctxt, patch.delta, idx)))
-			error = diff_patch_generate(&ctxt, &patch);
+		if (!(error = diff_patch_load(&ctxt, &patch))) {
 
-		diff_patch_unload(&patch);
+			/* invoke file callback */
+			error = diff_delta_file_callback(&ctxt, patch.delta, idx);
+
+			/* generate diffs and invoke hunk and line callbacks */
+			if (!error)
+				error = diff_patch_generate(&ctxt, &patch);
+
+			diff_patch_unload(&patch);
+		}
 
 		if (error < 0)
 			break;
@@ -899,25 +887,32 @@ static char pick_suffix(int mode)
 		return ' ';
 }
 
+char git_diff_status_char(git_delta_t status)
+{
+	char code;
+
+	switch (status) {
+	case GIT_DELTA_ADDED:     code = 'A'; break;
+	case GIT_DELTA_DELETED:   code = 'D'; break;
+	case GIT_DELTA_MODIFIED:  code = 'M'; break;
+	case GIT_DELTA_RENAMED:   code = 'R'; break;
+	case GIT_DELTA_COPIED:    code = 'C'; break;
+	case GIT_DELTA_IGNORED:   code = 'I'; break;
+	case GIT_DELTA_UNTRACKED: code = '?'; break;
+	default:                  code = ' '; break;
+	}
+
+	return code;
+}
+
 static int print_compact(void *data, git_diff_delta *delta, float progress)
 {
 	diff_print_info *pi = data;
-	char code, old_suffix, new_suffix;
+	char old_suffix, new_suffix, code = git_diff_status_char(delta->status);
 
 	GIT_UNUSED(progress);
 
-	switch (delta->status) {
-	case GIT_DELTA_ADDED: code = 'A'; break;
-	case GIT_DELTA_DELETED: code = 'D'; break;
-	case GIT_DELTA_MODIFIED: code = 'M'; break;
-	case GIT_DELTA_RENAMED: code = 'R'; break;
-	case GIT_DELTA_COPIED: code = 'C'; break;
-	case GIT_DELTA_IGNORED: code = 'I'; break;
-	case GIT_DELTA_UNTRACKED: code = '?'; break;
-	default: code = 0;
-	}
-
-	if (!code)
+	if (code == ' ')
 		return 0;
 
 	old_suffix = pick_suffix(delta->old_file.mode);
@@ -1288,7 +1283,7 @@ int git_diff_get_patch(
 		&ctxt, diff, diff->repo, &diff->opts,
 		NULL, NULL, diff_patch_hunk_cb, diff_patch_line_cb);
 
-	if (diff_delta_should_skip(&ctxt, delta))
+	if (git_diff_delta__should_skip(ctxt.opts, delta))
 		return 0;
 
 	patch = diff_patch_alloc(&ctxt, delta);