[MERGE] move smart_add_tree to MutableTree, tested on WorkingTree..

[Martin Pool]

On 7/4/07, Aaron Bentley <aaron.bentley at utoronto.ca> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Robert Collins wrote:
> > This moves smart_add_tree as I discussed yesterday. Because its not
> > fully refactored - it still uses inventory, and uses os specific
> > functions not transport functions while it is on MutableTree (where i
> > think it should be), its only functional on WorkingTree today.
>
> In that case, I would expect MutableTree.smart_add_tree to raise
> NotImplementedError, and WorkingTree to implement it.

I think it could in principle be fixed to work on MemoryTree though,
so it's ok to leave it there while it's being cleaned up.

> > +
> > +    @needs_tree_write_lock
> > +    def smart_add(self, file_list, recurse=True, action=None, save=True):
> > +        """Version file_list, optionally recursing into directories.
> > +
> > +        This is designed more towards DWIM for humans than API clarity.
> > +        For the specific behaviour see the help for cmd_add().
> > +
> > +        Returns the number of files added.
>
> ^^^ needs to be updated.

This reminds me that it should probably return a result object rather
than an int, but I think it can merge without that as  more updates
are expected.

> > +        :param save: Save the inventory after completing the adds. If False
> > +            this provides dry-run functionality by doing the add and not saving
> > +            the inventory.  Note that the modified inventory is left in place,
> > +            allowing further dry-run tasks to take place. To restore the
> > +            original inventory call self.read_working_inventory().
>
> ^^^ Retaining the modified inventory doesn't mesh well with dirstate
> trees.  I think if we're introducing this as a new function, it would
> make sense change this behavior.

At minimum, we should change the docstring so as not to promise this
behaviour.  But maybe it should in fact revert the inventory in a dry
run, and change the name of this parameter.
> >              else:
> >                  file_id = osutils.safe_file_id(file_id)
> >                  inv.add_path(f, kind=kind, file_id=file_id)
> > -        self._write_inventory(inv)
> > +            self._inventory_is_modified = True
>
> It would be nice to use apply_inventory_delta instead.

I agree, but this patch is still an incremental improvement.


-- 
Martin