git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Johan Herland <johan@herland.net>
Date
Aug 12, 2015, 00:04 UTC
Message-ID
<CALKQrgcYpbae0eu2sT_rOZYq3DsnVSpYua1SZr=TC=dYfG_fpw@mail.gmail.com>
In-Reply-To
<1439326641-9447-4-git-send-email-jacob.e.keller@intel.com>
On Tue, Aug 11, 2015 at 10:57 PM, Jacob Keller <jacob.e.keller@intel.com> wrote:
Show 15 quoted lines
> From: Jacob Keller <jacob.keller@gmail.com>
>
> Teach git-notes about "notes.merge" 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 <jacob.keller@gmail.com>
> ---
>  Documentation/config.txt            |  7 ++++++
>  Documentation/git-notes.txt         | 14 +++++++++++-
>  builtin/notes.c                     | 44 +++++++++++++++++++++++--------------
>  notes-merge.h                       | 16 ++++++++------
>  t/t3309-notes-merge-auto-resolve.sh | 29 ++++++++++++++++++++++++
>  5 files changed, 86 insertions(+), 24 deletions(-)
>
[...]
Show 67 quoted lines
> diff --git a/builtin/notes.c b/builtin/notes.c
> index 63f95fc55439..3c705f5e26ff 100644
> --- a/builtin/notes.c
> +++ b/builtin/notes.c
> @@ -737,6 +737,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 *prefix)
>  {
>         struct strbuf remote_ref = STRBUF_INIT, msg = STRBUF_INIT;
> @@ -745,7 +763,7 @@ 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;
> +       const char *strategy = NULL, *configured_strategy = NULL;
>         struct option options[] = {
>                 OPT_GROUP(N_("General options")),
>                 OPT__VERBOSITY(&verbosity),
> @@ -795,21 +813,15 @@ 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);
> -               }
> +       git_config_get_string_const("notes.merge", &configured_strategy);
> +
> +       if (configured_strategy &&
> +           parse_notes_strategy(configured_strategy, &o.strategy))
> +               die("Unknown notes merge strategy: %s", configured_strategy);
> +
> +       if (strategy && parse_notes_strategy(strategy, &o.strategy)) {
> +               error("Unknown -s/--strategy: %s", strategy);
> +               usage_with_options(git_notes_merge_usage, options);
>         }

Do you need to look at the notes.merge config variable at all if -s/--strategy is given? I'd expect the above to rather look something like:

if (strategy) {
        if (parse_notes_strategy(strategy, &o.strategy)) {
                error("Unknown -s/--strategy: %s", strategy);
                usage_with_options(git_notes_merge_usage, options);
        }
} else if (configured_strategy) {
        if (parse_notes_strategy(configured_strategy, &o.strategy))
                die("Unknown notes merge strategy: %s", configured_strategy);
} /* else o.strategy = NOTES_MERGE_RESOLVE_MANUAL; */
Otherwise, this patch looks good to me.
...Johan
-- 
Johan Herland, <johan@herland.net>
www.herland.net
Previous: Jacob KellerNext: Jacob Keller
Message 7 of 21 in “add notes strategy configuration options”
  1. 0/4 add notes strategy configuration optionsJacob Keller, Aug 11, 2015
  2. 1/4 notes: document cat_sort_uniq rewriteModeJacob Keller, Aug 11, 2015
  3. Johan HerlandAug 12, 2015
  4. 2/4 notes: add tests for --commit/--abort/--strategy exclusivityJacob Keller, Aug 11, 2015
  5. Johan HerlandAug 12, 2015
  6. 3/4 notes: add notes.merge option to select default strategyJacob Keller, Aug 11, 2015
  7. Johan HerlandAug 12, 2015
  8. 4/4 notes: teach git-notes about notes.<ref>.merge optionJacob Keller, Aug 11, 2015
  9. Eric SunshineAug 12, 2015
  10. Johan HerlandAug 12, 2015
  11. Junio C HamanoAug 12, 2015
  12. Jacob KellerAug 12, 2015
  13. Junio C HamanoAug 12, 2015
  14. Jacob KellerAug 12, 2015
  15. Jacob KellerAug 12, 2015
  16. Johan HerlandAug 12, 2015
  17. Johan HerlandAug 12, 2015
  18. Jacob KellerAug 12, 2015
  19. Jacob KellerAug 12, 2015
  20. Junio C HamanoAug 12, 2015
  21. Jacob KellerAug 12, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.