Philip Martin <philip@codematters.co.uk> writes:
> > + /* Sanity check. */
> > + if ((start->kind == svn_client_revision_unspecified)
> > + || (end->kind == svn_client_revision_unspecified))
> > + {
> > + return svn_error_create
> > + (SVN_ERR_CLIENT_BAD_REVISION, 0, NULL, pool,
> > + "svn_client_diff: caller failed to specify any revisions");
> > + }
>
> This is why I had second thoughts about adding unspecified to the
> client interface. Using unspecified means that every client function
> needs this sort of check, and yet in practice the application will
> never pass in such a revision. Typical application pseudo-code
Not really. Most client code can fall through to the default
"unrecognized revision kind" case, that is, the final `else' in the
chain. Only functions that promise a particular behavior in the case
of `unspecified' (a behavior different from the unrecognized case)
need to have an explicit check.
Whether the above code actually does that I don't know, it's all
ripped up now anyway probably. There is at least one function in my
patch as of right now that promises different behaviors for the two,
however: svn_client__get_revision() will return SVN_INVALID_REVNUM for
`unspecified', but error outright on an unrecognized kind. That seems
like the Right Thing for the internal interface, though I'd be hard
pressed to articulate why. :-)
> Diff is awkward because at present it can take unspecified revisions,
> however I would be happy to see code like
>
> if start_revision is unspecified and end_revision is not unspecified
> start_revision = CURRENT
>
> if start_revision is unspecified and end_revision is unspecified
> svn_diff_text_base_to_wc()
>
> else if end_revision is unspecified
> svn_diff_repos_to_wc( start_revision )
>
> else
> svn_diff_repos_to_repos( start_revision, end_revision )
>
> in the application, so that diff doesn't require unspecified
> revisions.
The svn_client_diff() interface doesn't do anything useful with
`unspecified' revisions, but it promises to error on them as on
unrecognized kinds.
I don't think having `unspecified' directly in the libsvn_client type
actually results in more code, in practice, for the reasons given
above. But it does allow the client library to handle `unspecified'
explicitly in certain cases, which may result in smaller code in the
calling client. So I do think it's better this way.
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Oct 21 14:37:01 2006