bzr mv tweaks

[Spencer Chastain]

I've attached some touch-ups that get all but 3 of the existing mv tests
working.  The remaining three use regular expressions to evaluate successful
(or rather erroneous) completion, and I'm at a loss as to why the changes
I've provided are not passing (in particular, I've been focusing test_mv.py,
test_mv_invalid:

    def test_mv_invalid(self):
        tree = self.make_branch_and_tree('.')
        self.build_tree(['test.txt', 'sub1/'])
        tree.add(['test.txt'])

        self.run_bzr_error (
            ["^No Files Moved!$"
             "^$"
             "^Not Moved:$"
             "^\ttest\.txt : sub1 is not versioned\.$"],
            'mv test.txt sub1')

        self.run_bzr_error(
            ["^bzr: ERROR: Could not move test.txt => .*hello.txt: "
             "sub1 is not versioned\.$"],
            'mv test.txt sub1/hello.txt')

This last version of mods to the above test was a bit of a grasp at
desperation ... I still don't see why the expression:
    "No Files Moved!\n\nNot Moved:\n\ttest\.txt : sub1 is not versioned.\n"
won't allow the test to pass.

Any pointers in getting past this would be greatly appreciated.

--Spencer

On Jan 12, 2008 11:08 PM, Spencer Chastain < sechastain at gmail.com> wrote:

>
> A few weeks ago I reported a bug (and, really, it's not a bug so much as
> it is just lack of info to the user) with the mv command where the command
> will fail and tell you why but hasn't told you what else it has done and
> what it has yet to do.  If you want better details, check out the bug report
> here: https://bugs.launchpad.net/bzr/+bug/177809
>
> This weekend, I started messing around trying to solve this problem for
> myself.  Seemed straightforward enough of a problem.
>
> I started off by tweaking the command code itself and worked up the diff
> included at the end of this email.
>
> I then started working on fixing up the test code so that my changes would
> pass the selftest, and it's at this point where I thought I'd get a kosher
> check on my changes and intended future changes to address passing the
> tests.
>
> Essentially, my intent is to leave the format of the output below the same
> and modify the content with the 'extra' information that comes with the
> exceptions out of move.  So your error message would look something like:
>
> $ bzr mv burg child_dir/
> > No Files Moved!
> >
> > Not Moved:
> >         burg : burg is not versioned.
> >
>
> I'll have to modify the mv command to handle each different exception that
> can be raised out of the move function call (no biggie, just tedious for me
> to figure it out) and then update each test to check the proper output.
>
> The last thing I'd do would be to create two tests, one that tries to move
> a file name twice (a not un-common thing to do when you're moving lots of
> files at one time) and then a test to move a bunch files, some that exist,
> some that are dups, and some that don't exist or would fail for other
> reasons.
>
> The other thing I could do would be to update the move function call to
> behave as I would like the command to do, but that would mean messing with
> its return  to be a list of lists - a list of files that moved and a list of
> files that failed.  I wasn't sure how much of an impact that would be, and
> figured it was easier (and safer) to leave this kind of fix at the user
> interface level.
>
> I guess that's the long of it.  Let me know if I'm on target here.
>
> Thanks,
>
> --Spencer
>
> === modified file 'bzrlib/builtins.py'
> --- bzrlib/builtins.py  2007-12-29 18:55:20 +0000
> +++ bzrlib/builtins.py  2008-01-13 02:52:59 +0000
> @@ -537,8 +537,29 @@
>
>          if os.path.isdir(names_list[-1]):
>              # move into existing directory
> -            for pair in tree.move (rel_names[:-1], rel_names[-1],
> after=after):
> -                self.outf.write("%s => %s\n" % pair)
> +            moved_files = []
> +            dne_names = []
> +            for rel_name in rel_names[:-1]:
> +                try:
> +                    pair = tree.move([rel_name], rel_names[-1],
> after=after)
> +                    moved_files.append(pair)
> +                except errors.BzrError:
> +                    found = False
> +                    for check in moved_files:
> +                        if check[0][0] == rel_name:
> +                            found = True
> +                    if not found:
> +                        dne_names.append(rel_name)
> +            if len(moved_files) > 0:
> +                self.outf.write("Moved Files:\n")
> +                for moved in moved_files:
> +                    self.outf.write("\t%s => %s\n" % moved[0])
> +            else:
> +                self.outf.write("No Files Moved!\n\n")
> +            if len(dne_names) > 0:
> +                self.outf.write("Files Not Moved (Do Not Exist):\n")
> +                for dne_name in dne_names:
> +                    self.outf.write("\t%s\n" % dne_name)
>          else:
>              if len(names_list) != 2:
>                  raise errors.BzrCommandError('to mv multiple files the'
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: https://lists.ubuntu.com/archives/bazaar/attachments/20080113/c54f0ba8/attachment-0001.htm 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bzr_mv.patch
Type: text/x-patch
Size: 6906 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20080113/c54f0ba8/attachment-0001.bin