Re: [PATCH 00/12] RFC: In-core git merge-tree ("Server side merges")

[Christian Couder <christian.couder@gmail.com>]

Hi Dscho,

On Fri, Jan 28, 2022 at 1:58 PM Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
>
> Hi Christian,
>
> On Wed, 26 Jan 2022, Christian Couder wrote:
>
> > On Wed, Jan 26, 2022 at 1:03 PM Johannes Schindelin
> > <Johannes.Schindelin@gmx.de> wrote:
> > >
> > > On Wed, 26 Jan 2022, Christian Couder wrote:
> > >
> > > > The reason is that I think in many cases when there are conflicts,
> > > > the conflicts will be small and the user will want to see them. So
> > > > it would be simpler to just have an option to show any conflict
> > > > right away, rather than have the user launch another command (a
> > > > diff-tree against which tree and with which options?).
> > >
> > > That assumes that server-side merge UIs will present merge conflicts in
> > > the form of diffs containing merge conflict markers. Which I don't think
> > > will happen, like, ever.
> >
> > Please take a look at:
> >
> > https://docs.gitlab.com/ee/user/project/merge_requests/conflicts.html#resolve-conflicts-in-the-inline-editor
> >
> > As you can see in the image there are conflict markers in the file
> > displayed by the server UI.
>
> Please note the difference between what I wrote above (present merge
> conflicts in the form of diffs containing merge conflict markers) and what
> is shown in the document you linked to (present a file annotated with
> merge conflict markers).
>
> There is no diff in that page.

The server UI could just get a diff with the conflicts inside instead
of the full files with conflict inside, as the diff would be smaller
to parse than the full files. So even if it's not shown, the diff
could still be useful.

Also just above the section of the link I sent, there is this section

https://docs.gitlab.com/ee/user/project/merge_requests/conflicts.html#resolve-conflicts-in-interactive-mode

where one can see diff markers in the image. There are no conflict
markers in those images, but it's possible that a future UI could
combine both a diff and conflict markers.

Also please note that I don't absolutely require diffs. At the
beginning of the paragraph from my original email that you quoted
above, there was:

"It's not a big issue for me to not include them right now as long as
it's possible to add cli options later that add them."

So I was just saying that the format and code should be flexible
enough to be able to easily accommodate sending further data like
diffs with additional options. I think it's a very reasonable request.
So please don't make it a huge issue. You can always NACK a patch
adding such an option later.

> What's more: there are not only conflict markers in the editor,

You don't see the ">>>>>>>"?

> there is
> clearly a visual marker next to the line number that indicates that the
> editor has a fundamental understanding where the conflict markers are.

Yeah, so this shows that those markers can be important for the editor.

> Which means that the conflict markers must have been generated
> independently of Git rather than parsed in some random diff that was
> produced by Git.

Why couldn't they be generated by Git and then just parsed from a diff
in the future, even if that was not the case here?

> In other words: you are making my case for me that `git merge-tree` should
> not generate diff output because it would not even be used.

The other link above in this email actually shows that diffs are used
right now to resolve conflicts.

> > > In short: I completely disagree that we should introduce a new command,
> > > and I also completely disagree that the `merge-tree` command should output
> > > any diffs.
> > >
> > > I do agree that we need to be mindful of what we actually need, and in
> > > that regard, I reiterate that we need to let concrete use cases guide us.
> > > As part of GitLab, you might be in an excellent position to look at
> > > GitLab's concrete server-side needs when it comes to use `git merge-tree`
> > > to perform merges.
> >
> > I hope I provided a concrete use case with the link above.
>
> Sorry, I apparently was a bit unclear.
>
> In the context of discussing `git merge-tree`, a low-level Git command,
> when I talk about a user, I do not mean a human being, but a program that
> calls that command and parses its output.

So it could very well parse diffs containing conflict markers and show
those conflict markers.

[...]

> Of course, I am still left guessing what the server-side needs concretely,
> because that is not at all obvious from the user-facing web site to which
> you sent me. What is needed is a good, hard look at the actual _code_, the
> code that calls into libgit2 to perform a merge, and that could instead
> spawn a `git merge-tree` process to accomplish the same thing.
>
> We need to get away from hypothetical scenarios. They're not helping.

I am not even asking for a feature, just to make it possible to extend
the output of a brand new command in an RFC with some possibly useful
things, and you are making such requests...

Please relax a bit on this.