Patchwork D11877: filemerge: stop passing around 3 unused `None` values in `files` argument

login
register
mail settings
Submitter phabricator
Date Dec. 7, 2021, 5:03 p.m.
Message ID <differential-rev-PHID-DREV-7ge6r56xysxw2dxllthb-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/50203/
State New
Headers show

Comments

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

REVISION SUMMARY
  Since 67cfffbfb6a0 <https://phab.mercurial-scm.org/rHG67cfffbfb6a0b9c798c8d245091e15aa3025bfa5> (filemerge: eliminate most uses of tempfiles,
  2017-08-31), we have used only the last entry in the `files` tuple
  that gets passed around to various functions. That commit said
  "Emphasize that they're unused so we can more easily remove them
  later.". It's now "later".

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  mercurial/filemerge.py

CHANGE DETAILS




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

Patch

diff --git a/mercurial/filemerge.py b/mercurial/filemerge.py
--- a/mercurial/filemerge.py
+++ b/mercurial/filemerge.py
@@ -399,11 +399,10 @@ 
         return filectx
 
 
-def _premerge(repo, fcd, fco, fca, toolconf, files, labels=None):
+def _premerge(repo, fcd, fco, fca, toolconf, backup, labels=None):
     tool, toolpath, binary, symlink, scriptfn = toolconf
     if symlink or fcd.isabsent() or fco.isabsent():
         return 1
-    unused, unused, unused, backup = files
 
     ui = repo.ui
 
@@ -463,7 +462,7 @@ 
     return True
 
 
-def _merge(repo, mynode, orig, fcd, fco, fca, toolconf, files, labels, mode):
+def _merge(repo, mynode, orig, fcd, fco, fca, toolconf, backup, labels, mode):
     """
     Uses the internal non-interactive simple merge algorithm for merging
     files. It will fail if there are any conflicts and leave markers in
@@ -484,13 +483,13 @@ 
     ),
     precheck=_mergecheck,
 )
-def _iunion(repo, mynode, orig, fcd, fco, fca, toolconf, files, labels=None):
+def _iunion(repo, mynode, orig, fcd, fco, fca, toolconf, backup, labels=None):
     """
     Uses the internal non-interactive simple merge algorithm for merging
     files. It will use both left and right sides for conflict regions.
     No markers are inserted."""
     return _merge(
-        repo, mynode, orig, fcd, fco, fca, toolconf, files, labels, b'union'
+        repo, mynode, orig, fcd, fco, fca, toolconf, backup, labels, b'union'
     )
 
 
@@ -503,14 +502,14 @@ 
     ),
     precheck=_mergecheck,
 )
-def _imerge(repo, mynode, orig, fcd, fco, fca, toolconf, files, labels=None):
+def _imerge(repo, mynode, orig, fcd, fco, fca, toolconf, backup, labels=None):
     """
     Uses the internal non-interactive simple merge algorithm for merging
     files. It will fail if there are any conflicts and leave markers in
     the partially merged file. Markers will have two sections, one for each side
     of merge."""
     return _merge(
-        repo, mynode, orig, fcd, fco, fca, toolconf, files, labels, b'merge'
+        repo, mynode, orig, fcd, fco, fca, toolconf, backup, labels, b'merge'
     )
 
 
@@ -523,7 +522,7 @@ 
     ),
     precheck=_mergecheck,
 )
-def _imerge3(repo, mynode, orig, fcd, fco, fca, toolconf, files, labels=None):
+def _imerge3(repo, mynode, orig, fcd, fco, fca, toolconf, backup, labels=None):
     """
     Uses the internal non-interactive simple merge algorithm for merging
     files. It will fail if there are any conflicts and leave markers in
@@ -533,7 +532,7 @@ 
         labels = _defaultconflictlabels
     if len(labels) < 3:
         labels.append(b'base')
-    return _imerge(repo, mynode, orig, fcd, fco, fca, toolconf, files, labels)
+    return _imerge(repo, mynode, orig, fcd, fco, fca, toolconf, backup, labels)
 
 
 @internaltool(
@@ -565,7 +564,7 @@ 
     precheck=_mergecheck,
 )
 def _imerge_diff(
-    repo, mynode, orig, fcd, fco, fca, toolconf, files, labels=None
+    repo, mynode, orig, fcd, fco, fca, toolconf, backup, labels=None
 ):
     """
     Uses the internal non-interactive simple merge algorithm for merging
@@ -578,7 +577,16 @@ 
     if len(labels) < 3:
         labels.append(b'base')
     return _merge(
-        repo, mynode, orig, fcd, fco, fca, toolconf, files, labels, b'mergediff'
+        repo,
+        mynode,
+        orig,
+        fcd,
+        fco,
+        fca,
+        toolconf,
+        backup,
+        labels,
+        b'mergediff',
     )
 
 
@@ -590,7 +598,7 @@ 
     fco,
     fca,
     toolconf,
-    files,
+    backup,
     labels=None,
     localorother=None,
 ):
@@ -631,7 +639,9 @@ 
         b"tool of your choice)\n"
     ),
 )
-def _itagmerge(repo, mynode, orig, fcd, fco, fca, toolconf, files, labels=None):
+def _itagmerge(
+    repo, mynode, orig, fcd, fco, fca, toolconf, backup, labels=None
+):
     """
     Uses the internal tag merge algorithm (experimental).
     """
@@ -640,7 +650,7 @@ 
 
 
 @internaltool(b'dump', fullmerge, binary=True, symlink=True)
-def _idump(repo, mynode, orig, fcd, fco, fca, toolconf, files, labels=None):
+def _idump(repo, mynode, orig, fcd, fco, fca, toolconf, backup, labels=None):
     """
     Creates three versions of the files to merge, containing the
     contents of local, other and base. These files can then be used to
@@ -669,16 +679,20 @@ 
 
 
 @internaltool(b'forcedump', mergeonly, binary=True, symlink=True)
-def _forcedump(repo, mynode, orig, fcd, fco, fca, toolconf, files, labels=None):
+def _forcedump(
+    repo, mynode, orig, fcd, fco, fca, toolconf, backup, labels=None
+):
     """
     Creates three versions of the files as same as :dump, but omits premerge.
     """
     return _idump(
-        repo, mynode, orig, fcd, fco, fca, toolconf, files, labels=labels
+        repo, mynode, orig, fcd, fco, fca, toolconf, backup, labels=labels
     )
 
 
-def _xmergeimm(repo, mynode, orig, fcd, fco, fca, toolconf, files, labels=None):
+def _xmergeimm(
+    repo, mynode, orig, fcd, fco, fca, toolconf, backup, labels=None
+):
     # In-memory merge simply raises an exception on all external merge tools,
     # for now.
     #
@@ -746,7 +760,7 @@ 
     ui.status(t.renderdefault(props))
 
 
-def _xmerge(repo, mynode, orig, fcd, fco, fca, toolconf, files, labels):
+def _xmerge(repo, mynode, orig, fcd, fco, fca, toolconf, backup, labels):
     tool, toolpath, binary, symlink, scriptfn = toolconf
     uipathfn = scmutil.getuipathfn(repo)
     if fcd.isabsent() or fco.isabsent():
@@ -755,7 +769,6 @@ 
             % (tool, uipathfn(fcd.path()))
         )
         return False, 1, None
-    unused, unused, unused, backup = files
     localpath = _workingpath(repo, fcd)
     args = _toolstr(repo.ui, tool, b"args")
 
@@ -1120,7 +1133,6 @@ 
         return True, 1, False
 
     backup = _makebackup(repo, ui, wctx, fcd)
-    files = (None, None, None, backup)
     r = 1
     try:
         internalmarkerstyle = ui.config(b'ui', b'mergemarkers')
@@ -1155,7 +1167,7 @@ 
                 )
 
             r = _premerge(
-                repo, fcd, fco, fca, toolconf, files, labels=premergelabels
+                repo, fcd, fco, fca, toolconf, backup, labels=premergelabels
             )
             # we're done if premerge was successful (r is 0)
             if not r:
@@ -1169,12 +1181,12 @@ 
             fco,
             fca,
             toolconf,
-            files,
+            backup,
             labels=formattedlabels,
         )
 
         if needcheck:
-            r = _check(repo, r, ui, tool, fcd, files)
+            r = _check(repo, r, ui, tool, fcd, backup)
 
         if r:
             if onfailure:
@@ -1223,10 +1235,9 @@ 
     )
 
 
-def _check(repo, r, ui, tool, fcd, files):
+def _check(repo, r, ui, tool, fcd, backup):
     fd = fcd.path()
     uipathfn = scmutil.getuipathfn(repo)
-    unused, unused, unused, backup = files
 
     if not r and (
         _toolbool(ui, tool, b"checkconflicts")