[PATCH] Re: Feature Request: 'bzr mv --after' to tell bzr that file(s) have already been moved in the working tree

[Alexander Belchenko]

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

Hanns-Steffen Eichenberg пишет:
> This patch is a draft.
> 
> 
> On 11/3/06, John Arbash Meinel <john at arbash-meinel.com> wrote:
>>
>>
>> I thought I saw you mention on IRC you actually wrote a patch for this,
>> which I would be happy to look over.
> 
> 
> Here it is. Since this is my first work for bzr and my first lines of
> python
> for more than 3 years i would be happy if
> you can give the code a thorough review.
> 
> What i have done:
>  - modified the move code and the command line documentation
>  - some tests on the command line with a real world project
> 
> What i have not done:
>  - written any tests, because i do not know where and how. I would
> appreciate some help here
>  - run 'bzr selftest', because the selftest of 'bzr.dev' seems to hang on
> my machine
> 
> 
> The code was developed on WinXP. Did not had time to test it on my Linux
> box.

Some comments.

1) Please, don't zip patches (aka bundles). This make review harder.
2) I'm sure that in latest bzr.dev selftest works correct. It may starts
with big delay but in fact it works. And on windows you probably want to
run selftest on branch you currently working on. So you need to run
command: "python bzr selftest -v" from the root of your branch.

3) Some comments on your patch below. I don't actually check your new mv
logic, only obvious cosmetic things.


# Bazaar revision bundle v0.8
#
# message:
#   updated API and commandline documentation
#
# committer: eichenbs <eichenbs at gilpnb051b>
# date: Mon 2006-11-06 11:28:14.865999937 +0100

=== modified file .bzrignore //
last-changed:eichenbs at gilpnb051b-20061103151123
... -dcb18125ed861bdc
- --- .bzrignore
+++ .bzrignore
@@ -30,4 +30,6 @@
 ./tools/win32/bzr.iss
 ./dist
 # performance history data file
- -./.perf_history
\ No newline at end of file
+./.perf_history
+make_bat.last
+make_bat.py


^-- I'm sure you don't need this ignores because you can specify global
    ones in your
    C:\Document and Settings\eichenbs\Application Data\bazaar\2.0\ignore
    file



=== modified file bzrlib/builtins.py
- --- bzrlib/builtins.py
+++ bzrlib/builtins.py
@@ -425,16 +425,25 @@

     If the last argument is a versioned directory, all the other names
     are moved into it.  Otherwise, there must be exactly two arguments
- -    and the file is changed to a new name, which must not already exist.
+    and the file is changed to a new name.
+
+    If OLDNAME does not exist on the filesystem but is versioned and
+    NEWNAME does exist on the filesystem but is not versioned, mv
+    assumes that the file has been manually moved and only updates
+    its internal inventory to reflect that change.
+    The same is valid when moving many SOURCE files to a DESTINATION.

     Files cannot be moved between branches.
     """

     takes_args = ['names*']
+    takes_options = [Option("after", help="indicates that the files
have already been manually moved "
+		                                      "on the filesystem. Use this
flag if bzr is not able "
+                                              "to detect it itself.")]


^-- PEP-8! Don't use tabs! Only spaces and use 4 spaces for indenting.
    And keep length of lines less than 80 characters.



     aliases = ['move', 'rename']
     encoding_type = 'replace'

- -    def run(self, names_list):
+    def run(self, names_list, after=False):
         if names_list is None:
             names_list = []

@@ -444,13 +453,13 @@

         if os.path.isdir(names_list[-1]):
             # move into existing directory
- -            for pair in tree.move(rel_names[:-1], rel_names[-1]):
- -                self.outf.write("%s => %s\n" % pair)
+            for tuple in tree.move(rel_names[:-1], rel_names[-1], after):
+                self.outf.write("%s => %s\n" % (tuple['from_rel'],
tuple['to_rel']))


^-- 'tuple' is built-in function for creating python tuples objects.
    So better don't use this name as usual variable.
    In in the another places where you are using it it better to change
    name.



         else:
             if len(names_list) != 2:
                 raise errors.BzrCommandError('to mv multiple files the
destination '
                                              'must be a versioned
directory')
- -            tree.rename_one(rel_names[0], rel_names[1])
+            tree.rename_one(rel_names[0], rel_names[1], after)
             self.outf.write("%s => %s\n" % (rel_names[0], rel_names[1]))



=== modified file bzrlib/workingtree.py
- --- bzrlib/workingtree.py
+++ bzrlib/workingtree.py
@@ -989,7 +989,7 @@
                 stack.pop()

     @needs_tree_write_lock
- -    def move(self, from_paths, to_name):
+    def move(self, from_paths, to_dir, after=False):

^-- I don't tjink you *really* need to change argument name
    from 'to_name' to 'to_dir'


- --
Alexander

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.5 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFFT1jVzYr338mxwCURAiHdAJ9cGIoZr8B6Ot4l+NTVyXPjK06a9ACfR3uG
E1pp7oJ/VlCEcC0juSL9cf4=
=RUa3
-----END PGP SIGNATURE-----