Fwd: [MERGE] enhanced mv command

Marius Kruger amanic at gmail.com
Thu Dec 21 19:39:36 GMT 2006


my bundle is a bit big, so I'm sending a smaller mail now again,
but zipped this time...


---------- Forwarded message ----------
From: Marius Kruger <amanic at gmail.com>
Date: Dec 21, 2006 9:31 PM
Subject: Re: [MERGE] enhanced mv command
To: bazaar-ng at lists.canonical.com
Cc: Hanns-Steffen Eichenberg <scameronde at googlemail.com>, John Arbash Meinel
<john at arbash-meinel.com>

right,
I've addressed most of the critiques, (but there will be new ones I'm sure:)

TODO: refactor the tests a bit: add more white-box testing and less
black-box testing.
  I have not done much manual testing but all the selftests pass.

But at least here is a bundle from where we can work from.
Its a combination of the original patch and my work.
For easier review I also attached a diff containing only the changes I made.


> On 12/19/06, John A Meinel < john at arbash-meinel.com > wrote:
> >
> > John Arbash Meinel has voted -0.
> > Status is now: Waiting
> > Comment:
> > There was some feedback about how this needs to be improved. At that
> > point it should be merged.
> >
> > We would really like this for 0.14, so if nobody can get to it, ping me
> > and I'll look it over.
> >
> > I'm giving it -0, because I don't want us to forget it.
> >
> > For details, see: http://bundlebuggy.aaronbentley.com/request/%3Ca27e0da0611130107u5119c163sb4f55252ce1b56a2%40mail.gmail.com%3E
> >
> >
>

-- 



I code therefore I am.



-- 



I code therefore I am.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: https://lists.ubuntu.com/archives/bazaar/attachments/20061221/7bb28b79/attachment.htm 
-------------- next part --------------
=== modified file 'bzrlib/builtins.py'
--- bzrlib/builtins.py	2006-12-21 05:11:49 +0000
+++ bzrlib/builtins.py	2006-12-21 16:36:59 +0000
@@ -472,10 +472,9 @@
     """
 
     takes_args = ['names*']
-    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.")]
+    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.")]
     aliases = ['move', 'rename']
     encoding_type = 'replace'
 

=== modified file 'bzrlib/errors.py'
--- bzrlib/errors.py	2006-12-07 23:18:34 +0000
+++ bzrlib/errors.py	2006-12-21 19:14:35 +0000
@@ -319,6 +319,33 @@
     _fmt = "File exists: %(path)r%(extra)s"
 
 
+class FilesExist(PathError):
+    
+    _fmt = "File(s) exist: %(paths_as_string)s%(extra)s"
+
+    # used when reporting that files do exist
+
+    def __init__(self, paths, extra=None):
+        # circular import
+        from bzrlib.osutils import quotefn
+        BzrError.__init__(self)
+        self.paths = paths
+        self.paths_as_string = ' '.join([quotefn(p) for p in paths])
+        if extra:
+            self.extra = ': ' + str(extra)
+        else:
+            self.extra = ''
+
+class NotADirectory(PathError):
+
+    _fmt = "%(path)r is not a directory %(extra)s"
+
+
+class NotInWorkingDirectory(PathError):
+
+    _fmt = "%(path)r is not a the working directory %(extra)s"
+
+
 class DirectoryNotEmpty(PathError):
 
     _fmt = "Directory not empty: %(path)r%(extra)s"
@@ -491,17 +518,50 @@
         self.repo_format = repo_format
 
 
+class AlreadyVersionedError(BzrError):
+    #Used when a path is expected not to be versioned, but it is.
+    
+    _fmt = "%(contextInfo)s%(path)s is already versioned"
+
+    def __init__(self, path, contextInfo=None):
+        """Construct a new NotVersionedError.
+
+        :param path: This is the path which is versioned, 
+        which should be in a user friendly form.
+        :param contextInfo: If given, this is information about the context,
+        which could explain why this is expected to not be versioned.
+        """
+        BzrError.__init__(self)
+        self.path = path
+        if contextInfo is None:
+            self.contextInfo = ''
+        else:
+            self.contextInfo = contextInfo + ": "
+
+
 class NotVersionedError(BzrError):
-
-    _fmt = "%(path)s is not versioned"
-
-    def __init__(self, path):
+    #Used when a path is expected to be versioned, but it is not.
+    
+    _fmt = "%(contextInfo)s%(path)s is not versioned"
+
+    def __init__(self, path, contextInfo=None):
+        """Construct a new NotVersionedError.
+
+        :param path: This is the path which is not versioned, 
+        which should be in a user friendly form.
+        :param contextInfo: If given, this is information about the context,
+        which could explain why this is expected to be versioned.
+        """
         BzrError.__init__(self)
         self.path = path
-
-
+        if contextInfo is None:
+            self.contextInfo = ''
+        else:
+            self.contextInfo = contextInfo + ": "
+        
+        
 class PathsNotVersionedError(BzrError):
-    # used when reporting several paths are not versioned
+    # used when reporting several paths which are not versioned
 
     _fmt = "Path(s) are not versioned: %(paths_as_string)s"
 
@@ -514,17 +574,21 @@
 
 class PathsDoNotExist(BzrError):
 
-    _fmt = "Path(s) do not exist: %(paths_as_string)s"
+    _fmt = "Path(s) do not exist: %(paths_as_string)s%(extra)s"
 
     # used when reporting that paths are neither versioned nor in the working
     # tree
 
-    def __init__(self, paths):
+    def __init__(self, paths, extra=None):
         # circular import
         from bzrlib.osutils import quotefn
         BzrError.__init__(self)
         self.paths = paths
         self.paths_as_string = ' '.join([quotefn(p) for p in paths])
+        if extra:
+            self.extra = ': ' + str(extra)
+        else:
+            self.extra = ''
 
 
 class BadFileKindError(BzrError):

=== modified file 'bzrlib/tests/blackbox/test_mv.py'
--- bzrlib/tests/blackbox/test_mv.py	2006-12-21 05:11:49 +0000
+++ bzrlib/tests/blackbox/test_mv.py	2006-12-21 19:14:35 +0000
@@ -78,7 +78,7 @@
         tree.add(['test.txt'])
 
         self.run_bzr_error(
-            ["^bzr: ERROR: destination .* is not a versioned directory$"],
+            ["^bzr: ERROR: Invalid move destination: .* is not versioned$"],
             'mv', 'test.txt', 'sub1')
         
         self.run_bzr_error(
@@ -171,7 +171,7 @@
     
         os.remove('a')
         self.run_bzr_error(
-            ["^bzr: ERROR: can't rename: new name .* is already versioned$"],
+            ["^bzr: ERROR: Invalid move destination: .* is already versioned$"],
             'mv', 'a', 'b')
         self.failIfExists('a')
         self.failUnlessExists('b')
@@ -228,8 +228,8 @@
         tree.commit('initial commit')
 
         os.rename('a1', 'sub1/a1')
-        self.run_bzr_error(
-            ["^bzr: ERROR: destination .* is not a versioned directory$"],
+        self.run_bzr_error(            
+            ["^bzr: ERROR: Invalid move destination: .* is not versioned$"],
             'mv', 'a1', 'a2', 'sub1')
         self.failIfExists('a1')
         self.failUnlessExists('a2')
@@ -245,8 +245,8 @@
         tree.commit('initial commit')
 
         self.run_bzr_error(
-            ["^bzr: ERROR: can't rename: both, old name .* and new name .*"
-             " exist. Use option '--after' to force rename."],
+            ["^bzr: ERROR: File\(s\) exist: .+ .+: can't rename."
+             " Use option '--after' to force rename."],
             'mv', 'a', 'b')
         self.failUnlessExists('a')
         self.failUnlessExists('b')
@@ -273,8 +273,8 @@
         tree.commit('initial commit')
 
         self.run_bzr_error(
-            ["^bzr: ERROR: can't rename: both, old name .* and new name .*"
-             " exist. Use option '--after' to force rename."],
+            ["^bzr: ERROR: File\(s\) exist: .+ .+: can't rename."
+             " Use option '--after' to force rename."],
             'mv', 'a1', 'a2', 'sub1')
         self.failUnlessExists('a1')
         self.failUnlessExists('a2')
@@ -295,4 +295,3 @@
         self.failUnlessExists('a2')
         self.failUnlessExists('sub1/a1')
         self.failUnlessExists('sub1/a2')
-

=== modified file 'bzrlib/workingtree.py'
--- bzrlib/workingtree.py	2006-12-21 05:11:49 +0000
+++ bzrlib/workingtree.py	2006-12-21 19:14:35 +0000
@@ -72,15 +72,19 @@
 
 from bzrlib import symbol_versioning
 from bzrlib.decorators import needs_read_lock, needs_write_lock
-from bzrlib.errors import (BzrCheckError,
+from bzrlib.errors import (AlreadyVersionedError,
+                           BzrCheckError,
                            BzrError,
                            ConflictFormatError,
-                           WeaveRevisionNotPresent,
+                           FilesExist,                     
+                           NotADirectory,
                            NotBranchError,
+                           NotInWorkingDirectory,
                            NoSuchFile,
                            NotVersionedError,
                            MergeModifiedFormatError,
                            UnsupportedOperation,
+                           WeaveRevisionNotPresent,
                            )
 from bzrlib.inventory import InventoryEntry, Inventory, ROOT_ID
 from bzrlib.lockable_files import LockableFiles, TransportLock
@@ -1023,7 +1027,7 @@
         Everything else results in an error.
 
         This returns a list of (from_path, to_path) pairs for each
-        entry that is moved.
+        entry, that is moved.
         """
         rename_entries = []
         rename_tuples = []
@@ -1043,24 +1047,27 @@
         inv = self.inventory
         to_abs = self.abspath(to_dir)
         if not isdir(to_abs):
-            raise BzrError("destination %r is not a directory" % to_abs)
+            raise NotADirectory(to_abs, extra="Invalid move destination")
         if not self.has_filename(to_dir):
-            raise BzrError("destination %r not in working directory" % to_abs)
+            raise NotInWorkingDirectory(to_dir, extra=
+                "(Invalid move destination)")
         to_dir_id = inv.path2id(to_dir)
         if to_dir_id is None:
-            raise BzrError("destination %r is not a versioned"
-                           " directory" % to_dir)
+            raise NotVersionedError(path=str(to_dir),
+                contextInfo="Invalid move destination")
+            
         to_dir_ie = inv[to_dir_id]
         if to_dir_ie.kind != 'directory':
-            raise BzrError("destination %r is not a directory" % to_abs)
+            raise NotADirectory(to_abs, extra="Invalid move destination")
 
         # 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)
+                raise NotVersionedError(path=str(from_rel),
+                    contextInfo="Invalid source")
+
             from_entry = inv[from_id]
             from_parent_id = from_entry.parent_id
             to_rel = pathjoin(to_dir, from_tail)
@@ -1107,18 +1114,18 @@
 
             # check the inventory for source and destination
             if from_id is None:
-                raise BzrError("can't rename: old name %r is not versioned" % 
-                               from_rel)
+                raise NotVersionedError(path=str(from_rel),
+                    contextInfo="Invalid move source")
             if to_id is not None:
-                raise BzrError("can't rename: new name %r is already"
-                               " versioned" % to_rel)
+                raise AlreadyVersionedError(path=str(to_rel),
+                    contextInfo="Invalid move destination")
 
             # try to determine the mode for rename (only change inv or change 
             # inv and file system)
             if after:
                 if not self.has_filename(to_rel):
-                    raise BzrError("can't rename: new working file %r does not"
-                                   " exist" % to_rel)
+                    raise NoSuchFile(path=str(to_rel),
+                        extra="New file has not been created yet")
                 only_change_inv = True
             elif not self.has_filename(from_rel) and self.has_filename(to_rel):
                 only_change_inv = True
@@ -1128,13 +1135,13 @@
                 # something is wrong, so lets determine what exactly
                 if not self.has_filename(from_rel) and \
                    not self.has_filename(to_rel):
-                    raise BzrError("can't rename: neither old name %r nor new"
-                                   " name %r exist" % (from_rel, to_rel))
+                    raise PathsDoNotExist(paths=(str(from_rel), str(to_rel)),
+                        extra="can't rename")
                 else:
-                    raise BzrError("can't rename: both, old name %r and new"
-                                   " name %r exist. Use option '--after' to"
-                                   " force rename." % (from_rel, to_rel))
-            rename_entry.set_only_change_inv(only_change_inv)                       
+                    raise FilesExist(paths=(str(from_rel), str(to_rel)),
+                        extra="can't rename. Use option '--after' to"
+                              " force rename.")
+            rename_entry.only_change_inv = only_change_inv                       
         return rename_entries
 
     def _move(self, rename_entries):
@@ -1167,10 +1174,10 @@
             try:
                 self._move_entry(entry, inverse=True)
             except OSError, e:
-                raise BzrError("error moving files. rollback failed. the"
+                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])
+                               " Please consider doing a 'bzr revert'."
+                               " Error message is: %s" % e[1])
 
     def _move_entry(self, entry, inverse=False):
         inv = self.inventory
@@ -1196,8 +1203,8 @@
 
         This can change the directory or the filename or both.
 
-        rename_one has several 'modes' to work. First it can rename a physical 
-        file and change the file_id. That is the normal mode. Second it can 
+        rename_one has several 'modes' to work. First, it can rename a physical 
+        file and change the file_id. That is the normal mode. Second, it can 
         only change the file_id without touching any physical file. This is 
         the new mode introduced in version 0.13.
         
@@ -1268,9 +1275,6 @@
             self.to_parent_id = to_parent_id
             self.only_change_inv = False
 
-        def set_only_change_inv(self, only_change_inv):
-            self.only_change_inv = only_change_inv
-
     @needs_read_lock
     def unknowns(self):
         """Return all unknown files.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: enhancedmv2.patch.zip
Type: application/zip
Size: 245836 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20061221/7bb28b79/attachment.zip 


More information about the bazaar mailing list