[MERGE] Allow appending path segments to the :<name> style aliases.

Michael Hudson michael.hudson at canonical.com
Thu Aug 14 21:20:01 BST 2008


John Arbash Meinel wrote:
> Michael Hudson wrote:
>> John Arbash Meinel wrote:
>>> Michael Hudson wrote:
>>>> This simple patch extends the directory service to allow path segments
>>>> after the name of a branch location alias.  For example you could do
>>>> this in a lightweight checkout of a loom to push a non-loomified version
>>>> of the current thread of the loom to its default location:
>>>> bzr export-loom :this; bzr push -d :this/`bzr nick`
>>>> Cheers,
>>>> mwh
>>> +        if '/' in url:
>>> +            name = url[1:url.find('/')]
>>> +            extra = url[url.find('/') + 1:]
>>> +        else:
>>> +            name = url[1:]
>>> +            extra = None
>>>
>>> ^- This seems like a really inefficient way of saying:
>>>
>>> name, extra = url[1:].split('/', 1)
>> Um... no?
> 
>>>>> ':this'.split('/', 1)
>> [':this']
> 
>> What I wrote may be verbose and lack cuteness, but I was delibarately
>> going for obvious correctness :)
> 
> Actually, I was talking about just the portion in the first 'if' statement:
> 
> if '/' in url:
>   name, extra = url[1:].split('/')
> else:
>   name = url[1:]
>   extra = None
> 
> Which is, as far as I can tell, true, and avoids doing 2 string finds
> and a bit of additions.

I don't mean to be rude, but I think you may have worried about
performance too much for your own good...

> The other way would be to:
> try:
>   name, extra = url[1:].split('/')
> except IndexError:
>   name = url[1:]
>   extra = None
> 
> but I probably prefer the "if '/' in url".
> 
> I suppose I wasn't clear on the portion that I was objecting to.

No you weren't :)

> BB:tweak
> 
> This isn't critical, and whoever merges it can chose if they want to
> change it.

Sounds good to me, doesn't seem worth generating a new bundle for.

Cheers,
mwh



More information about the bazaar mailing list