[MERGE] Revert merge and pull display using TreeTransform._iter_changes

[John Arbash Meinel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Aaron Bentley wrote:
> Hi all,
> 
> This patch implements Transform._iter_changes, which translates a
> pending transform into _iter_changes format, for display purposes.
> 
> It then changes "revert" to use TreeTransform._iter_changes, so that it
> reflects conflict resolution better.  It adds the same output to "merge"
> and "pull".
> 
> Since the output is provided using trace.note(), --quiet will silence it.
> 
> Aaron


- -            count = tree_to.pull(branch_from, overwrite, rev_id)
+            count = tree_to.pull(branch_from, overwrite, rev_id,
+            delta.ChangeReporter(tree_to.inventory))

^- can you indent this, so it is clearer you are continuing the previous
line?

         else:
             count = branch_to.pull(branch_from, overwrite, rev_id)
         note('%d revision(s) pulled.' % (count,))
@@ -2333,6 +2335,7 @@
             merge_type = _mod_merge.Merge3Merger

         tree = WorkingTree.open_containing(u'.')[0]
+        change_reporter = delta.ChangeReporter(tree.inventory)

...

@@ -344,8 +344,8 @@
         exe_change = False
         # files are "renamed" if they are moved or if name changes, as long
         # as it had a value
- -        if None not in name and (name[0] != name[1] or
- -                                 parent_id[0] != parent_id[1]):
+        if None not in name and None not in parent_id and\
+            (name[0] != name[1] or parent_id[0] != parent_id[1]):
             renamed = True
         else:
             renamed = False

^- "None not in parent_id" looks incorrect. It might be "None not in
parent_ids" or "parent_id is not None"

But at least from the variables it looks like you are trying to do a set
check over a single value. (But then again, it looks the same for name).

You might consider using plurals for variables that contain more than
one value. "None not in name" is a lot more confusing than "None not in
names".


v- Incorrect import line. There are quite a few places I've seen you do
this. It really should be: "from bzrlib import delta". And really that
should be done up at the top of the module, possibly in a lazy_import
statement. (The other place I just noticed is an 'import annotate' in
WorkingTree.annotate_iter())

+            if change_reporter is not None:
+                import delta
+                delta.report_changes(self.tt._iter_changes(),
change_reporter)
             self.cook_conflicts(fs_conflicts)
             for conflict in self.cooked_conflicts:
                 warning(conflict)
...


v- This function seems a little long. Are there reasonable places that
you could break it up? There may not be. But I generally avoid super
long functions.

+    def _iter_changes(self):
+        """Produce output in the same format as Tree._iter_changes.
+
+        Will produce nonsensical results if invoked while
inventory/filesystem
+        conflicts (as reported by TreeTransform.find_conflicts()) are
present.
+
+        This reads the Transform, but only reproduces changes involving a
+        file_id.  Files that are not versioned in either of the FROM or TO
+        states are not reflected.
+        """
+        final_paths = FinalPaths(self)

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.3 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFF02VsJdeBCYSNAAMRAhhEAJsEOtNBj1cyem1VjA1qIlzj9dYlogCgzzdt
8JFztG8cRGBN5NgxZyDX+no=
=ahgJ
-----END PGP SIGNATURE-----