[MERGE] Shelf 2/5: ShelfCreator

[John Arbash Meinel]

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

Aaron Bentley wrote:
> Hi all,
> 
> This is the object that is used to select changes to sheve.  It builds
> up two transforms: the work_transform, which is applied to the working
> tree, and the shelf_transform, which is ultimately serialized.
> 
> The target_tree is typically the basis revision, but may be any other
> revision.  As work_tree and target_tree are compared, the opportunity to
>  shelve differences between the two is provided.  The two transforms are
> updated as opposites, so shelving a deletion in the work_transform
> applies a deletion to the shelf_tranform.
> 
> I'm quite proud of shelve_lines, which, given a new version of the file,
> will figure out what lines you have removed, and add them to the
> shelf_transform.  This means that virtually any tool can be used to
> create the new version of the file.
> 
> Aaron
> 

+                if names[0] != names[1] or parents[0] != parents[1]:
+                    self.renames[file_id] = (names, parents)
+                    yield ('rename', file_id) + paths
+
+                if kind[0] != kind [1]:
+                    yield ('change kind', file_id, kind[0], kind[1],
paths[0])
+                elif changed:
+                    yield ('modify text', file_id)

^- If I understand correctly, if you have a rename object that is also
changed (kind or text) then you'll get yield 2 results for that entry,
correct?


I'm not 100% sure about the api for ShelfCreator. It seems that you
iterate over it and make calls into 'shelf_XXX'. My concern is that it
is acting a bit as a hybrid, rather than clearly an iterating class, or
a class you make calls on.

Perhaps if we just changed __iter__ to ".iter_XXX". I'm not strict about
it, just feels a bit odd.

...

+    def read_tree_lines(self, tree, file_id):
+        """Read text lines from a tree.
+
+        (Tree.get_file_lines is not an official API)
+        """
+        return osutils.split_lines(tree.get_file_text(file_id))
+

^- When your other patch goes through, I'd rather not reproduce
get_file_lines() here (I don't know any other way to remember to do that
then mentioning it during review.)


...

+    def make_shelf_filename(self):
+        """Generate a filename for a shelf."""
+        transport = self.work_tree.bzrdir.root_transport.clone('.shelf2')
+        transport.ensure_base()
+        return transport.local_abspath('01')
+

^- Is this the responsibility of 'ShelfCreator' or should it be
something you request of the WT. It feels like a WT function to me, not
to mention I think we decided to put it into .bzr/checkout/shelf/*
rather than .shelf2.

...


BB:tweak

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

iEYEARECAAYFAkj89JgACgkQJdeBCYSNAAMVYgCdEemOpXSNz8f03UqpnoFgalCL
d6sAn0AHX0RMQjdbvsV6/bRC7/Gd5FtB
=MU6Z
-----END PGP SIGNATURE-----