Karl Fogel <kfogel@newton.ch.collab.net> writes:
> Philip Martin <philip@codematters.co.uk> writes:
> > I may give it a go in the next few days, but if you want to do it I
> > have no strong attachment to the task :)
>
> I'm not likely to do it before Monday, so if you start it before then,
> it's all yours. :-) Otherwise, I probably will start it then.
I'm not going to do before then, so it's all yours. I have thought a
bit more about the problem, so I'll add some further comments.
My original idea was for a libsvn_client interface (the mail included
an svn_cl__ prototype but that was a typo), however I would now use
something like:
enum svn_client_revision_type {
svn_client_revision_number, /* -r NUMBER */
svn_client_revision_date, /* -D DATE */
svn_client_revision_commited, /* .svn/entries commited-rev */
svn_client_revision_previous, /* .svn/entries commited-rev-1 ??? */
svn_client_revision_current, /* .svn/entries revision */
svn_client_revision_head /* -r HEAD, RA youngest */
};
typedef struct svn_client_revision_t {
enum svn_client_revision_type type;
union {
svn_revnum_t number;
apr_time_t date;
} value;
} svn_client_revision_t;
svn_error_t *
svn_client__get_revision_number (svn_ra_plugin_t *ra_lib,
void *session,
svn_client_revision_t *revision,
const svn_stringbuf_t *path,
svn_revnum_t *number,
apr_pool_t *pool);
struct svn_cl__revision_t {
svn_client_revision_t revision;
svn_boolean_t specified;
};
Originally, I introduced svn_client_revision_unspecified in
svn_client_revision_type because svn_client_diff needs to know whether
there was a -r on the command line. This is ugly. I now think that the
svn_client_revision_type should not have this setting, unspecified
revisions should be limited to the application.
The function svn_client_diff should be split into three functions,
svn_client_diff_text_base_to_wc with no revision parameters,
svn_client_diff_repository_to_wc with one revision parameter, and
svn_client_diff_repository_to_repository with two revision parameters,
and svn_cl__diff should decide which one to call based on the number
of revisions specified on the command line.
The advantage of doing this is that the client functions do not need
to deal with unspecified revisions. It also solves the current diff
problem, where if -DDATE1:DATE2 has two dates that resolve to the same
revision then diff compares the revision against the wc which is
wrong.
Next, svn_client_revision_commited and svn_client_revision_current
(and svn_client_revision_previous if included) are a little tricky. I
was assuming that svn_client__get_revision_number would extract the
relevant revision from the entries file, and then that revision number
would be used as if specified explicitly on the command line. When
operating recursively this would mean that "current" referred to the
directory and not to the elements within which may have different
current revision numbers. Personally I would find this acceptable, but
it would need to be made clear to the users.
Finally, a couple of minor points:
The application should reject attempts to specify two revisions for
commands that only accept one, 'svn -r1:2 up' should fail for example.
It should probably also reject -rREV:REV a repeated revision.
The application currently initialises start_revision to
SVN_INVALID_REVNUM and end_revision to 1. In the case of the log
command I see no reason for the HEAD:1 default, HEAD:0 would be
better.
--
Philip
---------------------------------------------------------------------
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:00 2006