{"thread":{"id":"48520","subject":"[PATCH 1/5] merge-recursive: fix miscellaneous grammar error in comment","startedAt":"2018-05-19T02:07:08Z","lastAt":"2018-06-10T04:16:45Z","messageCount":27,"participants":["Elijah Newren","Johannes Sixt","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"348024","messageId":"20180519020700.2241-2-newren@gmail.com","threadId":"48520","inReplyTo":"20180519020700.2241-1-newren@gmail.com","subject":"[PATCH 1/5] merge-recursive: fix miscellaneous grammar error in comment","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-05-19T02:06:56Z","receivedAt":"2018-05-19T02:07:08Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Signed-off-by: Elijah Newren \n---\n merge-recursive.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 35df695fa4..c430fd72bc 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -531,7 +531,7 @@ static void record_df_conflict_files(struct merge_options *o,\n \t\t\t\t struct string_list *entries)\n {\n \t/* If there is a D/F conflict and the file for such a conflict\n-\t * currently exist in the working tree, we want to allow it to be\n+\t * currently exists in the working tree, we want to allow it to be\n \t * removed to make room for the corresponding directory if needed.\n \t * The files underneath the directories of such D/F conflicts will\n \t * be processed before the corresponding file involved in the D/F\n-- \n2.17.0.847.g20b8963732\n\n"},{"id":"348025","messageId":"20180519020700.2241-6-newren@gmail.com","threadId":"48520","inReplyTo":"20180519020700.2241-1-newren@gmail.com","subject":"[PATCH 5/5] merge-recursive: simplify handle_change_delete","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-05-19T02:07:00Z","receivedAt":"2018-05-19T02:07:12Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"There is really no need for four branches of nearly identical messages\nwhen we can store the differences into small variables before printing.\nIt does require a few allocations this way, but makes the code much\neasier to parse for human readers.\n\nSigned-off-by: Elijah Newren \n---\n merge-recursive.c | 36 +++++++++++-------------------------\n 1 file changed, 11 insertions(+), 25 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 273ee79afa..3bd727995b 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -1290,31 +1290,17 @@ static int handle_change_delete(struct merge_options *o,\n \t\tif (!ret)\n \t\t\tret = update_file(o, 0, o_oid, o_mode, update_path);\n \t} else {\n-\t\tif (!alt_path) {\n-\t\t\tif (!old_path) {\n-\t\t\t\toutput(o, 1, _(\"CONFLICT (%s/delete): %s deleted in %s \"\n-\t\t\t\t \"and %s in %s. Version %s of %s left in tree.\"),\n-\t\t\t\t change, path, delete_branch, change_past,\n-\t\t\t\t change_branch, change_branch, path);\n-\t\t\t} else {\n-\t\t\t\toutput(o, 1, _(\"CONFLICT (%s/delete): %s deleted in %s \"\n-\t\t\t\t \"and %s to %s in %s. Version %s of %s left in tree.\"),\n-\t\t\t\t change, old_path, delete_branch, change_past, path,\n-\t\t\t\t change_branch, change_branch, path);\n-\t\t\t}\n-\t\t} else {\n-\t\t\tif (!old_path) {\n-\t\t\t\toutput(o, 1, _(\"CONFLICT (%s/delete): %s deleted in %s \"\n-\t\t\t\t \"and %s in %s. Version %s of %s left in tree at %s.\"),\n-\t\t\t\t change, path, delete_branch, change_past,\n-\t\t\t\t change_branch, change_branch, path, alt_path);\n-\t\t\t} else {\n-\t\t\t\toutput(o, 1, _(\"CONFLICT (%s/delete): %s deleted in %s \"\n-\t\t\t\t \"and %s to %s in %s. Version %s of %s left in tree at %s.\"),\n-\t\t\t\t change, old_path, delete_branch, change_past, path,\n-\t\t\t\t change_branch, change_branch, path, alt_path);\n-\t\t\t}\n-\t\t}\n+\t\tconst char *deleted_path = old_path ? old_path : path;\n+\t\tchar *supp1 = xstrfmt(old_path ? \" to %s\" : \"\", path);\n+\t\tchar *supp2 = xstrfmt(alt_path ? \" at %s\" : \"\", alt_path);\n+\n+\t\toutput(o, 1, _(\"CONFLICT (%s/delete): %s deleted in %s \"\n+\t\t \"and %s%s in %s. Version %s of %s left in tree%s.\"),\n+\t\t change, deleted_path, delete_branch, change_past,\n+\t\t supp1, change_branch, change_branch, path, supp2);\n+\t\tfree(supp1);\n+\t\tfree(supp2);\n+\n \t\t/*\n \t\t * No need to call update_file() on path when change_branch ==\n \t\t * o->branch1 && !alt_path, since that would needlessly touch\n-- \n2.17.0.847.g20b8963732\n\n"},{"id":"348026","messageId":"20180519020700.2241-3-newren@gmail.com","threadId":"48520","inReplyTo":"20180519020700.2241-1-newren@gmail.com","subject":"[PATCH 2/5] merge-recursive: fix numerous argument alignment issues","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-05-19T02:06:57Z","receivedAt":"2018-05-19T02:07:15Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Various refactorings throughout the code have left lots of alignment\nissues that were driving me crazy; fix them.\n\nSigned-off-by: Elijah Newren \n---\n merge-recursive.c | 47 ++++++++++++++++++++++++-----------------------\n 1 file changed, 24 insertions(+), 23 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex c430fd72bc..01306c87eb 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -308,8 +308,8 @@ static void output_commit_title(struct merge_options *o, struct commit *commit)\n }\n \n static int add_cacheinfo(struct merge_options *o,\n-\t\tunsigned int mode, const struct object_id *oid,\n-\t\tconst char *path, int stage, int refresh, int options)\n+\t\t\t unsigned int mode, const struct object_id *oid,\n+\t\t\t const char *path, int stage, int refresh, int options)\n {\n \tstruct cache_entry *ce;\n \tint ret;\n@@ -409,8 +409,8 @@ struct tree *write_tree_from_memory(struct merge_options *o)\n }\n \n static int save_files_dirs(const struct object_id *oid,\n-\t\tstruct strbuf *base, const char *path,\n-\t\tunsigned int mode, int stage, void *context)\n+\t\t\t struct strbuf *base, const char *path,\n+\t\t\t unsigned int mode, int stage, void *context)\n {\n \tstruct path_hashmap_entry *entry;\n \tint baselen = base->len;\n@@ -1257,13 +1257,13 @@ static int conflict_rename_dir(struct merge_options *o,\n }\n \n static int handle_change_delete(struct merge_options *o,\n-\t\t\t\t const char *path, const char *old_path,\n-\t\t\t\t const struct object_id *o_oid, int o_mode,\n-\t\t\t\t const struct object_id *changed_oid,\n-\t\t\t\t int changed_mode,\n-\t\t\t\t const char *change_branch,\n-\t\t\t\t const char *delete_branch,\n-\t\t\t\t const char *change, const char *change_past)\n+\t\t\t\tconst char *path, const char *old_path,\n+\t\t\t\tconst struct object_id *o_oid, int o_mode,\n+\t\t\t\tconst struct object_id *changed_oid,\n+\t\t\t\tint changed_mode,\n+\t\t\t\tconst char *change_branch,\n+\t\t\t\tconst char *delete_branch,\n+\t\t\t\tconst char *change, const char *change_past)\n {\n \tchar *alt_path = NULL;\n \tconst char *update_path = path;\n@@ -1324,9 +1324,9 @@ static int handle_change_delete(struct merge_options *o,\n }\n \n static int conflict_rename_delete(struct merge_options *o,\n-\t\t\t\t struct diff_filepair *pair,\n-\t\t\t\t const char *rename_branch,\n-\t\t\t\t const char *delete_branch)\n+\t\t\t\t struct diff_filepair *pair,\n+\t\t\t\t const char *rename_branch,\n+\t\t\t\t const char *delete_branch)\n {\n \tconst struct diff_filespec *orig = pair->one;\n \tconst struct diff_filespec *dest = pair->two;\n@@ -1429,7 +1429,7 @@ static int handle_file(struct merge_options *o,\n }\n \n static int conflict_rename_rename_1to2(struct merge_options *o,\n-\t\t\t\t\tstruct rename_conflict_info *ci)\n+\t\t\t\t struct rename_conflict_info *ci)\n {\n \t/* One file was renamed in both branches, but to different names. */\n \tstruct diff_filespec *one = ci->pair1->one;\n@@ -1491,7 +1491,7 @@ static int conflict_rename_rename_1to2(struct merge_options *o,\n }\n \n static int conflict_rename_rename_2to1(struct merge_options *o,\n-\t\t\t\t\tstruct rename_conflict_info *ci)\n+\t\t\t\t struct rename_conflict_info *ci)\n {\n \t/* Two files, a & b, were renamed to the same thing, c. */\n \tstruct diff_filespec *a = ci->pair1->one;\n@@ -2710,7 +2710,8 @@ static struct object_id *stage_oid(const struct object_id *oid, unsigned mode)\n }\n \n static int read_oid_strbuf(struct merge_options *o,\n-\tconst struct object_id *oid, struct strbuf *dst)\n+\t\t\t const struct object_id *oid,\n+\t\t\t struct strbuf *dst)\n {\n \tvoid *buf;\n \tenum object_type type;\n@@ -2763,10 +2764,10 @@ static int blob_unchanged(struct merge_options *opt,\n }\n \n static int handle_modify_delete(struct merge_options *o,\n-\t\t\t\t const char *path,\n-\t\t\t\t struct object_id *o_oid, int o_mode,\n-\t\t\t\t struct object_id *a_oid, int a_mode,\n-\t\t\t\t struct object_id *b_oid, int b_mode)\n+\t\t\t\tconst char *path,\n+\t\t\t\tstruct object_id *o_oid, int o_mode,\n+\t\t\t\tstruct object_id *a_oid, int a_mode,\n+\t\t\t\tstruct object_id *b_oid, int b_mode)\n {\n \tconst char *modify_branch, *delete_branch;\n \tstruct object_id *changed_oid;\n@@ -3303,14 +3304,14 @@ int merge_recursive_generic(struct merge_options *o,\n \t\t\tstruct commit *base;\n \t\t\tif (!(base = get_ref(base_list[i], oid_to_hex(base_list[i]))))\n \t\t\t\treturn err(o, _(\"Could not parse object '%s'\"),\n-\t\t\t\t\toid_to_hex(base_list[i]));\n+\t\t\t\t\t oid_to_hex(base_list[i]));\n \t\t\tcommit_list_insert(base, &ca);\n \t\t}\n \t}\n \n \thold_locked_index(&lock, LOCK_DIE_ON_ERROR);\n \tclean = merge_recursive(o, head_commit, next_commit, ca,\n-\t\t\tresult);\n+\t\t\t\tresult);\n \tif (clean < 0) {\n \t\trollback_lock_file(&lock);\n \t\treturn clean;\n-- \n2.17.0.847.g20b8963732\n\n"},{"id":"348027","messageId":"20180519020700.2241-1-newren@gmail.com","threadId":"48520","inReplyTo":null,"subject":"[PATCH 0/5] merge-recursive code cleanups","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-05-19T02:06:55Z","receivedAt":"2018-05-19T02:07:17Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"This patch series contains several small code cleanups for merge-recursive.\nI have removed a couple small cleanup chunks in order to avoid conflicts\nwith any other in-flight topics in pu (namely, nd/commit-util-to-slab and\nsb/submodule-merge-in-merge-recursive). I may resend those later\nseparately.\n\nThe series was made on top of next (specifically commit c95db04db (\"Merge\nbranch 'sb/object-store-replace' into next\")); it will not apply to\nmaster.\n\nElijah Newren (5):\n merge-recursive: fix miscellaneous grammar error in comment\n merge-recursive: fix numerous argument alignment issues\n merge-recursive: clarify the rename_dir/RENAME_DIR meaning\n merge-recursive: rename conflict_rename_*() family of functions\n merge-recursive: simplify handle_change_delete\n\n merge-recursive.c | 173 ++++++++++++++++++++++------------------------\n 1 file changed, 83 insertions(+), 90 deletions(-)\n\n-- \n2.17.0.847.g20b8963732\n\n"},{"id":"348028","messageId":"20180519020700.2241-4-newren@gmail.com","threadId":"48520","inReplyTo":"20180519020700.2241-1-newren@gmail.com","subject":"[PATCH 3/5] merge-recursive: clarify the rename_dir/RENAME_DIR meaning","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-05-19T02:06:58Z","receivedAt":"2018-05-19T02:07:19Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"We had an enum of rename types which included RENAME_DIR; this name felt\nmisleading since it was not about an entire directory but was a status for\neach individual file add that occurred within a renamed directory. Since\nthis type is for signifying that the files in question were being renamed\ndue to directory rename detection, rename this enum value to\nRENAME_VIA_DIR. Make a similar change to the conflict_rename_dir()\nfunction, and add a comment to the top of that function explaining its\npurpose (it may not be quite as obvious as for the other\nconflict_rename_*() functions).\n\nSigned-off-by: Elijah Newren \n---\n merge-recursive.c | 28 +++++++++++++++++-----------\n 1 file changed, 17 insertions(+), 11 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 01306c87eb..d30085d9c7 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -180,7 +180,7 @@ static int oid_eq(const struct object_id *a, const struct object_id *b)\n \n enum rename_type {\n \tRENAME_NORMAL = 0,\n-\tRENAME_DIR,\n+\tRENAME_VIA_DIR,\n \tRENAME_DELETE,\n \tRENAME_ONE_FILE_TO_ONE,\n \tRENAME_ONE_FILE_TO_TWO,\n@@ -1224,11 +1224,17 @@ static int merge_file_one(struct merge_options *o,\n \treturn merge_file_1(o, &one, &a, &b, path, branch1, branch2, mfi);\n }\n \n-static int conflict_rename_dir(struct merge_options *o,\n-\t\t\t struct diff_filepair *pair,\n-\t\t\t const char *rename_branch,\n-\t\t\t const char *other_branch)\n+static int conflict_rename_via_dir(struct merge_options *o,\n+\t\t\t\t struct diff_filepair *pair,\n+\t\t\t\t const char *rename_branch,\n+\t\t\t\t const char *other_branch)\n {\n+\t/*\n+\t * Handle file adds that need to be renamed due to directory rename\n+\t * detection. This differs from handle_rename_normal, because\n+\t * there is no content merge to do; just move the path into the\n+\t * desired final location.\n+\t */\n \tconst struct diff_filespec *dest = pair->two;\n \n \tif (!o->call_depth && would_lose_untracked(dest->path)) {\n@@ -2498,7 +2504,7 @@ static int process_renames(struct merge_options *o,\n \n \t\t\tif (oid_eq(&src_other.oid, &null_oid) &&\n \t\t\t ren1->add_turned_into_rename) {\n-\t\t\t\tsetup_rename_conflict_info(RENAME_DIR,\n+\t\t\t\tsetup_rename_conflict_info(RENAME_VIA_DIR,\n \t\t\t\t\t\t\t ren1->pair,\n \t\t\t\t\t\t\t NULL,\n \t\t\t\t\t\t\t branch1,\n@@ -2944,12 +2950,12 @@ static int process_entry(struct merge_options *o,\n \t\t\t\t\t\t\t b_oid, b_mode,\n \t\t\t\t\t\t\t conflict_info);\n \t\t\tbreak;\n-\t\tcase RENAME_DIR:\n+\t\tcase RENAME_VIA_DIR:\n \t\t\tclean_merge = 1;\n-\t\t\tif (conflict_rename_dir(o,\n-\t\t\t\t\t\tconflict_info->pair1,\n-\t\t\t\t\t\tconflict_info->branch1,\n-\t\t\t\t\t\tconflict_info->branch2))\n+\t\t\tif (conflict_rename_via_dir(o,\n+\t\t\t\t\t\t conflict_info->pair1,\n+\t\t\t\t\t\t conflict_info->branch1,\n+\t\t\t\t\t\t conflict_info->branch2))\n \t\t\t\tclean_merge = -1;\n \t\t\tbreak;\n \t\tcase RENAME_DELETE:\n-- \n2.17.0.847.g20b8963732\n\n"},{"id":"348029","messageId":"20180519020700.2241-5-newren@gmail.com","threadId":"48520","inReplyTo":"20180519020700.2241-1-newren@gmail.com","subject":"[PATCH 4/5] merge-recursive: rename conflict_rename_*() family of functions","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-05-19T02:06:59Z","receivedAt":"2018-05-19T02:07:21Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"These functions were added because processing of these conflicts needed\nto be deferred until process_entry() in order to get D/F conflicts and\nsuch right. The number of these has grown over time, and now include\nsome whose name is misleading:\n * conflict_rename_normal() is for handling normal file renames; a\n typical rename may need content merging, but we expect conflicts\n from that to be more the exception than the rule.\n * conflict_rename_via_dir() will not be a conflict; it was just an\n add that turned into a move due to directory rename detection.\n (If there was a file in the way of the move, that would have been\n detected and reported earlier.)\n * conflict_rename_rename_2to1 and conflict_rename_add (the latter\n of which doesn't exist yet but has been submitted before and I\n intend to resend) technically might not be conflicts if the\n colliding paths happen to match exactly.\nRename this family of functions to handle_rename_*().\n\nAlso rename handle_renames() to detect_and_process_renames() both to make\nit clearer what it does, and to differentiate it as a pre-processing step\nfrom all the handle_rename_*() functions which are called from\nprocess_entry().\n\nSigned-off-by: Elijah Newren \n---\n merge-recursive.c | 86 +++++++++++++++++++++++------------------------\n 1 file changed, 43 insertions(+), 43 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex d30085d9c7..273ee79afa 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -1224,10 +1224,10 @@ static int merge_file_one(struct merge_options *o,\n \treturn merge_file_1(o, &one, &a, &b, path, branch1, branch2, mfi);\n }\n \n-static int conflict_rename_via_dir(struct merge_options *o,\n-\t\t\t\t struct diff_filepair *pair,\n-\t\t\t\t const char *rename_branch,\n-\t\t\t\t const char *other_branch)\n+static int handle_rename_via_dir(struct merge_options *o,\n+\t\t\t\t struct diff_filepair *pair,\n+\t\t\t\t const char *rename_branch,\n+\t\t\t\t const char *other_branch)\n {\n \t/*\n \t * Handle file adds that need to be renamed due to directory rename\n@@ -1329,10 +1329,10 @@ static int handle_change_delete(struct merge_options *o,\n \treturn ret;\n }\n \n-static int conflict_rename_delete(struct merge_options *o,\n-\t\t\t\t struct diff_filepair *pair,\n-\t\t\t\t const char *rename_branch,\n-\t\t\t\t const char *delete_branch)\n+static int handle_rename_delete(struct merge_options *o,\n+\t\t\t\tstruct diff_filepair *pair,\n+\t\t\t\tconst char *rename_branch,\n+\t\t\t\tconst char *delete_branch)\n {\n \tconst struct diff_filespec *orig = pair->one;\n \tconst struct diff_filespec *dest = pair->two;\n@@ -1434,8 +1434,8 @@ static int handle_file(struct merge_options *o,\n \treturn ret;\n }\n \n-static int conflict_rename_rename_1to2(struct merge_options *o,\n-\t\t\t\t struct rename_conflict_info *ci)\n+static int handle_rename_rename_1to2(struct merge_options *o,\n+\t\t\t\t struct rename_conflict_info *ci)\n {\n \t/* One file was renamed in both branches, but to different names. */\n \tstruct diff_filespec *one = ci->pair1->one;\n@@ -1496,8 +1496,8 @@ static int conflict_rename_rename_1to2(struct merge_options *o,\n \treturn 0;\n }\n \n-static int conflict_rename_rename_2to1(struct merge_options *o,\n-\t\t\t\t struct rename_conflict_info *ci)\n+static int handle_rename_rename_2to1(struct merge_options *o,\n+\t\t\t\t struct rename_conflict_info *ci)\n {\n \t/* Two files, a & b, were renamed to the same thing, c. */\n \tstruct diff_filespec *a = ci->pair1->one;\n@@ -2231,7 +2231,7 @@ static void apply_directory_rename_modifications(struct merge_options *o,\n \t * \"NOTE\" in update_stages(), doing so will modify the current\n \t * in-memory index which will break calls to would_lose_untracked()\n \t * that we need to make. Instead, we need to just make sure that\n-\t * the various conflict_rename_*() functions update the index\n+\t * the various handle_rename_*() functions update the index\n \t * explicitly rather than relying on unpack_trees() to have done it.\n \t */\n \tget_tree_entry(&tree->object.oid,\n@@ -2635,12 +2635,12 @@ static void initial_cleanup_rename(struct diff_queue_struct *pairs,\n \tfree(pairs);\n }\n \n-static int handle_renames(struct merge_options *o,\n-\t\t\t struct tree *common,\n-\t\t\t struct tree *head,\n-\t\t\t struct tree *merge,\n-\t\t\t struct string_list *entries,\n-\t\t\t struct rename_info *ri)\n+static int detect_and_process_renames(struct merge_options *o,\n+\t\t\t\t struct tree *common,\n+\t\t\t\t struct tree *head,\n+\t\t\t\t struct tree *merge,\n+\t\t\t\t struct string_list *entries,\n+\t\t\t\t struct rename_info *ri)\n {\n \tstruct diff_queue_struct *head_pairs, *merge_pairs;\n \tstruct hashmap *dir_re_head, *dir_re_merge;\n@@ -2911,12 +2911,12 @@ static int merge_content(struct merge_options *o,\n \treturn !is_dirty && mfi.clean;\n }\n \n-static int conflict_rename_normal(struct merge_options *o,\n-\t\t\t\t const char *path,\n-\t\t\t\t struct object_id *o_oid, unsigned int o_mode,\n-\t\t\t\t struct object_id *a_oid, unsigned int a_mode,\n-\t\t\t\t struct object_id *b_oid, unsigned int b_mode,\n-\t\t\t\t struct rename_conflict_info *ci)\n+static int handle_rename_normal(struct merge_options *o,\n+\t\t\t\tconst char *path,\n+\t\t\t\tstruct object_id *o_oid, unsigned int o_mode,\n+\t\t\t\tstruct object_id *a_oid, unsigned int a_mode,\n+\t\t\t\tstruct object_id *b_oid, unsigned int b_mode,\n+\t\t\t\tstruct rename_conflict_info *ci)\n {\n \t/* Merge the content and write it out */\n \treturn merge_content(o, path, was_dirty(o, path),\n@@ -2943,37 +2943,37 @@ static int process_entry(struct merge_options *o,\n \t\tswitch (conflict_info->rename_type) {\n \t\tcase RENAME_NORMAL:\n \t\tcase RENAME_ONE_FILE_TO_ONE:\n-\t\t\tclean_merge = conflict_rename_normal(o,\n-\t\t\t\t\t\t\t path,\n-\t\t\t\t\t\t\t o_oid, o_mode,\n-\t\t\t\t\t\t\t a_oid, a_mode,\n-\t\t\t\t\t\t\t b_oid, b_mode,\n-\t\t\t\t\t\t\t conflict_info);\n+\t\t\tclean_merge = handle_rename_normal(o,\n+\t\t\t\t\t\t\t path,\n+\t\t\t\t\t\t\t o_oid, o_mode,\n+\t\t\t\t\t\t\t a_oid, a_mode,\n+\t\t\t\t\t\t\t b_oid, b_mode,\n+\t\t\t\t\t\t\t conflict_info);\n \t\t\tbreak;\n \t\tcase RENAME_VIA_DIR:\n \t\t\tclean_merge = 1;\n-\t\t\tif (conflict_rename_via_dir(o,\n-\t\t\t\t\t\t conflict_info->pair1,\n-\t\t\t\t\t\t conflict_info->branch1,\n-\t\t\t\t\t\t conflict_info->branch2))\n+\t\t\tif (handle_rename_via_dir(o,\n+\t\t\t\t\t\t conflict_info->pair1,\n+\t\t\t\t\t\t conflict_info->branch1,\n+\t\t\t\t\t\t conflict_info->branch2))\n \t\t\t\tclean_merge = -1;\n \t\t\tbreak;\n \t\tcase RENAME_DELETE:\n \t\t\tclean_merge = 0;\n-\t\t\tif (conflict_rename_delete(o,\n-\t\t\t\t\t\t conflict_info->pair1,\n-\t\t\t\t\t\t conflict_info->branch1,\n-\t\t\t\t\t\t conflict_info->branch2))\n+\t\t\tif (handle_rename_delete(o,\n+\t\t\t\t\t\t conflict_info->pair1,\n+\t\t\t\t\t\t conflict_info->branch1,\n+\t\t\t\t\t\t conflict_info->branch2))\n \t\t\t\tclean_merge = -1;\n \t\t\tbreak;\n \t\tcase RENAME_ONE_FILE_TO_TWO:\n \t\t\tclean_merge = 0;\n-\t\t\tif (conflict_rename_rename_1to2(o, conflict_info))\n+\t\t\tif (handle_rename_rename_1to2(o, conflict_info))\n \t\t\t\tclean_merge = -1;\n \t\t\tbreak;\n \t\tcase RENAME_TWO_FILES_TO_ONE:\n \t\t\tclean_merge = 0;\n-\t\t\tif (conflict_rename_rename_2to1(o, conflict_info))\n+\t\t\tif (handle_rename_rename_2to1(o, conflict_info))\n \t\t\t\tclean_merge = -1;\n \t\t\tbreak;\n \t\tdefault:\n@@ -3112,8 +3112,8 @@ int merge_trees(struct merge_options *o,\n \t\tget_files_dirs(o, merge);\n \n \t\tentries = get_unmerged();\n-\t\tclean = handle_renames(o, common, head, merge, entries,\n-\t\t\t\t &re_info);\n+\t\tclean = detect_and_process_renames(o, common, head, merge,\n+\t\t\t\t\t\t entries, &re_info);\n \t\trecord_df_conflict_files(o, entries);\n \t\tif (clean < 0)\n \t\t\tgoto cleanup;\n-- \n2.17.0.847.g20b8963732\n\n"},{"id":"348072","messageId":"3b05f4b1-e236-989d-afe0-e85b28c2e278@kdbg.org","threadId":"48520","inReplyTo":"20180519020700.2241-6-newren@gmail.com","subject":"Re: [PATCH 5/5] merge-recursive: simplify handle_change_delete","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2018-05-19T07:32:04Z","receivedAt":"2018-05-19T07:32:10Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 19.05.2018 um 04:07 schrieb Elijah Newren:\n> There is really no need for four branches of nearly identical messages\n> when we can store the differences into small variables before printing.\n\nOh, there is a reason for the repeated message text: translations! \nPlease do not play sentence Lego with translated strings. The original \ncode is preferable.\n\n> It does require a few allocations this way, but makes the code much\n> easier to parse for human readers.\n> \n> Signed-off-by: Elijah Newren \n> ---\n> merge-recursive.c | 36 +++++++++++-------------------------\n> 1 file changed, 11 insertions(+), 25 deletions(-)\n> \n> diff --git a/merge-recursive.c b/merge-recursive.c\n> index 273ee79afa..3bd727995b 100644\n> --- a/merge-recursive.c\n> +++ b/merge-recursive.c\n> @@ -1290,31 +1290,17 @@ static int handle_change_delete(struct merge_options *o,\n> \t\tif (!ret)\n> \t\t\tret = update_file(o, 0, o_oid, o_mode, update_path);\n> \t} else {\n> -\t\tif (!alt_path) {\n> -\t\t\tif (!old_path) {\n> -\t\t\t\toutput(o, 1, _(\"CONFLICT (%s/delete): %s deleted in %s \"\n> -\t\t\t\t \"and %s in %s. Version %s of %s left in tree.\"),\n> -\t\t\t\t change, path, delete_branch, change_past,\n> -\t\t\t\t change_branch, change_branch, path);\n> -\t\t\t} else {\n> -\t\t\t\toutput(o, 1, _(\"CONFLICT (%s/delete): %s deleted in %s \"\n> -\t\t\t\t \"and %s to %s in %s. Version %s of %s left in tree.\"),\n> -\t\t\t\t change, old_path, delete_branch, change_past, path,\n> -\t\t\t\t change_branch, change_branch, path);\n> -\t\t\t}\n> -\t\t} else {\n> -\t\t\tif (!old_path) {\n> -\t\t\t\toutput(o, 1, _(\"CONFLICT (%s/delete): %s deleted in %s \"\n> -\t\t\t\t \"and %s in %s. Version %s of %s left in tree at %s.\"),\n> -\t\t\t\t change, path, delete_branch, change_past,\n> -\t\t\t\t change_branch, change_branch, path, alt_path);\n> -\t\t\t} else {\n> -\t\t\t\toutput(o, 1, _(\"CONFLICT (%s/delete): %s deleted in %s \"\n> -\t\t\t\t \"and %s to %s in %s. Version %s of %s left in tree at %s.\"),\n> -\t\t\t\t change, old_path, delete_branch, change_past, path,\n> -\t\t\t\t change_branch, change_branch, path, alt_path);\n> -\t\t\t}\n> -\t\t}\n> +\t\tconst char *deleted_path = old_path ? old_path : path;\n> +\t\tchar *supp1 = xstrfmt(old_path ? \" to %s\" : \"\", path);\n> +\t\tchar *supp2 = xstrfmt(alt_path ? \" at %s\" : \"\", alt_path);\n> +\n> +\t\toutput(o, 1, _(\"CONFLICT (%s/delete): %s deleted in %s \"\n> +\t\t \"and %s%s in %s. Version %s of %s left in tree%s.\"),\n> +\t\t change, deleted_path, delete_branch, change_past,\n> +\t\t supp1, change_branch, change_branch, path, supp2);\n> +\t\tfree(supp1);\n> +\t\tfree(supp2);\n> +\n> \t\t/*\n> \t\t * No need to call update_file() on path when change_branch ==\n> \t\t * o->branch1 && !alt_path, since that would needlessly touch\n> \n\n"},{"id":"348078","messageId":"CABPp-BFjF7ZAjqi79aRbT+0M+stPnrcEXwYoV75qCHTb2oLiXg@mail.gmail.com","threadId":"48520","inReplyTo":"3b05f4b1-e236-989d-afe0-e85b28c2e278@kdbg.org","subject":"Re: [PATCH 5/5] merge-recursive: simplify handle_change_delete","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-05-19T15:39:04Z","receivedAt":"2018-05-19T15:39:08Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Sat, May 19, 2018 at 12:32 AM, Johannes Sixt wrote:\n> Am 19.05.2018 um 04:07 schrieb Elijah Newren:\n>>\n>> There is really no need for four branches of nearly identical messages\n>> when we can store the differences into small variables before printing.\n>\n> Oh, there is a reason for the repeated message text: translations! Please do\n> not play sentence Lego with translated strings. The original code is\n> preferable.\n\nAh, translations; that makes sense now. I'm still annoyed by the\ncode, but I retract patch 5 then.\n"},{"id":"348200","messageId":"nycvar.QRO.7.76.6.1805211540120.77@tvgsbejvaqbjf.bet","threadId":"48520","inReplyTo":"CABPp-BFjF7ZAjqi79aRbT+0M+stPnrcEXwYoV75qCHTb2oLiXg@mail.gmail.com","subject":"Re: [PATCH 5/5] merge-recursive: simplify handle_change_delete","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-05-21T13:41:21Z","receivedAt":"2018-05-21T13:41:22Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Elijah,\n\nOn Sat, 19 May 2018, Elijah Newren wrote:\n\n> On Sat, May 19, 2018 at 12:32 AM, Johannes Sixt wrote:\n> > Am 19.05.2018 um 04:07 schrieb Elijah Newren:\n> >>\n> >> There is really no need for four branches of nearly identical messages\n> >> when we can store the differences into small variables before printing.\n> >\n> > Oh, there is a reason for the repeated message text: translations! Please do\n> > not play sentence Lego with translated strings. The original code is\n> > preferable.\n> \n> Ah, translations; that makes sense now. I'm still annoyed by the\n> code, but I retract patch 5 then.\n\nMaybe you can remove the temptation for others, too, my replacing your\npatch 5 with one that adds the code comment \"No sentence Lego! Think of\nour poor translators and refrain from making their life miserable!\" or\nsome more appropriate one?\n\nCiao,\nDscho\n"},{"id":"348201","messageId":"nycvar.QRO.7.76.6.1805211542160.77@tvgsbejvaqbjf.bet","threadId":"48520","inReplyTo":"20180519020700.2241-3-newren@gmail.com","subject":"Re: [PATCH 2/5] merge-recursive: fix numerous argument alignment issues","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-05-21T13:42:43Z","receivedAt":"2018-05-21T13:42:46Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Elijah,\n\nOn Fri, 18 May 2018, Elijah Newren wrote:\n\n> Various refactorings throughout the code have left lots of alignment\n> issues that were driving me crazy; fix them.\n\nI hope you did not do that manually. What is your code formatting tool of\nchoice?\n\nThe patch looks obviously good to me.\n\nCiao,\nDscho\n"},{"id":"348203","messageId":"nycvar.QRO.7.76.6.1805211626240.77@tvgsbejvaqbjf.bet","threadId":"48520","inReplyTo":"20180519020700.2241-4-newren@gmail.com","subject":"Re: [PATCH 3/5] merge-recursive: clarify the rename_dir/RENAME_DIR meaning","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-05-21T14:28:24Z","receivedAt":"2018-05-21T14:28:25Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Elijah,\n\nOn Fri, 18 May 2018, Elijah Newren wrote:\n\n> We had an enum of rename types which included RENAME_DIR; this name felt\n> misleading since it was not about an entire directory but was a status for\n> each individual file add that occurred within a renamed directory. Since\n> this type is for signifying that the files in question were being renamed\n> due to directory rename detection, rename this enum value to\n> RENAME_VIA_DIR. Make a similar change to the conflict_rename_dir()\n> function, and add a comment to the top of that function explaining its\n> purpose (it may not be quite as obvious as for the other\n> conflict_rename_*() functions).\n> \n> Signed-off-by: Elijah Newren \n> ---\n\nMake sense. I think the reading flow could be improved a little bit by\nsplitting this paragraph into three.\n\n> diff --git a/merge-recursive.c b/merge-recursive.c\n> index 01306c87eb..d30085d9c7 100644\n> --- a/merge-recursive.c\n> +++ b/merge-recursive.c\n> -static int conflict_rename_dir(struct merge_options *o,\n> -\t\t\t struct diff_filepair *pair,\n> -\t\t\t const char *rename_branch,\n> -\t\t\t const char *other_branch)\n> +static int conflict_rename_via_dir(struct merge_options *o,\n> +\t\t\t\t struct diff_filepair *pair,\n> +\t\t\t\t const char *rename_branch,\n> +\t\t\t\t const char *other_branch)\n> {\n> +\t/*\n> +\t * Handle file adds that need to be renamed due to directory rename\n> +\t * detection. This differs from handle_rename_normal, because\n> +\t * there is no content merge to do; just move the path into the\n\nTechnically, we do not move the path, but the file.\n\nThe rest of the diff looks obviously correct to me.\n\nThanks,\nDscho\n"},{"id":"348204","messageId":"nycvar.QRO.7.76.6.1805211629450.77@tvgsbejvaqbjf.bet","threadId":"48520","inReplyTo":"20180519020700.2241-5-newren@gmail.com","subject":"Re: [PATCH 4/5] merge-recursive: rename conflict_rename_*() family of functions","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-05-21T14:30:04Z","receivedAt":"2018-05-21T14:30:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Elijah,\n\n\nOn Fri, 18 May 2018, Elijah Newren wrote:\n\n> These functions were added because processing of these conflicts needed\n> to be deferred until process_entry() in order to get D/F conflicts and\n> such right. The number of these has grown over time, and now include\n> some whose name is misleading:\n> * conflict_rename_normal() is for handling normal file renames; a\n> typical rename may need content merging, but we expect conflicts\n> from that to be more the exception than the rule.\n> * conflict_rename_via_dir() will not be a conflict; it was just an\n> add that turned into a move due to directory rename detection.\n> (If there was a file in the way of the move, that would have been\n> detected and reported earlier.)\n> * conflict_rename_rename_2to1 and conflict_rename_add (the latter\n> of which doesn't exist yet but has been submitted before and I\n> intend to resend) technically might not be conflicts if the\n> colliding paths happen to match exactly.\n> Rename this family of functions to handle_rename_*().\n> \n> Also rename handle_renames() to detect_and_process_renames() both to make\n> it clearer what it does, and to differentiate it as a pre-processing step\n> from all the handle_rename_*() functions which are called from\n> process_entry().\n> \n> Signed-off-by: Elijah Newren \n\nMakes sense, and the patch looks obviously correct to me.\n\nThanks,\nDscho\n"},{"id":"348217","messageId":"CABPp-BGR1jaGzVcqyCO=WYgdX3SKQoj9pzuKCvXoc7+R=OA8PQ@mail.gmail.com","threadId":"48520","inReplyTo":"nycvar.QRO.7.76.6.1805211542160.77@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 2/5] merge-recursive: fix numerous argument alignment issues","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-05-21T16:48:17Z","receivedAt":"2018-05-21T16:48:22Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi Dscho,\n\nOn Mon, May 21, 2018 at 6:42 AM, Johannes Schindelin\n wrote:\n> Hi Elijah,\n>\n> On Fri, 18 May 2018, Elijah Newren wrote:\n>\n>> Various refactorings throughout the code have left lots of alignment\n>> issues that were driving me crazy; fix them.\n>\n> I hope you did not do that manually. What is your code formatting tool of\n> choice?\n\nSorry to disappoint but it was manual. I noticed and fixed one of\nthem many months ago, tossing it into a 'misc' branch. Then ran\nacross another and added it. When I hit the third, I was annoyed and\ncleaned them all up -- and combined them with other changes into this\nseries.\n\nHowever, it's hard to call this formatting entirely manual. A quick\nregex found the relevant sites pretty easily, and 'M-x indent-region'\n(emacs) fixes the indentation for a block of lines all at once. I\nguess if I had taken the time to fix a few other emacs formatting\nrules, I could have highlighted the whole file and ran C-M-\\ (a.k.a.\nindent-region), but didn't.\n\n> The patch looks obviously good to me.\n\nThanks for taking a look!\n"},{"id":"348222","messageId":"CABPp-BFHb+=1tWDH3GsiyYWCcbr83we46stjuU0D_Z_-H=Jx9A@mail.gmail.com","threadId":"48520","inReplyTo":"nycvar.QRO.7.76.6.1805211540120.77@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 5/5] merge-recursive: simplify handle_change_delete","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-05-21T17:22:40Z","receivedAt":"2018-05-21T17:22:45Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi Dscho,\n\nOn Mon, May 21, 2018 at 6:41 AM, Johannes Schindelin\n wrote:\n> On Sat, 19 May 2018, Elijah Newren wrote:\n>> On Sat, May 19, 2018 at 12:32 AM, Johannes Sixt wrote:\n\n>> >\n>> > Oh, there is a reason for the repeated message text: translations! Please do\n>> > not play sentence Lego with translated strings. The original code is\n>> > preferable.\n>>\n>> Ah, translations; that makes sense now. I'm still annoyed by the\n>> code, but I retract patch 5 then.\n>\n> Maybe you can remove the temptation for others, too, my replacing your\n> patch 5 with one that adds the code comment \"No sentence Lego! Think of\n> our poor translators and refrain from making their life miserable!\" or\n> some more appropriate one?\n\nThat sounds reasonable. I'll make that change (plus your other\nsuggested changes on other patches; thanks for all the reviews) in the\nnext round.\n\nI'm also considering adopting a setup_unpack_trees_porcelain() style\nof handling (from unpack-trees.c) for all these messages in\nmerge-recursive.c, but that's a bigger change that I'd probably put in\na separate series if I decide to go with it.\n\nElijah\n"},{"id":"348269","messageId":"20180522004327.13085-5-newren@gmail.com","threadId":"48520","inReplyTo":"20180522004327.13085-1-newren@gmail.com","subject":"[PATCH v2 4/5] merge-recursive: rename conflict_rename_*() family of functions","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-05-22T00:43:26Z","receivedAt":"2018-05-22T00:43:42Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"These functions were added because processing of these conflicts needed\nto be deferred until process_entry() in order to get D/F conflicts and\nsuch right. The number of these has grown over time, and now include\nsome whose name is misleading:\n * conflict_rename_normal() is for handling normal file renames; a\n typical rename may need content merging, but we expect conflicts\n from that to be more the exception than the rule.\n * conflict_rename_via_dir() will not be a conflict; it was just an\n add that turned into a move due to directory rename detection.\n (If there was a file in the way of the move, that would have been\n detected and reported earlier.)\n * conflict_rename_rename_2to1 and conflict_rename_add (the latter\n of which doesn't exist yet but has been submitted before and I\n intend to resend) technically might not be conflicts if the\n colliding paths happen to match exactly.\nRename this family of functions to handle_rename_*().\n\nAlso rename handle_renames() to detect_and_process_renames() both to make\nit clearer what it does, and to differentiate it as a pre-processing step\nfrom all the handle_rename_*() functions which are called from\nprocess_entry().\n\nAcked-by: Johannes Schindelin \nSigned-off-by: Elijah Newren \n---\n merge-recursive.c | 86 +++++++++++++++++++++++------------------------\n 1 file changed, 43 insertions(+), 43 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex e2b0db0645..270a4d2aad 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -1224,10 +1224,10 @@ static int merge_file_one(struct merge_options *o,\n \treturn merge_file_1(o, &one, &a, &b, path, branch1, branch2, mfi);\n }\n \n-static int conflict_rename_via_dir(struct merge_options *o,\n-\t\t\t\t struct diff_filepair *pair,\n-\t\t\t\t const char *rename_branch,\n-\t\t\t\t const char *other_branch)\n+static int handle_rename_via_dir(struct merge_options *o,\n+\t\t\t\t struct diff_filepair *pair,\n+\t\t\t\t const char *rename_branch,\n+\t\t\t\t const char *other_branch)\n {\n \t/*\n \t * Handle file adds that need to be renamed due to directory rename\n@@ -1329,10 +1329,10 @@ static int handle_change_delete(struct merge_options *o,\n \treturn ret;\n }\n \n-static int conflict_rename_delete(struct merge_options *o,\n-\t\t\t\t struct diff_filepair *pair,\n-\t\t\t\t const char *rename_branch,\n-\t\t\t\t const char *delete_branch)\n+static int handle_rename_delete(struct merge_options *o,\n+\t\t\t\tstruct diff_filepair *pair,\n+\t\t\t\tconst char *rename_branch,\n+\t\t\t\tconst char *delete_branch)\n {\n \tconst struct diff_filespec *orig = pair->one;\n \tconst struct diff_filespec *dest = pair->two;\n@@ -1434,8 +1434,8 @@ static int handle_file(struct merge_options *o,\n \treturn ret;\n }\n \n-static int conflict_rename_rename_1to2(struct merge_options *o,\n-\t\t\t\t struct rename_conflict_info *ci)\n+static int handle_rename_rename_1to2(struct merge_options *o,\n+\t\t\t\t struct rename_conflict_info *ci)\n {\n \t/* One file was renamed in both branches, but to different names. */\n \tstruct diff_filespec *one = ci->pair1->one;\n@@ -1496,8 +1496,8 @@ static int conflict_rename_rename_1to2(struct merge_options *o,\n \treturn 0;\n }\n \n-static int conflict_rename_rename_2to1(struct merge_options *o,\n-\t\t\t\t struct rename_conflict_info *ci)\n+static int handle_rename_rename_2to1(struct merge_options *o,\n+\t\t\t\t struct rename_conflict_info *ci)\n {\n \t/* Two files, a & b, were renamed to the same thing, c. */\n \tstruct diff_filespec *a = ci->pair1->one;\n@@ -2231,7 +2231,7 @@ static void apply_directory_rename_modifications(struct merge_options *o,\n \t * \"NOTE\" in update_stages(), doing so will modify the current\n \t * in-memory index which will break calls to would_lose_untracked()\n \t * that we need to make. Instead, we need to just make sure that\n-\t * the various conflict_rename_*() functions update the index\n+\t * the various handle_rename_*() functions update the index\n \t * explicitly rather than relying on unpack_trees() to have done it.\n \t */\n \tget_tree_entry(&tree->object.oid,\n@@ -2635,12 +2635,12 @@ static void initial_cleanup_rename(struct diff_queue_struct *pairs,\n \tfree(pairs);\n }\n \n-static int handle_renames(struct merge_options *o,\n-\t\t\t struct tree *common,\n-\t\t\t struct tree *head,\n-\t\t\t struct tree *merge,\n-\t\t\t struct string_list *entries,\n-\t\t\t struct rename_info *ri)\n+static int detect_and_process_renames(struct merge_options *o,\n+\t\t\t\t struct tree *common,\n+\t\t\t\t struct tree *head,\n+\t\t\t\t struct tree *merge,\n+\t\t\t\t struct string_list *entries,\n+\t\t\t\t struct rename_info *ri)\n {\n \tstruct diff_queue_struct *head_pairs, *merge_pairs;\n \tstruct hashmap *dir_re_head, *dir_re_merge;\n@@ -2911,12 +2911,12 @@ static int merge_content(struct merge_options *o,\n \treturn !is_dirty && mfi.clean;\n }\n \n-static int conflict_rename_normal(struct merge_options *o,\n-\t\t\t\t const char *path,\n-\t\t\t\t struct object_id *o_oid, unsigned int o_mode,\n-\t\t\t\t struct object_id *a_oid, unsigned int a_mode,\n-\t\t\t\t struct object_id *b_oid, unsigned int b_mode,\n-\t\t\t\t struct rename_conflict_info *ci)\n+static int handle_rename_normal(struct merge_options *o,\n+\t\t\t\tconst char *path,\n+\t\t\t\tstruct object_id *o_oid, unsigned int o_mode,\n+\t\t\t\tstruct object_id *a_oid, unsigned int a_mode,\n+\t\t\t\tstruct object_id *b_oid, unsigned int b_mode,\n+\t\t\t\tstruct rename_conflict_info *ci)\n {\n \t/* Merge the content and write it out */\n \treturn merge_content(o, path, was_dirty(o, path),\n@@ -2943,37 +2943,37 @@ static int process_entry(struct merge_options *o,\n \t\tswitch (conflict_info->rename_type) {\n \t\tcase RENAME_NORMAL:\n \t\tcase RENAME_ONE_FILE_TO_ONE:\n-\t\t\tclean_merge = conflict_rename_normal(o,\n-\t\t\t\t\t\t\t path,\n-\t\t\t\t\t\t\t o_oid, o_mode,\n-\t\t\t\t\t\t\t a_oid, a_mode,\n-\t\t\t\t\t\t\t b_oid, b_mode,\n-\t\t\t\t\t\t\t conflict_info);\n+\t\t\tclean_merge = handle_rename_normal(o,\n+\t\t\t\t\t\t\t path,\n+\t\t\t\t\t\t\t o_oid, o_mode,\n+\t\t\t\t\t\t\t a_oid, a_mode,\n+\t\t\t\t\t\t\t b_oid, b_mode,\n+\t\t\t\t\t\t\t conflict_info);\n \t\t\tbreak;\n \t\tcase RENAME_VIA_DIR:\n \t\t\tclean_merge = 1;\n-\t\t\tif (conflict_rename_via_dir(o,\n-\t\t\t\t\t\t conflict_info->pair1,\n-\t\t\t\t\t\t conflict_info->branch1,\n-\t\t\t\t\t\t conflict_info->branch2))\n+\t\t\tif (handle_rename_via_dir(o,\n+\t\t\t\t\t\t conflict_info->pair1,\n+\t\t\t\t\t\t conflict_info->branch1,\n+\t\t\t\t\t\t conflict_info->branch2))\n \t\t\t\tclean_merge = -1;\n \t\t\tbreak;\n \t\tcase RENAME_DELETE:\n \t\t\tclean_merge = 0;\n-\t\t\tif (conflict_rename_delete(o,\n-\t\t\t\t\t\t conflict_info->pair1,\n-\t\t\t\t\t\t conflict_info->branch1,\n-\t\t\t\t\t\t conflict_info->branch2))\n+\t\t\tif (handle_rename_delete(o,\n+\t\t\t\t\t\t conflict_info->pair1,\n+\t\t\t\t\t\t conflict_info->branch1,\n+\t\t\t\t\t\t conflict_info->branch2))\n \t\t\t\tclean_merge = -1;\n \t\t\tbreak;\n \t\tcase RENAME_ONE_FILE_TO_TWO:\n \t\t\tclean_merge = 0;\n-\t\t\tif (conflict_rename_rename_1to2(o, conflict_info))\n+\t\t\tif (handle_rename_rename_1to2(o, conflict_info))\n \t\t\t\tclean_merge = -1;\n \t\t\tbreak;\n \t\tcase RENAME_TWO_FILES_TO_ONE:\n \t\t\tclean_merge = 0;\n-\t\t\tif (conflict_rename_rename_2to1(o, conflict_info))\n+\t\t\tif (handle_rename_rename_2to1(o, conflict_info))\n \t\t\t\tclean_merge = -1;\n \t\t\tbreak;\n \t\tdefault:\n@@ -3112,8 +3112,8 @@ int merge_trees(struct merge_options *o,\n \t\tget_files_dirs(o, merge);\n \n \t\tentries = get_unmerged();\n-\t\tclean = handle_renames(o, common, head, merge, entries,\n-\t\t\t\t &re_info);\n+\t\tclean = detect_and_process_renames(o, common, head, merge,\n+\t\t\t\t\t\t entries, &re_info);\n \t\trecord_df_conflict_files(o, entries);\n \t\tif (clean < 0)\n \t\t\tgoto cleanup;\n-- \n2.17.0.847.g20b8963732\n\n"},{"id":"348270","messageId":"20180522004327.13085-6-newren@gmail.com","threadId":"48520","inReplyTo":"20180522004327.13085-1-newren@gmail.com","subject":"[PATCH v2 5/5] merge-recursive: add pointer about unduly complex looking code","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-05-22T00:43:27Z","receivedAt":"2018-05-22T00:43:43Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"handle_change_delete() has a block of code displaying one of four nearly\nidentical messages. Each contains about half a dozen variable interpolations,\nwhich use nearly identical variables as well. Someone trying to parse this\nmay be slowed down trying to parse the differences and why they are here; help\nthem out by adding a comment explaining the differences.\n\nFurther, point out that this code structure isn't collapsed into something\nmore concise and readable for the programmer, because we want to keep full\nmessages intact in order to make translators' jobs much easier.\n\nSigned-off-by: Elijah Newren \n---\n merge-recursive.c | 15 +++++++++++++++\n 1 file changed, 15 insertions(+)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 270a4d2aad..28f39b3e9f 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -1290,6 +1290,21 @@ static int handle_change_delete(struct merge_options *o,\n \t\tif (!ret)\n \t\t\tret = update_file(o, 0, o_oid, o_mode, update_path);\n \t} else {\n+\t\t/*\n+\t\t * Despite the four nearly duplicate messages and argument\n+\t\t * lists below and the ugliness of the nested if-statements,\n+\t\t * having complete messages makes the job easier for\n+\t\t * translators.\n+\t\t *\n+\t\t * The slight variance among the cases is due to the fact\n+\t\t * that:\n+\t\t * 1) directory/file conflicts (in effect if\n+\t\t * !alt_path) could cause us to need to write the\n+\t\t * file to a different path.\n+\t\t * 2) renames (in effect if !old_path) could mean that\n+\t\t * there are two names for the path that the user\n+\t\t * may know the file by.\n+\t\t */\n \t\tif (!alt_path) {\n \t\t\tif (!old_path) {\n \t\t\t\toutput(o, 1, _(\"CONFLICT (%s/delete): %s deleted in %s \"\n-- \n2.17.0.847.g20b8963732\n\n"},{"id":"348271","messageId":"20180522004327.13085-2-newren@gmail.com","threadId":"48520","inReplyTo":"20180522004327.13085-1-newren@gmail.com","subject":"[PATCH v2 1/5] merge-recursive: fix miscellaneous grammar error in comment","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-05-22T00:43:23Z","receivedAt":"2018-05-22T00:43:46Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Signed-off-by: Elijah Newren \n---\n merge-recursive.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 35df695fa4..c430fd72bc 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -531,7 +531,7 @@ static void record_df_conflict_files(struct merge_options *o,\n \t\t\t\t struct string_list *entries)\n {\n \t/* If there is a D/F conflict and the file for such a conflict\n-\t * currently exist in the working tree, we want to allow it to be\n+\t * currently exists in the working tree, we want to allow it to be\n \t * removed to make room for the corresponding directory if needed.\n \t * The files underneath the directories of such D/F conflicts will\n \t * be processed before the corresponding file involved in the D/F\n-- \n2.17.0.847.g20b8963732\n\n"},{"id":"348272","messageId":"20180522004327.13085-1-newren@gmail.com","threadId":"48520","inReplyTo":"20180519020700.2241-1-newren@gmail.com","subject":"[PATCH v2 0/5] merge-recursive code cleanups","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-05-22T00:43:22Z","receivedAt":"2018-05-22T00:43:47Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"This patch series contains several small code cleanups for merge-recursive.\nI have removed a couple small cleanup chunks in order to avoid conflicts\nwith any other in-flight topics in pu (namely, nd/commit-util-to-slab and\nsb/submodule-merge-in-merge-recursive). I may resend those later\nseparately.\n\nThe series was made on top of next (specifically commit 4fb28f7b1639\n(\"Merge branch 'sb/object-store-replace' into next\")); it will not\napply to master.\n\n\nChanges since v1:\n * As suggested by Dscho, replaced patch 5 with a pointer about the\n fact that the code is that way to help translators\n * Small rewordings or paragraph splittings suggested by Dscho\n * Added Dscho's Acked-by on relevant patches (2-4)\n\n\nElijah Newren (5):\n merge-recursive: fix miscellaneous grammar error in comment\n merge-recursive: fix numerous argument alignment issues\n merge-recursive: clarify the rename_dir/RENAME_DIR meaning\n merge-recursive: rename conflict_rename_*() family of functions\n merge-recursive: add pointer about unduly complex looking code\n\n merge-recursive.c | 152 ++++++++++++++++++++++++++--------------------\n 1 file changed, 87 insertions(+), 65 deletions(-)\n\n\nbranch-diff:\n 1: 1af1c7df17 = 1: 96225eddbb merge-recursive: fix miscellaneous grammar error in comment\n 2: 8ed7c8b588 ! 2: c63f72e96d merge-recursive: fix numerous argument alignment issues\n @@ -5,6 +5,7 @@\n Various refactorings throughout the code have left lots of alignment\n issues that were driving me crazy; fix them.\n \n + Acked-by: Johannes Schindelin \n Signed-off-by: Elijah Newren \n \n diff --git a/merge-recursive.c b/merge-recursive.c\n 3: e96b2cd9ae ! 3: 3d95e8b756 merge-recursive: clarify the rename_dir/RENAME_DIR meaning\n @@ -4,14 +4,17 @@\n \n We had an enum of rename types which included RENAME_DIR; this name felt\n misleading since it was not about an entire directory but was a status for\n - each individual file add that occurred within a renamed directory. Since\n - this type is for signifying that the files in question were being renamed\n - due to directory rename detection, rename this enum value to\n - RENAME_VIA_DIR. Make a similar change to the conflict_rename_dir()\n - function, and add a comment to the top of that function explaining its\n - purpose (it may not be quite as obvious as for the other\n - conflict_rename_*() functions).\n + each individual file add that occurred within a renamed directory.\n \n + Since this type is for signifying that the files in question were being\n + renamed due to directory rename detection, rename this enum value to\n + RENAME_VIA_DIR.\n +\n + Make a similar change to the conflict_rename_dir() function, and add a\n + comment to the top of that function explaining its purpose (it may not be\n + quite as obvious as for the other conflict_rename_*() functions).\n +\n + Acked-by: Johannes Schindelin \n Signed-off-by: Elijah Newren \n \n diff --git a/merge-recursive.c b/merge-recursive.c\n @@ -42,7 +45,7 @@\n +\t/*\n +\t * Handle file adds that need to be renamed due to directory rename\n +\t * detection. This differs from handle_rename_normal, because\n -+\t * there is no content merge to do; just move the path into the\n ++\t * there is no content merge to do; just move the file into the\n +\t * desired final location.\n +\t */\n \tconst struct diff_filespec *dest = pair->two;\n 4: 4155613a6b ! 4: dfd6ab22db merge-recursive: rename conflict_rename_*() family of functions\n @@ -24,6 +24,7 @@\n from all the handle_rename_*() functions which are called from\n process_entry().\n \n + Acked-by: Johannes Schindelin \n Signed-off-by: Elijah Newren \n \n diff --git a/merge-recursive.c b/merge-recursive.c\n 5: 20b8963732 < --: ---------- merge-recursive: simplify handle_change_delete\n--: ---------- > 5: 122629ef7a merge-recursive: add pointer about unduly complex looking code\n\n-- \n2.17.0.847.g20b8963732\n\n"},{"id":"348273","messageId":"20180522004327.13085-3-newren@gmail.com","threadId":"48520","inReplyTo":"20180522004327.13085-1-newren@gmail.com","subject":"[PATCH v2 2/5] merge-recursive: fix numerous argument alignment issues","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-05-22T00:43:24Z","receivedAt":"2018-05-22T00:43:52Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Various refactorings throughout the code have left lots of alignment\nissues that were driving me crazy; fix them.\n\nAcked-by: Johannes Schindelin \nSigned-off-by: Elijah Newren \n---\n merge-recursive.c | 47 ++++++++++++++++++++++++-----------------------\n 1 file changed, 24 insertions(+), 23 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex c430fd72bc..01306c87eb 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -308,8 +308,8 @@ static void output_commit_title(struct merge_options *o, struct commit *commit)\n }\n \n static int add_cacheinfo(struct merge_options *o,\n-\t\tunsigned int mode, const struct object_id *oid,\n-\t\tconst char *path, int stage, int refresh, int options)\n+\t\t\t unsigned int mode, const struct object_id *oid,\n+\t\t\t const char *path, int stage, int refresh, int options)\n {\n \tstruct cache_entry *ce;\n \tint ret;\n@@ -409,8 +409,8 @@ struct tree *write_tree_from_memory(struct merge_options *o)\n }\n \n static int save_files_dirs(const struct object_id *oid,\n-\t\tstruct strbuf *base, const char *path,\n-\t\tunsigned int mode, int stage, void *context)\n+\t\t\t struct strbuf *base, const char *path,\n+\t\t\t unsigned int mode, int stage, void *context)\n {\n \tstruct path_hashmap_entry *entry;\n \tint baselen = base->len;\n@@ -1257,13 +1257,13 @@ static int conflict_rename_dir(struct merge_options *o,\n }\n \n static int handle_change_delete(struct merge_options *o,\n-\t\t\t\t const char *path, const char *old_path,\n-\t\t\t\t const struct object_id *o_oid, int o_mode,\n-\t\t\t\t const struct object_id *changed_oid,\n-\t\t\t\t int changed_mode,\n-\t\t\t\t const char *change_branch,\n-\t\t\t\t const char *delete_branch,\n-\t\t\t\t const char *change, const char *change_past)\n+\t\t\t\tconst char *path, const char *old_path,\n+\t\t\t\tconst struct object_id *o_oid, int o_mode,\n+\t\t\t\tconst struct object_id *changed_oid,\n+\t\t\t\tint changed_mode,\n+\t\t\t\tconst char *change_branch,\n+\t\t\t\tconst char *delete_branch,\n+\t\t\t\tconst char *change, const char *change_past)\n {\n \tchar *alt_path = NULL;\n \tconst char *update_path = path;\n@@ -1324,9 +1324,9 @@ static int handle_change_delete(struct merge_options *o,\n }\n \n static int conflict_rename_delete(struct merge_options *o,\n-\t\t\t\t struct diff_filepair *pair,\n-\t\t\t\t const char *rename_branch,\n-\t\t\t\t const char *delete_branch)\n+\t\t\t\t struct diff_filepair *pair,\n+\t\t\t\t const char *rename_branch,\n+\t\t\t\t const char *delete_branch)\n {\n \tconst struct diff_filespec *orig = pair->one;\n \tconst struct diff_filespec *dest = pair->two;\n@@ -1429,7 +1429,7 @@ static int handle_file(struct merge_options *o,\n }\n \n static int conflict_rename_rename_1to2(struct merge_options *o,\n-\t\t\t\t\tstruct rename_conflict_info *ci)\n+\t\t\t\t struct rename_conflict_info *ci)\n {\n \t/* One file was renamed in both branches, but to different names. */\n \tstruct diff_filespec *one = ci->pair1->one;\n@@ -1491,7 +1491,7 @@ static int conflict_rename_rename_1to2(struct merge_options *o,\n }\n \n static int conflict_rename_rename_2to1(struct merge_options *o,\n-\t\t\t\t\tstruct rename_conflict_info *ci)\n+\t\t\t\t struct rename_conflict_info *ci)\n {\n \t/* Two files, a & b, were renamed to the same thing, c. */\n \tstruct diff_filespec *a = ci->pair1->one;\n@@ -2710,7 +2710,8 @@ static struct object_id *stage_oid(const struct object_id *oid, unsigned mode)\n }\n \n static int read_oid_strbuf(struct merge_options *o,\n-\tconst struct object_id *oid, struct strbuf *dst)\n+\t\t\t const struct object_id *oid,\n+\t\t\t struct strbuf *dst)\n {\n \tvoid *buf;\n \tenum object_type type;\n@@ -2763,10 +2764,10 @@ static int blob_unchanged(struct merge_options *opt,\n }\n \n static int handle_modify_delete(struct merge_options *o,\n-\t\t\t\t const char *path,\n-\t\t\t\t struct object_id *o_oid, int o_mode,\n-\t\t\t\t struct object_id *a_oid, int a_mode,\n-\t\t\t\t struct object_id *b_oid, int b_mode)\n+\t\t\t\tconst char *path,\n+\t\t\t\tstruct object_id *o_oid, int o_mode,\n+\t\t\t\tstruct object_id *a_oid, int a_mode,\n+\t\t\t\tstruct object_id *b_oid, int b_mode)\n {\n \tconst char *modify_branch, *delete_branch;\n \tstruct object_id *changed_oid;\n@@ -3303,14 +3304,14 @@ int merge_recursive_generic(struct merge_options *o,\n \t\t\tstruct commit *base;\n \t\t\tif (!(base = get_ref(base_list[i], oid_to_hex(base_list[i]))))\n \t\t\t\treturn err(o, _(\"Could not parse object '%s'\"),\n-\t\t\t\t\toid_to_hex(base_list[i]));\n+\t\t\t\t\t oid_to_hex(base_list[i]));\n \t\t\tcommit_list_insert(base, &ca);\n \t\t}\n \t}\n \n \thold_locked_index(&lock, LOCK_DIE_ON_ERROR);\n \tclean = merge_recursive(o, head_commit, next_commit, ca,\n-\t\t\tresult);\n+\t\t\t\tresult);\n \tif (clean < 0) {\n \t\trollback_lock_file(&lock);\n \t\treturn clean;\n-- \n2.17.0.847.g20b8963732\n\n"},{"id":"348274","messageId":"20180522004327.13085-4-newren@gmail.com","threadId":"48520","inReplyTo":"20180522004327.13085-1-newren@gmail.com","subject":"[PATCH v2 3/5] merge-recursive: clarify the rename_dir/RENAME_DIR meaning","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-05-22T00:43:25Z","receivedAt":"2018-05-22T00:43:53Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"We had an enum of rename types which included RENAME_DIR; this name felt\nmisleading since it was not about an entire directory but was a status for\neach individual file add that occurred within a renamed directory.\n\nSince this type is for signifying that the files in question were being\nrenamed due to directory rename detection, rename this enum value to\nRENAME_VIA_DIR.\n\nMake a similar change to the conflict_rename_dir() function, and add a\ncomment to the top of that function explaining its purpose (it may not be\nquite as obvious as for the other conflict_rename_*() functions).\n\nAcked-by: Johannes Schindelin \nSigned-off-by: Elijah Newren \n---\n merge-recursive.c | 28 +++++++++++++++++-----------\n 1 file changed, 17 insertions(+), 11 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 01306c87eb..e2b0db0645 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -180,7 +180,7 @@ static int oid_eq(const struct object_id *a, const struct object_id *b)\n \n enum rename_type {\n \tRENAME_NORMAL = 0,\n-\tRENAME_DIR,\n+\tRENAME_VIA_DIR,\n \tRENAME_DELETE,\n \tRENAME_ONE_FILE_TO_ONE,\n \tRENAME_ONE_FILE_TO_TWO,\n@@ -1224,11 +1224,17 @@ static int merge_file_one(struct merge_options *o,\n \treturn merge_file_1(o, &one, &a, &b, path, branch1, branch2, mfi);\n }\n \n-static int conflict_rename_dir(struct merge_options *o,\n-\t\t\t struct diff_filepair *pair,\n-\t\t\t const char *rename_branch,\n-\t\t\t const char *other_branch)\n+static int conflict_rename_via_dir(struct merge_options *o,\n+\t\t\t\t struct diff_filepair *pair,\n+\t\t\t\t const char *rename_branch,\n+\t\t\t\t const char *other_branch)\n {\n+\t/*\n+\t * Handle file adds that need to be renamed due to directory rename\n+\t * detection. This differs from handle_rename_normal, because\n+\t * there is no content merge to do; just move the file into the\n+\t * desired final location.\n+\t */\n \tconst struct diff_filespec *dest = pair->two;\n \n \tif (!o->call_depth && would_lose_untracked(dest->path)) {\n@@ -2498,7 +2504,7 @@ static int process_renames(struct merge_options *o,\n \n \t\t\tif (oid_eq(&src_other.oid, &null_oid) &&\n \t\t\t ren1->add_turned_into_rename) {\n-\t\t\t\tsetup_rename_conflict_info(RENAME_DIR,\n+\t\t\t\tsetup_rename_conflict_info(RENAME_VIA_DIR,\n \t\t\t\t\t\t\t ren1->pair,\n \t\t\t\t\t\t\t NULL,\n \t\t\t\t\t\t\t branch1,\n@@ -2944,12 +2950,12 @@ static int process_entry(struct merge_options *o,\n \t\t\t\t\t\t\t b_oid, b_mode,\n \t\t\t\t\t\t\t conflict_info);\n \t\t\tbreak;\n-\t\tcase RENAME_DIR:\n+\t\tcase RENAME_VIA_DIR:\n \t\t\tclean_merge = 1;\n-\t\t\tif (conflict_rename_dir(o,\n-\t\t\t\t\t\tconflict_info->pair1,\n-\t\t\t\t\t\tconflict_info->branch1,\n-\t\t\t\t\t\tconflict_info->branch2))\n+\t\t\tif (conflict_rename_via_dir(o,\n+\t\t\t\t\t\t conflict_info->pair1,\n+\t\t\t\t\t\t conflict_info->branch1,\n+\t\t\t\t\t\t conflict_info->branch2))\n \t\t\t\tclean_merge = -1;\n \t\t\tbreak;\n \t\tcase RENAME_DELETE:\n-- \n2.17.0.847.g20b8963732\n\n"},{"id":"349846","messageId":"20180610041616.4448-2-newren@gmail.com","threadId":"48520","inReplyTo":"20180610041616.4448-1-newren@gmail.com","subject":"[PATCH v3 1/6] merge-recursive: fix miscellaneous grammar error in comment","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-06-10T04:16:11Z","receivedAt":"2018-06-10T04:16:28Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Signed-off-by: Elijah Newren \n---\n merge-recursive.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex ac27abbd4c..921f8e2d2d 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -539,7 +539,7 @@ static void record_df_conflict_files(struct merge_options *o,\n \t\t\t\t struct string_list *entries)\n {\n \t/* If there is a D/F conflict and the file for such a conflict\n-\t * currently exist in the working tree, we want to allow it to be\n+\t * currently exists in the working tree, we want to allow it to be\n \t * removed to make room for the corresponding directory if needed.\n \t * The files underneath the directories of such D/F conflicts will\n \t * be processed before the corresponding file involved in the D/F\n-- \n2.18.0.rc1.6.gffeb3ef585\n\n"},{"id":"349847","messageId":"20180610041616.4448-3-newren@gmail.com","threadId":"48520","inReplyTo":"20180610041616.4448-1-newren@gmail.com","subject":"[PATCH v3 2/6] merge-recursive: fix numerous argument alignment issues","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-06-10T04:16:12Z","receivedAt":"2018-06-10T04:16:31Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Various refactorings throughout the code have left lots of alignment\nissues that were driving me crazy; fix them.\n\nAcked-by: Johannes Schindelin \nSigned-off-by: Elijah Newren \n---\n merge-recursive.c | 75 ++++++++++++++++++++++++-----------------------\n 1 file changed, 38 insertions(+), 37 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 921f8e2d2d..09c249b93e 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -309,8 +309,8 @@ static void output_commit_title(struct merge_options *o, struct commit *commit)\n }\n \n static int add_cacheinfo(struct merge_options *o,\n-\t\tunsigned int mode, const struct object_id *oid,\n-\t\tconst char *path, int stage, int refresh, int options)\n+\t\t\t unsigned int mode, const struct object_id *oid,\n+\t\t\t const char *path, int stage, int refresh, int options)\n {\n \tstruct cache_entry *ce;\n \tint ret;\n@@ -417,8 +417,8 @@ struct tree *write_tree_from_memory(struct merge_options *o)\n }\n \n static int save_files_dirs(const struct object_id *oid,\n-\t\tstruct strbuf *base, const char *path,\n-\t\tunsigned int mode, int stage, void *context)\n+\t\t\t struct strbuf *base, const char *path,\n+\t\t\t unsigned int mode, int stage, void *context)\n {\n \tstruct path_hashmap_entry *entry;\n \tint baselen = base->len;\n@@ -913,7 +913,7 @@ static int make_room_for_path(struct merge_options *o, const char *path)\n \t */\n \tif (would_lose_untracked(path))\n \t\treturn err(o, _(\"refusing to lose untracked file at '%s'\"),\n-\t\t\t path);\n+\t\t\t path);\n \n \t/* Successful unlink is good.. */\n \tif (!unlink(path))\n@@ -992,7 +992,7 @@ static int update_file_flags(struct merge_options *o,\n \t\t\tunlink(path);\n \t\t\tif (symlink(lnk, path))\n \t\t\t\tret = err(o, _(\"failed to symlink '%s': %s\"),\n-\t\t\t\t\tpath, strerror(errno));\n+\t\t\t\t\t path, strerror(errno));\n \t\t\tfree(lnk);\n \t\t} else\n \t\t\tret = err(o,\n@@ -1090,7 +1090,7 @@ static int merge_3way(struct merge_options *o,\n }\n \n static int find_first_merges(struct object_array *result, const char *path,\n-\t\tstruct commit *a, struct commit *b)\n+\t\t\t struct commit *a, struct commit *b)\n {\n \tint i, j;\n \tstruct object_array merges = OBJECT_ARRAY_INIT;\n@@ -1108,7 +1108,7 @@ static int find_first_merges(struct object_array *result, const char *path,\n \n \t/* get all revisions that merge commit a */\n \txsnprintf(merged_revision, sizeof(merged_revision), \"^%s\",\n-\t\t\toid_to_hex(&a->object.oid));\n+\t\t oid_to_hex(&a->object.oid));\n \tinit_revisions(&revs, NULL);\n \trev_opts.submodule = path;\n \t/* FIXME: can't handle linked worktrees in submodules yet */\n@@ -1250,12 +1250,12 @@ static int merge_submodule(struct merge_options *o,\n \t\toutput(o, 2, _(\"Found a possible merge resolution for the submodule:\\n\"));\n \t\tprint_commit((struct commit *) merges.objects[0].item);\n \t\toutput(o, 2, _(\n-\t\t\t\"If this is correct simply add it to the index \"\n-\t\t\t\"for example\\n\"\n-\t\t\t\"by using:\\n\\n\"\n-\t\t\t\" git update-index --cacheinfo 160000 %s \\\"%s\\\"\\n\\n\"\n-\t\t\t\"which will accept this suggestion.\\n\"),\n-\t\t\toid_to_hex(&merges.objects[0].item->oid), path);\n+\t\t \"If this is correct simply add it to the index \"\n+\t\t \"for example\\n\"\n+\t\t \"by using:\\n\\n\"\n+\t\t \" git update-index --cacheinfo 160000 %s \\\"%s\\\"\\n\\n\"\n+\t\t \"which will accept this suggestion.\\n\"),\n+\t\t oid_to_hex(&merges.objects[0].item->oid), path);\n \t\tbreak;\n \n \tdefault:\n@@ -1332,10 +1332,10 @@ static int merge_file_1(struct merge_options *o,\n \t\t\tresult->clean = (merge_status == 0);\n \t\t} else if (S_ISGITLINK(a->mode)) {\n \t\t\tresult->clean = merge_submodule(o, &result->oid,\n-\t\t\t\t\t\t one->path,\n-\t\t\t\t\t\t &one->oid,\n-\t\t\t\t\t\t &a->oid,\n-\t\t\t\t\t\t &b->oid);\n+\t\t\t\t\t\t\tone->path,\n+\t\t\t\t\t\t\t&one->oid,\n+\t\t\t\t\t\t\t&a->oid,\n+\t\t\t\t\t\t\t&b->oid);\n \t\t} else if (S_ISLNK(a->mode)) {\n \t\t\tswitch (o->recursive_variant) {\n \t\t\tcase MERGE_RECURSIVE_NORMAL:\n@@ -1443,13 +1443,13 @@ static int conflict_rename_dir(struct merge_options *o,\n }\n \n static int handle_change_delete(struct merge_options *o,\n-\t\t\t\t const char *path, const char *old_path,\n-\t\t\t\t const struct object_id *o_oid, int o_mode,\n-\t\t\t\t const struct object_id *changed_oid,\n-\t\t\t\t int changed_mode,\n-\t\t\t\t const char *change_branch,\n-\t\t\t\t const char *delete_branch,\n-\t\t\t\t const char *change, const char *change_past)\n+\t\t\t\tconst char *path, const char *old_path,\n+\t\t\t\tconst struct object_id *o_oid, int o_mode,\n+\t\t\t\tconst struct object_id *changed_oid,\n+\t\t\t\tint changed_mode,\n+\t\t\t\tconst char *change_branch,\n+\t\t\t\tconst char *delete_branch,\n+\t\t\t\tconst char *change, const char *change_past)\n {\n \tchar *alt_path = NULL;\n \tconst char *update_path = path;\n@@ -1510,9 +1510,9 @@ static int handle_change_delete(struct merge_options *o,\n }\n \n static int conflict_rename_delete(struct merge_options *o,\n-\t\t\t\t struct diff_filepair *pair,\n-\t\t\t\t const char *rename_branch,\n-\t\t\t\t const char *delete_branch)\n+\t\t\t\t struct diff_filepair *pair,\n+\t\t\t\t const char *rename_branch,\n+\t\t\t\t const char *delete_branch)\n {\n \tconst struct diff_filespec *orig = pair->one;\n \tconst struct diff_filespec *dest = pair->two;\n@@ -1615,7 +1615,7 @@ static int handle_file(struct merge_options *o,\n }\n \n static int conflict_rename_rename_1to2(struct merge_options *o,\n-\t\t\t\t\tstruct rename_conflict_info *ci)\n+\t\t\t\t struct rename_conflict_info *ci)\n {\n \t/* One file was renamed in both branches, but to different names. */\n \tstruct diff_filespec *one = ci->pair1->one;\n@@ -1677,7 +1677,7 @@ static int conflict_rename_rename_1to2(struct merge_options *o,\n }\n \n static int conflict_rename_rename_2to1(struct merge_options *o,\n-\t\t\t\t\tstruct rename_conflict_info *ci)\n+\t\t\t\t struct rename_conflict_info *ci)\n {\n \t/* Two files, a & b, were renamed to the same thing, c. */\n \tstruct diff_filespec *a = ci->pair1->one;\n@@ -2904,7 +2904,8 @@ static struct object_id *stage_oid(const struct object_id *oid, unsigned mode)\n }\n \n static int read_oid_strbuf(struct merge_options *o,\n-\tconst struct object_id *oid, struct strbuf *dst)\n+\t\t\t const struct object_id *oid,\n+\t\t\t struct strbuf *dst)\n {\n \tvoid *buf;\n \tenum object_type type;\n@@ -2957,10 +2958,10 @@ static int blob_unchanged(struct merge_options *opt,\n }\n \n static int handle_modify_delete(struct merge_options *o,\n-\t\t\t\t const char *path,\n-\t\t\t\t struct object_id *o_oid, int o_mode,\n-\t\t\t\t struct object_id *a_oid, int a_mode,\n-\t\t\t\t struct object_id *b_oid, int b_mode)\n+\t\t\t\tconst char *path,\n+\t\t\t\tstruct object_id *o_oid, int o_mode,\n+\t\t\t\tstruct object_id *a_oid, int a_mode,\n+\t\t\t\tstruct object_id *b_oid, int b_mode)\n {\n \tconst char *modify_branch, *delete_branch;\n \tstruct object_id *changed_oid;\n@@ -3493,14 +3494,14 @@ int merge_recursive_generic(struct merge_options *o,\n \t\t\tstruct commit *base;\n \t\t\tif (!(base = get_ref(base_list[i], oid_to_hex(base_list[i]))))\n \t\t\t\treturn err(o, _(\"Could not parse object '%s'\"),\n-\t\t\t\t\toid_to_hex(base_list[i]));\n+\t\t\t\t\t oid_to_hex(base_list[i]));\n \t\t\tcommit_list_insert(base, &ca);\n \t\t}\n \t}\n \n \thold_locked_index(&lock, LOCK_DIE_ON_ERROR);\n \tclean = merge_recursive(o, head_commit, next_commit, ca,\n-\t\t\tresult);\n+\t\t\t\tresult);\n \tif (clean < 0) {\n \t\trollback_lock_file(&lock);\n \t\treturn clean;\n-- \n2.18.0.rc1.6.gffeb3ef585\n\n"},{"id":"349848","messageId":"20180610041616.4448-1-newren@gmail.com","threadId":"48520","inReplyTo":"20180522004327.13085-1-newren@gmail.com","subject":"[PATCH v3 0/6] merge-recursive code cleanups","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-06-10T04:16:10Z","receivedAt":"2018-06-10T04:16:35Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"This patch series contains several small code cleanups for\nmerge-recursive.\n\nChanges since v2:\n * Now built on master (the topics this depended on have graduated);\n merges cleanly with next and pu.\n * Patch #2 has a few additional argument alignment fixes; some in\n code added by topics that have since graduated, and a couple that\n were overlooked previously.\n * Patch #3 is entirely new.\n * Re-wrapped the commit message of the final patch.\n (range-diff below)\n\nElijah Newren (6):\n merge-recursive: fix miscellaneous grammar error in comment\n merge-recursive: fix numerous argument alignment issues\n merge-recursive: align labels with their respective code blocks\n merge-recursive: clarify the rename_dir/RENAME_DIR meaning\n merge-recursive: rename conflict_rename_*() family of functions\n merge-recursive: add pointer about unduly complex looking code\n\n merge-recursive.c | 186 ++++++++++++++++++++++++++--------------------\n 1 file changed, 104 insertions(+), 82 deletions(-)\n\nrange-diff against v2:\n\n 1: 4222f174de = 1: 4222f174de merge-recursive: fix miscellaneous grammar error in comment\n 2: 4ebf93822d ! 2: 284a5fee3d merge-recursive: fix numerous argument alignment issues\n @@ -33,6 +33,76 @@\n {\n \tstruct path_hashmap_entry *entry;\n \tint baselen = base->len;\n +@@\n + \t */\n + \tif (would_lose_untracked(path))\n + \t\treturn err(o, _(\"refusing to lose untracked file at '%s'\"),\n +-\t\t\t path);\n ++\t\t\t path);\n + \n + \t/* Successful unlink is good.. */\n + \tif (!unlink(path))\n +@@\n + \t\t\tunlink(path);\n + \t\t\tif (symlink(lnk, path))\n + \t\t\t\tret = err(o, _(\"failed to symlink '%s': %s\"),\n +-\t\t\t\t\tpath, strerror(errno));\n ++\t\t\t\t\t path, strerror(errno));\n + \t\t\tfree(lnk);\n + \t\t} else\n + \t\t\tret = err(o,\n +@@\n + }\n + \n + static int find_first_merges(struct object_array *result, const char *path,\n +-\t\tstruct commit *a, struct commit *b)\n ++\t\t\t struct commit *a, struct commit *b)\n + {\n + \tint i, j;\n + \tstruct object_array merges = OBJECT_ARRAY_INIT;\n +@@\n + \n + \t/* get all revisions that merge commit a */\n + \txsnprintf(merged_revision, sizeof(merged_revision), \"^%s\",\n +-\t\t\toid_to_hex(&a->object.oid));\n ++\t\t oid_to_hex(&a->object.oid));\n + \tinit_revisions(&revs, NULL);\n + \trev_opts.submodule = path;\n + \t/* FIXME: can't handle linked worktrees in submodules yet */\n +@@\n + \t\toutput(o, 2, _(\"Found a possible merge resolution for the submodule:\\n\"));\n + \t\tprint_commit((struct commit *) merges.objects[0].item);\n + \t\toutput(o, 2, _(\n +-\t\t\t\"If this is correct simply add it to the index \"\n +-\t\t\t\"for example\\n\"\n +-\t\t\t\"by using:\\n\\n\"\n +-\t\t\t\" git update-index --cacheinfo 160000 %s \\\"%s\\\"\\n\\n\"\n +-\t\t\t\"which will accept this suggestion.\\n\"),\n +-\t\t\toid_to_hex(&merges.objects[0].item->oid), path);\n ++\t\t \"If this is correct simply add it to the index \"\n ++\t\t \"for example\\n\"\n ++\t\t \"by using:\\n\\n\"\n ++\t\t \" git update-index --cacheinfo 160000 %s \\\"%s\\\"\\n\\n\"\n ++\t\t \"which will accept this suggestion.\\n\"),\n ++\t\t oid_to_hex(&merges.objects[0].item->oid), path);\n + \t\tbreak;\n + \n + \tdefault:\n +@@\n + \t\t\tresult->clean = (merge_status == 0);\n + \t\t} else if (S_ISGITLINK(a->mode)) {\n + \t\t\tresult->clean = merge_submodule(o, &result->oid,\n +-\t\t\t\t\t\t one->path,\n +-\t\t\t\t\t\t &one->oid,\n +-\t\t\t\t\t\t &a->oid,\n +-\t\t\t\t\t\t &b->oid);\n ++\t\t\t\t\t\t\tone->path,\n ++\t\t\t\t\t\t\t&one->oid,\n ++\t\t\t\t\t\t\t&a->oid,\n ++\t\t\t\t\t\t\t&b->oid);\n + \t\t} else if (S_ISLNK(a->mode)) {\n + \t\t\tswitch (o->recursive_variant) {\n + \t\t\tcase MERGE_RECURSIVE_NORMAL:\n @@\n }\n \n--: ---------- > 3: 6bae2a267f merge-recursive: align labels with their respective code blocks\n 3: 585759f07a = 4: aecf1267d8 merge-recursive: clarify the rename_dir/RENAME_DIR meaning\n 4: 3cfb8b01b8 = 5: f7637bef12 merge-recursive: rename conflict_rename_*() family of functions\n 5: d2a24f5b38 ! 6: ffeb3ef585 merge-recursive: add pointer about unduly complex looking code\n @@ -3,10 +3,11 @@\n merge-recursive: add pointer about unduly complex looking code\n \n handle_change_delete() has a block of code displaying one of four nearly\n - identical messages. Each contains about half a dozen variable interpolations,\n - which use nearly identical variables as well. Someone trying to parse this\n - may be slowed down trying to parse the differences and why they are here; help\n - them out by adding a comment explaining the differences.\n + identical messages. Each contains about half a dozen variable\n + interpolations, which use nearly identical variables as well. Someone\n + trying to parse this may be slowed down trying to parse the differences\n + and why they are here; help them out by adding a comment explaining the\n + differences.\n \n Further, point out that this code structure isn't collapsed into something\n more concise and readable for the programmer, because we want to keep full\n\n\n-- \n2.18.0.rc1.6.gffeb3ef585\n"},{"id":"349849","messageId":"20180610041616.4448-5-newren@gmail.com","threadId":"48520","inReplyTo":"20180610041616.4448-1-newren@gmail.com","subject":"[PATCH v3 4/6] merge-recursive: clarify the rename_dir/RENAME_DIR meaning","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-06-10T04:16:14Z","receivedAt":"2018-06-10T04:16:36Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"We had an enum of rename types which included RENAME_DIR; this name felt\nmisleading since it was not about an entire directory but was a status for\neach individual file add that occurred within a renamed directory.\n\nSince this type is for signifying that the files in question were being\nrenamed due to directory rename detection, rename this enum value to\nRENAME_VIA_DIR.\n\nMake a similar change to the conflict_rename_dir() function, and add a\ncomment to the top of that function explaining its purpose (it may not be\nquite as obvious as for the other conflict_rename_*() functions).\n\nAcked-by: Johannes Schindelin \nSigned-off-by: Elijah Newren \n---\n merge-recursive.c | 28 +++++++++++++++++-----------\n 1 file changed, 17 insertions(+), 11 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 5538038178..910f0b70f0 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -181,7 +181,7 @@ static int oid_eq(const struct object_id *a, const struct object_id *b)\n \n enum rename_type {\n \tRENAME_NORMAL = 0,\n-\tRENAME_DIR,\n+\tRENAME_VIA_DIR,\n \tRENAME_DELETE,\n \tRENAME_ONE_FILE_TO_ONE,\n \tRENAME_ONE_FILE_TO_TWO,\n@@ -1410,11 +1410,17 @@ static int merge_file_one(struct merge_options *o,\n \treturn merge_file_1(o, &one, &a, &b, path, branch1, branch2, mfi);\n }\n \n-static int conflict_rename_dir(struct merge_options *o,\n-\t\t\t struct diff_filepair *pair,\n-\t\t\t const char *rename_branch,\n-\t\t\t const char *other_branch)\n+static int conflict_rename_via_dir(struct merge_options *o,\n+\t\t\t\t struct diff_filepair *pair,\n+\t\t\t\t const char *rename_branch,\n+\t\t\t\t const char *other_branch)\n {\n+\t/*\n+\t * Handle file adds that need to be renamed due to directory rename\n+\t * detection. This differs from handle_rename_normal, because\n+\t * there is no content merge to do; just move the file into the\n+\t * desired final location.\n+\t */\n \tconst struct diff_filespec *dest = pair->two;\n \n \tif (!o->call_depth && would_lose_untracked(dest->path)) {\n@@ -2692,7 +2698,7 @@ static int process_renames(struct merge_options *o,\n \n \t\t\tif (oid_eq(&src_other.oid, &null_oid) &&\n \t\t\t ren1->add_turned_into_rename) {\n-\t\t\t\tsetup_rename_conflict_info(RENAME_DIR,\n+\t\t\t\tsetup_rename_conflict_info(RENAME_VIA_DIR,\n \t\t\t\t\t\t\t ren1->pair,\n \t\t\t\t\t\t\t NULL,\n \t\t\t\t\t\t\t branch1,\n@@ -3138,12 +3144,12 @@ static int process_entry(struct merge_options *o,\n \t\t\t\t\t\t\t b_oid, b_mode,\n \t\t\t\t\t\t\t conflict_info);\n \t\t\tbreak;\n-\t\tcase RENAME_DIR:\n+\t\tcase RENAME_VIA_DIR:\n \t\t\tclean_merge = 1;\n-\t\t\tif (conflict_rename_dir(o,\n-\t\t\t\t\t\tconflict_info->pair1,\n-\t\t\t\t\t\tconflict_info->branch1,\n-\t\t\t\t\t\tconflict_info->branch2))\n+\t\t\tif (conflict_rename_via_dir(o,\n+\t\t\t\t\t\t conflict_info->pair1,\n+\t\t\t\t\t\t conflict_info->branch1,\n+\t\t\t\t\t\t conflict_info->branch2))\n \t\t\t\tclean_merge = -1;\n \t\t\tbreak;\n \t\tcase RENAME_DELETE:\n-- \n2.18.0.rc1.6.gffeb3ef585\n\n"},{"id":"349850","messageId":"20180610041616.4448-4-newren@gmail.com","threadId":"48520","inReplyTo":"20180610041616.4448-1-newren@gmail.com","subject":"[PATCH v3 3/6] merge-recursive: align labels with their respective code blocks","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-06-10T04:16:13Z","receivedAt":"2018-06-10T04:16:40Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Signed-off-by: Elijah Newren \n---\n merge-recursive.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 09c249b93e..5538038178 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -998,10 +998,10 @@ static int update_file_flags(struct merge_options *o,\n \t\t\tret = err(o,\n \t\t\t\t _(\"do not know what to do with %06o %s '%s'\"),\n \t\t\t\t mode, oid_to_hex(oid), path);\n- free_buf:\n+\tfree_buf:\n \t\tfree(buf);\n \t}\n- update_index:\n+update_index:\n \tif (!ret && update_cache)\n \t\tif (add_cacheinfo(o, mode, oid, path, 0, update_wd,\n \t\t\t\t ADD_CACHE_OK_TO_ADD))\n@@ -3326,7 +3326,7 @@ int merge_trees(struct merge_options *o,\n \t\t\t\t entries->items[i].string);\n \t\t}\n \n-cleanup:\n+\tcleanup:\n \t\tfinal_cleanup_renames(&re_info);\n \n \t\tstring_list_clear(entries, 1);\n-- \n2.18.0.rc1.6.gffeb3ef585\n\n"},{"id":"349851","messageId":"20180610041616.4448-7-newren@gmail.com","threadId":"48520","inReplyTo":"20180610041616.4448-1-newren@gmail.com","subject":"[PATCH v3 6/6] merge-recursive: add pointer about unduly complex looking code","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-06-10T04:16:16Z","receivedAt":"2018-06-10T04:16:42Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"handle_change_delete() has a block of code displaying one of four nearly\nidentical messages. Each contains about half a dozen variable\ninterpolations, which use nearly identical variables as well. Someone\ntrying to parse this may be slowed down trying to parse the differences\nand why they are here; help them out by adding a comment explaining the\ndifferences.\n\nFurther, point out that this code structure isn't collapsed into something\nmore concise and readable for the programmer, because we want to keep full\nmessages intact in order to make translators' jobs much easier.\n\nSigned-off-by: Elijah Newren \n---\n merge-recursive.c | 15 +++++++++++++++\n 1 file changed, 15 insertions(+)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 7cf11dc04c..284b27d895 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -1476,6 +1476,21 @@ static int handle_change_delete(struct merge_options *o,\n \t\tif (!ret)\n \t\t\tret = update_file(o, 0, o_oid, o_mode, update_path);\n \t} else {\n+\t\t/*\n+\t\t * Despite the four nearly duplicate messages and argument\n+\t\t * lists below and the ugliness of the nested if-statements,\n+\t\t * having complete messages makes the job easier for\n+\t\t * translators.\n+\t\t *\n+\t\t * The slight variance among the cases is due to the fact\n+\t\t * that:\n+\t\t * 1) directory/file conflicts (in effect if\n+\t\t * !alt_path) could cause us to need to write the\n+\t\t * file to a different path.\n+\t\t * 2) renames (in effect if !old_path) could mean that\n+\t\t * there are two names for the path that the user\n+\t\t * may know the file by.\n+\t\t */\n \t\tif (!alt_path) {\n \t\t\tif (!old_path) {\n \t\t\t\toutput(o, 1, _(\"CONFLICT (%s/delete): %s deleted in %s \"\n-- \n2.18.0.rc1.6.gffeb3ef585\n\n"},{"id":"349852","messageId":"20180610041616.4448-6-newren@gmail.com","threadId":"48520","inReplyTo":"20180610041616.4448-1-newren@gmail.com","subject":"[PATCH v3 5/6] merge-recursive: rename conflict_rename_*() family of functions","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-06-10T04:16:15Z","receivedAt":"2018-06-10T04:16:45Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"These functions were added because processing of these conflicts needed\nto be deferred until process_entry() in order to get D/F conflicts and\nsuch right. The number of these has grown over time, and now include\nsome whose name is misleading:\n * conflict_rename_normal() is for handling normal file renames; a\n typical rename may need content merging, but we expect conflicts\n from that to be more the exception than the rule.\n * conflict_rename_via_dir() will not be a conflict; it was just an\n add that turned into a move due to directory rename detection.\n (If there was a file in the way of the move, that would have been\n detected and reported earlier.)\n * conflict_rename_rename_2to1 and conflict_rename_add (the latter\n of which doesn't exist yet but has been submitted before and I\n intend to resend) technically might not be conflicts if the\n colliding paths happen to match exactly.\nRename this family of functions to handle_rename_*().\n\nAlso rename handle_renames() to detect_and_process_renames() both to make\nit clearer what it does, and to differentiate it as a pre-processing step\nfrom all the handle_rename_*() functions which are called from\nprocess_entry().\n\nAcked-by: Johannes Schindelin \nSigned-off-by: Elijah Newren \n---\n merge-recursive.c | 86 +++++++++++++++++++++++------------------------\n 1 file changed, 43 insertions(+), 43 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 910f0b70f0..7cf11dc04c 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -1410,10 +1410,10 @@ static int merge_file_one(struct merge_options *o,\n \treturn merge_file_1(o, &one, &a, &b, path, branch1, branch2, mfi);\n }\n \n-static int conflict_rename_via_dir(struct merge_options *o,\n-\t\t\t\t struct diff_filepair *pair,\n-\t\t\t\t const char *rename_branch,\n-\t\t\t\t const char *other_branch)\n+static int handle_rename_via_dir(struct merge_options *o,\n+\t\t\t\t struct diff_filepair *pair,\n+\t\t\t\t const char *rename_branch,\n+\t\t\t\t const char *other_branch)\n {\n \t/*\n \t * Handle file adds that need to be renamed due to directory rename\n@@ -1515,10 +1515,10 @@ static int handle_change_delete(struct merge_options *o,\n \treturn ret;\n }\n \n-static int conflict_rename_delete(struct merge_options *o,\n-\t\t\t\t struct diff_filepair *pair,\n-\t\t\t\t const char *rename_branch,\n-\t\t\t\t const char *delete_branch)\n+static int handle_rename_delete(struct merge_options *o,\n+\t\t\t\tstruct diff_filepair *pair,\n+\t\t\t\tconst char *rename_branch,\n+\t\t\t\tconst char *delete_branch)\n {\n \tconst struct diff_filespec *orig = pair->one;\n \tconst struct diff_filespec *dest = pair->two;\n@@ -1620,8 +1620,8 @@ static int handle_file(struct merge_options *o,\n \treturn ret;\n }\n \n-static int conflict_rename_rename_1to2(struct merge_options *o,\n-\t\t\t\t struct rename_conflict_info *ci)\n+static int handle_rename_rename_1to2(struct merge_options *o,\n+\t\t\t\t struct rename_conflict_info *ci)\n {\n \t/* One file was renamed in both branches, but to different names. */\n \tstruct diff_filespec *one = ci->pair1->one;\n@@ -1682,8 +1682,8 @@ static int conflict_rename_rename_1to2(struct merge_options *o,\n \treturn 0;\n }\n \n-static int conflict_rename_rename_2to1(struct merge_options *o,\n-\t\t\t\t struct rename_conflict_info *ci)\n+static int handle_rename_rename_2to1(struct merge_options *o,\n+\t\t\t\t struct rename_conflict_info *ci)\n {\n \t/* Two files, a & b, were renamed to the same thing, c. */\n \tstruct diff_filespec *a = ci->pair1->one;\n@@ -2425,7 +2425,7 @@ static void apply_directory_rename_modifications(struct merge_options *o,\n \t * \"NOTE\" in update_stages(), doing so will modify the current\n \t * in-memory index which will break calls to would_lose_untracked()\n \t * that we need to make. Instead, we need to just make sure that\n-\t * the various conflict_rename_*() functions update the index\n+\t * the various handle_rename_*() functions update the index\n \t * explicitly rather than relying on unpack_trees() to have done it.\n \t */\n \tget_tree_entry(&tree->object.oid,\n@@ -2829,12 +2829,12 @@ static void initial_cleanup_rename(struct diff_queue_struct *pairs,\n \tfree(pairs);\n }\n \n-static int handle_renames(struct merge_options *o,\n-\t\t\t struct tree *common,\n-\t\t\t struct tree *head,\n-\t\t\t struct tree *merge,\n-\t\t\t struct string_list *entries,\n-\t\t\t struct rename_info *ri)\n+static int detect_and_process_renames(struct merge_options *o,\n+\t\t\t\t struct tree *common,\n+\t\t\t\t struct tree *head,\n+\t\t\t\t struct tree *merge,\n+\t\t\t\t struct string_list *entries,\n+\t\t\t\t struct rename_info *ri)\n {\n \tstruct diff_queue_struct *head_pairs, *merge_pairs;\n \tstruct hashmap *dir_re_head, *dir_re_merge;\n@@ -3105,12 +3105,12 @@ static int merge_content(struct merge_options *o,\n \treturn !is_dirty && mfi.clean;\n }\n \n-static int conflict_rename_normal(struct merge_options *o,\n-\t\t\t\t const char *path,\n-\t\t\t\t struct object_id *o_oid, unsigned int o_mode,\n-\t\t\t\t struct object_id *a_oid, unsigned int a_mode,\n-\t\t\t\t struct object_id *b_oid, unsigned int b_mode,\n-\t\t\t\t struct rename_conflict_info *ci)\n+static int handle_rename_normal(struct merge_options *o,\n+\t\t\t\tconst char *path,\n+\t\t\t\tstruct object_id *o_oid, unsigned int o_mode,\n+\t\t\t\tstruct object_id *a_oid, unsigned int a_mode,\n+\t\t\t\tstruct object_id *b_oid, unsigned int b_mode,\n+\t\t\t\tstruct rename_conflict_info *ci)\n {\n \t/* Merge the content and write it out */\n \treturn merge_content(o, path, was_dirty(o, path),\n@@ -3137,37 +3137,37 @@ static int process_entry(struct merge_options *o,\n \t\tswitch (conflict_info->rename_type) {\n \t\tcase RENAME_NORMAL:\n \t\tcase RENAME_ONE_FILE_TO_ONE:\n-\t\t\tclean_merge = conflict_rename_normal(o,\n-\t\t\t\t\t\t\t path,\n-\t\t\t\t\t\t\t o_oid, o_mode,\n-\t\t\t\t\t\t\t a_oid, a_mode,\n-\t\t\t\t\t\t\t b_oid, b_mode,\n-\t\t\t\t\t\t\t conflict_info);\n+\t\t\tclean_merge = handle_rename_normal(o,\n+\t\t\t\t\t\t\t path,\n+\t\t\t\t\t\t\t o_oid, o_mode,\n+\t\t\t\t\t\t\t a_oid, a_mode,\n+\t\t\t\t\t\t\t b_oid, b_mode,\n+\t\t\t\t\t\t\t conflict_info);\n \t\t\tbreak;\n \t\tcase RENAME_VIA_DIR:\n \t\t\tclean_merge = 1;\n-\t\t\tif (conflict_rename_via_dir(o,\n-\t\t\t\t\t\t conflict_info->pair1,\n-\t\t\t\t\t\t conflict_info->branch1,\n-\t\t\t\t\t\t conflict_info->branch2))\n+\t\t\tif (handle_rename_via_dir(o,\n+\t\t\t\t\t\t conflict_info->pair1,\n+\t\t\t\t\t\t conflict_info->branch1,\n+\t\t\t\t\t\t conflict_info->branch2))\n \t\t\t\tclean_merge = -1;\n \t\t\tbreak;\n \t\tcase RENAME_DELETE:\n \t\t\tclean_merge = 0;\n-\t\t\tif (conflict_rename_delete(o,\n-\t\t\t\t\t\t conflict_info->pair1,\n-\t\t\t\t\t\t conflict_info->branch1,\n-\t\t\t\t\t\t conflict_info->branch2))\n+\t\t\tif (handle_rename_delete(o,\n+\t\t\t\t\t\t conflict_info->pair1,\n+\t\t\t\t\t\t conflict_info->branch1,\n+\t\t\t\t\t\t conflict_info->branch2))\n \t\t\t\tclean_merge = -1;\n \t\t\tbreak;\n \t\tcase RENAME_ONE_FILE_TO_TWO:\n \t\t\tclean_merge = 0;\n-\t\t\tif (conflict_rename_rename_1to2(o, conflict_info))\n+\t\t\tif (handle_rename_rename_1to2(o, conflict_info))\n \t\t\t\tclean_merge = -1;\n \t\t\tbreak;\n \t\tcase RENAME_TWO_FILES_TO_ONE:\n \t\t\tclean_merge = 0;\n-\t\t\tif (conflict_rename_rename_2to1(o, conflict_info))\n+\t\t\tif (handle_rename_rename_2to1(o, conflict_info))\n \t\t\t\tclean_merge = -1;\n \t\t\tbreak;\n \t\tdefault:\n@@ -3307,8 +3307,8 @@ int merge_trees(struct merge_options *o,\n \t\tget_files_dirs(o, merge);\n \n \t\tentries = get_unmerged();\n-\t\tclean = handle_renames(o, common, head, merge, entries,\n-\t\t\t\t &re_info);\n+\t\tclean = detect_and_process_renames(o, common, head, merge,\n+\t\t\t\t\t\t entries, &re_info);\n \t\trecord_df_conflict_files(o, entries);\n \t\tif (clean < 0)\n \t\t\tgoto cleanup;\n-- \n2.18.0.rc1.6.gffeb3ef585\n\n"}]}