From: Michael Grubb <devel@dailyvoid.com>
To: Thiago Farina <tfransosi@gmail.com>
Cc: git@vger.kernel.org, vmiklos@frugalware.org, deskinm@umich.edu,
gitster@pobox.com
Subject: Re: [PATCH] Add default merge options for all branches
Date: Mon, 02 May 2011 13:22:29 -0500 [thread overview]
Message-ID: <4DBEF665.20309@dailyvoid.com> (raw)
In-Reply-To: <BANLkTikgcwL6KRNAvxyAiHxLPzG7jeU6Vg@mail.gmail.com>
Sorry for that, it's my own habit I'm afraid, however I did go back and
fix it up so that it is consistent with the rest of the file. Should I
resubmit the patch in reply to this thread?
On 5/2/11 1:12 PM, Thiago Farina wrote:
> On Mon, May 2, 2011 at 1:19 PM, Michael Grubb <devel@dailyvoid.com> wrote:
>> Introduce a new configuration variable, merge.mergeoptions.
>> The semantics of this new variable are the same as the branch specific
>> branch.<branch>.mergeoptions. However, if a branch specific setting is
>> found, this option will not override it.
>>
>> The need for this arises from the fact that there is currently not an
>> easy way to set merge options for all branches. Instead of having to
>> specify merge options for each individual branch there should be a way
>> to set defaults for all branches and then override a specific branch's
>> options.
>>
>> The approach taken is to make note of whether a branch specific
>> mergeoptions key has been seen and only apply the global value if it
>> hasn't. An alternative method would be to keep the
>> branch.<branch>.mergeoptions semantics, but assign a special value for
>> <branch> to be the global default.
>>
>> Signed-off-by: Michael Grubb <devel@dailyvoid.com>
>> ---
>> Documentation/merge-config.txt | 7 +++++++
>> builtin/merge.c | 27 ++++++++++++++++++++++-----
>> t/t7600-merge.sh | 27 +++++++++++++++++++++++++++
>> 3 files changed, 56 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/merge-config.txt b/Documentation/merge-config.txt
>> index 8920258..0fc7511 100644
>> --- a/Documentation/merge-config.txt
>> +++ b/Documentation/merge-config.txt
>> @@ -57,6 +57,13 @@ merge.verbosity::
>> above outputs debugging information. The default is level 2.
>> Can be overridden by the 'GIT_MERGE_VERBOSITY' environment variable.
>> +merge.mergeoptions::
>> + Sets default options for merging any branch. This value is only
>> + used if there is not a branch.<name>.mergeoptions value set.
>> + The syntax and supported options are the same as those of 'git
>> + merge', but option values containing whitespace characters are
>> + currently not supported.
>> +
>> merge.<driver>.name::
>> Defines a human-readable name for a custom low-level
>> merge driver. See linkgit:gitattributes[5] for details.
>> diff --git a/builtin/merge.c b/builtin/merge.c
>> index 0bdd19a..1d4f852 100644
>> --- a/builtin/merge.c
>> +++ b/builtin/merge.c
>> @@ -505,9 +505,18 @@ cleanup:
>> static int git_merge_config(const char *k, const char *v, void *cb)
>> {
>> - if (branch && !prefixcmp(k, "branch.") &&
>> - !prefixcmp(k + 7, branch) &&
>> - !strcmp(k + 7 + strlen(branch), ".mergeoptions")) {
>> + static int branch_merge_options_set = 0;
>> + int merge_option_mode = 0;
>> +
>> + if ( !strcmp(k, "merge.mergeoptions") )
>
> please, no need of spaces between if ( ... )
>
> There are more cases in this change below.
>
> I don't know if Junio is strong about this and if you need to resend.
> But at least that is not consistent with the rest of the file.
>
> Just my 0.02 cents.
>
>> + merge_option_mode = 1;
>> + else if ( branch && !prefixcmp(k, "branch.") &&
>> + !prefixcmp(k + 7, branch) &&
>> + !strcmp(k + 7 + strlen(branch), ".mergeoptions"))
>> + merge_option_mode = 2;
>> +
>> + if ( (merge_option_mode == 1 && !branch_merge_options_set) ||
>> + merge_option_mode == 2) {
>> const char **argv;
>> int argc;
>> char *buf;
>> @@ -515,14 +524,22 @@ static int git_merge_config(const char *k, const
>> char *v, void *cb)
>> buf = xstrdup(v);
>> argc = split_cmdline(buf, &argv);
>> if (argc < 0)
>> - die(_("Bad branch.%s.mergeoptions string: %s"), branch,
>> - split_cmdline_strerror(argc));
>> + {
>> + if ( merge_option_mode == 1 )
>> + die(_("Bad merge.mergeoptions string: %s"), +
>> split_cmdline_strerror(argc));
>> + else
>> + die(_("Bad branch.%s.mergeoptions string: %s"), branch,
>> + split_cmdline_strerror(argc));
>> + }
>> argv = xrealloc(argv, sizeof(*argv) * (argc + 2));
>> memmove(argv + 1, argv, sizeof(*argv) * (argc + 1));
>> argc++;
>> parse_options(argc, argv, NULL, builtin_merge_options,
>> builtin_merge_usage, 0);
>> free(buf);
>> + if ( merge_option_mode == 2 )
>> + branch_merge_options_set = 1;
>> }
>> if (!strcmp(k, "merge.diffstat") || !strcmp(k, "merge.stat"))
>> diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
>> index 87d5d78..15e9418 100755
>> --- a/t/t7600-merge.sh
>> +++ b/t/t7600-merge.sh
>> @@ -415,6 +415,33 @@ test_expect_success 'merge c0 with c1 (no-ff)' '
>> test_debug 'git log --graph --decorate --oneline --all'
>> +test_expect_success 'merge c0 with c1 (global no-ff)' '
>> + git reset --hard c0 &&
>> + git config --unset branch.master.mergeoptions &&
>> + git config merge.mergeoptions "--no-ff" &&
>> + test_tick &&
>> + git merge c1 &&
>> + git config --remove-section merge &&
>> + verify_merge file result.1 &&
>> + verify_parents $c0 $c1
>> +'
>> +
>> +test_debug 'git log --graph --decorate --oneline --all'
>> +
>> +test_expect_success 'combine merge.mergeoptions with
>> branch.x.mergeoptions' '
>> + git reset --hard c0 &&
>> + git config --remove-section branch.master &&
>> + git config merge.mergeoptions "--no-ff" &&
>> + git config branch.master.mergeoptions "--ff" &&
>> + test_tick &&
>> + git merge c1 &&
>> + git config --remove-section merge &&
>> + verify_merge file result.1 &&
>> + verify_parents "$c0"
>> +'
>> +
>> +test_debug 'git log --graph --decorate --oneline --all'
>> +
>> test_expect_success 'combining --squash and --no-ff is refused' '
>> test_must_fail git merge --squash --no-ff c1 &&
>> test_must_fail git merge --no-ff --squash c1
>> --
>> 1.7.5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe git" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
next prev parent reply other threads:[~2011-05-02 18:22 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-05-02 16:19 [PATCH] Add default merge options for all branches Michael Grubb
2011-05-02 18:12 ` Thiago Farina
2011-05-02 18:22 ` Michael Grubb [this message]
2011-05-02 18:40 ` Junio C Hamano
2011-05-02 18:23 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4DBEF665.20309@dailyvoid.com \
--to=devel@dailyvoid.com \
--cc=deskinm@umich.edu \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=tfransosi@gmail.com \
--cc=vmiklos@frugalware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).