diff mbox series

[v5,10/10] merge-recursive: combine error handling

Message ID 20181108044031.25885-11-newren@gmail.com (mailing list archive)
State New, archived
Headers show
Series Improve path collision conflict resolutions | expand

Commit Message

Elijah Newren Nov. 8, 2018, 4:40 a.m. UTC 
From: Derrick Stolee <stolee@gmail.com>

In handle_rename_rename_1to2(), we have duplicated error handling
around colliding paths. Specifically, when we want to write out
the file and there is a directory or untracked file in the way,
we need to create a temporary file to hold the contents. This has
some special output to alert the user, and this output is
duplicated for each side of the conflict.

Simplify the call by generating this new path in a helper
function.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 merge-recursive.c | 53 ++++++++++++++++++++++++-----------------------
 1 file changed, 27 insertions(+), 26 deletions(-)

Comments

Junio C Hamano Nov. 8, 2018, 5:25 a.m. UTC | #1 
Elijah Newren <newren@gmail.com> writes:

> From: Derrick Stolee <stolee@gmail.com>
>
> In handle_rename_rename_1to2(), we have duplicated error handling
> around colliding paths. Specifically, when we want to write out
> the file and there is a directory or untracked file in the way,
> we need to create a temporary file to hold the contents. This has
> some special output to alert the user, and this output is
> duplicated for each side of the conflict.
>
> Simplify the call by generating this new path in a helper
> function.
>
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  merge-recursive.c | 53 ++++++++++++++++++++++++-----------------------
>  1 file changed, 27 insertions(+), 26 deletions(-)

Thanks, both.

Let's advance these patches to 'next' soonish.
diff mbox series

Patch

diff --git a/merge-recursive.c b/merge-recursive.c
index 3e2a63d094..ecf8db0b71 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1710,6 +1710,27 @@  static int handle_rename_add(struct merge_options *o,
 				     ci->dst_entry1->stages[other_stage].mode);
 }
 
+static char *find_path_for_conflict(struct merge_options *o,
+				    const char *path,
+				    const char *branch1,
+				    const char *branch2)
+{
+	char *new_path = NULL;
+	if (dir_in_way(path, !o->call_depth, 0)) {
+		new_path = unique_path(o, path, branch1);
+		output(o, 1, _("%s is a directory in %s adding "
+			       "as %s instead"),
+		       path, branch2, new_path);
+	} else if (would_lose_untracked(path)) {
+		new_path = unique_path(o, path, branch1);
+		output(o, 1, _("Refusing to lose untracked file"
+			       " at %s; adding as %s instead"),
+		       path, new_path);
+	}
+
+	return new_path;
+}
+
 static int handle_rename_rename_1to2(struct merge_options *o,
 				     struct rename_conflict_info *ci)
 {
@@ -1784,19 +1805,9 @@  static int handle_rename_rename_1to2(struct merge_options *o,
 						  &add->oid, add->mode) < 0)
 				return -1;
 		} else {
-			char *new_path = NULL;
-			if (dir_in_way(a->path, !o->call_depth, 0)) {
-				new_path = unique_path(o, a->path, ci->branch1);
-				output(o, 1, _("%s is a directory in %s adding "
-					       "as %s instead"),
-				       a->path, ci->branch2, new_path);
-			} else if (would_lose_untracked(a->path)) {
-				new_path = unique_path(o, a->path, ci->branch1);
-				output(o, 1, _("Refusing to lose untracked file"
-					       " at %s; adding as %s instead"),
-				       a->path, new_path);
-			}
-
+			char *new_path = find_path_for_conflict(o, a->path,
+								ci->branch1,
+								ci->branch2);
 			if (update_file(o, 0, &mfi.oid, mfi.mode, new_path ? new_path : a->path))
 				return -1;
 			free(new_path);
@@ -1813,19 +1824,9 @@  static int handle_rename_rename_1to2(struct merge_options *o,
 						  &mfi.oid, mfi.mode) < 0)
 				return -1;
 		} else {
-			char *new_path = NULL;
-			if (dir_in_way(b->path, !o->call_depth, 0)) {
-				new_path = unique_path(o, b->path, ci->branch2);
-				output(o, 1, _("%s is a directory in %s adding "
-					       "as %s instead"),
-				       b->path, ci->branch1, new_path);
-			} else if (would_lose_untracked(b->path)) {
-				new_path = unique_path(o, b->path, ci->branch2);
-				output(o, 1, _("Refusing to lose untracked file"
-					       " at %s; adding as %s instead"),
-				       b->path, new_path);
-			}
-
+			char *new_path = find_path_for_conflict(o, b->path,
+								ci->branch2,
+								ci->branch1);
 			if (update_file(o, 0, &mfi.oid, mfi.mode, new_path ? new_path : b->path))
 				return -1;
 			free(new_path);