[PATCH v1] builtin/merge.c - cleanup of code in for-cycle that tests strategies

[Edmundo Carmona Antoranz <eantoranz@gmail.com>]

Previous code was a little convoluted to follow logic
New code is shorter and logic is easier to follow

- Easier to see what happens when merge is successful
	and how --no-commit affects result
- Simpler to see that for-cycle will stop when merge_was_ok is set
- Easier to spot what logic will run through best_strategy
- Easier to see that in case of ret being 2, cycle will continue
- Keep a single break case (when automerge succedes and a revision will
  be created)
- Put together closing actions when automerge succedes if a revision
  will be created

Signed-off-by: Edmundo Carmona Antoranz <eantoranz@gmail.com>
---
 builtin/merge.c | 51 ++++++++++++++++++-------------------------------
 1 file changed, 19 insertions(+), 32 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index 6e99aead46..94f2713bea 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1586,7 +1586,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	    save_state(&stash))
 		oidclr(&stash);
 
-	for (i = 0; i < use_strategies_nr; i++) {
+	for (i = 0; !merge_was_ok && i < use_strategies_nr; i++) {
 		int ret;
 		if (i) {
 			printf(_("Rewinding the tree to pristine...\n"));
@@ -1604,40 +1604,26 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		ret = try_merge_strategy(use_strategies[i]->name,
 					 common, remoteheads,
 					 head_commit);
-		if (!option_commit && !ret) {
-			merge_was_ok = 1;
-			/*
-			 * This is necessary here just to avoid writing
-			 * the tree, but later we will *not* exit with
-			 * status code 1 because merge_was_ok is set.
-			 */
-			ret = 1;
-		}
-
-		if (ret) {
-			/*
-			 * The backend exits with 1 when conflicts are
-			 * left to be resolved, with 2 when it does not
-			 * handle the given merge at all.
-			 */
-			if (ret == 1) {
-				int cnt = evaluate_result();
-
-				if (best_cnt <= 0 || cnt <= best_cnt) {
-					best_strategy = use_strategies[i]->name;
-					best_cnt = cnt;
+		/*
+		 * The backend exits with 1 when conflicts are
+		 * left to be resolved, with 2 when it does not
+		 * handle the given merge at all.
+		 */
+		if (ret < 2) {
+			if (!ret) {
+				if (option_commit) {
+					/* Automerge succeeded. */
+					automerge_was_ok = 1;
+					break;
 				}
+				merge_was_ok = 1;
+			}
+			int cnt = evaluate_result();
+			if (best_cnt <= 0 || cnt <= best_cnt) {
+				best_strategy = use_strategies[i]->name;
+				best_cnt = cnt;
 			}
-			if (merge_was_ok)
-				break;
-			else
-				continue;
 		}
-
-		/* Automerge succeeded. */
-		write_tree_trivial(&result_tree);
-		automerge_was_ok = 1;
-		break;
 	}
 
 	/*
@@ -1645,6 +1631,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	 * auto resolved the merge cleanly.
 	 */
 	if (automerge_was_ok) {
+		write_tree_trivial(&result_tree);
 		ret = finish_automerge(head_commit, head_subsumed,
 				       common, remoteheads,
 				       &result_tree, wt_strategy);
-- 
2.20.1