Re: [PATCH 2/3] rebase --rebase-merges: add support for octopus merges

Hi Eric,

On Wed, 11 Jul 2018, Eric Sunshine wrote:

On Wed, Jul 11, 2018 at 8:38 AM Johannes Schindelin via GitGitGadget
[off-list ref] wrote:

quoted

diff --git a/sequencer.c b/sequencer.c

@@ -2932,7 +2966,8 @@ static int do_merge(struct commit *commit, const char *arg, int arg_len,
-                       strbuf_addf(&buf, "Merge branch '%.*s'",
+                       strbuf_addf(&buf, "Merge %s '%.*s'",
+                                   to_merge->next ? "branches" : "branch",

Error messages in this file are already localizable. If this text ever
becomes localizable, then this "sentence lego" could be problematic
for translators.

As pointed out by Junio, this is not really a message eligible for
localization, at least not right now. If it becomes localized later, it
will be very easy to change the sentence lego then.

I am just wary of making life harder for the compiler that wants to verify
that the printf() style parameters match the format.

quoted

@@ -2956,28 +2991,76 @@ static int do_merge(struct commit *commit, const char *arg, int arg_len,
+               cmd.git_cmd = 1;
+               argv_array_push(&cmd.args, "merge");
+               argv_array_push(&cmd.args, "-s");
+               argv_array_push(&cmd.args, "octopus");

argv_array_pushl(&cmd.args, "-s", "octopus", NULL);

which would make it clear that these two arguments must remain
together, and prevent someone from coming along and inserting a new
argument between them.

I use a single `argv_array_pushl()` call now, with intuitive line
wrapping.

quoted

+               argv_array_push(&cmd.args, "--no-edit");
+               argv_array_push(&cmd.args, "--no-ff");
+               argv_array_push(&cmd.args, "--no-log");
+               argv_array_push(&cmd.args, "--no-stat");
+               argv_array_push(&cmd.args, "-F");
+               argv_array_push(&cmd.args, git_path_merge_msg());

Ditto:
argv_array_pushl(&cmd.args, "-L", git_path_merge_msg(), NULL);

quoted

diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh

@@ -329,4 +329,38 @@ test_expect_success 'labels that are object IDs are rewritten' '
+test_expect_success 'octopus merges' '
+       git checkout -b three &&
+       test_commit before-octopus &&
+       test_commit three &&
+       git checkout -b two HEAD^ &&
+       test_commit two &&
+       git checkout -b one HEAD^ &&
+       test_commit one &&
+       test_tick &&
+       (GIT_AUTHOR_NAME="Hank" GIT_AUTHOR_EMAIL="hank@sea.world" \
+        git merge -m "Tüntenfüsch" two three) &&

Unnecessary subshell?

Yes, I agree. I don't recall why I did it that way, probably I tried
something else originally that required a subshell, or something.

Thank you for helping me make the patches better.

Ciao,
Dscho