[MERGE] Shelf 2/5: ShelfCreator

[Aaron Bentley]

John Arbash Meinel wrote:
> Aaron Bentley wrote:
> +                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?

Correct and intended.  This allows the user to decide whether to shelve
the rename separately from shelving the change.

> 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.

If that makes you happy, I can do it.

> 
> ...
> 
> +    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.)

Sure.

> +    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.

This is the bare minimum required to pass the test suite.  It doesn't
even produce a unique name, just .shelf2/01 eternally.  It is removed
when ShelfManager is introduced.  But I can change it anyhow.

Aaron