Hi Junio,
On Wed, 21 Sep 2022, Junio C Hamano wrote:
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@xxxxxxxxx>
> writes:
>
> > + const struct object_id *tree_oid;
> >
> > parent1 = get_merge_parent(branch1);
> > if (!parent1)
> > @@ -446,7 +447,8 @@ static int real_merge(struct merge_tree_options *o,
> > if (o->show_messages == -1)
> > o->show_messages = !result.clean;
> >
> > - printf("%s%c", oid_to_hex(&result.tree->object.oid), line_termination);
> > + tree_oid = result.tree ? &result.tree->object.oid : null_oid();
> > + printf("%s%c", oid_to_hex(tree_oid), line_termination);
>
> As we are not using tree_oid anywhere else, it might be cleaner to
> switch on result.tree more explicitly, especially given that the
> return statement also uses result.tree to decide what exit status to
> use, e.g.
>
> if (result.tree)
> fputs(oid_to_hex(&result.tree->object.oid), stdout);
> else
> fputs(oid_to_hex(null_oid()), stdout);
> fputc(line_termination, stdout);
>
> but that is merely "might be cleaner".
I would call it "more readable", even, not just "cleaner" :-)
> > - return !result.clean; /* result.clean < 0 handled above */
> > + return !result.tree || !result.clean; /* result.clean < 0 handled above */
>
> The original returned 1 when we failed to cleanly merge and 0 when
> we cleanly merged. Now, if we failed to come up with a merged tree,
> result.tree would be NULL and we return 1 from the function, because
> we failed to cleanly merge.
>
> OK. These negations make my head spin X-<.
And here I thought that you couldn't possibly not like if I didn't refrain
from not avoiding double negations...
Ciao,
Dscho
