Re: Philip, question about svn_client_diff()

Karl Fogel <kfogel@newton.ch.collab.net> writes:

> I noticed this code, which you obviously mean to come back to:
>
> /* ### TODO: awful revision handling */
> if (!path_is_url
> && !SVN_IS_VALID_REVNUM(start_revision) && end_revision == 1
> && !start_date && !end_date)
> {

It matches the magic numbers in subversion/clients/cmdline/main.c

  apr_initialize ();
  pool = svn_pool_create (NULL);
  memset (&opt_state, 0, sizeof (opt_state));
  opt_state.start_revision = SVN_INVALID_REVNUM; /* default to youngest */
  opt_state.end_revision = 1; /* default to oldest */

  /* No args? Show usage. */
  if (argc <= 1)
    {

So those values mean that there was no -r command line option.

>
> Of course, I immediately asked myself why we were comparing
> end_revision with 1 :-). In order to understand it better, I went to
> the doc string for svn_client_diff() in svn_client.h. But it doesn't
> really clarify things:
>
> START_REVISION/START_DATE and END_REVISION/END_DATE are the two
> repository versions, for each specify either the revision of the
> date. If the two revisions are the different the two repository versions
> are compared. If the two revisions are the same the working copy is
> compared against the repository.
>
> There's a lot of ambiguity there. I can't remember if I wrote it or
> not, possibly I did :-).
>
> Anyway, I'd like to revamp and clarify that portion, to something like
> this:
>
> START_REVISION/START_DATE and END_REVISION/END_DATE are the two
> repository versions (a date is equivalent to whichever revision was
> current at that date). If the two revisions are different, those
> two repository versions are compared. If the two revisions are the
> same, the working file is compared against that repository
> revision. A revision of SVN_INVALID_REVNUM means the base revision
> of the working file.
>
> Does that seem right to you? Does it fail to cover any cases?
>
> It may or may not match the current behavior of the function. If it
> doesn't, we should change the function once we decide the behavior we
> want.
>
> (Of course, we'll shortly be adding two-path support, so the
> documentation will have to be adjusted again. But one thing at a
> time; let's get this straightened out first.)
>

I've been meaning to post a proposal about revision numbers. Quick
summary, the libsvn_client interface is ugly. Some functions take two
revisions and two dates (diff), others only take revisions (log). What
I think we need is based on a suggestion from Branko

struct svn_revision_t {
  enum svn_revision_type {
    svn_revision_unspecified,
    svn_revision_date,
    svn_revision_number,
    svn_revision_head,
    svn_revison_current,
    svn_revison_last
  } type;
  svn_revnum_t number;
  apr_time_t date;
};

With some functions/macros for access/manipulation. I'll finish and
post it.

-- 
Philip
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org