Re: [PATCH] merge-tree: accept 3 trees as arguments

[Junio C Hamano <gitster@pobox.com>]

Elijah Newren <newren@gmail.com> writes:

>> ---merge-base=<commit>::
>> +--merge-base=<tree-ish>::
>
> A very minor point, but any chance we can just use `<tree>`, like
> git-restore does?  I've never liked the '-ish' that we use as it seems
> to throw users, and I think they understand that they can use a commit
> or a tag where a tree is expected

You are right that the "X-ish" was invented exactly so that folks
(including the nitpicky types among us) can tell if only "X" is
accepted, or other types of objects that uniquely can be peeled to a
"X" also can be used.  Many commands at the Porcelain level may take
a commit or a tag that eventually peel to a tree where at the lowest
level we only need it to be a tree, but it still matters in some
lower level corners of the system where the plumbing commands that
only accept a tree but not a tree-ish exist.  We've discussed this
in the past, e.g.:

    https://lore.kernel.org/git/7voc8ihq4a.fsf@alter.siamese.dyndns.org/

In general, I am OK to update the documentation and usage strings to
drop the "-ish" suffix when it is clear from the context.  But what
does it exactly mean that "--merge-base=<tree>" is meant to take any
tree-ish from the context of this documentation we are discussing?

 - Is it because merge-tree is a Porcelain command and we would
   adopt "Porcelains are to peel liberally what they accept, and
   when they are documented to take X they always accept any X-ish"
   rule?

 - Is it because the description that follows "--merge-base=<tree>"
   header does not mention "contrary to our usual convention, this
   option only accepts a tree and not a commit or a tag that points
   at a tree" and we would adopt "all commands and options that are
   documented to take X take X-ish, unless explicitly documented
   they only take X"?

As long as we clearly spell out such a ground rule and make sure
readers of the documentation understand it, I will not object to
such a tree-wide clean-up.  The current ground rule is "We write X
when we mean to take only X and we write X-ish otherwise", if I am
not mistaken.

>>         opt.branch1 = branch1;
>>         opt.branch2 = branch2;
>
> If branch1 and branch2 refer to trees, then when users hit conflicts
> they'll see e.g.
>
> <<<<<<< aaaaaaa
>   somecode();
> =======
>   othercode();
>>>>>>>> bbbbbbb
>
> but aaaaaaa and bbbbbbb are not commits that they can find.

Correct.  They are what they fed as two trees to be merged, aren't
they?  They (or the script that drives merge-tree and fed these two
trees) should be recognise these two trees, as long as they are told
that is what we show, no?

> That raises the question: if the user passes trees in, should we
> require helpful names be provided as additional parameters?

If the user wants to give human-readable name to override whatever
the code would internally produce, such new options may help them,
and should probably apply equally to input that happens to be a
commit (or a tag that is a tree-ish) as well, I would think.

> Or are
> the usecases such that we don't expect callers to have any useful
> information about where these trees come from and these suboptimal
> conflicts are the best we can do?

I do not necessarily think the output is "suboptimal" from the point
of view of our users, exactly because that is what they fed into the
command themselves.