Replace diff delta binary with flags

Previously the git_diff_delta recorded if the delta was binary.
This replaces that (with no net change in structure size) with
a full set of flags.  The flag values that were already in use
for individual git_diff_file objects are reused for the delta
flags, too (along with renaming those flags to make it clear that
they are used more generally).

This (a) makes things somewhat more consistent (because I was
using a -1 value in the "boolean" binary field to indicate unset,
whereas now I can just use the flags that are easier to understand),
and (b) will make it easier for me to add some additional flags to
the delta object in the future, such as marking the results of a
copy/rename detection or other deltas that might want a special
indicator.

While making this change, I officially moved some of the flags that
were internal only into the private diff header.

This also allowed me to remove a gross hack in rename/copy detect
code where I was overwriting the status field with an internal
value.

include/git2/diff.h

@@ -88,10 +88,9 @@ typedef enum {
 	GIT_DIFF_INCLUDE_UNTRACKED = (1 << 8),
 	/** Include unmodified files in the diff list */
 	GIT_DIFF_INCLUDE_UNMODIFIED = (1 << 9),
-	/** Even with the GIT_DIFF_INCLUDE_UNTRACKED flag, when an untracked
-	 *  directory is found, only a single entry for the directory is added
-	 *  to the diff list; with this flag, all files under the directory will
-	 *  be included, too.
+	/** Even with GIT_DIFF_INCLUDE_UNTRACKED, an entire untracked directory
+	 *  will be marked with only a single entry in the diff list; this flag
+	 *  adds all files under the directory as UNTRACKED entries, too.
 	 */
 	GIT_DIFF_RECURSE_UNTRACKED_DIRS = (1 << 10),
 	/** If the pathspec is set in the diff options, this flags means to
@@ -120,6 +119,11 @@ typedef enum {
 	GIT_DIFF_INCLUDE_TYPECHANGE_TREES  = (1 << 16),
 	/** Ignore file mode changes */
 	GIT_DIFF_IGNORE_FILEMODE = (1 << 17),
+	/** Even with GIT_DIFF_INCLUDE_IGNORED, an entire ignored directory
+	 *  will be marked with only a single entry in the diff list; this flag
+	 *  adds all files under the directory as IGNORED entries, too.
+	 */
+	GIT_DIFF_RECURSE_IGNORED_DIRS = (1 << 10),
 } git_diff_option_t;
 
 /**
@@ -133,20 +137,18 @@ typedef enum {
 typedef struct git_diff_list git_diff_list;
 
 /**
- * Flags for the file object on each side of a diff.
+ * Flags for the delta object and the file objects on each side.
  *
- * Note: most of these flags are just for **internal** consumption by
- * libgit2, but some of them may be interesting to external users.
+ * These flags are used for both the `flags` value of the `git_diff_delta`
+ * and the flags for the `git_diff_file` objects representing the old and
+ * new sides of the delta.  Values outside of this public range should be
+ * considered reserved for internal or future use.
  */
 typedef enum {
-	GIT_DIFF_FILE_VALID_OID  = (1 << 0), /** `oid` value is known correct */
-	GIT_DIFF_FILE_FREE_PATH  = (1 << 1), /** `path` is allocated memory */
-	GIT_DIFF_FILE_BINARY     = (1 << 2), /** should be considered binary data */
-	GIT_DIFF_FILE_NOT_BINARY = (1 << 3), /** should be considered text data */
-	GIT_DIFF_FILE_FREE_DATA  = (1 << 4), /** internal file data is allocated */
-	GIT_DIFF_FILE_UNMAP_DATA = (1 << 5), /** internal file data is mmap'ed */
-	GIT_DIFF_FILE_NO_DATA    = (1 << 6), /** file data should not be loaded */
-} git_diff_file_flag_t;
+	GIT_DIFF_FLAG_BINARY     = (1 << 0), /** file(s) treated as binary data */
+	GIT_DIFF_FLAG_NOT_BINARY = (1 << 1), /** file(s) treated as text data */
+	GIT_DIFF_FLAG_VALID_OID  = (1 << 2), /** `oid` value is known correct */
+} git_diff_flag_t;
 
 /**
  * What type of change is described by a git_diff_delta?
@@ -186,18 +188,17 @@ typedef enum {
  *
  * `size` is the size of the entry in bytes.
  *
- * `flags` is a combination of the `git_diff_file_flag_t` types, but those
- * are largely internal values.
+ * `flags` is a combination of the `git_diff_flag_t` types
  *
  * `mode` is, roughly, the stat() `st_mode` value for the item.  This will
  * be restricted to one of the `git_filemode_t` values.
  */
 typedef struct {
-	git_oid oid;
+	git_oid     oid;
 	const char *path;
-	git_off_t size;
-	unsigned int flags;
-	uint16_t mode;
+	git_off_t   size;
+	uint32_t    flags;
+	uint16_t    mode;
 } git_diff_file;
 
 /**
@@ -219,16 +220,17 @@ typedef struct {
  *
  * Under some circumstances, in the name of efficiency, not all fields will
  * be filled in, but we generally try to fill in as much as possible.  One
- * example is that the "binary" field will not examine file contents if you
- * do not pass in hunk and/or line callbacks to the diff foreach iteration
- * function.  It will just use the git attributes for those files.
+ * example is that the "flags" field may not have either the `BINARY` or the
+ * `NOT_BINARY` flag set to avoid examining file contents if you do not pass
+ * in hunk and/or line callbacks to the diff foreach iteration function.  It
+ * will just use the git attributes for those files.
  */
 typedef struct {
 	git_diff_file old_file;
 	git_diff_file new_file;
 	git_delta_t   status;
-	unsigned int  similarity; /**< for RENAMED and COPIED, value 0-100 */
-	int           binary;
+	uint32_t      similarity; /**< for RENAMED and COPIED, value 0-100 */
+	uint32_t      flags;
 } git_diff_delta;
 
 /**

src/checkout.c

@@ -78,7 +78,7 @@ static int checkout_notify(
 		git_oid_cpy(&wdfile.oid, &wditem->oid);
 		wdfile.path = wditem->path;
 		wdfile.size = wditem->file_size;
-		wdfile.flags = GIT_DIFF_FILE_VALID_OID;
+		wdfile.flags = GIT_DIFF_FLAG_VALID_OID;
 		wdfile.mode = wditem->mode;
 
 		workdir = &wdfile;

src/diff.c

@@ -92,11 +92,11 @@ static int diff_delta__from_one(
 		git_oid_cpy(&delta->new_file.oid, &entry->oid);
 	}
 
-	delta->old_file.flags |= GIT_DIFF_FILE_VALID_OID;
+	delta->old_file.flags |= GIT_DIFF_FLAG_VALID_OID;
 
 	if (delta->status == GIT_DELTA_DELETED ||
 		!git_oid_iszero(&delta->new_file.oid))
-		delta->new_file.flags |= GIT_DIFF_FILE_VALID_OID;
+		delta->new_file.flags |= GIT_DIFF_FLAG_VALID_OID;
 
 	notify_res = diff_notify(diff, delta, matched_pathspec);
 
@@ -142,7 +142,7 @@ static int diff_delta__from_two(
 	git_oid_cpy(&delta->old_file.oid, &old_entry->oid);
 	delta->old_file.size = old_entry->file_size;
 	delta->old_file.mode = old_mode;
-	delta->old_file.flags |= GIT_DIFF_FILE_VALID_OID;
+	delta->old_file.flags |= GIT_DIFF_FLAG_VALID_OID;
 
 	git_oid_cpy(&delta->new_file.oid, &new_entry->oid);
 	delta->new_file.size = new_entry->file_size;
@@ -156,7 +156,7 @@ static int diff_delta__from_two(
 	}
 
 	if (new_oid || !git_oid_iszero(&new_entry->oid))
-		delta->new_file.flags |= GIT_DIFF_FILE_VALID_OID;
+		delta->new_file.flags |= GIT_DIFF_FLAG_VALID_OID;
 
 	notify_res = diff_notify(diff, delta, matched_pathspec);
 

src/diff.h

@@ -28,8 +28,14 @@ enum {
 	GIT_DIFFCAPS_USE_DEV          = (1 << 4), /* use st_dev? */
 };
 
-#define GIT_DELTA__TO_DELETE 10
-#define GIT_DELTA__TO_SPLIT  11
+enum {
+	GIT_DIFF_FLAG__FREE_PATH  = (1 << 7),  /* `path` is allocated memory */
+	GIT_DIFF_FLAG__FREE_DATA  = (1 << 8),  /* internal file data is allocated */
+	GIT_DIFF_FLAG__UNMAP_DATA = (1 << 9),  /* internal file data is mmap'ed */
+	GIT_DIFF_FLAG__NO_DATA    = (1 << 10), /* file data should not be loaded */
+	GIT_DIFF_FLAG__TO_DELETE  = (1 << 11), /* delete entry during rename det. */
+	GIT_DIFF_FLAG__TO_SPLIT   = (1 << 12), /* split entry during rename det. */
+};
 
 struct git_diff_list {
 	git_refcount     rc;

src/diff_tform.c

@@ -315,10 +315,10 @@ static int apply_splits_and_deletes(git_diff_list *diff, size_t expected_size)
 
 	/* build new delta list without TO_DELETE and splitting TO_SPLIT */
 	git_vector_foreach(&diff->deltas, i, delta) {
-		if (delta->status == GIT_DELTA__TO_DELETE)
+		if ((delta->flags & GIT_DIFF_FLAG__TO_DELETE) != 0)
 			continue;
 
-		if (delta->status == GIT_DELTA__TO_SPLIT) {
+		if ((delta->flags & GIT_DIFF_FLAG__TO_SPLIT) != 0) {
 			git_diff_delta *deleted = diff_delta__dup(delta, &diff->pool);
 			if (!deleted)
 				goto on_error;
@@ -326,7 +326,7 @@ static int apply_splits_and_deletes(git_diff_list *diff, size_t expected_size)
 			deleted->status = GIT_DELTA_DELETED;
 			memset(&deleted->new_file, 0, sizeof(deleted->new_file));
 			deleted->new_file.path = deleted->old_file.path;
-			deleted->new_file.flags |= GIT_DIFF_FILE_VALID_OID;
+			deleted->new_file.flags |= GIT_DIFF_FLAG_VALID_OID;
 
 			if (git_vector_insert(&onto, deleted) < 0)
 				goto on_error;
@@ -334,7 +334,7 @@ static int apply_splits_and_deletes(git_diff_list *diff, size_t expected_size)
 			delta->status = GIT_DELTA_ADDED;
 			memset(&delta->old_file, 0, sizeof(delta->old_file));
 			delta->old_file.path = delta->new_file.path;
-			delta->old_file.flags |= GIT_DIFF_FILE_VALID_OID;
+			delta->old_file.flags |= GIT_DIFF_FLAG_VALID_OID;
 		}
 
 		if (git_vector_insert(&onto, delta) < 0)
@@ -343,7 +343,7 @@ static int apply_splits_and_deletes(git_diff_list *diff, size_t expected_size)
 
 	/* cannot return an error past this point */
 	git_vector_foreach(&diff->deltas, i, delta)
-		if (delta->status == GIT_DELTA__TO_DELETE)
+		if ((delta->flags & GIT_DIFF_FLAG__TO_DELETE) != 0)
 			git__free(delta);
 
 	/* swap new delta list into place */
@@ -411,7 +411,7 @@ int git_diff_find_similar(
 			/* calc_similarity(NULL, &from->old_file, from->new_file); */
 
 			if (similarity < opts.break_rewrite_threshold) {
-				from->status = GIT_DELTA__TO_SPLIT;
+				from->flags |= GIT_DIFF_FLAG__TO_SPLIT;
 				num_changes++;
 			}
 		}
@@ -502,7 +502,7 @@ int git_diff_find_similar(
 			to->status = GIT_DELTA_RENAMED;
 			memcpy(&to->old_file, &from->old_file, sizeof(to->old_file));
 
-			from->status = GIT_DELTA__TO_DELETE;
+			from->flags |= GIT_DIFF_FLAG__TO_DELETE;
 			num_changes++;
 
 			continue;
@@ -522,7 +522,7 @@ int git_diff_find_similar(
 				from->status = GIT_DELTA_ADDED;
 				memset(&from->old_file, 0, sizeof(from->old_file));
 				from->old_file.path = to->old_file.path;
-				from->old_file.flags |= GIT_DIFF_FILE_VALID_OID;
+				from->old_file.flags |= GIT_DIFF_FLAG_VALID_OID;
 
 				continue;
 			}

tests-clar/diff/diff_helpers.c

@@ -32,7 +32,7 @@ int diff_file_cb(
 
 	e->files++;
 
-	if (delta->binary)
+	if ((delta->flags & GIT_DIFF_FLAG_BINARY) != 0)
 		e->files_binary++;
 
 	cl_assert(delta->status <= GIT_DELTA_TYPECHANGE);
@@ -126,7 +126,8 @@ int diff_foreach_via_iterator(
 
 		/* if there are no changes, then the patch will be NULL */
 		if (!patch) {
-			cl_assert(delta->status == GIT_DELTA_UNMODIFIED || delta->binary == 1);
+			cl_assert(delta->status == GIT_DELTA_UNMODIFIED ||
+					  (delta->flags & GIT_DIFF_FLAG_BINARY) != 0);
 			continue;
 		}
 

tests-clar/diff/diffiter.c

@@ -152,8 +152,8 @@ void test_diff_diffiter__max_size_threshold(void)
 		file_count++;
 		hunk_count += (int)git_diff_patch_num_hunks(patch);
 
-		assert(delta->binary == 0 || delta->binary == 1);
-		binary_count += delta->binary;
+		assert((delta->flags & (GIT_DIFF_FLAG_BINARY|GIT_DIFF_FLAG_NOT_BINARY)) != 0);
+		binary_count += ((delta->flags & GIT_DIFF_FLAG_BINARY) != 0);
 
 		git_diff_patch_free(patch);
 	}
@@ -185,8 +185,8 @@ void test_diff_diffiter__max_size_threshold(void)
 		file_count++;
 		hunk_count += (int)git_diff_patch_num_hunks(patch);
 
-		assert(delta->binary == 0 || delta->binary == 1);
-		binary_count += delta->binary;
+		assert((delta->flags & (GIT_DIFF_FLAG_BINARY|GIT_DIFF_FLAG_NOT_BINARY)) != 0);
+		binary_count += ((delta->flags & GIT_DIFF_FLAG_BINARY) != 0);
 
 		git_diff_patch_free(patch);
 	}