On Sat, Oct 2, 2010 at 7:00 PM, Peter Samuelson <peter_at_p12n.org> wrote:
>
> So, in 1.3 or 1.4 we gained '-c' as a convenience for specifying
> changesets as opposed to revision endpoints.
Hi Peter,
FWIW diff/merge gained -c in 1.4 and log got it in 1.5.
> Then in 1.5 we got the
> svn:mergeinfo property, which expresses ranges of changsets. I don't
> know who decided to use "-" instead of ":" as the range operator, but I
> do note that the 'svn diff' pretty-printer for svn:mergeinfo follows
> the same "-" convention.
>
> The following patch allows "-c" on the command line to use the same
> range syntax as svn:mergeinfo. Thus you can cut and paste those ranges
> from 'svn diff' output, into commands such as 'svn log' and 'svn merge'.
Your patch is against 1.6 right? Usually it's best to go against
trunk, since that is where the change will be made.
Unfortunately I can't apply this patch, I keep getting 'The chunk size
did not match the number of added/removed lines' error...But doing the
match everything looks correct. I'm using TortoiseMerge and I assume
it doesn't like the timestamp in the diff's header and rather expects
a revision number or "working copy" comment.
Regardless I'm +1 on the concept and the patch (assuming it passes the
trunk test suite).
> For example, if I look at a pending merge in my wc:
>
> $ svn diff -N
>
> Property changes on: .
> ___________________________________________________________________
> Modified: svn:mergeinfo
> Merged /dc/trunk:r13053-13055,13069-13071,13342,13389
>
> if I want to look at the log messages, I can now cut and paste the
> revision ranges:
>
> $ svn log -v ^/dc/trunk -c13053-13055,13069-13071,13342,13389
>
> whereas in the past I'd have to convert to -r syntax:
>
> $ svn log -v ^/dc/trunk -r13053:13055 -r13069:13071 -r13342 -r13389
>
> The same is true if I want to redo the merge elsewhere, but even more
> so, as the -r syntax would have entailed subtracting one from the
> starting point of each range.
>
> Is this, then, a worthwhile feature addition? I don't want to add
> syntax that nobody else wants. In particular, this patch highlights
> the existing inconsistency of ":" vs. "-" as range operators.
On Sat, Oct 2, 2010 at 9:34 PM, Peter Samuelson <peter_at_p12n.org> wrote:
> Hmmm, in tests/cmdline/log_tests.py XFail(log_chanage_range), I see
> Lieven thought -c might someday take ranges with ":" instead of "-".
> But since the output uses "-", I'd argue the cut-and-paste utility of
> sticking with that outweighs the consistency of ":". Unless perhaps we
> should support both, which would be trivial.
No objections to supporting both. Really all we are asking then is
that users remember that -c'X-Y' is inclusive of X and '-cX:Y' is
exclusive. Of course we may rapidly be approaching the point where -r
and -c both support ':' and '-' and effectively become the same
option.
Paul
> [[[
> Parse ranges in '-c' in the 'svn' client.
>
> * subversion/svn/main.c
> (main): Parse "-" in -c arguments. Also move three variables one
> scope inward.
>
> * subversion/svn/log-cmd.c
> (svn_cl__log): Don't assume -c ranges comprise exactly one revision.
> ]]]
>
> --- subversion/svn/main.c 2010-10-01 10:54:52.000000000 -0500
> +++ subversion/svn/main.c 2010-10-02 17:55:45.000000000 -0500
> @@ -1211,9 +1211,6 @@ main(int argc, const char *argv[])
> break;
> case 'c':
> {
> - char *end;
> - svn_revnum_t changeno;
> - svn_opt_revision_range_t *range;
> apr_array_header_t *change_revs =
> svn_cstring_split(opt_arg, ", \n\r\t\v", TRUE, pool);
>
> @@ -1227,6 +1224,9 @@ main(int argc, const char *argv[])
>
> for (i = 0; i < change_revs->nelts; i++)
> {
> + char *end;
> + svn_revnum_t changeno, changeno_end;
> + svn_opt_revision_range_t *range;
> const char *change_str =
> APR_ARRAY_IDX(change_revs, i, const char *);
>
> @@ -1236,7 +1236,14 @@ main(int argc, const char *argv[])
> ### "{DATE}" and the special words. */
> while (*change_str == 'r')
> change_str++;
> - changeno = strtol(change_str, &end, 10);
> + changeno = changeno_end = strtol(change_str, &end, 10);
> + if (end != change_str && *end == '-')
> + {
> + change_str = end+1;
> + while (change_str == 'r')
> + change_str++;
> + changeno_end = strtol(change_str, &end, 10);
> + }
> if (end == change_str || *end != '\0')
> {
> err = svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
> @@ -1259,12 +1266,14 @@ main(int argc, const char *argv[])
> if (changeno > 0)
> {
> range->start.value.number = changeno - 1;
> - range->end.value.number = changeno;
> + range->end.value.number = changeno_end;
> }
> else
> {
> changeno = -changeno;
> - range->start.value.number = changeno;
> + if (changeno_end < 0)
> + changeno_end = -changeno_end;
> + range->start.value.number = changeno_end;
> range->end.value.number = changeno - 1;
> }
> opt_state.used_change_arg = TRUE;
> --- subversion/svn/log-cmd.c 2010-10-01 10:54:52.000000000 -0500
> +++ subversion/svn/log-cmd.c 2010-10-02 17:55:45.000000000 -0500
> @@ -481,9 +481,9 @@ svn_cl__log(apr_getopt_t *os,
> range = APR_ARRAY_IDX(opt_state->revision_ranges, i,
> svn_opt_revision_range_t *);
> if (range->start.value.number < range->end.value.number)
> - range->start = range->end;
> + range->start.value.number++;
> else
> - range->end = range->start;
> + range->end.value.number++;
> }
> }
>
>
Received on 2010-10-04 17:28:51 CEST