Merge lp:~nmb/qbzr/diffview-refactor into lp:qbzr

[Neil Martinsen-Burrell]

This abstracts the diff loading code from out of DiffWindow so that it can be use in other windows which want to use diff previews. I cherrypicked these changes from lp:~hid-iwata/qbzr/qshelve as a separate revision. This is a standalone change which is separate from the added shelve functionality of any toolbar changes.

[IWATA Hidetaka (hid-iwata)]

This changes are separated from qshelve code originally.

You can merge it simply by doing::
  bzr merge lp:~hid-iwata/qbzr/qshelve -r revid:<email address hidden>

And it would make merging qshelve code more easier.

Regards.

[Alexander Belchenko (bialix)]

If this is just mechanical extract of the Iwata code, then I think that's ok to land it.

[Alexander Belchenko (bialix)]

Neil, as I can see the same refactoring is available in qshelve branch, and this is really the first revision in that branch so we can merge it preserving the history.

[Alexander Belchenko (bialix)]

Neil, as I can see your code is different because you've also added the load method:

+ def load(self):
+ if self._lines is None:
+ self._load_lines()

Why it's needed for you?

review: Needs Information

[IWATA Hidetaka (hid-iwata)]

It's only used from ShelveListWidget. (it was added by r1360 of qshelve branch.)

> Neil, as I can see your code is different because you've also added the load
> method:
>
>
> + def load(self):
> + if self._lines is None:
> + self._load_lines()
>
> Why it's needed for you?

[Alexander Belchenko (bialix)]

iwata пишет:
> It's only used from ShelveListWidget. (it was added by r1360 of qshelve branch.)

OK, thanks for clarifications Hidetaka.

=== modified file 'lib/diffwindow.py'
--- lib/diffwindow.py	2011-05-11 11:49:14 +0000
+++ lib/diffwindow.py	2011-05-13 21:40:03 +0000
@@ -126,6 +126,168 @@
     return 'Unknown tree'
 
 
+class DiffItem(object):
+    
+    @classmethod
+    def create(klass, trees, file_id, paths, changed_content, versioned, 
+            parent, name, kind, executable, filter = None):
+
+        if parent == (None, None): # filter out TREE_ROOT (?)
+            return None
+
+        # check for manually deleted files (w/o using bzr rm commands)
+        if kind[1] is None:
+            if versioned == (False, True):
+                # added and missed
+                return None
+            if versioned == (True, True):
+                versioned = (True, False)
+                paths = (paths[0], None)
+
+        renamed = (parent[0], name[0]) != (parent[1], name[1])
+
+        dates = [None, None]
+        for ix in range(2):
+            if versioned[ix]:
+                try:
+                    dates[ix] = trees[ix].get_file_mtime(file_id, paths[ix])
+                except OSError, e:
+                    if not renamed or e.errno != errno.ENOENT:
+                        raise
+                    # If we get ENOENT error then probably we trigger
+                    # bug #251532 in bzrlib. Take current time instead
+                    dates[ix] = time.time()
+                except FileTimestampUnavailable:
+                    # ghosts around us (see Bug #513096)
+                    dates[ix] = 0  # using 1970/1/1 instead
+
+        properties_changed = [] 
+        if bool(executable[0]) != bool(executable[1]):
+            descr = {True: "+x", False: "-x", None: None}
+            properties_changed.append((descr[executable[0]],
+                                       descr[executable[1]]))
+
+        if versioned == (True, False):
+            status = N_('removed')
+        elif versioned == (False, True):
+            status = N_('added')
+        elif renamed and changed_content:
+            status = N_('renamed and modified')
+        elif renamed:
+            status = N_('renamed')
+        else:
+            status = N_('modified')
+        # check filter options
+        if filter and not filter(status):
+            return None
+
+        return klass(trees, file_id, paths, changed_content, versioned, kind, 
+                        properties_changed, dates, status)
+
+    def __init__(self, trees, file_id, paths, changed_content, versioned, kind,
+                        properties_changed, dates, status):
+        self.trees = trees
+        self.file_id = file_id
+        self.paths = paths
+        self.changed_content = changed_content
+        self.versioned = versioned
+        self.kind = kind
+        self.properties_changed = properties_changed
+        self.dates = dates
+        self.status = status
+
+        self._lines = None
+        self._binary = None
+        self._group_cache = {}
+        self._encodings = [None, None]
+        self._ulines = [None, None]
+
+    def load(self):
+        if self._lines is None:
+            self._load_lines()
+
+    def _load_lines(self):
+        if ((self.versioned[0] != self.versioned[1] or self.changed_content)
+            and (self.kind[0] == 'file' or self.kind[1] == 'file')):
+            lines = []
+            binary = False
+            for ix, tree in enumerate(self.trees):
+                content = ()
+                if self.versioned[ix] and self.kind[ix] == 'file':
+                    content = get_file_lines_from_tree(tree, self.file_id)
+                lines.append(content)
+                binary = binary or is_binary_content(content)
+            self._lines = lines
+            self._binary = binary
+        else:
+            self._lines = ((),())
+            self._binary = False
+
+    @property
+    def lines(self):
+        if self._lines is None:
+            self._load_lines()
+        return self._lines
+
+    @property
+    def binary(self):
+        if self._binary is None:
+            self._load_lines()
+        return self._binary
+
+    def groups(self, complete, ignore_whitespace):
+        key = (complete, ignore_whitespace)
+        groups = self._group_cache.get(key)
+        if groups is not None:
+            return groups
+
+        lines = self.lines
+
+        if not self.binary:
+            if self.versioned == (True, False):
+                groups = [[('delete', 0, len(lines[0]), 0, 0)]]
+            elif self.versioned == (False, True):
+                groups = [[('insert', 0, 0, 0, len(lines[1]))]]
+            else:
+                groups = self.difference_groups(lines, complete, ignore_whitespace)
+        else:
+            groups = []
+
+        self._group_cache[key] = groups
+        return groups
+
+    def difference_groups(self, lines, complete, ignore_whitespace):
+        left, right = lines
+        if ignore_whitespace:
+            table = string.maketrans("", "")
+            strip = lambda l : l.translate(table, string.whitespace)
+            left  = (line.translate(table, string.whitespace) for line in left)
+            right = (line.translate(table, string.whitespace) for line in right)
+        matcher = SequenceMatcher(None, left, right)
+        if complete:
+            groups = list([matcher.get_opcodes()])
+        else:
+            groups = list(matcher.get_grouped_opcodes())
+
+        return groups
+
+
+    def encode(self, encodings):
+        lines = self.lines
+        ulines = self._ulines
+        for i in range(2):
+            if encodings[i] != self._encodings[i]:
+                self._encodings[i] = encodings[i]
+                if self.binary:
+                    ulines[i] = lines[i][:]
+                else:
+                    try:
+                        ulines[i] = [l.decode(encodings[i]) for l in lines[i]]
+                    except UnicodeDecodeError, e:
+                        trace.note('Failed to decode using %s, falling back to latin1', e.encoding)
+                        ulines[i] = [l.decode('latin1') for l in lines[i]]
+        return ulines
+
 class DiffWindow(QBzrWindow):
 
     def __init__(self, arg_provider, parent=None,
@@ -459,96 +621,24 @@
                     #       tree, e.g. for added/deleted file
 
                     self.processEvents()
-
-                    if parent == (None, None):  # filter out TREE_ROOT (?)
-                        continue
-
-                    # check for manually deleted files (w/o using bzr rm commands)
-                    if kind[1] is None:
-                        if versioned == (False, True):
-                            # added and missed
-                            continue
-                        if versioned == (True, True):
-                            versioned = (True, False)
-                            paths = (paths[0], None)
-
-                    renamed = (parent[0], name[0]) != (parent[1], name[1])
-
-                    dates = [None, None]
-                    for ix in range(2):
-                        if versioned[ix]:
-                            try:
-                                dates[ix] = self.trees[ix].get_file_mtime(file_id, paths[ix])
-                            except OSError, e:
-                                if not renamed or e.errno != errno.ENOENT:
-                                    raise
-                                # If we get ENOENT error then probably we trigger
-                                # bug #251532 in bzrlib. Take current time instead
-                                dates[ix] = time.time()
-                            except FileTimestampUnavailable:
-                                # ghosts around us (see Bug #513096)
-                                dates[ix] = 0  # using 1970/1/1 instead
-
-                    properties_changed = []
-                    if bool(executable[0]) != bool(executable[1]):
-                        descr = {True: "+x", False: "-x", None: None}
-                        properties_changed.append((descr[executable[0]],
-                                                   descr[executable[1]]))
-
-                    if versioned == (True, False):
-                        status = N_('removed')
-                    elif versioned == (False, True):
-                        status = N_('added')
-                    elif renamed and changed_content:
-                        status = N_('renamed and modified')
-                    elif renamed:
-                        status = N_('renamed')
-                    else:
-                        status = N_('modified')
-                    # check filter options
-                    if not self.filter_options.check(status):
-                        continue
-
-                    if ((versioned[0] != versioned[1] or changed_content)
-                        and (kind[0] == 'file' or kind[1] == 'file')):
-                        lines = []
-                        binary = False
-                        for ix, tree in enumerate(self.trees):
-                            content = ()
-                            if versioned[ix] and kind[ix] == 'file':
-                                content = get_file_lines_from_tree(tree, file_id)
-                            lines.append(content)
-                            binary = binary or is_binary_content(content)
-                            self.processEvents()
-                        if not binary:
-                            if versioned == (True, False):
-                                groups = [[('delete', 0, len(lines[0]), 0, 0)]]
-                            elif versioned == (False, True):
-                                groups = [[('insert', 0, 0, 0, len(lines[1]))]]
-                            else:
-                                groups = self.difference_groups(lines[0], lines[1])
-                            ulines = []
-                            for l, encoding in zip(lines, [self.encoding_selector_left.encoding,
-                                                           self.encoding_selector_right.encoding]):
-                                try:
-                                    ulines.append([i.decode(encoding) for i in l])
-                                except UnicodeDecodeError, e:
-                                    trace.note('Failed to decode using %s, falling back to latin1', e.encoding)
-                                    ulines.append([i.decode('latin1') for i in l])
-                            lines = ulines
-                            data = ((),())
-                        else:
-                            groups = []
-                        data = [''.join(l) for l in lines]
-                    else:
-                        binary = False
-                        lines = ((),())
-                        groups = ()
-                        data = ("", "")
+                    di = DiffItem.create(self.trees, file_id, paths, changed_content,
+                            versioned, parent, name, kind, executable, 
+                            filter = self.filter_options.check)
+                    if not di:
+                        continue
+
+                    lines = di.lines
+                    self.processEvents()
+                    groups = di.groups(self.complete, self.ignore_whitespace)
+                    self.processEvents()
+                    ulines = di.encode([self.encoding_selector_left.encoding,
+                                        self.encoding_selector_right.encoding])
+                    data = [''.join(l) for l in ulines]
+
                     for view in self.views:
-                        view.append_diff(list(paths), file_id, kind, status,
-                                         dates, versioned, binary, lines, groups,
-                                         data, properties_changed)
+                        view.append_diff(list(di.paths), di.file_id, di.kind, di.status,
+                                         di.dates, di.versioned, di.binary, ulines, groups,
+                                         data, di.properties_changed)
                         self.processEvents()
                     no_changes = False
             except PathsNotVersionedError, e:
@@ -565,21 +655,6 @@
                 gettext('&OK'))
         self.view_refresh.setEnabled(self.can_refresh())
 
-    def difference_groups(self, left, right):
-        if self.ignore_whitespace:
-            table = string.maketrans("", "")
-            strip = lambda l : l.translate(table, string.whitespace)
-            left  = (line.translate(table, string.whitespace) for line in left)
-            right = (line.translate(table, string.whitespace) for line in right)
-        matcher = SequenceMatcher(None, left, right)
-        self.processEvents()
-        if self.complete:
-            groups = list([matcher.get_opcodes()])
-        else:
-            groups = list(matcher.get_grouped_opcodes())
-
-        return groups
-
     def click_toggle_view_mode(self, checked):
         if checked:
             view = self.sdiffview