Re: [PATCH 4/4] merge-recursive: rename merge_file_1() and merge_content()

[Stefan Beller]

On Wed, Sep 19, 2018 at 9:15 AM Elijah Newren  wrote:
>
> Summary:
>   merge_file_1()  -> merge_mode_and_contents()
>   merge_content() -> handle_content_merge()
>
> merge_file_1() is a very unhelpful name.  Rename it to
> merge_mode_and_contents() to reflect what it does.
>
> merge_content() calls merge_mode_and_contents() to do the main part of
> its work, but most of this function was about higher level stuff, e.g.
> printing out conflict messages, updating skip_worktree bits, checking
> for ability to avoid updating the working tree or for D/F conflicts
> being in the way, etc.  Since there are several handle_*() functions for
> similar levels of checking and handling in merge-recursive.c (e.g.
> handle_change_delete(), handle_rename_rename_2to1()), let's rename this
> function to handle_content_merge().
>
> Signed-off-by: Elijah Newren 

I looked through the whole series and 1,2,4 are obvious improvements IMHO,
and 3 in itself is not worth it if it weren't for 4 (as now we don't
have neither
_1 and _one functions in this file).

The whole series looks good to me.

Thanks,
Stefan

[PATCH 4/4] merge-recursive: rename merge_file_1() and merge_content()

[Elijah Newren]

Summary:
  merge_file_1()  -> merge_mode_and_contents()
  merge_content() -> handle_content_merge()

merge_file_1() is a very unhelpful name.  Rename it to
merge_mode_and_contents() to reflect what it does.

merge_content() calls merge_mode_and_contents() to do the main part of
its work, but most of this function was about higher level stuff, e.g.
printing out conflict messages, updating skip_worktree bits, checking
for ability to avoid updating the working tree or for D/F conflicts
being in the way, etc.  Since there are several handle_*() functions for
similar levels of checking and handling in merge-recursive.c (e.g.
handle_change_delete(), handle_rename_rename_2to1()), let's rename this
function to handle_content_merge().

Signed-off-by: Elijah Newren 
---
 merge-recursive.c | 66 ---
 1 file changed, 34 insertions(+), 32 deletions(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index 2654a8a485..5206d6cfb6 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1274,14 +1274,14 @@ static int merge_submodule(struct merge_options *o,
return 0;
 }
 
-static int merge_file_1(struct merge_options *o,
-   const struct diff_filespec *one,
-   const struct diff_filespec *a,
-   const struct diff_filespec *b,
-   const char *filename,
-   const char *branch1,
-   const char *branch2,
-   struct merge_file_info *result)
+static int merge_mode_and_contents(struct merge_options *o,
+  const struct diff_filespec *one,
+  const struct diff_filespec *a,
+  const struct diff_filespec *b,
+  const char *filename,
+  const char *branch1,
+  const char *branch2,
+  struct merge_file_info *result)
 {
result->merge = 0;
result->clean = 1;
@@ -1609,8 +1609,8 @@ static int handle_rename_rename_1to2(struct merge_options 
*o,
struct merge_file_info mfi;
struct diff_filespec other;
struct diff_filespec *add;
-   if (merge_file_1(o, one, a, b, one->path,
-ci->branch1, ci->branch2, &mfi))
+   if (merge_mode_and_contents(o, one, a, b, one->path,
+   ci->branch1, ci->branch2, &mfi))
return -1;
 
/*
@@ -1676,10 +1676,10 @@ static int handle_rename_rename_2to1(struct 
merge_options *o,
 
path_side_1_desc = xstrfmt("%s (was %s)", path, a->path);
path_side_2_desc = xstrfmt("%s (was %s)", path, b->path);
-   if (merge_file_1(o, a, c1, &ci->ren1_other, path_side_1_desc,
-o->branch1, o->branch2, &mfi_c1) ||
-   merge_file_1(o, b, &ci->ren2_other, c2, path_side_2_desc,
-o->branch1, o->branch2, &mfi_c2))
+   if (merge_mode_and_contents(o, a, c1, &ci->ren1_other, path_side_1_desc,
+   o->branch1, o->branch2, &mfi_c1) ||
+   merge_mode_and_contents(o, b, &ci->ren2_other, c2, path_side_2_desc,
+   o->branch1, o->branch2, &mfi_c2))
return -1;
free(path_side_1_desc);
free(path_side_2_desc);
@@ -2723,9 +2723,9 @@ static int process_renames(struct merge_options *o,
b.mode = dst_other.mode;
b.path = one.path;
 
-   if (merge_file_1(o, &one, &a, &b, 
ren1_dst,
-branch1, branch2,
-&mfi)) {
+   if (merge_mode_and_contents(o, &one, 
&a, &b, ren1_dst,
+   branch1, 
branch2,
+   &mfi)) {
clean_merge = -1;
goto cleanup_and_return;
}
@@ -2975,13 +2975,13 @@ static int handle_modify_delete(struct merge_options *o,
_("modify"), _("modified"));
 }
 
-static int merge_content(struct merge_options *o,
-const char *path,
-int is_dirty,
-struct object_id *o_oid, int o_mode,
-struct object_id *a_oid, int a_mode,
-struct object_id *b_oid, int b_mode,
-struct rename_conflict_info *rename_conflict_info)
+static int handle_content_merge(struct merge_options *o,
+