[PATCH] merge: improve conflict markers by pointing to introrev

[Pierre-Yves David]

On 03/07/2016 05:12 PM, Simon Farnsworth wrote:
> On 07/03/2016, 14:56, "Yuya Nishihara" <youjah at gmail.com on behalf of yuya at tcha.org> wrote:
>
>
>> On Mon, 7 Mar 2016 13:58:29 +0000, Simon Farnsworth wrote:
>>> On 07/03/2016, 13:22, "Yuya Nishihara" <youjah at gmail.com on behalf of yuya at tcha.org> wrote:
>>>
>>>
>>>
>>>> On Fri, 4 Mar 2016 08:31:40 -0800, Simon Farnsworth wrote:
>>>>> # HG changeset patch
>>>>> # User Simon Farnsworth <simonfar at fb.com>
>>>>> # Date 1457034054 0
>>>>> #      Thu Mar 03 19:40:54 2016 +0000
>>>>> # Node ID f9c8bf605e5b96530f6a4636c78f9c444767c445
>>>>> # Parent  9d6c19c8413a039aff8399d6b1db573cb6610fab
>>>>> merge: improve conflict markers by pointing to introrev
>>>>>
>>>>> If you're working in a fast moving repository, the nodes chosen for conflict
>>>>> markers are apparently nonsensical - they're the revisions you're actually
>>>>> merging, even if the file in conflict did not change in those revisions.
>>>>>
>>>>> Change the conflict markers to find the introrev for local and other, and
>>>>> use that as the node instead. This means that the conflict markers now point
>>>>> to the commits in which the conflicting file was last changed.
>>>>
>>>> The default conflict marker includes {tags} and {bookmarks}. I don't think
>>>> they should be changed to the introrev. Perhaps we'll need new template keyword
>>>> for the introrev.
>>>
>>> Why do you think that {tags} and {bookmarks} should be on the nodes being merged, not on the introrevs?
>>>
>>> My reasoning for changing the node itself (hence also changing {tags} and {bookmarks}) is that when you're dealing with a merge conflict, you're not interested in the merge heads themselves - in the common cases, you either know exactly which nodes are being merged (because you're merging two trees of your own work together), or one is familiar (your own work), and the other is random (because you don't know the state of the remote tree at the point you did "hg pull"). Having the tags and bookmarks in the conflict marker come from the introrev means that I get told if the conflict happened at an interesting point in the tree. For slow-moving trees, this is uninteresting information - the introrev of a conflict is the same as the merge revision, because I simply don't get many conflicts in the first place; it's once you get into fast moving trees that conflicts get interesting, and in those cases, the merge nodes are pseudo-random anyway.
>>
>> If you're merging two bookmarked branches, {bookmarks} can provide which side
>> is which branch. I think that's why the conflict marker shows {bookmarks}.
>
> My assumption is that you already know which side is which bookmark - and I'm enhancing merge state to have descriptions of "local" and "other" that will help with that.

It is not clear to me that "you already know which side is which 
bookmark", people get confused easily especially on large merge. 
Furthermore when they use command overlay that hide them the underlying 
Mercurial invocation (like your employer do).

>> Does the bookmark of the introrev provide something interesting?
>
> Depends on your workflow; it does if you're in the habit of creating bookmarks at interesting intermediate states, as then you'll get told that this conflict is with the interesting intermediate state, not with the latest state.
>
> I can see the point of giving the conflict markers both nodes, though (introrev and merge point), and defaulting to tags and bookmarks from the merge node, not the introrev node. I'll look into this for a V2.

My general opinion on this "using introrev" business is that it is a red 
herring. Revision touching a file are likely to be numberous so print 
the latest one is going to marginaly help in some case and confuse user 
in all the other. I would rather see provided an easy way to see the 
"what revision touched that file on each side" information than trying 
to try to fit very complexe information into the limited conflict marker 
entry.

-- 
Pierre-Yves David