[MERGE] enhanced mv command

[Aaron Bentley]

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

Hanns-Steffen Eichenberg wrote:
> this patch contains an enhanced version of the mv command.

This is very close.  Just a few issues.

> +    takes_options = [Option("after", help="move only the bzr identifier of the"
> +                                          " file (file has already been" 
> +                                          " moved). Use this flag if bzr is"
> +                                          " not able to detect this itself.")]

This is a style thing, but I think most of us would use a smaller indent
here:

    takes_options = [Option("after", help="move only the bzr identifier"
        " of the file (file has already been moved). Use this flag if"
        " bzr is not able to detect this itself.")]

This is optional, of course.

> === modified file bzrlib/tests/blackbox/test_mv.py // last-changed:scameronde at g
> ... ooglemail.com-20061108154710-5c1b0731ddb9365e // executable:yes
> --- bzrlib/tests/blackbox/test_mv.py
> +++ bzrlib/tests/blackbox/test_mv.py

[...]

> +        self.run_bzr('mv', 'a1', 'a2', 'sub1', '--after')
> +        self.failUnlessExists('a1')
> +        self.failUnlessExists('a2')
> +        self.failUnlessExists('sub1/a1')
> +        self.failUnlessExists('sub1/a2')
> +
> 

You have an extra newline here.


> === modified file bzrlib/workingtree.py // executable:yes
> --- bzrlib/workingtree.py
> +++ bzrlib/workingtree.py
> @@ -959,123 +959,287 @@
>                  stack.pop()
>  
>      @needs_tree_write_lock
> -    def move(self, from_paths, to_name):
> +    def move(self, from_paths, to_dir=None, after=False, **kwargs):
>          """Rename files.
>  
> -        to_name must exist in the inventory.
> +        to_dir must exist in the inventory.
>  
> -        If to_name exists and is a directory, the files are moved into
> +        If to_dir exists and is a directory, the files are moved into
>          it, keeping their old names.  
>  
> -        Note that to_name is only the last component of the new name;
> +        Note that to_dir is only the last component of the new name;
>          this doesn't change the directory.
>  
> +        For each entry in from_paths the move mode will be determined 
> +        independently.

Should have a comma after "For each entry"

> +        # create rename entries and tuples
> +        for from_rel in from_paths:
> +            from_tail = splitpath(from_rel)[-1]
> +            from_id = inv.path2id(from_rel)
> +            if from_id is None:
> +                raise BzrError("can't rename: old name %r is not versioned" % 
> +                               from_rel)

I think this should be a custom error class, because different callers
might want to handle errors in different ways.  Same for the other
errors emitted by this method.

> +                raise BzrError("error moving files. rollback failed. the"
> +                               " working tree is in an inconsistent state."
> +                               " please consider doing a 'bzr revert'."
> +                               " error message is: %s" % e[1])

Needs capitals on "rollback", "the", "please", "error".

> +        rename_one has several 'modes' to work. First it can rename a physical 

'First' needs a comma.

> +        file and change the file_id. That is the normal mode. Second it can

'Second' needs a comma.

> +    class _RenameEntry(object):

[...]

> +        def set_only_change_inv(self, only_change_inv):
> +            self.only_change_inv = only_change_inv

This method doesn't seem to have a purpose.

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

iD8DBQFFW+QI0F+nu1YWqI0RAhyhAJ954KkBIbue1znNi2hjOYj6O8bZCgCfZspj
jUXQ3Iswe3oNq87EV+8yG50=
=m6St
-----END PGP SIGNATURE-----