Patchwork [1,of,5,V3] merge: add conflict marker formatter (BC)

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

Comments

Durham Goode - May 13, 2014, 12:47 a.m.
# HG changeset patch
# User Durham Goode <durham@fb.com>
# Date 1399593022 25200
#      Thu May 08 16:50:22 2014 -0700
# Node ID 09732bdb2784d9402e278c36406819a98919c2ca
# Parent  697fba94dec9f192bd1fa877e2a98e3d10042fc6
merge: add conflict marker formatter (BC)

Adds a conflict marker formatter that can produce custom conflict marker
descriptions. It can be set via merge.conflictmarkertemplate. The old behavior
can be used still by setting merge.oldconflictmarkers=True.

The default format is similar to:

  {node|short} {tag} {branch} {bookmarks} - {author}: "{desc|firstline}"

And renders as:

  contextblahblah
  <<<<<<< local: c7fdd7ce4652 - durham: "Fix broken stuff in my feature branch"
  line from my changes
  =======
  line from the other changes
  >>>>>>> other: a3e55d7f4d38  master - sid0: "This is a commit to master th...
  morecontextblahblah
Pierre-Yves David - May 13, 2014, 8:34 a.m.
On 05/12/2014 05:47 PM, Durham Goode wrote:
> # HG changeset patch
> # User Durham Goode <durham@fb.com>
> # Date 1399593022 25200
> #      Thu May 08 16:50:22 2014 -0700
> # Node ID 09732bdb2784d9402e278c36406819a98919c2ca
> # Parent  697fba94dec9f192bd1fa877e2a98e3d10042fc6
> merge: add conflict marker formatter (BC)
>
> Adds a conflict marker formatter that can produce custom conflict marker
> descriptions. It can be set via merge.conflictmarkertemplate. The old behavior
> can be used still by setting merge.oldconflictmarkers=True.

1. Do not use "old" this will put the one doing the next update to this 
topic in trouble.

Wouldn't that make more sense to have a it as:

[merge-tools]
internal:merge.conflictmarkers = simplest


>
> The default format is similar to:
>
>    {node|short} {tag} {branch} {bookmarks} - {author}: "{desc|firstline}"
>
> And renders as:
>
>    contextblahblah
>    <<<<<<< local: c7fdd7ce4652 - durham: "Fix broken stuff in my feature branch"
>    line from my changes
>    =======
>    line from the other changes
>    >>>>>>> other: a3e55d7f4d38  master - sid0: "This is a commit to master th...
>    morecontextblahblah
>
> diff --git a/mercurial/filemerge.py b/mercurial/filemerge.py
> --- a/mercurial/filemerge.py
> +++ b/mercurial/filemerge.py
> @@ -7,7 +7,7 @@
>
>   from node import short
>   from i18n import _
> -import util, simplemerge, match, error
> +import util, simplemerge, match, error, templater, templatekw
>   import os, tempfile, re, filecmp
>
>   def _toolstr(ui, tool, part, default=""):
> @@ -269,6 +269,53 @@
>           return True, r
>       return False, 0
>
> +def _formatconflictmarker(repo, ctx, template, label, pad):

This function could use a docstring.
Durham Goode - May 13, 2014, 4:51 p.m.
On 5/13/14, 1:34 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 1399593022 25200
>> #      Thu May 08 16:50:22 2014 -0700
>> # Node ID 09732bdb2784d9402e278c36406819a98919c2ca
>> # Parent  697fba94dec9f192bd1fa877e2a98e3d10042fc6
>> merge: add conflict marker formatter (BC)
>>
>> Adds a conflict marker formatter that can produce custom conflict marker
>> descriptions. It can be set via merge.conflictmarkertemplate. The old
>>behavior
>> can be used still by setting merge.oldconflictmarkers=True.
>
>1. Do not use "old" this will put the one doing the next update to this
>topic in trouble.
>
>Wouldn't that make more sense to have a it as:
>
>[merge-tools]
>internal:merge.conflictmarkers = simplest

I don¹t like Œ= simplest¹ since there are no other values.
Œinternal:merge.basicconflictmarkers = True¹ perhaps.

That said, I don¹t really care what we name this thing.  If Matt wants to
chime in and deliver a final verdict, I¹m happy to update and resend.
Augie Fackler - May 14, 2014, 12:56 a.m.
On May 13, 2014, at 12:51 PM, Durham Goode <durham@fb.com> wrote:

> On 5/13/14, 1:34 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 1399593022 25200
>>> #      Thu May 08 16:50:22 2014 -0700
>>> # Node ID 09732bdb2784d9402e278c36406819a98919c2ca
>>> # Parent  697fba94dec9f192bd1fa877e2a98e3d10042fc6
>>> merge: add conflict marker formatter (BC)
>>> 
>>> Adds a conflict marker formatter that can produce custom conflict marker
>>> descriptions. It can be set via merge.conflictmarkertemplate. The old
>>> behavior
>>> can be used still by setting merge.oldconflictmarkers=True.
>> 
>> 1. Do not use "old" this will put the one doing the next update to this
>> topic in trouble.
>> 
>> Wouldn't that make more sense to have a it as:
>> 
>> [merge-tools]
>> internal:merge.conflictmarkers = simplest
> 
> I don¹t like Œ= simplest¹ since there are no other values.
> Œinternal:merge.basicconflictmarkers = True¹ perhaps.
> 
> That said, I don¹t really care what we name this thing.  If Matt wants to
> chime in and deliver a final verdict, I¹m happy to update and resend.

I'd be a fan of naming the conflict marker formats. I might call the existing one "traditional" or "basic".

> 
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Pierre-Yves David - May 15, 2014, 8:09 a.m.
On 05/13/2014 05:56 PM, Augie Fackler wrote:
>
> On May 13, 2014, at 12:51 PM, Durham Goode <durham@fb.com> wrote:
>
>> On 5/13/14, 1:34 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 1399593022 25200
>>>> #      Thu May 08 16:50:22 2014 -0700
>>>> # Node ID 09732bdb2784d9402e278c36406819a98919c2ca
>>>> # Parent  697fba94dec9f192bd1fa877e2a98e3d10042fc6
>>>> merge: add conflict marker formatter (BC)
>>>>
>>>> Adds a conflict marker formatter that can produce custom conflict marker
>>>> descriptions. It can be set via merge.conflictmarkertemplate. The old
>>>> behavior
>>>> can be used still by setting merge.oldconflictmarkers=True.
>>>
>>> 1. Do not use "old" this will put the one doing the next update to this
>>> topic in trouble.
>>>
>>> Wouldn't that make more sense to have a it as:
>>>
>>> [merge-tools]
>>> internal:merge.conflictmarkers = simplest
>>
>> I don¹t like Œ= simplest¹ since there are no other values.
>> Œinternal:merge.basicconflictmarkers = True¹ perhaps.

Well, the would be obviously be another value.

could be basic/detailed. This leave the field open for other option if 
someone come with a better idea 3 year from now.

Or even in one week if we reusing the same option to includes the base:

    basic/detailed/withbase

>> That said, I don¹t really care what we name this thing.  If Matt wants to
>> chime in and deliver a final verdict, I¹m happy to update and resend.

Durham quite convinced me that exposing internal:merge to the user was ugly.

[ui]
mergemarkers=basic

> I'd be a fan of naming the conflict marker formats. I might call the existing one "traditional" or "basic".

The naming part will be fun.

The old version could be named "basic" or "1980th".

the new one could be named "detailed" or "templated"

leaning toward basic/detailed, but I love the message conveyed by "1980th"

Patch

diff --git a/mercurial/filemerge.py b/mercurial/filemerge.py
--- a/mercurial/filemerge.py
+++ b/mercurial/filemerge.py
@@ -7,7 +7,7 @@ 
 
 from node import short
 from i18n import _
-import util, simplemerge, match, error
+import util, simplemerge, match, error, templater, templatekw
 import os, tempfile, re, filecmp
 
 def _toolstr(ui, tool, part, default=""):
@@ -269,6 +269,53 @@ 
         return True, r
     return False, 0
 
+def _formatconflictmarker(repo, ctx, template, label, pad):
+        if ctx.node() is None:
+            ctx = ctx.p1()
+
+        props = templatekw.keywords.copy()
+        props['templ'] = template
+        props['ctx'] = ctx
+        props['repo'] = repo
+        templateresult = template('conflictmarker', **props)
+
+        label = ('%s:' % label).ljust(pad + 1)
+        mark = '%s %s' % (label, templater.stringify(templateresult))
+
+        # The <<< marks add 8 to the length, and '...' adds three, so max
+        # length of the actual marker is 69.
+        maxlength = 80 - 8 - 3
+        if len(mark) > maxlength:
+            mark = mark[:maxlength] + '...'
+        return mark
+
+_defaultconflictmarker = ('{node|short} ' +
+    '{ifeq(tags, "tip", "", "{tags} ")}' +
+    '{if(bookmarks, "{bookmarks} ")}' +
+    '{ifeq(branch, "default", "", "{branch} ")}' +
+    '- {author|user}: "{desc|firstline}"')
+
+def _formatlabels(repo, fcd, fco, labels):
+    """Formats the given labels using the conflict marker template.
+
+    Returns a list of formatted labels.
+    """
+    cd = fcd.changectx()
+    co = fco.changectx()
+
+    ui = repo.ui
+    template = ui.config('merge', 'conflictmarkertemplate',
+        _defaultconflictmarker)
+    template = templater.parsestring(template, quoted=False)
+    tmpl = templater.templater(None, cache={ 'conflictmarker' : template })
+
+    pad = max(len(labels[0]), len(labels[1]))
+
+    return [
+        _formatconflictmarker(repo, cd, tmpl, labels[0], pad),
+        _formatconflictmarker(repo, co, tmpl, labels[1], pad),
+    ]
+
 def filemerge(repo, mynode, orig, fcd, fco, fca):
     """perform a 3-way merge in the working directory
 
@@ -327,8 +374,13 @@ 
     ui.debug("my %s other %s ancestor %s\n" % (fcd, fco, fca))
 
     labels = ['local', 'other']
+    if ui.configbool('merge', 'oldconflictmarkers', False):
+        formattedlabels = labels
+    else:
+        formattedlabels = _formatlabels(repo, fcd, fco, labels)
+
     needcheck, r = func(repo, mynode, orig, fcd, fco, fca, toolconf,
-                        (a, b, c, back), labels=labels)
+                        (a, b, c, back), labels=formattedlabels)
     if not needcheck:
         if r:
             if onfailure:
diff --git a/tests/test-commit-amend.t b/tests/test-commit-amend.t
--- a/tests/test-commit-amend.t
+++ b/tests/test-commit-amend.t
@@ -589,7 +589,7 @@ 
   no more unresolved files
   $ hg ci -m 'merge bar'
   $ hg log --config diff.git=1 -pr .
-  changeset:   23:d51446492733
+  changeset:   23:29ee7aa200c8
   tag:         tip
   parent:      22:30d96aeaf27b
   parent:      21:1aa437659d19
@@ -604,11 +604,11 @@ 
   --- a/cc
   +++ b/cc
   @@ -1,1 +1,5 @@
-  +<<<<<<< local
+  +<<<<<<< local: 30d96aeaf27b - test: "aa"
    dd
   +=======
   +cc
-  +>>>>>>> other
+  +>>>>>>> other: 1aa437659d19  bar - test: "aazzcc"
   diff --git a/z b/zz
   rename from z
   rename to zz
@@ -621,7 +621,7 @@ 
   cc not renamed
   $ hg ci --amend -m 'merge bar (amend message)'
   $ hg log --config diff.git=1 -pr .
-  changeset:   24:59de3dce7a79
+  changeset:   24:ba3eb3e8e8c2
   tag:         tip
   parent:      22:30d96aeaf27b
   parent:      21:1aa437659d19
@@ -636,11 +636,11 @@ 
   --- a/cc
   +++ b/cc
   @@ -1,1 +1,5 @@
-  +<<<<<<< local
+  +<<<<<<< local: 30d96aeaf27b - test: "aa"
    dd
   +=======
   +cc
-  +>>>>>>> other
+  +>>>>>>> other: 1aa437659d19  bar - test: "aazzcc"
   diff --git a/z b/zz
   rename from z
   rename to zz
@@ -654,7 +654,7 @@ 
   $ hg mv zz z
   $ hg ci --amend -m 'merge bar (undo rename)'
   $ hg log --config diff.git=1 -pr .
-  changeset:   26:7fb89c461f81
+  changeset:   26:0ce8747233f6
   tag:         tip
   parent:      22:30d96aeaf27b
   parent:      21:1aa437659d19
@@ -669,11 +669,11 @@ 
   --- a/cc
   +++ b/cc
   @@ -1,1 +1,5 @@
-  +<<<<<<< local
+  +<<<<<<< local: 30d96aeaf27b - test: "aa"
    dd
   +=======
   +cc
-  +>>>>>>> other
+  +>>>>>>> other: 1aa437659d19  bar - test: "aazzcc"
   
   $ hg debugrename z
   z not renamed
@@ -690,9 +690,9 @@ 
   $ echo aa >> aaa
   $ hg ci -m 'merge bar again'
   $ hg log --config diff.git=1 -pr .
-  changeset:   28:982d7a34ffee
+  changeset:   28:b8235574e741
   tag:         tip
-  parent:      26:7fb89c461f81
+  parent:      26:0ce8747233f6
   parent:      27:4c94d5bc65f5
   user:        test
   date:        Thu Jan 01 00:00:00 1970 +0000
@@ -725,9 +725,9 @@ 
   $ hg mv aaa aa
   $ hg ci --amend -m 'merge bar again (undo rename)'
   $ hg log --config diff.git=1 -pr .
-  changeset:   30:522688c0e71b
+  changeset:   30:dbafc132c18a
   tag:         tip
-  parent:      26:7fb89c461f81
+  parent:      26:0ce8747233f6
   parent:      27:4c94d5bc65f5
   user:        test
   date:        Thu Jan 01 00:00:00 1970 +0000
@@ -765,9 +765,9 @@ 
   use (c)hanged version or (d)elete? c
   $ hg ci -m 'merge bar (with conflicts)'
   $ hg log --config diff.git=1 -pr .
-  changeset:   33:5f9904c491b8
+  changeset:   33:8b0c83445ff5
   tag:         tip
-  parent:      32:01780b896f58
+  parent:      32:f60ace0fe178
   parent:      31:67db8847a540
   user:        test
   date:        Thu Jan 01 00:00:00 1970 +0000
@@ -777,9 +777,9 @@ 
   $ hg rm aa
   $ hg ci --amend -m 'merge bar (with conflicts, amended)'
   $ hg log --config diff.git=1 -pr .
-  changeset:   35:6ce0c89781a3
+  changeset:   35:f9b6726d8bd2
   tag:         tip
-  parent:      32:01780b896f58
+  parent:      32:f60ace0fe178
   parent:      31:67db8847a540
   user:        test
   date:        Thu Jan 01 00:00:00 1970 +0000
diff --git a/tests/test-conflict.t b/tests/test-conflict.t
--- a/tests/test-conflict.t
+++ b/tests/test-conflict.t
@@ -22,11 +22,11 @@ 
   32e80765d7fe+75234512624c+ tip
 
   $ cat a
-  <<<<<<< local
+  <<<<<<< local: 32e80765d7fe - test: "branch2"
   something else
   =======
   something
-  >>>>>>> other
+  >>>>>>> other: 75234512624c  - test: "branch1"
 
   $ hg status
   M a
diff --git a/tests/test-keyword.t b/tests/test-keyword.t
--- a/tests/test-keyword.t
+++ b/tests/test-keyword.t
@@ -1049,11 +1049,11 @@ 
   [1]
   $ cat m
   $Id$
-  <<<<<<< local
+  <<<<<<< local: 88a80c8d172e - test: "8bar"
   bar
   =======
   foo
-  >>>>>>> other
+  >>>>>>> other: 85d2d2d732a5  - test: "simplemerge"
 
 resolve to local
 
diff --git a/tests/test-merge-revert2.t b/tests/test-merge-revert2.t
--- a/tests/test-merge-revert2.t
+++ b/tests/test-merge-revert2.t
@@ -57,11 +57,11 @@ 
   @@ -1,3 +1,7 @@
    added file1
    another line of text
-  +<<<<<<< local
+  +<<<<<<< local: c3fa057dd86f  - test: "added file1 and file2"
   +changed file1 different
   +=======
    changed file1
-  +>>>>>>> other
+  +>>>>>>> other: dfab7f3c2efb - test: "changed file1"
 
   $ hg status
   M file1
diff --git a/tests/test-merge-tools.t b/tests/test-merge-tools.t
--- a/tests/test-merge-tools.t
+++ b/tests/test-merge-tools.t
@@ -66,11 +66,11 @@ 
   [1]
   $ aftermerge
   # cat f
-  <<<<<<< local
+  <<<<<<< local: ef83787e2614  - test: "revision 1"
   revision 1
   =======
   revision 2
-  >>>>>>> other
+  >>>>>>> other: 0185f4e0cf02  - test: "revision 2"
   space
   # hg stat
   M f
diff --git a/tests/test-merge-types.t b/tests/test-merge-types.t
--- a/tests/test-merge-types.t
+++ b/tests/test-merge-types.t
@@ -290,18 +290,18 @@ 
   U h
   $ tellmeabout a
   a is a plain file with content:
-  <<<<<<< local
+  <<<<<<< local: 0139c5610547 - test: "2"
   2
   =======
   1
-  >>>>>>> other
+  >>>>>>> other: 97e29675e796  - test: "1"
   $ tellmeabout b
   b is a plain file with content:
-  <<<<<<< local
+  <<<<<<< local: 0139c5610547 - test: "2"
   2
   =======
   1
-  >>>>>>> other
+  >>>>>>> other: 97e29675e796  - test: "1"
   $ tellmeabout c
   c is a plain file with content:
   x
@@ -345,18 +345,18 @@ 
   [1]
   $ tellmeabout a
   a is a plain file with content:
-  <<<<<<< local
+  <<<<<<< local: 97e29675e796  - test: "1"
   1
   =======
   2
-  >>>>>>> other
+  >>>>>>> other: 0139c5610547 - test: "2"
   $ tellmeabout b
   b is an executable file with content:
-  <<<<<<< local
+  <<<<<<< local: 97e29675e796  - test: "1"
   1
   =======
   2
-  >>>>>>> other
+  >>>>>>> other: 0139c5610547 - test: "2"
   $ tellmeabout c
   c is an executable file with content:
   x
diff --git a/tests/test-merge7.t b/tests/test-merge7.t
--- a/tests/test-merge7.t
+++ b/tests/test-merge7.t
@@ -98,11 +98,11 @@ 
 
   $ cat test.txt
   one
-  <<<<<<< local
+  <<<<<<< local: 50c3a7e29886  - test: "Merge 1"
   two-point-five
   =======
   two-point-one
-  >>>>>>> other
+  >>>>>>> other: 40d11a4173a8 - test: "two -> two-point-one"
   three
 
   $ hg debugindex test.txt
diff --git a/tests/test-shelve.t b/tests/test-shelve.t
--- a/tests/test-shelve.t
+++ b/tests/test-shelve.t
@@ -210,11 +210,11 @@ 
   +++ b/a/a
   @@ -1,2 +1,6 @@
    a
-  +<<<<<<< local
+  +<<<<<<< local: *  - shelve: "pending changes temporary commit" (glob)
    c
   +=======
   +a
-  +>>>>>>> other
+  +>>>>>>> other: * - shelve: "changes to '[mq]: second.patch'" (glob)
   diff --git a/b.rename/b b/b.rename/b
   new file mode 100644
   --- /dev/null
@@ -604,11 +604,11 @@ 
   M f
   ? f.orig
   $ cat f
-  <<<<<<< local
+  <<<<<<< local: 5f6b880e719b  - shelve: "pending changes temporary commit"
   g
   =======
   f
-  >>>>>>> other
+  >>>>>>> other: 23b29cada8ba - shelve: "changes to 'commit stuff'"
   $ cat f.orig
   g
   $ hg unshelve --abort
@@ -647,11 +647,11 @@ 
   M f
   ? f.orig
   $ cat f
-  <<<<<<< local
+  <<<<<<< local: 6b563750f973  - test: "intermediate other change"
   g
   =======
   f
-  >>>>>>> other
+  >>>>>>> other: 23b29cada8ba - shelve: "changes to 'commit stuff'"
   $ cat f.orig
   g
   $ hg unshelve --abort
diff --git a/tests/test-subrepo.t b/tests/test-subrepo.t
--- a/tests/test-subrepo.t
+++ b/tests/test-subrepo.t
@@ -298,11 +298,11 @@ 
 should conflict
 
   $ cat t/t
-  <<<<<<< local
+  <<<<<<< local: 20a0db6fbf6c - test: "10"
   conflict
   =======
   t3
-  >>>>>>> other
+  >>>>>>> other: 7af322bc1198  - test: "7"
 
 clone