From: Jacob Keller <jacob.kel...@gmail.com>

Teach git-notes about a new configuration option "notes.merge" for
selecting the default notes merge strategy. Document the option in
config.txt and git-notes.txt

Add tests for the configuration option. Ensure that command line
--strategy option overrides the configured value. Ensure that -s can't
be passed with --commit or --abort. Ensure that --abort and --commit
can't be used together.

Signed-off-by: Jacob Keller <jacob.kel...@gmail.com>
---
 Documentation/config.txt              |  7 ++++
 Documentation/git-notes.txt           | 30 +++++++++-----
 builtin/notes.c                       | 75 +++++++++++++++++++++++++----------
 notes-merge.h                         | 16 ++++----
 t/t3309-notes-merge-auto-resolve.sh   | 29 ++++++++++++++
 t/t3310-notes-merge-manual-resolve.sh | 12 ++++++
 6 files changed, 129 insertions(+), 40 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 3c1e4df09beb..85c15126e4ce 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1937,6 +1937,13 @@ mergetool.writeToTemp::
 mergetool.prompt::
        Prompt before each invocation of the merge resolution program.
 
+notes.merge::
+       Which merge strategy to choose by default when resolving notes
+       conflicts. Must be one of `manual`, `ours`, `theirs`, `union`,
+       or `cat_sort_uniq`. Defaults to `manual`. See "NOTES MERGE
+       STRATEGIES" section of linkgit:git-notes[1] for more information
+       on each strategy.
+
 notes.displayRef::
        The (fully qualified) refname from which to show notes when
        showing commit messages.  The value of this variable can be set
diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
index 674682b34b83..d8944f5aec60 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt
@@ -101,13 +101,13 @@ merge::
        any) into the current notes ref (called "local").
 +
 If conflicts arise and a strategy for automatically resolving
-conflicting notes (see the -s/--strategy option) is not given,
-the "manual" resolver is used. This resolver checks out the
-conflicting notes in a special worktree (`.git/NOTES_MERGE_WORKTREE`),
-and instructs the user to manually resolve the conflicts there.
-When done, the user can either finalize the merge with
-'git notes merge --commit', or abort the merge with
-'git notes merge --abort'.
+conflicting notes (see the -s/--strategy option or notes.merge
+config option) is not given, the "manual" resolver is used.
+This resolver checks out the conflicting notes in a special
+worktree (`.git/NOTES_MERGE_WORKTREE`), and instructs the user
+to manually resolve the conflicts there. When done, the user
+can either finalize the merge with 'git notes merge --commit',
+or abort the merge with 'git notes merge --abort'.
 
 remove::
        Remove the notes for given objects (defaults to HEAD). When
@@ -181,10 +181,10 @@ OPTIONS
 -s <strategy>::
 --strategy=<strategy>::
        When merging notes, resolve notes conflicts using the given
-       strategy. The following strategies are recognized: "manual"
-       (default), "ours", "theirs", "union" and "cat_sort_uniq".
-       See the "NOTES MERGE STRATEGIES" section below for more
-       information on each notes merge strategy.
+       strategy. Overrides "notes.merge". The following strategies are
+       recognized: `manual`, `ours`, `theirs`, `union` and
+       `cat_sort_uniq`. See the `NOTES MERGE STRATEGIES` section below
+       for more information on each notes merge strategy.
 
 --commit::
        Finalize an in-progress 'git notes merge'. Use this option
@@ -310,6 +310,14 @@ core.notesRef::
        This setting can be overridden through the environment and
        command line.
 
+notes.merge::
+       Which merge strategy to choose by default when resolving notes
+       conflicts. Must be one of `manual`, `ours`, `theirs`, `union`,
+       or `cat_sort_uniq`. Defaults to `manual`. See "NOTES MERGE
+       STRATEGIES" section above for more information on each strategy.
++
+This setting can be overridden by passing the `--strategy` option.
+
 notes.displayRef::
        Which ref (or refs, if a glob or specified more than once), in
        addition to the default set by `core.notesRef` or
diff --git a/builtin/notes.c b/builtin/notes.c
index 88fdafb32bc0..7123311b563e 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -93,6 +93,9 @@ static const char * const git_notes_get_ref_usage[] = {
 static const char note_template[] =
        "\nWrite/edit the notes for the following object:\n";
 
+static int override_strategy = 0;
+static enum notes_merge_strategy strategy;
+
 struct note_data {
        int given;
        int use_editor;
@@ -741,6 +744,36 @@ static int merge_commit(struct notes_merge_options *o)
        return ret;
 }
 
+static int parse_notes_strategy(const char *var, const char *arg,
+                               enum notes_merge_strategy *strategy)
+{
+       if (!strcmp(arg, "manual"))
+               *strategy = NOTES_MERGE_RESOLVE_MANUAL;
+       else if (!strcmp(arg, "ours"))
+               *strategy = NOTES_MERGE_RESOLVE_OURS;
+       else if (!strcmp(arg, "theirs"))
+               *strategy = NOTES_MERGE_RESOLVE_THEIRS;
+       else if (!strcmp(arg, "union"))
+               *strategy = NOTES_MERGE_RESOLVE_UNION;
+       else if (!strcmp(arg, "cat_sort_uniq"))
+               *strategy = NOTES_MERGE_RESOLVE_CAT_SORT_UNIQ;
+       else {
+               return error("Unknown notes merge strategy: %s", arg);
+       }
+
+       return 0;
+}
+
+static int parse_opt_strategy(const struct option *opt, const char *arg, int 
unset)
+{
+       enum notes_merge_strategy *strategy = opt->value;
+
+       /* let merge() know we overrode the configured strategy */
+       override_strategy = 1;
+
+       return parse_notes_strategy(NULL, arg, strategy);
+}
+
 static int merge(int argc, const char **argv, const char *prefix)
 {
        struct strbuf remote_ref = STRBUF_INIT, msg = STRBUF_INIT;
@@ -749,14 +782,16 @@ static int merge(int argc, const char **argv, const char 
*prefix)
        struct notes_merge_options o;
        int do_merge = 0, do_commit = 0, do_abort = 0;
        int verbosity = 0, result;
-       const char *strategy = NULL;
        struct option options[] = {
                OPT_GROUP(N_("General options")),
                OPT__VERBOSITY(&verbosity),
                OPT_GROUP(N_("Merge options")),
-               OPT_STRING('s', "strategy", &strategy, N_("strategy"),
-                          N_("resolve notes conflicts using the given strategy 
"
-                             "(manual/ours/theirs/union/cat_sort_uniq)")),
+               {
+                       OPTION_CALLBACK, 's', "strategy", &strategy, 
N_("strategy"),
+                       N_("resolve notes conflicts using the given strategy "
+                          "(manual/ours/theirs/union/cat_sort_uniq)"),
+                       PARSE_OPT_NONEG, parse_opt_strategy
+               },
                OPT_GROUP(N_("Committing unmerged notes")),
                { OPTION_SET_INT, 0, "commit", &do_commit, NULL,
                        N_("finalize notes merge by committing unmerged notes"),
@@ -771,7 +806,7 @@ static int merge(int argc, const char **argv, const char 
*prefix)
        argc = parse_options(argc, argv, prefix, options,
                             git_notes_merge_usage, 0);
 
-       if (strategy || do_commit + do_abort == 0)
+       if (override_strategy || do_commit + do_abort == 0)
                do_merge = 1;
        if (do_merge + do_commit + do_abort != 1) {
                error("cannot mix --commit, --abort or -s/--strategy");
@@ -799,22 +834,7 @@ static int merge(int argc, const char **argv, const char 
*prefix)
        expand_notes_ref(&remote_ref);
        o.remote_ref = remote_ref.buf;
 
-       if (strategy) {
-               if (!strcmp(strategy, "manual"))
-                       o.strategy = NOTES_MERGE_RESOLVE_MANUAL;
-               else if (!strcmp(strategy, "ours"))
-                       o.strategy = NOTES_MERGE_RESOLVE_OURS;
-               else if (!strcmp(strategy, "theirs"))
-                       o.strategy = NOTES_MERGE_RESOLVE_THEIRS;
-               else if (!strcmp(strategy, "union"))
-                       o.strategy = NOTES_MERGE_RESOLVE_UNION;
-               else if (!strcmp(strategy, "cat_sort_uniq"))
-                       o.strategy = NOTES_MERGE_RESOLVE_CAT_SORT_UNIQ;
-               else {
-                       error("Unknown -s/--strategy: %s", strategy);
-                       usage_with_options(git_notes_merge_usage, options);
-               }
-       }
+       o.strategy = strategy;
 
        t = init_notes_check("merge", NOTES_INIT_WRITABLE);
 
@@ -950,6 +970,17 @@ static int get_ref(int argc, const char **argv, const char 
*prefix)
        return 0;
 }
 
+static int git_notes_config(const char *var, const char *value, void *cb)
+{
+       if (!strcmp(var, "notes.merge")) {
+               if (!value)
+                       return config_error_nonbool(var);
+               return parse_notes_strategy(var, value, &strategy);
+       }
+
+       return git_default_config(var, value, cb);
+}
+
 int cmd_notes(int argc, const char **argv, const char *prefix)
 {
        int result;
@@ -960,7 +991,7 @@ int cmd_notes(int argc, const char **argv, const char 
*prefix)
                OPT_END()
        };
 
-       git_config(git_default_config, NULL);
+       git_config(git_notes_config, NULL);
        argc = parse_options(argc, argv, prefix, options, git_notes_usage,
                             PARSE_OPT_STOP_AT_NON_OPTION);
 
diff --git a/notes-merge.h b/notes-merge.h
index 1d01f6aacf54..bda8c0c8d348 100644
--- a/notes-merge.h
+++ b/notes-merge.h
@@ -8,18 +8,20 @@ enum notes_merge_verbosity {
        NOTES_MERGE_VERBOSITY_MAX = 5
 };
 
+enum notes_merge_strategy {
+       NOTES_MERGE_RESOLVE_MANUAL = 0,
+       NOTES_MERGE_RESOLVE_OURS,
+       NOTES_MERGE_RESOLVE_THEIRS,
+       NOTES_MERGE_RESOLVE_UNION,
+       NOTES_MERGE_RESOLVE_CAT_SORT_UNIQ
+};
+
 struct notes_merge_options {
        const char *local_ref;
        const char *remote_ref;
        struct strbuf commit_msg;
        int verbosity;
-       enum {
-               NOTES_MERGE_RESOLVE_MANUAL = 0,
-               NOTES_MERGE_RESOLVE_OURS,
-               NOTES_MERGE_RESOLVE_THEIRS,
-               NOTES_MERGE_RESOLVE_UNION,
-               NOTES_MERGE_RESOLVE_CAT_SORT_UNIQ
-       } strategy;
+       enum notes_merge_strategy strategy;
        unsigned has_worktree:1;
 };
 
diff --git a/t/t3309-notes-merge-auto-resolve.sh 
b/t/t3309-notes-merge-auto-resolve.sh
index 461fd84755d7..a773b01b73db 100755
--- a/t/t3309-notes-merge-auto-resolve.sh
+++ b/t/t3309-notes-merge-auto-resolve.sh
@@ -298,6 +298,13 @@ test_expect_success 'merge z into y with invalid strategy 
=> Fail/No changes' '
        verify_notes y y
 '
 
+test_expect_success 'merge z into y with invalid configuration option => 
Fail/No changes' '
+       git config core.notesRef refs/notes/y &&
+       test_must_fail git -c notes.merge="foo" notes merge z &&
+       # Verify no changes (y)
+       verify_notes y y
+'
+
 cat <<EOF | sort >expect_notes_ours
 68b8630d25516028bed862719855b3d6768d7833 $commit_sha15
 5de7ea7ad4f47e7ff91989fb82234634730f75df $commit_sha14
@@ -365,6 +372,17 @@ test_expect_success 'reset to pre-merge state (y)' '
        verify_notes y y
 '
 
+test_expect_success 'merge z into y with "ours" configuration option => 
Non-conflicting 3-way merge' '
+       git -c notes.merge="ours" notes merge z &&
+       verify_notes y ours
+'
+
+test_expect_success 'reset to pre-merge state (y)' '
+       git update-ref refs/notes/y refs/notes/y^1 &&
+       # Verify pre-merge state
+       verify_notes y y
+'
+
 cat <<EOF | sort >expect_notes_theirs
 9b4b2c61f0615412da3c10f98ff85b57c04ec765 $commit_sha15
 5de7ea7ad4f47e7ff91989fb82234634730f75df $commit_sha14
@@ -432,6 +450,17 @@ test_expect_success 'reset to pre-merge state (y)' '
        verify_notes y y
 '
 
+test_expect_success 'merge z into y with "theirs" strategy overriding 
configuration option "ours" => Non-conflicting 3-way merge' '
+       git -c notes.merge="ours" notes merge --strategy=theirs z &&
+       verify_notes y theirs
+'
+
+test_expect_success 'reset to pre-merge state (y)' '
+       git update-ref refs/notes/y refs/notes/y^1 &&
+       # Verify pre-merge state
+       verify_notes y y
+'
+
 cat <<EOF | sort >expect_notes_union
 7c4e546efd0fe939f876beb262ece02797880b54 $commit_sha15
 5de7ea7ad4f47e7ff91989fb82234634730f75df $commit_sha14
diff --git a/t/t3310-notes-merge-manual-resolve.sh 
b/t/t3310-notes-merge-manual-resolve.sh
index 195bb97f859d..d5572121da69 100755
--- a/t/t3310-notes-merge-manual-resolve.sh
+++ b/t/t3310-notes-merge-manual-resolve.sh
@@ -314,6 +314,18 @@ y and z notes on 1st commit
 
 EOF
 
+test_expect_success 'do not allow mixing --commit and --abort' '
+       test_must_fail git notes merge --commit --abort
+'
+
+test_expect_success 'do not allow mixing --commit and --strategy' '
+       test_must_fail git notes merge --commit --strategy theirs
+'
+
+test_expect_success 'do not allow mixing --abort and --strategy' '
+       test_must_fail git notes merge --abort --strategy theirs
+'
+
 test_expect_success 'finalize conflicting merge (z => m)' '
        # Resolve conflicts and finalize merge
        cat >.git/NOTES_MERGE_WORKTREE/$commit_sha1 <<EOF &&
-- 
2.5.0.482.gfcd5645

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to