Patchwork D12013: simplemerge: take arguments as annotated context objects

login
register
mail settings
Submitter phabricator
Date Jan. 21, 2022, 11:37 p.m.
Message ID <differential-rev-PHID-DREV-od4fo4sirozqczefnds4-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/50341/
State New
Headers show

Comments

phabricator - Jan. 21, 2022, 11:37 p.m.
martinvonz created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  The labels we put in conflict markers are formatted so the part before
  the ':' (typically says things like "local") is padded so the ':' is
  aligned among the labels. That means that if you specify a long label
  for "base" but the conflict marker style is "merge" (i.e. 2-way), the
  other two will have unwanted padding. We often don't specify a label
  for the base, so we don't notice the problem (and it may very well be
  that it didn't exist before my D11972 <https://phab.mercurial-scm.org/D11972>).
  
  I think the best fix is to pass the labels along with the context
  objects, so the low-level code that switches on the marker style to
  use (i.e. `simplemerge`) can do the formatting. This patch starts
  doing that by passing a fully-formatted label to `simplemerge`. A
  coming patch will move the formatting to `simplemerge`.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D12013

AFFECTED FILES
  contrib/simplemerge
  mercurial/filemerge.py
  mercurial/simplemerge.py

CHANGE DETAILS




To: martinvonz, #hg-reviewers
Cc: mercurial-patches, mercurial-devel

Patch

diff --git a/mercurial/simplemerge.py b/mercurial/simplemerge.py
--- a/mercurial/simplemerge.py
+++ b/mercurial/simplemerge.py
@@ -19,6 +19,7 @@ 
 from __future__ import absolute_import
 
 from .i18n import _
+from .thirdparty import attr
 from . import (
     error,
     mdiff,
@@ -284,13 +285,14 @@ 
     return text
 
 
-def _picklabels(overrides):
-    if len(overrides) > 3:
-        raise error.Abort(_(b"can only specify three labels."))
-    result = [None, None, None]
-    for i, override in enumerate(overrides):
-        result[i] = override
-    return result
+def _format_labels(*inputs):
+    labels = []
+    for input in inputs:
+        if input.label:
+            labels.append(input.label)
+        else:
+            labels.append(None)
+    return labels
 
 
 def _detect_newline(m3):
@@ -462,7 +464,13 @@ 
     return lines
 
 
-def simplemerge(ui, localctx, basectx, otherctx, **opts):
+@attr.s
+class MergeInput(object):
+    fctx = attr.ib()
+    label = attr.ib(default=None)
+
+
+def simplemerge(ui, local, base, other, **opts):
     """Performs the simplemerge algorithm.
 
     The merged result is written into `localctx`.
@@ -479,9 +487,9 @@ 
         return _verifytext(ctx.decodeddata(), ctx.path(), ui, opts)
 
     try:
-        localtext = readctx(localctx)
-        basetext = readctx(basectx)
-        othertext = readctx(otherctx)
+        localtext = readctx(local.fctx)
+        basetext = readctx(base.fctx)
+        othertext = readctx(other.fctx)
     except error.Abort:
         return True
 
@@ -495,13 +503,15 @@ 
     elif mode == b'other':
         lines = _resolve(m3, (2,))
     else:
-        name_a, name_b, name_base = _picklabels(opts.get('label', []))
         if mode == b'mergediff':
-            lines, conflicts = render_mergediff(m3, name_a, name_b, name_base)
+            labels = _format_labels(local, other, base)
+            lines, conflicts = render_mergediff(m3, *labels)
         elif mode == b'merge3':
-            lines, conflicts = render_merge3(m3, name_a, name_b, name_base)
+            labels = _format_labels(local, other, base)
+            lines, conflicts = render_merge3(m3, *labels)
         else:
-            lines, conflicts = render_minimized(m3, name_a, name_b)
+            labels = _format_labels(local, other)
+            lines, conflicts = render_minimized(m3, *labels)
 
     mergedtext = b''.join(lines)
     if opts.get('print'):
@@ -509,6 +519,6 @@ 
     else:
         # localctx.flags() already has the merged flags (done in
         # mergestate.resolve())
-        localctx.write(mergedtext, localctx.flags())
+        local.fctx.write(mergedtext, local.fctx.flags())
 
     return conflicts
diff --git a/mercurial/filemerge.py b/mercurial/filemerge.py
--- a/mercurial/filemerge.py
+++ b/mercurial/filemerge.py
@@ -428,8 +428,11 @@ 
             mode = b'mergediff'
         elif premerge == b'keep-merge3':
             mode = b'merge3'
+        local = simplemerge.MergeInput(fcd, labels[0])
+        other = simplemerge.MergeInput(fco, labels[1])
+        base = simplemerge.MergeInput(fca, labels[2])
         r = simplemerge.simplemerge(
-            ui, fcd, fca, fco, quiet=True, label=labels, mode=mode
+            ui, local, base, other, quiet=True, mode=mode
         )
         if not r:
             ui.debug(b" premerge successful\n")
@@ -469,7 +472,16 @@ 
     of merge, unless mode equals 'union' which suppresses the markers."""
     ui = repo.ui
 
-    r = simplemerge.simplemerge(ui, fcd, fca, fco, label=labels, mode=mode)
+    local = simplemerge.MergeInput(fcd)
+    if len(labels) > 0:
+        local.label = labels[0]
+    other = simplemerge.MergeInput(fco)
+    if len(labels) > 1:
+        other.label = labels[1]
+    base = simplemerge.MergeInput(fca)
+    if len(labels) > 2:
+        base.label = labels[2]
+    r = simplemerge.simplemerge(ui, local, base, other, mode=mode)
     return True, r, False
 
 
diff --git a/contrib/simplemerge b/contrib/simplemerge
--- a/contrib/simplemerge
+++ b/contrib/simplemerge
@@ -84,15 +84,25 @@ 
         opts[b'mode'] = b'merge3'
     local, base, other = args
     overrides = opts[b'label']
+    if len(overrides) > 3:
+        raise error.InputError(b'can only specify three labels.')
     labels = [local, other, base]
     labels[: len(overrides)] = overrides
-    opts[b'label'] = labels
+    local_input = simplemerge.MergeInput(
+        context.arbitraryfilectx(local), labels[0]
+    )
+    other_input = simplemerge.MergeInput(
+        context.arbitraryfilectx(other), labels[1]
+    )
+    base_input = simplemerge.MergeInput(
+        context.arbitraryfilectx(base), labels[2]
+    )
     sys.exit(
         simplemerge.simplemerge(
             uimod.ui.load(),
-            context.arbitraryfilectx(local),
-            context.arbitraryfilectx(base),
-            context.arbitraryfilectx(other),
+            local_input,
+            base_input,
+            other_input,
             **pycompat.strkwargs(opts)
         )
     )