Merge lp:~fullermd/bzr/revspec-dwim into lp:bzr

This allows unqualified revspecs to be tried as revnos, tags, revids, dates, or branches (respectively) rather than being assumed to be revnos.

I agree in principle with this and the code looks ok to me. There are 2 main concerns I have:

1. If something looks like a revno and isn't, I'd prefer that we raised an error in that case rather than continuing to look. The main issue here is tags like x.y.z I guess. I'd prefer we err'ed on the side of safety. It's not a big deal for projects to use a more explicit tag (like release-x.y.z) or for the user to type tag:x.y.z if they choose not to.

2. The time to look up a revision-id needs discussion. I'd prefer if it was done last. (If that's a performance issue, I'd prefer it was done immediately after revno but before the other specifiers.)

After discussing this on IRC with a few people, I'm ok to proceed with this as is. Matthew has pinged the list just in case others have strong opinions. We ought to give that a day or two to see what feedback, if any, there is.

 review needs-fixing

(however thats said now :()

This manually reproduces the registry of revspecs. Lets not do that,
instead query it dynamically.

If an ordering is needed, see the transport registry which has some
sorting done to it.

-Rob

On Thu, Oct 15, 2009 at 05:33:12AM -0000 I heard the voice of
Robert Collins, and lo! it spake thus:
>
> This manually reproduces the registry of revspecs. Lets not do that,
> instead query it dynamically.
>
> If an ordering is needed, see the transport registry which has some
> sorting done to it.

I disagree.

Leaving aside the very large practical difficulties in doing it out of
the registry (and there are several), I consider it the wrong
approach. This isn't RevisionSpec_tryeverything, it's
RevisionSpec_dwim. An important part of a non-screwyou DWIM system is
carefully limiting to scope, and precisely defining the order within
that.

In this case, the dwim tries just half (rounded) of the revspecs we
have, because trying the rest makes no sense at all (e.g., if we don't
match it somewhere else, trying to match it to 'before:' would be
nonsensical, and in the bizarre case it actually DID something, it's
sure to be Wrong).

--
Matthew Fuller
<email address hidden>

I disagree with your disagreement :)

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

You can add this as a future enchancement if you like, but I think the
code will be smaller and more compact if you go straight to this.

-Rob

> 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>