Julian Foad wrote on Mon, Apr 07, 2014 at 13:53:36 +0100:
> Bert Huijben wrote:
> > Julian Foad wrote:
> >> +1 to the suggestion. The current two-way info is pretty difficult to use in any
> >> non-trivial case, and the 3-way suggestion is significantly more usable and not
> >> much more difficult to understand. Furthermore it is what I expect to see for
> >> a three-way merge, and, if I remember correctly, is what I already do see in
> >> the interactive conflict resolver.
> >
> > +1 on the idea of improving the merge information..
> >
> > But how this eventually should end up in 'svn' depends really on the
> > question:
> > * Is the file containing the markers the place to learn about the different
> > sides or is it just a file created to allow easy resolving.
>
>
> Good question. I suppose we need to decide whether this file is more an API or more a UI. If we decide it's more important to maintain backward compatibility and treat it as an API, then we should not change it, and should provide the requested change another way.
>
We seem to be in agreement that foo.c is not an API, but
foo.c{.merge-left,.merge-right,.mine} _are_ an API. The former file can
be derived from the latter three files.
> Is your merge tool, or any software you know of, relying on this file's current format?
>
>
> > The file only contains the information about the conflicting hunks, while
> > you would really need to know about all changes to decide what to do. And in
> > that respect the 'svn resolve' interactive resolver provides much more
> > information.
>
What is that additional information?
To be clear, my use-case had plenty of conflicts that couldn't be
resolved by any of the pre-cooked uptions (mf/mc/tf/tc/12/12/etc); they
had to be manually merged. Conflicts that can be solved by the
pre-cooked option I just resolve during the update or merge, so they
never get as far as being rendered in the target file on disk.
(Full disclosure: my use-case was with git-svn, not with svn itself.
When I say later "I used 3-way conflicts rendering", it was with
git-svn, not with a patched svn build. But I believe the experience
carries over.)
>
> Yes, I agree. Or, to talk about the same level of API, the set of
> conflict 'artifact' files (filename.mine/filename.yours/etc.) hold the
> full information needed.
>
>
> > Before 1.7/1.8 we couldn't really restart the resolving after an operation,
> > but this is now a much better tool to resolve conflicts than we can ever
> > create in a plain text file.
>
>
> [...]
> >> It's true that the target file has a pristine version, but the pristine version isn't
> >> *particularly* associated with the merge itself. We merge changes into the
> >> working/actual version, and whether I commit local mods before starting the
> >> merge makes no difference to the merging procedure or to the desired
> >> result. The pristine version is just "the version before the version I'm merging
> >> into". The fact that it was uncommitted makes it interesting from a WC
> >> management perspective, but not particularly from a merge perspective.
> >
> > Essentially a merge is incorporating the changes between LEFT and RIGHT into
> > the PRISTINE file.
>
>
> We probably mean the same thing but are describing it in different
> ways. To me, the essence of a merge is it merges into the working
> file. That's the purpose, the logical idea, the intended
> functionality. When there are no local changes the pristine text is
> the same as the working text but that's just just incidental.
>
Agreed, merge takes the delta from LEFT to RIGHT and applies it to WORKING.
The PRISTINE contents is ignored.
>
> > But as we support merging into a locally changed file we just ignore the
> > PRISTINE file and use the original WORKING as if it were the pristine file.
> >
> > That doesn't make the difference between PRISTINE and WORKING not
> > interesting... It still contains the important notion of what is really an
> > uncommitted local change (MINE) vs what was already there on checkout
> > (BASE/PRISTINE).
>
Certainly. But this isn't a property unique to PRISTINE; just like a
PRISTINE-aware merge tool could say that a conflict is caused by the
pre-merge local changes, a PREV-aware merge tool could flag a conflict
as caused by PREV's successor commit (the one that resulted in
PRISTINE). More generally, a merge tool might run 'blame -r WORKING' on
the .mine file and attribute each conflict that way.
>
> Again, we understand the same thing in different ways.
>
> > Sure there are even more interesting revisions if you know the full graph of
> > changes, but the merge code in libsvn_wc just knows these 4
> > files(PRISTINE,LEFT,RIGHT,WORKING), and produces a MERGED file from them:
> > and a Boolean conflicted vs not conflicted.
>
>
> Right, but the only reason the WC library needs to know about the pristine version is because that's such a big part of how the WC is architected. (There is no fully abstracted API for accessing the working/actual layer without talking about the pristines.) If you look at the merge logic in libsvn_client, it doesn't care.
>
>
>
> > I'm pretty sure that you need more than a plain text file GUI to add the
> > notion of more than 3 trees in a way that our users are able to understand
> > it.
>
>
> Very likely.
>
True, but not really relevant: users don't need to understand the
concept of three trees to be able to resolve a conflict rendered in
the three-way form.
All users need to know is "Apply the delta from |||||| to ======
to the <<<<<< part".
And, mind you, that's a hell of a clearer instruction then "Take the
<<<< and ==== part, solve the problem and let me know when you have done
so. Good luck.", which is basically what the 2-way display gives.
> > But I'm thinking that calling either LEFT or PRISTINE 'original' in the
> > MERGED file is not really going to help our typical user.
Then the labels can say something else. We could use single words or
even sentences (up to 60 bytes long, assuming an 80-column file).
> > And I wish my merge tool was smart enough to really understand 4 inputs...
> > Showing LEFT as the older version is showing a lot of non-changes as if they
> > were interesting for the merge, while they could have been filtered as
> > unrelated with access to PRISTINE.
>
What does this have to do with the proposed behaviour change?
It sounds like you're describing general problems you see with merging
(lack of good three-way tree-aware diff3, and a merge tool that flags
false-positive changes). I don't understand what any of those problems
have to do with the proposed one-line change.
The --accept=postpone text rendering isn't supposed to be a bells and
whistles tool. It's the built-in, default tool. The question that
matters isn't what's the best merge tool, or the best way to approach a
given merge; the question that matters is: _provided_ that the user is
using the built-in tool, _and_ has chosen --accept=postpone, whether
they would be better served by a 2-way or 3-way rendering of conflicts.
And I think the 3-way is better. I've used it and it makes resolution a
breeze. All the information I need is right there in front of me, in
the editor instance I already have open. That's a superset of the
information given by the 2-way renderer, and the additional information
is useful: it means I have a *clear algorithm* to follow to resolve the
problem and move on with whatever I'm working on.
It's not a general three-way tree-aware merge tool. It simply is a
change that solves a real problem.
Daniel
Received on 2014-04-07 21:16:44 CEST