Code review comment for lp:~fullermd/bzr/revspec-dwim

Revision history for this message
Matthew Fuller (fullermd) wrote :

> specifically, git:, hg:, svn:, and thread: specs are added by
> plugins and desirable to search too.

OK, so it sounds like there're actually two separate issues being a
little conflated here:

1) Make the DWIM use a registry instead of hand-written code.

   I'm not necessarily opposed to that; certainly IWBNI plugins could
   register their own specs DWIM should try. But I don't think I'll
   be doing it (certainly not anytime soon; $freetime--). And it's
   not quite as easy as it sounds, since we have to keep track of
   which exceptions each class raises for 'no, I didn't find this' (as
   opposed to 'crap, something went wrong').

   Also, instantiating the RevSpec_* classes _doesn't_ determine
   whether the given value points at anything, it only sets up the
   object to lazily search later. We reach inside them in the _dwim
   class to evaluate on the spot (when _dwim is lazily searched) to
   find out whether the value means anything to that spectype.

   So it's not just as simple as "do this then this then this";
   encoding that in a registry will take more knowledge of what you
   can do with python than I have. As it is, the body of the dwim
   _match_on() is about 40 lines (and with my characteristic heavy use
   of whitespace and commenting, so the actual code is probably under
   25), and explicit about the complications and what's going on.

2) Have that registry be the same as the revspec_registry registry
   that the prefix:'s are searched in.

   This I would be against, because 'prefixes we recognize' and
   'things we want to DWIM' are rather different, and cramming all of
   the above info, plus ordering, plus info on which specs we even
   DWIM at all, into that registry is a giant mess of making one thing
   do two almost-totally-different things. This is mostly what I was
   conceptually arguing against in the previous message, not so much
   (1) above.

--
Matthew Fuller
<email address hidden>

« Back to merge proposal