[MERGE] Re: Implementation and tests for -r revno:N:path

[John Arbash Meinel]

Matthieu Moy wrote:
> John Arbash Meinel <john at arbash-meinel.com> writes:
> 
>> What about:
>>
>> if (revision is not None and len(revision) == 2
>>     and not revision[0].needs_tree()
>>     and not revision[1].needs_tree()):
>>     tree1, tree2 = None, None
>> else:
>>     raise
> 
> No problem. Indeed, I was probably tired the day I wrote this because
> I didn't manage to get the lazy evaluation right ;-).

I actually wonder if it is more understandable as:

if (revision is None or len(revision != 2)
    or revision[0].needs_branch()
    or revision[1].needs_branch()):
    raise
else:
  tree1, tree2 = None, None

But you've written what I asked, so I'm not going to block anything.

> 
>> I also think that 'needs_tree' is an inaccurate statement. It should be
>> 'needs_branch'.
> 
> Hmm, right. Indeed, the UI needs a tree, but this is managed in
> builtins.py, so the revision spec should only talk about branches.
> 

You have a small typo in needs_branch().
The final """ should be on a line by itself.

...

> I also think so, and my patch is relatively trivial and well tested,
> so there shouldn't be any problem.
> 
> BTW, my patch now also fixes a typo in diff test that I even had
> cut-and-pasted to my own test ;-).
> 

So +1 from me, and +1 from Aaron. I'll go ahead and clean up the
docstring, and submit it.

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/20060727/91a55971/attachment.pgp