This has multiple benefits: with more than one of {--ff, --no-ff,
--ff-only} respects the last option; also the command-line option to
always take precedence over the config file option.
Signed-off-by: Miklos Vajna vmik...@suse.cz
---
builtin/merge.c | 55 +--
t/t7600-merge.sh | 12 +---
2 files changed, 42 insertions(+), 25 deletions(-)
On Tue, Jul 02, 2013 at 10:42:39AM +0200, Michael Haggerty
mhag...@alum.mit.edu wrote:
You allow --no-ff-only but ignore it, which I think is incorrect. In
git merge --ff-only --no-ff-only [...]
, the --no-ff-only should presumably cancel the effect of the previous
--ff-only (i.e., be equivalent to --ff). But it is a little bit
subtle because
git merge --no-ff --no-ff-only
should presumably be equivalent to --no-ff. So I think that
--no-ff-only should do something like
if (fast_forward == FF_ONLY)
fast_forward = FF_ALLOW;
Do we really want --no-ff-only? I would rather just disable it, see the
updated patch.
I'm no options guru, but I think it would be possible to implement --ff
and --no-ff without callbacks if you choose constants such that
FF_NO==0, something like:
Indeed, done.
Should these do the same as the versions with the option order reversed?
Or should the command line option that appears later take precedence?
The latter implies that {--ff, --no-ff, --ff-only, --squash} actually
constitute a single *quad-state* option, representing how the results
of the merge should be handled, and, for example,
git merge --squash --ff-only
ignores the --squash option, and
git merge --ff-only --squash
ignores the --ff-only option.
I'm not sure.
Actually there is also --no-squash, used by e.g. git-pull internally.
You definitely don't want a five-state option. :-) So for now I would
rather let --squash/--no-squash alone.
diff --git a/builtin/merge.c b/builtin/merge.c
index 2ebe732..149f32a 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -47,8 +47,8 @@ static const char * const builtin_merge_usage[] = {
};
static int show_diffstat = 1, shortlog_len = -1, squash;
-static int option_commit = 1, allow_fast_forward = 1;
-static int fast_forward_only, option_edit = -1;
+static int option_commit = 1;
+static int option_edit = -1;
static int allow_trivial = 1, have_message, verify_signatures;
static int overwrite_ignore = 1;
static struct strbuf merge_msg = STRBUF_INIT;
@@ -76,6 +76,14 @@ static struct strategy all_strategy[] = {
static const char *pull_twohead, *pull_octopus;
+enum ff_type {
+ FF_NO,
+ FF_ALLOW,
+ FF_ONLY
+};
+
+static enum ff_type fast_forward = FF_ALLOW;
+
static int option_parse_message(const struct option *opt,
const char *arg, int unset)
{
@@ -178,6 +186,13 @@ static int option_parse_n(const struct option *opt,
return 0;
}
+static int option_parse_ff_only(const struct option *opt,
+ const char *arg, int unset)
+{
+ fast_forward = FF_ONLY;
+ return 0;
+}
+
static struct option builtin_merge_options[] = {
{ OPTION_CALLBACK, 'n', NULL, NULL, NULL,
N_(do not show a diffstat at the end of the merge),
@@ -194,10 +209,10 @@ static struct option builtin_merge_options[] = {
N_(perform a commit if the merge succeeds (default))),
OPT_BOOL('e', edit, option_edit,
N_(edit message before committing)),
- OPT_BOOLEAN(0, ff, allow_fast_forward,
- N_(allow fast-forward (default))),
- OPT_BOOLEAN(0, ff-only, fast_forward_only,
- N_(abort if fast-forward is not possible)),
+ OPT_SET_INT(0, ff, fast_forward, N_(allow fast-forward (default)),
FF_ALLOW),
+ { OPTION_CALLBACK, 0, ff-only, NULL, NULL,
+ N_(abort if fast-forward is not possible),
+ PARSE_OPT_NOARG | PARSE_OPT_NONEG, option_parse_ff_only },
OPT_RERERE_AUTOUPDATE(allow_rerere_auto),
OPT_BOOL(0, verify-signatures, verify_signatures,
N_(Verify that the named commit has a valid GPG signature)),
@@ -581,10 +596,9 @@ static int git_merge_config(const char *k, const char *v,
void *cb)
else if (!strcmp(k, merge.ff)) {
int boolval = git_config_maybe_bool(k, v);
if (0 = boolval) {
- allow_fast_forward = boolval;
+ fast_forward = boolval ? FF_ALLOW : FF_NO;
} else if (v !strcmp(v, only)) {
- allow_fast_forward = 1;
- fast_forward_only = 1;
+ fast_forward = FF_ONLY;
} /* do not barf on values from future versions of git */
return 0;
} else if (!strcmp(k, merge.defaulttoupstream)) {
@@ -863,7 +877,7 @@ static int finish_automerge(struct commit *head,
free_commit_list(common);
parents
