Overriding commands in plugins

[Martin Pool]

On 9 March 2010 19:56, Michael Gliwinski
<Michael.Gliwinski at henderson-group.com> wrote:
> On Tuesday 09 Mar 2010 06:42:45 Martin Pool wrote:
>> On 9 March 2010 04:06, Gary van der Merwe <garyvdm at gmail.com> wrote:
>> > On 08/03/2010 17:18, Michael Gliwinski wrote:
>> >> What is the best/proper/right/working way to override some functionality
>> >> of a command in a plugin?
>> >
>> > I looked into this in detail when trying to solve the incompatibility
>> > between qbzr and bzr-pipeline which were both trying to modify the merge
>> > command. I came to the conclusion that it's not possible to do cleanly
>> > with how bzr currently is. I started making changes to bzr to make this
>> > possible[1], but ran out of motivation, and qbzr ended up losing some
>> > functionality (merge --qpreview)
>> >
>> > [1]:
>> > https://code.launchpad.net/~garyvdm/bzr/register_lazy_decorated/+merge/84
>> >30
>>
>> The easiest way is to override the plugin altogether and super-call it.
>
> I'm not sure what you mean by that.

I meant to subclass the base implementation, then call super(...).run()

>
>> The best way is to (ask for help to) change the command so that it can
>> more easily be extended in different directions; this is the only
>> feasible way to accommodate different plugins all wanting to enhance
>> eg the merge command.
>
> Yes, I agree, e.g. callbacks at certain points of the operation would make
> things simple.  I believe bzrlib uses hooks for this kind of extensibility at
> lower levels (than commands), are you thinking of similar/same infrastructure
> for commands as well?

Yes, I think that would be reasonable.  We could add hooks run at the
start and end of commands, which would allow some types of extension.
But to do more interesting things you would want a hook that runs at
interesting points during the command.  And really many of these
probably don't want to be in the CLI in particular so much as at a
somewhat lower level.  For instance you would perhaps want your hook
to also run if the "switch" operation was run through some other UI.

> Also, there is the other use case (apart from extending a command) of
> replacing functionality depending on certain conditions, that's what e.g. loom
> does with switch command.
>
>> In your  example of printing a message before switching, we might add
>> a hook run before switching the working tree, or perhaps we should
>> make it always send a notification to the ui and you can tweak how
>> that is displayed.  Was that actually what you wanted or is there a
>> more real example?
>
> Yeah, that was just an example, but in reality it's not far from what I'm
> trying to accomplish.  Basically I need to run some code before the operation
> and depending on some conditions be able to abort it.
>
> My use case now is to have switch warn interactively if there are uncommited
> changes/unknown files.  I know it is perfectly valid for switch to just
> propagate these as it does now, but I have a lot of users here (most quite new
> to using VCS at all) who get bitten by this quite often (e.g. forgetting to
> run status often and after a couple of switches ending up with conflicts in
> conflict files, etc.).

I think this probably ought to be in a WorkingTree.pre_switch hook?

If other developers feel this would be useful I can help you add it.

> I noticed the command hooks yesterday, from what I understand they are
> actually run when the command object is created, yes?  Would that be valid to
> wrap the run method in a hook?

As Robert says, you could do it, but it seems a bit of a blunt instrument.

-- 
Martin <http://launchpad.net/~mbp/>