Re: [PATCH v3 3/4] notes: add notes.merge option to select default strategy
On Sun, Aug 2, 2015 at 6:10 AM, Jacob Keller jacob.e.kel...@intel.com wrote:
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 use of the configuration option. Include a test to ensure
that --strategy correctly overrides the configured setting.
Signed-off-by: Jacob Keller jacob.kel...@gmail.com
---
diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
index 674682b34b83..9c4f8536182f 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt
@@ -101,7 +101,7 @@ merge::
If conflicts arise and a strategy for automatically resolving
-conflicting notes (see the -s/--strategy option) is not given,
+conflicting notes (see the NOTES MERGE STRATEGIES section) 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.
@@ -183,6 +183,7 @@ OPTIONS
When merging notes, resolve notes conflicts using the given
strategy. The following strategies are recognized: manual
(default), ours, theirs, union and cat_sort_uniq.
+ This option overrides the notes.merge configuration setting.
See the NOTES MERGE STRATEGIES section below for more
information on each notes merge strategy.
These two documentation updates are much easier to digest than the
noisy-diff versions of the previous attempt; and the patch overall is
a more pleasant read than v1.
diff --git a/builtin/notes.c b/builtin/notes.c
index 63f95fc55439..de0caa00df1b 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -945,6 +955,20 @@ 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);
+ if (parse_notes_strategy(value, merge_strategy))
+ return error(Unknown notes merge strategy: %s,
value);
+ else
+ return 0;
A purely subjective stylistic suggestion, which you can freely ignore
if your preference differs:
if (!value)
return ...;
if (parse_notes_strategy(...))
return ...;
return 0;
+ }
+
+ return git_default_config(var, value, cb);
+}
+
--
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
Re: [PATCH v3 3/4] notes: add notes.merge option to select default strategy
Jacob Keller jacob.e.kel...@intel.com writes:
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 use of the configuration option. Include a test to ensure
that --strategy correctly overrides the configured setting.
Signed-off-by: Jacob Keller jacob.kel...@gmail.com
Cc: Johan Herland jo...@herland.net
Cc: Michael Haggerty mhag...@alum.mit.edu
Cc: Eric Sunshine sunsh...@sunshineco.com
---
Perhaps I am biased because we do not usually use Cc: around here,
but the above looks in a somewhat strange order. Shouldn't your
sign-off be at the end?
+static enum notes_merge_strategy merge_strategy;
OK.
+static int parse_notes_strategy(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 -1;
+
+ return 0;
+}
OK.
static int merge(int argc, const char **argv, const char *prefix)
{
struct strbuf remote_ref = STRBUF_INIT, msg = STRBUF_INIT;
@@ -795,23 +815,13 @@ 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);
- }
+ if (strategy parse_notes_strategy(strategy, merge_strategy)) {
+ error(Unknown -s/--strategy: %s, strategy);
+ usage_with_options(git_notes_merge_usage, options);
}
+ o.strategy = merge_strategy;
+
This may be a minor taste thing, but initializing o.strategy with
merge_strategy at the same place as o.remote_ref is initialized,
and then passing o.merge_strategy to parse_notes_strategy(), may be
easier to follow.
Renaming the global merge_strategy to configured_merge_strategy
might make it even easier to follow. If anybody uses the variable
instead of o.strategy after this point, it would be immediately
obvious that it is either a bug or it is deliberately using the
value from the configuration file, not command line.
Thanks.
--
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
[PATCH v3 3/4] notes: add notes.merge option to select default strategy
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 use of the configuration option. Include a test to ensure
that --strategy correctly overrides the configured setting.
Signed-off-by: Jacob Keller jacob.kel...@gmail.com
Cc: Johan Herland jo...@herland.net
Cc: Michael Haggerty mhag...@alum.mit.edu
Cc: Eric Sunshine sunsh...@sunshineco.com
---
Documentation/config.txt| 7 +
Documentation/git-notes.txt | 14 +-
builtin/notes.c | 56 ++---
notes-merge.h | 16 ++-
t/t3309-notes-merge-auto-resolve.sh | 29 +++
5 files changed, 98 insertions(+), 24 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 4ce5c553e50f..0fa88a29dd65 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1911,6 +1911,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..9c4f8536182f 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt
@@ -101,7 +101,7 @@ 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,
+conflicting notes (see the NOTES MERGE STRATEGIES section) 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.
@@ -183,6 +183,7 @@ OPTIONS
When merging notes, resolve notes conflicts using the given
strategy. The following strategies are recognized: manual
(default), ours, theirs, union and cat_sort_uniq.
+ This option overrides the notes.merge configuration setting.
See the NOTES MERGE STRATEGIES section below for more
information on each notes merge strategy.
@@ -247,6 +248,9 @@ When done, the user can either finalize the merge with
'git notes merge --commit', or abort the merge with
'git notes merge --abort'.
+Users may select an automated merge strategy from among the following using
+either -s/--strategy option or configuring notes.merge accordingly:
+
ours automatically resolves conflicting notes in favor of the local
version (i.e. the current notes ref).
@@ -310,6 +314,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 63f95fc55439..de0caa00df1b 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -92,6 +92,8 @@ static const char * const git_notes_get_ref_usage[] = {
static const char note_template[] =
\nWrite/edit the notes for the following object:\n;
+static enum notes_merge_strategy merge_strategy;
+
struct note_data {
int given;
int use_editor;
@@ -737,6 +739,24 @@ static int merge_commit(struct notes_merge_options *o)
return ret;
}
+static int parse_notes_strategy(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 -1;
+
+ return 0;
+}
+
static int merge(int argc, const char **argv, const char
