[MERGE] WorkingTree.unversion

[John Arbash Meinel]

Robert Collins wrote:

...

> 
> All the others that I know of are the explicit multi-line version.
> 
> I dont like that abuse of kwargs to set value, for some reason I thought
> it had been removed in BzrNewError.

I kind of liked using kwargs for it, because it made it clearer that you
expected BzrNewError to know about a member so it could create the doc
string later.

But I don't really care either way. Just that knowing:

self.foo = bar

Creates an entry in self.__dict__ is a little bit of voodoo.

Speaking of error voodoo. What do you think about changing the base
error so that instead of using self.__doc__, we create a class member
like error_str = ''

There 3 advantages I can think of

1) It means bzr will still run with -OO
2) Multi-line error strings are easier and look nicer. Instead of:

class Foo(BzrNewError):
    """First line: %(baz)s
Second line: %(bar)s"""

You can do:

class Foo(BzrNewError):
    """This should be raised when this particular situation arises."""

    error_str = ("First line: %(baz)s\n"
		 "Second line: %(bar)s")

3) It actually gives us a place to tell *developers* what the exception
is supposed to mean, rather than only containing the user documentation.

While I'm on that topic, though. I might also want to mention that for
many of our exceptions, the user-visible value is sub-optimal. For
example a lot of them give BzrBranch5('file://url/to/branch/that/failed)
Rather than just:
Could not do Foo in /path/to/branch/that/failed.

I'm thinking we probably should extend our Error api a bit. This could
include giving an encoding for the error string, so that urls can show
up as nice unicode paths.

> 
> 
>> ...
>>
>> v- I thought PEP8 said
>> while to_find_delete:
>>
>> was preferable to:
>> while len(to_find_delete):
> 
> I was remembering a discussion we had about clarity a few weeks back.
> Personally I prefer 
> while to_find_delete:
> 
> so - very happy to change if you agree that that is fine.

I prefer it too. Though I may have flip-flopped in the past. :)

> 
>> Also, I think Aaron made a good point that remove_recursive() might be a
>> better name.
> 
> Hes also expressed a desire for a entry based call, so about about
> calling this one
> remove_recursive_id.
> 
> -Rob

remove_recursive_id sounds good to me. Though why he would need to do a
lookup rather than just doing 'ie.file_id' I don't really understand.

With the name change, +1. The rest is just discussion.

John
=:->

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 254 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060907/4acd33e0/attachment.pgp