[PATCH] Re: Feature Request: 'bzr mv --after' to tell bzr that file(s) have already been moved in the working tree
Hanns-Steffen Eichenberg
scameronde at googlemail.com
Mon Nov 6 19:26:38 GMT 2006
Thank you for the review, Alexander.
On 11/6/06, Alexander Belchenko <bialix at ukr.net> wrote:
>
> -----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.
'bzr selftest' did the first 534 tests (or so, i am at my linux box at the
moment,
not at the WinXP box i used to develop the code) and then just did nothing.
no
progress, no cpu usage. Left it idle for more than 2 hours, then killed the
process.
I will give more information when i am back at the other computer.
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
I didn't know that before. Thanks. Will update accordingly.
=== 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.
That was not intentionally. I must have messed something completely.
I thought i have set 'expandtab' and did a '%s/\t/ /g' before commiting,
just to be sure. Will double check in the future.
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.
Will make it a ValueObject instead.
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'
API compatibility .... now write it on the board one hundred times..
- --
> Alexander
Ciao,
Steffen
-------------- next part --------------
An HTML attachment was scrubbed...
URL: https://lists.ubuntu.com/archives/bazaar/attachments/20061106/c82cd2be/attachment.htm
More information about the bazaar
mailing list