Overriding commands in plugins

[Matt Nordhoff]

Michael Gliwinski wrote:
> On Monday 08 Mar 2010 15:34:52 Matt Nordhoff wrote:
>> Michael Gliwinski wrote:
>>> Hi all,
>>>
>>> What is the best/proper/right/working way to override some functionality
>>> of a command in a plugin?
>>>
>>> Let's say for example that I want to change 'switch' to print a nice
>>> message before it goes on doing its work :)
>>>
>>> From looking at some of the examples I did:
>>>
>>> class cmd_switch(bzrlib.builtins.cmd_switch):
>>>     __doc__ = bzrlib.builtins.cmd_switch.__doc__
>>>     def run(**kwargs):
>>>         self.outf.write('Hello World!'\n)
>>>         super(cmd_switch, self).run(**kwargs)
>>>
>>> but it seems to me that doesn't take into account the fact that switch
>>> may be overriden by other plugins.  So it seems the result of that may be
>>> a bit random, depending on which plugin is loaded first?
>>>
>>> Then I also saw this done in the loom plugin:
>>>
>>> class cmd_switch(bzrlib.builtins.cmd_switch):
>>>     def run_argv_aliases(self, argv, alias_argv=None):
>>>         try:
>>>             super(cmd_switch, self).run_argv_aliases(list(argv),
>>> alias_argv) except (errors.MustUseDecorated, errors.BzrOptionsError): if
>>> self._original_command is None:
>>>                 raise
>>>             self._original_command().run_argv_aliases(argv, alias_argv)
>>>
>>> so, IIUC this uses the original command (returned by
>>> bzrlib.commands.plugin_cmds.register) stored in _original_command
>>> attribute, tries to run the overriden command and in case of problems
>>> falls back to original command, yes?
>>>
>>> Is this the more "proper" way?  Does it have to be done in
>>> run_argv_aliases? I.e. if I'm only adding certain functionality it would
>>> be bit more convenient to do it in run.
>>>
>>> I'll try to update http://doc.bazaar.canonical.com/plugins/en/plugin-
>>> development.html#extending-an-existing-command if I can figure out
>>> something solid to add there.
>>>
>>> Thanks,
>>> Michael
>> I know of another way, using bzrlib.commands.get_command_object(), and I
>> /think/ it's the best way. As an example, this is what Loggerhead does:
>>
>>         import bzrlib.builtins
>>         from bzrlib.commands import get_cmd_object, register_command
>>         from bzrlib.option import Option
>>
>>         _original_command = get_cmd_object('serve')
>>
>>         class cmd_serve(bzrlib.builtins.cmd_serve):
>>             __doc__ = _original_command.__doc__
>>
>>             takes_options = _original_command.takes_options + [
>>                 Option('http', help=HELP)]
>>
>>             def run(self, *args, **kw):
>>                 if 'http' in kw:
>>                     # ... snip ...
>>                 else:
>>                     super(cmd_serve, self).run(*args, **kw)
>>
>> <http://bazaar.launchpad.net/~loggerhead-team/loggerhead/trunk-rich/annotat
>> e/head%3A/__init__.py#L91> (<http://xrl.us/bgxved>)
>>
> 
> OK, so from what I can see using get_cmd_object instead of return value of 
> register gives you support for command hooks and access to original command 
> even if your command is defined in a different module than where the command is 
> registered, sounds about right?
> 
> Still, in your run() you use `super(cmd_serve, self).run(...)', doesn't that 
> bypass any other classes that might have overriden cmd_serve (since you have 
> to inherit from builtin so super will use it't run(), yes?)
> 
> Thanks for the tip,
> Michael

Hmm, you're right, that's probably a problem...

Well, I didn't write that code originally, and it's only used in old
versions of bzr (recent versions have a registry so it's not necessary
to override the command), and it's only cmd_serve, so I don't care very
much. :-P
-- 
Matt Nordhoff