stash_apply: provide its own options structure

4 files changed, 128 insertions, 62 deletions

diff --git a/include/git2/stash.h b/include/git2/stash.h
index bb17933bc..1f773dcb0 100644
--- a/include/git2/stash.h
+++ b/include/git2/stash.h
@@ -70,14 +70,50 @@ GIT_EXTERN(int) git_stash_save(
 	const char *message,
 	unsigned int flags);
 
+/** Stash application flags. */
 typedef enum {
-	GIT_APPLY_DEFAULT = 0,
+	GIT_STASH_APPLY_DEFAULT = 0,
 
 	/* Try to reinstate not only the working tree's changes,
-	 * but also the index's ones.
+	 * but also the index's changes.
 	 */
-	GIT_APPLY_REINSTATE_INDEX = (1 << 0),
-} git_apply_flags;
+	GIT_STASH_APPLY_REINSTATE_INDEX = (1 << 0),
+} git_stash_apply_flags;
+
+/** Stash application options structure.
+ *
+ * Initialize with the `GIT_STASH_APPLY_OPTIONS_INIT` macro to set
+ * sensible defaults; for example:
+ *
+ *		git_stash_apply_options opts = GIT_STASH_APPLY_OPTIONS_INIT;
+ */
+typedef struct git_stash_apply_options {
+	unsigned int version;
+
+	/** See `git_stash_apply_flags_t`, above. */
+	git_stash_apply_flags flags;
+
+	/** Options to use when writing files to the working directory. */
+	git_checkout_options checkout_options;
+} git_stash_apply_options;
+
+#define GIT_STASH_APPLY_OPTIONS_VERSION 1
+#define GIT_STASH_APPLY_OPTIONS_INIT { \
+	GIT_STASH_APPLY_OPTIONS_VERSION, \
+	GIT_STASH_APPLY_DEFAULT, \
+	GIT_CHECKOUT_OPTIONS_INIT }
+
+/**
+ * Initializes a `git_stash_apply_options` with default values. Equivalent to
+ * creating an instance with GIT_STASH_APPLY_OPTIONS_INIT.
+ *
+ * @param opts the `git_stash_apply_options` instance to initialize.
+ * @param version the version of the struct; you should pass
+ *        `GIT_STASH_APPLY_OPTIONS_INIT` here.
+ * @return Zero on success; -1 on failure.
+ */
+int git_stash_apply_init_options(
+	git_stash_apply_options *opts, unsigned int version);
 
 /**
  * Apply a single stashed state from the stash list.
@@ -89,17 +125,17 @@ typedef enum {
  * ignored files and there is a conflict when applying the modified files,
  * then those files will remain in the working directory.
  *
- * If passing the GIT_APPLY_REINSTATE_INDEX flag and there would be conflicts
- * when reinstating the index, the function will return GIT_EMERGECONFLICT
- * and both the working directory and index will be left unmodified.
+ * If passing the GIT_STASH_APPLY_REINSTATE_INDEX flag and there would be
+ * conflicts when reinstating the index, the function will return
+ * GIT_EMERGECONFLICT and both the working directory and index will be left
+ * unmodified.
+ *
+ * Note that a minimum checkout strategy of `GIT_CHECKOUT_SAFE` is implied.
  *
  * @param repo The owning repository.
  * @param index The position within the stash list. 0 points to the
  *              most recent stashed state.
- * @param checkout_options Options to control how files are checked out.
- *                         A minimum strategy of `GIT_CHECKOUT_SAFE` is
- *                         implied.
- * @param flags Flags to control the applying process. (see GIT_APPLY_* above)
+ * @param options Options to control how stashes are applied.
  *
  * @return 0 on success, GIT_ENOTFOUND if there's no stashed state for the
  *         given index, GIT_EMERGECONFLICT if changes exist in the working
@@ -108,8 +144,7 @@ typedef enum {
 GIT_EXTERN(int) git_stash_apply(
 	git_repository *repo,
 	size_t index,
-	const git_checkout_options *checkout_options,
-	unsigned int flags);
+	const git_stash_apply_options *options);
 
 /**
  * This is a callback function you can provide to iterate over all the
@@ -169,8 +204,7 @@ GIT_EXTERN(int) git_stash_drop(
  * @param repo The owning repository.
  * @param index The position within the stash list. 0 points to the
  *              most recent stashed state.
- * @param checkout_options Options to control how files are checked out
- * @param flags Flags to control the applying process. (see GIT_APPLY_* above)
+ * @param options Options to control how stashes are applied.
  *
  * @return 0 on success, GIT_ENOTFOUND if there's no stashed state for the given
  * index, or error code. (see git_stash_apply() above for details)
@@ -178,8 +212,7 @@ GIT_EXTERN(int) git_stash_drop(
 GIT_EXTERN(int) git_stash_pop(
 	git_repository *repo,
 	size_t index,
-	const git_checkout_options *checkout_options,
-	unsigned int flags);
+	const git_stash_apply_options *options);
 
 /** @} */
 GIT_END_DECL
diff --git a/src/stash.c b/src/stash.c
index 3f60ee017..dade06f94 100644
--- a/src/stash.c
+++ b/src/stash.c
@@ -673,34 +673,40 @@ done:
 	return error;
 }
 
-static void normalize_checkout_options(
-	git_checkout_options *checkout_opts,
-	const git_checkout_options *given_checkout_opts)
+static void normalize_apply_options(
+	git_stash_apply_options *opts,
+	const git_stash_apply_options *given_apply_opts)
 {	
-	if (given_checkout_opts != NULL) {
-		memcpy(checkout_opts, given_checkout_opts, sizeof(git_checkout_options));
+	if (given_apply_opts != NULL) {
+		memcpy(opts, given_apply_opts, sizeof(git_stash_apply_options));
 	} else {
-		git_checkout_options default_checkout_opts = GIT_CHECKOUT_OPTIONS_INIT;
-		memcpy(checkout_opts, &default_checkout_opts, sizeof(git_checkout_options));
+		git_stash_apply_options default_apply_opts = GIT_STASH_APPLY_OPTIONS_INIT;
+		memcpy(opts, &default_apply_opts, sizeof(git_stash_apply_options));
 	}
 
-	if ((checkout_opts->checkout_strategy & (GIT_CHECKOUT_SAFE | GIT_CHECKOUT_FORCE)) == 0)
-		checkout_opts->checkout_strategy = GIT_CHECKOUT_SAFE;
+	if ((opts->checkout_options.checkout_strategy & (GIT_CHECKOUT_SAFE | GIT_CHECKOUT_FORCE)) == 0)
+		opts->checkout_options.checkout_strategy = GIT_CHECKOUT_SAFE;
 
-	if (!checkout_opts->our_label)
-		checkout_opts->our_label = "Updated upstream";
+	if (!opts->checkout_options.our_label)
+		opts->checkout_options.our_label = "Updated upstream";
 
-	if (!checkout_opts->their_label)
-		checkout_opts->their_label = "Stashed changes";
+	if (!opts->checkout_options.their_label)
+		opts->checkout_options.their_label = "Stashed changes";
+}
+
+int git_stash_apply_init_options(git_stash_apply_options *opts, unsigned int version)
+{
+	GIT_INIT_STRUCTURE_FROM_TEMPLATE(
+		opts, version, git_stash_apply_options, GIT_STASH_APPLY_OPTIONS_INIT);
+	return 0;
 }
 
 int git_stash_apply(
 	git_repository *repo,
 	size_t index,
-	const git_checkout_options *given_checkout_opts,
-	unsigned int flags)
+	const git_stash_apply_options *given_opts)
 {
-	git_checkout_options checkout_opts;
+	git_stash_apply_options opts;
 	unsigned int checkout_strategy;
 	git_commit *stash_commit = NULL;
 	git_tree *stash_tree = NULL;
@@ -714,8 +720,10 @@ int git_stash_apply(
 	git_index *untracked_index = NULL;
 	int error;
 
-	normalize_checkout_options(&checkout_opts, given_checkout_opts);
-	checkout_strategy = checkout_opts.checkout_strategy;
+	GITERR_CHECK_VERSION(given_opts, GIT_STASH_APPLY_OPTIONS_VERSION, "git_stash_apply_options");
+
+	normalize_apply_options(&opts, given_opts);
+	checkout_strategy = opts.checkout_options.checkout_strategy;
 
 	/* Retrieve commit corresponding to the given stash */
 	if ((error = retrieve_stash_commit(&stash_commit, repo, index)) < 0)
@@ -732,7 +740,7 @@ int git_stash_apply(
 		goto cleanup;
 
 	/* Restore index if required */
-	if ((flags & GIT_APPLY_REINSTATE_INDEX) &&
+	if ((opts.flags & GIT_STASH_APPLY_REINSTATE_INDEX) &&
 		git_oid_cmp(git_tree_id(stash_parent_tree), git_tree_id(index_tree))) {
 
 		if ((error = merge_index_and_tree(
@@ -756,12 +764,12 @@ int git_stash_apply(
 		goto cleanup;
 
 	if (untracked_index) {
-		checkout_opts.checkout_strategy |= GIT_CHECKOUT_DONT_UPDATE_INDEX;
+		opts.checkout_options.checkout_strategy |= GIT_CHECKOUT_DONT_UPDATE_INDEX;
 
-		if ((error = git_checkout_index(repo, untracked_index, &checkout_opts)) < 0)
+		if ((error = git_checkout_index(repo, untracked_index, &opts.checkout_options)) < 0)
 			goto cleanup;
 
-		checkout_opts.checkout_strategy = checkout_strategy;
+		opts.checkout_options.checkout_strategy = checkout_strategy;
 	}
 
 
@@ -771,15 +779,15 @@ int git_stash_apply(
 	 */
 
 	if (!git_index_has_conflicts(modified_index))
-		checkout_opts.checkout_strategy |= GIT_CHECKOUT_DONT_UPDATE_INDEX;
+		opts.checkout_options.checkout_strategy |= GIT_CHECKOUT_DONT_UPDATE_INDEX;
 
 	/* Check out the modified index using the existing repo index as baseline,
 	 * so that existing modifications in the index can be rewritten even when
 	 * checking out safely.
 	 */
-	checkout_opts.baseline_index = repo_index;
+	opts.checkout_options.baseline_index = repo_index;
 
-	if ((error = git_checkout_index(repo, modified_index, &checkout_opts)) < 0)
+	if ((error = git_checkout_index(repo, modified_index, &opts.checkout_options)) < 0)
 		goto cleanup;
 
 	if (unstashed_index && !git_index_has_conflicts(modified_index)) {
@@ -903,12 +911,11 @@ cleanup:
 int git_stash_pop(
 	git_repository *repo,
 	size_t index,
-	const git_checkout_options *checkout_options,
-	unsigned int flags)
+	const git_stash_apply_options *options)
 {
 	int error;
 
-	if ((error = git_stash_apply(repo, index, checkout_options, flags)) < 0)
+	if ((error = git_stash_apply(repo, index, options)) < 0)
 		return error;
 
 	return git_stash_drop(repo, index);
diff --git a/tests/core/structinit.c b/tests/core/structinit.c
index 25254b713..e9f7b4a74 100644
--- a/tests/core/structinit.c
+++ b/tests/core/structinit.c
@@ -131,6 +131,11 @@ void test_core_structinit__compare(void)
 		git_revert_options, GIT_REVERT_OPTIONS_VERSION, \
 		GIT_REVERT_OPTIONS_INIT, git_revert_init_options);
 
+	/* stash apply */
+	CHECK_MACRO_FUNC_INIT_EQUAL( \
+		git_stash_apply_options, GIT_STASH_APPLY_OPTIONS_VERSION, \
+		GIT_STASH_APPLY_OPTIONS_INIT, git_stash_apply_init_options);
+
 	/* status */
 	CHECK_MACRO_FUNC_INIT_EQUAL( \
 		git_status_options, GIT_STATUS_OPTIONS_VERSION, \
diff --git a/tests/stash/apply.c b/tests/stash/apply.c
index 2f7e6f205..de330e9d3 100644
--- a/tests/stash/apply.c
+++ b/tests/stash/apply.c
@@ -63,7 +63,7 @@ void test_stash_apply__cleanup(void)
 
 void test_stash_apply__with_default(void)
 {
-	cl_git_pass(git_stash_apply(repo, 0, NULL, GIT_APPLY_DEFAULT));
+	cl_git_pass(git_stash_apply(repo, 0, NULL));
 
 	cl_assert_equal_i(git_index_has_conflicts(repo_index), 0);
 	assert_status(repo, "what", GIT_STATUS_WT_MODIFIED);
@@ -74,7 +74,11 @@ void test_stash_apply__with_default(void)
 
 void test_stash_apply__with_reinstate_index(void)
 {
-	cl_git_pass(git_stash_apply(repo, 0, NULL, GIT_APPLY_REINSTATE_INDEX));
+	git_stash_apply_options opts = GIT_STASH_APPLY_OPTIONS_INIT;
+
+	opts.flags = GIT_STASH_APPLY_REINSTATE_INDEX;
+
+	cl_git_pass(git_stash_apply(repo, 0, &opts));
 
 	cl_assert_equal_i(git_index_has_conflicts(repo_index), 0);
 	assert_status(repo, "what", GIT_STATUS_WT_MODIFIED);
@@ -93,7 +97,7 @@ void test_stash_apply__conflict_index_with_default(void)
 	cl_git_pass(git_index_add_bypath(repo_index, "who"));
 	cl_git_pass(git_index_write(repo_index));
 
-	cl_git_pass(git_stash_apply(repo, 0, NULL, GIT_APPLY_DEFAULT));
+	cl_git_pass(git_stash_apply(repo, 0, NULL));
 
 	cl_assert_equal_i(git_index_has_conflicts(repo_index), 1);
 	assert_status(repo, "what", GIT_STATUS_INDEX_MODIFIED);
@@ -104,11 +108,15 @@ void test_stash_apply__conflict_index_with_default(void)
 
 void test_stash_apply__conflict_index_with_reinstate_index(void)
 {
+	git_stash_apply_options opts = GIT_STASH_APPLY_OPTIONS_INIT;
+
+	opts.flags = GIT_STASH_APPLY_REINSTATE_INDEX;
+
 	cl_git_rewritefile("stash/who", "nothing\n");
 	cl_git_pass(git_index_add_bypath(repo_index, "who"));
 	cl_git_pass(git_index_write(repo_index));
 
-	cl_git_fail_with(git_stash_apply(repo, 0, NULL, GIT_APPLY_REINSTATE_INDEX), GIT_EMERGECONFLICT);
+	cl_git_fail_with(git_stash_apply(repo, 0, &opts), GIT_EMERGECONFLICT);
 
 	cl_assert_equal_i(git_index_has_conflicts(repo_index), 0);
 	assert_status(repo, "what", GIT_STATUS_CURRENT);
@@ -119,9 +127,11 @@ void test_stash_apply__conflict_index_with_reinstate_index(void)
 
 void test_stash_apply__conflict_untracked_with_default(void)
 {
+	git_stash_apply_options opts = GIT_STASH_APPLY_OPTIONS_INIT;
+
 	cl_git_mkfile("stash/when", "nothing\n");
 
-	cl_git_fail_with(git_stash_apply(repo, 0, NULL, GIT_APPLY_DEFAULT), GIT_EMERGECONFLICT);
+	cl_git_fail_with(git_stash_apply(repo, 0, &opts), GIT_EMERGECONFLICT);
 
 	cl_assert_equal_i(git_index_has_conflicts(repo_index), 0);
 	assert_status(repo, "what", GIT_STATUS_CURRENT);
@@ -132,9 +142,13 @@ void test_stash_apply__conflict_untracked_with_default(void)
 
 void test_stash_apply__conflict_untracked_with_reinstate_index(void)
 {
+	git_stash_apply_options opts = GIT_STASH_APPLY_OPTIONS_INIT;
+
+	opts.flags = GIT_STASH_APPLY_REINSTATE_INDEX;
+
 	cl_git_mkfile("stash/when", "nothing\n");
 
-	cl_git_fail_with(git_stash_apply(repo, 0, NULL, GIT_APPLY_REINSTATE_INDEX), GIT_EMERGECONFLICT);
+	cl_git_fail_with(git_stash_apply(repo, 0, &opts), GIT_EMERGECONFLICT);
 
 	cl_assert_equal_i(git_index_has_conflicts(repo_index), 0);
 	assert_status(repo, "what", GIT_STATUS_CURRENT);
@@ -147,7 +161,7 @@ void test_stash_apply__conflict_workdir_with_default(void)
 {
 	cl_git_rewritefile("stash/what", "ciao\n");
 
-	cl_git_fail_with(git_stash_apply(repo, 0, NULL, GIT_APPLY_DEFAULT), GIT_EMERGECONFLICT);
+	cl_git_fail_with(git_stash_apply(repo, 0, NULL), GIT_EMERGECONFLICT);
 
 	cl_assert_equal_i(git_index_has_conflicts(repo_index), 0);
 	assert_status(repo, "what", GIT_STATUS_WT_MODIFIED);
@@ -158,9 +172,13 @@ void test_stash_apply__conflict_workdir_with_default(void)
 
 void test_stash_apply__conflict_workdir_with_reinstate_index(void)
 {
+	git_stash_apply_options opts = GIT_STASH_APPLY_OPTIONS_INIT;
+
+	opts.flags = GIT_STASH_APPLY_REINSTATE_INDEX;
+
 	cl_git_rewritefile("stash/what", "ciao\n");
 
-	cl_git_fail_with(git_stash_apply(repo, 0, NULL, GIT_APPLY_REINSTATE_INDEX), GIT_EMERGECONFLICT);
+	cl_git_fail_with(git_stash_apply(repo, 0, &opts), GIT_EMERGECONFLICT);
 
 	cl_assert_equal_i(git_index_has_conflicts(repo_index), 0);
 	assert_status(repo, "what", GIT_STATUS_WT_MODIFIED);
@@ -179,7 +197,7 @@ void test_stash_apply__conflict_commit_with_default(void)
 	cl_git_pass(git_index_add_bypath(repo_index, "what"));
 	cl_repo_commit_from_index(NULL, repo, signature, 0, "Other commit");
 
-	cl_git_pass(git_stash_apply(repo, 0, NULL, GIT_APPLY_DEFAULT));
+	cl_git_pass(git_stash_apply(repo, 0, NULL));
 
 	cl_assert_equal_i(git_index_has_conflicts(repo_index), 1);
 	cl_git_pass(git_index_conflict_get(&ancestor, &our, &their, repo_index, "what")); /* unmerged */
@@ -190,15 +208,18 @@ void test_stash_apply__conflict_commit_with_default(void)
 
 void test_stash_apply__conflict_commit_with_reinstate_index(void)
 {
+	git_stash_apply_options opts = GIT_STASH_APPLY_OPTIONS_INIT;
 	const git_index_entry *ancestor;
 	const git_index_entry *our;
 	const git_index_entry *their;
 
+	opts.flags = GIT_STASH_APPLY_REINSTATE_INDEX;
+
 	cl_git_rewritefile("stash/what", "ciao\n");
 	cl_git_pass(git_index_add_bypath(repo_index, "what"));
 	cl_repo_commit_from_index(NULL, repo, signature, 0, "Other commit");
 
-	cl_git_pass(git_stash_apply(repo, 0, NULL, GIT_APPLY_REINSTATE_INDEX));
+	cl_git_pass(git_stash_apply(repo, 0, &opts));
 
 	cl_assert_equal_i(git_index_has_conflicts(repo_index), 1);
 	cl_git_pass(git_index_conflict_get(&ancestor, &our, &their, repo_index, "what")); /* unmerged */
@@ -209,9 +230,9 @@ void test_stash_apply__conflict_commit_with_reinstate_index(void)
 
 void test_stash_apply__pop(void)
 {
-	cl_git_pass(git_stash_pop(repo, 0, NULL, GIT_APPLY_DEFAULT));
+	cl_git_pass(git_stash_pop(repo, 0, NULL));
 
-	cl_git_fail_with(git_stash_pop(repo, 0, NULL, GIT_APPLY_DEFAULT), GIT_ENOTFOUND);
+	cl_git_fail_with(git_stash_pop(repo, 0, NULL), GIT_ENOTFOUND);
 }
 
 struct seen_paths {
@@ -245,14 +266,14 @@ int checkout_notify(
 
 void test_stash_apply__executes_notify_cb(void)
 {
-	git_checkout_options checkout_opts = GIT_CHECKOUT_OPTIONS_INIT;
+	git_stash_apply_options opts = GIT_STASH_APPLY_OPTIONS_INIT;
 	struct seen_paths seen_paths = {0};
 
-	checkout_opts.notify_cb = checkout_notify;
-	checkout_opts.notify_flags = GIT_CHECKOUT_NOTIFY_ALL;
-	checkout_opts.notify_payload = &seen_paths;
+	opts.checkout_options.notify_cb = checkout_notify;
+	opts.checkout_options.notify_flags = GIT_CHECKOUT_NOTIFY_ALL;
+	opts.checkout_options.notify_payload = &seen_paths;
 
-	cl_git_pass(git_stash_apply(repo, 0, &checkout_opts, GIT_APPLY_DEFAULT));
+	cl_git_pass(git_stash_apply(repo, 0, &opts));
 
 	cl_assert_equal_i(git_index_has_conflicts(repo_index), 0);
 	assert_status(repo, "what", GIT_STATUS_WT_MODIFIED);