Add ignore_submodules to diff options

This adds correct support for an equivalent to --ignore-submodules
in diff, where an actual ignore value can be passed to diff to
override the per submodule settings in the configuration.

This required tweaking the constants for ignore values so that
zero would not be used and could represent an unset option to the
diff.  This was an opportunity to move the submodule values into
include/git2/types.h and to rename the poorly named DEFAULT values
for ignore and update constants to RESET instead.

Now the GIT_DIFF_IGNORE_SUBMODULES flag is exactly the same as
setting the ignore_submodules option to GIT_SUBMODULE_IGNORE_ALL
(which is actually a minor change from the old behavior in that
submodules will now be treated as UNMODIFIED deltas instead of
being left out totally - if you set GIT_DIFF_INCLUDE_UNMODIFIED).

This includes tests for the various new settings.

Author: arrbee

include/git2/diff.h

@@ -78,7 +78,7 @@ typedef enum {
 	GIT_DIFF_IGNORE_WHITESPACE_CHANGE = (1 << 3),
 	/** Ignore whitespace at end of line */
 	GIT_DIFF_IGNORE_WHITESPACE_EOL = (1 << 4),
-	/** Exclude submodules from the diff completely */
+	/** Treat all submodules as unmodified */
 	GIT_DIFF_IGNORE_SUBMODULES = (1 << 5),
 	/** Use the "patience diff" algorithm (currently unimplemented) */
 	GIT_DIFF_PATIENCE = (1 << 6),
@@ -314,6 +314,8 @@ typedef int (*git_diff_notify_cb)(
  * - `notify_cb` is an optional callback function, notifying the consumer of
  *   which files are being examined as the diff is generated
  * - `notify_payload` is the payload data to pass to the `notify_cb` function
+ * - `ignore_submodules` overrides the submodule ignore setting for all
+ *   submodules in the diff.
  */
 typedef struct {
 	unsigned int version;      /**< version for the struct */
@@ -326,6 +328,7 @@ typedef struct {
 	git_off_t max_size;        /**< defaults to 512MB */
 	git_diff_notify_cb notify_cb;
 	void *notify_payload;
+	git_submodule_ignore_t ignore_submodules; /** << submodule ignore rule */
 } git_diff_options;
 
 #define GIT_DIFF_OPTIONS_VERSION 1

include/git2/submodule.h

@@ -14,51 +14,18 @@
 /**
  * @file git2/submodule.h
  * @brief Git submodule management utilities
- * @defgroup git_submodule Git submodule management routines
- * @ingroup Git
- * @{
- */
-GIT_BEGIN_DECL
-
-/**
- * Opaque structure representing a submodule.
  *
  * Submodule support in libgit2 builds a list of known submodules and keeps
  * it in the repository.  The list is built from the .gitmodules file, the
  * .git/config file, the index, and the HEAD tree.  Items in the working
  * directory that look like submodules (i.e. a git repo) but are not
  * mentioned in those places won't be tracked.
- */
-typedef struct git_submodule git_submodule;
-
-/**
- * Values that could be specified for the update rule of a submodule.
  *
- * Use the DEFAULT value if you have altered the update value via
- * `git_submodule_set_update()` and wish to reset to the original default.
- */
-typedef enum {
-	GIT_SUBMODULE_UPDATE_DEFAULT = -1,
-	GIT_SUBMODULE_UPDATE_CHECKOUT = 0,
-	GIT_SUBMODULE_UPDATE_REBASE = 1,
-	GIT_SUBMODULE_UPDATE_MERGE = 2,
-	GIT_SUBMODULE_UPDATE_NONE = 3
-} git_submodule_update_t;
-
-/**
- * Values that could be specified for how closely to examine the
- * working directory when getting submodule status.
- *
- * Use the DEFUALT value if you have altered the ignore value via
- * `git_submodule_set_ignore()` and wish to reset to the original value.
+ * @defgroup git_submodule Git submodule management routines
+ * @ingroup Git
+ * @{
  */
-typedef enum {
-	GIT_SUBMODULE_IGNORE_DEFAULT = -1,  /* reset to default */
-	GIT_SUBMODULE_IGNORE_NONE = 0,      /* any change or untracked == dirty */
-	GIT_SUBMODULE_IGNORE_UNTRACKED = 1, /* dirty if tracked files change */
-	GIT_SUBMODULE_IGNORE_DIRTY = 2,     /* only dirty if HEAD moved */
-	GIT_SUBMODULE_IGNORE_ALL = 3        /* never dirty */
-} git_submodule_ignore_t;
+GIT_BEGIN_DECL
 
 /**
  * Return codes for submodule status.
@@ -377,9 +344,9 @@ GIT_EXTERN(git_submodule_ignore_t) git_submodule_ignore(
  * submodule is in memory.  You should call `git_submodule_save()` if you
  * want to persist the new ignore role.
  *
- * Calling this again with GIT_SUBMODULE_IGNORE_DEFAULT or calling
- * `git_submodule_reload()` will revert the rule to the value that was in the
- * original config.
+ * Calling this again with GIT_SUBMODULE_IGNORE_RESET or calling
+ * `git_submodule_reload()` will revert the rule to the value that was in
+ * the original config.
  *
  * @return old value for ignore
  */
@@ -399,9 +366,9 @@ GIT_EXTERN(git_submodule_update_t) git_submodule_update(
  * This sets the update rule in memory for the submodule.  You should call
  * `git_submodule_save()` if you want to persist the new update rule.
  *
- * Calling this again with GIT_SUBMODULE_UPDATE_DEFAULT or calling
- * `git_submodule_reload()` will revert the rule to the value that was in the
- * original config.
+ * Calling this again with GIT_SUBMODULE_UPDATE_RESET or calling
+ * `git_submodule_reload()` will revert the rule to the value that was in
+ * the original config.
  *
  * @return old value for update
  */

include/git2/types.h

@@ -229,6 +229,40 @@ typedef struct git_transfer_progress {
  */
 typedef int (*git_transfer_progress_callback)(const git_transfer_progress *stats, void *payload);
 
+/**
+ * Opaque structure representing a submodule.
+ */
+typedef struct git_submodule git_submodule;
+
+/**
+ * Values that could be specified for the update rule of a submodule.
+ *
+ * Use the RESET value if you have altered the in-memory update value via
+ * `git_submodule_set_update()` and wish to reset to the original default.
+ */
+typedef enum {
+	GIT_SUBMODULE_UPDATE_RESET = -1,
+	GIT_SUBMODULE_UPDATE_CHECKOUT = 1,
+	GIT_SUBMODULE_UPDATE_REBASE = 2,
+	GIT_SUBMODULE_UPDATE_MERGE = 3,
+	GIT_SUBMODULE_UPDATE_NONE = 4
+} git_submodule_update_t;
+
+/**
+ * Values that could be specified for how closely to examine the
+ * working directory when getting submodule status.
+ *
+ * Use the RESET value if you have altered the in-memory ignore value via
+ * `git_submodule_set_ignore()` and wish to reset to the original value.
+ */
+typedef enum {
+	GIT_SUBMODULE_IGNORE_RESET = -1,    /* reset to on-disk value */
+	GIT_SUBMODULE_IGNORE_NONE = 1,      /* any change or untracked == dirty */
+	GIT_SUBMODULE_IGNORE_UNTRACKED = 2, /* dirty if tracked files change */
+	GIT_SUBMODULE_IGNORE_DIRTY = 3,     /* only dirty if HEAD moved */
+	GIT_SUBMODULE_IGNORE_ALL = 4        /* never dirty */
+} git_submodule_ignore_t;
+
 /** @} */
 GIT_END_DECL
 

src/diff.c

@@ -78,10 +78,6 @@ static int diff_delta__from_one(
 		DIFF_FLAG_ISNT_SET(diff, GIT_DIFF_INCLUDE_UNTRACKED))
 		return 0;
 
-	if (entry->mode == GIT_FILEMODE_COMMIT &&
-		DIFF_FLAG_IS_SET(diff, GIT_DIFF_IGNORE_SUBMODULES))
-		return 0;
-
 	if (!git_pathspec__match(
 			&diff->pathspec, entry->path,
 			DIFF_FLAG_IS_SET(diff, GIT_DIFF_DISABLE_PATHSPEC_MATCH),
@@ -141,11 +137,6 @@ static int diff_delta__from_two(
 		DIFF_FLAG_ISNT_SET(diff, GIT_DIFF_INCLUDE_UNMODIFIED))
 		return 0;
 
-	if (old_entry->mode == GIT_FILEMODE_COMMIT &&
-		new_entry->mode == GIT_FILEMODE_COMMIT &&
-		DIFF_FLAG_IS_SET(diff, GIT_DIFF_IGNORE_SUBMODULES))
-		return 0;
-
 	if (DIFF_FLAG_IS_SET(diff, GIT_DIFF_REVERSE)) {
 		uint32_t temp_mode = old_mode;
 		const git_index_entry *temp_entry = old_entry;
@@ -431,8 +422,18 @@ static int diff_list_apply_options(
 	if (!opts) {
 		diff->opts.context_lines = config_int(cfg, "diff.context", 3);
 
-		if (config_bool(cfg, "diff.ignoreSubmodules", 0))
-			diff->opts.flags |= GIT_DIFF_IGNORE_SUBMODULES;
+		/* add other defaults here */
+	}
+
+	/* if ignore_submodules not explicitly set, check diff config */
+	if (diff->opts.ignore_submodules <= 0) {
+		const char *str;
+
+		if (git_config_get_string(&str , cfg, "diff.ignoreSubmodules") < 0)
+			giterr_clear();
+		else if (str != NULL &&
+			git_submodule_parse_ignore(&diff->opts.ignore_submodules, str) < 0)
+			giterr_clear();
 	}
 
 	/* if either prefix is not set, figure out appropriate value */
@@ -596,36 +597,44 @@ static int maybe_modified_submodule(
 	int error = 0;
 	git_submodule *sub;
 	unsigned int sm_status = 0;
+	git_submodule_ignore_t ign = diff->opts.ignore_submodules;
 
 	*status = GIT_DELTA_UNMODIFIED;
 
-	if (!DIFF_FLAG_IS_SET(diff, GIT_DIFF_IGNORE_SUBMODULES) &&
-		!(error = git_submodule_lookup(
-			  &sub, diff->repo, info->nitem->path)) &&
-		git_submodule_ignore(sub) != GIT_SUBMODULE_IGNORE_ALL &&
-		!(error = git_submodule__status(
-			  &sm_status, NULL, NULL, found_oid, sub,
-			  GIT_SUBMODULE_IGNORE_DEFAULT)))
-	{
-		/* check IS_WD_UNMODIFIED because this case is only used
-		 * when the new side of the diff is the working directory
-		 */
-		if (!GIT_SUBMODULE_STATUS_IS_WD_UNMODIFIED(sm_status))
-			*status = GIT_DELTA_MODIFIED;
+	if (DIFF_FLAG_IS_SET(diff, GIT_DIFF_IGNORE_SUBMODULES) ||
+		ign == GIT_SUBMODULE_IGNORE_ALL)
+		return 0;
 
-		/* now that we have a HEAD OID, check if HEAD moved */
-		if ((sm_status & GIT_SUBMODULE_STATUS_IN_WD) != 0 &&
-			!git_oid_equal(&info->oitem->oid, found_oid))
-			*status = GIT_DELTA_MODIFIED;
-	}
+	if ((error = git_submodule_lookup(
+			&sub, diff->repo, info->nitem->path)) < 0) {
 
-	/* GIT_EEXISTS means a dir with .git in it was found - ignore it */
-	if (error == GIT_EEXISTS) {
-		giterr_clear();
-		error = 0;
+		/* GIT_EEXISTS means dir with .git in it was found - ignore it */
+		if (error == GIT_EEXISTS) {
+			giterr_clear();
+			error = 0;
+		}
+		return error;
 	}
 
-	return error;
+	if (ign <= 0 && git_submodule_ignore(sub) == GIT_SUBMODULE_IGNORE_ALL)
+		return 0;
+
+	if ((error = git_submodule__status(
+			&sm_status, NULL, NULL, found_oid, sub, ign)) < 0)
+		return error;
+
+	/* check IS_WD_UNMODIFIED because this case is only used
+	 * when the new side of the diff is the working directory
+	 */
+	if (!GIT_SUBMODULE_STATUS_IS_WD_UNMODIFIED(sm_status))
+		*status = GIT_DELTA_MODIFIED;
+
+	/* now that we have a HEAD OID, check if HEAD moved */
+	if ((sm_status & GIT_SUBMODULE_STATUS_IN_WD) != 0 &&
+		!git_oid_equal(&info->oitem->oid, found_oid))
+		*status = GIT_DELTA_MODIFIED;
+
+	return 0;
 }
 
 static int maybe_modified(