Patchwork D11887: simplemerge: make `localorother` a "mode" instead of a separate thing

login
register
mail settings
Submitter phabricator
Date Dec. 8, 2021, 5:56 a.m.
Message ID <differential-rev-PHID-DREV-wqc3kuxe2tb5bgwtbpeb-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/50212/
State New
Headers show

Comments

phabricator - Dec. 8, 2021, 5:56 a.m.
martinvonz created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  `simplemerge()` takes a `mode` argument, which can be "union", "merge"
  or "mergediff", and a `localorother` argument, which can be `None`,
  "local", or "other". The two options are not at all orthogonal -- most
  combinations don't make sense. Also, at least "union", "local", and
  "other" are very closely related. Therefore, it makes sense to combine
  them into one.
  
  It probably makes sense to split the `mode` argument into `resolve`
  and `marker_style`, where the former can be `None`, "union", "local",
  or "other", and the latter can be "merge", "merge3", "mergediff", or
  "minimize". This is a good step in that direction whether or not we
  end up doing that.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  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
@@ -516,13 +516,17 @@ 
 
     m3 = Merge3Text(basetext, localtext, othertext)
     extrakwargs = {
-        "localorother": opts.get("localorother", None),
+        "localorother": None,
         'minimize': True,
     }
     if mode == b'union':
         extrakwargs['start_marker'] = None
         extrakwargs['mid_marker'] = None
         extrakwargs['end_marker'] = None
+    elif mode == b'local':
+        extrakwargs['localorother'] = b'local'
+    elif mode == b'other':
+        extrakwargs['localorother'] = b'other'
     elif name_base is not None:
         extrakwargs['base_marker'] = b'|||||||'
         extrakwargs['name_base'] = name_base
diff --git a/mercurial/filemerge.py b/mercurial/filemerge.py
--- a/mercurial/filemerge.py
+++ b/mercurial/filemerge.py
@@ -579,43 +579,28 @@ 
     )
 
 
-def _imergeauto(
-    repo,
-    mynode,
-    fcd,
-    fco,
-    fca,
-    toolconf,
-    backup,
-    labels=None,
-    localorother=None,
+@internaltool(b'merge-local', mergeonly, precheck=_mergecheck)
+def _imergelocal(
+    repo, mynode, fcd, fco, fca, toolconf, backup, labels=None
 ):
     """
-    Generic driver for _imergelocal and _imergeother
-    """
-    assert localorother is not None
-    r = simplemerge.simplemerge(
-        repo.ui, fcd, fca, fco, label=labels, localorother=localorother
-    )
-    return True, r
-
-
-@internaltool(b'merge-local', mergeonly, precheck=_mergecheck)
-def _imergelocal(*args, **kwargs):
-    """
     Like :merge, but resolve all conflicts non-interactively in favor
     of the local `p1()` changes."""
-    success, status = _imergeauto(localorother=b'local', *args, **kwargs)
-    return success, status, False
+    return _merge(
+        repo, mynode, fcd, fco, fca, toolconf, backup, labels, b'local'
+    )
 
 
 @internaltool(b'merge-other', mergeonly, precheck=_mergecheck)
-def _imergeother(*args, **kwargs):
+def _imergeother(
+    repo, mynode, fcd, fco, fca, toolconf, backup, labels=None
+):
     """
     Like :merge, but resolve all conflicts non-interactively in favor
     of the other `p2()` changes."""
-    success, status = _imergeauto(localorother=b'other', *args, **kwargs)
-    return success, status, False
+    return _merge(
+        repo, mynode, fcd, fco, fca, toolconf, backup, labels, b'other'
+    )
 
 
 @internaltool(