threads / patch / 39147

patchmerge: simplify code flow

Subject: [PATCH] merge: simplify code flow

## tl;dr

3 messages between Apr 23, 2015 and Apr 23, 2015. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Junio C Hamano· Apr 23, 2015, 20:01 UTC · lore

One of the first things cmd_merge() does is to see if the "--abort" option is given and run "reset --merge" and exit. When the control reaches this point, we know "--abort" was not given.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/merge.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

Show changes to builtin/merge.c +8 −8

diff --git a/builtin/merge.c b/builtin/merge.c
index 3b0f8f9..0ab7142 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1158,15 +1158,15 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		option_commit = 0;
 	}
 
-	if (!abort_current_merge) {
-		if (!argc) {
-			if (default_to_upstream)
-				argc = setup_with_upstream(&argv);
-			else
-				die(_("No commit specified and merge.defaultToUpstream not set."));
-		} else if (argc == 1 && !strcmp(argv[0], "-"))
-			argv[0] = "@{-1}";
+	if (!argc) {
+		if (default_to_upstream)
+			argc = setup_with_upstream(&argv);
+		else
+			die(_("No commit specified and merge.defaultToUpstream not set."));
+	} else if (argc == 1 && !strcmp(argv[0], "-")) {
+		argv[0] = "@{-1}";
 	}
+
 	if (!argc)
 		usage_with_options(builtin_merge_usage,
 			builtin_merge_options);

Jeff King· Apr 23, 2015, 20:10 UTC · re: Junio C Hamano · lore

Re: [PATCH] merge: simplify code flow

On Thu, Apr 23, 2015 at 01:01:44PM -0700, Junio C Hamano wrote:

Show 5 quoted lines

> One of the first things cmd_merge() does is to see if the "--abort"
> option is given and run "reset --merge" and exit.  When the control
> reaches this point, we know "--abort" was not given.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Yeah, this looks obviously correct.

As an aside, the fact that the actual "if (abort_current_merge)" conditional ends with a "goto" to a cleanup label that is 500 lines away is...kind of gross.

-Peff

Junio C Hamano· Apr 23, 2015, 20:15 UTC · re: Jeff King · lore

Re: [PATCH] merge: simplify code flow

Jeff King <peff@peff.net> writes:

> Yeah, this looks obviously correct.

It is funny that this dates back to the very beginning of defaulting to the upstream, the code that "if ()" block implements, introduced at 93e535a5 (merge: merge with the default upstream branch without argument, 2011-03-23).

> As an aside, the fact that the actual "if (abort_current_merge)"
> conditional ends with a "goto" to a cleanup label that is 500 lines away
> is...kind of gross.

Reading "git show 93e535a5" output, I see that it is a more recent "breakage" ;-)