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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Johannes,

On Tue, Jan 30, 2024 at 12:04 PM Johannes Schindelin
<Johannes.Schindelin@xxxxxx> wrote:
>
> Hi Elijah,
>
> On Mon, 29 Jan 2024, Elijah Newren wrote:
>
> > On Fri, Jan 26, 2024 at 6:18 AM Johannes Schindelin via GitGitGadget
> > <gitgitgadget@xxxxxxxxx> wrote:
> > >
[...]
> That's funny: I asked Victoria Dye to look over the patch, and she pointed
> out the exact opposite: I had written `<tree>` and she remarked that most
> of Git's manual pages would call this a `<tree-ish>` :-)

A code review isn't complete until you get two contradictory requests, I guess?

> On another funny note, I tried to establish the term "ent" for this category
> almost 222 months ago because I also disliked the "-ish" convention:
> https://lore.kernel.org/git/Pine.LNX.4.63.0508051655480.8418@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/

:-)

> > > diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
> > > index 3bdec53fbe5..cbd8e15af6d 100644
> > > --- a/builtin/merge-tree.c
> > > +++ b/builtin/merge-tree.c
> > > @@ -429,35 +429,43 @@ static int real_merge(struct merge_tree_options *o,
> > >         struct merge_options opt;
> > >
> > >         copy_merge_options(&opt, &o->merge_options);
> > > -       parent1 = get_merge_parent(branch1);
> > > -       if (!parent1)
> > > -               help_unknown_ref(branch1, "merge-tree",
> > > -                                _("not something we can merge"));
> > > -
> > > -       parent2 = get_merge_parent(branch2);
> > > -       if (!parent2)
> > > -               help_unknown_ref(branch2, "merge-tree",
> > > -                                _("not something we can merge"));
> > > -
> > >         opt.show_rename_progress = 0;
> > >
> > >         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.
>
> That is true. And it is not totally obvious to many users that they could
> then call `git show aaaaaaa:file` to see the full pre-image on the
> first-parent side.
>
> On the other hand, the label that is shown is precisely what the user
> specified on the command-line.

So this is only for direct use?  I was curious if the user was using
some other tool of yours, perhaps even some web GUI, and thus that
something else was controlling what was passed to git-merge-tree.

> For example:
>
>         $ git merge-tree --merge-base=v2.42.0:t v2.43.0~11:t v2.43.0~10^2:t
>
> will result in the following conflict markers:
>
>         $ git show 021c3ce211:t0091-bugreport.sh
>         [...]
>         <<<<<<< v2.43.0~11:t
>                 grep usage output &&
>                 test_path_is_missing git-bugreport-*
>         '
>
>         test_expect_success 'incorrect positional arguments abort with usage and hint' '
>                 test_must_fail git bugreport false 2>output &&
>                 grep usage output &&
>                 grep false output &&
>         =======
>                 test_grep usage output &&
>         >>>>>>> v2.43.0~10^2:t
>         [...]
>
> which I personally find very pleasing output.

Oh, good point -- if users provide trees in the revision:path format
then they still have access to more information about why the change
was made via the revision.

Of course, if users are using the tool directly, presumably they have
access to more information about where those trees came from anyway
even without the conflict label.

> Besides, the manual page of `git merge-tree` says in no sugar-coated
> words:
>
>         Do NOT look through the resulting toplevel tree to try to find which
>         files conflict; [...]
>
> :-)

Right, but this isn't using the tree to find which files conflict;
they already are looking at the conflict.  They are instead wanting to
learn why certain textual changes were made on one side of history to
better inform how to resolve an otherwise less than obvious conflict
resolution.  At least, that's the only thing I've seen those conflict
labels be used for.





[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux