[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

[PATCH] Make revision range ends optional (was Re: [PATCH] Issue #2100: --revision ARG1:ARG2 handling improvement)

From: Ryan Kersh <rpk_at_cs.uoregon.edu>
Date: 2005-03-29 08:34:07 CEST

Julian Foad wrote:
> I don't recall any discussion of this issue on the mailing list; was
> there? My first thoughts are along the lines of: isn't this horribly
> inconsistent for some commands where the default is not 0:HEAD, e.g.
> "svn log" which defaults to BASE:1 ?

I wanted to become more acquainted with the code-base so I selected one
of the bite-sized tasks that looked interesting to me. I assumed that
some discussion of this issue had already taken place because it was in
the issue tracker. Sorry about that.

More on your second question below...

> Maybe I'm wrong. I can certainly see that it would be handy in some
> situations. List the uses of it in different Subversion commands,
> comparing to the present defaults, and convince me that there are no
> ugly contradictions or surprises, only goodness.

I did some black-box testing, and AFAICT, the only commands that accept
ARG1:ARG2 revision arguments are blame, diff, log, and merge. Of those,
only log has a non 0:HEAD default. The merge command requires the -r
option and has no default.

It just seems natural to me that if the revision range ends become
optional that we would use 0:HEAD, ascending from left to right, as the
default. If "svn log" defaults to BASE:1 and "svn log -r:" is 0:HEAD, I
think it would be okay, because the user would have to be pretty
sophisticated to use the -r option in the first place, and would know
what they were doing. I can see where you're coming from, but for me
the handiness of it outweighs the possible surprises it might cause.
You are probably a better judge of this... do you still think the 0:HEAD
default is bad?

>> Should I add a test to subversion/tests/libsvn_subr/opt-test.c to put
>> svn_opt_parse_revision through the paces as well?
> That would be very nice, though not essential.

I've included a new test in opt-test.c in the revised patch (attached).

> The description of this issue and this email thread and this patch could
> be more specific. Maybe "Make revision range ends optional"?

I like your description. I've updated the email thread and log message
accordingly. I don't think I have the permissions to change the issue

> A documentation patch to go with this would be really nice.

Do you mean the doxygen comment in svn_opt.h. If so, it's already done.
  Otherwise, I'd be happy to provide a documentation patch for anything
else if you point me in the right direction.

Here is the revised log message:

Implement Issue #2100: Make revision range ends optional.

* subversion/include/svn_opt.h
   (svn_opt_parse_revision): Fixed up doc string comment so that it
     reflects the enhancements to --revision ARG1:ARG2 handling.

* subversion/libsvn_subr/opt.c
   (parse_one_rev): Added 'else if' to handle ':' character.
   (svn_opt_parse_revision): Added 'if' statement to handle special
     case where arg is ':'. Also, reworked some code so that right_rev
     will get parsed correctly by parse_one_rev (now that it has

* subversion/tests/libsvn_subr/opt-test.c
   (test_parse_revision): New test.
   (svn_test_descriptor_t): Added test_parse_revision to list of tests.

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

Received on Tue Mar 29 08:35:47 2005

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.