Patchwork [2,of,5,V3] merge: add labels parameter from merge.update to filemerge

login
register
mail settings
Submitter Durham Goode
Date May 13, 2014, 12:47 a.m.
Message ID <20a401c6feed49f6597e.1399942054@dev2000.prn2.facebook.com>
Download mbox | patch
Permalink /patch/4733/
State Superseded
Headers show

Comments

Durham Goode - May 13, 2014, 12:47 a.m.
# HG changeset patch
# User Durham Goode <durham@fb.com>
# Date 1399593263 25200
#      Thu May 08 16:54:23 2014 -0700
# Node ID 20a401c6feed49f6597e76789b055b0e84b20265
# Parent  09732bdb2784d9402e278c36406819a98919c2ca
merge: add labels parameter from merge.update to filemerge

Adds a labels function parameter to all the functions between merge.update and
filemerge.filemerge. This will allow commands like rebase to specify custom
marker labels.
Pierre-Yves David - May 13, 2014, 8:42 a.m.
On 05/12/2014 05:47 PM, Durham Goode wrote:
> # HG changeset patch
> # User Durham Goode<durham@fb.com>
> # Date 1399593263 25200
> #      Thu May 08 16:54:23 2014 -0700
> # Node ID 20a401c6feed49f6597e76789b055b0e84b20265
> # Parent  09732bdb2784d9402e278c36406819a98919c2ca
> merge: add labels parameter from merge.update to filemerge
>
> Adds a labels function parameter to all the functions between merge.update and
> filemerge.filemerge. This will allow commands like rebase to specify custom
> marker labels.

Note that all the internal configuration still use "local" and "other". 
including (but probably not limited to:

1. internal:local and internal:other
2. $local $other $base argument in mergetools-config (and it's help)
3. merge-tools help topic: (hg help merge-tools)

What's you plan to not confuse user to death in when local and other get 
renamed?
Durham Goode - May 13, 2014, 4:54 p.m.
On 5/13/14, 1:42 AM, "Pierre-Yves David" <pierre-yves.david@ens-lyon.org>
wrote:

>
>
>On 05/12/2014 05:47 PM, Durham Goode wrote:
>> # HG changeset patch
>> # User Durham Goode<durham@fb.com>
>> # Date 1399593263 25200
>> #      Thu May 08 16:54:23 2014 -0700
>> # Node ID 20a401c6feed49f6597e76789b055b0e84b20265
>> # Parent  09732bdb2784d9402e278c36406819a98919c2ca
>> merge: add labels parameter from merge.update to filemerge
>>
>> Adds a labels function parameter to all the functions between
>>merge.update and
>> filemerge.filemerge. This will allow commands like rebase to specify
>>custom
>> marker labels.
>
>Note that all the internal configuration still use "local" and "other".
>including (but probably not limited to:
>
>1. internal:local and internal:other
>2. $local $other $base argument in mergetools-config (and it's help)
>3. merge-tools help topic: (hg help merge-tools)
>
>What's you plan to not confuse user to death in when local and other get
>renamed?

User¹s are already confused to death, so my plan is leave the
documentation alone and be satisfied that at least the conflict markers
aren¹t confusing anymore.
Pierre-Yves David - May 15, 2014, 8:09 a.m.
On 05/13/2014 09:54 AM, Durham Goode wrote:
> On 5/13/14, 1:42 AM, "Pierre-Yves David" <pierre-yves.david@ens-lyon.org>
> wrote:
>
>>
>>
>> On 05/12/2014 05:47 PM, Durham Goode wrote:
>>> # HG changeset patch
>>> # User Durham Goode<durham@fb.com>
>>> # Date 1399593263 25200
>>> #      Thu May 08 16:54:23 2014 -0700
>>> # Node ID 20a401c6feed49f6597e76789b055b0e84b20265
>>> # Parent  09732bdb2784d9402e278c36406819a98919c2ca
>>> merge: add labels parameter from merge.update to filemerge
>>>
>>> Adds a labels function parameter to all the functions between
>>> merge.update and
>>> filemerge.filemerge. This will allow commands like rebase to specify
>>> custom
>>> marker labels.
>>
>> Note that all the internal configuration still use "local" and "other".
>> including (but probably not limited to:
>>
>> 1. internal:local and internal:other
>> 2. $local $other $base argument in mergetools-config (and it's help)
>> 3. merge-tools help topic: (hg help merge-tools)
>>
>> What's you plan to not confuse user to death in when local and other get
>> renamed?
>
> User¹s are already confused to death, so my plan is leave the
> documentation alone and be satisfied that at least the conflict markers
> aren¹t confusing anymore.

As discussed over diner:

- This change is a massive step forward to reduce confusion.
- light updates the merge-tools docs about the subtleties introduces by 
this changes would be great.

Patch

diff --git a/hgext/largefiles/overrides.py b/hgext/largefiles/overrides.py
--- a/hgext/largefiles/overrides.py
+++ b/hgext/largefiles/overrides.py
@@ -452,9 +452,9 @@ 
 
 # Override filemerge to prompt the user about how they wish to merge
 # largefiles. This will handle identical edits without prompting the user.
-def overridefilemerge(origfn, repo, mynode, orig, fcd, fco, fca):
+def overridefilemerge(origfn, repo, mynode, orig, fcd, fco, fca, labels=None):
     if not lfutil.isstandin(orig):
-        return origfn(repo, mynode, orig, fcd, fco, fca)
+        return origfn(repo, mynode, orig, fcd, fco, fca, labels=labels)
 
     ahash = fca.data().strip().lower()
     dhash = fcd.data().strip().lower()
diff --git a/mercurial/filemerge.py b/mercurial/filemerge.py
--- a/mercurial/filemerge.py
+++ b/mercurial/filemerge.py
@@ -295,6 +295,8 @@ 
     '{ifeq(branch, "default", "", "{branch} ")}' +
     '- {author|user}: "{desc|firstline}"')
 
+_defaultconflictlabels = ['local', 'other']
+
 def _formatlabels(repo, fcd, fco, labels):
     """Formats the given labels using the conflict marker template.
 
@@ -316,7 +318,7 @@ 
         _formatconflictmarker(repo, co, tmpl, labels[1], pad),
     ]
 
-def filemerge(repo, mynode, orig, fcd, fco, fca):
+def filemerge(repo, mynode, orig, fcd, fco, fca, labels=None):
     """perform a 3-way merge in the working directory
 
     mynode = parent node before merge
@@ -373,10 +375,12 @@ 
 
     ui.debug("my %s other %s ancestor %s\n" % (fcd, fco, fca))
 
-    labels = ['local', 'other']
     if ui.configbool('merge', 'oldconflictmarkers', False):
-        formattedlabels = labels
+        formattedlabels = _defaultconflictlabels
     else:
+        if not labels:
+            labels = _defaultconflictlabels
+
         formattedlabels = _formatlabels(repo, fcd, fco, labels)
 
     needcheck, r = func(repo, mynode, orig, fcd, fco, fca, toolconf,
diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -264,7 +264,7 @@ 
             if entry[0] == 'u':
                 yield f
 
-    def resolve(self, dfile, wctx):
+    def resolve(self, dfile, wctx, labels=None):
         """rerun merge process for file path `dfile`"""
         if self[dfile] == 'r':
             return 0
@@ -287,7 +287,8 @@ 
         f = self._repo.opener("merge/" + hash)
         self._repo.wwrite(dfile, f.read(), flags)
         f.close()
-        r = filemerge.filemerge(self._repo, self._local, lfile, fcd, fco, fca)
+        r = filemerge.filemerge(self._repo, self._local, lfile, fcd, fco, fca,
+            labels=labels)
         if r is None:
             # no real conflict
             del self._state[dfile]
@@ -610,7 +611,7 @@ 
     if i > 0:
         yield i, f
 
-def applyupdates(repo, actions, wctx, mctx, overwrite):
+def applyupdates(repo, actions, wctx, mctx, overwrite, labels=None):
     """apply the merge action list to the working directory
 
     wctx is the working copy context
@@ -698,7 +699,7 @@ 
                                  overwrite)
                 continue
             audit(f)
-            r = ms.resolve(f, wctx)
+            r = ms.resolve(f, wctx, labels=labels)
             if r is not None and r > 0:
                 unresolved += 1
             else:
@@ -911,7 +912,7 @@ 
                 repo.dirstate.normal(f)
 
 def update(repo, node, branchmerge, force, partial, ancestor=None,
-           mergeancestor=False):
+           mergeancestor=False, labels=None):
     """
     Perform a merge between the working directory and the given node
 
@@ -1091,7 +1092,7 @@ 
             # note that we're in the middle of an update
             repo.vfs.write('updatestate', p2.hex())
 
-        stats = applyupdates(repo, actions, wc, p2, overwrite)
+        stats = applyupdates(repo, actions, wc, p2, overwrite, labels=labels)
 
         if not partial:
             repo.setparents(fp1, fp2)