[MERGE] enhanced mv command

[Aaron Bentley]

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

Steffen Eichenberg wrote:

> 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. 
> 
> 
>> Wouldn't that break the API of the methods 'move()' and 'rename_one()' ?

All our custom errors are derived from BzrError.  We don't consider it
API breakage to replace BzrError with a subclass of BzrError, (or any
exception class with a subclass of it).

>> Both methods used to throw BzrErrors. Most of the errors and error
>> messages are
>> from the original code.

Okay.  It's certainly reasonable to maintain existing behavior, even if
it would be poor practice for new code.  So if you'd really prefer not
to write the custom error classes, I can let it slide.

> This method doesn't seem to have a purpose.
> 
>> Like most accessors in Java. Just an old habit.
>> Fixed like all other errors you found.

That's what it looked like.  But since you can retroactively turn a
variable into a property in  Python, accessors are rarely needed.

>> Btw. i asked John a question in the old thread. It is about coding
>> style. I got no answer from him. Maybe
>> you can answer that. Here is Johns remark and my question:
> 
>>> > +    class _RenameEntry(object):
>>> > +        def __init__(self, from_rel, from_id, from_tail,
>> from_parent_id,
>>> > +                     to_rel, to_tail, to_parent_id):
>>> > +            self.from_rel = from_rel
>>> > +            self.from_id = from_id
>>> > +             self.from_tail = from_tail
>>> > +            self.from_parent_id = from_parent_id
>>> > +            self.to_rel = to_rel
>>> > +            self.to_tail = to_tail
>>> > +            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
>>> > +
>>> >
>>> > I think it is better if _RenameEntry is just a private class at module
>>> > scope, rather than a nested class.
>>> >
>>> > Or at the very least, I would define a nested class at the start of the
>>> > encompassing scope, rather than in the middle.
>>>
>>> I would like to have a stronger statement here. What is the desired
>> code style for bzr?
>>>   - Is it OK to use nested classes or not?
>>>   - where *exactly* should nested or private classes be placed in the
>> source?

We never use nested classes, but I don't think it's a matter of policy.
Certainly for me, it's because with so much indenting, you quickly run
out of space.

I've never heard anything against nested classes, so I would say it *is*
OK.  But we've never had opportunity to hammer the style issues out before.

>>> I am asking, because in most projects i encountered the rules 'keep
>> related things together,
>>> because it is easier to read' and 'define elements in the scope you are
>> using them'.

That seems reasonable.  I don't think we have precise ordering guidelines.

>>> Please do not get me wrong. I do not want to question the code style of
>> bzr, i want
>>> to understand and learn it.

We start with PEP8, and there are a few extra things in HACKING.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFFXHRm0F+nu1YWqI0RAn3IAJ9ALqtGAtg3yFUeTE330BcJul1P2gCfZQoR
eRM9B/QfReKS6VigW4blzao=
=XDOA
-----END PGP SIGNATURE-----