RFC: alias and unalias command

[John Arbash Meinel]

Tim Penhey wrote:
> I'm the sort of person that has aliases for things that I perform
> often.  I have a number of shell aliases, and several bzr aliases.
> 
> Every now and then I thought it would be good to have bzr tell
> me what those aliases were without me having to go and read
> ~/.bazaar/bazaar.conf.  So I wrote an `alias` and `unalias` command
> that operates in a similar way to the shell alias.
> 
> It produces output like: 
> 
> tim at slacko:~/src/bzr/alias-command$ ./bzr alias
> bzr alias cbranch='cbranch --lightweight'
> bzr alias col='checkout --lightweight'
> bzr alias lastlog='log -r-2..-1'
> bzr alias ll='log --line -r-10..-1'
> tim at slacko:~/src/bzr/alias-command$ ./bzr alias ll
> bzr alias ll='log --line -r-10..-1'
> tim at slacko:~/src/bzr/alias-command$ ./bzr alias foo
> bzr alias foo: not found
> tim at slacko:~/src/bzr/alias-command$ ./bzr alias foo='log --line -r-5..-1'
> tim at slacko:~/src/bzr/alias-command$ ./bzr foo
> 2903: Tim Penhey 2008-02-04 Wrong argument passed to unalias.
> 2902: Tim Penhey 2008-01-20 A working alias command.
> 2901: Tim Penhey 2008-01-20 Removed some annoying trailing whitespace.
> 2900: Canonical.com Pat... 2007-10-09 (Andrew Bennetts) Fix bug 146715: bzr+ssh:// and sftp:// should not assume port-not-specified...
> 2899: Canonical.com Pat... 2007-10-09 Fix incorrect comparison in Dirstate.set_state_from_inventory, and bug #146176
> tim at slacko:~/src/bzr/alias-command$ ./bzr alias foo
> bzr alias foo='log --line -r-5..-1'
> tim at slacko:~/src/bzr/alias-command$ ./bzr unalias fubar
> bzr unalias: fubar: not found
> tim at slacko:~/src/bzr/alias-command$ ./bzr unalias foo
> tim at slacko:~/src/bzr/alias-command$
> 
> The attached merge directive contains the changes so far,
> but I'm not sure the best way to test it.
> 
> If I can get some concrete suggestions, I don't mind writing the tests,
> or even better, feel free to write some for me :-)
> 
> Comments?
> 
> Tim
> 

A couple comments:

1) It seems interesting. If people find it useful, I guess it doesn't 
hurt. There is always the "command bloat" issue (which might hint that 
"bzr alias --remove" would be a bit better, though people used to shell 
will be used to 'unalias'.)

2) We generally don't fix up trailing whitespace unless it is near the 
code we are changing:
=== modified file 'bzrlib/config.py'
--- bzrlib/config.py	2007-10-05 14:52:02 +0000
+++ bzrlib/config.py	2008-01-20 05:06:12 +0000
@@ -221,16 +221,16 @@

      def username(self):
          """Return email-style username.
-
+
          Something similar to 'Martin Pool <mbp at sourcefrog.net>'
-
+


You weren't changing anything else here. We chose this as a compromise. 
We would like to clean things up, but it introduces lots of cases for 
things to 'spuriously' conflict.

I personally find the "vimdiff" plugin a good way to "re-introduce" the 
trailing whitespace when I get overzealous.

3) I understand why you are using "bzr alias foo=bar" but I would sort 
of rather see "bzr alias foo bar". This is certainly open for debate.

4) Would this be better as a plugin? Just to keep the core commands low.

5) As for tests....

I would like to see blackbox smoke tests for each command. blackbox 
tests are meant to exercise only the "cmd_foo" class, so a test for: 
"bzr alias" no args, "bzr alias foo", and "bzr alias foo=bar" and 
similarly "bzr unalias foo" when foo does and doesn't exist.

Then the rest should be "whitebox" tests in bzrlib/tests/test_config, 
which test that you can set an alias, and then get it back from the 
config. (Probably should be re-opening the config to make sure it is 
serialized properly.)
And you should obviously be able to remove an entry. Probably should 
also test that it doesn't destroy another entry which also exists.

John
=:->