Re: [PATCH 07/12] merge-tree: support including merge messages in output

>
> Hi Elijah,
>
> On Sat, 22 Jan 2022, Elijah Newren via GitGitGadget wrote:
>
> > From: Elijah Newren <newren@gmail.com>
> >
> > When running `git merge-tree --write-tree`, we previously would only
> > return an exit status reflecting the cleanness of a merge, and print out
> > the toplevel tree of the resulting merge.  Merges also have
> > informational messages, ("Auto-merging <PATH>", "CONFLICT (content):
> > ...", "CONFLICT (file/directory)", etc.)  In fact, when non-content
> > conflicts occur (such as file/directory, modify/delete, add/add with
> > differing modes, rename/rename (1to2), etc.), these informational
> > messages are often the only notification since these conflicts are not
> > representable in the contents of the file.
> >
> > Add a --[no-]messages option so that callers can request these messages
> > be included at the end of the output.  Include such messages by default
> > when there are conflicts, and omit them by default when the merge is
> > clean.
>
> Makes sense.
>
> > diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
> > index 0c19639594d..560640ad911 100644
> > --- a/builtin/merge-tree.c
> > +++ b/builtin/merge-tree.c
> > @@ -440,22 +441,30 @@ static int real_merge(struct merge_tree_options *o,
> >               commit_list_insert(j->item, &merge_bases);
> >
> >       merge_incore_recursive(&opt, merge_bases, parent1, parent2, &result);
> > -     printf("%s\n", oid_to_hex(&result.tree->object.oid));
> > +
> >       if (result.clean < 0)
> >               die(_("failure to merge"));
> > -     else if (!result.clean)
> > -             printf(_("Conflicts!\n"));
> > +
> > +     if (o->show_messages == -1)
> > +             o->show_messages = !result.clean;
> > +
> > +     printf("%s\n", oid_to_hex(&result.tree->object.oid));
> > +     if (o->show_messages) {
> > +             printf("\n");
> > +             merge_display_update_messages(&opt, &result, stdout);
> > +     }
>
> Excellent.
>
> >       merge_finalize(&opt, &result);
> >       return !result.clean; /* result.clean < 0 handled above */
> >  }
> >
> >  int cmd_merge_tree(int argc, const char **argv, const char *prefix)
> >  {
> > -     struct merge_tree_options o = { 0 };
> > +     struct merge_tree_options o = { .show_messages = -1 };
> >       int expected_remaining_argc;
> > +     int original_argc;
> >
> >       const char * const merge_tree_usage[] = {
> > -             N_("git merge-tree [--write-tree] <branch1> <branch2>"),
> > +             N_("git merge-tree [--write-tree] [<options>] <branch1> <branch2>"),
> >               N_("git merge-tree [--trivial-merge] <base-tree> <branch1> <branch2>"),
> >               NULL
> >       };
> > @@ -464,6 +473,8 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)
> >                        N_("do a real merge instead of a trivial merge")),
> >               OPT_BOOL(0, "trivial-merge", &o.trivial,
> >                        N_("do a trivial merge only")),
> > +             OPT_BOOL(0, "messages", &o.show_messages,
> > +                      N_("also show informational/conflict messages")),
> >               OPT_END()
> >       };
> >
> > @@ -472,10 +483,13 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)
> >               usage_with_options(merge_tree_usage, mt_options);
> >
> >       /* Parse arguments */
> > +     original_argc = argc;
> >       argc = parse_options(argc, argv, prefix, mt_options,
> >                            merge_tree_usage, 0);
> >       if (o.real && o.trivial)
> >               die(_("--write-tree and --trivial-merge are incompatible"));
> > +     if (!o.real && original_argc < argc)
> > +             die(_("--write-tree must be specified if any other options are"));
>
> Hmm. Well. Hmm.
>
>
> I'd rather keep `--write-tree` neat and optional. What's wrong with
> allowing
>
>         git merge-tree --no-messages HEAD MERGE_HEAD
>
> ?