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
description.
> 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
changed).
* 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