[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