git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Thiago Farina <tfransosi@gmail.com>
To: Michael Grubb <devel@dailyvoid.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, 2 May 2011 15:12:07 -0300	[thread overview]
Message-ID: <BANLkTikgcwL6KRNAvxyAiHxLPzG7jeU6Vg@mail.gmail.com> (raw)
In-Reply-To: <4DBED99E.3050709@dailyvoid.com>

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
>

  reply	other threads:[~2011-05-02 18:12 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 [this message]
2011-05-02 18:22   ` Michael Grubb
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=BANLkTikgcwL6KRNAvxyAiHxLPzG7jeU6Vg@mail.gmail.com \
    --to=tfransosi@gmail.com \
    --cc=deskinm@umich.edu \
    --cc=devel@dailyvoid.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.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).