You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Bart Robinson <lo...@pobox.com> on 2006/01/12 22:02:15 UTC

[PATCH] --reverse option to invert -c/--change

Hi,
Here is a patch to help flesh out the utility of the -c/--change
option.

Some background in this thread:
http://svn.haxx.se/dev/archive-2005-10/1324.shtml
(should I put that in the commit message?)

E.g.,
  svn merge -c 100 --reverse URL
same as
  svn merge -r100:99 URL

Do the tests look ok?  I'm not a python stud so I just basically
imitated what Dan Berlin did in r17054.

All tests pass aside from a utf8 one that was SKIP'd.

Thanks
-- bart

[[[
Add a --reverse option reverse the sense of the -c/--change option,
making it possible to back out changes with -c rather than -r.

* subversion/svn/cl.h
  (svn_cl__longopt_t): Add enum for '--reverse' option.

* subversion/svn/main.c
  (svn_cl__options): Add 'reverse' option.
  (svn_cl__cmd_table): Merge and diff can take 'reverse' argument.
  (main): Handle parsing for 'reverse'.

* subversion/tests/cmdline/diff_tests.py
  (diff_pure_repository_update_a_file): Modify test to also verify
  that '--reverse' may not be used with '-r', only '-c'.
  (diff_only_property_change): Modify test to also use '--reverse'
  with '-c'.

* subversion/tests/cmdline/merge_tests.py
  (merge_with_implicit_target_using_c_and_reverse): New test: a copy
  of merge_with_implicit_target that uses the '-c' and '--reverse'
  options.
  (test_list): Add the new test.
]]]

Re: [PATCH] --reverse option to invert -c/--change

Posted by Bart Robinson <lo...@pobox.com>.
I'll first list the options suggested (that I've noticed) so I
can refer to them.

1) Add --reverse to pair with -c N.

2) Use negative revisions with -c, like '-c -123'.

3) Add new option, such as -C/--rchange.

On 2006-1-17 Greg Hudson <gh...@MIT.EDU> wrote:
 > On Tue, 2006-01-17 at 14:23 +0100, Molle Bestefich wrote:
 > > The question about how to roll back a revision comes up at least once
 > > a month (or so) over on the tsvn lists.  (Which of course immediately
 > > results in someone pointing to the 'revert revision' menu option and
 > > the OP feeling stupid.. hee hee :-)).
 > 
 > Once a month isn't terribly often.  But there's a deeper question here
 > than the frequency of user rollback operations.

I agree that it isn't used often, but it is not altogether rare.
It gets more common with a "stable trunk" model and lots of
committers.

 > The forward -c option is both a convenience for common operations ("show
 > me what changed in revision N", "merge the change made in revision N")
 > and also a road towards a more precise command set, where we can talk
 > about revisions either as changes or as states.
 >
 > A reverse -c option is simply a convenience for an operation which may
 > be common ("roll back the change made in revision N").  It allows the
 > command set to talk about inverse-revisions as changes, but in my
 > opinion that's just muddying the waters.

I think about it the other way: *not* having a reverse -c option
is basically an incomplete implementation of the "change"
concept.

Having (forward) -c is more important as you say, but since (I
assume) it will become the preferred syntax, I think it would
make sense if the reverse operation was similar.

That said, using -r100:99 to back out a change isn't all *that*
different from using -c100 to put it in.  It isn't like other
systems that use an entirely different *concept* to back out a
change vs. putting it in.  So I don't want to overstate the
utility of the reverse -c idea.

 > The argument has been made that "svn merge -c -N etc." is more intuitive
 > than "svn merge -r N:N-1 etc.", and will result in fewer user questions
 > asking how to roll back a change.  I have great doubts; I think roughly
 > the same class of users will need to be taught how to roll back changes
 > either way.
 > 
 > As I originally said, I'm only -0 on this concept, not -1.  I think it's
 > adding complexity to the command syntax for insufficient gain, but not
 > so much that I'm going to haul out a veto.

Schemes (2) and (3) should not add any significant complexity or
maintenance burden unless I'm missing something.  (I tried a
negative arg to -c and it just gave me a warning that negative
revisions were not valid-- it didn't try to parse -123 as an
option or anything.)

Even though I wrote a patch for scheme (1), I don't think it is
the way to go (nor was it my original idea, I was just trying to
help get the party started).  It is unlikely --reverse will be
used for anything else besides -c, so it would pretty much be a
syntax wart.

-- bart

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

Re: [PATCH] --reverse option to invert -c/--change

Posted by Greg Hudson <gh...@MIT.EDU>.
On Tue, 2006-01-17 at 14:23 +0100, Molle Bestefich wrote:
> The question about how to roll back a revision comes up at least once
> a month (or so) over on the tsvn lists.  (Which of course immediately
> results in someone pointing to the 'revert revision' menu option and
> the OP feeling stupid.. hee hee :-)).

Once a month isn't terribly often.  But there's a deeper question here
than the frequency of user rollback operations.

The forward -c option is both a convenience for common operations ("show
me what changed in revision N", "merge the change made in revision N")
and also a road towards a more precise command set, where we can talk
about revisions either as changes or as states.

A reverse -c option is simply a convenience for an operation which may
be common ("roll back the change made in revision N").  It allows the
command set to talk about inverse-revisions as changes, but in my
opinion that's just muddying the waters.

The argument has been made that "svn merge -c -N etc." is more intuitive
than "svn merge -r N:N-1 etc.", and will result in fewer user questions
asking how to roll back a change.  I have great doubts; I think roughly
the same class of users will need to be taught how to roll back changes
either way.

As I originally said, I'm only -0 on this concept, not -1.  I think it's
adding complexity to the command syntax for insufficient gain, but not
so much that I'm going to haul out a veto.


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

Re: [PATCH] --reverse option to invert -c/--change

Posted by Molle Bestefich <mo...@gmail.com>.
Brian W. Fitzpatrick wrote:
> Greg Hudson wrote:
>>>> (Yeah, I know, "svn merge -c N --reverse" to roll back a change.  I
>>>> don't think rollbacks are all that common.)
>> [...]
>>> I think that if we made it easier ('-c REV' is the first step in that
>>> direction, '-c NEGATIVE_REV' is another huge step in that direction),
>>> then we'd see people rolling back very often.
>>
>> I disagree that there's a frequent call for rollback, but we can't
>> settle that on the dev list.
>
> Agreed.

What?  No..
The question about how to roll back a revision comes up at least once
a month (or so) over on the tsvn lists.  (Which of course immediately
results in someone pointing to the 'revert revision' menu option and
the OP feeling stupid.. hee hee :-)).

This week, my personal count is 1 multi-revision revert, 1
one-revision revert and 2 partial (single file) reverts.

(None of those happened to be my own code, just in case you're
wondering whether I'm some sort of really lousy developer, humn...)

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


Re: [PATCH] --reverse option to invert -c/--change

Posted by Daniel Rall <dl...@collab.net>.
On Fri, 13 Jan 2006, Brian W. Fitzpatrick wrote:

> 
> On Jan 13, 2006, at 2:36 PM, Greg Hudson wrote:
> 
> >On Fri, 2006-01-13 at 14:23 -0600, Brian W. Fitzpatrick wrote:
> >>>(Yeah, I know, "svn merge -c N --reverse" to roll back a change.  I
> >>>don't think rollbacks are all that common.)
> >[...]
> >>I think that if we made it easier ('-c REV' is the first step in that
> >>direction, '-c NEGATIVE_REV' is another huge step in that direction),
> >>then we'd see people rolling back very often.
> >
> >I disagree that there's a frequent call for rollback, but we can't
> >settle that on the dev list.
> 
> Agreed.
> 
> >>BTW, one use case for rolling back is the desire to commit something
> >>that I wrote but don't want to use in my code *at this moment*: I
> >>commit the patch with an appropriate log message, then I roll it back
> >>right away.
> >
> >That may be a use case, but it doesn't seem like a terribly good one.
> >"svn cp . branch-url" is the preferred way of recording changes  
> >without
> >corrupting the mainline.
> 
> Fair enough.  But I still think that the "I screwed up my last commit  
> and want to revert it" use case stands.

Indeed, the "revert revision N" is definitely a valid use case (my
personal count is at twice this week, which IMHO is on the high side).
However, it occurs much less often than the "merge revision N for
backport" use case (I'm at four or five this week, which is fairly
typical), so while valid, it's not necessarily compelling enough to
warrant a redundant-though-slightly-quicker option for the
command-line client, even one with as small a footprint as the style
suggested by Fitz.
-- 

Daniel Rall

Re: [PATCH] --reverse option to invert -c/--change

Posted by "Brian W. Fitzpatrick" <fi...@red-bean.com>.
On Jan 13, 2006, at 2:36 PM, Greg Hudson wrote:

> On Fri, 2006-01-13 at 14:23 -0600, Brian W. Fitzpatrick wrote:
>>> (Yeah, I know, "svn merge -c N --reverse" to roll back a change.  I
>>> don't think rollbacks are all that common.)
> [...]
>> I think that if we made it easier ('-c REV' is the first step in that
>> direction, '-c NEGATIVE_REV' is another huge step in that direction),
>> then we'd see people rolling back very often.
>
> I disagree that there's a frequent call for rollback, but we can't
> settle that on the dev list.

Agreed.

>> BTW, one use case for rolling back is the desire to commit something
>> that I wrote but don't want to use in my code *at this moment*: I
>> commit the patch with an appropriate log message, then I roll it back
>> right away.
>
> That may be a use case, but it doesn't seem like a terribly good one.
> "svn cp . branch-url" is the preferred way of recording changes  
> without
> corrupting the mainline.

Fair enough.  But I still think that the "I screwed up my last commit  
and want to revert it" use case stands.

> If we ever get a highly-intelligent merge algorithm which supports
> convergence, this kind of "commit and then immediately revert"  
> operation
> may confuse it.  One of the problems intelligent merge operations have
> is trying to figure out why you reverted a change: did you just not  
> want
> the change *yet*, or did you not want it *at all*?  So it can have
> trouble figuring out what to do with the graph:
>
>   A --> B --> A
>     --> B
>
> If you reverted B in the first line because you didn't want the change
> yet, then you want the result to be B.  If you reverted it because you
> decided the change was a bad idea, then you want the result to be A.

Hm.  I wonder what p4 and clearcase do in this situation... anyone?

-Fitz

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

Re: [PATCH] --reverse option to invert -c/--change

Posted by Greg Hudson <gh...@MIT.EDU>.
On Fri, 2006-01-13 at 14:23 -0600, Brian W. Fitzpatrick wrote:
> > (Yeah, I know, "svn merge -c N --reverse" to roll back a change.  I
> > don't think rollbacks are all that common.)
[...]
> I think that if we made it easier ('-c REV' is the first step in that  
> direction, '-c NEGATIVE_REV' is another huge step in that direction),  
> then we'd see people rolling back very often.

I disagree that there's a frequent call for rollback, but we can't
settle that on the dev list.

> BTW, one use case for rolling back is the desire to commit something  
> that I wrote but don't want to use in my code *at this moment*: I  
> commit the patch with an appropriate log message, then I roll it back  
> right away.

That may be a use case, but it doesn't seem like a terribly good one.
"svn cp . branch-url" is the preferred way of recording changes without
corrupting the mainline.

If we ever get a highly-intelligent merge algorithm which supports
convergence, this kind of "commit and then immediately revert" operation
may confuse it.  One of the problems intelligent merge operations have
is trying to figure out why you reverted a change: did you just not want
the change *yet*, or did you not want it *at all*?  So it can have
trouble figuring out what to do with the graph:

  A --> B --> A
    --> B

If you reverted B in the first line because you didn't want the change
yet, then you want the result to be B.  If you reverted it because you
decided the change was a bad idea, then you want the result to be A.


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

Re: [PATCH] --reverse option to invert -c/--change

Posted by "Brian W. Fitzpatrick" <fi...@red-bean.com>.
On Jan 13, 2006, at 12:06 PM, Greg Hudson wrote:

> On Fri, 2006-01-13 at 16:37 +0000, Julian Foad wrote:
>> It seems to me fairly certain that we want this ability to reverse  
>> the
>> direction of a change.
>
> For the record, I'm -0 on it.  It feels like unnecessary syntactic
> fluff.  The -c option itself makes sense because it's common to  
> want to
> see the change made in a given revision, but I don't think it's common
> to want to see the inverse of that change, and you can get the inverse
> with svn diff -r N:N-1.
>
> Rephrased: it's good to have shorthand for talking about revisions as
> changes, but I don't see the point in having shorthand for talking  
> about
> revisions as inverse-changes.
>
> (Yeah, I know, "svn merge -c N --reverse" to roll back a change.  I
> don't think rollbacks are all that common.)

In my experience, they're not all that common just because it's  
somewhat of a PITA for people (read: people who don't write version  
control systems) to remember the syntax and get it right.

I think that if we made it easier ('-c REV' is the first step in that  
direction, '-c NEGATIVE_REV' is another huge step in that direction),  
then we'd see people rolling back very often.

BTW, one use case for rolling back is the desire to commit something  
that I wrote but don't want to use in my code *at this moment*: I  
commit the patch with an appropriate log message, then I roll it back  
right away.

-Fitz

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

Re: [PATCH] --reverse option to invert -c/--change

Posted by Greg Hudson <gh...@MIT.EDU>.
On Fri, 2006-01-13 at 16:37 +0000, Julian Foad wrote:
> It seems to me fairly certain that we want this ability to reverse the 
> direction of a change.

For the record, I'm -0 on it.  It feels like unnecessary syntactic
fluff.  The -c option itself makes sense because it's common to want to
see the change made in a given revision, but I don't think it's common
to want to see the inverse of that change, and you can get the inverse
with svn diff -r N:N-1.

Rephrased: it's good to have shorthand for talking about revisions as
changes, but I don't see the point in having shorthand for talking about
revisions as inverse-changes.

(Yeah, I know, "svn merge -c N --reverse" to roll back a change.  I
don't think rollbacks are all that common.)


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

Re: [PATCH] --reverse option to invert -c/--change

Posted by Malcolm Rowe <ma...@farside.org.uk>.
On Fri, Jan 13, 2006 at 04:37:03PM +0000, Julian Foad wrote:
> Do we want the reversal switch to apply to "-r" as well?  That is not 
> necessary in the basic cases (such as "-r X:Y") but we might decide it is 
> convenient and consistent.  Indeed it might enable some desirable diffs 
> that are not possible at present: like the reverse of a plain diff (WC 
> against BASE -> BASE against WC).  (Am I right about that?  If so, a better 
> solution to that particular problem might be to introduce a "WORKING" or 
> "WC" revision-keyword.)
> 

It would allow us to ask for both a WORKING->BASE diff, which currently
isn't supported by libsvn_wc at all, and for a WORKING->repos diff, which
is 'supported' but extremely buggy (due primarily to its non-testability).

I'm not really keen on the idea of a --reverse switch anyway: -c is
already syntactic sugar for something else (-rN-1:N), but that something
else is common enough that I think it's a good idea, whereas I'm not
sure that the reverse idea is common enough to warrant it.

I've also been thinking lately about whether a WORKING keyword would
make sense, and I kind-of like the idea, though I'm not sure whether
I've thought through all the consequences.

Regards,
Malcolm

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

Re: [PATCH] --reverse option to invert -c/--change

Posted by Bart Robinson <lo...@pobox.com>.
On 2006-1-13 Julian Foad <ju...@btopenworld.com> wrote:
 > Bart Robinson wrote:
 > > Hi,
 > > Here is a patch to help flesh out the utility of the -c/--change
 > > option.
 > > 
 > > Some background in this thread:
 > > http://svn.haxx.se/dev/archive-2005-10/1324.shtml
 > > (should I put that in the commit message?)
 > > 
 > > E.g.,
 > >   svn merge -c 100 --reverse URL
 > > same as
 > >   svn merge -r100:99 URL
 > 
 > It seems to me fairly certain that we want this ability to reverse the 
 > direction of a change.
 >
 > "--reverse" is perhaps too generic a name for the option: we
 > might well want to reverse other things than the direction of
 > a change, and it seems unlikely that it will make sense to
 > share a common option name for reversing completely different
 > things.  We should consider a more specific option name like
 > "--reverse-change".

I couldn't come up with ideas for any other options that
--reverse could modify, so it does feel too general to me as
well.

Actually, it *seems* more general to have this modifier idea,
but it could actually be seen as more restrictive: If there were
other reversible options and you wanted to use more than one in
one command, you wouldn't be able to reverse the sense of just
one of them (unless --reverse applied only to the arg next to
it, but that conflicts with svn's idea of 'put args in any
order' and would complicate arg parsing).

 > If the switch will only apply to "-c" then we could make it
 > an alternative to "-c" rather than an addition to it:
 > e.g. "--reverse-change REVNUM" (possibly with a short name
 > "-C REVNUM").

I agree that something else could be better, however
--reverse-change seems confusing when compared with --change
since one uses "change" as a verb/operation and the other like a
noun.  Maybe --rchange/-C and --change/-c?  Or make them both
verbs like --reverse-change and --use/add/apply-change (that
makes sense with "merge", which actually does stuff to your WC,
but is less natural with "diff", which just produces output).

-- bart

 > Do we want the reversal switch to apply to "-r" as well?
 > That is not necessary in the basic cases (such as "-r X:Y")
 > but we might decide it is convenient and consistent.  Indeed
 > it might enable some desirable diffs that are not possible at
 > present: like the reverse of a plain diff (WC against BASE ->
 > BASE against WC).  (Am I right about that?  If so, a better
 > solution to that particular problem might be to introduce a
 > "WORKING" or "WC" revision-keyword.)
 > 
 > Thoughts?

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

Re: [PATCH] --reverse option to invert -c/--change

Posted by "Brian W. Fitzpatrick" <fi...@red-bean.com>.
On Jan 13, 2006, at 11:21 AM, Michael Brouwer wrote:

> I think this was mentioned before, but rather than adding more
> switches why not do what svk does and have
> -c NEGATIVE_NUMBER
> imply the reverse of NUMBER.

I really like this idea.  It sort of implies "Subtract this revision".

-Fitz

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

Re: [PATCH] --reverse option to invert -c/--change

Posted by Bart Robinson <lo...@pobox.com>.
I think the argument against that was that it would complicate
arg parsing.
It is covered in one of these threads, I think the 2nd:
  http://svn.haxx.se/dev/archive-2005-10/1199.shtml
  http://svn.haxx.se/dev/archive-2005-10/1324.shtml
  http://svn.haxx.se/dev/archive-2005-10/1334.shtml
sorry I don't have the exact link handy.

-- bart

On 2006-1-13 Michael Brouwer <mb...@gmail.com> wrote:
 > I think this was mentioned before, but rather than adding more
 > switches why not do what svk does and have
 > -c NEGATIVE_NUMBER
 > imply the reverse of NUMBER.
 > 
 > Michael.
 > 
 > On 1/13/06, Julian Foad <ju...@btopenworld.com> wrote:
 > > Bart Robinson wrote:
 > > > Hi,
 > > > Here is a patch to help flesh out the utility of the -c/--change
 > > > option.

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

Re: [PATCH] --reverse option to invert -c/--change

Posted by Michael Brouwer <mb...@gmail.com>.
I think this was mentioned before, but rather than adding more
switches why not do what svk does and have
-c NEGATIVE_NUMBER
imply the reverse of NUMBER.

Michael.

On 1/13/06, Julian Foad <ju...@btopenworld.com> wrote:
> Bart Robinson wrote:
> > Hi,
> > Here is a patch to help flesh out the utility of the -c/--change
> > option.

Re: [PATCH] --reverse option to invert -c/--change

Posted by Julian Foad <ju...@btopenworld.com>.
Bart Robinson wrote:
> Hi,
> Here is a patch to help flesh out the utility of the -c/--change
> option.
> 
> Some background in this thread:
> http://svn.haxx.se/dev/archive-2005-10/1324.shtml
> (should I put that in the commit message?)
> 
> E.g.,
>   svn merge -c 100 --reverse URL
> same as
>   svn merge -r100:99 URL

It seems to me fairly certain that we want this ability to reverse the 
direction of a change.

"--reverse" is perhaps too generic a name for the option: we might well want to 
reverse other things than the direction of a change, and it seems unlikely that 
it will make sense to share a common option name for reversing completely 
different things.  We should consider a more specific option name like 
"--reverse-change".

If the switch will only apply to "-c" then we could make it an alternative to 
"-c" rather than an addition to it: e.g. "--reverse-change REVNUM" (possibly 
with a short name "-C REVNUM").

Do we want the reversal switch to apply to "-r" as well?  That is not necessary 
in the basic cases (such as "-r X:Y") but we might decide it is convenient and 
consistent.  Indeed it might enable some desirable diffs that are not possible 
at present: like the reverse of a plain diff (WC against BASE -> BASE against 
WC).  (Am I right about that?  If so, a better solution to that particular 
problem might be to introduce a "WORKING" or "WC" revision-keyword.)

Thoughts?

- Julian

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