git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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
> 

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