Patchwork [2,of,4] trydiff: replace 'binarydiff' variable by 'binary' variable

login
register
mail settings
Submitter Martin von Zweigbergk
Date Feb. 6, 2015, midnight
Message ID <81d13573f5528ec86c90.1423180844@martinvonz.mtv.corp.google.com>
Download mbox | patch
Permalink /patch/7694/
State Accepted
Commit ae453d166d5148e9cc0936545c4f5260bf9f0973
Headers show

Comments

Martin von Zweigbergk - Feb. 6, 2015, midnight
# HG changeset patch
# User Martin von Zweigbergk <martinvonz@google.com>
# Date 1421989437 28800
#      Thu Jan 22 21:03:57 2015 -0800
# Node ID 81d13573f5528ec86c90ebb49a3b3424a07b8833
# Parent  68c7005db5d1268720f4a3806482fa5314af5283
trydiff: replace 'binarydiff' variable by 'binary' variable

It's not obvious, but every path in the 'if opts.git or losedatafn:'
block will have checked whether the file is binary [1]. Let's assign
the result of this check to a variable so we can simplify by checking
'binary and opts.git' in only one place instead of every place we
currently assign to 'binarydiff'.

 [1] Except when deleting an empty file, but checking whether an empty
     string is binary is very cheap anyway.

Patch

diff -r 68c7005db5d1 -r 81d13573f552 mercurial/patch.py
--- a/mercurial/patch.py	Fri Jan 16 15:09:21 2015 -0800
+++ b/mercurial/patch.py	Thu Jan 22 21:03:57 2015 -0800
@@ -1776,7 +1776,7 @@ 
         flag2 = None
         content1 = None
         content2 = None
-        binarydiff = False
+        binary = False
         copyop = None
         if f not in addedset:
             content1 = getfilectx(f, ctx1).data()
@@ -1801,11 +1801,9 @@ 
                 else:
                     if not opts.git and flag2:
                         losedatafn(f)
-                if util.binary(content1) or util.binary(content2):
-                    if opts.git:
-                        binarydiff = True
-                    else:
-                        losedatafn(f)
+                binary = util.binary(content1) or util.binary(content2)
+                if not opts.git and binary:
+                    losedatafn(f)
                 if not opts.git and not content2:
                     # regular diffs cannot represent new empty file
                     losedatafn(f)
@@ -1817,19 +1815,17 @@ 
                         continue
                     else:
                         flag1 = ctx1.flags(f)
-                        if util.binary(content1):
-                            binarydiff = True
-                elif not content1 or util.binary(content1):
-                    # regular diffs cannot represent empty file deletion
-                    losedatafn(f)
+                        binary = util.binary(content1)
+                else:
+                    binary = util.binary(content1)
+                    if not content1 or binary:
+                        # regular diffs cannot represent empty file deletion
+                        losedatafn(f)
             else:
                 flag1 = ctx1.flags(f)
                 flag2 = ctx2.flags(f)
                 binary = util.binary(content1) or util.binary(content2)
-                if opts.git:
-                    if binary:
-                        binarydiff = True
-                elif binary or flag2 != flag1:
+                if not opts.git and (binary or flag2 != flag1):
                     losedatafn(f)
 
         path1 = posixpath.join(prefix, f1)
@@ -1853,9 +1849,9 @@ 
         elif revs and not repo.ui.quiet:
             header.append(diffline(path1, revs))
 
-        if binarydiff and not opts.nobinary:
+        if binary and opts.git and not opts.nobinary:
             text = mdiff.b85diff(content1, content2)
-            if text and opts.git:
+            if text:
                 header.append('index %s..%s' %
                               (gitindex(content1), gitindex(content2)))
         else: