Re: [PATCH v6 3/4] notes: add notes.mergestrategy option to select default strategy

2015-08-14 Thread Jacob Keller
On Fri, Aug 14, 2015 at 2:06 PM, Eric Sunshine  wrote:
> const char *value;
>
> if (!git_config_get_string_const(key, &value)) {
> if (parse_notes_strategy(value, strategy))
> git_die_config(key, "unknown notes merge strategy '%s'", value);
> return 0;
> }
> return 1;
>
> Or, the equivalent, but less indented:
>
> if (git_config_get_string_const(key, &value))
> return 1;
> if (parse_notes_strategy(value, strategy))
> git_die_config(key, "unknown notes merge strategy '%s'", value);
> return 0;


I like it. Will do.
--
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 v6 3/4] notes: add notes.mergestrategy option to select default strategy

2015-08-14 Thread Eric Sunshine
On Fri, Aug 14, 2015 at 4:48 PM, Jacob Keller  wrote:
> From: Jacob Keller 
>
> Teach git-notes about "notes.mergestrategy" to select a general strategy
> for all notes merges. This enables a user to always get expected merge
> strategy such as "cat_sort_uniq" without having to pass the "-s" option
> manually.
>
> Signed-off-by: Jacob Keller 
> ---
> diff --git a/builtin/notes.c b/builtin/notes.c
> index 042348082709..d65134f89b40 100644
> --- a/builtin/notes.c
> +++ b/builtin/notes.c
> @@ -738,6 +738,41 @@ static int merge_commit(struct notes_merge_options *o)
> +static int git_config_get_notes_strategy(const char *key,
> +enum notes_merge_strategy *strategy)
> +{
> +   const char *value = NULL;
> +
> +   git_config_get_string_const(key, &value);
> +
> +   if (value) {
> +   if (parse_notes_strategy(value, strategy))
> +   git_die_config(key, "unknown notes merge strategy 
> '%s'", value);
> +   else
> +   return 0;
> +   }
> +
> +   return 1;
> +}

Nice. This seems like a better (and more user-helpful) approach.

Instead of initializing value to NULL and ignoring the return value of
git_config_get_string_const(), would it instead make sense to respect
the return of git_config_get_string_const(), like this?

const char *value;

if (!git_config_get_string_const(key, &value)) {
if (parse_notes_strategy(value, strategy))
git_die_config(key, "unknown notes merge strategy '%s'", value);
return 0;
}
return 1;

Or, the equivalent, but less indented:

if (git_config_get_string_const(key, &value))
return 1;
if (parse_notes_strategy(value, strategy))
git_die_config(key, "unknown notes merge strategy '%s'", value);
return 0;
--
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 v6 3/4] notes: add notes.mergestrategy option to select default strategy

2015-08-14 Thread Jacob Keller
From: Jacob Keller 

Teach git-notes about "notes.mergestrategy" to select a general strategy
for all notes merges. This enables a user to always get expected merge
strategy such as "cat_sort_uniq" without having to pass the "-s" option
manually.

Signed-off-by: Jacob Keller 
---
 Documentation/config.txt|  7 ++
 Documentation/git-notes.txt | 14 ++-
 builtin/notes.c | 49 -
 notes-merge.h   | 16 ++--
 t/t3309-notes-merge-auto-resolve.sh | 29 ++
 5 files changed, 96 insertions(+), 19 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index de67ad1fdedf..5e3e03459de7 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1919,6 +1919,13 @@ mergetool.writeToTemp::
 mergetool.prompt::
Prompt before each invocation of the merge resolution program.
 
+notes.mergestrategy::
+   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..89c8829a0543 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.mergestrategy" 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.mergestrategy 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.mergestrategy::
+   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 042348082709..d65134f89b40 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -738,6 +738,41 @@ 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 git_config_get_notes_strategy(const char *key,
+enum notes_merge_strategy *strategy)
+{
+   const char *value = NULL;
+
+   git_config_get_string_const(key, &value);
+
+   if (value) {
+   if (parse_notes_strategy(value, strategy))
+   git_die_config(key, "unknown notes merge strategy 
'%s'", value);
+   else
+   return 0;
+   }
+
+   return 1;
+}
+
 static int merge(int a