[MERGE] enhanced mv command
aaron.bentley at utoronto.ca
Thu Nov 16 04:07:36 GMT 2006
-----BEGIN PGP SIGNED MESSAGE-----
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
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 @@
> - 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)
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.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.3 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
-----END PGP SIGNATURE-----
More information about the bazaar