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

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 11, 2018, 21:52 UTC
Message-ID: <xmqqfu0p1luz.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <CAPig+cS1KQseynMCGC_6FZwzB0waJX8+C7CVKNqACzOhjB4uSg@mail.gmail.com>

Eric Sunshine <sunshine@sunshineco.com> writes:

Show 9 quoted lines

>> 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.

I do not think we'd want to localize these default merge messages, though.

Show 11 quoted lines

>> @@ -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.

A valid point. It is OK to break "merge" and "-s octopus" into separate push invocations, but not "-s" and "octopus". Or perhaps push it as a single "--strategy=octopus" argument, which would be a better approach anyway.

>> +       (GIT_AUTHOR_NAME="Hank" GIT_AUTHOR_EMAIL="hank@sea.world" \
>> +        git merge -m "Tüntenfüsch" two three) &&
>
> Unnecessary subshell?

Right.