[MERGE] Make command discovery entirely callback driven.

[Robert Collins]

On Mon, 2009-06-15 at 15:58 +1000, Ian Clatworthy wrote:
> 
> > Well there are two things; firstly extend_command still applies to
> > missing commands, which I think is useful as it allows for UI
> decoration
> > to still apply. Secondly, if a missing command is found, then we
> don't
> > show the 'no such command' error - and thats important.
> 
> Sure. But there's no reason I can see for a plugin to return a command
> via get_missing_command() when it can return one via get_command().
> It's
> not a big deal and I certainly won't block on having both - I just
> can't
> see why you need get_missing_command() in it's current form.

install the 'cmd-heuristic' plugin (hypothetical, its on my TODO).
bzr commti
get_command:
 every plugin passes
get_missing_command:
 the plugin-lookup fails
 the cmd-heuristic lookup does a list_commands and then calculates a
  string distance for all commands against 'commti', decides that commit
  is close enough and returns that command object.

If you just have get_command and get_missing_command raising, then the
order in which lookups are done matters because the heuristic check I'm
mentioning would have to run *after* all other lookups and *before* the
missing command hook, or else it would mask some plugins, or never run.

> >> 3. Maybe we need to explicitly say that there's no need to use
> these
> >>    (new) hooks when simply registering new commands in plugins.
> > 
> > It may be very efficient to use these directly from plugins. I'm ok
> with
> > putting some text about this, but I don't want to prejudge what
> > should/shouldn't be done. Are you worried that folk will see these
> hooks
> > and use them rather than the current interface?
> 
> Yes.

I don't see that anything would be wrong with people using this
interface directly. So I'm inclined not to warn people off from doing
this.

> >>> + at deprecated_function(deprecated_in((1, 16, 0)))
> >>> +def get_all_cmds():
> >>> +    """Return canonical name and class for most commands.
> >>> +    
> >>> +    NB: This does not return all commands since the introduction
> of
> >>> +    command hooks, and returning the class is not sufficient to 
> >>> +    get correctly setup commands, which is why it is deprecated.
> >>> +
> >>> +    Use 'all_command_names' + 'get_cmd_object' instead.
> >>> +    """
> >>>      d = _builtin_commands()
> >>>      if plugins_override:
> >>>          d.update(plugin_cmds.iteritems())
> >>> -    return d
> >> get_all_cmds() has lost it's plugins_override parameter in your
> >> refactoring here.
> > 
> > Yes. This is deliberate - its not used in the codebase, 
> 
> Actually, it is used in the implementation of get_all_cmds() I
> reviewed.
> See above.

Huh?

$ grep get_all_cmds bzrlib/*.py
bzrlib/commands.py:187:def get_all_cmds():

remaining issues:
 - do we need to warn people off from using the hooks
 - what is up re: get_all_cmds
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20090615/ac994228/attachment.pgp