Patchwork [2,of,5,V2] extdiff: refactor logic to diff revs of versions of files

login
register
mail settings
Submitter Pulkit Goyal
Date July 16, 2020, 9:56 a.m.
Message ID <5a5891c75cc4646e4e6e.1594893407@workspace>
Download mbox | patch
Permalink /patch/46761/
State Accepted
Headers show

Comments

Pulkit Goyal - July 16, 2020, 9:56 a.m.
# HG changeset patch
# User Pulkit Goyal <7895pulkit@gmail.com>
# Date 1594107798 -19800
#      Tue Jul 07 13:13:18 2020 +0530
# Node ID 5a5891c75cc4646e4e6ea10ed4e9c3aab7e9a049
# Parent  03b3d29d310bd6debbd0aad4a8f2812f467829fe
# EXP-Topic diff-refactor
extdiff: refactor logic to diff revs of versions of files

Now that code for both cases, diffing patches or files is in separate function,
it will be better to refactor them more and understand.

Differential Revision: https://phab.mercurial-scm.org/D8687

Patch

diff --git a/hgext/extdiff.py b/hgext/extdiff.py
--- a/hgext/extdiff.py
+++ b/hgext/extdiff.py
@@ -382,6 +382,145 @@  def diffpatch(ui, repo, node1a, node2, t
     return 1
 
 
+def diffrevs(
+    ui,
+    repo,
+    node1a,
+    node1b,
+    node2,
+    matcher,
+    tmproot,
+    cmdline,
+    do3way,
+    guitool,
+    opts,
+):
+
+    subrepos = opts.get(b'subrepos')
+    st = repo.status(node1a, node2, matcher, listsubrepos=subrepos)
+    mod_a, add_a, rem_a = set(st.modified), set(st.added), set(st.removed)
+    if do3way:
+        stb = repo.status(node1b, node2, matcher, listsubrepos=subrepos)
+        mod_b, add_b, rem_b = (
+            set(stb.modified),
+            set(stb.added),
+            set(stb.removed),
+        )
+    else:
+        mod_b, add_b, rem_b = set(), set(), set()
+    modadd = mod_a | add_a | mod_b | add_b
+    common = modadd | rem_a | rem_b
+    if not common:
+        return 0
+    # Always make a copy of node1a (and node1b, if applicable)
+    dir1a_files = mod_a | rem_a | ((mod_b | add_b) - add_a)
+    dir1a = snapshot(ui, repo, dir1a_files, node1a, tmproot, subrepos)[0]
+    rev1a = b'@%d' % repo[node1a].rev()
+    if do3way:
+        dir1b_files = mod_b | rem_b | ((mod_a | add_a) - add_b)
+        dir1b = snapshot(ui, repo, dir1b_files, node1b, tmproot, subrepos)[0]
+        rev1b = b'@%d' % repo[node1b].rev()
+    else:
+        dir1b = None
+        rev1b = b''
+
+    fnsandstat = []
+
+    # If node2 in not the wc or there is >1 change, copy it
+    dir2root = b''
+    rev2 = b''
+    if node2:
+        dir2 = snapshot(ui, repo, modadd, node2, tmproot, subrepos)[0]
+        rev2 = b'@%d' % repo[node2].rev()
+    elif len(common) > 1:
+        # we only actually need to get the files to copy back to
+        # the working dir in this case (because the other cases
+        # are: diffing 2 revisions or single file -- in which case
+        # the file is already directly passed to the diff tool).
+        dir2, fnsandstat = snapshot(ui, repo, modadd, None, tmproot, subrepos)
+    else:
+        # This lets the diff tool open the changed file directly
+        dir2 = b''
+        dir2root = repo.root
+
+    label1a = rev1a
+    label1b = rev1b
+    label2 = rev2
+
+    # If only one change, diff the files instead of the directories
+    # Handle bogus modifies correctly by checking if the files exist
+    if len(common) == 1:
+        common_file = util.localpath(common.pop())
+        dir1a = os.path.join(tmproot, dir1a, common_file)
+        label1a = common_file + rev1a
+        if not os.path.isfile(dir1a):
+            dir1a = pycompat.osdevnull
+        if do3way:
+            dir1b = os.path.join(tmproot, dir1b, common_file)
+            label1b = common_file + rev1b
+            if not os.path.isfile(dir1b):
+                dir1b = pycompat.osdevnull
+        dir2 = os.path.join(dir2root, dir2, common_file)
+        label2 = common_file + rev2
+
+    if not opts.get(b'per_file'):
+        # Run the external tool on the 2 temp directories or the patches
+        cmdline = formatcmdline(
+            cmdline,
+            repo.root,
+            do3way=do3way,
+            parent1=dir1a,
+            plabel1=label1a,
+            parent2=dir1b,
+            plabel2=label1b,
+            child=dir2,
+            clabel=label2,
+        )
+        ui.debug(b'running %r in %s\n' % (pycompat.bytestr(cmdline), tmproot))
+        ui.system(cmdline, cwd=tmproot, blockedtag=b'extdiff')
+    else:
+        # Run the external tool once for each pair of files
+        _runperfilediff(
+            cmdline,
+            repo.root,
+            ui,
+            guitool=guitool,
+            do3way=do3way,
+            confirm=opts.get(b'confirm'),
+            commonfiles=common,
+            tmproot=tmproot,
+            dir1a=dir1a,
+            dir1b=dir1b,
+            dir2root=dir2root,
+            dir2=dir2,
+            rev1a=rev1a,
+            rev1b=rev1b,
+            rev2=rev2,
+        )
+
+    for copy_fn, working_fn, st in fnsandstat:
+        cpstat = os.lstat(copy_fn)
+        # Some tools copy the file and attributes, so mtime may not detect
+        # all changes.  A size check will detect more cases, but not all.
+        # The only certain way to detect every case is to diff all files,
+        # which could be expensive.
+        # copyfile() carries over the permission, so the mode check could
+        # be in an 'elif' branch, but for the case where the file has
+        # changed without affecting mtime or size.
+        if (
+            cpstat[stat.ST_MTIME] != st[stat.ST_MTIME]
+            or cpstat.st_size != st.st_size
+            or (cpstat.st_mode & 0o100) != (st.st_mode & 0o100)
+        ):
+            ui.debug(
+                b'file changed while diffing. '
+                b'Overwriting: %s (src: %s)\n' % (working_fn, copy_fn)
+            )
+            util.copyfile(copy_fn, working_fn)
+
+    return 1
+
+
 def dodiff(ui, repo, cmdline, pats, opts, guitool=False):
     '''Do the actual diff:
 
@@ -406,9 +545,6 @@  def dodiff(ui, repo, cmdline, pats, opts
         else:
             ctx1b = repo[nullid]
 
-    perfile = opts.get(b'per_file')
-    confirm = opts.get(b'confirm')
-
     node1a = ctx1a.node()
     node1b = ctx1b.node()
     node2 = ctx2.node()
@@ -418,33 +554,15 @@  def dodiff(ui, repo, cmdline, pats, opts
         if node1b == nullid:
             do3way = False
 
-    subrepos = opts.get(b'subrepos')
-
     matcher = scmutil.match(repo[node2], pats, opts)
 
     if opts.get(b'patch'):
-        if subrepos:
+        if opts.get(b'subrepos'):
             raise error.Abort(_(b'--patch cannot be used with --subrepos'))
-        if perfile:
+        if opts.get(b'per_file'):
             raise error.Abort(_(b'--patch cannot be used with --per-file'))
         if node2 is None:
             raise error.Abort(_(b'--patch requires two revisions'))
-    else:
-        st = repo.status(node1a, node2, matcher, listsubrepos=subrepos)
-        mod_a, add_a, rem_a = set(st.modified), set(st.added), set(st.removed)
-        if do3way:
-            stb = repo.status(node1b, node2, matcher, listsubrepos=subrepos)
-            mod_b, add_b, rem_b = (
-                set(stb.modified),
-                set(stb.added),
-                set(stb.removed),
-            )
-        else:
-            mod_b, add_b, rem_b = set(), set(), set()
-        modadd = mod_a | add_a | mod_b | add_b
-        common = modadd | rem_a | rem_b
-        if not common:
-            return 0
 
     tmproot = pycompat.mkdtemp(prefix=b'extdiff.')
     try:
@@ -453,119 +571,20 @@  def dodiff(ui, repo, cmdline, pats, opts
                 ui, repo, node1a, node2, tmproot, matcher, cmdline, do3way
             )
 
-        # Always make a copy of node1a (and node1b, if applicable)
-        dir1a_files = mod_a | rem_a | ((mod_b | add_b) - add_a)
-        dir1a = snapshot(ui, repo, dir1a_files, node1a, tmproot, subrepos)[0]
-        rev1a = b'@%d' % repo[node1a].rev()
-        if do3way:
-            dir1b_files = mod_b | rem_b | ((mod_a | add_a) - add_b)
-            dir1b = snapshot(ui, repo, dir1b_files, node1b, tmproot, subrepos)[
-                0
-            ]
-            rev1b = b'@%d' % repo[node1b].rev()
-        else:
-            dir1b = None
-            rev1b = b''
-
-        fnsandstat = []
-
-        # If node2 in not the wc or there is >1 change, copy it
-        dir2root = b''
-        rev2 = b''
-        if node2:
-            dir2 = snapshot(ui, repo, modadd, node2, tmproot, subrepos)[0]
-            rev2 = b'@%d' % repo[node2].rev()
-        elif len(common) > 1:
-            # we only actually need to get the files to copy back to
-            # the working dir in this case (because the other cases
-            # are: diffing 2 revisions or single file -- in which case
-            # the file is already directly passed to the diff tool).
-            dir2, fnsandstat = snapshot(
-                ui, repo, modadd, None, tmproot, subrepos
-            )
-        else:
-            # This lets the diff tool open the changed file directly
-            dir2 = b''
-            dir2root = repo.root
-
-        label1a = rev1a
-        label1b = rev1b
-        label2 = rev2
-
-        # If only one change, diff the files instead of the directories
-        # Handle bogus modifies correctly by checking if the files exist
-        if len(common) == 1:
-            common_file = util.localpath(common.pop())
-            dir1a = os.path.join(tmproot, dir1a, common_file)
-            label1a = common_file + rev1a
-            if not os.path.isfile(dir1a):
-                dir1a = pycompat.osdevnull
-            if do3way:
-                dir1b = os.path.join(tmproot, dir1b, common_file)
-                label1b = common_file + rev1b
-                if not os.path.isfile(dir1b):
-                    dir1b = pycompat.osdevnull
-            dir2 = os.path.join(dir2root, dir2, common_file)
-            label2 = common_file + rev2
+        return diffrevs(
+            ui,
+            repo,
+            node1a,
+            node1b,
+            node2,
+            matcher,
+            tmproot,
+            cmdline,
+            do3way,
+            guitool,
+            opts,
+        )
 
-        if not perfile:
-            # Run the external tool on the 2 temp directories or the patches
-            cmdline = formatcmdline(
-                cmdline,
-                repo.root,
-                do3way=do3way,
-                parent1=dir1a,
-                plabel1=label1a,
-                parent2=dir1b,
-                plabel2=label1b,
-                child=dir2,
-                clabel=label2,
-            )
-            ui.debug(
-                b'running %r in %s\n' % (pycompat.bytestr(cmdline), tmproot)
-            )
-            ui.system(cmdline, cwd=tmproot, blockedtag=b'extdiff')
-        else:
-            # Run the external tool once for each pair of files
-            _runperfilediff(
-                cmdline,
-                repo.root,
-                ui,
-                guitool=guitool,
-                do3way=do3way,
-                confirm=confirm,
-                commonfiles=common,
-                tmproot=tmproot,
-                dir1a=dir1a,
-                dir1b=dir1b,
-                dir2root=dir2root,
-                dir2=dir2,
-                rev1a=rev1a,
-                rev1b=rev1b,
-                rev2=rev2,
-            )
-
-        for copy_fn, working_fn, st in fnsandstat:
-            cpstat = os.lstat(copy_fn)
-            # Some tools copy the file and attributes, so mtime may not detect
-            # all changes.  A size check will detect more cases, but not all.
-            # The only certain way to detect every case is to diff all files,
-            # which could be expensive.
-            # copyfile() carries over the permission, so the mode check could
-            # be in an 'elif' branch, but for the case where the file has
-            # changed without affecting mtime or size.
-            if (
-                cpstat[stat.ST_MTIME] != st[stat.ST_MTIME]
-                or cpstat.st_size != st.st_size
-                or (cpstat.st_mode & 0o100) != (st.st_mode & 0o100)
-            ):
-                ui.debug(
-                    b'file changed while diffing. '
-                    b'Overwriting: %s (src: %s)\n' % (working_fn, copy_fn)
-                )
-                util.copyfile(copy_fn, working_fn)
-
-        return 1
     finally:
         ui.note(_(b'cleaning up temp directory\n'))
         shutil.rmtree(tmproot)