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

login
register
mail settings
Submitter Durham Goode
Date May 15, 2014, 8:52 p.m.
Message ID <10d7bf67218dc9b26a83.1400187158@dev2000.prn2.facebook.com>
Download mbox | patch
Permalink /patch/4757/
State Superseded
Commit 25d5a9ecbb855e090e1712297a3022587637f847
Headers show

Comments

Durham Goode - May 15, 2014, 8:52 p.m.
# HG changeset patch
# User Durham Goode <durham@fb.com>
# Date 1399593022 25200
#      Thu May 08 16:50:22 2014 -0700
# Node ID 10d7bf67218dc9b26a838925410d646310fd6ed9
# Parent  1ae3cd6f836c3c96ee3e9a872c8e966750910c2d
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.mergemarkers=basic.

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 16, 2014, 4:34 a.m.
On 05/15/2014 01:52 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 10d7bf67218dc9b26a838925410d646310fd6ed9
> # Parent  1ae3cd6f836c3c96ee3e9a872c8e966750910c2d
> 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.mergemarkers=basic.

We have no `merge` section. We have a ui, merge-patterns and merge-tools 
section.

This should go in "ui" (as mergemarkers), or in "merge-tools" (as 
internal:merge.mergemarkers)

I feel like "ui" would be a better fit.


This smoothly bring use to the next topic: You did not document those 
option in the help (otherwise we would have spotted that the section 
they belong to did not existed)

It is important that you update the documentation about those two new 
options. Sorry for not spotting that sooner.

> 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,58 @@
>           return True, r
>       return False, 0
>
> +def _formatconflictmarker(repo, ctx, template, label, pad):
> +    """Applies the given template to the ctx, prefixed by the label.
> +
> +    Pad is the minimum width of the label prefix, so that multiple markers
> +    can have aligned templated parts.
> +    """
> +    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
>
> @@ -326,9 +378,15 @@
>
>       ui.debug("my %s other %s ancestor %s\n" % (fcd, fco, fca))
>
> +    markerstyle = ui.configbool('merge', 'mergemarkers', 'detailed')

Ss you already had spotted configbool is not the function you are 
looking for. This highlight the lack of testing. But you have already 
seen all that. (pointing it to prevent another reviewer to see it)

>       labels = ['local', 'other']
> +    if markerstyle == 'basic':
> +        formattedlabels = labels
> +    else:

So if the value is unknown we used `detailed`. It make sense to have 
that as a default. Make sure the doc mention it.

> +        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:


Patches looks good to me otherwise

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,58 @@ 
         return True, r
     return False, 0
 
+def _formatconflictmarker(repo, ctx, template, label, pad):
+    """Applies the given template to the ctx, prefixed by the label.
+
+    Pad is the minimum width of the label prefix, so that multiple markers
+    can have aligned templated parts.
+    """
+    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
 
@@ -326,9 +378,15 @@ 
 
     ui.debug("my %s other %s ancestor %s\n" % (fcd, fco, fca))
 
+    markerstyle = ui.configbool('merge', 'mergemarkers', 'detailed')
     labels = ['local', 'other']
+    if markerstyle == 'basic':
+        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