Re: [PATCH] add limit argument to svn_repos_history

Daniel Rall wrote:
> So, nor are you in favor of adding them to functions which don't take them?

[Trying to be careful with negatives here]

I am not opposed to adding cancel_funcs to functions lacking them.

I see the purposes of the two types of cancellation as meaningfully
different here.

The cancel_func is intended to be a mechanism for interrupting an operation
at a somewhat-arbitrary-yet-safe location, is typically hooked up to some
user/UI action (Control-C, or clicking "Cancel" on the TSVN dialog, or...),
and propogates up the stack toward program exit as an error code. Because
of this, functions in the stack need only worry that they don't corrupt the
data they are working with, but are otherwise free to bounce the error up
the stack. The error is, ultimately, intended to stop "an operation".

Allowing a callback function to return a non-fatal code which indicates that
it has programatically finished doing what it needs to do is a little
different, generally isn't tied to a user's action (though might, as we see
with --limit, be tied to user-specified constraints), and doesn't propogate
as an error.

Because of these differences, I strongly advise that we create a new error
code for this type of callback response, and document which callback driver
functions recognize the response. SVN_ERR_MY_CUP_RUNNETH_OVER, anyone? ;-)

> I'm attaching a patch implementing your suggestion for
> svn_repos_history2(), along with a new 'svnlook history --limit'
> option. Note that this doesn't rev the underlying API, but does change
> its behavior WRT the SVN_ERR_CANCELLED error code.

And that patch is a wonderful example of what I was talking about doing with
this concept. (Though, again, I'd use a different error code.)

-- 
C. Michael Pilato <cmpilato@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand