Lazily loading pull request diffs

Author: danvk

webdiff/argparser.py

@@ -96,5 +96,4 @@ def diff_for_args(args):
 
     if 'github' in args:
         gh = args['github']
-        a_dir, b_dir = github_fetcher.fetch_pull_request(gh['owner'], gh['repo'], gh['num'])
-        return dirdiff.diff(a_dir, b_dir)
+        return github_fetcher.fetch_pull_request(gh['owner'], gh['repo'], gh['num'])

webdiff/diff.py

@@ -24,7 +24,7 @@ def get_thick_dict(diff):
     d = get_thin_dict(diff)
     d.update({
         'is_image_diff': is_image_diff(diff),
-        'no_changes': diff.no_changes()
+        'no_changes': no_changes(diff)
     })
     if d['is_image_diff']:
         if d['a']: d['image_a'] = util.image_metadata(diff.a_path)
@@ -50,6 +50,12 @@ def get_thin_list(diffs, thick_idx=None):
     return ds
 
 
+def no_changes(diff):
+    if diff.a_path and diff.b_path:
+        return util.are_files_identical(diff.a_path, diff.b_path)
+    return False
+
+
 def is_image_diff(diff):
     '''Determine whether this diff is appropriate for image diff UI.
 

webdiff/github_fetcher.py

@@ -3,6 +3,9 @@
 This will create symlinks or clone git repos as needed.
 """
 
+# Use this PR for testing to see all four types of change at once:
+# https://github.com/danvk/test-repo/pull/2/
+
 import atexit
 from collections import OrderedDict
 import errno
@@ -14,19 +17,68 @@
 
 from github import Github, UnknownObjectException
 
+from util import memoize
+
+
+class GitHubDiff(object):
+    def __init__(self, pr, github_file):
+        self._pr = pr
+        self._file = github_file
+        self.type = {
+                'modified': 'change',
+                'renamed': 'move',
+                'added': 'add',
+                'removed': 'delete'
+                }[github_file.status]
+        self._a_path = None
+        self._b_path = None
+
+    @property
+    def a(self):
+        if self.type == 'move':
+            return self._file.raw_data['previous_filename']
+        elif self.type == 'add':
+            return None
+        else:
+            return self._file.filename
 
-temp_dirs = []
+    @property
+    def b(self):
+        if self.type == 'delete':
+            return None
+        else:
+            return self._file.filename
 
-_github_memo = None
-def _github():
-    global _github_memo
-    if _github_memo: return _github_memo
+    # NB: these are @memoize'd via fetch()
+    @property
+    def a_path(self):
+        return fetch(self._pr.base.repo, self.a, self._pr.base.sha)
+
+    @property
+    def b_path(self):
+        return fetch(self._pr.head.repo, self.b, self._pr.head.sha)
+
+    def __repr__(self):
+        return '%s (%s)' % (self.a or self.b, self.type)
+
+    # TOOD: diffstats are accessible via file.{changes,additions,deletions}
+
+
+@memoize
+def fetch(repo, filename, sha):
+    if filename is None: return None
+    data = repo.get_file_contents(filename, sha).decoded_content
+    _, ext = os.path.splitext(filename)
+    fd, path = tempfile.mkstemp(suffix=ext)
+    open(path, 'wb').write(data)
+    return path
 
+
+@memoize
+def _github():
     def simple_fallback(message=None):
-        global _github_memo
         if message: sys.stderr.write(message + '\n')
-        _github_memo = Github()
-        return _github_memo
+        return Github()
 
     github_rc = os.path.join(os.path.expanduser('~'), '.githubrc')
     if os.path.exists(github_rc):
@@ -45,53 +97,20 @@ def simple_fallback(message=None):
                 return simple_fallback('.githubrc missing user.login. Using anonymous API access.')
             if not kvs.get('user.password'):
                 return simple_fallback('.githubrc missing user.password. Using anonymous API access.')
-            _github_memo = Github(kvs['user.login'], kvs['user.password'])
+            return Github(kvs['user.login'], kvs['user.password'])
     else:
-        _github_memo = Github()
-    return _github_memo
-
-
-def put_in_dir(dirname, filename, contents):
-    """Puts contents into filename in dirname.
-    
-    This creates intermediate directories as needed, e.g. if filename is a/b/c.
-    """
-    d = os.path.join(dirname, os.path.dirname(filename))
-    try:
-        os.makedirs(d)
-    except OSError, e:
-        # be happy if someone already created the path
-        if e.errno != errno.EEXIST:
-            raise
-    open(os.path.join(dirname, filename), 'w').write(contents)
+        return Github()
 
 
 def fetch_pull_request(owner, repo, num):
-    """Pull down the pull request into two local directories.
-
-    Returns before_dir, after_dir.
-    """
-    sys.stderr.write('Loading pull request %s/%s#%s from github...\n' % (owner, repo, num))
+    '''Return a list of Diff objects for a pull request.'''
+    sys.stderr.write('Loading pull request %s/%s#%s from github...\n' % (
+            owner, repo, num))
     g = _github()
     pr = g.get_user(owner).get_repo(repo).get_pull(num)
-    base_repo = pr.base.repo
-    head_repo = pr.head.repo
-    files = list(pr.get_files())
-
-    a_dir, b_dir = tempfile.mkdtemp(), tempfile.mkdtemp()
-    temp_dirs.extend([a_dir, b_dir])
-
-    for f in files:
-        sys.stderr.write('  %s...\n' % f.filename)
-        if f.status == 'modified' or f.status == 'deleted':
-            contents = base_repo.get_file_contents(f.filename, pr.base.sha).decoded_content
-            put_in_dir(a_dir, f.filename, contents)
-        if f.status == 'modified' or f.status == 'added':
-            contents = head_repo.get_file_contents(f.filename, pr.head.sha).decoded_content
-            put_in_dir(b_dir, f.filename, contents)
-        # f.status == 'moved'?
-
-    return a_dir, b_dir
+    files = pr.get_files()
+
+    return [GitHubDiff(pr, f) for f in files]
 
 
 class NoRemoteError(Exception):

webdiff/localfilediff.py

@@ -41,10 +41,5 @@ def type(self):
             return 'move'
         return 'change'
 
-    def no_changes(self):
-        if self.a_path and self.b_path:
-            return util.are_files_identical(self.a_path, self.b_path)
-        return False
-
     def __repr__(self):
         return '%s/%s (%s)' % (self.a, self.b, self.type)

webdiff/util.py

@@ -10,8 +10,6 @@
 
 from PIL import Image
 
-import github_fetcher
-
 
 class ImageMagickNotAvailableError(Exception):
     pass