[RFC][PATCH] support for aliases in bazaar.conf

[John Arbash Meinel]

Erik Bågfors wrote:
> 2006/2/12, John A Meinel <john at arbash-meinel.com>:
>> Erik Bågfors wrote:
>>> 2006/2/11, Aaron Bentley <aaron.bentley at utoronto.ca>:
>>>> -----BEGIN PGP SIGNED MESSAGE-----
>>>> Hash: SHA1
>>>>
>>>> Martin Pool wrote:
>>>>
>>>> | If we did this, it would seem to be a proper superset of what you can do
>>>> | with default options.  Aaron, would you be OK to use this instead?
>>>>
>>>> Yes, as long as overrides still work.  e.g. if I have log aliased to
>>>> 'log -r -10..-1', I should still be about to specify 'log -r 5'.
>>>>
>>> This should work now.  You can even have the alias
>>> ll=log --line -r-5..-1
>>> and do
>>> ll -r10
>>> which will turn it into log --line -r10
>>>
>>> There are tests to prove it :)
>>>
>>> Take a look at don't be affraid to criticize me. I never used python before bzr.
>>>
>>> If you guys want me to rewrite anything that's fine also.  Mail me off
>>> list if you don't want to clutter the list.
>>>
>>> branch at the same place.
>>>
>>> Regards,
>>> Erik
>>>
>> You need a double space before TestAliases in test_aliases.py
> 
> Double space? I'm trying to look through the other test_*py files to
> understand what you mean, but I'm lost.

PEP8 says that there should be 2 blank lines between top level items. So
you end up with:

import os

import bzrlib # One line between stdlib includes, and local includes


class Foo: # 2 lines between import and class

  def something(self):
    return stuff

  def something_else(self): # One line between class items
    return other


def function(): # 2 spaces
  return x


def other(): # Another 2 spaces
  return y


> 
>> Instead of doing a double loop over alias_argv and then argv, why not do
>> a single loop with
>>
>> for arg in alias_argv + argv:
>>
>> It also makes your patch smaller, since it doesn't looked like you
>> changed all the lines that you just indented.
> 
> Well, that was my first thought as well, but I need to know if I'm in
> "alias_argv" or in "argv" to get overrides to work.
> 
> After I have gone trough alias_argv I set
> proc_aliasarg = False # Done with alias argv

You need this because you only allow overriding between them? If so, I
guess that is good enough.

By the way, does this allow you to do:

selftest=selftest --no-plugins

That would be really nice, but I wouldn't expect it. Since probably you
parse aliases long after parsing globals.

> 
> I don't know how to do that if I just concatenate them together.
> 
>> get_alias() needs 2 blank lines before and after it.
> 
> Ok,
> 
>> Ultimately, I think some of your changes will be superceeded by using
>> optparse, but in the mean time, it is probably useful.
> 
> Absolutely, I realized that as well. I'm fine with that.
> 
> Regards,
> Erik
> 

So with the whitespace cleanups, +1 from me.
John
=:->

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 249 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060212/29f48d36/attachment.pgp