Re: [PATCH 11/18] git notes merge: Add automatic conflict resolvers (ours, theirs, union)
- From
Johan Herland <johan@herland.net>
- Date
- Oct 4, 2010, 15:10 UTC
- Message-ID
- <201010041710.02046.johan@herland.net>
- In-Reply-To
- <4CA6F806.9030209@gmail.com>
On Saturday 2. October 2010 11.14.46 Stephen Boyd wrote:
Show 20 quoted lines
> On 09/28/2010 05:23 PM, Johan Herland wrote:
> > @@ -788,6 +792,21 @@ 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 {
> > + error("Unknown -X/--resolve strategy: %s", strategy);
>
> Is it -X/--resolve or -s/--strategy? This error confuses me.No, the message should say "Unknown -s/--strategy: %s". Will be fixed in the next iteration. Thanks for noticing.
Show 37 quoted lines
> > diff --git a/notes-merge.c b/notes-merge.c
> > index f625ebd..6fa59d8 100644
> > --- a/notes-merge.c
> > +++ b/notes-merge.c
> > @@ -262,6 +262,35 @@ static void diff_tree_local(struct
> > notes_merge_options *o,
> >
> > diff_tree_release_paths(&opt);
> >
> > }
> >
> > +static int merge_one_change(struct notes_merge_options *o,
> > + struct notes_merge_pair *p, struct notes_tree *t)
> > +{
> > + /*
> > + * Return 0 if change was resolved (and added to notes_tree),
> > + * 1 if conflict
> > + */
> > + switch (o->strategy) {
> > + case NOTES_MERGE_RESOLVE_MANUAL:
> > + return 1;
> > + case NOTES_MERGE_RESOLVE_OURS:
> > + OUTPUT(o, 2, "Using local notes for %s", sha1_to_hex(p->obj));
> > + /* nothing to do */
> > + return 0;
> > + case NOTES_MERGE_RESOLVE_THEIRS:
> > + OUTPUT(o, 2, "Using remote notes for %s", sha1_to_hex(p-
>obj));
> > + if (add_note(t, p->obj, p->remote, combine_notes_overwrite))
> > + die("confused: combine_notes_overwrite failed");
>
> This will say:
>
> fatal: confused: combine_notes_overwrite failed
>
> Do we actually need the "confused" part? Heh, maybe we need a confused()
> function?Well, combine_notes_overwrite() can only return 0, so if we get a non-zero return here I will certainly be confused, since this should be impossible. Maybe better to drop the check altogether and simply leave add_note(...)?
Have fun! :)
...Johan
-- Johan Herland, <johan@herland.net> www.herland.net