>
> > Index: subversion/include/svn_client.h
> > ===================================================================
> > --- subversion/include/svn_client.h (revision 15298)
> > +++ subversion/include/svn_client.h (working copy)
> > @@ -915,7 +915,8 @@
> > /**
> > * Invoke @a receiver with @a receiver_baton on each log message from @a
> > * start to @a end in turn, inclusive (but never invoke @a receiver on a
> > - * given log message more than once).
> > + * given log message more than once). @a peg_revision indicates in which
> > + * revision @a targets are valid.
>
> This needs to specify what svn_opt_revision_unspecified means as a peg revision ...
>
> > *
> > * @a targets contains either a URL followed by zero or more relative
> > * paths, or a list of working copy paths, as <tt>const char *</tt>,
> [...]
> > /**
> > + * Similar to svn_client_log3(), but with the @a peg_revision
> > + * parameter always set to @c svn_opt_revision_unspecified.
>
> ... if svn_client_log2() is documented in terms of it. I wasn't easily able to
> determine the meaning for myself.
>
I guess you are saying that, if an explanation of
svn_opt_revision_unspecified in the svn_client_log3() doc string is
included, then the svn_client_log2() doc string becomes easier to
understand. Will do.
> > + if (start->kind == svn_opt_revision_number &&
> > + end->kind == svn_opt_revision_number)
> > + revision = (start->value.number > end->value.number) ? *start : *end;
> > + else
> > + revision.kind = svn_opt_revision_unspecified;
> >
> > + SVN_ERR (svn_client__ra_session_from_path (&ra_session, &rev,
> > + &url_p, path,
> > + peg_revision, &revision, ctx,
> > + pool));
> > +
>
> Please could you explain this change. I don't understand why something special
> is happening if both the start and end revisions are specified as numbers
> rather than, for example, dates. And I don't really understand _what_ special
> behaviour is happening in that case.
>
> I see from the implementation of svn_client__ra_session_from_path() that an
> unspecified "revision" means "revision = peg_revision", so wouldn't it be
> clearer to make your "else" clause say "revision = *peg_revision" (or an
> equivalent pointer assignment) ?
I overlooked the fact that dates/keywords can also be specified as
revisions. I can change that to,
if (start->kind == svn_opt_revision_number) &&
(end->kind == svn_opt_revision_number)
revision = (start->value.number > end->value.number) ? *start : *end;
else if (start->kind == svn_opt_revision_date) &&
(end->kind == svn_opt_revision_date)
revision = (start->value.date > end->value.date) ? *start : *end;
Revision numbers can also be specified as a combination of dates and
revision numbers, as well as a combination of keywords (PREV, HEAD, etc),
numbers etc. In those cases the revision gets set to
svn_opt_revision_unspecified in the current patch(v2).
>
> The rest of the code looks fine. (I haven't studued the regression tests.)
>
I haven't added any new regression tests. Just modified existing ones
to work with the new syntax.
Thanks
Ramaswamy
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Aug 5 13:51:57 2005